Add more unit tests and comments

Author: xxx0624

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)\"" \

ecs-agent/daemonimages/csidriver/driver/driver.go

@@ -25,17 +25,21 @@ 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
 
 	srv     *grpc.Server
 	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",

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
 }

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
 }
 

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")
 	}
 

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")
 			},
 		},
 	}

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
 }

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
 }

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")
+			}
+		})
+	}
+
+}