Remove vsphere credentials from csi node daemonset

[RaunakShah]

What this PR does / why we need it:
Changing the default to remove csi-vsphere.conf as a mounted volume to node daemonset. This is only needed for topology aware clusters.
Also addressing an issue where the node daemonset would fail if this volume was not mounted.

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #
This PR addresses #95

Special notes for your reviewer:

Testing:
For non-topology aware setups:

Logs showing node daemonset can successfully register with kubelet

{"log":"I1205 20:21:52.304096       1 service.go:88] configured: csi.vsphere.vmware.com with map[mode:node]\n","stream":"stderr","time":"2019-12-05T20:21:52.307895822Z"}
{"log":"time=\"2019-12-05T20:21:52Z\" level=info msg=\"identity service registered\"\n","stream":"stderr","time":"2019-12-05T20:21:52.308266734Z"}
{"log":"time=\"2019-12-05T20:21:52Z\" level=info msg=\"node service registered\"\n","stream":"stderr","time":"2019-12-05T20:21:52.308275623Z"}
{"log":"time=\"2019-12-05T20:21:52Z\" level=info msg=serving endpoint=\"unix:///csi/csi.sock\"\n","stream":"stderr","time":"2019-12-05T20:21:52.308279094Z"}
{"log":"I1205 20:21:54.591428       1 config.go:261] GetCnsconfig called with cfgPath: /etc/cloud/csi-vsphere.conf\n","stream":"stderr","time":"2019-12-05T20:21:54.591716655Z"}
{"log":"I1205 20:21:54.591838       1 config.go:265] Could not stat /etc/cloud/csi-vsphere.conf, reading config params from env\n","stream":"stderr","time":"2019-12-05T20:21:54.592128802Z"}
{"log":"E1205 20:21:54.592305       1 config.go:202] No Virtual Center hosts defined\n","stream":"stderr","time":"2019-12-05T20:21:54.592819448Z"}
{"log":"E1205 20:21:54.592936       1 config.go:269] Failed to get config params from env. Err: No Virtual Center hosts defined\n","stream":"stderr","time":"2019-12-05T20:21:54.594325039Z"}
{"log":"I1205 20:21:54.593568       1 node.go:367] Config file not provided to node daemonset. Assuming non-topology aware cluster\n","stream":"stderr","time":"2019-12-05T20:21:54.594349877Z"}

Create a PVC and POD

root@k8-master-642:~# kubectl get pvc
NAME                        STATUS   VOLUME                                     CAPACITY   ACCESS MODES   STORAGECLASS               AGE
example-vanilla-block-pvc   Bound    pvc-924d23cd-163b-11ea-ad42-00505691fe6a   1Mi        RWO            example-vanilla-block-sc   42h
root@k8-master-642:~# kubectl get pod
NAME                        READY   STATUS    RESTARTS   AGE
example-vanilla-block-pod   1/1     Running   0          2m59s

For topology aware setups:

Logs showing node daemonset can successfully register with kubelet

{"log":"I1206 00:57:06.057890       1 service.go:88] configured: csi.vsphere.vmware.com with map[mode:node]\n","stream":"stderr","time":"2019-12-06T00:57:06.075632954Z"}
{"log":"time=\"2019-12-06T00:57:06Z\" level=info msg=\"identity service registered\"\n","stream":"stderr","time":"2019-12-06T00:57:06.075952709Z"}
{"log":"time=\"2019-12-06T00:57:06Z\" level=info msg=\"node service registered\"\n","stream":"stderr","time":"2019-12-06T00:57:06.075968483Z"}
{"log":"time=\"2019-12-06T00:57:06Z\" level=info msg=serving endpoint=\"unix:///csi/csi.sock\"\n","stream":"stderr","time":"2019-12-06T00:57:06.075972504Z"}
{"log":"I1206 00:57:08.113064       1 config.go:261] GetCnsconfig called with cfgPath: /etc/cloud/csi-vsphere.conf\n","stream":"stderr","time":"2019-12-06T00:57:08.121567715Z"}
{"log":"I1206 00:57:08.114773       1 config.go:206] Initializing vc server 10.160.122.172\n","stream":"stderr","time":"2019-12-06T00:57:08.121621019Z"}
{"log":"I1206 00:57:08.114914       1 node.go:379] Config file provided with Zone and Region. Assuming topology aware cluster.\n","stream":"stderr","time":"2019-12-06T00:57:08.121628155Z”}

csinodes CRD contains zone and region keys

root@k8s-master:~# kubectl describe csinodes
Name:         k8s-node1
Namespace:    
Labels:       <none>
Annotations:  <none>
API Version:  storage.k8s.io/v1beta1
Kind:         CSINode
Metadata:
  Creation Timestamp:  2019-12-06T00:57:09Z
  Owner References:
    API Version:     v1
    Kind:            Node
    Name:            k8s-node1
    UID:             b2c6642d-a5a7-43f4-bcd6-9017cbb751cf
  Resource Version:  15588
  Self Link:         /apis/storage.k8s.io/v1beta1/csinodes/k8s-node1
  UID:               ef0a8c72-d613-4d1b-96d6-0fe8ce497daa
Spec:
  Drivers:
    Name:     csi.vsphere.vmware.com
    Node ID:  k8s-node1
    Topology Keys:
      failure-domain.beta.kubernetes.io/region
      failure-domain.beta.kubernetes.io/zone
Events:  <none>

Create PVC and Pod:

root@k8s-master:~# kubectl get pvc
NAME                        STATUS   VOLUME                                     CAPACITY   ACCESS MODES   STORAGECLASS               AGE
example-vanilla-block-pvc   Bound    pvc-5d61d5ba-9fe6-409a-be32-c773c4c77089   5Gi        RWO            example-vanilla-block-sc   16s
root@k8s-master:~# kubectl get pod
NAME                        READY   STATUS    RESTARTS   AGE
example-vanilla-block-pod   1/1     Running   0          21s

Release note:

Changing the default to remove `csi-vsphere.conf` as a mounted volume to node daemonset. 

[k8s-ci-robot]

Hi @RaunakShah. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[RaunakShah]

/assign @divyenpatel

[divyenpatel]

/lgtm

[divyenpatel]

/hold

[divyenpatel]

@RaunakShah manifest points to the older images which does not have this fix. Do we plan to cut new release on github for this change?

@manojvs157 @SandeepPissay when can we cut a new release on github for this fix? This issue was reported by @misterikkit as well.

[misterikkit]

Based on #86 it seems like there is a chicken-egg problem where the images can't be tagged and released until after the fix is merged. So there would need to be a follow-up PR to update the manifests?

[SandeepPissay]

@RaunakShah Could you please describe details of testing you have performed to validate that this does not break the existing functionality? It will be nice if you can update it in the PR description. Thanks!

[manojvs157]

@RaunakShah manifest points to the older images which does not have this fix. Do we plan to cut new release on github for this change?

@manojvs157 @SandeepPissay when can we cut a new release on github for this fix? This issue was reported by @misterikkit as well.

Another issue for forward compatibility was also observed. Let's may be collect all must fix issues and then cut a release.

@RaunakShah did you run this fix through topology aware testbed?

[RaunakShah]

@divyenpatel wow that completely went above me!

I agree that we can cut a release after merging this and other known issues/enhancements like #72
What about i remove the diff in the YAMLs and leave the change in source code? We can cut a release when we're ready.

[RaunakShah]

@SandeepPissay @manojvs157 Let me post the logs (for topology and non-topology aware setups), somehow missed adding them to the description.

[divyenpatel]

What about i remove the diff in the YAMLs and leave the change in source code?

@RaunakShah that sounds like a good idea. Let’s check-in the needed code without updating manifest. We will bump up images in the manifest and remove mounted config volume in the follow up PR.

[RaunakShah]

@divyenpatel can you take a look?

[divyenpatel]

/ok-to-test

[divyenpatel]

/lgtm

[divyenpatel]

/hold cancel

[dougm]

/lgtm

[divyenpatel]

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: divyenpatel, RaunakShah

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [divyenpatel]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment