add nil check for Node(Un)PrepareResources.

Signed-off-by: Ayato Tokubi <atokubi@redhat.com>

pkg/kubelet/cm/dra/manager.go

@@ -171,7 +171,7 @@ func (m *ManagerImpl) PrepareResources(pod *v1.Pod) error {
 			if reqClaim == nil {
 				return fmt.Errorf("NodePrepareResources returned result for unknown claim UID %s", claimUID)
 			}
-			if result.Error != "" {
+			if result.GetError() != "" {
 				return fmt.Errorf("NodePrepareResources failed for claim %s/%s: %s", reqClaim.Namespace, reqClaim.Name, result.Error)
 			}
 
@@ -179,11 +179,11 @@ func (m *ManagerImpl) PrepareResources(pod *v1.Pod) error {
 
 			// Add the CDI Devices returned by NodePrepareResources to
 			// the claimInfo object.
-			err = claimInfo.addCDIDevices(pluginName, result.CDIDevices)
+			err = claimInfo.addCDIDevices(pluginName, result.GetCDIDevices())
 			if err != nil {
 				return fmt.Errorf("failed to add CDIDevices to claimInfo %+v: %+v", claimInfo, err)
 			}
-			// mark claim as (successfully) prepared by manager, so next time we dont prepare it.
+			// mark claim as (successfully) prepared by manager, so next time we don't prepare it.
 			claimInfo.prepared = true
 
 			// TODO: We (re)add the claimInfo object to the cache and
@@ -379,7 +379,7 @@ func (m *ManagerImpl) UnprepareResources(pod *v1.Pod) error {
 			if reqClaim == nil {
 				return fmt.Errorf("NodeUnprepareResources returned result for unknown claim UID %s", claimUID)
 			}
-			if result.Error != "" {
+			if result.GetError() != "" {
 				return fmt.Errorf("NodeUnprepareResources failed for claim %s/%s: %s", reqClaim.Namespace, reqClaim.Name, result.Error)
 			}
 

pkg/kubelet/cm/dra/manager_test.go

@@ -51,6 +51,8 @@ type fakeDRADriverGRPCServer struct {
 	timeout                    *time.Duration
 	prepareResourceCalls       atomic.Uint32
 	unprepareResourceCalls     atomic.Uint32
+	prepareResourcesResponse   *drapbv1.NodePrepareResourcesResponse
+	unprepareResourcesResponse *drapbv1.NodeUnprepareResourcesResponse
 }
 
 func (s *fakeDRADriverGRPCServer) NodePrepareResources(ctx context.Context, req *drapbv1.NodePrepareResourcesRequest) (*drapbv1.NodePrepareResourcesResponse, error) {
@@ -59,9 +61,8 @@ func (s *fakeDRADriverGRPCServer) NodePrepareResources(ctx context.Context, req
 	if s.timeout != nil {
 		time.Sleep(*s.timeout)
 	}
-	deviceName := "claim-" + req.Claims[0].Uid
+
-	result := s.driverName + "/" + driverClassName + "=" + deviceName
+	return s.prepareResourcesResponse, nil
-	return &drapbv1.NodePrepareResourcesResponse{Claims: map[string]*drapbv1.NodePrepareResourceResponse{req.Claims[0].Uid: {CDIDevices: []string{result}}}}, nil
 }
 
 func (s *fakeDRADriverGRPCServer) NodeUnprepareResources(ctx context.Context, req *drapbv1.NodeUnprepareResourcesRequest) (*drapbv1.NodeUnprepareResourcesResponse, error) {
@@ -70,7 +71,8 @@ func (s *fakeDRADriverGRPCServer) NodeUnprepareResources(ctx context.Context, re
 	if s.timeout != nil {
 		time.Sleep(*s.timeout)
 	}
-	return &drapbv1.NodeUnprepareResourcesResponse{Claims: map[string]*drapbv1.NodeUnprepareResourceResponse{req.Claims[0].Uid: {}}}, nil
+
+	return s.unprepareResourcesResponse, nil
 }
 
 type tearDown func()
@@ -84,7 +86,7 @@ type fakeDRAServerInfo struct {
 	teardownFn tearDown
 }
 
-func setupFakeDRADriverGRPCServer(shouldTimeout bool, pluginClientTimeout *time.Duration) (fakeDRAServerInfo, error) {
+func setupFakeDRADriverGRPCServer(shouldTimeout bool, pluginClientTimeout *time.Duration, prepareResourcesResponse *drapbv1.NodePrepareResourcesResponse, unprepareResourcesResponse *drapbv1.NodeUnprepareResourcesResponse) (fakeDRAServerInfo, error) {
 	socketDir, err := os.MkdirTemp("", "dra")
 	if err != nil {
 		return fakeDRAServerInfo{
@@ -115,6 +117,8 @@ func setupFakeDRADriverGRPCServer(shouldTimeout bool, pluginClientTimeout *time.
 	s := grpc.NewServer()
 	fakeDRADriverGRPCServer := &fakeDRADriverGRPCServer{
 		driverName:                 driverName,
+		prepareResourcesResponse:   prepareResourcesResponse,
+		unprepareResourcesResponse: unprepareResourcesResponse,
 	}
 	if shouldTimeout {
 		timeout := *pluginClientTimeout * 2
@@ -319,9 +323,11 @@ func TestPrepareResources(t *testing.T) {
 		pod                  *v1.Pod
 		claimInfo            *ClaimInfo
 		resourceClaim        *resourcev1alpha2.ResourceClaim
+		resp                 *drapbv1.NodePrepareResourcesResponse
 		wantErr              bool
 		wantTimeout          bool
 		wantResourceSkipped  bool
+		expectedCDIDevices   []string
 		ExpectedPrepareCalls uint32
 	}{
 		{
@@ -406,6 +412,124 @@ func TestPrepareResources(t *testing.T) {
 			},
 			wantErr: true,
 		},
+		{
+			description: "should prepare resources, driver returns nil value",
+			driverName:  driverName,
+			pod: &v1.Pod{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-pod",
+					Namespace: "test-namespace",
+					UID:       "test-reserved",
+				},
+				Spec: v1.PodSpec{
+					ResourceClaims: []v1.PodResourceClaim{
+						{
+							Name: "test-pod-claim-nil",
+							Source: v1.ClaimSource{
+								ResourceClaimName: func() *string {
+									s := "test-pod-claim-nil"
+									return &s
+								}(),
+							},
+						},
+					},
+					Containers: []v1.Container{
+						{
+							Resources: v1.ResourceRequirements{
+								Claims: []v1.ResourceClaim{
+									{
+										Name: "test-pod-claim-nil",
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			resourceClaim: &resourcev1alpha2.ResourceClaim{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-pod-claim-nil",
+					Namespace: "test-namespace",
+					UID:       "test-reserved",
+				},
+				Spec: resourcev1alpha2.ResourceClaimSpec{
+					ResourceClassName: "test-class",
+				},
+				Status: resourcev1alpha2.ResourceClaimStatus{
+					DriverName: driverName,
+					Allocation: &resourcev1alpha2.AllocationResult{
+						ResourceHandles: []resourcev1alpha2.ResourceHandle{
+							{Data: "test-data", DriverName: driverName},
+						},
+					},
+					ReservedFor: []resourcev1alpha2.ResourceClaimConsumerReference{
+						{UID: "test-reserved"},
+					},
+				},
+			},
+			resp:                 &drapbv1.NodePrepareResourcesResponse{Claims: map[string]*drapbv1.NodePrepareResourceResponse{"test-reserved": nil}},
+			expectedCDIDevices:   []string{},
+			ExpectedPrepareCalls: 1,
+		},
+		{
+			description: "should prepare resources, driver returns empty result",
+			driverName:  driverName,
+			pod: &v1.Pod{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-pod",
+					Namespace: "test-namespace",
+					UID:       "test-reserved",
+				},
+				Spec: v1.PodSpec{
+					ResourceClaims: []v1.PodResourceClaim{
+						{
+							Name: "test-pod-claim-empty",
+							Source: v1.ClaimSource{
+								ResourceClaimName: func() *string {
+									s := "test-pod-claim-empty"
+									return &s
+								}(),
+							},
+						},
+					},
+					Containers: []v1.Container{
+						{
+							Resources: v1.ResourceRequirements{
+								Claims: []v1.ResourceClaim{
+									{
+										Name: "test-pod-claim-empty",
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			resourceClaim: &resourcev1alpha2.ResourceClaim{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-pod-claim-empty",
+					Namespace: "test-namespace",
+					UID:       "test-reserved",
+				},
+				Spec: resourcev1alpha2.ResourceClaimSpec{
+					ResourceClassName: "test-class",
+				},
+				Status: resourcev1alpha2.ResourceClaimStatus{
+					DriverName: driverName,
+					Allocation: &resourcev1alpha2.AllocationResult{
+						ResourceHandles: []resourcev1alpha2.ResourceHandle{
+							{Data: "test-data", DriverName: driverName},
+						},
+					},
+					ReservedFor: []resourcev1alpha2.ResourceClaimConsumerReference{
+						{UID: "test-reserved"},
+					},
+				},
+			},
+			resp:                 &drapbv1.NodePrepareResourcesResponse{Claims: map[string]*drapbv1.NodePrepareResourceResponse{"test-reserved": nil}},
+			expectedCDIDevices:   []string{},
+			ExpectedPrepareCalls: 1,
+		},
 		{
 			description: "pod is not allowed to use resource claim",
 			driverName:  driverName,
@@ -555,6 +679,7 @@ func TestPrepareResources(t *testing.T) {
 					},
 				},
 			},
+			expectedCDIDevices:  []string{fmt.Sprintf("%s/%s=claim-test-reserved", driverName, driverClassName)},
 			wantResourceSkipped: true,
 		},
 		{
@@ -610,6 +735,11 @@ func TestPrepareResources(t *testing.T) {
 					},
 				},
 			},
+			resp: &drapbv1.NodePrepareResourcesResponse{
+				Claims: map[string]*drapbv1.NodePrepareResourceResponse{
+					"test-reserved": {CDIDevices: []string{fmt.Sprintf("%s/%s=claim-test-reserved", driverName, driverClassName)}},
+				},
+			},
 			wantErr:              true,
 			wantTimeout:          true,
 			ExpectedPrepareCalls: 1,
@@ -667,6 +797,12 @@ func TestPrepareResources(t *testing.T) {
 					},
 				},
 			},
+			resp: &drapbv1.NodePrepareResourcesResponse{
+				Claims: map[string]*drapbv1.NodePrepareResourceResponse{
+					"test-reserved": {CDIDevices: []string{fmt.Sprintf("%s/%s=claim-test-reserved", driverName, driverClassName)}},
+				},
+			},
+			expectedCDIDevices:   []string{fmt.Sprintf("%s/%s=claim-test-reserved", driverName, driverClassName)},
 			ExpectedPrepareCalls: 1,
 		},
 		{
@@ -738,6 +874,12 @@ func TestPrepareResources(t *testing.T) {
 					},
 				},
 			},
+			resp: &drapbv1.NodePrepareResourcesResponse{
+				Claims: map[string]*drapbv1.NodePrepareResourceResponse{
+					"test-reserved": {CDIDevices: []string{fmt.Sprintf("%s/%s=claim-test-reserved", driverName, driverClassName)}},
+				},
+			},
+			expectedCDIDevices:   []string{fmt.Sprintf("%s/%s=claim-test-reserved", driverName, driverClassName)},
 			ExpectedPrepareCalls: 1,
 		},
 	} {
@@ -764,7 +906,7 @@ func TestPrepareResources(t *testing.T) {
 				pluginClientTimeout = &timeout
 			}
 
-			draServerInfo, err := setupFakeDRADriverGRPCServer(test.wantTimeout, pluginClientTimeout)
+			draServerInfo, err := setupFakeDRADriverGRPCServer(test.wantTimeout, pluginClientTimeout, test.resp, nil)
 			if err != nil {
 				t.Fatal(err)
 			}
@@ -811,10 +953,8 @@ func TestPrepareResources(t *testing.T) {
 			if len(claimInfo.PodUIDs) != 1 || !claimInfo.PodUIDs.Has(string(test.pod.UID)) {
 				t.Fatalf("podUIDs mismatch: expected [%s], got %v", test.pod.UID, claimInfo.PodUIDs)
 			}
-			expectedResourceClaimDriverName := fmt.Sprintf("%s/%s=claim-%s", driverName, driverClassName, string(test.resourceClaim.Status.ReservedFor[0].UID))
+			assert.ElementsMatchf(t, claimInfo.CDIDevices[test.resourceClaim.Status.DriverName], test.expectedCDIDevices,
-			if len(claimInfo.CDIDevices[test.resourceClaim.Status.DriverName]) != 1 || claimInfo.CDIDevices[test.resourceClaim.Status.DriverName][0] != expectedResourceClaimDriverName {
+				"cdiDevices mismatch: expected [%v], got %v", test.expectedCDIDevices, claimInfo.CDIDevices[test.resourceClaim.Status.DriverName])
-				t.Fatalf("cdiDevices mismatch: expected [%s], got %v", []string{expectedResourceClaimDriverName}, claimInfo.CDIDevices[test.resourceClaim.Status.DriverName])
-			}
 		})
 	}
 }
@@ -827,6 +967,7 @@ func TestUnprepareResources(t *testing.T) {
 		driverName             string
 		pod                    *v1.Pod
 		claimInfo              *ClaimInfo
+		resp                   *drapbv1.NodeUnprepareResourcesResponse
 		wantErr                bool
 		wantTimeout            bool
 		wantResourceSkipped    bool
@@ -853,6 +994,17 @@ func TestUnprepareResources(t *testing.T) {
 							},
 						},
 					},
+					Containers: []v1.Container{
+						{
+							Resources: v1.ResourceRequirements{
+								Claims: []v1.ResourceClaim{
+									{
+										Name: "another-claim-test",
+									},
+								},
+							},
+						},
+					},
 				},
 			},
 			claimInfo: &ClaimInfo{
@@ -957,6 +1109,7 @@ func TestUnprepareResources(t *testing.T) {
 					},
 				},
 			},
+			resp:                   &drapbv1.NodeUnprepareResourcesResponse{Claims: map[string]*drapbv1.NodeUnprepareResourceResponse{"test-reserved": {}}},
 			wantErr:                true,
 			wantTimeout:            true,
 			expectedUnprepareCalls: 1,
@@ -1007,6 +1160,7 @@ func TestUnprepareResources(t *testing.T) {
 				},
 				prepared: true,
 			},
+			resp:                   &drapbv1.NodeUnprepareResourcesResponse{Claims: map[string]*drapbv1.NodeUnprepareResourceResponse{"": {}}},
 			expectedUnprepareCalls: 1,
 		},
 		{
@@ -1055,6 +1209,59 @@ func TestUnprepareResources(t *testing.T) {
 				},
 				prepared: false,
 			},
+			resp:                   &drapbv1.NodeUnprepareResourcesResponse{Claims: map[string]*drapbv1.NodeUnprepareResourceResponse{"": {}}},
+			expectedUnprepareCalls: 1,
+		},
+		{
+			description: "should unprepare resource, driver returns nil value",
+			driverName:  driverName,
+			pod: &v1.Pod{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-pod",
+					Namespace: "test-namespace",
+					UID:       "test-reserved",
+				},
+				Spec: v1.PodSpec{
+					ResourceClaims: []v1.PodResourceClaim{
+						{
+							Name: "test-pod-claim-nil",
+							Source: v1.ClaimSource{
+								ResourceClaimName: func() *string {
+									s := "test-pod-claim-nil"
+									return &s
+								}(),
+							},
+						},
+					},
+					Containers: []v1.Container{
+						{
+							Resources: v1.ResourceRequirements{
+								Claims: []v1.ResourceClaim{
+									{
+										Name: "test-pod-claim-nil",
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			claimInfo: &ClaimInfo{
+				ClaimInfoState: state.ClaimInfoState{
+					DriverName: driverName,
+					ClaimName:  "test-pod-claim-nil",
+					Namespace:  "test-namespace",
+					ClaimUID:   "test-reserved",
+					ResourceHandles: []resourcev1alpha2.ResourceHandle{
+						{
+							DriverName: driverName,
+							Data:       "test data",
+						},
+					},
+				},
+				prepared: true,
+			},
+			resp:                   &drapbv1.NodeUnprepareResourcesResponse{Claims: map[string]*drapbv1.NodeUnprepareResourceResponse{"test-reserved": nil}},
 			expectedUnprepareCalls: 1,
 		},
 	} {
@@ -1070,7 +1277,7 @@ func TestUnprepareResources(t *testing.T) {
 				pluginClientTimeout = &timeout
 			}
 
-			draServerInfo, err := setupFakeDRADriverGRPCServer(test.wantTimeout, pluginClientTimeout)
+			draServerInfo, err := setupFakeDRADriverGRPCServer(test.wantTimeout, pluginClientTimeout, nil, test.resp)
 			if err != nil {
 				t.Fatal(err)
 			}