From 06f3b26012e09318935c248e62864e9349da6cd8 Mon Sep 17 00:00:00 2001 From: David Zhu Date: Thu, 8 Nov 2018 13:13:14 -0800 Subject: [PATCH] Change semantics of driver install and uninstall in CSINodeInfo to use new fields. --- cluster/addons/storage-crds/csinodeinfo.yaml | 54 +++++-- pkg/volume/csi/csi_plugin.go | 9 +- pkg/volume/csi/nodeinfomanager/BUILD | 1 - .../csi/nodeinfomanager/nodeinfomanager.go | 148 ++++++++++-------- .../csi-api/pkg/apis/csi/v1alpha1/README.md | 5 +- 5 files changed, 127 insertions(+), 90 deletions(-) diff --git a/cluster/addons/storage-crds/csinodeinfo.yaml b/cluster/addons/storage-crds/csinodeinfo.yaml index a9556fc1e8..7b12ba5766 100644 --- a/cluster/addons/storage-crds/csinodeinfo.yaml +++ b/cluster/addons/storage-crds/csinodeinfo.yaml @@ -13,20 +13,42 @@ spec: validation: openAPIV3Schema: properties: - csiDrivers: - description: List of CSI drivers running on the node and their properties. - items: - properties: - driver: - description: The CSI driver that this object refers to. - type: string - nodeID: - description: The node from the driver point of view. - type: string - topologyKeys: - description: List of keys supported by the driver. - items: - type: string - type: array - type: array + spec: + description: Specification of CSINodeInfo + properties: + drivers: + description: List of CSI drivers running on the node and their specs. + type: array + items: + properties: + name: + description: The CSI driver that this object refers to. + type: string + nodeID: + description: The node from the driver point of view. + type: string + topologyKeys: + description: List of keys supported by the driver. + 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 diff --git a/pkg/volume/csi/csi_plugin.go b/pkg/volume/csi/csi_plugin.go index 4b8f97c3d6..5c8240b81c 100644 --- a/pkg/volume/csi/csi_plugin.go +++ b/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 { - glog.Errorf("Error unregistering CSI driver: %v", err) + if err := nim.UninstallCSIDriver(driverName); err != nil { + glog.Errorf("Error uninstalling CSI driver: %v", err) return err } diff --git a/pkg/volume/csi/nodeinfomanager/BUILD b/pkg/volume/csi/nodeinfomanager/BUILD index 3eadf06129..a240c7f589 100644 --- a/pkg/volume/csi/nodeinfomanager/BUILD +++ b/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", diff --git a/pkg/volume/csi/nodeinfomanager/nodeinfomanager.go b/pkg/volume/csi/nodeinfomanager/nodeinfomanager.go index 0a28d9c4ba..72057cae67 100644 --- a/pkg/volume/csi/nodeinfomanager/nodeinfomanager.go +++ b/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 { - err := nim.removeCSINodeInfo(driverName) +func (nim *nodeInfoManager) UninstallCSIDriver(driverName string) error { + 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( - driverName string, - driverNodeID string, - topology *csipb.Topology) error { +func (nim *nodeInfoManager) CreateCSINodeInfo() (*csiv1alpha1.CSINodeInfo, 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") - } - - 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) - } + return nil, fmt.Errorf("error getting CSI client") } 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{ - { - Driver: driverName, - NodeID: driverNodeID, - TopologyKeys: topologyKeys, - }, + Spec: csiv1alpha1.CSINodeInfoSpec{ + Drivers: []csiv1alpha1.CSIDriverInfoSpec{}, + }, + Status: csiv1alpha1.CSINodeInfoStatus{ + Drivers: []csiv1alpha1.CSIDriverInfoStatus{}, }, } - _, err = csiKubeClient.CsiV1alpha1().CSINodeInfos().Create(nodeInfo) - return err // do not wrap error + return csiKubeClient.CsiV1alpha1().CSINodeInfos().Create(nodeInfo) } -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, - // unless the driver is identical to information provided, in which case no update is necessary. - var newDriverInfos []csiv1alpha1.CSIDriverInfo - for _, driverInfo := range nodeInfo.CSIDrivers { - if driverInfo.Driver == driverName { - prevTopologyKeys := sets.NewString(driverInfo.TopologyKeys...) - if driverInfo.NodeID == driverNodeID && prevTopologyKeys.Equal(topologyKeys) { - // No update needed - return nil + specModified := true + statusModified := true + // Clone driver list, omitting the driver that matches the given driverName + newDriverSpecs := []csiv1alpha1.CSIDriverInfoSpec{} + for _, driverInfoSpec := range nodeInfo.Spec.Drivers { + if driverInfoSpec.Name == driverName { + if driverInfoSpec.NodeID == driverNodeID && + sets.NewString(driverInfoSpec.TopologyKeys...).Equal(topologyKeys) { + specModified = false } } else { - // Omit driverInfo matching given driverName - newDriverInfos = append(newDriverInfos, driverInfo) + // Omit driverInfoSpec matching given driverName + 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{ - Driver: driverName, + driverSpec := csiv1alpha1.CSIDriverInfoSpec{ + Name: driverName, NodeID: driverNodeID, TopologyKeys: topologyKeys.List(), } - newDriverInfos = append(newDriverInfos, driverInfo) - nodeInfo.CSIDrivers = newDriverInfos + driverStatus := csiv1alpha1.CSIDriverInfoStatus{ + 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 - var newDriverInfos []csiv1alpha1.CSIDriverInfo - for _, driverInfo := range nodeInfo.CSIDrivers { - if driverInfo.Driver != csiDriverName { - newDriverInfos = append(newDriverInfos, driverInfo) + hasModified := false + newDriverStatuses := []csiv1alpha1.CSIDriverInfoStatus{} + for _, driverStatus := range nodeInfo.Status.Drivers { + if driverStatus.Name == csiDriverName { + // 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 diff --git a/staging/src/k8s.io/csi-api/pkg/apis/csi/v1alpha1/README.md b/staging/src/k8s.io/csi-api/pkg/apis/csi/v1alpha1/README.md index d77a72925a..79a7ec17ee 100644 --- a/staging/src/k8s.io/csi-api/pkg/apis/csi/v1alpha1/README.md +++ b/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