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 }