no operation for block device in NodeExpandVolume

kubernetes-sigs · Jan 19, 2021 · 137061c · 137061c
1 parent 0df45a5
commit 137061c
Show file tree

Hide file tree

Showing 2 changed files with 131 additions and 4 deletions.
diff --git 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
@@ -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"