From 137061c3fd19974b7ff3df12deef9b7084a5c352 Mon Sep 17 00:00:00 2001 From: Xiang Li Date: Tue, 19 Jan 2021 11:36:02 -0800 Subject: [PATCH] no operation for block device in NodeExpandVolume --- pkg/driver/node.go | 49 +++++++++++++++++++++-- pkg/driver/node_test.go | 86 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 131 insertions(+), 4 deletions(-) diff --git a/pkg/driver/node.go b/pkg/driver/node.go index 68329998d5..c57d7d2101 100644 --- a/pkg/driver/node.go +++ b/pkg/driver/node.go @@ -254,14 +254,49 @@ func (d *nodeService) NodeExpandVolume(ctx context.Context, req *csi.NodeExpandV if len(volumeID) == 0 { return nil, status.Error(codes.InvalidArgument, "Volume ID not provided") } + volumePath := req.GetVolumePath() + if len(volumePath) == 0 { + return nil, status.Error(codes.InvalidArgument, "volume path must be provided") + } + + volumeCapability := req.GetVolumeCapability() + // VolumeCapability is optional, if specified, use that as source of truth + if volumeCapability != nil { + caps := []*csi.VolumeCapability{volumeCapability} + if !isValidVolumeCapabilities(caps) { + return nil, status.Error(codes.InvalidArgument, fmt.Sprintf("VolumeCapability is invalid: %v", volumeCapability)) + } + + if blk := volumeCapability.GetBlock(); blk != nil { + // Noop for Block NodeExpandVolume + klog.V(4).Infof("NodeExpandVolume called for %v at %s. Since it is a block device, ignoring...", volumeID, volumePath) + return &csi.NodeExpandVolumeResponse{}, nil + } + } else { + // VolumeCapability is nil, check if volumePath point to a block device + isBlock, err := d.IsBlockDevice(volumePath) + if err != nil { + return nil, status.Errorf(codes.Internal, "failed to determine device path for volumePath [%v]: %v", volumePath, err) + } + if isBlock { + // Skip resizing for Block NodeExpandVolume + bcap, err := d.getBlockSizeBytes(volumePath) + if err != nil { + return nil, status.Errorf(codes.Internal, "failed to get block capacity on path %s: %v", req.VolumePath, err) + } + klog.V(4).Infof("NodeExpandVolume called for %v at %s, since given volumePath is a block device, ignoring...", volumeID, volumePath) + return &csi.NodeExpandVolumeResponse{ + CapacityBytes: bcap, + }, nil + } + } - args := []string{"-o", "source", "--noheadings", "--target", req.GetVolumePath()} + args := []string{"-o", "source", "--noheadings", "--target", volumePath} output, err := d.mounter.Command("findmnt", args...).Output() if err != nil { return nil, status.Errorf(codes.Internal, "Could not determine device path: %v", err) } - devicePath := strings.TrimSpace(string(output)) if len(devicePath) == 0 { return nil, status.Errorf(codes.Internal, "Could not get valid device for mount path: %q", req.GetVolumePath()) @@ -274,11 +309,17 @@ func (d *nodeService) NodeExpandVolume(ctx context.Context, req *csi.NodeExpandV }) // TODO: lock per volume ID to have some idempotency - if _, err := r.Resize(devicePath, req.GetVolumePath()); err != nil { + if _, err := r.Resize(devicePath, volumePath); err != nil { return nil, status.Errorf(codes.Internal, "Could not resize volume %q (%q): %v", volumeID, devicePath, err) } - return &csi.NodeExpandVolumeResponse{}, nil + bcap, err := d.getBlockSizeBytes(devicePath) + if err != nil { + return nil, status.Errorf(codes.Internal, "failed to get block capacity on path %s: %v", req.VolumePath, err) + } + return &csi.NodeExpandVolumeResponse{ + CapacityBytes: bcap, + }, nil } func (d *nodeService) NodePublishVolume(ctx context.Context, req *csi.NodePublishVolumeRequest) (*csi.NodePublishVolumeResponse, error) { diff --git a/pkg/driver/node_test.go b/pkg/driver/node_test.go index 12f88dee5f..3a2572db19 100644 --- a/pkg/driver/node_test.go +++ b/pkg/driver/node_test.go @@ -1082,6 +1082,92 @@ func TestNodePublishVolume(t *testing.T) { t.Run(tc.name, tc.testFunc) } } +func TestNodeExpandVolume(t *testing.T) { + mockCtl := gomock.NewController(t) + defer mockCtl.Finish() + + mockMetadata := mocks.NewMockMetadataService(mockCtl) + mockMounter := mocks.NewMockMounter(mockCtl) + + awsDriver := &nodeService{ + metadata: mockMetadata, + mounter: mockMounter, + inFlight: internal.NewInFlight(), + } + + tests := []struct { + name string + request csi.NodeExpandVolumeRequest + expectResponseCode codes.Code + }{ + { + name: "fail missing volumeId", + request: csi.NodeExpandVolumeRequest{}, + expectResponseCode: codes.InvalidArgument, + }, + { + name: "fail missing volumePath", + request: csi.NodeExpandVolumeRequest{ + StagingTargetPath: "/testDevice/Path", + VolumeId: "test-volume-id", + }, + expectResponseCode: codes.InvalidArgument, + }, + { + name: "fail volume path not exist", + request: csi.NodeExpandVolumeRequest{ + VolumePath: "./test", + VolumeId: "test-volume-id", + }, + expectResponseCode: codes.Internal, + }, + { + name: "Fail validate VolumeCapability", + request: csi.NodeExpandVolumeRequest{ + VolumePath: "./test", + VolumeId: "test-volume-id", + VolumeCapability: &csi.VolumeCapability{ + AccessType: &csi.VolumeCapability_Block{ + Block: &csi.VolumeCapability_BlockVolume{}, + }, + AccessMode: &csi.VolumeCapability_AccessMode{ + Mode: csi.VolumeCapability_AccessMode_UNKNOWN, + }, + }, + }, + expectResponseCode: codes.InvalidArgument, + }, + { + name: "Success [VolumeCapability is block]", + request: csi.NodeExpandVolumeRequest{ + VolumePath: "./test", + VolumeId: "test-volume-id", + VolumeCapability: &csi.VolumeCapability{ + AccessType: &csi.VolumeCapability_Block{ + Block: &csi.VolumeCapability_BlockVolume{}, + }, + AccessMode: &csi.VolumeCapability_AccessMode{ + Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER, + }, + }, + }, + expectResponseCode: codes.OK, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + _, err := awsDriver.NodeExpandVolume(context.Background(), &test.request) + if err != nil { + if test.expectResponseCode != codes.OK { + expectErr(t, err, test.expectResponseCode) + } else { + t.Fatalf("Expect no error but got: %v", err) + } + } + }) + } +} func TestNodeUnpublishVolume(t *testing.T) { targetPath := "/test/path"