diff --git a/ecs-agent/daemonimages/csidriver/Makefile b/ecs-agent/daemonimages/csidriver/Makefile index f4ca70ae882..b884e597c7b 100644 --- a/ecs-agent/daemonimages/csidriver/Makefile +++ b/ecs-agent/daemonimages/csidriver/Makefile @@ -18,14 +18,6 @@ build-csi-driver: bin/csi-driver ./build-csi-driver-image $(MAKE) clean -.PHONY: test -test: - go test -v -race -tags unit -timeout=60s ./... - -.PHONY: get-deps -get-deps: - go install github.com/golang/mock/mockgen@v1.6.0 - .PHONY: mockgen mockgen: ./update-gomock diff --git a/ecs-agent/daemonimages/csidriver/driver/mock_mount.go b/ecs-agent/daemonimages/csidriver/driver/mock_mount.go index 81820dd43be..c597d8a87e1 100644 --- a/ecs-agent/daemonimages/csidriver/driver/mock_mount.go +++ b/ecs-agent/daemonimages/csidriver/driver/mock_mount.go @@ -8,12 +8,10 @@ import ( reflect "reflect" gomock "github.com/golang/mock/gomock" - mount_utils "k8s.io/mount-utils" ) // MockMounter is a mock of Mounter interface. type MockMounter struct { - mount_utils.Interface ctrl *gomock.Controller recorder *MockMounterMockRecorder } diff --git a/ecs-agent/daemonimages/csidriver/driver/node.go b/ecs-agent/daemonimages/csidriver/driver/node.go index 1250062cf04..86de82c484f 100644 --- a/ecs-agent/daemonimages/csidriver/driver/node.go +++ b/ecs-agent/daemonimages/csidriver/driver/node.go @@ -21,6 +21,7 @@ import ( "google.golang.org/grpc/status" "k8s.io/klog/v2" + "github.com/aws/amazon-ecs-agent/ecs-agent/daemonimages/csidriver/util" "github.com/aws/amazon-ecs-agent/ecs-agent/daemonimages/csidriver/volume" ) @@ -62,7 +63,7 @@ func (d *nodeService) NodeGetVolumeStats(ctx context.Context, req *csi.NodeGetVo return nil, status.Errorf(codes.NotFound, "path %s does not exist", req.VolumePath) } - isBlock, err := d.IsBlockDevice(req.VolumePath) + isBlock, err := util.IsBlockDevice(req.VolumePath) if err != nil { return nil, status.Errorf(codes.Internal, "failed to determine whether %s is block device: %v", req.VolumePath, err) } diff --git a/ecs-agent/daemonimages/csidriver/driver/node_linux.go b/ecs-agent/daemonimages/csidriver/driver/node_linux.go index 94e4035cc8c..173ee582a64 100644 --- a/ecs-agent/daemonimages/csidriver/driver/node_linux.go +++ b/ecs-agent/daemonimages/csidriver/driver/node_linux.go @@ -20,21 +20,8 @@ import ( "fmt" "strconv" "strings" - - "golang.org/x/sys/unix" ) -// IsBlock checks if the given path is a block device -func (d *nodeService) IsBlockDevice(fullPath string) (bool, error) { - var st unix.Stat_t - err := unix.Stat(fullPath, &st) - if err != nil { - return false, err - } - - return (st.Mode & unix.S_IFMT) == unix.S_IFBLK, nil -} - func (d *nodeService) getBlockSizeBytes(devicePath string) (int64, error) { cmd := d.mounter.(*NodeMounter).Exec.Command("blockdev", "--getsize64", devicePath) output, err := cmd.Output() diff --git a/ecs-agent/daemonimages/csidriver/driver/node_test.go b/ecs-agent/daemonimages/csidriver/driver/node_test.go index 83d306c27b6..77fa77ec777 100644 --- a/ecs-agent/daemonimages/csidriver/driver/node_test.go +++ b/ecs-agent/daemonimages/csidriver/driver/node_test.go @@ -19,6 +19,7 @@ package driver import ( "context" "errors" + "fmt" "os" "testing" @@ -85,11 +86,11 @@ func TestNodeGetVolumeStats(t *testing.T) { VolumePath: VolumePath, } _, err := awsDriver.NodeGetVolumeStats(context.TODO(), req) - expectErr(t, err, codes.NotFound) + expectErrorCode(t, err, codes.NotFound) }, }, { - name: "fail can't determine block device", + name: "fail can't determine block device due to no such file", testFunc: func(t *testing.T) { mockCtl := gomock.NewController(t) defer mockCtl.Finish() @@ -108,7 +109,8 @@ func TestNodeGetVolumeStats(t *testing.T) { VolumePath: VolumePath, } _, err := awsDriver.NodeGetVolumeStats(context.TODO(), req) - expectErr(t, err, codes.Internal) + expectErrorCode(t, err, codes.Internal) + expectErrorMessage(t, err, fmt.Sprintf("failed to determine whether %s is block device:", VolumePath)) }, }, { @@ -131,7 +133,7 @@ func TestNodeGetVolumeStats(t *testing.T) { VolumePath: VolumePath, } _, err := awsDriver.NodeGetVolumeStats(context.TODO(), req) - expectErr(t, err, codes.Internal) + expectErrorCode(t, err, codes.Internal) }, }, } @@ -142,17 +144,30 @@ func TestNodeGetVolumeStats(t *testing.T) { } -func expectErr(t *testing.T, actualErr error, expectedCode codes.Code) { - if actualErr == nil { - t.Fatalf("Expect error but got no error") - } +func expectErrorCode(t *testing.T, actualErr error, expectedCode codes.Code) { + require.NotNil(t, actualErr, "Expect error but got no error") status, ok := status.FromError(actualErr) - if !ok { - t.Fatalf("Failed to get error status code from error: %v", actualErr) - } + require.True(t, ok, fmt.Sprintf("Failed to get error status from error: %v", actualErr)) + + require.Equal( + t, + expectedCode, + status.Code(), + fmt.Sprintf("Expected error code %d, got %d message %s", codes.InvalidArgument, status.Code(), status.Message()), + ) +} - if status.Code() != expectedCode { - t.Fatalf("Expected error code %d, got %d message %s", codes.InvalidArgument, status.Code(), status.Message()) - } +func expectErrorMessage(t *testing.T, actualErr error, expectedPartialMsg string) { + require.NotNil(t, actualErr, "Expect error but got no error") + + status, ok := status.FromError(actualErr) + require.True(t, ok, fmt.Sprintf("Failed to get error status from error: %v", actualErr)) + + require.Containsf( + t, + status.Message(), + expectedPartialMsg, + fmt.Sprintf("Expected partial error message %s", expectedPartialMsg), + ) } diff --git a/ecs-agent/daemonimages/csidriver/driver/node_windows.go b/ecs-agent/daemonimages/csidriver/driver/node_windows.go index 9f58c9bf3be..c33c2846ed5 100644 --- a/ecs-agent/daemonimages/csidriver/driver/node_windows.go +++ b/ecs-agent/daemonimages/csidriver/driver/node_windows.go @@ -18,12 +18,6 @@ package driver import "errors" -// IsBlockDevice checks if the given path is a block device -func (d *nodeService) IsBlockDevice(fullPath string) (bool, error) { - // TODO - return false, errors.New("not supported") -} - // getBlockSizeBytes gets the size of the disk in bytes func (d *nodeService) getBlockSizeBytes(devicePath string) (int64, error) { // TODO diff --git a/ecs-agent/daemonimages/csidriver/tarfiles/csi-driver-amd64.tar b/ecs-agent/daemonimages/csidriver/tarfiles/csi-driver-amd64.tar index e539c8e94fb..9b377087d7e 100644 Binary files a/ecs-agent/daemonimages/csidriver/tarfiles/csi-driver-amd64.tar and b/ecs-agent/daemonimages/csidriver/tarfiles/csi-driver-amd64.tar differ diff --git a/ecs-agent/daemonimages/csidriver/update-gomock b/ecs-agent/daemonimages/csidriver/update-gomock index 108cbc8adaa..62c3e857ca7 100755 --- a/ecs-agent/daemonimages/csidriver/update-gomock +++ b/ecs-agent/daemonimages/csidriver/update-gomock @@ -3,7 +3,3 @@ set -euo pipefail mockgen -package driver -destination=./driver/mock_mount.go -source ./driver/mount.go - -# Fixes "Mounter Type cannot implement 'Mounter' as it has a non-exported method and is defined in a different package" -# See https://github.com/kubernetes/mount-utils/commit/a20fcfb15a701977d086330b47b7efad51eb608e for context. -sed -i '/type MockMounter struct {/a \\tmount_utils.Interface' ./driver/mock_mount.go diff --git a/ecs-agent/daemonimages/csidriver/util/utils_linux.go b/ecs-agent/daemonimages/csidriver/util/utils_linux.go new file mode 100644 index 00000000000..513eb346320 --- /dev/null +++ b/ecs-agent/daemonimages/csidriver/util/utils_linux.go @@ -0,0 +1,30 @@ +//go:build linux +// +build linux + +// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"). You may +// not use this file except in compliance with the License. A copy of the +// License is located at +// +// http://aws.amazon.com/apache2.0/ +// +// or in the "license" file accompanying this file. This file is distributed +// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either +// express or implied. See the License for the specific language governing +// permissions and limitations under the License. + +package util + +import "golang.org/x/sys/unix" + +// IsBlockDevice checks if the given path is a block device +func IsBlockDevice(fullPath string) (bool, error) { + var st unix.Stat_t + err := unix.Stat(fullPath, &st) + if err != nil { + return false, err + } + + return (st.Mode & unix.S_IFMT) == unix.S_IFBLK, nil +} diff --git a/ecs-agent/daemonimages/csidriver/util/utils_linux_test.go b/ecs-agent/daemonimages/csidriver/util/utils_linux_test.go index e8845702507..6b7919b3c84 100644 --- a/ecs-agent/daemonimages/csidriver/util/utils_linux_test.go +++ b/ecs-agent/daemonimages/csidriver/util/utils_linux_test.go @@ -18,11 +18,25 @@ package util import ( "fmt" + "os" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) +func TestIsBlockDevice(t *testing.T) { + volumePath := "./test" + err := os.MkdirAll(volumePath, 0644) + require.NoError(t, err, "fail to create dir") + defer os.RemoveAll(volumePath) + + isBlockDevice, err := IsBlockDevice(volumePath) + + assert.Nil(t, err) + assert.Equal(t, false, isBlockDevice) +} + func TestParseEndpoint(t *testing.T) { testCases := []struct { name string diff --git a/ecs-agent/daemonimages/csidriver/util/utils_windows.go b/ecs-agent/daemonimages/csidriver/util/utils_windows.go new file mode 100644 index 00000000000..bf4e0facf59 --- /dev/null +++ b/ecs-agent/daemonimages/csidriver/util/utils_windows.go @@ -0,0 +1,25 @@ +//go:build windows +// +build windows + +// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"). You may +// not use this file except in compliance with the License. A copy of the +// License is located at +// +// http://aws.amazon.com/apache2.0/ +// +// or in the "license" file accompanying this file. This file is distributed +// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either +// express or implied. See the License for the specific language governing +// permissions and limitations under the License. + +package util + +import "errors" + +// IsBlockDevice checks if the given path is a block device +func IsBlockDevice(fullPath string) (bool, error) { + // TODO + return false, errors.New("not supported") +}