Address review comments

jbhou · Nov 19, 2020 · 1a32327 · 1a32327
1 parent b6ae233
commit 1a32327
Show file tree

Hide file tree

Showing 4 changed files with 7 additions and 2 deletions.
diff --git a/staging/src/k8s.io/legacy-cloud-providers/vsphere/nodemanager.go b/staging/src/k8s.io/legacy-cloud-providers/vsphere/nodemanager.go
@@ -311,6 +311,8 @@ func (nm *NodeManager) GetNodeDetails() ([]NodeDetails, error) {
 	return nodeDetails, nil
 }
 
+// GetNodeNames returns list of nodes that are known to vsphere cloudprovider.
+// These are typically nodes that make up k8s cluster.
 func (nm *NodeManager) GetNodeNames() []k8stypes.NodeName {
 	nodes := nm.getNodes()
 	var nodeNameList []k8stypes.NodeName

diff --git a/staging/src/k8s.io/legacy-cloud-providers/vsphere/vsphere.go b/staging/src/k8s.io/legacy-cloud-providers/vsphere/vsphere.go
@@ -1221,7 +1221,7 @@ func (vs *VSphere) DisksAreAttached(nodeVolumes map[k8stypes.NodeName][]string)
 			}
 
 		}
-		klog.V(4).Infof("DisksAreAttach successfully executed. result: %+v", attached)
+		klog.V(4).Infof("DisksAreAttached successfully executed. result: %+v", attached)
 		// There could be nodes in cluster which do not have any pods with vsphere volumes running on them
 		// such nodes won't be part of nodeVolumes map because attach-detach controller does not keep track
 		// such nodes. But such nodes may still have dangling volumes on them and hence we need to scan all the

diff --git a/staging/src/k8s.io/legacy-cloud-providers/vsphere/vsphere_util.go b/staging/src/k8s.io/legacy-cloud-providers/vsphere/vsphere_util.go
@@ -636,14 +636,14 @@ func (vs *VSphere) BuildMissingVolumeNodeMap(ctx context.Context) {
 
 	for _, nodeNames := range dcNodes {
 		// Start go routines per VC-DC to check disks are attached
+		wg.Add(1)
 		go func(nodes []k8stypes.NodeName) {
 			err := vs.checkNodeDisks(ctx, nodeNames)
 			if err != nil {
 				klog.Errorf("Failed to check disk attached for nodes: %+v. err: %+v", nodes, err)
 			}
 			wg.Done()
 		}(nodeNames)
-		wg.Add(1)
 	}
 	wg.Wait()
 }

diff --git a/staging/src/k8s.io/legacy-cloud-providers/vsphere/vsphere_volume_map.go b/staging/src/k8s.io/legacy-cloud-providers/vsphere/vsphere_volume_map.go
@@ -74,6 +74,9 @@ func (vsphereVolume *VsphereVolumeMap) CheckForVolume(path string) (k8stypes.Nod
 	return "", false
 }
 
+// CheckForNode returns true if given node has already been processed by volume
+// verification mechanism. This is used to skip verifying attached disks on nodes
+// which were previously verified.
 func (vsphereVolume *VsphereVolumeMap) CheckForNode(nodeName k8stypes.NodeName) bool {
 	vsphereVolume.lock.RLock()
 	defer vsphereVolume.lock.RUnlock()