Change semantics of driver install and uninstall in CSINodeInfo to use new fields.

cluster/addons/storage-crds/csinodeinfo.yaml

@@ -13,11 +13,15 @@ spec:
   validation:
     openAPIV3Schema:
       properties:
-        csiDrivers:
+        spec:
-          description: List of CSI drivers running on the node and their properties.
+          description: Specification of CSINodeInfo
+          properties:
+            drivers:
+              description: List of CSI drivers running on the node and their specs.
+              type: array
               items:
                 properties:
-              driver:
+                  name:
                     description: The CSI driver that this object refers to.
                     type: string
                   nodeID:
@@ -28,5 +32,23 @@ spec:
                     items:
                       type: string
                     type: array
+        status:
+          description: Status of CSINodeInfo
+          properties:
+            drivers:
+              description: List of CSI drivers running on the node and their statuses.
               type: array
+              items:
+                properties:
+                  name:
+                    description: The CSI driver that this object refers to.
+                    type: string
+                  available:
+                    description: Whether the CSI driver is installed.
+                    type: boolean
+                  volumePluginMechanism:
+                    description: Indicates to external components the required mechanism
+                      to use for any in-tree plugins replaced by this driver.
+                    pattern: in-tree|csi
+                    type: string
   version: v1alpha1

pkg/volume/csi/csi_plugin.go

@@ -146,7 +146,7 @@ func (h *RegistrationHandler) RegisterPlugin(pluginName string, endpoint string)
 		return err
 	}
 
-	err = nim.AddNodeInfo(pluginName, driverNodeID, maxVolumePerNode, accessibleTopology)
+	err = nim.InstallCSIDriver(pluginName, driverNodeID, maxVolumePerNode, accessibleTopology)
 	if err != nil {
 		glog.Error(log("registrationHandler.RegisterPlugin failed at AddNodeInfo: %v", err))
 		if unregErr := unregisterDriver(pluginName); unregErr != nil {
@@ -188,6 +188,9 @@ func (p *csiPlugin) Init(host volume.VolumeHost) error {
 	csiDrivers = csiDriversStore{driversMap: map[string]csiDriver{}}
 	nim = nodeinfomanager.NewNodeInfoManager(host.GetNodeName(), host)
 
+	// TODO(#70514) Init CSINodeInfo object if the CRD exists and create Driver
+	// objects for migrated drivers.
+
 	return nil
 }
 
@@ -583,8 +586,8 @@ func unregisterDriver(driverName string) error {
 		delete(csiDrivers.driversMap, driverName)
 	}()
 
-	if err := nim.RemoveNodeInfo(driverName); err != nil {
+	if err := nim.UninstallCSIDriver(driverName); err != nil {
-		glog.Errorf("Error unregistering CSI driver: %v", err)
+		glog.Errorf("Error uninstalling CSI driver: %v", err)
 		return err
 	}
 

pkg/volume/csi/nodeinfomanager/BUILD

@@ -49,7 +49,6 @@ go_test(
         "//pkg/volume/testing:go_default_library",
         "//pkg/volume/util:go_default_library",
         "//staging/src/k8s.io/api/core/v1:go_default_library",
-        "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/api/resource:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library",

pkg/volume/csi/nodeinfomanager/nodeinfomanager.go

@@ -58,15 +58,17 @@ type nodeUpdateFunc func(*v1.Node) (newNode *v1.Node, updated bool, err error)
 
 // Interface implements an interface for managing labels of a node
 type Interface interface {
+	CreateCSINodeInfo() (*csiv1alpha1.CSINodeInfo, error)
+
 	// Record in the cluster the given node information from the CSI driver with the given name.
-	// Concurrent calls to AddNodeInfo() is allowed, but they should not be intertwined with calls
+	// Concurrent calls to InstallCSIDriver() is allowed, but they should not be intertwined with calls
 	// to other methods in this interface.
-	AddNodeInfo(driverName string, driverNodeID string, maxVolumeLimit int64, topology *csipb.Topology) error
+	InstallCSIDriver(driverName string, driverNodeID string, maxVolumeLimit int64, topology *csipb.Topology) error
 
 	// Remove in the cluster node information from the CSI driver with the given name.
-	// Concurrent calls to RemoveNodeInfo() is allowed, but they should not be intertwined with calls
+	// Concurrent calls to UninstallCSIDriver() is allowed, but they should not be intertwined with calls
 	// to other methods in this interface.
-	RemoveNodeInfo(driverName string) error
+	UninstallCSIDriver(driverName string) error
 }
 
 // NewNodeInfoManager initializes nodeInfoManager
@@ -79,11 +81,11 @@ func NewNodeInfoManager(
 	}
 }
 
-// AddNodeInfo updates the node ID annotation in the Node object and CSIDrivers field in the
+// InstallCSIDriver updates the node ID annotation in the Node object and CSIDrivers field in the
 // CSINodeInfo object. If the CSINodeInfo object doesn't yet exist, it will be created.
-// If multiple calls to AddNodeInfo() are made in parallel, some calls might receive Node or
+// If multiple calls to InstallCSIDriver() are made in parallel, some calls might receive Node or
 // CSINodeInfo update conflicts, which causes the function to retry the corresponding update.
-func (nim *nodeInfoManager) AddNodeInfo(driverName string, driverNodeID string, maxAttachLimit int64, topology *csipb.Topology) error {
+func (nim *nodeInfoManager) InstallCSIDriver(driverName string, driverNodeID string, maxAttachLimit int64, topology *csipb.Topology) error {
 	if driverNodeID == "" {
 		return fmt.Errorf("error adding CSI driver node info: driverNodeID must not be empty")
 	}
@@ -114,14 +116,14 @@ func (nim *nodeInfoManager) AddNodeInfo(driverName string, driverNodeID string,
 	return nil
 }
 
-// RemoveNodeInfo removes the node ID annotation from the Node object and CSIDrivers field from the
+// UninstallCSIDriver removes the node ID annotation from the Node object and CSIDrivers field from the
 // CSINodeInfo object. If the CSINOdeInfo object contains no CSIDrivers, it will be deleted.
-// If multiple calls to RemoveNodeInfo() are made in parallel, some calls might receive Node or
+// If multiple calls to UninstallCSIDriver() are made in parallel, some calls might receive Node or
 // CSINodeInfo update conflicts, which causes the function to retry the corresponding update.
-func (nim *nodeInfoManager) RemoveNodeInfo(driverName string) error {
+func (nim *nodeInfoManager) UninstallCSIDriver(driverName string) error {
-	err := nim.removeCSINodeInfo(driverName)
+	err := nim.uninstallDriverFromCSINodeInfo(driverName)
 	if err != nil {
-		return fmt.Errorf("error removing CSI driver node info from CSINodeInfo object %v", err)
+		return fmt.Errorf("error uninstalling CSI driver from CSINodeInfo object %v", err)
 	}
 
 	err = nim.updateNode(
@@ -333,13 +335,13 @@ func (nim *nodeInfoManager) updateCSINodeInfo(
 	retryErr := retry.RetryOnConflict(retry.DefaultRetry, func() error {
 		nodeInfo, err := csiKubeClient.CsiV1alpha1().CSINodeInfos().Get(string(nim.nodeName), metav1.GetOptions{})
 		if nodeInfo == nil || errors.IsNotFound(err) {
-			return nim.createNodeInfoObject(driverName, driverNodeID, topology)
+			nodeInfo, err = nim.CreateCSINodeInfo()
 		}
 		if err != nil {
 			return err // do not wrap error
 		}
 
-		return nim.updateNodeInfoObject(nodeInfo, driverName, driverNodeID, topology)
+		return nim.installDriverToCSINodeInfo(nodeInfo, driverName, driverNodeID, topology)
 	})
 	if retryErr != nil {
 		return fmt.Errorf("CSINodeInfo update failed: %v", retryErr)
@@ -347,31 +349,21 @@ func (nim *nodeInfoManager) updateCSINodeInfo(
 	return nil
 }
 
-func (nim *nodeInfoManager) createNodeInfoObject(
+func (nim *nodeInfoManager) CreateCSINodeInfo() (*csiv1alpha1.CSINodeInfo, error) {
-	driverName string,
-	driverNodeID string,
-	topology *csipb.Topology) error {
 
 	kubeClient := nim.volumeHost.GetKubeClient()
 	if kubeClient == nil {
-		return fmt.Errorf("error getting kube client")
+		return nil, fmt.Errorf("error getting kube client")
 	}
 
 	csiKubeClient := nim.volumeHost.GetCSIClient()
 	if csiKubeClient == nil {
-		return fmt.Errorf("error getting CSI client")
+		return nil, fmt.Errorf("error getting CSI client")
-	}
-
-	topologyKeys := []string{} // must be an empty slice instead of nil to satisfy CRD OpenAPI Schema validation
-	if topology != nil {
-		for k := range topology.Segments {
-			topologyKeys = append(topologyKeys, k)
-		}
 	}
 
 	node, err := kubeClient.CoreV1().Nodes().Get(string(nim.nodeName), metav1.GetOptions{})
 	if err != nil {
-		return err // do not wrap error
+		return nil, err // do not wrap error
 	}
 
 	nodeInfo := &csiv1alpha1.CSINodeInfo{
@@ -386,20 +378,18 @@ func (nim *nodeInfoManager) createNodeInfoObject(
 				},
 			},
 		},
-		CSIDrivers: []csiv1alpha1.CSIDriverInfo{
+		Spec: csiv1alpha1.CSINodeInfoSpec{
-			{
+			Drivers: []csiv1alpha1.CSIDriverInfoSpec{},
-				Driver:       driverName,
-				NodeID:       driverNodeID,
-				TopologyKeys: topologyKeys,
 		},
+		Status: csiv1alpha1.CSINodeInfoStatus{
+			Drivers: []csiv1alpha1.CSIDriverInfoStatus{},
 		},
 	}
 
-	_, err = csiKubeClient.CsiV1alpha1().CSINodeInfos().Create(nodeInfo)
+	return csiKubeClient.CsiV1alpha1().CSINodeInfos().Create(nodeInfo)
-	return err // do not wrap error
 }
 
-func (nim *nodeInfoManager) updateNodeInfoObject(
+func (nim *nodeInfoManager) installDriverToCSINodeInfo(
 	nodeInfo *csiv1alpha1.CSINodeInfo,
 	driverName string,
 	driverNodeID string,
@@ -417,36 +407,62 @@ func (nim *nodeInfoManager) updateNodeInfoObject(
 		}
 	}
 
-	// Clone driver list, omitting the driver that matches the given driverName,
+	specModified := true
-	// unless the driver is identical to information provided, in which case no update is necessary.
+	statusModified := true
-	var newDriverInfos []csiv1alpha1.CSIDriverInfo
+	// Clone driver list, omitting the driver that matches the given driverName
-	for _, driverInfo := range nodeInfo.CSIDrivers {
+	newDriverSpecs := []csiv1alpha1.CSIDriverInfoSpec{}
-		if driverInfo.Driver == driverName {
+	for _, driverInfoSpec := range nodeInfo.Spec.Drivers {
-			prevTopologyKeys := sets.NewString(driverInfo.TopologyKeys...)
+		if driverInfoSpec.Name == driverName {
-			if driverInfo.NodeID == driverNodeID && prevTopologyKeys.Equal(topologyKeys) {
+			if driverInfoSpec.NodeID == driverNodeID &&
-				// No update needed
+				sets.NewString(driverInfoSpec.TopologyKeys...).Equal(topologyKeys) {
-				return nil
+				specModified = false
 			}
 		} else {
-			// Omit driverInfo matching given driverName
+			// Omit driverInfoSpec matching given driverName
-			newDriverInfos = append(newDriverInfos, driverInfo)
+			newDriverSpecs = append(newDriverSpecs, driverInfoSpec)
+		}
+	}
+	newDriverStatuses := []csiv1alpha1.CSIDriverInfoStatus{}
+	for _, driverInfoStatus := range nodeInfo.Status.Drivers {
+		if driverInfoStatus.Name == driverName {
+			if driverInfoStatus.Available &&
+				/* TODO(https://github.com/kubernetes/enhancements/issues/625): Add actual migration status */
+				driverInfoStatus.VolumePluginMechanism == csiv1alpha1.VolumePluginMechanismInTree {
+				statusModified = false
+			}
+		} else {
+			// Omit driverInfoSpec matching given driverName
+			newDriverStatuses = append(newDriverStatuses, driverInfoStatus)
 		}
 	}
 
+	if !specModified && !statusModified {
+		return nil
+	}
+
 	// Append new driver
-	driverInfo := csiv1alpha1.CSIDriverInfo{
+	driverSpec := csiv1alpha1.CSIDriverInfoSpec{
-		Driver:       driverName,
+		Name:         driverName,
 		NodeID:       driverNodeID,
 		TopologyKeys: topologyKeys.List(),
 	}
-	newDriverInfos = append(newDriverInfos, driverInfo)
+	driverStatus := csiv1alpha1.CSIDriverInfoStatus{
-	nodeInfo.CSIDrivers = newDriverInfos
+		Name:      driverName,
+		Available: true,
+		// TODO(https://github.com/kubernetes/enhancements/issues/625): Add actual migration status
+		VolumePluginMechanism: csiv1alpha1.VolumePluginMechanismInTree,
+	}
+
+	newDriverSpecs = append(newDriverSpecs, driverSpec)
+	newDriverStatuses = append(newDriverStatuses, driverStatus)
+	nodeInfo.Spec.Drivers = newDriverSpecs
+	nodeInfo.Status.Drivers = newDriverStatuses
 
 	_, err := csiKubeClient.CsiV1alpha1().CSINodeInfos().Update(nodeInfo)
 	return err // do not wrap error
 }
 
-func (nim *nodeInfoManager) removeCSINodeInfo(csiDriverName string) error {
+func (nim *nodeInfoManager) uninstallDriverFromCSINodeInfo(csiDriverName string) error {
 	retryErr := retry.RetryOnConflict(retry.DefaultRetry, func() error {
 
 		csiKubeClient := nim.volumeHost.GetCSIClient()
@@ -456,32 +472,28 @@ func (nim *nodeInfoManager) removeCSINodeInfo(csiDriverName string) error {
 
 		nodeInfoClient := csiKubeClient.CsiV1alpha1().CSINodeInfos()
 		nodeInfo, err := nodeInfoClient.Get(string(nim.nodeName), metav1.GetOptions{})
-		if nodeInfo == nil || errors.IsNotFound(err) {
-			// do nothing
-			return nil
-		}
 		if err != nil {
 			return err // do not wrap error
 		}
 
-		// Remove matching driver from driver list
+		hasModified := false
-		var newDriverInfos []csiv1alpha1.CSIDriverInfo
+		newDriverStatuses := []csiv1alpha1.CSIDriverInfoStatus{}
-		for _, driverInfo := range nodeInfo.CSIDrivers {
+		for _, driverStatus := range nodeInfo.Status.Drivers {
-			if driverInfo.Driver != csiDriverName {
+			if driverStatus.Name == csiDriverName {
-				newDriverInfos = append(newDriverInfos, driverInfo)
+				// Uninstall the driver if we find it
+				hasModified = driverStatus.Available
+				driverStatus.Available = false
 			}
+			newDriverStatuses = append(newDriverStatuses, driverStatus)
 		}
 
-		if len(newDriverInfos) == len(nodeInfo.CSIDrivers) {
+		nodeInfo.Status.Drivers = newDriverStatuses
+
+		if !hasModified {
 			// No changes, don't update
 			return nil
 		}
 
-		if len(newDriverInfos) == 0 {
-			// No drivers left, delete CSINodeInfo object
-			return nodeInfoClient.Delete(string(nim.nodeName), &metav1.DeleteOptions{})
-		}
-
 		// TODO (verult) make sure CSINodeInfo has validation logic to prevent duplicate driver names
 		_, updateErr := nodeInfoClient.Update(nodeInfo)
 		return updateErr // do not wrap error

staging/src/k8s.io/csi-api/pkg/apis/csi/v1alpha1/README.md

@@ -14,10 +14,11 @@ cluster and it is registered through the Kubelet device registration mechanism.
 
 ## Who updates CSIDriverInfo Spec and when
 
-The CSIDriverInfoSpec ror a driver is created upon installation of the CSI
+The CSIDriverInfoSpec for a driver is created upon installation of the CSI
 Driver to the cluster and it is registered through the Kubelet device
 registration mechanism. The spec is populated with information about the driver
-through the nodeinfomanager and will remain unchanged from then on.
+through the nodeinfomanager (inside Kubelet) and will remain unchanged from then
+on.
 
 ## Who updates Status and when