Repository: kubernetes/kubernetes (language: go)

Issue:
csi.volume.kubernetes.io/nodeid node annotation is not properly set when multiple plugins register at the same time
**What happened**:

I added two CSI plugins to my Kubernetes cluster via a single call to `kubectl apply`. When I did this, the `csi.volume.kubernetes.io/nodeid` annotation was improperly set on 2 out of 3 nodes. Only one of the plugins appeared:

```
apiVersion: v1
kind: Node
metadata:
  annotations:
    csi.volume.kubernetes.io/nodeid: '{"cephfs.csi.ceph.com":"ip-172-31-17-163"}'
```

**What you expected to happen**:

Both cephfs.csi.ceph.com and rbd.csi.ceph.com plugins should have appeared in the csi.volume.kubernetes.io/nodeid annotation.

**How to reproduce it (as minimally and precisely as possible)**:

Apply two or more CSI plugins via a single call to `kubectl apply`.

**Anything else we need to know?**:

Kubelet logs indicate that it registered both plugins at the same time:

```
I0831 16:54:43.682576   61626 operation_generator.go:195] parsed scheme: ""
I0831 16:54:43.682617   61626 operation_generator.go:195] scheme "" not registered, fallback to default scheme
I0831 16:54:43.682638   61626 passthrough.go:48] ccResolverWrapper: sending update to cc: {[{/var/lib/kubelet/plugins_registry/rbd.csi.ceph.com-reg.sock 0  <nil>}] <nil>}
I0831 16:54:43.682655   61626 clientconn.go:577] ClientConn switching balancer to "pick_first"
I0831 16:54:43.682739   61626 operation_generator.go:195] parsed scheme: ""
I0831 16:54:43.682757   61626 operation_generator.go:195] scheme "" not registered, fallback to default scheme
I0831 16:54:43.682774   61626 passthrough.go:48] ccResolverWrapper: sending update to cc: {[{/var/lib/kubelet/plugins_registry/cephfs.csi.ceph.com-reg.sock 0  <nil>}] <nil>}
I0831 16:54:43.682783   61626 clientconn.go:577] ClientConn switching balancer to "pick_first"
I0831 16:54:43.683658   61626 csi_plugin.go:99] kubernetes.io/csi: Trying to validate a new CSI Driver with name: rbd.csi.ceph.com endpoint: /var/lib/kubelet/plugins/rbd.csi.ceph.com/csi.sock versions: 1.0.0
I0831 16:54:43.683698   61626 csi_plugin.go:112] kubernetes.io/csi: Register new plugin with name: rbd.csi.ceph.com at endpoint: /var/lib/kubelet/plugins/rbd.csi.ceph.com/csi.sock
I0831 16:54:43.683729   61626 clientconn.go:104] parsed scheme: ""
I0831 16:54:43.683738   61626 clientconn.go:104] scheme "" not registered, fallback to default scheme
I0831 16:54:43.683759   61626 csi_plugin.go:99] kubernetes.io/csi: Trying to validate a new CSI Driver with name: cephfs.csi.ceph.com endpoint: /var/lib/kubelet/plugins/cephfs.csi.ceph.com/csi.sock versions: 1.0.0
I0831 16:54:43.683780   61626 passthrough.go:48] ccResolverWrapper: sending update to cc: {[{/var/lib/kubelet/plugins/rbd.csi.ceph.com/csi.sock 0  <nil>}] <nil>}
I0831 16:54:43.683789   61626 clientconn.go:577] ClientConn switching balancer to "pick_first"
I0831 16:54:43.683838   61626 csi_plugin.go:112] kubernetes.io/csi: Register new plugin with name: cephfs.csi.ceph.com at endpoint: /var/lib/kubelet/plugins/cephfs.csi.ceph.com/csi.sock
I0831 16:54:43.683866   61626 clientconn.go:104] parsed scheme: ""
I0831 16:54:43.683875   61626 clientconn.go:104] scheme "" not registered, fallback to default scheme
I0831 16:54:43.683927   61626 passthrough.go:48] ccResolverWrapper: sending update to cc: {[{/var/lib/kubelet/plugins/cephfs.csi.ceph.com/csi.sock 0  <nil>}] <nil>}
I0831 16:54:43.683937   61626 clientconn.go:577] ClientConn switching balancer to "pick_first"
E0831 16:54:43.713415   61626 nodeinfomanager.go:567] Invalid attach limit value 0 cannot be added to CSINode object for "rbd.csi.ceph.com"
E0831 16:54:43.834273   61626 nodeinfomanager.go:567] Invalid attach limit value 0 cannot be added to CSINode object for "cephfs.csi.ceph.com"
```

The CSINode object looks complete:

```
apiVersion: storage.k8s.io/v1
kind: CSINode
metadata:
  name: ip-172-31-17-163
  ...
spec:
  drivers:
  - name: rbd.csi.ceph.com
    nodeID: ip-172-31-17-163
    topologyKeys: null
  - name: cephfs.csi.ceph.com
    nodeID: ip-172-31-17-163
    topologyKeys: null
```

Only the node's csi.volume.kubernetes.io/nodeid annotation did not include rbd.csi.ceph.com.

Restarting kubelet fixed the issue.

It looks like a race condition when InstallCSIDriver is called concurrently for multiple drivers. The code here: https://github.com/kubernetes/kubernetes/blob/a28a6929174b4cfdebb2d3d95f69d3a299097667/pkg/volume/csi/nodeinfomanager/nodeinfomanager.go#L187-L206

could lead to each thread observing the csi.volume.kubernetes.io/nodeid annotation as being empty, and then setting its own annotation that includes only itself.

**Environment**:
- Kubernetes version (use `kubectl version`): v1.17.11
- Cloud provider or hardware configuration: AWS
- OS (e.g: `cat /etc/os-release`): Ubuntu 20.04.1 LTS
- Kernel (e.g. `uname -a`): Linux ip-172-31-17-163 5.4.0-1021-aws #21-Ubuntu SMP Fri Jul 24 09:42:29 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
- Install tools: juju


Discussion / hints:
/sig storage
/assign
I can reproduce this issue with commands below

```
kubectl get no dce-10-7-177-14 -o yaml  | grep csi.volume.kubernetes.io
kubectl annotate node dce-10-7-177-14  csi.volume.kubernetes.io/nodeid-
kubectl get no dce-10-7-177-14 -o yaml  | grep csi.volume.kubernetes.io
kubectl get pod -A -o wide | grep  csi | grep dce-10-7-177-14 | grep driver | awk '{print $2}' | xargs kubectl delete pod -n storage-system --force --grace-period=0
kubectl get no dce-10-7-177-14 -o yaml  | grep csi.volume.kubernetes.io
```
![image](https://user-images.githubusercontent.com/2010320/91951081-251d1780-ed33-11ea-8fa9-6869c0416794.png)

The correct csi nodeid would be like this
```
    csi.volume.kubernetes.io/nodeid: '{"csi.dvol.storage.daocloud.io":"dce-10-7-177-14","iscsi.csi.xsky.com":"dce-10-7-177-14"}'
```


Fix patch:
diff --git a/pkg/volume/csi/nodeinfomanager/nodeinfomanager.go b/pkg/volume/csi/nodeinfomanager/nodeinfomanager.go
index 333c55792db49..3d348a35b254a 100644
--- a/pkg/volume/csi/nodeinfomanager/nodeinfomanager.go
+++ b/pkg/volume/csi/nodeinfomanager/nodeinfomanager.go
@@ -379,6 +379,9 @@ func (nim *nodeInfoManager) tryUpdateCSINode(
 	maxAttachLimit int64,
 	topology map[string]string) error {
 
+	nim.lock.Lock()
+	defer nim.lock.Unlock()
+
 	nodeInfo, err := csiKubeClient.StorageV1().CSINodes().Get(context.TODO(), string(nim.nodeName), metav1.GetOptions{})
 	if nodeInfo == nil || errors.IsNotFound(err) {
 		nodeInfo, err = nim.CreateCSINode()
@@ -412,6 +415,9 @@ func (nim *nodeInfoManager) InitializeCSINodeWithAnnotation() error {
 }
 
 func (nim *nodeInfoManager) tryInitializeCSINodeWithAnnotation(csiKubeClient clientset.Interface) error {
+	nim.lock.Lock()
+	defer nim.lock.Unlock()
+
 	nodeInfo, err := csiKubeClient.StorageV1().CSINodes().Get(context.TODO(), string(nim.nodeName), metav1.GetOptions{})
 	if nodeInfo == nil || errors.IsNotFound(err) {
 		// CreateCSINode will set the annotation
@@ -602,6 +608,9 @@ func (nim *nodeInfoManager) tryUninstallDriverFromCSINode(
 	csiKubeClient clientset.Interface,
 	csiDriverName string) error {
 
+	nim.lock.Lock()
+	defer nim.lock.Unlock()
+
 	nodeInfoClient := csiKubeClient.StorageV1().CSINodes()
 	nodeInfo, err := nodeInfoClient.Get(context.TODO(), string(nim.nodeName), metav1.GetOptions{})
 	if err != nil && errors.IsNotFound(err) {


Test patch: