From 86f7309499cd486e6c0d14e815d8d77d3b7cf137 Mon Sep 17 00:00:00 2001 From: andyzhangx Date: Sat, 23 Nov 2024 03:04:36 +0000 Subject: [PATCH] chore: fix ut --- pkg/nfs/controllerserver.go | 5 ++++ pkg/nfs/identityserver.go | 1 + pkg/nfs/nodeserver.go | 7 +++--- pkg/nfs/nodeserver_test.go | 50 ++++++++++++++++++------------------- 4 files changed, 35 insertions(+), 28 deletions(-) diff --git a/pkg/nfs/controllerserver.go b/pkg/nfs/controllerserver.go index 39a38e67..f160c645 100644 --- a/pkg/nfs/controllerserver.go +++ b/pkg/nfs/controllerserver.go @@ -40,6 +40,7 @@ import ( // ControllerServer controller server setting type ControllerServer struct { Driver *Driver + csi.UnimplementedControllerServer } // nfsVolume is an internal representation of a volume @@ -347,6 +348,10 @@ func (cs *ControllerServer) GetCapacity(_ context.Context, _ *csi.GetCapacityReq return nil, status.Error(codes.Unimplemented, "") } +func (d *Driver) ControllerModifyVolume(_ context.Context, _ *csi.ControllerModifyVolumeRequest) (*csi.ControllerModifyVolumeResponse, error) { + return nil, status.Error(codes.Unimplemented, "") +} + // ControllerGetCapabilities implements the default GRPC callout. // Default supports all capabilities func (cs *ControllerServer) ControllerGetCapabilities(_ context.Context, _ *csi.ControllerGetCapabilitiesRequest) (*csi.ControllerGetCapabilitiesResponse, error) { diff --git a/pkg/nfs/identityserver.go b/pkg/nfs/identityserver.go index d76fcf49..ed050cb9 100644 --- a/pkg/nfs/identityserver.go +++ b/pkg/nfs/identityserver.go @@ -26,6 +26,7 @@ import ( type IdentityServer struct { Driver *Driver + csi.UnimplementedIdentityServer } func (ids *IdentityServer) GetPluginInfo(_ context.Context, _ *csi.GetPluginInfoRequest) (*csi.GetPluginInfoResponse, error) { diff --git a/pkg/nfs/nodeserver.go b/pkg/nfs/nodeserver.go index afbde9d0..6f11d6e1 100644 --- a/pkg/nfs/nodeserver.go +++ b/pkg/nfs/nodeserver.go @@ -38,6 +38,7 @@ import ( type NodeServer struct { Driver *Driver mounter mount.Interface + csi.UnimplementedNodeServer } // NodePublishVolume mount the volume @@ -223,9 +224,9 @@ func (ns *NodeServer) NodeGetVolumeStats(_ context.Context, req *csi.NodeGetVolu return nil, status.Errorf(codes.Internal, "%v", err) } if cache != nil { - resp := cache.(csi.NodeGetVolumeStatsResponse) + resp := cache.(*csi.NodeGetVolumeStatsResponse) klog.V(6).Infof("NodeGetVolumeStats: volume stats for volume %s path %s is cached", req.VolumeId, req.VolumePath) - return &resp, nil + return resp, nil } if _, err := os.Lstat(req.VolumePath); err != nil { @@ -284,7 +285,7 @@ func (ns *NodeServer) NodeGetVolumeStats(_ context.Context, req *csi.NodeGetVolu } // cache the volume stats per volume - ns.Driver.volStatsCache.Set(req.VolumeId, resp) + ns.Driver.volStatsCache.Set(req.VolumeId, &resp) return &resp, err } diff --git a/pkg/nfs/nodeserver_test.go b/pkg/nfs/nodeserver_test.go index 224c1056..0ae5bd47 100644 --- a/pkg/nfs/nodeserver_test.go +++ b/pkg/nfs/nodeserver_test.go @@ -74,24 +74,24 @@ func TestNodePublishVolume(t *testing.T) { tests := []struct { desc string setup func() - req csi.NodePublishVolumeRequest + req *csi.NodePublishVolumeRequest skipOnWindows bool expectedErr error cleanup func() }{ { desc: "[Error] Volume capabilities missing", - req: csi.NodePublishVolumeRequest{}, + req: &csi.NodePublishVolumeRequest{}, expectedErr: status.Error(codes.InvalidArgument, "Volume capability missing in request"), }, { desc: "[Error] Volume ID missing", - req: csi.NodePublishVolumeRequest{VolumeCapability: &csi.VolumeCapability{AccessMode: &volumeCap}}, + req: &csi.NodePublishVolumeRequest{VolumeCapability: &csi.VolumeCapability{AccessMode: &volumeCap}}, expectedErr: status.Error(codes.InvalidArgument, "Volume ID missing in request"), }, { desc: "[Error] Target path missing", - req: csi.NodePublishVolumeRequest{VolumeCapability: &csi.VolumeCapability{AccessMode: &volumeCap}, + req: &csi.NodePublishVolumeRequest{VolumeCapability: &csi.VolumeCapability{AccessMode: &volumeCap}, VolumeId: "vol_1"}, expectedErr: status.Error(codes.InvalidArgument, "Target path not provided"), }, @@ -100,7 +100,7 @@ func TestNodePublishVolume(t *testing.T) { setup: func() { ns.Driver.volumeLocks.TryAcquire(lockKey) }, - req: csi.NodePublishVolumeRequest{VolumeCapability: &csi.VolumeCapability{AccessMode: &volumeCap}, + req: &csi.NodePublishVolumeRequest{VolumeCapability: &csi.VolumeCapability{AccessMode: &volumeCap}, VolumeId: "vol_1", VolumeContext: params, TargetPath: targetTest}, @@ -111,7 +111,7 @@ func TestNodePublishVolume(t *testing.T) { }, { desc: "[Success] Stage target path missing", - req: csi.NodePublishVolumeRequest{ + req: &csi.NodePublishVolumeRequest{ VolumeContext: params, VolumeCapability: &csi.VolumeCapability{AccessMode: &volumeCap}, VolumeId: "vol_1", @@ -120,7 +120,7 @@ func TestNodePublishVolume(t *testing.T) { }, { desc: "[Success] Valid request read only", - req: csi.NodePublishVolumeRequest{ + req: &csi.NodePublishVolumeRequest{ VolumeContext: params, VolumeCapability: &csi.VolumeCapability{AccessMode: &volumeCap}, VolumeId: "vol_1", @@ -130,7 +130,7 @@ func TestNodePublishVolume(t *testing.T) { }, { desc: "[Success] Valid request already mounted", - req: csi.NodePublishVolumeRequest{ + req: &csi.NodePublishVolumeRequest{ VolumeContext: params, VolumeCapability: &csi.VolumeCapability{AccessMode: &volumeCap}, VolumeId: "vol_1", @@ -140,7 +140,7 @@ func TestNodePublishVolume(t *testing.T) { }, { desc: "[Success] Valid request", - req: csi.NodePublishVolumeRequest{ + req: &csi.NodePublishVolumeRequest{ VolumeContext: params, VolumeCapability: &csi.VolumeCapability{AccessMode: &volumeCap}, VolumeId: "vol_1", @@ -150,7 +150,7 @@ func TestNodePublishVolume(t *testing.T) { }, { desc: "[Success] Valid request with pv/pvc metadata", - req: csi.NodePublishVolumeRequest{ + req: &csi.NodePublishVolumeRequest{ VolumeContext: paramsWithMetadata, VolumeCapability: &csi.VolumeCapability{AccessMode: &volumeCap}, VolumeId: "vol_1", @@ -160,7 +160,7 @@ func TestNodePublishVolume(t *testing.T) { }, { desc: "[Success] Valid request with 0 mountPermissions", - req: csi.NodePublishVolumeRequest{ + req: &csi.NodePublishVolumeRequest{ VolumeContext: paramsWithZeroPermissions, VolumeCapability: &csi.VolumeCapability{AccessMode: &volumeCap}, VolumeId: "vol_1", @@ -170,7 +170,7 @@ func TestNodePublishVolume(t *testing.T) { }, { desc: "[Error] invalid mountPermissions", - req: csi.NodePublishVolumeRequest{ + req: &csi.NodePublishVolumeRequest{ VolumeContext: invalidParams, VolumeCapability: &csi.VolumeCapability{AccessMode: &volumeCap}, VolumeId: "vol_1", @@ -188,7 +188,7 @@ func TestNodePublishVolume(t *testing.T) { if tc.setup != nil { tc.setup() } - _, err := ns.NodePublishVolume(context.Background(), &tc.req) + _, err := ns.NodePublishVolume(context.Background(), tc.req) if !reflect.DeepEqual(err, tc.expectedErr) { t.Errorf("Desc:%v\nUnexpected error: %v\nExpected: %v", tc.desc, err, tc.expectedErr) } @@ -219,30 +219,30 @@ func TestNodeUnpublishVolume(t *testing.T) { tests := []struct { desc string setup func() - req csi.NodeUnpublishVolumeRequest + req *csi.NodeUnpublishVolumeRequest expectedErr error cleanup func() }{ { desc: "[Error] Volume ID missing", - req: csi.NodeUnpublishVolumeRequest{TargetPath: targetTest}, + req: &csi.NodeUnpublishVolumeRequest{TargetPath: targetTest}, expectedErr: status.Error(codes.InvalidArgument, "Volume ID missing in request"), }, { desc: "[Error] Target missing", - req: csi.NodeUnpublishVolumeRequest{VolumeId: "vol_1"}, + req: &csi.NodeUnpublishVolumeRequest{VolumeId: "vol_1"}, expectedErr: status.Error(codes.InvalidArgument, "Target path missing in request"), }, { desc: "[Success] Volume not mounted", - req: csi.NodeUnpublishVolumeRequest{TargetPath: targetFile, VolumeId: "vol_1"}, + req: &csi.NodeUnpublishVolumeRequest{TargetPath: targetFile, VolumeId: "vol_1"}, }, { desc: "[Error] Volume operation in progress", setup: func() { ns.Driver.volumeLocks.TryAcquire(lockKey) }, - req: csi.NodeUnpublishVolumeRequest{TargetPath: targetTest, VolumeId: "vol_1"}, + req: &csi.NodeUnpublishVolumeRequest{TargetPath: targetTest, VolumeId: "vol_1"}, expectedErr: status.Error(codes.Aborted, fmt.Sprintf(volumeOperationAlreadyExistsFmt, "vol_1")), cleanup: func() { ns.Driver.volumeLocks.Release(lockKey) @@ -257,7 +257,7 @@ func TestNodeUnpublishVolume(t *testing.T) { if tc.setup != nil { tc.setup() } - _, err := ns.NodeUnpublishVolume(context.Background(), &tc.req) + _, err := ns.NodeUnpublishVolume(context.Background(), tc.req) if !reflect.DeepEqual(err, tc.expectedErr) { if err == nil || tc.expectedErr == nil || !strings.Contains(err.Error(), tc.expectedErr.Error()) { t.Errorf("Desc:%v\nUnexpected error: %v\nExpected: %v", tc.desc, err, tc.expectedErr) @@ -329,27 +329,27 @@ func TestNodeGetVolumeStats(t *testing.T) { tests := []struct { desc string - req csi.NodeGetVolumeStatsRequest + req *csi.NodeGetVolumeStatsRequest expectedErr error }{ { desc: "[Error] Volume ID missing", - req: csi.NodeGetVolumeStatsRequest{VolumePath: targetTest}, + req: &csi.NodeGetVolumeStatsRequest{VolumePath: targetTest}, expectedErr: status.Error(codes.InvalidArgument, "NodeGetVolumeStats volume ID was empty"), }, { desc: "[Error] VolumePath missing", - req: csi.NodeGetVolumeStatsRequest{VolumeId: "vol_1"}, + req: &csi.NodeGetVolumeStatsRequest{VolumeId: "vol_1"}, expectedErr: status.Error(codes.InvalidArgument, "NodeGetVolumeStats volume path was empty"), }, { desc: "[Error] Incorrect volume path", - req: csi.NodeGetVolumeStatsRequest{VolumePath: nonexistedPath, VolumeId: "vol_1"}, + req: &csi.NodeGetVolumeStatsRequest{VolumePath: nonexistedPath, VolumeId: "vol_1"}, expectedErr: status.Errorf(codes.NotFound, "path /not/a/real/directory does not exist"), }, { desc: "[Success] Standard success", - req: csi.NodeGetVolumeStatsRequest{VolumePath: fakePath, VolumeId: "vol_1"}, + req: &csi.NodeGetVolumeStatsRequest{VolumePath: fakePath, VolumeId: "vol_1"}, expectedErr: nil, }, } @@ -362,7 +362,7 @@ func TestNodeGetVolumeStats(t *testing.T) { } for _, test := range tests { - _, err := ns.NodeGetVolumeStats(context.Background(), &test.req) + _, err := ns.NodeGetVolumeStats(context.Background(), test.req) if !reflect.DeepEqual(err, test.expectedErr) { t.Errorf("desc: %v, expected error: %v, actual error: %v", test.desc, test.expectedErr, err) }