Merge pull request #2673 from kubernetes-sigs/refine-KataCCLabel

cleanup: refine kataNode label matching

Author: andyzhangx

pkg/azurefile/azurefile.go

@@ -162,6 +162,7 @@ const (
 	premium                           = "premium"
 	selectRandomMatchingAccountField  = "selectrandommatchingaccount"
 	accountQuotaField                 = "accountquota"
+	defaultKataCCLabel                = "kubernetes.azure.com/kata-cc-isolation"
 
 	accountNotProvisioned = "StorageAccountIsNotProvisioned"
 	// this is a workaround fix for 429 throttling issue, will update cloud provider for better fix later
@@ -1342,32 +1343,30 @@ func (d *Driver) getFileShareClientForSub(subscriptionID string) (fileshareclien
 	return d.cloud.ComputeClientFactory.GetFileShareClientForSub(subscriptionID)
 }
 
-func getNodeInfoFromLabels(ctx context.Context, nodeID string, kubeClient clientset.Interface) (string, string, string, error) {
+func isKataNode(ctx context.Context, nodeID string, kubeClient clientset.Interface) bool {
+	if nodeID == "" {
+		return false
+	}
+
 	if kubeClient == nil || kubeClient.CoreV1() == nil {
-		return "", "", "", fmt.Errorf("kubeClient is nil")
+		klog.Warningf("kubeClient is nil, cannot check if node(%s) is a kata node", nodeID)
+		return false
 	}
 
 	node, err := kubeClient.CoreV1().Nodes().Get(ctx, nodeID, metav1.GetOptions{})
 	if err != nil {
-		return "", "", "", fmt.Errorf("get node(%s) failed with %v", nodeID, err)
-	}
-
-	if len(node.Labels) == 0 {
-		return "", "", "", fmt.Errorf("node(%s) label is empty", nodeID)
+		klog.Warningf("failed to get node(%s): %v", nodeID, err)
+		return false
 	}
-	return node.Labels["kubernetes.azure.com/kata-cc-isolation"], node.Labels["kubernetes.azure.com/kata-mshv-vm-isolation"], node.Labels["katacontainers.io/kata-runtime"], nil
-}
 
-func isKataNode(ctx context.Context, nodeID string, kubeClient clientset.Interface) bool {
-	if nodeID == "" {
+	if node == nil || len(node.Labels) == 0 {
 		return false
 	}
-	kataCCIsolationLabel, kataVMIsolationLabel, kataRuntimeLabel, err := getNodeInfoFromLabels(ctx, nodeID, kubeClient)
 
-	if err != nil {
-		klog.Warningf("failed to get node info from labels: %v", err)
+	// Check for the kata isolation labels
+	if _, ok := node.Labels[defaultKataCCLabel]; !ok {
 		return false
 	}
-	klog.V(4).Infof("node(%s) labels: kataVMIsolationLabel(%s), kataRuntimeLabel(%s)", nodeID, kataVMIsolationLabel, kataRuntimeLabel)
-	return strings.EqualFold(kataCCIsolationLabel, "true")
+	klog.V(4).Infof("node(%s) is a kata node with labels: %v", nodeID, node.Labels)
+	return true
 }

pkg/azurefile/azurefile_test.go

@@ -1635,90 +1635,26 @@ func TestGetFileShareClientForSub(t *testing.T) {
 	}
 }
 
-func TestGetNodeInfoFromLabels(t *testing.T) {
+func TestIsKataNode(t *testing.T) {
 	testCases := []struct {
-		name         string
-		nodeName     string
-		labels       map[string]string
-		setupClient  bool
-		expectedVals [3]string
-		expectedErr  error
+		name        string
+		nodeName    string
+		labels      map[string]string
+		setupClient bool
+		expected    bool
 	}{
 		{
 			name:        "Error when kubeClient is nil",
 			nodeName:    "test-node",
 			setupClient: false,
-			expectedErr: fmt.Errorf("kubeClient is nil"),
-		},
-		{
-			name:        "Error when node does not exist",
-			nodeName:    "nonexistent-node",
-			setupClient: true,
-			expectedErr: fmt.Errorf("get node(nonexistent-node) failed with nodes \"nonexistent-node\" not found"),
+			expected:    false,
 		},
 		{
-			name:        "Error when node has no labels",
-			nodeName:    "test-node",
-			setupClient: true,
-			labels:      map[string]string{}, // Node exists but has no labels
-			expectedErr: fmt.Errorf("node(test-node) label is empty"),
+			name:        "nodeName is empty",
+			nodeName:    "",
+			setupClient: false,
+			expected:    false,
 		},
-		{
-			name:        "Success with kata labels",
-			nodeName:    "test-node",
-			setupClient: true,
-			labels: map[string]string{
-				"kubernetes.azure.com/kata-cc-isolation":      "true",
-				"kubernetes.azure.com/kata-mshv-vm-isolation": "true",
-				"katacontainers.io/kata-runtime":              "false",
-			},
-			expectedVals: [3]string{"true", "true", "false"},
-			expectedErr:  nil,
-		},
-	}
-
-	for _, tc := range testCases {
-		t.Run(tc.name, func(t *testing.T) {
-			ctx := context.TODO()
-			var clientset kubernetes.Interface
-
-			if tc.setupClient {
-				clientset = fake.NewSimpleClientset()
-			}
-
-			if tc.labels != nil && tc.setupClient {
-				node := &v1api.Node{
-					ObjectMeta: metav1.ObjectMeta{
-						Name:   tc.nodeName,
-						Labels: tc.labels,
-					},
-				}
-				_, err := clientset.CoreV1().Nodes().Create(ctx, node, metav1.CreateOptions{})
-				assert.NoError(t, err)
-			}
-
-			kataCCIsolation, kataVMIsolation, kataRuntime, err := getNodeInfoFromLabels(ctx, tc.nodeName, clientset)
-
-			if tc.expectedErr != nil {
-				assert.EqualError(t, err, tc.expectedErr.Error())
-			} else {
-				assert.NoError(t, err)
-				assert.Equal(t, tc.expectedVals[0], kataCCIsolation)
-				assert.Equal(t, tc.expectedVals[1], kataVMIsolation)
-				assert.Equal(t, tc.expectedVals[2], kataRuntime)
-			}
-		})
-	}
-}
-
-func TestIsKataNode(t *testing.T) {
-	testCases := []struct {
-		name        string
-		nodeName    string
-		labels      map[string]string
-		setupClient bool
-		expected    bool
-	}{
 		{
 			name:        "Node does not exist",
 			nodeName:    "",
@@ -1739,7 +1675,18 @@ func TestIsKataNode(t *testing.T) {
 			nodeName:    "test-node",
 			setupClient: true,
 			labels: map[string]string{
-				"kubernetes.azure.com/kata-cc-isolation": "true",
+				defaultKataCCLabel: "",
+			},
+			expected: true,
+		},
+		{
+			name:        "Node has kata labels",
+			nodeName:    "test-node",
+			setupClient: true,
+			labels: map[string]string{
+				defaultKataCCLabel: "test",
+				"kubernetes.azure.com/kata-mshv-vm-isolation": "true",
+				"katacontainers.io/kata-runtime":              "true",
 			},
 			expected: true,
 		},