Merge pull request #124091 from bitoku/dra-nil-check

kubelet: add nil check for Node(Un)PrepareResources.
This commit is contained in:
Kubernetes Prow Robot 2024-04-18 10:46:05 -07:00 committed by GitHub
commit bbfd2145de
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 227 additions and 20 deletions

View File

@ -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)
}

View File

@ -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)
}