feat(testing): properly implement k8s sanity tests & fix most of them

.gitignore

@@ -2,7 +2,7 @@
 /vendor
 
 # test cluster config
-kubeconfig
+test/.env
 
 # v1.x files
 _legacy

.gitlab-ci.yml

@@ -8,10 +8,9 @@ k8s sanity tests:
   before_script:
     - apk add --update git gcc musl-dev
   script:
-    - go test ./cmd/controller
+    - test/sanity
   except:
     - tags
-  allow_failure: true # TODO: remove me
 
 build dirty docker image:
   stage: test

cmd/controller/controller_test.go

@@ -24,7 +24,9 @@ func Test(t *testing.T) {
 	defer node.Stop()
 
 	sanity.Test(t, &sanity.Config{
-		Address:           nodeSocketPath,
-		ControllerAddress: controllerSocketPath,
+		Address:                  nodeSocketPath,
+		ControllerAddress:        controllerSocketPath,
+		SecretsFile:              "../../test/secrets.yml",
+		TestVolumeParametersFile: "../../test/config.yml",
 	})
 }

example/storageclass.yml

@@ -7,8 +7,6 @@ provisioner: dothill.csi.enix.io
 parameters:
   pool: B
   fsType: ext4
-  initiatorName: iqn.2019-05.io.enix:arthurs-dev-cluster
-  uniqueInitiatorNameByPvc: "false"
   iqn: iqn.2015-11.com.hpe:storage.msa2050.18323cc9ed
   portals: 10.14.84.211,10.14.84.212
 

pkg/common/driver.go

@@ -18,16 +18,14 @@ const PluginName = "dothill.csi.enix.io"
 
 // Configuration constants
 const (
-	FsTypeConfigKey                   = "fsType"
-	PoolConfigKey                     = "pool"
-	TargetIQNConfigKey                = "iqn"
-	PortalsConfigKey                  = "portals"
-	InitiatorNameConfigKey            = "initiatorName"
-	APIAddressConfigKey               = "apiAddress"
-	UniqueInitiatorNameByPvcConfigKey = "uniqueInitiatorNameByPvc"
-	UsernameSecretKey                 = "username"
-	PasswordSecretKey                 = "password"
-	StorageClassAnnotationKey         = "storageClass"
+	FsTypeConfigKey           = "fsType"
+	PoolConfigKey             = "pool"
+	TargetIQNConfigKey        = "iqn"
+	PortalsConfigKey          = "portals"
+	APIAddressConfigKey       = "apiAddress"
+	UsernameSecretKey         = "username"
+	PasswordSecretKey         = "password"
+	StorageClassAnnotationKey = "storageClass"
 
 	MaximumLUN          = 255
 	VolumeNameMaxLength = 32
@@ -44,8 +42,8 @@ type Driver struct {
 // DriverCtx contains data common to most calls
 type DriverCtx struct {
 	Credentials map[string]string
-	Parameters  map[string]string
-	VolumeCaps  []*csi.VolumeCapability
+	Parameters  *map[string]string
+	VolumeCaps  *[]*csi.VolumeCapability
 	Req         interface{}
 }
 

pkg/controller/attacher.go

@@ -9,11 +9,23 @@ import (
 
 	"github.com/container-storage-interface/spec/lib/go/csi"
 	"github.com/enix/dothill-storage-controller/pkg/common"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
 	"k8s.io/klog"
 )
 
 // ControllerPublishVolume attaches the given volume to the node
 func (driver *Driver) ControllerPublishVolume(ctx context.Context, req *csi.ControllerPublishVolumeRequest) (*csi.ControllerPublishVolumeResponse, error) {
+	if len(req.GetVolumeId()) == 0 {
+		return nil, status.Error(codes.InvalidArgument, "cannot publish volume with empty ID")
+	}
+	if len(req.GetNodeId()) == 0 {
+		return nil, status.Error(codes.InvalidArgument, "cannot publish volume to a node with empty ID")
+	}
+	if req.GetVolumeCapability() == nil {
+		return nil, status.Error(codes.InvalidArgument, "cannot publish volume without capabilities")
+	}
+
 	err := driver.beginRoutine(&common.DriverCtx{
 		Req:         req,
 		Credentials: req.GetSecrets(),
@@ -23,7 +35,7 @@ func (driver *Driver) ControllerPublishVolume(ctx context.Context, req *csi.Cont
 		return nil, err
 	}
 
-	initiatorName := getInitiatorName(req.GetVolumeContext())
+	initiatorName := req.GetNodeId()
 	klog.Infof("attach request for initiator %s, volume id : %s", initiatorName, req.GetVolumeId())
 
 	lun, err := driver.chooseLUN()
@@ -47,6 +59,10 @@ func (driver *Driver) ControllerPublishVolume(ctx context.Context, req *csi.Cont
 
 // ControllerUnpublishVolume deattaches the given volume from the node
 func (driver *Driver) ControllerUnpublishVolume(ctx context.Context, req *csi.ControllerUnpublishVolumeRequest) (*csi.ControllerUnpublishVolumeResponse, error) {
+	if len(req.GetVolumeId()) == 0 {
+		return nil, status.Error(codes.InvalidArgument, "cannot unpublish volume with empty ID")
+	}
+
 	err := driver.beginRoutine(&common.DriverCtx{
 		Req:         req,
 		Credentials: req.GetSecrets(),
@@ -59,7 +75,7 @@ func (driver *Driver) ControllerUnpublishVolume(ctx context.Context, req *csi.Co
 	klog.Infof("unmapping volume %s from all initiators", req.GetVolumeId())
 	_, status, err := driver.dothillClient.UnmapVolume(req.GetVolumeId(), "")
 	if err != nil {
-		if status.ReturnCode == unmapFailedErrorCode {
+		if status != nil && status.ReturnCode == unmapFailedErrorCode {
 			klog.Info("unmap failed, assuming volume is already unmapped")
 			return &csi.ControllerUnpublishVolumeResponse{}, nil
 		}
@@ -102,12 +118,17 @@ func (driver *Driver) chooseLUN() (int, error) {
 
 func (driver *Driver) mapVolume(volumeName, initiatorName string, lun int) error {
 	klog.Infof("trying to map volume %s for initiator %s on LUN %d", volumeName, initiatorName, lun)
-	_, status, err := driver.dothillClient.MapVolume(volumeName, initiatorName, "rw", lun)
-	if err != nil && status == nil {
-		return err
+	_, metadata, err := driver.dothillClient.MapVolume(volumeName, initiatorName, "rw", lun)
+	if err != nil && metadata == nil {
+		return status.Error(codes.Internal, err.Error())
 	}
-	if status.ReturnCode == hostDoesNotExistsErrorCode {
-		nodeName := strings.Split(initiatorName, ":")[1]
+	if metadata.ReturnCode == hostDoesNotExistsErrorCode {
+		nodeIDParts := strings.Split(initiatorName, ":")
+		if len(nodeIDParts) != 2 {
+			return status.Error(codes.InvalidArgument, "specified node ID is not a valid IQN")
+		}
+
+		nodeName := nodeIDParts[1]
 		klog.Infof("initiator does not exist, creating it with nickname %s", nodeName)
 		_, _, err = driver.dothillClient.CreateHost(nodeName, initiatorName)
 		if err != nil {
@@ -118,25 +139,27 @@ func (driver *Driver) mapVolume(volumeName, initiatorName string, lun int) error
 		if err != nil {
 			return err
 		}
+	} else if metadata.ReturnCode == volumeNotFoundErrorCode {
+		return status.Errorf(codes.NotFound, "volume %s not found", volumeName)
 	} else if err != nil {
-		return err
+		return status.Error(codes.Internal, err.Error())
 	}
 
 	return nil
 }
 
-func getInitiatorName(volumeContext map[string]string) string {
-	initiatorName := volumeContext[common.InitiatorNameConfigKey]
-	// overrideInitiatorName, overrideExists := options.PVC.Annotations[initiatorNameConfigKey]
-	// if overrideExists {
-	// 	initiatorName = overrideInitiatorName
-	// 	klog.Infof("custom initiator name was specified in PVC annotation: %s", initiatorName)
-	// } else if options.Parameters[uniqueInitiatorNameByPvcConfigKey] == "true" {
-	// 	year, month, _ := time.Now().Date()
-	// 	uniquePart := fmt.Sprintf("%d", rand.Int())[:8]
-	// 	initiatorName = fmt.Sprintf("iqn.%d-%02d.local.cluster:%s", year, int(month), uniquePart)
-	// 	klog.Infof("generated initiator name: %s", initiatorName)
-	// }
-
-	return initiatorName
-}
+// func getInitiatorName(volumeContext map[string]string) string {
+// 	initiatorName := volumeContext[common.InitiatorNameConfigKey]
+// 	overrideInitiatorName, overrideExists := options.PVC.Annotations[initiatorNameConfigKey]
+// 	if overrideExists {
+// 		initiatorName = overrideInitiatorName
+// 		klog.Infof("custom initiator name was specified in PVC annotation: %s", initiatorName)
+// 	} else if options.Parameters[uniqueInitiatorNameByPvcConfigKey] == "true" {
+// 		year, month, _ := time.Now().Date()
+// 		uniquePart := fmt.Sprintf("%d", rand.Int())[:8]
+// 		initiatorName = fmt.Sprintf("iqn.%d-%02d.local.cluster:%s", year, int(month), uniquePart)
+// 		klog.Infof("generated initiator name: %s", initiatorName)
+// 	}
+
+// 	return initiatorName
+// }

pkg/controller/controller.go

@@ -17,6 +17,7 @@ const (
 	hostDoesNotExistsErrorCode    = -10386
 	hostMapDoesNotExistsErrorCode = -10074
 	unmapFailedErrorCode          = -10509
+	volumeNotFoundErrorCode       = -10075
 )
 
 // Driver is the implementation of csi.ControllerServer
@@ -121,6 +122,11 @@ func (driver *Driver) configureClient(credentials map[string]string) error {
 	username := string(credentials[common.UsernameSecretKey])
 	password := string(credentials[common.PasswordSecretKey])
 	apiAddr := string(credentials[common.APIAddressConfigKey])
+
+	if len(apiAddr) == 0 || len(username) == 0 || len(password) == 0 {
+		return status.Error(codes.InvalidArgument, "at least one field is missing in credentials secret")
+	}
+
 	klog.Infof("using dothill API at address %s", apiAddr)
 	if driver.dothillClient.Addr == apiAddr && driver.dothillClient.Username == username {
 		klog.Info("dothill client is already configured for this API, skipping login")
@@ -133,9 +139,50 @@ func (driver *Driver) configureClient(credentials map[string]string) error {
 	klog.Infof("login into %s as user %s", driver.dothillClient.Addr, driver.dothillClient.Username)
 	err := driver.dothillClient.Login()
 	if err != nil {
-		return err
+		return status.Error(codes.Unauthenticated, err.Error())
 	}
 
 	klog.Info("login was successful")
 	return nil
 }
+
+func runPreflightChecks(parameters *map[string]string, capabilities *[]*csi.VolumeCapability) error {
+	checkIfKeyExistsInConfig := func(key string) error {
+		if parameters == nil {
+			return nil
+		}
+
+		klog.V(2).Infof("checking for %s in storage class parameters", key)
+		_, ok := (*parameters)[key]
+		if !ok {
+			return status.Errorf(codes.InvalidArgument, "'%s' is missing from configuration", key)
+		}
+		return nil
+	}
+
+	if err := checkIfKeyExistsInConfig(common.FsTypeConfigKey); err != nil {
+		return err
+	}
+	if err := checkIfKeyExistsInConfig(common.PoolConfigKey); err != nil {
+		return err
+	}
+	if err := checkIfKeyExistsInConfig(common.TargetIQNConfigKey); err != nil {
+		return err
+	}
+	if err := checkIfKeyExistsInConfig(common.PortalsConfigKey); err != nil {
+		return err
+	}
+
+	if capabilities != nil {
+		if len(*capabilities) == 0 {
+			return status.Error(codes.InvalidArgument, "missing volume capabilities")
+		}
+		for _, capability := range *capabilities {
+			if capability.GetAccessMode().GetMode() != csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER {
+				return status.Error(codes.FailedPrecondition, "dothill storage only supports ReadWriteOnce access mode")
+			}
+		}
+	}
+
+	return nil
+}

pkg/controller/go.sum

@@ -6,6 +6,8 @@ github.com/container-storage-interface/spec v1.2.0 h1:bD9KIVgaVKKkQ/UbVUY9kCaH/C
 github.com/container-storage-interface/spec v1.2.0/go.mod h1:6URME8mwIBbpVyZV93Ce5St17xBiQJQY67NDsuohiy4=
 github.com/enix/dothill-api-go v1.4.1 h1:ePb7tUef0WOnJl1LvEBiQxlQHcnJpEtMjjE47QRDYtI=
 github.com/enix/dothill-api-go v1.4.1/go.mod h1:LchQqj/tHiZ3AU23geA+0EY9ia9L0MuzNIvqX9reipI=
+github.com/enix/dothill-api-go v1.4.2 h1:1heKdOUXFOa1Gsfdo3H6/bh79hTb/mvZbbmdKoaGBOQ=
+github.com/enix/dothill-api-go v1.4.2/go.mod h1:LchQqj/tHiZ3AU23geA+0EY9ia9L0MuzNIvqX9reipI=
 github.com/envoyproxy/go-control-plane v0.9.1-0.20191026205805-5f8ba28d4473/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4=
 github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c=
 github.com/fsnotify/fsnotify v1.4.7 h1:IXs+QLmnXW2CcXuY+8Mzv/fWEsPGWxqefPtCP5CnV9I=