Add validations for selector flags in RunE

This PR moves the flag validations to top order, so that bottom order functions do not have such side effects on invalid/wrong selector flags
minio · Oct 28, 2021 · baa94df · baa94df
1 parent cfc5f0e
commit baa94df
Show file tree

Hide file tree

Showing 14 changed files with 385 additions and 199 deletions.
diff --git a/cmd/kubectl-direct_csi/cmd.go b/cmd/kubectl-direct_csi/cmd.go
@@ -44,6 +44,8 @@ var (
 	yaml       = false
 )
 
+var drives, nodes, driveGlobs, driveSelectors, nodeGlobs, nodeSelectors []string
+
 var printer func(interface{}) error
 
 var pluginCmd = &cobra.Command{

diff --git a/cmd/kubectl-direct_csi/drives.go b/cmd/kubectl-direct_csi/drives.go
@@ -19,10 +19,12 @@
 package main
 
 import (
+	directcsi "github.com/minio/direct-csi/pkg/apis/direct.csi.min.io/v1beta3"
 	"github.com/spf13/cobra"
 )
 
-var drives, nodes, status, accessTiers []string
+var status, accessTiers, accessTierSelectors, statusGlobs []string
+var driveStatusList []directcsi.DriveStatus
 
 var drivesCmd = &cobra.Command{
 	Use:   "drives",
@@ -34,6 +36,24 @@ var drivesCmd = &cobra.Command{
 	},
 }
 
+func validateDriveSelectors() (err error) {
+	driveGlobs, driveSelectors, err = getValidDriveSelectors(drives)
+	if err != nil {
+		return err
+	}
+	nodeGlobs, nodeSelectors, err = getValidNodeSelectors(nodes)
+	if err != nil {
+		return err
+	}
+	accessTierSelectors, err = getValidAccessTierSelectors(accessTiers)
+	if err != nil {
+		return err
+	}
+	statusGlobs, driveStatusList, err = getValidDriveStatusSelectors(status)
+
+	return err
+}
+
 func init() {
 	drivesCmd.AddCommand(listDrivesCmd)
 	drivesCmd.AddCommand(formatDrivesCmd)

diff --git a/cmd/kubectl-direct_csi/drives_accesstier_set.go b/cmd/kubectl-direct_csi/drives_accesstier_set.go
@@ -27,6 +27,7 @@ import (
 	"github.com/minio/direct-csi/pkg/utils"
 
 	"github.com/spf13/cobra"
+	"k8s.io/klog/v2"
 )
 
 var accessTierSet = &cobra.Command{
@@ -65,16 +66,19 @@ $ kubectl direct-csi drives access-tier set hot --nodes=directcsi-1,othernode-2
 					utils.Bold("--status"))
 			}
 		}
-
 		if len(args) != 1 {
 			return fmt.Errorf("only one access tier must be specified. please use '%s' for examples to set access-tier", utils.Bold("--help"))
 		}
-
 		accessTier, err := directcsi.ToAccessTier(args[0])
 		if err != nil {
 			return err
 		}
-
+		if err := validateDriveSelectors(); err != nil {
+			return err
+		}
+		if len(driveGlobs) > 0 || len(nodeGlobs) > 0 || len(statusGlobs) > 0 {
+			klog.Warning("Glob matches will be deprecated soon. Please use ellipses instead")
+		}
 		return setAccessTier(c.Context(), accessTier)
 	},
 	Aliases: []string{},

diff --git a/cmd/kubectl-direct_csi/drives_accesstier_unset.go b/cmd/kubectl-direct_csi/drives_accesstier_unset.go
@@ -27,6 +27,7 @@ import (
 	"github.com/minio/direct-csi/pkg/utils"
 
 	"github.com/spf13/cobra"
+	"k8s.io/klog/v2"
 )
 
 var accessTierUnset = &cobra.Command{
@@ -66,6 +67,13 @@ $ kubectl direct-csi drives access-tier unset --nodes=directcsi-1,othernode-2 --
 					utils.Bold("--access-tier"))
 			}
 		}
+		if err := validateDriveSelectors(); err != nil {
+			return err
+		}
+		if len(driveGlobs) > 0 || len(nodeGlobs) > 0 || len(statusGlobs) > 0 {
+			klog.Warning("Glob matches will be deprecated soon. Please use ellipses instead")
+		}
+
 		return unsetAccessTier(c.Context())
 	},
 	Aliases: []string{},

diff --git a/cmd/kubectl-direct_csi/drives_format.go b/cmd/kubectl-direct_csi/drives_format.go
@@ -83,6 +83,12 @@ $ kubectl direct-csi drives format <drive_id_1> <drive_id_2>
 					utils.Bold("--nodes"))
 			}
 		}
+		if err := validateDriveSelectors(); err != nil {
+			return err
+		}
+		if len(driveGlobs) > 0 || len(nodeGlobs) > 0 {
+			klog.Warning("Glob matches will be deprecated soon. Please use ellipses instead")
+		}
 		return formatDrives(c.Context(), args)
 	},
 	Aliases: []string{},

diff --git a/cmd/kubectl-direct_csi/drives_format_test.go b/cmd/kubectl-direct_csi/drives_format_test.go
@@ -214,6 +214,10 @@ func TestFormatDrivesByAttributes(t1 *testing.T) {
 			all = tt.all
 			force = tt.force
 
+			if err := validateDriveSelectors(); err != nil {
+				t1.Fatalf("Test case name %s: validateDriveSelectors failed with %v", tt.name, err)
+			}
+
 			if err := formatDrives(ctx, []string{}); err != nil {
 				t1.Errorf("Test case name %s: Failed with %v", tt.name, err)
 			}

diff --git a/cmd/kubectl-direct_csi/drives_list.go b/cmd/kubectl-direct_csi/drives_list.go
@@ -41,8 +41,11 @@ var listDrivesCmd = &cobra.Command{
 	Short: "list drives in the DirectCSI cluster",
 	Long:  "",
 	Example: `
-# Filter drives by ellipses notation for drive paths and nodes
-$ kubectl direct-csi drives ls --drives='/dev/xvd{a...d}' --nodes='node-{1...4}'
+# List all drives
+$ kubectl direct-csi drives ls
+
+# List all drives (including 'unavailable' drives)
+$ kubectl direct-csi drives ls --all
 
 # Filter all ready drives 
 $ kubectl direct-csi drives ls --status=ready
@@ -61,8 +64,17 @@ $ kubectl direct-csi drives drives ls --access-tier="hot"
 
 # Filter all drives with access-tier being set
 $ kubectl direct-csi drives drives ls --access-tier="*"
+
+# Filter drives by ellipses notation for drive paths and nodes
+$ kubectl direct-csi drives ls --drives='/dev/xvd{a...d}' --nodes='node-{1...4}'
 `,
 	RunE: func(c *cobra.Command, args []string) error {
+		if err := validateDriveSelectors(); err != nil {
+			return err
+		}
+		if len(driveGlobs) > 0 || len(nodeGlobs) > 0 || len(statusGlobs) > 0 {
+			klog.Warning("Glob matches will be deprecated soon. Please use ellipses instead")
+		}
 		return listDrives(c.Context(), args)
 	},
 	Aliases: []string{
@@ -118,7 +130,10 @@ func listDrives(ctx context.Context, args []string) error {
 		ctx,
 		utils.GetDirectCSIClient().DirectCSIDrives(),
 		func(drive directcsi.DirectCSIDrive) bool {
-			return all || len(status) > 0 || drive.Status.DriveStatus != directcsi.DriveStatusUnavailable
+			if len(driveStatusList) > 0 {
+				return drive.MatchDriveStatus(driveStatusList)
+			}
+			return all || len(statusGlobs) > 0 || drive.Status.DriveStatus != directcsi.DriveStatusUnavailable
 		},
 	)
 	if err != nil {

diff --git a/cmd/kubectl-direct_csi/drives_release.go b/cmd/kubectl-direct_csi/drives_release.go
@@ -67,6 +67,13 @@ var releaseDrivesCmd = &cobra.Command{
 				return fmt.Errorf("atleast one among ['%s','%s','%s','%s'] should be specified", utils.Bold("--all"), utils.Bold("--drives"), utils.Bold("--nodes"), utils.Bold("--access-tier"))
 			}
 		}
+		if err := validateDriveSelectors(); err != nil {
+			return err
+		}
+		if len(driveGlobs) > 0 || len(nodeGlobs) > 0 {
+			klog.Warning("Glob matches will be deprecated soon. Please use ellipses instead")
+		}
+
 		return releaseDrives(c.Context(), args)
 	},
 	Aliases: []string{},

diff --git a/cmd/kubectl-direct_csi/drives_unrelease.go b/cmd/kubectl-direct_csi/drives_unrelease.go
@@ -74,7 +74,12 @@ var unreleaseDrivesCmd = &cobra.Command{
 					utils.Bold("--nodes"))
 			}
 		}
-
+		if err := validateDriveSelectors(); err != nil {
+			return err
+		}
+		if len(driveGlobs) > 0 || len(nodeGlobs) > 0 {
+			klog.Warning("Glob matches will be deprecated soon. Please use ellipses instead")
+		}
 		return unreleaseDrives(c.Context(), args)
 	},
 	Aliases: []string{},