From 3595dee6ccc31364660ab8bb431370875b6d8343 Mon Sep 17 00:00:00 2001 From: David Zhu Date: Thu, 8 Nov 2018 13:12:35 -0800 Subject: [PATCH 1/3] Add fields available and volumePluginMechanism to CSINodeInfo CRD API Object. Split CSINodeInfo into Spec and Status. --- .../csi-api/pkg/apis/csi/v1alpha1/README.md | 51 ++++++++++++ .../csi-api/pkg/apis/csi/v1alpha1/types.go | 64 +++++++++++++-- .../csi/v1alpha1/zz_generated.deepcopy.go | 77 ++++++++++++++++--- .../typed/csi/v1alpha1/csinodeinfo.go | 16 ++++ .../csi/v1alpha1/fake/fake_csinodeinfo.go | 11 +++ .../pkg/crd/manifests/csinodeinfo.yaml | 54 +++++++++---- 6 files changed, 238 insertions(+), 35 deletions(-) create mode 100644 staging/src/k8s.io/csi-api/pkg/apis/csi/v1alpha1/README.md 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 new file mode 100644 index 0000000000..d77a72925a --- /dev/null +++ b/staging/src/k8s.io/csi-api/pkg/apis/csi/v1alpha1/README.md @@ -0,0 +1,51 @@ +# CSINodeInfo and CSIDriverInfo Usage and Lifecycle + +CSINodeInfo is an API object representing CSI Information at the Node level. +CSINodeInfo contains a Spec and a Status, each containing Drivers represent the +driver spec and status respectively. The CSIDriverInfoSpec represents the +specification of the driver and is generally not changed, whereas the +CSIDriverInfoStatus represents the current status of the driver and can be +updated and reacted to by various components. + +## Who creates it and when + +CSINodeInfo is created by Kubelet when the first CSI Driver is installed to the +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 +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. + +## Who updates Status and when + +The CSIDriverInfoStatus for the driver is created upon installation of the CSI +Driver to the cluster (the same time as the spec) and it is registered through +the Kubelet device registration mechanism. The Status contains information about +installation and the required Volume Plugin Mechanism of the driver. When the +driver is installed/uninstalled through the Kubelet device registration +mechanism the Available flag is flipped from true/false respectively. The +migration status will also be updated when the flags for migration are set to +true/false on the Kubelet for that Driver on that node. + +## Consumers of Status and Spec + +Currently the only consumer of CSINodeInfo/CSIDriverInfo is the +csi-external-provisioner. In the future, the Attach Detach Controller (ADC) will +need to read this object to determine migration status on a per driver per node +basis. The creation of the CSINodeInfo object could possibly race with the +Attach/Detach controller as for CSI Migration the controller depend on the +existence of the API object for the driver but it will not have been created +yet. The ADC is expected to fail (and retry with exponential backoff) the +operation if it is expecting the object and it has not yet been created. + +## Creation of CSINodeInfo object on Kubelet startup + +For CSI Migration Alpha we expect any user who turns on the feature has both +Kubelet and ADC at a version where the CSINodeInfo's are being created on +Kubelet startup. We will not promote the feature to Beta (on by default) until +the CSINodeInfo's are being created on Kubelet startup for a 2 version skew to +prevent the case where the CSINodeInfo does not exist when the ADC depends on +it. This prevents the race described above becoming a permanent bad state. diff --git a/staging/src/k8s.io/csi-api/pkg/apis/csi/v1alpha1/types.go b/staging/src/k8s.io/csi-api/pkg/apis/csi/v1alpha1/types.go index 635f22af8d..6232b6b4d8 100644 --- a/staging/src/k8s.io/csi-api/pkg/apis/csi/v1alpha1/types.go +++ b/staging/src/k8s.io/csi-api/pkg/apis/csi/v1alpha1/types.go @@ -18,6 +18,16 @@ package v1alpha1 import metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +// VolumePluginMechanism is the type of mechanism components should use for volume operations +type VolumePluginMechanism string + +const ( + // VolumePluginMechanismInTree means components should use the in-tree plugin for volume operations + VolumePluginMechanismInTree VolumePluginMechanism = "in-tree" + // VolumePluginMechanismCSI means components should use the CSI Driver for volume operations + VolumePluginMechanismCSI VolumePluginMechanism = "csi" +) + // +genclient // +genclient:nonNamespaced // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object @@ -91,18 +101,56 @@ type CSINodeInfo struct { // metadata.name must be the Kubernetes node name. metav1.ObjectMeta `json:"metadata,omitempty"` - // List of CSI drivers running on the node and their properties. - // +patchMergeKey=driver - // +patchStrategy=merge - CSIDrivers []CSIDriverInfo `json:"csiDrivers" patchStrategy:"merge" patchMergeKey:"driver"` + // spec is the specification of CSINodeInfo + Spec CSINodeInfoSpec `json:"spec"` + + // status is the current status of CSINodeInfo + Status CSINodeInfoStatus `json:"status"` } -// CSIDriverInfo contains information about one CSI driver installed on a node. -type CSIDriverInfo struct { - // driver is the name of the CSI driver that this object refers to. +// CSINodeInfoSpec holds information about the specification of all CSI drivers installed on a node +type CSINodeInfoSpec struct { + // drivers is a list of specifications of CSIDriverInfo + // +patchMergeKey=name + // +patchStrategy=merge + Drivers []CSIDriverInfoSpec `json:"drivers" patchStrategy:"merge" patchMergeKey:"name"` +} + +// CSINodeInfoStatus holds information about the status of all CSI drivers installed on a node +type CSINodeInfoStatus struct { + // drivers is a list of the statuses of CSIDriverInfo + // +patchMergeKey=name + // +patchStrategy=merge + Drivers []CSIDriverInfoStatus `json:"drivers" patchStrategy:"merge" patchMergeKey:"name"` +} + +// CSIDriverInfoStatus holds information about the status of one CSI driver installed on a node +type CSIDriverInfoStatus struct { + // name is the name of the CSI driver that this object refers to. // This MUST be the same name returned by the CSI GetPluginName() call for // that driver. - Driver string `json:"driver"` + Name string `json:"name"` + + // available is a boolean representing whether the driver has been installed + // on this node or not. + Available bool `json:"available"` + + // volumePluginMechanism announces what mechanism underlies volume plugins. + // It is set by Kubelet. It is used by the attach/detach controller, which + // needs to know how to perform attachments. The allowed values are: + // * "in-tree": the volume operation (e.g., attach/detach) ought to be + // directly performed by the attach/detach controller. + // * "csi-plugin": the attach/detach controller ought to request + // the csi plugin to perform the volume operation rather than perform it directly. + VolumePluginMechanism VolumePluginMechanism `json:"volumePluginMechanism"` +} + +// CSIDriverInfoSpec holds information about the specification of one CSI driver installed on a node +type CSIDriverInfoSpec struct { + // name is the name of the CSI driver that this object refers to. + // This MUST be the same name returned by the CSI GetPluginName() call for + // that driver. + Name string `json:"name"` // nodeID of the node from the driver point of view. // This field enables Kubernetes to communicate with storage systems that do diff --git a/staging/src/k8s.io/csi-api/pkg/apis/csi/v1alpha1/zz_generated.deepcopy.go b/staging/src/k8s.io/csi-api/pkg/apis/csi/v1alpha1/zz_generated.deepcopy.go index aa9ccbbaae..915ef995a0 100644 --- a/staging/src/k8s.io/csi-api/pkg/apis/csi/v1alpha1/zz_generated.deepcopy.go +++ b/staging/src/k8s.io/csi-api/pkg/apis/csi/v1alpha1/zz_generated.deepcopy.go @@ -52,7 +52,7 @@ func (in *CSIDriver) DeepCopyObject() runtime.Object { } // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *CSIDriverInfo) DeepCopyInto(out *CSIDriverInfo) { +func (in *CSIDriverInfoSpec) DeepCopyInto(out *CSIDriverInfoSpec) { *out = *in if in.TopologyKeys != nil { in, out := &in.TopologyKeys, &out.TopologyKeys @@ -62,12 +62,28 @@ func (in *CSIDriverInfo) DeepCopyInto(out *CSIDriverInfo) { return } -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CSIDriverInfo. -func (in *CSIDriverInfo) DeepCopy() *CSIDriverInfo { +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CSIDriverInfoSpec. +func (in *CSIDriverInfoSpec) DeepCopy() *CSIDriverInfoSpec { if in == nil { return nil } - out := new(CSIDriverInfo) + out := new(CSIDriverInfoSpec) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *CSIDriverInfoStatus) DeepCopyInto(out *CSIDriverInfoStatus) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CSIDriverInfoStatus. +func (in *CSIDriverInfoStatus) DeepCopy() *CSIDriverInfoStatus { + if in == nil { + return nil + } + out := new(CSIDriverInfoStatus) in.DeepCopyInto(out) return out } @@ -136,13 +152,8 @@ func (in *CSINodeInfo) DeepCopyInto(out *CSINodeInfo) { *out = *in out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) - if in.CSIDrivers != nil { - in, out := &in.CSIDrivers, &out.CSIDrivers - *out = make([]CSIDriverInfo, len(*in)) - for i := range *in { - (*in)[i].DeepCopyInto(&(*out)[i]) - } - } + in.Spec.DeepCopyInto(&out.Spec) + in.Status.DeepCopyInto(&out.Status) return } @@ -196,3 +207,47 @@ func (in *CSINodeInfoList) DeepCopyObject() runtime.Object { } return nil } + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *CSINodeInfoSpec) DeepCopyInto(out *CSINodeInfoSpec) { + *out = *in + if in.Drivers != nil { + in, out := &in.Drivers, &out.Drivers + *out = make([]CSIDriverInfoSpec, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CSINodeInfoSpec. +func (in *CSINodeInfoSpec) DeepCopy() *CSINodeInfoSpec { + if in == nil { + return nil + } + out := new(CSINodeInfoSpec) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *CSINodeInfoStatus) DeepCopyInto(out *CSINodeInfoStatus) { + *out = *in + if in.Drivers != nil { + in, out := &in.Drivers, &out.Drivers + *out = make([]CSIDriverInfoStatus, len(*in)) + copy(*out, *in) + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CSINodeInfoStatus. +func (in *CSINodeInfoStatus) DeepCopy() *CSINodeInfoStatus { + if in == nil { + return nil + } + out := new(CSINodeInfoStatus) + in.DeepCopyInto(out) + return out +} diff --git a/staging/src/k8s.io/csi-api/pkg/client/clientset/versioned/typed/csi/v1alpha1/csinodeinfo.go b/staging/src/k8s.io/csi-api/pkg/client/clientset/versioned/typed/csi/v1alpha1/csinodeinfo.go index 8580127c02..2efc934691 100644 --- a/staging/src/k8s.io/csi-api/pkg/client/clientset/versioned/typed/csi/v1alpha1/csinodeinfo.go +++ b/staging/src/k8s.io/csi-api/pkg/client/clientset/versioned/typed/csi/v1alpha1/csinodeinfo.go @@ -37,6 +37,7 @@ type CSINodeInfosGetter interface { type CSINodeInfoInterface interface { Create(*v1alpha1.CSINodeInfo) (*v1alpha1.CSINodeInfo, error) Update(*v1alpha1.CSINodeInfo) (*v1alpha1.CSINodeInfo, error) + UpdateStatus(*v1alpha1.CSINodeInfo) (*v1alpha1.CSINodeInfo, error) Delete(name string, options *v1.DeleteOptions) error DeleteCollection(options *v1.DeleteOptions, listOptions v1.ListOptions) error Get(name string, options v1.GetOptions) (*v1alpha1.CSINodeInfo, error) @@ -113,6 +114,21 @@ func (c *cSINodeInfos) Update(cSINodeInfo *v1alpha1.CSINodeInfo) (result *v1alph return } +// UpdateStatus was generated because the type contains a Status member. +// Add a +genclient:noStatus comment above the type to avoid generating UpdateStatus(). + +func (c *cSINodeInfos) UpdateStatus(cSINodeInfo *v1alpha1.CSINodeInfo) (result *v1alpha1.CSINodeInfo, err error) { + result = &v1alpha1.CSINodeInfo{} + err = c.client.Put(). + Resource("csinodeinfos"). + Name(cSINodeInfo.Name). + SubResource("status"). + Body(cSINodeInfo). + Do(). + Into(result) + return +} + // Delete takes name of the cSINodeInfo and deletes it. Returns an error if one occurs. func (c *cSINodeInfos) Delete(name string, options *v1.DeleteOptions) error { return c.client.Delete(). diff --git a/staging/src/k8s.io/csi-api/pkg/client/clientset/versioned/typed/csi/v1alpha1/fake/fake_csinodeinfo.go b/staging/src/k8s.io/csi-api/pkg/client/clientset/versioned/typed/csi/v1alpha1/fake/fake_csinodeinfo.go index babda7c171..6e44b9d385 100644 --- a/staging/src/k8s.io/csi-api/pkg/client/clientset/versioned/typed/csi/v1alpha1/fake/fake_csinodeinfo.go +++ b/staging/src/k8s.io/csi-api/pkg/client/clientset/versioned/typed/csi/v1alpha1/fake/fake_csinodeinfo.go @@ -94,6 +94,17 @@ func (c *FakeCSINodeInfos) Update(cSINodeInfo *v1alpha1.CSINodeInfo) (result *v1 return obj.(*v1alpha1.CSINodeInfo), err } +// UpdateStatus was generated because the type contains a Status member. +// Add a +genclient:noStatus comment above the type to avoid generating UpdateStatus(). +func (c *FakeCSINodeInfos) UpdateStatus(cSINodeInfo *v1alpha1.CSINodeInfo) (*v1alpha1.CSINodeInfo, error) { + obj, err := c.Fake. + Invokes(testing.NewRootUpdateSubresourceAction(csinodeinfosResource, "status", cSINodeInfo), &v1alpha1.CSINodeInfo{}) + if obj == nil { + return nil, err + } + return obj.(*v1alpha1.CSINodeInfo), err +} + // Delete takes name of the cSINodeInfo and deletes it. Returns an error if one occurs. func (c *FakeCSINodeInfos) Delete(name string, options *v1.DeleteOptions) error { _, err := c.Fake. diff --git a/staging/src/k8s.io/csi-api/pkg/crd/manifests/csinodeinfo.yaml b/staging/src/k8s.io/csi-api/pkg/crd/manifests/csinodeinfo.yaml index a9556fc1e8..7b12ba5766 100644 --- a/staging/src/k8s.io/csi-api/pkg/crd/manifests/csinodeinfo.yaml +++ b/staging/src/k8s.io/csi-api/pkg/crd/manifests/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 From 4621887037b3bcd8d9c80d030b67a8694d855b22 Mon Sep 17 00:00:00 2001 From: David Zhu Date: Thu, 8 Nov 2018 13:12:54 -0800 Subject: [PATCH 2/3] Updated test files with new fields --- .../nodeinfomanager/nodeinfomanager_test.go | 132 +++++++++++------- .../noderestriction/admission_test.go | 43 ++++-- test/integration/auth/node_test.go | 32 ++++- 3 files changed, 137 insertions(+), 70 deletions(-) diff --git a/pkg/volume/csi/nodeinfomanager/nodeinfomanager_test.go b/pkg/volume/csi/nodeinfomanager/nodeinfomanager_test.go index 8e42f77869..b58ab813dc 100644 --- a/pkg/volume/csi/nodeinfomanager/nodeinfomanager_test.go +++ b/pkg/volume/csi/nodeinfomanager/nodeinfomanager_test.go @@ -24,7 +24,6 @@ import ( "github.com/container-storage-interface/spec/lib/go/csi/v0" "github.com/stretchr/testify/assert" "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" @@ -55,7 +54,6 @@ type testcase struct { expectedNodeIDMap map[string]string expectedTopologyMap map[string]sets.String expectedLabels map[string]string - expectNoNodeInfo bool expectedVolumeLimit int64 expectFail bool } @@ -64,9 +62,9 @@ type nodeIDMap map[string]string type topologyKeyMap map[string][]string type labelMap map[string]string -// TestAddNodeInfo tests AddNodeInfo with various existing Node and/or CSINodeInfo objects. +// TestInstallCSIDriver tests InstallCSIDriver with various existing Node and/or CSINodeInfo objects. // The node IDs in all test cases below are the same between the Node annotation and CSINodeInfo. -func TestAddNodeInfo(t *testing.T) { +func TestInstallCSIDriver(t *testing.T) { testcases := []testcase{ { name: "empty node", @@ -375,9 +373,9 @@ func TestAddNodeInfo(t *testing.T) { test(t, true /* addNodeInfo */, true /* csiNodeInfoEnabled */, testcases) } -// TestAddNodeInfo_CSINodeInfoDisabled tests AddNodeInfo with various existing Node annotations +// TestInstallCSIDriver_CSINodeInfoDisabled tests InstallCSIDriver with various existing Node annotations // and CSINodeInfo feature gate disabled. -func TestAddNodeInfo_CSINodeInfoDisabled(t *testing.T) { +func TestInstallCSIDriver_CSINodeInfoDisabled(t *testing.T) { testcases := []testcase{ { name: "empty node", @@ -420,16 +418,15 @@ func TestAddNodeInfo_CSINodeInfoDisabled(t *testing.T) { test(t, true /* addNodeInfo */, false /* csiNodeInfoEnabled */, testcases) } -// TestRemoveNodeInfo tests RemoveNodeInfo with various existing Node and/or CSINodeInfo objects. -func TestRemoveNodeInfo(t *testing.T) { +// TestUninstallCSIDriver tests UninstallCSIDriver with various existing Node and/or CSINodeInfo objects. +func TestUninstallCSIDriver(t *testing.T) { testcases := []testcase{ { - name: "empty node and no CSINodeInfo", + name: "empty node and empty CSINodeInfo", driverName: "com.example.csi/driver1", existingNode: generateNode(nil /* nodeIDs */, nil /* labels */, nil /*capacity*/), expectedNodeIDMap: nil, expectedLabels: nil, - expectNoNodeInfo: true, }, { name: "pre-existing node info from the same driver", @@ -451,7 +448,6 @@ func TestRemoveNodeInfo(t *testing.T) { ), expectedNodeIDMap: nil, expectedLabels: map[string]string{"com.example.csi/zone": "zoneA"}, - expectNoNodeInfo: true, }, { name: "pre-existing node info from different driver", @@ -480,7 +476,7 @@ func TestRemoveNodeInfo(t *testing.T) { expectedLabels: map[string]string{"net.example.storage/zone": "zoneA"}, }, { - name: "pre-existing info about the same driver in node, but no CSINodeInfo", + name: "pre-existing info about the same driver in node, but empty CSINodeInfo", driverName: "com.example.csi/driver1", existingNode: generateNode( nodeIDMap{ @@ -489,10 +485,9 @@ func TestRemoveNodeInfo(t *testing.T) { nil /* labels */, nil /*capacity*/), expectedNodeIDMap: nil, expectedLabels: nil, - expectNoNodeInfo: true, }, { - name: "pre-existing info about a different driver in node, but no CSINodeInfo", + name: "pre-existing info about a different driver in node, but empty CSINodeInfo", existingNode: generateNode( nodeIDMap{ "net.example.storage/other-driver": "net.example.storage/csi-node1", @@ -501,8 +496,7 @@ func TestRemoveNodeInfo(t *testing.T) { expectedNodeIDMap: map[string]string{ "net.example.storage/other-driver": "net.example.storage/csi-node1", }, - expectedLabels: nil, - expectNoNodeInfo: true, + expectedLabels: nil, }, { name: "new node with valid max limit", @@ -517,7 +511,6 @@ func TestRemoveNodeInfo(t *testing.T) { ), inputTopology: nil, inputNodeID: "com.example.csi/csi-node1", - expectNoNodeInfo: true, expectedVolumeLimit: 0, }, } @@ -525,9 +518,9 @@ func TestRemoveNodeInfo(t *testing.T) { test(t, false /* addNodeInfo */, true /* csiNodeInfoEnabled */, testcases) } -// TestRemoveNodeInfo tests RemoveNodeInfo with various existing Node objects and CSINodeInfo +// TestUninstallCSIDriver tests UninstallCSIDriver with various existing Node objects and CSINodeInfo // feature disabled. -func TestRemoveNodeInfo_CSINodeInfoDisabled(t *testing.T) { +func TestUninstallCSIDriver_CSINodeInfoDisabled(t *testing.T) { testcases := []testcase{ { name: "empty node", @@ -562,7 +555,7 @@ func TestRemoveNodeInfo_CSINodeInfoDisabled(t *testing.T) { test(t, false /* addNodeInfo */, false /* csiNodeInfoEnabled */, testcases) } -func TestAddNodeInfoExistingAnnotation(t *testing.T) { +func TestInstallCSIDriverExistingAnnotation(t *testing.T) { defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSINodeInfo, true)() driverName := "com.example.csi/driver1" @@ -573,7 +566,7 @@ func TestAddNodeInfoExistingAnnotation(t *testing.T) { existingNode *v1.Node }{ { - name: "pre-existing info about the same driver in node, but no CSINodeInfo", + name: "pre-existing info about the same driver in node, but empty CSINodeInfo", existingNode: generateNode( nodeIDMap{ "com.example.csi/driver1": "com.example.csi/csi-node1", @@ -581,7 +574,7 @@ func TestAddNodeInfoExistingAnnotation(t *testing.T) { nil /* labels */, nil /*capacity*/), }, { - name: "pre-existing info about a different driver in node, but no CSINodeInfo", + name: "pre-existing info about a different driver in node, but empty CSINodeInfo", existingNode: generateNode( nodeIDMap{ "net.example.storage/other-driver": "net.example.storage/test-node", @@ -613,9 +606,14 @@ func TestAddNodeInfoExistingAnnotation(t *testing.T) { nim := NewNodeInfoManager(types.NodeName(nodeName), host) // Act - err = nim.AddNodeInfo(driverName, nodeID, 0 /* maxVolumeLimit */, nil) // TODO test maxVolumeLimit + _, err = nim.CreateCSINodeInfo() if err != nil { - t.Errorf("expected no error from AddNodeInfo call but got: %v", err) + t.Errorf("expected no error from creating CSINodeinfo but got: %v", err) + continue + } + err = nim.InstallCSIDriver(driverName, nodeID, 0 /* maxVolumeLimit */, nil) // TODO test maxVolumeLimit + if err != nil { + t.Errorf("expected no error from InstallCSIDriver call but got: %v", err) continue } @@ -626,14 +624,15 @@ func TestAddNodeInfoExistingAnnotation(t *testing.T) { continue } - if len(nodeInfo.CSIDrivers) != 1 { - t.Errorf("expected 1 CSIDriverInfo entry but got: %d", len(nodeInfo.CSIDrivers)) + if len(nodeInfo.Spec.Drivers) != 1 || len(nodeInfo.Status.Drivers) != 1 { + t.Errorf("expected 1 CSIDriverInfoSpec and 1 CSIDriverInfoStatus entry but got: %d, %d", + len(nodeInfo.Spec.Drivers), len(nodeInfo.Status.Drivers)) continue } - driver := nodeInfo.CSIDrivers[0] - if driver.Driver != driverName || driver.NodeID != nodeID { - t.Errorf("expected Driver to be %q and NodeID to be %q, but got: %q:%q", driverName, nodeID, driver.Driver, driver.NodeID) + driver := nodeInfo.Spec.Drivers[0] + if driver.Name != driverName || driver.NodeID != nodeID { + t.Errorf("expected Driver to be %q and NodeID to be %q, but got: %q:%q", driverName, nodeID, driver.Name, driver.NodeID) } } } @@ -669,28 +668,29 @@ func test(t *testing.T, addNodeInfo bool, csiNodeInfoEnabled bool, testcases []t nim := NewNodeInfoManager(types.NodeName(nodeName), host) //// Act + nim.CreateCSINodeInfo() if addNodeInfo { - err = nim.AddNodeInfo(tc.driverName, tc.inputNodeID, tc.inputVolumeLimit, tc.inputTopology) + err = nim.InstallCSIDriver(tc.driverName, tc.inputNodeID, tc.inputVolumeLimit, tc.inputTopology) } else { - err = nim.RemoveNodeInfo(tc.driverName) + err = nim.UninstallCSIDriver(tc.driverName) } //// Assert if tc.expectFail { if err == nil { - t.Errorf("expected an error from AddNodeInfo call but got none") + t.Errorf("expected an error from InstallCSIDriver call but got none") } continue } else if err != nil { - t.Errorf("expected no error from AddNodeInfo call but got: %v", err) + t.Errorf("expected no error from InstallCSIDriver call but got: %v", err) continue } actions := client.Actions() var node *v1.Node - if hasPatchAction(actions) { - node, err = applyNodeStatusPatch(tc.existingNode, actions[1].(clienttesting.PatchActionImpl).GetPatch()) + if action := hasPatchAction(actions); action != nil { + node, err = applyNodeStatusPatch(tc.existingNode, action.(clienttesting.PatchActionImpl).GetPatch()) assert.NoError(t, err) } else { node, err = client.CoreV1().Nodes().Get(nodeName, metav1.GetOptions{}) @@ -710,6 +710,7 @@ func test(t *testing.T, addNodeInfo bool, csiNodeInfoEnabled bool, testcases []t } // Node ID annotation + foundInNode := false annNodeID, ok := node.Annotations[annotationKeyNodeID] if ok { if tc.expectedNodeIDMap == nil { @@ -723,6 +724,8 @@ func test(t *testing.T, addNodeInfo bool, csiNodeInfoEnabled bool, testcases []t if !helper.Semantic.DeepEqual(actualNodeIDs, tc.expectedNodeIDMap) { t.Errorf("expected annotation %v; got: %v", tc.expectedNodeIDMap, actualNodeIDs) + } else { + foundInNode = true } } } else { @@ -739,24 +742,35 @@ func test(t *testing.T, addNodeInfo bool, csiNodeInfoEnabled bool, testcases []t /* CSINodeInfo validation */ nodeInfo, err := csiClient.Csi().CSINodeInfos().Get(nodeName, metav1.GetOptions{}) - if tc.expectNoNodeInfo && errors.IsNotFound(err) { - continue - } else if err != nil { + if err != nil { t.Errorf("error getting CSINodeInfo: %v", err) continue } // Extract node IDs and topology keys + + availableDrivers := sets.String{} actualNodeIDs := make(map[string]string) actualTopologyKeys := make(map[string]sets.String) - for _, driver := range nodeInfo.CSIDrivers { - actualNodeIDs[driver.Driver] = driver.NodeID - actualTopologyKeys[driver.Driver] = sets.NewString(driver.TopologyKeys...) + for _, driver := range nodeInfo.Status.Drivers { + if driver.Available { + availableDrivers.Insert(driver.Name) + } + + } + for _, driver := range nodeInfo.Spec.Drivers { + if availableDrivers.Has(driver.Name) { + actualNodeIDs[driver.Name] = driver.NodeID + actualTopologyKeys[driver.Name] = sets.NewString(driver.TopologyKeys...) + } } // Node IDs - if !helper.Semantic.DeepEqual(actualNodeIDs, tc.expectedNodeIDMap) { - t.Errorf("expected node IDs %v from CSINodeInfo; got: %v", tc.expectedNodeIDMap, actualNodeIDs) + // No need to check if Node ID found in NodeInfo if it was present in the NodeID + if !foundInNode { + if !helper.Semantic.DeepEqual(actualNodeIDs, tc.expectedNodeIDMap) { + t.Errorf("expected node IDs %v from CSINodeInfo; got: %v", tc.expectedNodeIDMap, actualNodeIDs) + } } // Topology keys @@ -802,22 +816,34 @@ func generateNode(nodeIDs, labels map[string]string, capacity map[v1.ResourceNam } func generateNodeInfo(nodeIDs map[string]string, topologyKeys map[string][]string) *csiv1alpha1.CSINodeInfo { - var drivers []csiv1alpha1.CSIDriverInfo + driverInfoSpecs := []csiv1alpha1.CSIDriverInfoSpec{} + driverInfoStatuses := []csiv1alpha1.CSIDriverInfoStatus{} for k, nodeID := range nodeIDs { - d := csiv1alpha1.CSIDriverInfo{ - Driver: k, + dspec := csiv1alpha1.CSIDriverInfoSpec{ + Name: k, NodeID: nodeID, } - if top, exists := topologyKeys[k]; exists { - d.TopologyKeys = top + dstatus := csiv1alpha1.CSIDriverInfoStatus{ + Name: k, + Available: true, + VolumePluginMechanism: csiv1alpha1.VolumePluginMechanismInTree, } - drivers = append(drivers, d) + if top, exists := topologyKeys[k]; exists { + dspec.TopologyKeys = top + } + driverInfoSpecs = append(driverInfoSpecs, dspec) + driverInfoStatuses = append(driverInfoStatuses, dstatus) } return &csiv1alpha1.CSINodeInfo{ ObjectMeta: metav1.ObjectMeta{ Name: "node1", }, - CSIDrivers: drivers, + Spec: csiv1alpha1.CSINodeInfoSpec{ + Drivers: driverInfoSpecs, + }, + Status: csiv1alpha1.CSINodeInfoStatus{ + Drivers: driverInfoStatuses, + }, } } @@ -838,11 +864,11 @@ func applyNodeStatusPatch(originalNode *v1.Node, patch []byte) (*v1.Node, error) return updatedNode, nil } -func hasPatchAction(actions []clienttesting.Action) bool { +func hasPatchAction(actions []clienttesting.Action) clienttesting.Action { for _, action := range actions { if action.GetVerb() == "patch" { - return true + return action } } - return false + return nil } diff --git a/plugin/pkg/admission/noderestriction/admission_test.go b/plugin/pkg/admission/noderestriction/admission_test.go index 75ef000d39..79fd77c4a5 100644 --- a/plugin/pkg/admission/noderestriction/admission_test.go +++ b/plugin/pkg/admission/noderestriction/admission_test.go @@ -22,6 +22,7 @@ import ( "time" "fmt" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" @@ -216,11 +217,22 @@ func Test_nodePlugin_Admit(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: "mynode", }, - CSIDrivers: []csiv1alpha1.CSIDriverInfo{ - { - Driver: "com.example.csi/mydriver", - NodeID: "com.example.csi/mynode", - TopologyKeys: []string{"com.example.csi/zone"}, + Spec: csiv1alpha1.CSINodeInfoSpec{ + Drivers: []csiv1alpha1.CSIDriverInfoSpec{ + { + Name: "com.example.csi/mydriver", + NodeID: "com.example.csi/mynode", + TopologyKeys: []string{"com.example.csi/zone"}, + }, + }, + }, + Status: csiv1alpha1.CSINodeInfoStatus{ + Drivers: []csiv1alpha1.CSIDriverInfoStatus{ + { + Name: "com.example.csi/mydriver", + Available: true, + VolumePluginMechanism: csiv1alpha1.VolumePluginMechanismInTree, + }, }, }, } @@ -228,11 +240,22 @@ func Test_nodePlugin_Admit(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: "foo", }, - CSIDrivers: []csiv1alpha1.CSIDriverInfo{ - { - Driver: "com.example.csi/mydriver", - NodeID: "com.example.csi/foo", - TopologyKeys: []string{"com.example.csi/zone"}, + Spec: csiv1alpha1.CSINodeInfoSpec{ + Drivers: []csiv1alpha1.CSIDriverInfoSpec{ + { + Name: "com.example.csi/mydriver", + NodeID: "com.example.csi/foo", + TopologyKeys: []string{"com.example.csi/zone"}, + }, + }, + }, + Status: csiv1alpha1.CSINodeInfoStatus{ + Drivers: []csiv1alpha1.CSIDriverInfoStatus{ + { + Name: "com.example.csi/mydriver", + Available: true, + VolumePluginMechanism: csiv1alpha1.VolumePluginMechanismInTree, + }, }, }, } diff --git a/test/integration/auth/node_test.go b/test/integration/auth/node_test.go index 3d4038efb6..f075ea1d7d 100644 --- a/test/integration/auth/node_test.go +++ b/test/integration/auth/node_test.go @@ -420,11 +420,22 @@ func TestNodeAuthorizer(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: "node1", }, - CSIDrivers: []csiv1alpha1.CSIDriverInfo{ - { - Driver: "com.example.csi/driver1", - NodeID: "com.example.csi/node1", - TopologyKeys: []string{"com.example.csi/zone"}, + Spec: csiv1alpha1.CSINodeInfoSpec{ + Drivers: []csiv1alpha1.CSIDriverInfoSpec{ + { + Name: "com.example.csi/driver1", + NodeID: "com.example.csi/node1", + TopologyKeys: []string{"com.example.csi/zone"}, + }, + }, + }, + Status: csiv1alpha1.CSINodeInfoStatus{ + Drivers: []csiv1alpha1.CSIDriverInfoStatus{ + { + Name: "com.example.csi/driver1", + Available: true, + VolumePluginMechanism: csiv1alpha1.VolumePluginMechanismInTree, + }, }, }, } @@ -438,13 +449,20 @@ func TestNodeAuthorizer(t *testing.T) { if err != nil { return err } - nodeInfo.CSIDrivers = []csiv1alpha1.CSIDriverInfo{ + nodeInfo.Spec.Drivers = []csiv1alpha1.CSIDriverInfoSpec{ { - Driver: "com.example.csi/driver1", + Name: "com.example.csi/driver1", NodeID: "com.example.csi/node1", TopologyKeys: []string{"com.example.csi/rack"}, }, } + nodeInfo.Status.Drivers = []csiv1alpha1.CSIDriverInfoStatus{ + { + Name: "com.example.csi/driver1", + Available: true, + VolumePluginMechanism: csiv1alpha1.VolumePluginMechanismInTree, + }, + } _, err = client.CsiV1alpha1().CSINodeInfos().Update(nodeInfo) return err } From 06f3b26012e09318935c248e62864e9349da6cd8 Mon Sep 17 00:00:00 2001 From: David Zhu Date: Thu, 8 Nov 2018 13:13:14 -0800 Subject: [PATCH 3/3] 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