From cdd900dfbefaaa2e54d63a47fddb5322f1d4961c Mon Sep 17 00:00:00 2001 From: Xing Zheng Date: Thu, 29 Jun 2023 03:41:26 +0000 Subject: [PATCH] Add more unit tests and comments --- ecs-agent/daemonimages/csidriver/Makefile | 6 +- .../daemonimages/csidriver/driver/driver.go | 4 + .../daemonimages/csidriver/driver/mount.go | 4 +- .../daemonimages/csidriver/driver/node.go | 4 +- ecs-agent/daemonimages/csidriver/options.go | 10 ++- .../daemonimages/csidriver/options_test.go | 2 +- .../daemonimages/csidriver/util/fs/fs.go | 38 +++++---- .../csidriver/util/fs/fs_windows.go | 29 +++++-- .../daemonimages/csidriver/util/utils_test.go | 82 +++++++++++++++++++ 9 files changed, 145 insertions(+), 34 deletions(-) create mode 100644 ecs-agent/daemonimages/csidriver/util/utils_test.go diff --git a/ecs-agent/daemonimages/csidriver/Makefile b/ecs-agent/daemonimages/csidriver/Makefile index 169cc79e87..1ef3ae2ba9 100644 --- a/ecs-agent/daemonimages/csidriver/Makefile +++ b/ecs-agent/daemonimages/csidriver/Makefile @@ -3,15 +3,11 @@ GO111MODULE=on GOPATH=$(shell go env GOPATH) BUILD_DATE=$(shell date -u -Iseconds) -SOURCEDIR=./ -CSI_DRIVER_SOURCES=$(shell find $(SOURCEDIR) -name '*.go') - OS?=linux ARCH?=amd64 -# Build binary for linux only .PHONY: bin/csi-driver -bin/csi-driver: $(CSI_DRIVER_SOURCES) +bin/csi-driver: CGO_ENABLED=0 GOOS=$(OS) GOARCH=$(ARCH) go build -ldflags "\ -X \"github.com/aws/amazon-ecs-agent/ecs-agent/daemon_images/csi-driver/version.version=$(CSI_DRIVER_VERSION)\" \ -X \"github.com/aws/amazon-ecs-agent/ecs-agent/daemon_images/csi-driver/version.buildDate=$(BUILD_DATE)\"" \ diff --git a/ecs-agent/daemonimages/csidriver/driver/driver.go b/ecs-agent/daemonimages/csidriver/driver/driver.go index b83f69e508..46b4a0cba8 100644 --- a/ecs-agent/daemonimages/csidriver/driver/driver.go +++ b/ecs-agent/daemonimages/csidriver/driver/driver.go @@ -25,6 +25,8 @@ import ( "github.com/aws/amazon-ecs-agent/ecs-agent/daemonimages/csidriver/version" ) +// Driver encapsulates the GRPC server and the node service which implements all necessary interfaces, such as +// the EBS volume stats interface. type Driver struct { nodeService @@ -32,10 +34,12 @@ type Driver struct { options *DriverOptions } +// DriverOptions supports to custom the endpoint of the GRPC server. type DriverOptions struct { endpoint string } +// NewDriver creates a new driver. func NewDriver(options ...func(*DriverOptions)) (*Driver, error) { driverInfo := version.GetVersionInfo() klog.InfoS("Driver Information", diff --git a/ecs-agent/daemonimages/csidriver/driver/mount.go b/ecs-agent/daemonimages/csidriver/driver/mount.go index b8cbe114e7..6ee70f81d5 100644 --- a/ecs-agent/daemonimages/csidriver/driver/mount.go +++ b/ecs-agent/daemonimages/csidriver/driver/mount.go @@ -13,19 +13,21 @@ package driver -// Mounter is the interface implemented by NodeMounter. type Mounter interface { PathExists(path string) (bool, error) } // NodeMounter implements Mounter. type NodeMounter struct { + // TODO } func (nm NodeMounter) PathExists(path string) (bool, error) { + // TODO return false, nil } func newNodeMounter() (Mounter, error) { + // TODO return NodeMounter{}, nil } diff --git a/ecs-agent/daemonimages/csidriver/driver/node.go b/ecs-agent/daemonimages/csidriver/driver/node.go index 4a285b1614..9d05b335c5 100644 --- a/ecs-agent/daemonimages/csidriver/driver/node.go +++ b/ecs-agent/daemonimages/csidriver/driver/node.go @@ -17,9 +17,11 @@ import ( "github.com/container-storage-interface/spec/lib/go/csi" ) -// nodeService represents the node service of CSI driver +// nodeService represents the node service of CSI driver. type nodeService struct { mounter Mounter + // UnimplementedNodeServer implements all interfaces with empty implementation. As one mini version of csi driver, + // we only need to override the necessary interfaces. csi.UnimplementedNodeServer } diff --git a/ecs-agent/daemonimages/csidriver/options.go b/ecs-agent/daemonimages/csidriver/options.go index 93eda43aaf..31bfad2b62 100644 --- a/ecs-agent/daemonimages/csidriver/options.go +++ b/ecs-agent/daemonimages/csidriver/options.go @@ -19,7 +19,7 @@ import ( "os" ) -const EmptyCSIEndpoint = "" +const emptyCSIEndpoint = "" type ServerOptions struct { // Endpoint is the endpoint that the driver server should listen on. @@ -28,14 +28,18 @@ type ServerOptions struct { func GetServerOptions(fs *flag.FlagSet) (*ServerOptions, error) { serverOptions := &ServerOptions{} - fs.StringVar(&serverOptions.Endpoint, "endpoint", EmptyCSIEndpoint, "Endpoint for the CSI driver server") + fs.StringVar(&serverOptions.Endpoint, "endpoint", emptyCSIEndpoint, "Endpoint for the CSI driver server") args := os.Args[1:] if err := fs.Parse(args); err != nil { return nil, err } - if serverOptions.Endpoint == EmptyCSIEndpoint { + if len(args) == 0 { + return nil, errors.New("no argument is provided") + } + + if serverOptions.Endpoint == emptyCSIEndpoint { return nil, errors.New("no endpoint is provided") } diff --git a/ecs-agent/daemonimages/csidriver/options_test.go b/ecs-agent/daemonimages/csidriver/options_test.go index a415285e9a..3b8ce0e48d 100644 --- a/ecs-agent/daemonimages/csidriver/options_test.go +++ b/ecs-agent/daemonimages/csidriver/options_test.go @@ -55,7 +55,7 @@ func TestGetServerOptions(t *testing.T) { name: "No argument is given", testFunc: func(t *testing.T) { _, err := testFunc(t, nil) - assert.NotNil(t, err) + assert.EqualError(t, err, "no argument is provided") }, }, } diff --git a/ecs-agent/daemonimages/csidriver/util/fs/fs.go b/ecs-agent/daemonimages/csidriver/util/fs/fs.go index bcb5bf39dc..09173d7fe6 100644 --- a/ecs-agent/daemonimages/csidriver/util/fs/fs.go +++ b/ecs-agent/daemonimages/csidriver/util/fs/fs.go @@ -27,25 +27,33 @@ type UsageInfo struct { // Info linux returns (available bytes, byte capacity, byte usage, total inodes, inodes free, inode usage, error) // for the filesystem that path resides upon. -func Info(path string) (int64, int64, int64, int64, int64, int64, error) { +func Info(path string) ( + available int64, capacity int64, used int64, + totalInodes int64, freeInodes int64, usedInodes int64, + err error, +) { statfs := &unix.Statfs_t{} - err := unix.Statfs(path, statfs) + err = unix.Statfs(path, statfs) if err != nil { - return 0, 0, 0, 0, 0, 0, err + available = 0 + capacity = 0 + used = 0 + + totalInodes = 0 + freeInodes = 0 + usedInodes = 0 + return } // Available is blocks available * fragment size - available := int64(statfs.Bavail) * int64(statfs.Bsize) - + available = int64(statfs.Bavail) * int64(statfs.Bsize) // Capacity is total block count * fragment size - capacity := int64(statfs.Blocks) * int64(statfs.Bsize) - - // Usage is block being used * fragment size (aka block size). - usage := (int64(statfs.Blocks) - int64(statfs.Bfree)) * int64(statfs.Bsize) - - inodes := int64(statfs.Files) - inodesFree := int64(statfs.Ffree) - inodesUsed := inodes - inodesFree - - return available, capacity, usage, inodes, inodesFree, inodesUsed, nil + capacity = int64(statfs.Blocks) * int64(statfs.Bsize) + // Used is block being used * fragment size (aka block size). + used = (int64(statfs.Blocks) - int64(statfs.Bfree)) * int64(statfs.Bsize) + + totalInodes = int64(statfs.Files) + freeInodes = int64(statfs.Ffree) + usedInodes = totalInodes - freeInodes + return } diff --git a/ecs-agent/daemonimages/csidriver/util/fs/fs_windows.go b/ecs-agent/daemonimages/csidriver/util/fs/fs_windows.go index 1748a92514..70c58b67e0 100644 --- a/ecs-agent/daemonimages/csidriver/util/fs/fs_windows.go +++ b/ecs-agent/daemonimages/csidriver/util/fs/fs_windows.go @@ -36,10 +36,11 @@ type UsageInfo struct { // Info returns (available bytes, byte capacity, byte usage, total inodes, inodes free, inode usage, error) // for the filesystem that path resides upon. -func Info(path string) (int64, int64, int64, int64, int64, int64, error) { - var freeBytesAvailable, totalNumberOfBytes, totalNumberOfFreeBytes int64 - var err error - +func Info(path string) ( + available int64, capacity int64, used int64, + totalInodes int64, freeInodes int64, usedInodes int64, + err error) { + var totalNumberOfFreeBytes int64 // The equivalent linux call supports calls against files but the syscall for windows // fails for files with error code: The directory name is invalid. (#99173) // https://docs.microsoft.com/en-us/windows/win32/debug/system-error-codes--0-499- @@ -49,15 +50,27 @@ func Info(path string) (int64, int64, int64, int64, int64, int64, error) { procGetDiskFreeSpaceEx.Addr(), 4, uintptr(unsafe.Pointer(syscall.StringToUTF16Ptr(path))), - uintptr(unsafe.Pointer(&freeBytesAvailable)), - uintptr(unsafe.Pointer(&totalNumberOfBytes)), + uintptr(unsafe.Pointer(&available)), + uintptr(unsafe.Pointer(&capacity)), uintptr(unsafe.Pointer(&totalNumberOfFreeBytes)), 0, 0, ) if ret == 0 { - return 0, 0, 0, 0, 0, 0, err + available = 0 + capacity = 0 + used = 0 + + totalInodes = 0 + freeInodes = 0 + usedInodes = 0 + + return } - return freeBytesAvailable, totalNumberOfBytes, totalNumberOfBytes - freeBytesAvailable, 0, 0, 0, nil + used = capacity - available + totalInodes = 0 + freeInodes = 0 + usedInodes = 0 + return } diff --git a/ecs-agent/daemonimages/csidriver/util/utils_test.go b/ecs-agent/daemonimages/csidriver/util/utils_test.go new file mode 100644 index 0000000000..c0e502306d --- /dev/null +++ b/ecs-agent/daemonimages/csidriver/util/utils_test.go @@ -0,0 +1,82 @@ +// 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 ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestParseEndpoint(t *testing.T) { + testCases := []struct { + name string + endpoint string + expScheme string + expAddr string + expErr error + }{ + { + name: "valid unix endpoint 1", + endpoint: "unix:///csi/csi.sock", + expScheme: "unix", + expAddr: "/csi/csi.sock", + }, + { + name: "valid unix endpoint 2", + endpoint: "unix://csi/csi.sock", + expScheme: "unix", + expAddr: "/csi/csi.sock", + }, + { + name: "valid unix endpoint 3", + endpoint: "unix:/csi/csi.sock", + expScheme: "unix", + expAddr: "/csi/csi.sock", + }, + { + name: "valid tcp endpoint", + endpoint: "tcp:///127.0.0.1/", + expScheme: "tcp", + expAddr: "/127.0.0.1", + }, + { + name: "valid tcp endpoint", + endpoint: "tcp:///127.0.0.1", + expScheme: "tcp", + expAddr: "/127.0.0.1", + }, + { + name: "invalid endpoint", + endpoint: "http://127.0.0.1", + expErr: fmt.Errorf("unsupported protocol: http"), + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + scheme, addr, err := ParseEndpoint(tc.endpoint) + + if tc.expErr != nil { + assert.EqualError(t, err, tc.expErr.Error()) + } else { + assert.Nil(t, err) + assert.Equal(t, scheme, tc.expScheme, "scheme mismatches") + assert.Equal(t, addr, tc.expAddr, "address mismatches") + } + }) + } + +}