fix: consistent commands running (#792)

* fix: consistent commands running

* fix: consistent commands running

* fix: fixed todo comment

* fixed common

api/v1/testkube.yaml

@@ -1548,13 +1548,6 @@ components:
         uri:
           description: URI for rest based executors
           type: string
-        volumeMountPath:
-          description: VolumeMountPath - where should PV be monted inside job
-            pod for e.g. artifacts
-          type: string
-        volumeQuantity:
-          description: VolumeQuantity for kube-job PersistentVolume
-          type: string
 
     ExecutorDetails:
       description: Executor details with Executor data and additional information like list of executions

cmd/kubectl-testkube/commands/artifacts.go

@@ -4,6 +4,8 @@ import (
 	"fmt"
 	"os"
 
+	"github.com/kubeshop/testkube/cmd/kubectl-testkube/commands/common"
+	"github.com/kubeshop/testkube/cmd/kubectl-testkube/commands/common/validator"
 	"github.com/kubeshop/testkube/cmd/kubectl-testkube/commands/scripts"
 	"github.com/kubeshop/testkube/pkg/ui"
 	"github.com/spf13/cobra"
@@ -21,13 +23,14 @@ func NewArtifactsCmd() *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "artifacts",
 		Short: "Artifacts management commands",
+		Args:  validator.ExecutionID,
 		Run: func(cmd *cobra.Command, args []string) {
 			cmd.Help()
 		},
 		PersistentPreRun: func(cmd *cobra.Command, args []string) {
 			// version validation
 			// if client version is less than server version show warning
-			client, _ := scripts.GetClient(cmd)
+			client, _ := common.GetClient(cmd)
 
 			err := ValidateVersions(client)
 			if err != nil {
@@ -36,37 +39,28 @@ func NewArtifactsCmd() *cobra.Command {
 		},
 	}
 
-	cmd.PersistentFlags().StringVarP(&client, "client", "c", "proxy", "Client used for connecting to testkube API one of proxy|direct")
 	cmd.PersistentFlags().BoolVarP(&verbose, "verbose", "v", false, "should I show additional debug messages")
 
-	cmd.PersistentFlags().StringVarP(&executionID, "execution-id", "e", "", "ID of the execution")
-
 	cmd.AddCommand(NewListArtifactsCmd())
 	cmd.AddCommand(NewDownloadSingleArtifactsCmd())
 	cmd.AddCommand(NewDownloadAllArtifactsCmd())
-	// output renderer flags
+
 	return cmd
 }
 
 func NewListArtifactsCmd() *cobra.Command {
 	cmd := &cobra.Command{
-		Use:   "list",
+		Use:   "list <executionID>",
 		Short: "List artifacts of the given execution ID",
-		PreRunE: func(cmd *cobra.Command, args []string) error {
-			if executionID == "" {
-				cmd.SilenceUsage = true
-				return fmt.Errorf("execution-id is a required parameter")
-			}
-			return nil
-		},
-		RunE: func(cmd *cobra.Command, args []string) error {
+		Args:  validator.ExecutionID,
+		Run: func(cmd *cobra.Command, args []string) {
+			executionID = args[0]
 			cmd.SilenceUsage = true
-			client, _ := scripts.GetClient(cmd)
+			client, _ := common.GetClient(cmd)
 			artifacts, err := client.GetExecutionArtifacts(executionID)
 			ui.ExitOnError("getting artifacts ", err)
 
 			ui.Table(artifacts, os.Stdout)
-			return nil
 		},
 	}
 
@@ -81,32 +75,19 @@ func NewListArtifactsCmd() *cobra.Command {
 
 func NewDownloadSingleArtifactsCmd() *cobra.Command {
 	cmd := &cobra.Command{
-		Use:   "download-one",
+		Use:   "download-one <executionID> <fileName> <destinationDir>",
 		Short: "download artifact",
-		PreRunE: func(cmd *cobra.Command, args []string) error {
-			if executionID == "" {
-				cmd.SilenceUsage = true
-				return fmt.Errorf("execution-id is a required parameter")
-			}
-
-			if filename == "" {
-				cmd.SilenceUsage = true
-				return fmt.Errorf("fileName is a required parameter")
-			}
-			return nil
-		},
-		RunE: func(cmd *cobra.Command, args []string) error {
-			cmd.SilenceUsage = true
-			client, _ := scripts.GetClient(cmd)
-			if f, err := client.DownloadFile(executionID, filename, destination); err != nil {
-				cmd.SilenceUsage = true
-				return err
+		Args:  validator.ExecutionIDAndFileNames,
+		Run: func(cmd *cobra.Command, args []string) {
+			executionID := args[0]
+			filename := args[1]
+			destination := args[2]
 
-			} else {
-				fmt.Printf("File %s downloaded.\n", f)
-			}
+			client, _ := common.GetClient(cmd)
+			f, err := client.DownloadFile(executionID, filename, destination)
+			ui.ExitOnError("downloading file"+filename, err)
 
-			return nil
+			fmt.Printf("File %s downloaded.\n", f)
 		},
 	}
 
@@ -123,20 +104,13 @@ func NewDownloadSingleArtifactsCmd() *cobra.Command {
 
 func NewDownloadAllArtifactsCmd() *cobra.Command {
 	cmd := &cobra.Command{
-		Use:   "download",
-		Short: "download artifact",
-		PreRunE: func(cmd *cobra.Command, args []string) error {
-			if executionID == "" {
-				cmd.SilenceUsage = true
-				return fmt.Errorf("execution-id is a required parameter")
-			}
-
-			return nil
-		},
-		RunE: func(cmd *cobra.Command, args []string) error {
-			client, _ := scripts.GetClient(cmd)
+		Use:   "download <executionID>",
+		Short: "download artifacts",
+		Args:  validator.ExecutionID,
+		Run: func(cmd *cobra.Command, args []string) {
+			executionID := args[0]
+			client, _ := common.GetClient(cmd)
 			scripts.DownloadArtifacts(executionID, downloadDir, client)
-			return nil
 		},
 	}
 

cmd/kubectl-testkube/commands/executors/common.go → cmd/kubectl-testkube/commands/common/client.go

@@ -1,4 +1,4 @@
-package executors
+package common
 
 import (
 	"github.com/kubeshop/testkube/pkg/api/v1/client"

cmd/kubectl-testkube/commands/common/validator/directory.go

@@ -0,0 +1,14 @@
+package validator
+
+import (
+	"errors"
+
+	"github.com/spf13/cobra"
+)
+
+func ManifestsDirectory(cmd *cobra.Command, args []string) error {
+	if len(args) < 1 {
+		return errors.New("please pass directory with manifest files as argument")
+	}
+	return nil
+}

cmd/kubectl-testkube/commands/common/validator/executionid.go

@@ -0,0 +1,14 @@
+package validator
+
+import (
+	"errors"
+
+	"github.com/spf13/cobra"
+)
+
+func ExecutionID(cmd *cobra.Command, args []string) error {
+	if len(args) < 1 {
+		return errors.New("please pass execution ID as argument")
+	}
+	return nil
+}

cmd/kubectl-testkube/commands/common/validator/executionidfilename.go

@@ -0,0 +1,14 @@
+package validator
+
+import (
+	"errors"
+
+	"github.com/spf13/cobra"
+)
+
+func ExecutionIDAndFileNames(cmd *cobra.Command, args []string) error {
+	if len(args) < 3 {
+		return errors.New("please pass 'Execution ID' ,'Filename' and 'Destination Directory' as arguments")
+	}
+	return nil
+}

cmd/kubectl-testkube/commands/common/validator/executorname.go

@@ -0,0 +1,16 @@
+package validator
+
+import (
+	"errors"
+
+	"github.com/spf13/cobra"
+)
+
+func ExecutorName(cmd *cobra.Command, args []string) error {
+	// TODO validate executor name as valid kubernetes resource name
+
+	if len(args) < 1 {
+		return errors.New("please pass valid executor-name")
+	}
+	return nil
+}

cmd/kubectl-testkube/commands/common/validator/scriptname.go

@@ -0,0 +1,16 @@
+package validator
+
+import (
+	"errors"
+
+	"github.com/spf13/cobra"
+)
+
+func ScriptName(cmd *cobra.Command, args []string) error {
+	// TODO validate script name as valid kubernetes resource name
+
+	if len(args) < 1 {
+		return errors.New("please pass valid script-name")
+	}
+	return nil
+}

cmd/kubectl-testkube/commands/common/validator/testname.go

@@ -0,0 +1,16 @@
+package validator
+
+import (
+	"errors"
+
+	"github.com/spf13/cobra"
+)
+
+func TestName(cmd *cobra.Command, args []string) error {
+	// TODO validate test name as valid kubernetes resource name
+
+	if len(args) < 1 {
+		return errors.New("please pass valid test-name")
+	}
+	return nil
+}

cmd/kubectl-testkube/commands/crds/scripts_crds.go

@@ -10,6 +10,7 @@ import (
 	"strings"
 	"text/template"
 
+	"github.com/kubeshop/testkube/cmd/kubectl-testkube/commands/common/validator"
 	"github.com/kubeshop/testkube/pkg/api/v1/client"
 	"github.com/kubeshop/testkube/pkg/test/script/detector"
 	"github.com/kubeshop/testkube/pkg/ui"
@@ -18,21 +19,17 @@ import (
 
 func NewCRDScriptsCmd() *cobra.Command {
 	cmd := &cobra.Command{
-		Use:   "scripts",
+		Use:   "scripts <manifestDirectory>",
 		Short: "Generate scripts CRD file based on directory",
 		Long:  `Generate scripts manifest based on directory (e.g. for ArgoCD sync based on scripts files)`,
+		Args:  validator.ManifestsDirectory,
 		Run: func(cmd *cobra.Command, args []string) {
 			var err error
 
-			if len(args) == 0 {
-				ui.Failf("Please pass directory")
-			}
-
 			dir := args[0]
 			firstEntry := true
 
 			err = filepath.Walk(dir, func(path string, info fs.FileInfo, err error) error {
-
 				if err != nil {
 					return nil
 				}

cmd/kubectl-testkube/commands/executors.go

@@ -1,8 +1,8 @@
 package commands
 
 import (
+	"github.com/kubeshop/testkube/cmd/kubectl-testkube/commands/common"
 	"github.com/kubeshop/testkube/cmd/kubectl-testkube/commands/executors"
-	"github.com/kubeshop/testkube/cmd/kubectl-testkube/commands/scripts"
 	"github.com/kubeshop/testkube/pkg/ui"
 	"github.com/spf13/cobra"
 )
@@ -24,7 +24,7 @@ func NewExecutorsCmd() *cobra.Command {
 		PersistentPreRun: func(cmd *cobra.Command, args []string) {
 			// version validation
 			// if client version is less than server version show warning
-			client, _ := scripts.GetClient(cmd)
+			client, _ := common.GetClient(cmd)
 
 			err := ValidateVersions(client)
 			if err != nil {

cmd/kubectl-testkube/commands/executors/create.go

@@ -1,15 +1,16 @@
 package executors
 
 import (
+	"github.com/kubeshop/testkube/cmd/kubectl-testkube/commands/common"
 	apiClient "github.com/kubeshop/testkube/pkg/api/v1/client"
 	"github.com/kubeshop/testkube/pkg/ui"
 	"github.com/spf13/cobra"
 )
 
 func NewCreateExecutorCmd() *cobra.Command {
 	var (
-		types                                                      []string
-		name, executorType, image, uri, volueMount, volumeQuantity string
+		types                          []string
+		name, executorType, image, uri string
 	)
 
 	cmd := &cobra.Command{
@@ -21,22 +22,20 @@ func NewCreateExecutorCmd() *cobra.Command {
 
 			var err error
 
-			client, namespace := GetClient(cmd)
+			client, namespace := common.GetClient(cmd)
 
 			executor, _ := client.GetExecutor(name)
 			if name == executor.Name {
 				ui.Failf("Executor with name '%s' already exists in namespace %s", name, namespace)
 			}
 
 			options := apiClient.CreateExecutorOptions{
-				Name:            name,
-				Namespace:       namespace,
-				Types:           types,
-				ExecutorType:    executorType,
-				Image:           image,
-				Uri:             uri,
-				VolumeMountPath: volueMount,
-				VolumeQuantity:  volumeQuantity,
+				Name:         name,
+				Namespace:    namespace,
+				Types:        types,
+				ExecutorType: executorType,
+				Image:        image,
+				Uri:          uri,
 			}
 
 			_, err = client.CreateExecutor(options)
@@ -52,8 +51,6 @@ func NewCreateExecutorCmd() *cobra.Command {
 
 	cmd.Flags().StringVarP(&uri, "uri", "u", "", "if resource need to be loaded from URI")
 	cmd.Flags().StringVarP(&image, "image", "i", "", "if uri is git repository we can set additional branch parameter")
-	cmd.Flags().StringVarP(&volueMount, "volume-mount", "", "", "where volume should be mounted")
-	cmd.Flags().StringVarP(&volumeQuantity, "volume-quantity", "", "", "how big volume should be")
 
 	return cmd
 }