diff --git a/pkg/kubelet/cm/dra/manager.go b/pkg/kubelet/cm/dra/manager.go index 100deaae637..ad9b17dfbd8 100644 --- a/pkg/kubelet/cm/dra/manager.go +++ b/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) } diff --git a/pkg/kubelet/cm/dra/manager_test.go b/pkg/kubelet/cm/dra/manager_test.go index a802ca155bf..ffda44dd828 100644 --- a/pkg/kubelet/cm/dra/manager_test.go +++ b/pkg/kubelet/cm/dra/manager_test.go @@ -47,10 +47,12 @@ const ( type fakeDRADriverGRPCServer struct { drapbv1.UnimplementedNodeServer - driverName string - timeout *time.Duration - prepareResourceCalls atomic.Uint32 - unprepareResourceCalls atomic.Uint32 + driverName string + 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 &drapbv1.NodePrepareResourcesResponse{Claims: map[string]*drapbv1.NodePrepareResourceResponse{req.Claims[0].Uid: {CDIDevices: []string{result}}}}, nil + + return s.prepareResourcesResponse, 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{ @@ -114,7 +116,9 @@ func setupFakeDRADriverGRPCServer(shouldTimeout bool, pluginClientTimeout *time. s := grpc.NewServer() fakeDRADriverGRPCServer := &fakeDRADriverGRPCServer{ - driverName: driverName, + 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)) - if len(claimInfo.CDIDevices[test.resourceClaim.Status.DriverName]) != 1 || claimInfo.CDIDevices[test.resourceClaim.Status.DriverName][0] != expectedResourceClaimDriverName { - t.Fatalf("cdiDevices mismatch: expected [%s], got %v", []string{expectedResourceClaimDriverName}, claimInfo.CDIDevices[test.resourceClaim.Status.DriverName]) - } + assert.ElementsMatchf(t, claimInfo.CDIDevices[test.resourceClaim.Status.DriverName], test.expectedCDIDevices, + "cdiDevices mismatch: expected [%v], got %v", test.expectedCDIDevices, 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) }