From d14141c2f1c13274910dcc7ec64cee7229249c49 Mon Sep 17 00:00:00 2001 From: Mason Malone Date: Mon, 23 Sep 2024 20:46:31 -0700 Subject: [PATCH] refactor(cli): improve error handling. Fixes #1935 It's tricky to write unit tests for the CLI code for three main reasons: exiting on errors, lack of dependency injection, and insufficient mocks. https://github.com/argoproj/argo-workflows/pull/3917 tried to solve all of these issues, but appears to have been abandoned. I thought about trying to revive that PR, but it's been over 4 years and there's certainly going to be a ton of conflicts. I figure it's better to take a more focused, incremental approach. This PR is focused on improving error handling in the following ways: 1. Do argument validation using [cobra validators](https://cobra.dev/#positional-and-custom-arguments) instead of custom logic in the `Run` function. Besides being simpler, this leads to better error messages. 2. Replace all `Run` functions with `RunE`, which allows returning errors. This is the same approach that Argo Rollouts is taking (example: https://github.com/argoproj/argo-rollouts/blob/00e39b114849010dd96221a8db441d58e860d4d0/pkg/kubectl-argo-rollouts/cmd/abort/abort.go#L41) 3. Replace nearly all calls to `log.Fatal()`/`errors.CheckError()`/etc with error propagation. Signed-off-by: Mason Malone --- cmd/argo/commands/archive/delete.go | 18 ++-- cmd/argo/commands/archive/get.go | 23 +++-- cmd/argo/commands/archive/list.go | 19 ++-- cmd/argo/commands/archive/list_label_keys.go | 17 +++- .../commands/archive/list_label_values.go | 16 ++- cmd/argo/commands/archive/resubmit.go | 17 ++-- cmd/argo/commands/archive/retry.go | 26 ++--- cmd/argo/commands/archive/root.go | 4 +- cmd/argo/commands/auth/root.go | 4 +- cmd/argo/commands/auth/token.go | 13 +-- cmd/argo/commands/client/conn.go | 24 ++--- cmd/argo/commands/client/conn_test.go | 22 ++--- cmd/argo/commands/clustertemplate/create.go | 25 +++-- cmd/argo/commands/clustertemplate/delete.go | 25 +++-- cmd/argo/commands/clustertemplate/get.go | 14 +-- cmd/argo/commands/clustertemplate/lint.go | 13 ++- cmd/argo/commands/clustertemplate/list.go | 15 +-- cmd/argo/commands/clustertemplate/root.go | 4 +- cmd/argo/commands/clustertemplate/update.go | 27 +++--- cmd/argo/commands/common/logs.go | 13 ++- cmd/argo/commands/common/submit.go | 13 ++- cmd/argo/commands/common/wait.go | 26 +++-- cmd/argo/commands/common/watch.go | 27 ++++-- cmd/argo/commands/completion.go | 15 +-- cmd/argo/commands/cp.go | 11 ++- cmd/argo/commands/cron/create.go | 28 ++++-- cmd/argo/commands/cron/delete.go | 21 ++-- cmd/argo/commands/cron/get.go | 23 ++--- cmd/argo/commands/cron/lint.go | 12 +-- cmd/argo/commands/cron/list.go | 20 ++-- cmd/argo/commands/cron/resume.go | 17 +++- cmd/argo/commands/cron/root.go | 4 +- cmd/argo/commands/cron/suspend.go | 17 +++- cmd/argo/commands/cron/update.go | 30 +++--- cmd/argo/commands/cron/util.go | 15 --- cmd/argo/commands/delete.go | 47 +++++---- cmd/argo/commands/executorplugin/build.go | 6 +- cmd/argo/commands/executorplugin/root.go | 4 +- cmd/argo/commands/get.go | 27 +++--- cmd/argo/commands/lint.go | 19 ++-- cmd/argo/commands/lint_test.go | 30 ++++-- cmd/argo/commands/list.go | 38 +++++--- cmd/argo/commands/logs.go | 23 +++-- cmd/argo/commands/node.go | 29 +++--- cmd/argo/commands/resubmit.go | 17 ++-- cmd/argo/commands/resume.go | 22 +++-- cmd/argo/commands/retry.go | 25 +++-- cmd/argo/commands/root.go | 4 +- cmd/argo/commands/stop.go | 19 ++-- cmd/argo/commands/submit.go | 97 +++++++++++-------- cmd/argo/commands/submit_test.go | 17 +++- cmd/argo/commands/suspend.go | 11 ++- cmd/argo/commands/template/create.go | 25 +++-- cmd/argo/commands/template/delete.go | 27 +++--- cmd/argo/commands/template/get.go | 14 +-- cmd/argo/commands/template/lint.go | 12 +-- cmd/argo/commands/template/list.go | 20 ++-- cmd/argo/commands/template/root.go | 4 +- cmd/argo/commands/template/update.go | 27 +++--- cmd/argo/commands/terminate.go | 26 +++-- cmd/argo/commands/version.go | 17 +++- cmd/argo/commands/wait.go | 8 +- cmd/argo/commands/watch.go | 14 ++- cmd/argo/lint/lint.go | 16 ++- cmd/argo/main.go | 2 - cmd/argoexec/commands/agent.go | 7 +- cmd/argoexec/commands/data.go | 7 +- cmd/argoexec/commands/init.go | 7 +- cmd/argoexec/commands/resource.go | 13 +-- cmd/argoexec/commands/root.go | 4 +- cmd/argoexec/commands/wait.go | 7 +- cmd/workflow-controller/main.go | 5 +- test/e2e/cli_test.go | 2 +- util/cmd/cmd.go | 3 +- 74 files changed, 764 insertions(+), 556 deletions(-) diff --git a/cmd/argo/commands/archive/delete.go b/cmd/argo/commands/archive/delete.go index cfc801a72a61..534a3e0f542c 100644 --- a/cmd/argo/commands/archive/delete.go +++ b/cmd/argo/commands/archive/delete.go @@ -3,7 +3,6 @@ package archive import ( "fmt" - "github.com/argoproj/pkg/errors" "github.com/spf13/cobra" client "github.com/argoproj/argo-workflows/v3/cmd/argo/commands/client" @@ -14,15 +13,22 @@ func NewDeleteCommand() *cobra.Command { command := &cobra.Command{ Use: "delete UID...", Short: "delete a workflow in the archive", - Run: func(cmd *cobra.Command, args []string) { - ctx, apiClient := client.NewAPIClient(cmd.Context()) + RunE: func(cmd *cobra.Command, args []string) error { + ctx, apiClient, err := client.NewAPIClient(cmd.Context()) + if err != nil { + return err + } serviceClient, err := apiClient.NewArchivedWorkflowServiceClient() - errors.CheckError(err) + if err != nil { + return err + } for _, uid := range args { - _, err = serviceClient.DeleteArchivedWorkflow(ctx, &workflowarchivepkg.DeleteArchivedWorkflowRequest{Uid: uid}) - errors.CheckError(err) + if _, err = serviceClient.DeleteArchivedWorkflow(ctx, &workflowarchivepkg.DeleteArchivedWorkflowRequest{Uid: uid}); err != nil { + return err + } fmt.Printf("Archived workflow '%s' deleted\n", uid) } + return nil }, } return command diff --git a/cmd/argo/commands/archive/get.go b/cmd/argo/commands/archive/get.go index 5315b61f1e39..1f2d8e91a3c7 100644 --- a/cmd/argo/commands/archive/get.go +++ b/cmd/argo/commands/archive/get.go @@ -4,9 +4,7 @@ import ( "encoding/json" "fmt" "log" - "os" - "github.com/argoproj/pkg/errors" "github.com/argoproj/pkg/humanize" "github.com/spf13/cobra" "sigs.k8s.io/yaml" @@ -21,19 +19,24 @@ func NewGetCommand() *cobra.Command { command := &cobra.Command{ Use: "get UID", Short: "get a workflow in the archive", - Run: func(cmd *cobra.Command, args []string) { - if len(args) != 1 { - cmd.HelpFunc()(cmd, args) - os.Exit(1) - } + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { uid := args[0] - ctx, apiClient := client.NewAPIClient(cmd.Context()) + ctx, apiClient, err := client.NewAPIClient(cmd.Context()) + if err != nil { + return err + } serviceClient, err := apiClient.NewArchivedWorkflowServiceClient() - errors.CheckError(err) + if err != nil { + return err + } wf, err := serviceClient.GetArchivedWorkflow(ctx, &workflowarchivepkg.GetArchivedWorkflowRequest{Uid: uid}) - errors.CheckError(err) + if err != nil { + return err + } printWorkflow(wf, output) + return nil }, } command.Flags().StringVarP(&output, "output", "o", "wide", "Output format. One of: json|yaml|wide") diff --git a/cmd/argo/commands/archive/list.go b/cmd/argo/commands/archive/list.go index 5747ffa73e96..736a77cf7fc1 100644 --- a/cmd/argo/commands/archive/list.go +++ b/cmd/argo/commands/archive/list.go @@ -5,7 +5,6 @@ import ( "os" "sort" - "github.com/argoproj/pkg/errors" log "github.com/sirupsen/logrus" "github.com/spf13/cobra" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -25,15 +24,21 @@ func NewListCommand() *cobra.Command { command := &cobra.Command{ Use: "list", Short: "list workflows in the archive", - Run: func(cmd *cobra.Command, args []string) { - ctx, apiClient := client.NewAPIClient(cmd.Context()) + RunE: func(cmd *cobra.Command, args []string) error { + ctx, apiClient, err := client.NewAPIClient(cmd.Context()) + if err != nil { + return err + } serviceClient, err := apiClient.NewArchivedWorkflowServiceClient() - errors.CheckError(err) + if err != nil { + return err + } namespace := client.Namespace() workflows, err := listArchivedWorkflows(ctx, serviceClient, namespace, selector, chunkSize) - errors.CheckError(err) - err = printer.PrintWorkflows(workflows, os.Stdout, printer.PrintOpts{Output: output, Namespace: true, UID: true}) - errors.CheckError(err) + if err != nil { + return err + } + return printer.PrintWorkflows(workflows, os.Stdout, printer.PrintOpts{Output: output, Namespace: true, UID: true}) }, } command.Flags().StringVarP(&output, "output", "o", "wide", "Output format. One of: json|yaml|wide") diff --git a/cmd/argo/commands/archive/list_label_keys.go b/cmd/argo/commands/archive/list_label_keys.go index 315a8f43d622..9a37b3ca934d 100644 --- a/cmd/argo/commands/archive/list_label_keys.go +++ b/cmd/argo/commands/archive/list_label_keys.go @@ -3,7 +3,6 @@ package archive import ( "fmt" - "github.com/argoproj/pkg/errors" "github.com/spf13/cobra" "github.com/argoproj/argo-workflows/v3/cmd/argo/commands/client" @@ -14,15 +13,23 @@ func NewListLabelKeyCommand() *cobra.Command { command := &cobra.Command{ Use: "list-label-keys", Short: "list workflows label keys in the archive", - Run: func(cmd *cobra.Command, args []string) { - ctx, apiClient := client.NewAPIClient(cmd.Context()) + RunE: func(cmd *cobra.Command, args []string) error { + ctx, apiClient, err := client.NewAPIClient(cmd.Context()) + if err != nil { + return err + } serviceClient, err := apiClient.NewArchivedWorkflowServiceClient() - errors.CheckError(err) + if err != nil { + return err + } keys, err := serviceClient.ListArchivedWorkflowLabelKeys(ctx, &workflowarchivepkg.ListArchivedWorkflowLabelKeysRequest{}) - errors.CheckError(err) + if err != nil { + return err + } for _, str := range keys.Items { fmt.Printf("%s\n", str) } + return nil }, } return command diff --git a/cmd/argo/commands/archive/list_label_values.go b/cmd/argo/commands/archive/list_label_values.go index 0c24ae78ccdd..b62917a3b336 100644 --- a/cmd/argo/commands/archive/list_label_values.go +++ b/cmd/argo/commands/archive/list_label_values.go @@ -18,20 +18,28 @@ func NewListLabelValueCommand() *cobra.Command { command := &cobra.Command{ Use: "list-label-values", Short: "get workflow label values in the archive", - Run: func(cmd *cobra.Command, args []string) { + RunE: func(cmd *cobra.Command, args []string) error { listOpts := &metav1.ListOptions{ LabelSelector: selector, } - ctx, apiClient := client.NewAPIClient(cmd.Context()) + ctx, apiClient, err := client.NewAPIClient(cmd.Context()) + if err != nil { + return err + } serviceClient, err := apiClient.NewArchivedWorkflowServiceClient() - errors.CheckError(err) + if err != nil { + return err + } labels, err := serviceClient.ListArchivedWorkflowLabelValues(ctx, &workflowarchivepkg.ListArchivedWorkflowLabelValuesRequest{ListOptions: listOpts}) - errors.CheckError(err) + if err != nil { + return err + } for _, str := range labels.Items { fmt.Printf("%s\n", str) } + return nil }, } command.Flags().StringVarP(&selector, "selector", "l", "", "Selector (label query) to query on, allows 1 value (e.g. -l key1)") diff --git a/cmd/argo/commands/archive/resubmit.go b/cmd/argo/commands/archive/resubmit.go index 038bda4a718d..1143c42bffc6 100644 --- a/cmd/argo/commands/archive/resubmit.go +++ b/cmd/argo/commands/archive/resubmit.go @@ -3,7 +3,6 @@ package archive import ( "context" - "github.com/argoproj/pkg/errors" "github.com/spf13/cobra" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" @@ -67,18 +66,22 @@ func NewResubmitCommand() *cobra.Command { argo archive resubmit --log uid `, - Run: func(cmd *cobra.Command, args []string) { + RunE: func(cmd *cobra.Command, args []string) error { if cmd.Flag("priority").Changed { cliSubmitOpts.Priority = &resubmitOpts.priority } - ctx, apiClient := client.NewAPIClient(cmd.Context()) + ctx, apiClient, err := client.NewAPIClient(cmd.Context()) + if err != nil { + return err + } serviceClient := apiClient.NewWorkflowServiceClient() // needed for wait watch or log flags archiveServiceClient, err := apiClient.NewArchivedWorkflowServiceClient() - errors.CheckError(err) + if err != nil { + return err + } resubmitOpts.namespace = client.Namespace() - err = resubmitArchivedWorkflows(ctx, archiveServiceClient, serviceClient, resubmitOpts, cliSubmitOpts, args) - errors.CheckError(err) + return resubmitArchivedWorkflows(ctx, archiveServiceClient, serviceClient, resubmitOpts, cliSubmitOpts, args) }, } @@ -142,7 +145,7 @@ func resubmitArchivedWorkflows(ctx context.Context, archiveServiceClient workflo if len(resubmittedUids) == 1 { // watch or wait when there is only one workflow retried - common.WaitWatchOrLog(ctx, serviceClient, lastResubmitted.Namespace, []string{lastResubmitted.Name}, cliSubmitOpts) + return common.WaitWatchOrLog(ctx, serviceClient, lastResubmitted.Namespace, []string{lastResubmitted.Name}, cliSubmitOpts) } return nil } diff --git a/cmd/argo/commands/archive/retry.go b/cmd/argo/commands/archive/retry.go index 3eb315d739ba..de3eead996d9 100644 --- a/cmd/argo/commands/archive/retry.go +++ b/cmd/argo/commands/archive/retry.go @@ -2,10 +2,9 @@ package archive import ( "context" + "errors" "fmt" - "os" - "github.com/argoproj/pkg/errors" "github.com/spf13/cobra" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/fields" @@ -70,20 +69,25 @@ func NewRetryCommand() *cobra.Command { argo archive retry --log uid `, - Run: func(cmd *cobra.Command, args []string) { + Args: func(cmd *cobra.Command, args []string) error { if len(args) == 0 && !retryOpts.hasSelector() { - cmd.HelpFunc()(cmd, args) - os.Exit(1) + return errors.New("requires either selector or workflow") + } + return nil + }, + RunE: func(cmd *cobra.Command, args []string) error { + ctx, apiClient, err := client.NewAPIClient(cmd.Context()) + if err != nil { + return err } - - ctx, apiClient := client.NewAPIClient(cmd.Context()) serviceClient := apiClient.NewWorkflowServiceClient() archiveServiceClient, err := apiClient.NewArchivedWorkflowServiceClient() - errors.CheckError(err) + if err != nil { + return err + } retryOpts.namespace = client.Namespace() - err = retryArchivedWorkflows(ctx, archiveServiceClient, serviceClient, retryOpts, cliSubmitOpts, args) - errors.CheckError(err) + return retryArchivedWorkflows(ctx, archiveServiceClient, serviceClient, retryOpts, cliSubmitOpts, args) }, } @@ -146,7 +150,7 @@ func retryArchivedWorkflows(ctx context.Context, archiveServiceClient workflowar } if len(retriedUids) == 1 { // watch or wait when there is only one workflow retried - common.WaitWatchOrLog(ctx, serviceClient, lastRetried.Namespace, []string{lastRetried.Name}, cliSubmitOpts) + return common.WaitWatchOrLog(ctx, serviceClient, lastRetried.Namespace, []string{lastRetried.Name}, cliSubmitOpts) } return nil } diff --git a/cmd/argo/commands/archive/root.go b/cmd/argo/commands/archive/root.go index bf147daefd65..68efff692439 100644 --- a/cmd/argo/commands/archive/root.go +++ b/cmd/argo/commands/archive/root.go @@ -8,8 +8,8 @@ func NewArchiveCommand() *cobra.Command { command := &cobra.Command{ Use: "archive", Short: "manage the workflow archive", - Run: func(cmd *cobra.Command, args []string) { - cmd.HelpFunc()(cmd, args) + RunE: func(cmd *cobra.Command, args []string) error { + return cmd.Help() }, } diff --git a/cmd/argo/commands/auth/root.go b/cmd/argo/commands/auth/root.go index 6e49a14b50d2..daa8bc136bce 100644 --- a/cmd/argo/commands/auth/root.go +++ b/cmd/argo/commands/auth/root.go @@ -8,8 +8,8 @@ func NewAuthCommand() *cobra.Command { command := &cobra.Command{ Use: "auth", Short: "manage authentication settings", - Run: func(cmd *cobra.Command, args []string) { - cmd.HelpFunc()(cmd, args) + RunE: func(cmd *cobra.Command, args []string) error { + return cmd.Help() }, } command.AddCommand(NewTokenCommand()) diff --git a/cmd/argo/commands/auth/token.go b/cmd/argo/commands/auth/token.go index 6e280eeb3509..f31ea70f5043 100644 --- a/cmd/argo/commands/auth/token.go +++ b/cmd/argo/commands/auth/token.go @@ -2,7 +2,6 @@ package auth import ( "fmt" - "os" "github.com/spf13/cobra" @@ -13,12 +12,14 @@ func NewTokenCommand() *cobra.Command { return &cobra.Command{ Use: "token", Short: "Print the auth token", - Run: func(cmd *cobra.Command, args []string) { - if len(args) != 0 { - cmd.HelpFunc()(cmd, args) - os.Exit(1) + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, args []string) error { + authString, err := client.GetAuthString() + if err != nil { + return err } - fmt.Println(client.GetAuthString()) + fmt.Println(authString) + return nil }, } } diff --git a/cmd/argo/commands/client/conn.go b/cmd/argo/commands/client/conn.go index 2c8f100c28d0..cdf7f4783f04 100644 --- a/cmd/argo/commands/client/conn.go +++ b/cmd/argo/commands/client/conn.go @@ -54,23 +54,23 @@ func AddAPIClientFlagsToCmd(cmd *cobra.Command) { cmd.PersistentFlags().BoolVarP(&ArgoServerOpts.InsecureSkipVerify, "insecure-skip-verify", "k", os.Getenv("ARGO_INSECURE_SKIP_VERIFY") == "true", "If true, the Argo Server's certificate will not be checked for validity. This will make your HTTPS connections insecure. Defaults to the ARGO_INSECURE_SKIP_VERIFY environment variable.") } -func NewAPIClient(ctx context.Context) (context.Context, apiclient.Client) { - ctx, client, err := apiclient.NewClientFromOpts( +func NewAPIClient(ctx context.Context) (context.Context, apiclient.Client, error) { + return apiclient.NewClientFromOpts( apiclient.Opts{ ArgoServerOpts: ArgoServerOpts, InstanceID: instanceID, AuthSupplier: func() string { - return GetAuthString() + authString, err := GetAuthString() + if err != nil { + log.Fatal(err) + } + return authString }, ClientConfigSupplier: func() clientcmd.ClientConfig { return GetConfig() }, Offline: Offline, OfflineFiles: OfflineFiles, Context: ctx, }) - if err != nil { - log.Fatal(err) - } - return ctx, client } func Namespace() string { @@ -91,20 +91,20 @@ func Namespace() string { return namespace } -func GetAuthString() string { +func GetAuthString() (string, error) { token, ok := os.LookupEnv("ARGO_TOKEN") if ok { - return token + return token, nil } restConfig, err := GetConfig().ClientConfig() if err != nil { - log.Fatal(err) + return "", err } version := argo.GetVersion() restConfig = restclient.AddUserAgent(restConfig, fmt.Sprintf("argo-workflows/%s argo-cli", version.Version)) authString, err := kubeconfig.GetAuthString(restConfig, explicitPath) if err != nil { - log.Fatal(err) + return "", err } - return authString + return authString, nil } diff --git a/cmd/argo/commands/client/conn_test.go b/cmd/argo/commands/client/conn_test.go index b3d992d3e1f2..e338bc0ca339 100644 --- a/cmd/argo/commands/client/conn_test.go +++ b/cmd/argo/commands/client/conn_test.go @@ -4,13 +4,15 @@ import ( "context" "testing" - "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestGetAuthString(t *testing.T) { t.Setenv("ARGO_TOKEN", "my-token") - assert.Equal(t, "my-token", GetAuthString()) + authString, err := GetAuthString() + require.NoError(t, err) + assert.Equal(t, "my-token", authString) } func TestNamespace(t *testing.T) { @@ -20,26 +22,18 @@ func TestNamespace(t *testing.T) { func TestCreateOfflineClient(t *testing.T) { t.Run("creating an offline client with no files should not fail", func(t *testing.T) { - defer func() { logrus.StandardLogger().ExitFunc = nil }() - var fatal bool - logrus.StandardLogger().ExitFunc = func(int) { fatal = true } - Offline = true OfflineFiles = []string{} - NewAPIClient(context.TODO()) + _, _, err := NewAPIClient(context.TODO()) - assert.False(t, fatal, "should have exited") + assert.NoError(t, err) }) t.Run("creating an offline client with a non-existing file should fail", func(t *testing.T) { - defer func() { logrus.StandardLogger().ExitFunc = nil }() - var fatal bool - logrus.StandardLogger().ExitFunc = func(int) { fatal = true } - Offline = true OfflineFiles = []string{"non-existing-file"} - NewAPIClient(context.TODO()) + _, _, err := NewAPIClient(context.TODO()) - assert.True(t, fatal, "should have exited") + assert.Error(t, err) }) } diff --git a/cmd/argo/commands/clustertemplate/create.go b/cmd/argo/commands/clustertemplate/create.go index ddd72a18d970..94457fa5f494 100644 --- a/cmd/argo/commands/clustertemplate/create.go +++ b/cmd/argo/commands/clustertemplate/create.go @@ -2,8 +2,7 @@ package clustertemplate import ( "context" - "log" - "os" + "fmt" "github.com/spf13/cobra" @@ -31,13 +30,9 @@ func NewCreateCommand() *cobra.Command { argo cluster-template create FILE1 --strict false `, - Run: func(cmd *cobra.Command, args []string) { - if len(args) == 0 { - cmd.HelpFunc()(cmd, args) - os.Exit(1) - } - - createClusterWorkflowTemplates(cmd.Context(), args, &cliCreateOpts) + Args: cobra.MinimumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + return createClusterWorkflowTemplates(cmd.Context(), args, &cliCreateOpts) }, } command.Flags().StringVarP(&cliCreateOpts.output, "output", "o", "", "Output format. One of: name|json|yaml|wide") @@ -45,14 +40,17 @@ func NewCreateCommand() *cobra.Command { return command } -func createClusterWorkflowTemplates(ctx context.Context, filePaths []string, cliOpts *cliCreateOpts) { +func createClusterWorkflowTemplates(ctx context.Context, filePaths []string, cliOpts *cliCreateOpts) error { if cliOpts == nil { cliOpts = &cliCreateOpts{} } - ctx, apiClient := client.NewAPIClient(ctx) + ctx, apiClient, err := client.NewAPIClient(ctx) + if err != nil { + return err + } serviceClient, err := apiClient.NewClusterWorkflowTemplateServiceClient() if err != nil { - log.Fatal(err) + return err } clusterWorkflowTemplates := generateClusterWorkflowTemplates(filePaths, cliOpts.strict) @@ -62,8 +60,9 @@ func createClusterWorkflowTemplates(ctx context.Context, filePaths []string, cli Template: &wftmpl, }) if err != nil { - log.Fatalf("Failed to create cluster workflow template: %s, %v", wftmpl.Name, err) + return fmt.Errorf("Failed to create cluster workflow template: %s, %v", wftmpl.Name, err) } printClusterWorkflowTemplate(created, cliOpts.output) } + return nil } diff --git a/cmd/argo/commands/clustertemplate/delete.go b/cmd/argo/commands/clustertemplate/delete.go index fdf46a84ae7d..d412b232f5a9 100644 --- a/cmd/argo/commands/clustertemplate/delete.go +++ b/cmd/argo/commands/clustertemplate/delete.go @@ -4,7 +4,6 @@ import ( "context" "fmt" - "github.com/argoproj/pkg/errors" "github.com/spf13/cobra" "github.com/argoproj/argo-workflows/v3/cmd/argo/commands/client" @@ -18,8 +17,8 @@ func NewDeleteCommand() *cobra.Command { command := &cobra.Command{ Use: "delete WORKFLOW_TEMPLATE", Short: "delete a cluster workflow template", - Run: func(cmd *cobra.Command, args []string) { - apiServerDeleteClusterWorkflowTemplates(cmd.Context(), all, args) + RunE: func(cmd *cobra.Command, args []string) error { + return apiServerDeleteClusterWorkflowTemplates(cmd.Context(), all, args) }, } @@ -27,15 +26,22 @@ func NewDeleteCommand() *cobra.Command { return command } -func apiServerDeleteClusterWorkflowTemplates(ctx context.Context, allWFs bool, wfTmplNames []string) { - ctx, apiClient := client.NewAPIClient(ctx) +func apiServerDeleteClusterWorkflowTemplates(ctx context.Context, allWFs bool, wfTmplNames []string) error { + ctx, apiClient, err := client.NewAPIClient(ctx) + if err != nil { + return err + } serviceClient, err := apiClient.NewClusterWorkflowTemplateServiceClient() - errors.CheckError(err) + if err != nil { + return err + } var delWFTmplNames []string if allWFs { cwftmplList, err := serviceClient.ListClusterWorkflowTemplates(ctx, &clusterworkflowtemplate.ClusterWorkflowTemplateListRequest{}) - errors.CheckError(err) + if err != nil { + return err + } for _, cwfTmpl := range cwftmplList.Items { delWFTmplNames = append(delWFTmplNames, cwfTmpl.Name) } @@ -47,7 +53,10 @@ func apiServerDeleteClusterWorkflowTemplates(ctx context.Context, allWFs bool, w _, err := serviceClient.DeleteClusterWorkflowTemplate(ctx, &clusterworkflowtemplate.ClusterWorkflowTemplateDeleteRequest{ Name: cwfTmplName, }) - errors.CheckError(err) + if err != nil { + return err + } fmt.Printf("ClusterWorkflowTemplate '%s' deleted\n", cwfTmplName) } + return nil } diff --git a/cmd/argo/commands/clustertemplate/get.go b/cmd/argo/commands/clustertemplate/get.go index 5229150fb55c..ff704bf45a87 100644 --- a/cmd/argo/commands/clustertemplate/get.go +++ b/cmd/argo/commands/clustertemplate/get.go @@ -1,8 +1,6 @@ package clustertemplate import ( - "log" - "github.com/spf13/cobra" "github.com/argoproj/argo-workflows/v3/cmd/argo/commands/client" @@ -15,21 +13,25 @@ func NewGetCommand() *cobra.Command { command := &cobra.Command{ Use: "get CLUSTER WORKFLOW_TEMPLATE...", Short: "display details about a cluster workflow template", - Run: func(cmd *cobra.Command, args []string) { - ctx, apiClient := client.NewAPIClient(cmd.Context()) + RunE: func(cmd *cobra.Command, args []string) error { + ctx, apiClient, err := client.NewAPIClient(cmd.Context()) + if err != nil { + return err + } serviceClient, err := apiClient.NewClusterWorkflowTemplateServiceClient() if err != nil { - log.Fatal(err) + return err } for _, name := range args { wftmpl, err := serviceClient.GetClusterWorkflowTemplate(ctx, &clusterworkflowtmplpkg.ClusterWorkflowTemplateGetRequest{ Name: name, }) if err != nil { - log.Fatal(err) + return err } printClusterWorkflowTemplate(wftmpl, output) } + return nil }, } diff --git a/cmd/argo/commands/clustertemplate/lint.go b/cmd/argo/commands/clustertemplate/lint.go index cd4febb47f9a..141346189e60 100644 --- a/cmd/argo/commands/clustertemplate/lint.go +++ b/cmd/argo/commands/clustertemplate/lint.go @@ -19,13 +19,12 @@ func NewLintCommand() *cobra.Command { command := &cobra.Command{ Use: "lint FILE...", Short: "validate files or directories of cluster workflow template manifests", - Run: func(cmd *cobra.Command, args []string) { - if len(args) == 0 { - cmd.HelpFunc()(cmd, args) - os.Exit(1) + Args: cobra.MinimumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + ctx, apiClient, err := client.NewAPIClient(cmd.Context()) + if err != nil { + return err } - - ctx, apiClient := client.NewAPIClient(cmd.Context()) opts := lint.LintOptions{ Files: args, DefaultNamespace: client.Namespace(), @@ -33,7 +32,7 @@ func NewLintCommand() *cobra.Command { Printer: os.Stdout, } - lint.RunLint(ctx, apiClient, []string{wf.ClusterWorkflowTemplatePlural}, output, false, opts) + return lint.RunLint(ctx, apiClient, []string{wf.ClusterWorkflowTemplatePlural}, output, false, opts) }, } diff --git a/cmd/argo/commands/clustertemplate/list.go b/cmd/argo/commands/clustertemplate/list.go index d720f58b9fb7..71d714893681 100644 --- a/cmd/argo/commands/clustertemplate/list.go +++ b/cmd/argo/commands/clustertemplate/list.go @@ -2,7 +2,6 @@ package clustertemplate import ( "fmt" - "log" "os" "text/tabwriter" @@ -31,16 +30,19 @@ func NewListCommand() *cobra.Command { # List Cluster Workflow Templates by name only: argo cluster-template list -o name `, - Run: func(cmd *cobra.Command, args []string) { - ctx, apiClient := client.NewAPIClient(cmd.Context()) + RunE: func(cmd *cobra.Command, args []string) error { + ctx, apiClient, err := client.NewAPIClient(cmd.Context()) + if err != nil { + return err + } serviceClient, err := apiClient.NewClusterWorkflowTemplateServiceClient() if err != nil { - log.Fatal(err) + return err } cwftmplList, err := serviceClient.ListClusterWorkflowTemplates(ctx, &clusterworkflowtemplate.ClusterWorkflowTemplateListRequest{}) if err != nil { - log.Fatal(err) + return err } switch listArgs.output { case "", "wide": @@ -50,8 +52,9 @@ func NewListCommand() *cobra.Command { fmt.Println(cwftmp.ObjectMeta.Name) } default: - log.Fatalf("Unknown output mode: %s", listArgs.output) + return fmt.Errorf("Unknown output mode: %s", listArgs.output) } + return nil }, } command.Flags().StringVarP(&listArgs.output, "output", "o", "", "Output format. One of: wide|name") diff --git a/cmd/argo/commands/clustertemplate/root.go b/cmd/argo/commands/clustertemplate/root.go index 3a25195b32f1..a1ae2b07a386 100644 --- a/cmd/argo/commands/clustertemplate/root.go +++ b/cmd/argo/commands/clustertemplate/root.go @@ -9,8 +9,8 @@ func NewClusterTemplateCommand() *cobra.Command { Use: "cluster-template", Aliases: []string{"cwftmpl", "cwft"}, Short: "manipulate cluster workflow templates", - Run: func(cmd *cobra.Command, args []string) { - cmd.HelpFunc()(cmd, args) + RunE: func(cmd *cobra.Command, args []string) error { + return cmd.Help() }, } diff --git a/cmd/argo/commands/clustertemplate/update.go b/cmd/argo/commands/clustertemplate/update.go index 346e723380f6..2ef63615180e 100644 --- a/cmd/argo/commands/clustertemplate/update.go +++ b/cmd/argo/commands/clustertemplate/update.go @@ -2,8 +2,7 @@ package clustertemplate import ( "context" - "log" - "os" + "fmt" "github.com/spf13/cobra" @@ -30,13 +29,9 @@ func NewUpdateCommand() *cobra.Command { # Update a Cluster Workflow Template with relaxed validation: argo cluster-template update FILE1 --strict false `, - Run: func(cmd *cobra.Command, args []string) { - if len(args) == 0 { - cmd.HelpFunc()(cmd, args) - os.Exit(1) - } - - updateClusterWorkflowTemplates(cmd.Context(), args, &cliUpdateOpts) + Args: cobra.MinimumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + return updateClusterWorkflowTemplates(cmd.Context(), args, &cliUpdateOpts) }, } command.Flags().StringVarP(&cliUpdateOpts.output, "output", "o", "", "Output format. One of: name|json|yaml|wide") @@ -44,14 +39,17 @@ func NewUpdateCommand() *cobra.Command { return command } -func updateClusterWorkflowTemplates(ctx context.Context, filePaths []string, cliOpts *cliUpdateOpts) { +func updateClusterWorkflowTemplates(ctx context.Context, filePaths []string, cliOpts *cliUpdateOpts) error { if cliOpts == nil { cliOpts = &cliUpdateOpts{} } - ctx, apiClient := client.NewAPIClient(ctx) + ctx, apiClient, err := client.NewAPIClient(ctx) + if err != nil { + return err + } serviceClient, err := apiClient.NewClusterWorkflowTemplateServiceClient() if err != nil { - log.Fatal(err) + return err } clusterWorkflowTemplates := generateClusterWorkflowTemplates(filePaths, cliOpts.strict) @@ -61,15 +59,16 @@ func updateClusterWorkflowTemplates(ctx context.Context, filePaths []string, cli Name: wftmpl.Name, }) if err != nil { - log.Fatalf("Failed to get existing cluster workflow template %q to update: %v", wftmpl.Name, err) + return fmt.Errorf("Failed to get existing cluster workflow template %q to update: %v", wftmpl.Name, err) } wftmpl.ResourceVersion = current.ResourceVersion updated, err := serviceClient.UpdateClusterWorkflowTemplate(ctx, &clusterworkflowtemplate.ClusterWorkflowTemplateUpdateRequest{ Template: &wftmpl, }) if err != nil { - log.Fatalf("Failed to update cluster workflow template: %s, %v", wftmpl.Name, err) + return fmt.Errorf("Failed to update cluster workflow template: %s, %v", wftmpl.Name, err) } printClusterWorkflowTemplate(updated, cliOpts.output) } + return nil } diff --git a/cmd/argo/commands/common/logs.go b/cmd/argo/commands/common/logs.go index 2a007844e22c..be30b2b63264 100644 --- a/cmd/argo/commands/common/logs.go +++ b/cmd/argo/commands/common/logs.go @@ -5,13 +5,12 @@ import ( "fmt" "io" - "github.com/argoproj/pkg/errors" corev1 "k8s.io/api/core/v1" workflowpkg "github.com/argoproj/argo-workflows/v3/pkg/apiclient/workflow" ) -func LogWorkflow(ctx context.Context, serviceClient workflowpkg.WorkflowServiceClient, namespace, workflow, podName, grep, selector string, logOptions *corev1.PodLogOptions) { +func LogWorkflow(ctx context.Context, serviceClient workflowpkg.WorkflowServiceClient, namespace, workflow, podName, grep, selector string, logOptions *corev1.PodLogOptions) error { // logs stream, err := serviceClient.WorkflowLogs(ctx, &workflowpkg.WorkflowLogRequest{ Name: workflow, @@ -21,15 +20,19 @@ func LogWorkflow(ctx context.Context, serviceClient workflowpkg.WorkflowServiceC Selector: selector, Grep: grep, }) - errors.CheckError(err) + if err != nil { + return err + } // loop on log lines for { event, err := stream.Recv() if err == io.EOF { - return + return nil + } + if err != nil { + return err } - errors.CheckError(err) fmt.Println(ansiFormat(fmt.Sprintf("%s: %s", event.PodName, event.Content), ansiColorCode(event.PodName))) } } diff --git a/cmd/argo/commands/common/submit.go b/cmd/argo/commands/common/submit.go index 21e0fbdc687c..905e55ea4c08 100644 --- a/cmd/argo/commands/common/submit.go +++ b/cmd/argo/commands/common/submit.go @@ -22,21 +22,26 @@ type CliSubmitOpts struct { Parameters []string // --parameter } -func WaitWatchOrLog(ctx context.Context, serviceClient workflowpkg.WorkflowServiceClient, namespace string, workflowNames []string, cliSubmitOpts CliSubmitOpts) { +func WaitWatchOrLog(ctx context.Context, serviceClient workflowpkg.WorkflowServiceClient, namespace string, workflowNames []string, cliSubmitOpts CliSubmitOpts) error { if cliSubmitOpts.Log { for _, workflow := range workflowNames { - LogWorkflow(ctx, serviceClient, namespace, workflow, "", "", "", &corev1.PodLogOptions{ + if err := LogWorkflow(ctx, serviceClient, namespace, workflow, "", "", "", &corev1.PodLogOptions{ Container: common.MainContainerName, Follow: true, Previous: false, - }) + }); err != nil { + return err + } } } if cliSubmitOpts.Wait { WaitWorkflows(ctx, serviceClient, namespace, workflowNames, false, !(cliSubmitOpts.Output == "" || cliSubmitOpts.Output == "wide")) } else if cliSubmitOpts.Watch { for _, workflow := range workflowNames { - WatchWorkflow(ctx, serviceClient, namespace, workflow, cliSubmitOpts.GetArgs) + if err := WatchWorkflow(ctx, serviceClient, namespace, workflow, cliSubmitOpts.GetArgs); err != nil { + return err + } } } + return nil } diff --git a/cmd/argo/commands/common/wait.go b/cmd/argo/commands/common/wait.go index 69f7c6b46d47..5718826e99b7 100644 --- a/cmd/argo/commands/common/wait.go +++ b/cmd/argo/commands/common/wait.go @@ -7,7 +7,6 @@ import ( "os" "sync" - "github.com/argoproj/pkg/errors" log "github.com/sirupsen/logrus" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" @@ -26,7 +25,8 @@ func WaitWorkflows(ctx context.Context, serviceClient workflowpkg.WorkflowServic for _, name := range workflowNames { wg.Add(1) go func(name string) { - if !waitOnOne(serviceClient, ctx, name, namespace, ignoreNotFound, quiet) { + ok, err := waitOnOne(serviceClient, ctx, name, namespace, ignoreNotFound, quiet) + if !ok || err != nil { wfSuccessStatus = false } wg.Done() @@ -40,7 +40,7 @@ func WaitWorkflows(ctx context.Context, serviceClient workflowpkg.WorkflowServic } } -func waitOnOne(serviceClient workflowpkg.WorkflowServiceClient, ctx context.Context, wfName, namespace string, ignoreNotFound, quiet bool) bool { +func waitOnOne(serviceClient workflowpkg.WorkflowServiceClient, ctx context.Context, wfName, namespace string, ignoreNotFound, quiet bool) (bool, error) { req := &workflowpkg.WatchWorkflowsRequest{ Namespace: namespace, ListOptions: &metav1.ListOptions{ @@ -51,20 +51,26 @@ func waitOnOne(serviceClient workflowpkg.WorkflowServiceClient, ctx context.Cont stream, err := serviceClient.WatchWorkflows(ctx, req) if err != nil { if status.Code(err) == codes.NotFound && ignoreNotFound { - return true + return true, nil } - errors.CheckError(err) - return false + if err != nil { + return false, err + } + return false, nil } for { event, err := stream.Recv() if err == io.EOF { log.Debug("Re-establishing workflow watch") stream, err = serviceClient.WatchWorkflows(ctx, req) - errors.CheckError(err) + if err != nil { + return false, err + } continue } - errors.CheckError(err) + if err != nil { + return false, err + } if event == nil { continue } @@ -74,9 +80,9 @@ func waitOnOne(serviceClient workflowpkg.WorkflowServiceClient, ctx context.Cont fmt.Printf("%s %s at %v\n", wfName, wf.Status.Phase, wf.Status.FinishedAt) } if wf.Status.Phase == wfv1.WorkflowFailed || wf.Status.Phase == wfv1.WorkflowError { - return false + return false, nil } - return true + return true, nil } } } diff --git a/cmd/argo/commands/common/watch.go b/cmd/argo/commands/common/watch.go index f06513be22ee..4066645fcbba 100644 --- a/cmd/argo/commands/common/watch.go +++ b/cmd/argo/commands/common/watch.go @@ -16,7 +16,7 @@ import ( "github.com/argoproj/argo-workflows/v3/workflow/packer" ) -func WatchWorkflow(ctx context.Context, serviceClient workflowpkg.WorkflowServiceClient, namespace string, workflow string, getArgs GetFlags) { +func WatchWorkflow(ctx context.Context, serviceClient workflowpkg.WorkflowServiceClient, namespace string, workflow string, getArgs GetFlags) error { req := &workflowpkg.WatchWorkflowsRequest{ Namespace: namespace, ListOptions: &metav1.ListOptions{ @@ -25,7 +25,9 @@ func WatchWorkflow(ctx context.Context, serviceClient workflowpkg.WorkflowServic }, } stream, err := serviceClient.WatchWorkflows(ctx, req) - errors.CheckError(err) + if err != nil { + return err + } wfChan := make(chan *wfv1.Workflow) go func() { @@ -52,30 +54,35 @@ func WatchWorkflow(ctx context.Context, serviceClient workflowpkg.WorkflowServic case newWf := <-wfChan: // If we get a new event, update our workflow if newWf == nil { - return + return nil } wf = newWf case <-ticker.C: // If we don't, refresh the workflow screen every second case <-ctx.Done(): // When the context gets canceled - return + return nil } - printWorkflowStatus(wf, getArgs) + err := printWorkflowStatus(wf, getArgs) + if err != nil { + return err + } if wf != nil && !wf.Status.FinishedAt.IsZero() { - return + return nil } } } -func printWorkflowStatus(wf *wfv1.Workflow, getArgs GetFlags) { +func printWorkflowStatus(wf *wfv1.Workflow, getArgs GetFlags) error { if wf == nil { - return + return nil + } + if err := packer.DecompressWorkflow(wf); err != nil { + return err } - err := packer.DecompressWorkflow(wf) - errors.CheckError(err) print("\033[H\033[2J") print("\033[0;0H") fmt.Print(PrintWorkflowHelper(wf, getArgs)) + return nil } diff --git a/cmd/argo/commands/completion.go b/cmd/argo/commands/completion.go index 2b6cf88eb3cb..8e52cd9a3252 100644 --- a/cmd/argo/commands/completion.go +++ b/cmd/argo/commands/completion.go @@ -3,7 +3,6 @@ package commands import ( "fmt" "io" - "log" "os" "github.com/spf13/cobra" @@ -130,11 +129,8 @@ variable. For fish, output to a file in ~/.config/fish/completions `, - Run: func(cmd *cobra.Command, args []string) { - if len(args) != 1 { - cmd.HelpFunc()(cmd, args) - os.Exit(1) - } + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { shell := args[0] rootCommand := NewCommand() rootCommand.BashCompletionFunction = bashCompletionFunc @@ -145,12 +141,9 @@ For fish, output to a file in ~/.config/fish/completions } completion, ok := availableCompletions[shell] if !ok { - fmt.Printf("Invalid shell '%s'. The supported shells are bash and zsh.\n", shell) - os.Exit(1) - } - if err := completion(os.Stdout, rootCommand); err != nil { - log.Fatal(err) + return fmt.Errorf("Invalid shell '%s'. The supported shells are bash and zsh.\n", shell) } + return completion(os.Stdout, rootCommand) }, } return command diff --git a/cmd/argo/commands/cp.go b/cmd/argo/commands/cp.go index 36ca1230590a..b0781d87a4d5 100644 --- a/cmd/argo/commands/cp.go +++ b/cmd/argo/commands/cp.go @@ -48,7 +48,10 @@ func NewCpCommand() *cobra.Command { workflowName := args[0] outputDir := args[1] - ctx, apiClient := client.NewAPIClient(cmd.Context()) + ctx, apiClient, err := client.NewAPIClient(cmd.Context()) + if err != nil { + return err + } serviceClient := apiClient.NewWorkflowServiceClient() if len(namespace) == 0 { namespace = client.Namespace() @@ -117,7 +120,11 @@ func getAndStoreArtifactData(namespace string, workflowName string, nodeId strin if err != nil { return fmt.Errorf("failed to create request: %w", err) } - request.Header.Set("Authorization", client.GetAuthString()) + authString, err := client.GetAuthString() + if err != nil { + return err + } + request.Header.Set("Authorization", authString) resp, err := c.Do(request) if err != nil { return fmt.Errorf("request failed with: %w", err) diff --git a/cmd/argo/commands/cron/create.go b/cmd/argo/commands/cron/create.go index efe1d78da47e..fd97f535c58e 100644 --- a/cmd/argo/commands/cron/create.go +++ b/cmd/argo/commands/cron/create.go @@ -3,7 +3,6 @@ package cron import ( "context" "fmt" - "log" "github.com/spf13/cobra" @@ -28,10 +27,15 @@ func NewCreateCommand() *cobra.Command { command := &cobra.Command{ Use: "create FILE1 FILE2...", Short: "create a cron workflow", - Run: func(cmd *cobra.Command, args []string) { - checkArgs(cmd, args, parametersFile, &submitOpts) - - CreateCronWorkflows(cmd.Context(), args, &cliCreateOpts, &submitOpts) + Args: cobra.MinimumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + if parametersFile != "" { + err := util.ReadParametersFile(parametersFile, &submitOpts) + if err != nil { + return err + } + } + return CreateCronWorkflows(cmd.Context(), args, &cliCreateOpts, &submitOpts) }, } @@ -42,11 +46,14 @@ func NewCreateCommand() *cobra.Command { return command } -func CreateCronWorkflows(ctx context.Context, filePaths []string, cliOpts *cliCreateOpts, submitOpts *wfv1.SubmitOpts) { - ctx, apiClient := client.NewAPIClient(ctx) +func CreateCronWorkflows(ctx context.Context, filePaths []string, cliOpts *cliCreateOpts, submitOpts *wfv1.SubmitOpts) error { + ctx, apiClient, err := client.NewAPIClient(ctx) + if err != nil { + return err + } serviceClient, err := apiClient.NewCronWorkflowServiceClient() if err != nil { - log.Fatal(err) + return err } cronWorkflows := generateCronWorkflows(filePaths, cliOpts.strict) @@ -59,7 +66,7 @@ func CreateCronWorkflows(ctx context.Context, filePaths []string, cliOpts *cliCr newWf := wfv1.Workflow{Spec: cronWf.Spec.WorkflowSpec} err := util.ApplySubmitOpts(&newWf, submitOpts) if err != nil { - log.Fatal(err) + return err } cronWf.Spec.WorkflowSpec = newWf.Spec // We have only copied the workflow spec to the cron workflow but not the metadata @@ -81,8 +88,9 @@ func CreateCronWorkflows(ctx context.Context, filePaths []string, cliOpts *cliCr CronWorkflow: &cronWf, }) if err != nil { - log.Fatalf("Failed to create cron workflow: %v", err) + return fmt.Errorf("Failed to create cron workflow: %v", err) } fmt.Print(getCronWorkflowGet(created)) } + return nil } diff --git a/cmd/argo/commands/cron/delete.go b/cmd/argo/commands/cron/delete.go index 9e3bf4790ade..250524f0f39b 100644 --- a/cmd/argo/commands/cron/delete.go +++ b/cmd/argo/commands/cron/delete.go @@ -1,7 +1,6 @@ package cron import ( - "github.com/argoproj/pkg/errors" "github.com/spf13/cobra" "github.com/argoproj/argo-workflows/v3/cmd/argo/commands/client" @@ -15,15 +14,22 @@ func NewDeleteCommand() *cobra.Command { command := &cobra.Command{ Use: "delete [CRON_WORKFLOW... | --all]", Short: "delete a cron workflow", - Run: func(cmd *cobra.Command, args []string) { - ctx, apiClient := client.NewAPIClient(cmd.Context()) + RunE: func(cmd *cobra.Command, args []string) error { + ctx, apiClient, err := client.NewAPIClient(cmd.Context()) + if err != nil { + return err + } serviceClient, err := apiClient.NewCronWorkflowServiceClient() - errors.CheckError(err) + if err != nil { + return err + } if all { cronWfList, err := serviceClient.ListCronWorkflows(ctx, &cronworkflowpkg.ListCronWorkflowsRequest{ Namespace: client.Namespace(), }) - errors.CheckError(err) + if err != nil { + return err + } for _, cronWf := range cronWfList.Items { args = append(args, cronWf.Name) } @@ -33,8 +39,11 @@ func NewDeleteCommand() *cobra.Command { Name: name, Namespace: client.Namespace(), }) - errors.CheckError(err) + if err != nil { + return err + } } + return nil }, } diff --git a/cmd/argo/commands/cron/get.go b/cmd/argo/commands/cron/get.go index 9f3538f2f342..75e5e242e150 100644 --- a/cmd/argo/commands/cron/get.go +++ b/cmd/argo/commands/cron/get.go @@ -1,9 +1,6 @@ package cron import ( - "os" - - "github.com/argoproj/pkg/errors" "github.com/spf13/cobra" "github.com/argoproj/argo-workflows/v3/cmd/argo/commands/client" @@ -16,15 +13,16 @@ func NewGetCommand() *cobra.Command { command := &cobra.Command{ Use: "get CRON_WORKFLOW...", Short: "display details about a cron workflow", - Run: func(cmd *cobra.Command, args []string) { - if len(args) == 0 { - cmd.HelpFunc()(cmd, args) - os.Exit(1) + Args: cobra.MinimumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + ctx, apiClient, err := client.NewAPIClient(cmd.Context()) + if err != nil { + return err } - - ctx, apiClient := client.NewAPIClient(cmd.Context()) serviceClient, err := apiClient.NewCronWorkflowServiceClient() - errors.CheckError(err) + if err != nil { + return err + } namespace := client.Namespace() for _, arg := range args { @@ -32,9 +30,12 @@ func NewGetCommand() *cobra.Command { Name: arg, Namespace: namespace, }) - errors.CheckError(err) + if err != nil { + return err + } printCronWorkflow(cronWf, output) } + return nil }, } diff --git a/cmd/argo/commands/cron/lint.go b/cmd/argo/commands/cron/lint.go index 9f41235cfa99..5c7f3a863885 100644 --- a/cmd/argo/commands/cron/lint.go +++ b/cmd/argo/commands/cron/lint.go @@ -19,19 +19,19 @@ func NewLintCommand() *cobra.Command { command := &cobra.Command{ Use: "lint FILE...", Short: "validate files or directories of cron workflow manifests", - Run: func(cmd *cobra.Command, args []string) { - if len(args) == 0 { - cmd.HelpFunc()(cmd, args) - os.Exit(1) + Args: cobra.MinimumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + ctx, apiClient, err := client.NewAPIClient(cmd.Context()) + if err != nil { + return err } - ctx, apiClient := client.NewAPIClient(cmd.Context()) opts := lint.LintOptions{ Files: args, Strict: strict, DefaultNamespace: client.Namespace(), Printer: os.Stdout, } - lint.RunLint(ctx, apiClient, []string{wf.CronWorkflowPlural}, output, false, opts) + return lint.RunLint(ctx, apiClient, []string{wf.CronWorkflowPlural}, output, false, opts) }, } diff --git a/cmd/argo/commands/cron/list.go b/cmd/argo/commands/cron/list.go index fb7e05917700..d4ccfc4b118b 100644 --- a/cmd/argo/commands/cron/list.go +++ b/cmd/argo/commands/cron/list.go @@ -2,12 +2,10 @@ package cron import ( "fmt" - "log" "os" "text/tabwriter" "time" - "github.com/argoproj/pkg/errors" "github.com/argoproj/pkg/humanize" "github.com/spf13/cobra" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -28,10 +26,15 @@ func NewListCommand() *cobra.Command { command := &cobra.Command{ Use: "list", Short: "list cron workflows", - Run: func(cmd *cobra.Command, args []string) { - ctx, apiClient := client.NewAPIClient(cmd.Context()) + RunE: func(cmd *cobra.Command, args []string) error { + ctx, apiClient, err := client.NewAPIClient(cmd.Context()) + if err != nil { + return err + } serviceClient, err := apiClient.NewCronWorkflowServiceClient() - errors.CheckError(err) + if err != nil { + return err + } namespace := client.Namespace() if listArgs.allNamespaces { namespace = "" @@ -42,7 +45,9 @@ func NewListCommand() *cobra.Command { Namespace: namespace, ListOptions: &listOpts, }) - errors.CheckError(err) + if err != nil { + return err + } switch listArgs.output { case "", "wide": printTable(cronWfList.Items, &listArgs) @@ -51,8 +56,9 @@ func NewListCommand() *cobra.Command { fmt.Println(cronWf.ObjectMeta.Name) } default: - log.Fatalf("Unknown output mode: %s", listArgs.output) + return fmt.Errorf("Unknown output mode: %s", listArgs.output) } + return nil }, } command.Flags().BoolVarP(&listArgs.allNamespaces, "all-namespaces", "A", false, "Show workflows from all namespaces") diff --git a/cmd/argo/commands/cron/resume.go b/cmd/argo/commands/cron/resume.go index e3738ae80239..12811c83b695 100644 --- a/cmd/argo/commands/cron/resume.go +++ b/cmd/argo/commands/cron/resume.go @@ -3,7 +3,6 @@ package cron import ( "fmt" - "github.com/argoproj/pkg/errors" "github.com/spf13/cobra" "github.com/argoproj/argo-workflows/v3/cmd/argo/commands/client" @@ -15,19 +14,27 @@ func NewResumeCommand() *cobra.Command { command := &cobra.Command{ Use: "resume [CRON_WORKFLOW...]", Short: "resume zero or more cron workflows", - Run: func(cmd *cobra.Command, args []string) { - ctx, apiClient := client.NewAPIClient(cmd.Context()) + RunE: func(cmd *cobra.Command, args []string) error { + ctx, apiClient, err := client.NewAPIClient(cmd.Context()) + if err != nil { + return err + } serviceClient, err := apiClient.NewCronWorkflowServiceClient() - errors.CheckError(err) + if err != nil { + return err + } namespace := client.Namespace() for _, name := range args { _, err := serviceClient.ResumeCronWorkflow(ctx, &cronworkflowpkg.CronWorkflowResumeRequest{ Name: name, Namespace: namespace, }) - errors.CheckError(err) + if err != nil { + return err + } fmt.Printf("CronWorkflow '%s' resumed\n", name) } + return nil }, } diff --git a/cmd/argo/commands/cron/root.go b/cmd/argo/commands/cron/root.go index da11a6f3a886..37c6f060011b 100644 --- a/cmd/argo/commands/cron/root.go +++ b/cmd/argo/commands/cron/root.go @@ -9,8 +9,8 @@ func NewCronWorkflowCommand() *cobra.Command { Use: "cron", Short: "manage cron workflows", Long: `NextScheduledRun assumes that the workflow-controller uses UTC as its timezone`, - Run: func(cmd *cobra.Command, args []string) { - cmd.HelpFunc()(cmd, args) + RunE: func(cmd *cobra.Command, args []string) error { + return cmd.Help() }, } diff --git a/cmd/argo/commands/cron/suspend.go b/cmd/argo/commands/cron/suspend.go index 3a7e46ac5fff..084e2a7b3353 100644 --- a/cmd/argo/commands/cron/suspend.go +++ b/cmd/argo/commands/cron/suspend.go @@ -3,7 +3,6 @@ package cron import ( "fmt" - "github.com/argoproj/pkg/errors" "github.com/spf13/cobra" "github.com/argoproj/argo-workflows/v3/cmd/argo/commands/client" @@ -15,20 +14,28 @@ func NewSuspendCommand() *cobra.Command { command := &cobra.Command{ Use: "suspend CRON_WORKFLOW...", Short: "suspend zero or more cron workflows", - Run: func(cmd *cobra.Command, args []string) { - ctx, apiClient := client.NewAPIClient(cmd.Context()) + RunE: func(cmd *cobra.Command, args []string) error { + ctx, apiClient, err := client.NewAPIClient(cmd.Context()) + if err != nil { + return err + } serviceClient, err := apiClient.NewCronWorkflowServiceClient() - errors.CheckError(err) + if err != nil { + return err + } namespace := client.Namespace() for _, name := range args { cronWf, err := serviceClient.SuspendCronWorkflow(ctx, &cronworkflowpkg.CronWorkflowSuspendRequest{ Name: name, Namespace: namespace, }) - errors.CheckError(err) + if err != nil { + return err + } cronWf.Spec.Suspend = true fmt.Printf("CronWorkflow '%s' suspended\n", name) } + return nil }, } return command diff --git a/cmd/argo/commands/cron/update.go b/cmd/argo/commands/cron/update.go index cb92661543d0..e1340267fe95 100644 --- a/cmd/argo/commands/cron/update.go +++ b/cmd/argo/commands/cron/update.go @@ -3,7 +3,6 @@ package cron import ( "context" "fmt" - "log" "github.com/spf13/cobra" @@ -36,10 +35,15 @@ func NewUpdateCommand() *cobra.Command { # Update a Cron Workflow Template with relaxed validation: argo cron update FILE1 --strict false `, - Run: func(cmd *cobra.Command, args []string) { - checkArgs(cmd, args, parametersFile, &submitOpts) - - updateCronWorkflows(cmd.Context(), args, &cliUpdateOpts, &submitOpts) + Args: cobra.MinimumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + if parametersFile != "" { + err := util.ReadParametersFile(parametersFile, &submitOpts) + if err != nil { + return err + } + } + return updateCronWorkflows(cmd.Context(), args, &cliUpdateOpts, &submitOpts) }, } @@ -49,11 +53,14 @@ func NewUpdateCommand() *cobra.Command { return command } -func updateCronWorkflows(ctx context.Context, filePaths []string, cliOpts *cliUpdateOpts, submitOpts *wfv1.SubmitOpts) { - ctx, apiClient := client.NewAPIClient(ctx) +func updateCronWorkflows(ctx context.Context, filePaths []string, cliOpts *cliUpdateOpts, submitOpts *wfv1.SubmitOpts) error { + ctx, apiClient, err := client.NewAPIClient(ctx) + if err != nil { + return err + } serviceClient, err := apiClient.NewCronWorkflowServiceClient() if err != nil { - log.Fatal(err) + return err } cronWorkflows := generateCronWorkflows(filePaths, cliOpts.strict) @@ -62,7 +69,7 @@ func updateCronWorkflows(ctx context.Context, filePaths []string, cliOpts *cliUp newWf := wfv1.Workflow{Spec: cronWf.Spec.WorkflowSpec} err := util.ApplySubmitOpts(&newWf, submitOpts) if err != nil { - log.Fatal(err) + return err } if cronWf.Namespace == "" { cronWf.Namespace = client.Namespace() @@ -72,7 +79,7 @@ func updateCronWorkflows(ctx context.Context, filePaths []string, cliOpts *cliUp Namespace: cronWf.Namespace, }) if err != nil { - log.Fatalf("Failed to get existing cron workflow %q to update: %v", cronWf.Name, err) + return fmt.Errorf("Failed to get existing cron workflow %q to update: %v", cronWf.Name, err) } cronWf.ResourceVersion = current.ResourceVersion updated, err := serviceClient.UpdateCronWorkflow(ctx, &cronworkflowpkg.UpdateCronWorkflowRequest{ @@ -80,8 +87,9 @@ func updateCronWorkflows(ctx context.Context, filePaths []string, cliOpts *cliUp CronWorkflow: &cronWf, }) if err != nil { - log.Fatalf("Failed to update workflow template: %v", err) + return fmt.Errorf("Failed to update workflow template: %v", err) } fmt.Print(getCronWorkflowGet(updated)) } + return nil } diff --git a/cmd/argo/commands/cron/util.go b/cmd/argo/commands/cron/util.go index 457835a5821b..eefbd754c0c1 100644 --- a/cmd/argo/commands/cron/util.go +++ b/cmd/argo/commands/cron/util.go @@ -4,19 +4,16 @@ import ( "encoding/json" "fmt" "log" - "os" "strings" "time" argoJson "github.com/argoproj/pkg/json" "github.com/robfig/cron/v3" - "github.com/spf13/cobra" "sigs.k8s.io/yaml" "github.com/argoproj/argo-workflows/v3/workflow/common" "github.com/argoproj/argo-workflows/v3/workflow/util" - "github.com/argoproj/pkg/errors" "github.com/argoproj/pkg/humanize" "github.com/argoproj/argo-workflows/v3/pkg/apis/workflow/v1alpha1" @@ -61,18 +58,6 @@ func generateCronWorkflows(filePaths []string, strict bool) []v1alpha1.CronWorkf return cronWorkflows } -func checkArgs(cmd *cobra.Command, args []string, parametersFile string, submitOpts *v1alpha1.SubmitOpts) { - if len(args) == 0 { - cmd.HelpFunc()(cmd, args) - os.Exit(1) - } - - if parametersFile != "" { - err := util.ReadParametersFile(parametersFile, submitOpts) - errors.CheckError(err) - } -} - // unmarshalCronWorkflows unmarshals the input bytes as either json or yaml func unmarshalCronWorkflows(wfBytes []byte, strict bool) []wfv1.CronWorkflow { var cronWf wfv1.CronWorkflow diff --git a/cmd/argo/commands/delete.go b/cmd/argo/commands/delete.go index 49b75dff0c08..8fde329d3f79 100644 --- a/cmd/argo/commands/delete.go +++ b/cmd/argo/commands/delete.go @@ -1,10 +1,9 @@ package commands import ( + "errors" "fmt" - "os" - "github.com/argoproj/pkg/errors" "github.com/spf13/cobra" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" @@ -23,6 +22,10 @@ func NewDeleteCommand() *cobra.Command { allNamespaces bool dryRun bool force bool + hasFilterFlag = func() bool { + return all || allNamespaces || flags.completed || flags.resubmitted || flags.prefix != "" || + flags.labels != "" || flags.fields != "" || flags.finishedBefore != "" || len(flags.status) > 0 + } ) command := &cobra.Command{ Use: "delete [--dry-run] [WORKFLOW...|[--all] [--older] [--completed] [--resubmitted] [--prefix PREFIX] [--selector SELECTOR] [--force] [--status STATUS] ]", @@ -35,16 +38,17 @@ func NewDeleteCommand() *cobra.Command { argo delete @latest `, - Run: func(cmd *cobra.Command, args []string) { - hasFilterFlag := all || allNamespaces || flags.completed || flags.resubmitted || flags.prefix != "" || - flags.labels != "" || flags.fields != "" || flags.finishedBefore != "" || len(flags.status) > 0 - - if len(args) == 0 && !hasFilterFlag { - cmd.HelpFunc()(cmd, args) - os.Exit(1) + Args: func(cmd *cobra.Command, args []string) error { + if len(args) == 0 && !hasFilterFlag() { + return errors.New("requires either a workflow or other argument") + } + return nil + }, + RunE: func(cmd *cobra.Command, args []string) error { + ctx, apiClient, err := client.NewAPIClient(cmd.Context()) + if err != nil { + return err } - - ctx, apiClient := client.NewAPIClient(cmd.Context()) serviceClient := apiClient.NewWorkflowServiceClient() var workflows wfv1.Workflows if !allNamespaces { @@ -55,15 +59,17 @@ func NewDeleteCommand() *cobra.Command { ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: flags.namespace}, }) } - if hasFilterFlag { + if hasFilterFlag() { listed, err := listWorkflows(ctx, serviceClient, flags) - errors.CheckError(err) + if err != nil { + return err + } workflows = append(workflows, listed...) } if len(workflows) == 0 { fmt.Printf("No resources found\n") - return + return nil } for _, wf := range workflows { @@ -73,13 +79,18 @@ func NewDeleteCommand() *cobra.Command { } _, err := serviceClient.DeleteWorkflow(ctx, &workflowpkg.WorkflowDeleteRequest{Name: wf.Name, Namespace: wf.Namespace, Force: force}) - if err != nil && status.Code(err) == codes.NotFound { - fmt.Printf("Workflow '%s' not found\n", wf.Name) - continue + if err != nil { + if status.Code(err) == codes.NotFound { + fmt.Printf("Workflow '%s' not found\n", wf.Name) + continue + } else { + return err + } } - errors.CheckError(err) fmt.Printf("Workflow '%s' deleted\n", wf.Name) } + + return nil }, } diff --git a/cmd/argo/commands/executorplugin/build.go b/cmd/argo/commands/executorplugin/build.go index f9891187309d..447e6704eadf 100644 --- a/cmd/argo/commands/executorplugin/build.go +++ b/cmd/argo/commands/executorplugin/build.go @@ -2,7 +2,6 @@ package executorplugin import ( "fmt" - "os" "github.com/spf13/cobra" @@ -14,11 +13,8 @@ func NewBuildCommand() *cobra.Command { Use: "build DIR", Short: "build an executor plugin", SilenceUsage: true, + Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - if len(args) != 1 { - cmd.HelpFunc()(cmd, args) - os.Exit(1) - } pluginDir := args[0] plug, err := loadPluginManifest(pluginDir) if err != nil { diff --git a/cmd/argo/commands/executorplugin/root.go b/cmd/argo/commands/executorplugin/root.go index 3ce4dc35266b..46b626c43b3b 100644 --- a/cmd/argo/commands/executorplugin/root.go +++ b/cmd/argo/commands/executorplugin/root.go @@ -8,8 +8,8 @@ func NewRootCommand() *cobra.Command { command := &cobra.Command{ Use: "executor-plugin", Short: "manage executor plugins", - Run: func(cmd *cobra.Command, args []string) { - cmd.HelpFunc()(cmd, args) + RunE: func(cmd *cobra.Command, args []string) error { + return cmd.Help() }, } diff --git a/cmd/argo/commands/get.go b/cmd/argo/commands/get.go index cb98fb1c9750..c1b50cded144 100644 --- a/cmd/argo/commands/get.go +++ b/cmd/argo/commands/get.go @@ -3,10 +3,7 @@ package commands import ( "encoding/json" "fmt" - "log" - "os" - "github.com/argoproj/pkg/errors" "github.com/spf13/cobra" "sigs.k8s.io/yaml" @@ -29,12 +26,12 @@ func NewGetCommand() *cobra.Command { # Get the latest workflow: argo get @latest `, - Run: func(cmd *cobra.Command, args []string) { - if len(args) == 0 { - cmd.HelpFunc()(cmd, args) - os.Exit(1) + Args: cobra.MinimumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + ctx, apiClient, err := client.NewAPIClient(cmd.Context()) + if err != nil { + return err } - ctx, apiClient := client.NewAPIClient(cmd.Context()) serviceClient := apiClient.NewWorkflowServiceClient() namespace := client.Namespace() for _, name := range args { @@ -42,9 +39,14 @@ func NewGetCommand() *cobra.Command { Name: name, Namespace: namespace, }) - errors.CheckError(err) - printWorkflow(wf, getArgs) + if err != nil { + return err + } + if err := printWorkflow(wf, getArgs); err != nil { + return err + } } + return nil }, } @@ -56,7 +58,7 @@ func NewGetCommand() *cobra.Command { return command } -func printWorkflow(wf *wfv1.Workflow, getArgs common.GetFlags) { +func printWorkflow(wf *wfv1.Workflow, getArgs common.GetFlags) error { switch getArgs.Output { case "name": fmt.Println(wf.ObjectMeta.Name) @@ -69,6 +71,7 @@ func printWorkflow(wf *wfv1.Workflow, getArgs common.GetFlags) { case "short", "wide", "": fmt.Print(common.PrintWorkflowHelper(wf, getArgs)) default: - log.Fatalf("Unknown output format: %s", getArgs.Output) + return fmt.Errorf("Unknown output format: %s", getArgs.Output) } + return nil } diff --git a/cmd/argo/commands/lint.go b/cmd/argo/commands/lint.go index 6d6390d73986..bdcb9843a6f9 100644 --- a/cmd/argo/commands/lint.go +++ b/cmd/argo/commands/lint.go @@ -35,13 +35,9 @@ func NewLintCommand() *cobra.Command { # Lint only manifests of Workflows and CronWorkflows from stdin: cat manifests.yaml | argo lint --kinds=workflows,cronworkflows -`, - Run: func(cmd *cobra.Command, args []string) { - if len(args) == 0 { - cmd.HelpFunc()(cmd, args) - os.Exit(1) - } - - runLint(cmd.Context(), args, offline, lintKinds, output, strict) + Args: cobra.MinimumNArgs(0), + RunE: func(cmd *cobra.Command, args []string) error { + return runLint(cmd.Context(), args, offline, lintKinds, output, strict) }, } @@ -54,10 +50,13 @@ func NewLintCommand() *cobra.Command { return command } -func runLint(ctx context.Context, args []string, offline bool, lintKinds []string, output string, strict bool) { +func runLint(ctx context.Context, args []string, offline bool, lintKinds []string, output string, strict bool) error { client.Offline = offline client.OfflineFiles = args - ctx, apiClient := client.NewAPIClient(ctx) + ctx, apiClient, err := client.NewAPIClient(ctx) + if err != nil { + return err + } if len(lintKinds) == 0 || strings.Contains(strings.Join(lintKinds, ","), "all") { lintKinds = allKinds @@ -68,5 +67,5 @@ func runLint(ctx context.Context, args []string, offline bool, lintKinds []strin DefaultNamespace: client.Namespace(), Printer: os.Stdout, } - lint.RunLint(ctx, apiClient, lintKinds, output, offline, ops) + return lint.RunLint(ctx, apiClient, lintKinds, output, offline, ops) } diff --git a/cmd/argo/commands/lint_test.go b/cmd/argo/commands/lint_test.go index 6c2226fc554d..33df96027722 100644 --- a/cmd/argo/commands/lint_test.go +++ b/cmd/argo/commands/lint_test.go @@ -90,8 +90,9 @@ spec: var fatal bool logrus.StandardLogger().ExitFunc = func(int) { fatal = true } - runLint(context.Background(), []string{workflowPath}, true, nil, "pretty", true) + err = runLint(context.Background(), []string{workflowPath}, true, nil, "pretty", true) + require.NoError(t, err) assert.True(t, fatal, "should have exited") }) @@ -100,8 +101,9 @@ spec: var fatal bool logrus.StandardLogger().ExitFunc = func(int) { fatal = true } - runLint(context.Background(), []string{workflowPath, clusterWftmplPath}, true, nil, "pretty", true) + err = runLint(context.Background(), []string{workflowPath, clusterWftmplPath}, true, nil, "pretty", true) + require.NoError(t, err) assert.True(t, fatal, "should have exited") }) @@ -110,8 +112,9 @@ spec: var fatal bool logrus.StandardLogger().ExitFunc = func(int) { fatal = true } - runLint(context.Background(), []string{workflowPath, wftmplPath}, true, nil, "pretty", true) + err = runLint(context.Background(), []string{workflowPath, wftmplPath}, true, nil, "pretty", true) + require.NoError(t, err) assert.True(t, fatal, "should have exited") }) @@ -120,8 +123,9 @@ spec: var fatal bool logrus.StandardLogger().ExitFunc = func(int) { fatal = true } - runLint(context.Background(), []string{wftmplPath}, true, nil, "pretty", true) + err = runLint(context.Background(), []string{wftmplPath}, true, nil, "pretty", true) + require.NoError(t, err) assert.False(t, fatal, "should not have exited") }) @@ -130,8 +134,9 @@ spec: var fatal bool logrus.StandardLogger().ExitFunc = func(int) { fatal = true } - runLint(context.Background(), []string{clusterWftmplPath}, true, nil, "pretty", true) + err = runLint(context.Background(), []string{clusterWftmplPath}, true, nil, "pretty", true) + require.NoError(t, err) assert.False(t, fatal, "should not have exited") }) @@ -140,8 +145,9 @@ spec: var fatal bool logrus.StandardLogger().ExitFunc = func(int) { fatal = true } - runLint(context.Background(), []string{workflowPath, wftmplPath, clusterWftmplPath}, true, nil, "pretty", true) + err = runLint(context.Background(), []string{workflowPath, wftmplPath, clusterWftmplPath}, true, nil, "pretty", true) + require.NoError(t, err) assert.False(t, fatal, "should not have exited") }) @@ -150,8 +156,9 @@ spec: var fatal bool logrus.StandardLogger().ExitFunc = func(int) { fatal = true } - runLint(context.Background(), []string{dir}, true, nil, "pretty", true) + err = runLint(context.Background(), []string{dir}, true, nil, "pretty", true) + require.NoError(t, err) assert.False(t, fatal, "should not have exited") }) @@ -168,8 +175,9 @@ spec: _ = os.Stdin.Close() // close previously opened path to avoid errors trying to remove the file. }() - runLint(context.Background(), []string{workflowPath, wftmplPath, "-"}, true, nil, "pretty", true) + err = runLint(context.Background(), []string{workflowPath, wftmplPath, "-"}, true, nil, "pretty", true) + require.NoError(t, err) assert.False(t, fatal, "should not have exited") }) @@ -199,8 +207,9 @@ spec: var fatal bool logrus.StandardLogger().ExitFunc = func(int) { fatal = true } - runLint(context.Background(), []string{workflowCaseSensitivePath}, true, nil, "pretty", true) + err = runLint(context.Background(), []string{workflowCaseSensitivePath}, true, nil, "pretty", true) + require.NoError(t, err) assert.True(t, fatal, "should have exited") }) @@ -209,8 +218,9 @@ spec: var fatal bool logrus.StandardLogger().ExitFunc = func(int) { fatal = true } - runLint(context.Background(), []string{workflowCaseSensitivePath}, true, nil, "pretty", false) + err = runLint(context.Background(), []string{workflowCaseSensitivePath}, true, nil, "pretty", false) + require.NoError(t, err) assert.False(t, fatal, "should not have exited") }) } diff --git a/cmd/argo/commands/list.go b/cmd/argo/commands/list.go index 189797238d42..e5258175df2f 100644 --- a/cmd/argo/commands/list.go +++ b/cmd/argo/commands/list.go @@ -2,11 +2,11 @@ package commands import ( "context" + "errors" "os" "sort" "strings" - "github.com/argoproj/pkg/errors" argotime "github.com/argoproj/pkg/time" log "github.com/sirupsen/logrus" "github.com/spf13/cobra" @@ -92,20 +92,24 @@ func NewListCommand() *cobra.Command { argo list -l label1=value1,label2=value2 `, - Run: func(cmd *cobra.Command, args []string) { - ctx, apiClient := client.NewAPIClient(cmd.Context()) + RunE: func(cmd *cobra.Command, args []string) error { + ctx, apiClient, err := client.NewAPIClient(cmd.Context()) + if err != nil { + return err + } serviceClient := apiClient.NewWorkflowServiceClient() if !allNamespaces { listArgs.namespace = client.Namespace() } workflows, err := listWorkflows(ctx, serviceClient, listArgs) - errors.CheckError(err) - err = printer.PrintWorkflows(workflows, os.Stdout, printer.PrintOpts{ + if err != nil { + return err + } + return printer.PrintWorkflows(workflows, os.Stdout, printer.PrintOpts{ NoHeaders: listArgs.noHeaders, Namespace: allNamespaces, Output: listArgs.output, }) - errors.CheckError(err) }, } command.Flags().BoolVarP(&allNamespaces, "all-namespaces", "A", false, "Show workflows from all namespaces") @@ -129,7 +133,9 @@ func listWorkflows(ctx context.Context, serviceClient workflowpkg.WorkflowServic Limit: flags.chunkSize, } labelSelector, err := labels.Parse(flags.labels) - errors.CheckError(err) + if err != nil { + return nil, err + } if len(flags.status) != 0 { req, _ := labels.NewRequirement(common.LabelKeyPhase, selection.In, flags.status) if req != nil { @@ -137,7 +143,7 @@ func listWorkflows(ctx context.Context, serviceClient workflowpkg.WorkflowServic } } if flags.completed && flags.running { - log.Fatal("--completed and --running cannot be used together") + return nil, errors.New("--completed and --running cannot be used together") } if flags.completed { req, _ := labels.NewRequirement(common.LabelKeyCompleted, selection.Equals, []string{"true"}) @@ -176,19 +182,27 @@ func listWorkflows(ctx context.Context, serviceClient workflowpkg.WorkflowServic }) if flags.createdSince != "" && flags.finishedBefore != "" { startTime, err := argotime.ParseSince(flags.createdSince) - errors.CheckError(err) + if err != nil { + return nil, err + } endTime, err := argotime.ParseSince(flags.finishedBefore) - errors.CheckError(err) + if err != nil { + return nil, err + } workflows = workflows.Filter(wfv1.WorkflowRanBetween(*startTime, *endTime)) } else { if flags.createdSince != "" { t, err := argotime.ParseSince(flags.createdSince) - errors.CheckError(err) + if err != nil { + return nil, err + } workflows = workflows.Filter(wfv1.WorkflowCreatedAfter(*t)) } if flags.finishedBefore != "" { t, err := argotime.ParseSince(flags.finishedBefore) - errors.CheckError(err) + if err != nil { + return nil, err + } workflows = workflows.Filter(wfv1.WorkflowFinishedBefore(*t)) } } diff --git a/cmd/argo/commands/logs.go b/cmd/argo/commands/logs.go index b8b8b482c234..557c725f8f8f 100644 --- a/cmd/argo/commands/logs.go +++ b/cmd/argo/commands/logs.go @@ -1,11 +1,9 @@ package commands import ( - "log" - "os" + "errors" "time" - "github.com/argoproj/pkg/errors" "github.com/spf13/cobra" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -54,7 +52,8 @@ func NewLogsCommand() *cobra.Command { # Print the logs of the latest workflow: argo logs @latest `, - Run: func(cmd *cobra.Command, args []string) { + Args: cobra.RangeArgs(1, 2), + RunE: func(cmd *cobra.Command, args []string) error { // parse all the args workflow := "" podName := "" @@ -66,12 +65,11 @@ func NewLogsCommand() *cobra.Command { workflow = args[0] podName = args[1] default: - cmd.HelpFunc()(cmd, args) - os.Exit(1) + return errors.New("expected one or two arguments") } if since > 0 && sinceTime != "" { - log.Fatal("--since-time and --since cannot be used together") + return errors.New("--since-time and --since cannot be used together") } if since > 0 { @@ -80,7 +78,9 @@ func NewLogsCommand() *cobra.Command { if sinceTime != "" { parsedTime, err := time.Parse(time.RFC3339, sinceTime) - errors.CheckError(err) + if err != nil { + return err + } sinceTime := metav1.NewTime(parsedTime) logOptions.SinceTime = &sinceTime } @@ -90,11 +90,14 @@ func NewLogsCommand() *cobra.Command { } // set-up - ctx, apiClient := client.NewAPIClient(cmd.Context()) + ctx, apiClient, err := client.NewAPIClient(cmd.Context()) + if err != nil { + return err + } serviceClient := apiClient.NewWorkflowServiceClient() namespace := client.Namespace() - common.LogWorkflow(ctx, serviceClient, namespace, workflow, podName, grep, selector, logOptions) + return common.LogWorkflow(ctx, serviceClient, namespace, workflow, podName, grep, selector, logOptions) }, } command.Flags().StringVarP(&logOptions.Container, "container", "c", "main", "Print the logs of this container") diff --git a/cmd/argo/commands/node.go b/cmd/argo/commands/node.go index fb4403d37053..82abbd73485b 100644 --- a/cmd/argo/commands/node.go +++ b/cmd/argo/commands/node.go @@ -3,12 +3,9 @@ package commands import ( "encoding/json" "fmt" - "log" - "os" "strconv" "strings" - "github.com/argoproj/pkg/errors" "github.com/spf13/cobra" "k8s.io/apimachinery/pkg/fields" @@ -37,14 +34,10 @@ func NewNodeCommand() *cobra.Command { argo node set my-wf --message "We did it!"" --node-field-selector displayName=approve `, - Run: func(cmd *cobra.Command, args []string) { - if len(args) < 2 { - cmd.HelpFunc()(cmd, args) - os.Exit(1) - } - + Args: cobra.MinimumNArgs(2), + RunE: func(cmd *cobra.Command, args []string) error { if args[0] != "set" { - log.Fatalf("unknown action '%s'", args[0]) + return fmt.Errorf("unknown action '%s'", args[0]) } outputParameters := "" @@ -53,7 +46,7 @@ func NewNodeCommand() *cobra.Command { for _, param := range setArgs.outputParameters { parts := strings.SplitN(param, "=", 2) if len(parts) != 2 { - log.Fatalf("expected parameter of the form: NAME=VALUE. Received: %s", param) + return fmt.Errorf("expected parameter of the form: NAME=VALUE. Received: %s", param) } unquoted, err := strconv.Unquote(parts[1]) if err != nil { @@ -63,18 +56,21 @@ func NewNodeCommand() *cobra.Command { } res, err := json.Marshal(outputParams) if err != nil { - log.Fatalf("unable to parse output parameter set request: %s", err) + return fmt.Errorf("unable to parse output parameter set request: %s", err) } outputParameters = string(res) } - ctx, apiClient := client.NewAPIClient(cmd.Context()) + ctx, apiClient, err := client.NewAPIClient(cmd.Context()) + if err != nil { + return err + } serviceClient := apiClient.NewWorkflowServiceClient() namespace := client.Namespace() selector, err := fields.ParseSelector(setArgs.nodeFieldSelector) if err != nil { - log.Fatalf("Unable to parse node field selector '%s': %s", setArgs.nodeFieldSelector, err) + return fmt.Errorf("Unable to parse node field selector '%s': %s", setArgs.nodeFieldSelector, err) } _, err = serviceClient.SetWorkflow(ctx, &workflowpkg.WorkflowSetRequest{ @@ -85,8 +81,11 @@ func NewNodeCommand() *cobra.Command { Phase: setArgs.phase, OutputParameters: outputParameters, }) - errors.CheckError(err) + if err != nil { + return err + } fmt.Printf("workflow values set\n") + return nil }, } command.Flags().StringVar(&setArgs.nodeFieldSelector, "node-field-selector", "", "Selector of node to set, eg: --node-field-selector inputs.paramaters.myparam.value=abc") diff --git a/cmd/argo/commands/resubmit.go b/cmd/argo/commands/resubmit.go index 7eb4b7e9697b..049dafeb6101 100644 --- a/cmd/argo/commands/resubmit.go +++ b/cmd/argo/commands/resubmit.go @@ -3,7 +3,6 @@ package commands import ( "context" - "github.com/argoproj/pkg/errors" "github.com/spf13/cobra" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -70,16 +69,18 @@ func NewResubmitCommand() *cobra.Command { argo resubmit @latest `, - Run: func(cmd *cobra.Command, args []string) { + RunE: func(cmd *cobra.Command, args []string) error { if cmd.Flag("priority").Changed { cliSubmitOpts.Priority = &resubmitOpts.priority } - ctx, apiClient := client.NewAPIClient(cmd.Context()) + ctx, apiClient, err := client.NewAPIClient(cmd.Context()) + if err != nil { + return err + } serviceClient := apiClient.NewWorkflowServiceClient() resubmitOpts.namespace = client.Namespace() - err := resubmitWorkflows(ctx, serviceClient, resubmitOpts, cliSubmitOpts, args) - errors.CheckError(err) + return resubmitWorkflows(ctx, serviceClient, resubmitOpts, cliSubmitOpts, args) }, } @@ -140,11 +141,13 @@ func resubmitWorkflows(ctx context.Context, serviceClient workflowpkg.WorkflowSe if err != nil { return err } - printWorkflow(lastResubmitted, common.GetFlags{Output: cliSubmitOpts.Output}) + if err = printWorkflow(lastResubmitted, common.GetFlags{Output: cliSubmitOpts.Output}); err != nil { + return err + } } if len(resubmittedNames) == 1 { // watch or wait when there is only one workflow retried - common.WaitWatchOrLog(ctx, serviceClient, lastResubmitted.Namespace, []string{lastResubmitted.Name}, cliSubmitOpts) + return common.WaitWatchOrLog(ctx, serviceClient, lastResubmitted.Namespace, []string{lastResubmitted.Name}, cliSubmitOpts) } return nil } diff --git a/cmd/argo/commands/resume.go b/cmd/argo/commands/resume.go index 5837d36ff2c9..de6d1c88d348 100644 --- a/cmd/argo/commands/resume.go +++ b/cmd/argo/commands/resume.go @@ -1,9 +1,8 @@ package commands import ( + "errors" "fmt" - "log" - "os" "github.com/spf13/cobra" "k8s.io/apimachinery/pkg/fields" @@ -38,19 +37,23 @@ func NewResumeCommand() *cobra.Command { argo resume --node-field-selector inputs.paramaters.myparam.value=abc `, - Run: func(cmd *cobra.Command, args []string) { + Args: func(cmd *cobra.Command, args []string) error { if len(args) == 0 && resumeArgs.nodeFieldSelector == "" { - cmd.HelpFunc()(cmd, args) - os.Exit(1) + return errors.New("requires either node field selector or workflow") + } + return nil + }, + RunE: func(cmd *cobra.Command, args []string) error { + ctx, apiClient, err := client.NewAPIClient(cmd.Context()) + if err != nil { + return err } - - ctx, apiClient := client.NewAPIClient(cmd.Context()) serviceClient := apiClient.NewWorkflowServiceClient() namespace := client.Namespace() selector, err := fields.ParseSelector(resumeArgs.nodeFieldSelector) if err != nil { - log.Fatalf("Unable to parse node field selector '%s': %s", resumeArgs.nodeFieldSelector, err) + return fmt.Errorf("Unable to parse node field selector '%s': %s", resumeArgs.nodeFieldSelector, err) } for _, wfName := range args { @@ -60,10 +63,11 @@ func NewResumeCommand() *cobra.Command { NodeFieldSelector: selector.String(), }) if err != nil { - log.Fatalf("Failed to resume %s: %+v", wfName, err) + return fmt.Errorf("Failed to resume %s: %+v", wfName, err) } fmt.Printf("workflow %s resumed\n", wfName) } + return nil }, } command.Flags().StringVar(&resumeArgs.nodeFieldSelector, "node-field-selector", "", "selector of node to resume, eg: --node-field-selector inputs.paramaters.myparam.value=abc") diff --git a/cmd/argo/commands/retry.go b/cmd/argo/commands/retry.go index 06611b6f9999..e202e2fe23c3 100644 --- a/cmd/argo/commands/retry.go +++ b/cmd/argo/commands/retry.go @@ -2,10 +2,9 @@ package commands import ( "context" + "errors" "fmt" - "os" - "github.com/argoproj/pkg/errors" "github.com/spf13/cobra" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/fields" @@ -76,17 +75,21 @@ func NewRetryCommand() *cobra.Command { # Restart node with id 5 on successful workflow, using node-field-selector argo retry my-wf --restart-successful --node-field-selector id=5 `, - Run: func(cmd *cobra.Command, args []string) { + Args: func(cmd *cobra.Command, args []string) error { if len(args) == 0 && !retryOpts.hasSelector() { - cmd.HelpFunc()(cmd, args) - os.Exit(1) + return errors.New("requires either node field selector or workflow") + } + return nil + }, + RunE: func(cmd *cobra.Command, args []string) error { + ctx, apiClient, err := client.NewAPIClient(cmd.Context()) + if err != nil { + return err } - ctx, apiClient := client.NewAPIClient(cmd.Context()) serviceClient := apiClient.NewWorkflowServiceClient() retryOpts.namespace = client.Namespace() - err := retryWorkflows(ctx, serviceClient, retryOpts, cliSubmitOpts, args) - errors.CheckError(err) + return retryWorkflows(ctx, serviceClient, retryOpts, cliSubmitOpts, args) }, } command.Flags().StringArrayVarP(&cliSubmitOpts.Parameters, "parameter", "p", []string{}, "input parameter to override on the original workflow spec") @@ -147,11 +150,13 @@ func retryWorkflows(ctx context.Context, serviceClient workflowpkg.WorkflowServi if err != nil { return err } - printWorkflow(lastRetried, common.GetFlags{Output: cliSubmitOpts.Output}) + if err = printWorkflow(lastRetried, common.GetFlags{Output: cliSubmitOpts.Output}); err != nil { + return err + } } if len(retriedNames) == 1 { // watch or wait when there is only one workflow retried - common.WaitWatchOrLog(ctx, serviceClient, lastRetried.Namespace, []string{lastRetried.Name}, cliSubmitOpts) + return common.WaitWatchOrLog(ctx, serviceClient, lastRetried.Namespace, []string{lastRetried.Name}, cliSubmitOpts) } return nil } diff --git a/cmd/argo/commands/root.go b/cmd/argo/commands/root.go index 0e652f576346..322f215d3cae 100644 --- a/cmd/argo/commands/root.go +++ b/cmd/argo/commands/root.go @@ -88,8 +88,8 @@ If your server is behind an ingress with a path (running "argo server --base-hre ARGO_BASE_HREF=/argo `, - Run: func(cmd *cobra.Command, args []string) { - cmd.HelpFunc()(cmd, args) + RunE: func(cmd *cobra.Command, args []string) error { + return cmd.Help() }, } diff --git a/cmd/argo/commands/stop.go b/cmd/argo/commands/stop.go index 98cff94dfd18..68fe9d8427c8 100644 --- a/cmd/argo/commands/stop.go +++ b/cmd/argo/commands/stop.go @@ -2,10 +2,9 @@ package commands import ( "context" + "errors" "fmt" - "os" - "github.com/argoproj/pkg/errors" "github.com/spf13/cobra" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/fields" @@ -55,17 +54,21 @@ func NewStopCommand() *cobra.Command { argo stop --field-selector metadata.namespace=argo `, - Run: func(cmd *cobra.Command, args []string) { + Args: func(cmd *cobra.Command, args []string) error { if len(args) == 0 && !stopArgs.hasSelector() { - cmd.HelpFunc()(cmd, args) - os.Exit(1) + return errors.New("requires either selector or workflow") + } + return nil + }, + RunE: func(cmd *cobra.Command, args []string) error { + ctx, apiClient, err := client.NewAPIClient(cmd.Context()) + if err != nil { + return err } - ctx, apiClient := client.NewAPIClient(cmd.Context()) serviceClient := apiClient.NewWorkflowServiceClient() stopArgs.namespace = client.Namespace() - err := stopWorkflows(ctx, serviceClient, stopArgs, args) - errors.CheckError(err) + return stopWorkflows(ctx, serviceClient, stopArgs, args) }, } command.Flags().StringVar(&stopArgs.message, "message", "", "Message to add to previously running nodes") diff --git a/cmd/argo/commands/submit.go b/cmd/argo/commands/submit.go index ad097616ee29..0aa1f9f7d5ff 100644 --- a/cmd/argo/commands/submit.go +++ b/cmd/argo/commands/submit.go @@ -2,12 +2,11 @@ package commands import ( "context" + "errors" "fmt" - "os" "strings" "time" - "github.com/argoproj/pkg/errors" argoJson "github.com/argoproj/pkg/json" log "github.com/sirupsen/logrus" "github.com/spf13/cobra" @@ -56,7 +55,13 @@ func NewSubmitCommand() *cobra.Command { cat my-wf.yaml | argo submit - `, - Run: func(cmd *cobra.Command, args []string) { + Args: func(cmd *cobra.Command, args []string) error { + if from != "" && len(args) != 0 { + return errors.New("cannot combine --from with file arguments") + } + return nil + }, + RunE: func(cmd *cobra.Command, args []string) error { if cmd.Flag("priority").Changed { cliSubmitOpts.Priority = &priority } @@ -66,21 +71,21 @@ func NewSubmitCommand() *cobra.Command { } if parametersFile != "" { - err := util.ReadParametersFile(parametersFile, &submitOpts) - errors.CheckError(err) + if err := util.ReadParametersFile(parametersFile, &submitOpts); err != nil { + return err + } } - ctx, apiClient := client.NewAPIClient(cmd.Context()) + ctx, apiClient, err := client.NewAPIClient(cmd.Context()) + if err != nil { + return err + } serviceClient := apiClient.NewWorkflowServiceClient() namespace := client.Namespace() if from != "" { - if len(args) != 0 { - cmd.HelpFunc()(cmd, args) - os.Exit(1) - } - submitWorkflowFromResource(ctx, serviceClient, namespace, from, &submitOpts, &cliSubmitOpts) + return submitWorkflowFromResource(ctx, serviceClient, namespace, from, &submitOpts, &cliSubmitOpts) } else { - submitWorkflowsFromFile(ctx, serviceClient, namespace, args, &submitOpts, &cliSubmitOpts) + return submitWorkflowsFromFile(ctx, serviceClient, namespace, args, &submitOpts, &cliSubmitOpts) } }, } @@ -104,9 +109,11 @@ func NewSubmitCommand() *cobra.Command { return command } -func submitWorkflowsFromFile(ctx context.Context, serviceClient workflowpkg.WorkflowServiceClient, namespace string, filePaths []string, submitOpts *wfv1.SubmitOpts, cliOpts *common.CliSubmitOpts) { +func submitWorkflowsFromFile(ctx context.Context, serviceClient workflowpkg.WorkflowServiceClient, namespace string, filePaths []string, submitOpts *wfv1.SubmitOpts, cliOpts *common.CliSubmitOpts) error { fileContents, err := util.ReadManifest(filePaths...) - errors.CheckError(err) + if err != nil { + return err + } var workflows []wfv1.Workflow for _, body := range fileContents { @@ -114,65 +121,68 @@ func submitWorkflowsFromFile(ctx context.Context, serviceClient workflowpkg.Work workflows = append(workflows, wfs...) } - submitWorkflows(ctx, serviceClient, namespace, workflows, submitOpts, cliOpts) + return submitWorkflows(ctx, serviceClient, namespace, workflows, submitOpts, cliOpts) } -func validateOptions(workflows []wfv1.Workflow, submitOpts *wfv1.SubmitOpts, cliOpts *common.CliSubmitOpts) { +func validateOptions(workflows []wfv1.Workflow, submitOpts *wfv1.SubmitOpts, cliOpts *common.CliSubmitOpts) error { if cliOpts.Watch { if len(workflows) > 1 { - log.Fatalf("Cannot watch more than one workflow") + return errors.New("Cannot watch more than one workflow") } if cliOpts.Wait { - log.Fatalf("--wait cannot be combined with --watch") + return errors.New("--wait cannot be combined with --watch") } if submitOpts.DryRun { - log.Fatalf("--watch cannot be combined with --dry-run") + return errors.New("--watch cannot be combined with --dry-run") } if submitOpts.ServerDryRun { - log.Fatalf("--watch cannot be combined with --server-dry-run") + return errors.New("--watch cannot be combined with --server-dry-run") } } if cliOpts.Wait { if submitOpts.DryRun { - log.Fatalf("--wait cannot be combined with --dry-run") + return errors.New("--wait cannot be combined with --dry-run") } if submitOpts.ServerDryRun { - log.Fatalf("--wait cannot be combined with --server-dry-run") + return errors.New("--wait cannot be combined with --server-dry-run") } } if submitOpts.DryRun { if cliOpts.Output == "" { - log.Fatalf("--dry-run should have an output option") + return errors.New("--dry-run should have an output option") } if submitOpts.ServerDryRun { - log.Fatalf("--dry-run cannot be combined with --server-dry-run") + return errors.New("--dry-run cannot be combined with --server-dry-run") } } if submitOpts.ServerDryRun { if cliOpts.Output == "" { - log.Fatalf("--server-dry-run should have an output option") + return errors.New("--server-dry-run should have an output option") } } + return nil } -func submitWorkflowFromResource(ctx context.Context, serviceClient workflowpkg.WorkflowServiceClient, namespace string, resourceIdentifier string, submitOpts *wfv1.SubmitOpts, cliOpts *common.CliSubmitOpts) { +func submitWorkflowFromResource(ctx context.Context, serviceClient workflowpkg.WorkflowServiceClient, namespace string, resourceIdentifier string, submitOpts *wfv1.SubmitOpts, cliOpts *common.CliSubmitOpts) error { parts := strings.SplitN(resourceIdentifier, "/", 2) if len(parts) != 2 { - log.Fatalf("resource identifier '%s' is malformed. Should be `kind/name`, e.g. cronwf/hello-world-cwf", resourceIdentifier) + return fmt.Errorf("resource identifier '%s' is malformed. Should be `kind/name`, e.g. cronwf/hello-world-cwf", resourceIdentifier) } kind := parts[0] name := parts[1] tempwf := wfv1.Workflow{} - validateOptions([]wfv1.Workflow{tempwf}, submitOpts, cliOpts) + if err := validateOptions([]wfv1.Workflow{tempwf}, submitOpts, cliOpts); err != nil { + return err + } if cliOpts.ScheduledTime != "" { _, err := time.Parse(time.RFC3339, cliOpts.ScheduledTime) if err != nil { - log.Fatalf("scheduled-time contains invalid time.RFC3339 format. (e.g.: `2006-01-02T15:04:05-07:00`)") + return fmt.Errorf("scheduled-time contains invalid time.RFC3339 format. (e.g.: `2006-01-02T15:04:05-07:00`)") } submitOpts.Annotations = fmt.Sprintf("%s=%s", wfcommon.AnnotationKeyCronWfScheduledTime, cliOpts.ScheduledTime) } @@ -184,20 +194,23 @@ func submitWorkflowFromResource(ctx context.Context, serviceClient workflowpkg.W SubmitOptions: submitOpts, }) if err != nil { - log.Fatalf("Failed to submit workflow: %v", err) + return fmt.Errorf("Failed to submit workflow: %v", err) } - printWorkflow(created, common.GetFlags{Output: cliOpts.Output}) + if err = printWorkflow(created, common.GetFlags{Output: cliOpts.Output}); err != nil { + return err + } - common.WaitWatchOrLog(ctx, serviceClient, namespace, []string{created.Name}, *cliOpts) + return common.WaitWatchOrLog(ctx, serviceClient, namespace, []string{created.Name}, *cliOpts) } -func submitWorkflows(ctx context.Context, serviceClient workflowpkg.WorkflowServiceClient, namespace string, workflows []wfv1.Workflow, submitOpts *wfv1.SubmitOpts, cliOpts *common.CliSubmitOpts) { - validateOptions(workflows, submitOpts, cliOpts) +func submitWorkflows(ctx context.Context, serviceClient workflowpkg.WorkflowServiceClient, namespace string, workflows []wfv1.Workflow, submitOpts *wfv1.SubmitOpts, cliOpts *common.CliSubmitOpts) error { + if err := validateOptions(workflows, submitOpts, cliOpts); err != nil { + return err + } if len(workflows) == 0 { - log.Println("No Workflow found in given files") - os.Exit(1) + return errors.New("No Workflow found in given files") } var workflowNames []string @@ -208,7 +221,9 @@ func submitWorkflows(ctx context.Context, serviceClient workflowpkg.WorkflowServ wf.Namespace = namespace } err := util.ApplySubmitOpts(&wf, submitOpts) - errors.CheckError(err) + if err != nil { + return err + } if cliOpts.Priority != nil { wf.Spec.Priority = cliOpts.Priority } @@ -223,14 +238,16 @@ func submitWorkflows(ctx context.Context, serviceClient workflowpkg.WorkflowServ CreateOptions: options, }) if err != nil { - log.Fatalf("Failed to submit workflow: %v", err) + return fmt.Errorf("Failed to submit workflow: %v", err) } - printWorkflow(created, common.GetFlags{Output: cliOpts.Output, Status: cliOpts.GetArgs.Status}) + if err = printWorkflow(created, common.GetFlags{Output: cliOpts.Output, Status: cliOpts.GetArgs.Status}); err != nil { + return err + } workflowNames = append(workflowNames, created.Name) } - common.WaitWatchOrLog(ctx, serviceClient, namespace, workflowNames, *cliOpts) + return common.WaitWatchOrLog(ctx, serviceClient, namespace, workflowNames, *cliOpts) } // unmarshalWorkflows unmarshals the input bytes as either json or yaml diff --git a/cmd/argo/commands/submit_test.go b/cmd/argo/commands/submit_test.go index f3c8767d8f1b..47e25099f8da 100644 --- a/cmd/argo/commands/submit_test.go +++ b/cmd/argo/commands/submit_test.go @@ -6,6 +6,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/argoproj/argo-workflows/v3/cmd/argo/commands/common" @@ -15,14 +16,25 @@ import ( ) func Test_submitWorkflows(t *testing.T) { + t.Run("Submit workflow with invalid options", func(t *testing.T) { + c := &workflowmocks.WorkflowServiceClient{} + err := submitWorkflows(context.TODO(), c, "argo", []wfv1.Workflow{}, &wfv1.SubmitOpts{}, &common.CliSubmitOpts{Watch: true, Wait: true}) + require.Error(t, err, "--wait cannot be combined with --watch") + }) + t.Run("Submit without providing workflow", func(t *testing.T) { + c := &workflowmocks.WorkflowServiceClient{} + err := submitWorkflows(context.TODO(), c, "argo", []wfv1.Workflow{}, &wfv1.SubmitOpts{}, &common.CliSubmitOpts{}) + require.Error(t, err, "No Workflow found in given files") + }) t.Run("Submit workflow with priority set in spec", func(t *testing.T) { c := &workflowmocks.WorkflowServiceClient{} priority := int32(70) workflow := wfv1.Workflow{ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "argo"}, Spec: wfv1.WorkflowSpec{Priority: &priority}} c.On("CreateWorkflow", mock.Anything, mock.Anything).Return(&wfv1.Workflow{}, nil) - submitWorkflows(context.TODO(), c, "argo", []wfv1.Workflow{workflow}, &wfv1.SubmitOpts{}, &common.CliSubmitOpts{}) + err := submitWorkflows(context.TODO(), c, "argo", []wfv1.Workflow{workflow}, &wfv1.SubmitOpts{}, &common.CliSubmitOpts{}) + require.NoError(t, err) arg := c.Mock.Calls[0].Arguments[1] wfC, ok := arg.(*workflowpkg.WorkflowCreateRequest) if !ok { @@ -41,8 +53,9 @@ func Test_submitWorkflows(t *testing.T) { cliSubmitOpts := common.CliSubmitOpts{Priority: &priorityCLI} c.On("CreateWorkflow", mock.Anything, mock.Anything).Return(&wfv1.Workflow{}, nil) - submitWorkflows(context.TODO(), c, "argo", []wfv1.Workflow{workflow}, &wfv1.SubmitOpts{}, &cliSubmitOpts) + err := submitWorkflows(context.TODO(), c, "argo", []wfv1.Workflow{workflow}, &wfv1.SubmitOpts{}, &cliSubmitOpts) + require.NoError(t, err) arg := c.Mock.Calls[0].Arguments[1] wfC, ok := arg.(*workflowpkg.WorkflowCreateRequest) if !ok { diff --git a/cmd/argo/commands/suspend.go b/cmd/argo/commands/suspend.go index 814b3796a0aa..26446867d592 100644 --- a/cmd/argo/commands/suspend.go +++ b/cmd/argo/commands/suspend.go @@ -2,7 +2,6 @@ package commands import ( "fmt" - "log" "github.com/spf13/cobra" @@ -21,8 +20,11 @@ func NewSuspendCommand() *cobra.Command { # Suspend the latest workflow: argo suspend @latest `, - Run: func(cmd *cobra.Command, args []string) { - ctx, apiClient := client.NewAPIClient(cmd.Context()) + RunE: func(cmd *cobra.Command, args []string) error { + ctx, apiClient, err := client.NewAPIClient(cmd.Context()) + if err != nil { + return err + } serviceClient := apiClient.NewWorkflowServiceClient() namespace := client.Namespace() for _, wfName := range args { @@ -31,10 +33,11 @@ func NewSuspendCommand() *cobra.Command { Namespace: namespace, }) if err != nil { - log.Fatalf("Failed to suspended %s: %+v", wfName, err) + return fmt.Errorf("Failed to suspended %s: %+v", wfName, err) } fmt.Printf("workflow %s suspended\n", wfName) } + return nil }, } return command diff --git a/cmd/argo/commands/template/create.go b/cmd/argo/commands/template/create.go index 388ab98e9cd0..6003407ebd74 100644 --- a/cmd/argo/commands/template/create.go +++ b/cmd/argo/commands/template/create.go @@ -2,8 +2,7 @@ package template import ( "context" - "log" - "os" + "fmt" "github.com/spf13/cobra" @@ -21,13 +20,9 @@ func NewCreateCommand() *cobra.Command { command := &cobra.Command{ Use: "create FILE1 FILE2...", Short: "create a workflow template", - Run: func(cmd *cobra.Command, args []string) { - if len(args) == 0 { - cmd.HelpFunc()(cmd, args) - os.Exit(1) - } - - CreateWorkflowTemplates(cmd.Context(), args, &cliCreateOpts) + Args: cobra.MinimumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + return CreateWorkflowTemplates(cmd.Context(), args, &cliCreateOpts) }, } command.Flags().StringVarP(&cliCreateOpts.output, "output", "o", "", "Output format. One of: name|json|yaml|wide") @@ -35,14 +30,17 @@ func NewCreateCommand() *cobra.Command { return command } -func CreateWorkflowTemplates(ctx context.Context, filePaths []string, cliOpts *cliCreateOpts) { +func CreateWorkflowTemplates(ctx context.Context, filePaths []string, cliOpts *cliCreateOpts) error { if cliOpts == nil { cliOpts = &cliCreateOpts{} } - ctx, apiClient := client.NewAPIClient(ctx) + ctx, apiClient, err := client.NewAPIClient(ctx) + if err != nil { + return err + } serviceClient, err := apiClient.NewWorkflowTemplateServiceClient() if err != nil { - log.Fatal(err) + return err } workflowTemplates := generateWorkflowTemplates(filePaths, cliOpts.strict) @@ -56,8 +54,9 @@ func CreateWorkflowTemplates(ctx context.Context, filePaths []string, cliOpts *c Template: &wftmpl, }) if err != nil { - log.Fatalf("Failed to create workflow template: %v", err) + return fmt.Errorf("Failed to create workflow template: %v", err) } printWorkflowTemplate(created, cliOpts.output) } + return nil } diff --git a/cmd/argo/commands/template/delete.go b/cmd/argo/commands/template/delete.go index d3b221b88ce0..93edc52fd6d4 100644 --- a/cmd/argo/commands/template/delete.go +++ b/cmd/argo/commands/template/delete.go @@ -3,9 +3,7 @@ package template import ( "context" "fmt" - "log" - "github.com/argoproj/pkg/errors" "github.com/spf13/cobra" "github.com/argoproj/argo-workflows/v3/cmd/argo/commands/client" @@ -19,8 +17,8 @@ func NewDeleteCommand() *cobra.Command { command := &cobra.Command{ Use: "delete WORKFLOW_TEMPLATE", Short: "delete a workflow template", - Run: func(cmd *cobra.Command, args []string) { - apiServerDeleteWorkflowTemplates(cmd.Context(), all, args) + RunE: func(cmd *cobra.Command, args []string) error { + return apiServerDeleteWorkflowTemplates(cmd.Context(), all, args) }, } @@ -28,11 +26,14 @@ func NewDeleteCommand() *cobra.Command { return command } -func apiServerDeleteWorkflowTemplates(ctx context.Context, allWFs bool, wfTmplNames []string) { - ctx, apiClient := client.NewAPIClient(ctx) +func apiServerDeleteWorkflowTemplates(ctx context.Context, allWFs bool, wfTmplNames []string) error { + ctx, apiClient, err := client.NewAPIClient(ctx) + if err != nil { + return err + } serviceClient, err := apiClient.NewWorkflowTemplateServiceClient() if err != nil { - log.Fatal(err) + return err } namespace := client.Namespace() var delWFTmplNames []string @@ -41,7 +42,7 @@ func apiServerDeleteWorkflowTemplates(ctx context.Context, allWFs bool, wfTmplNa Namespace: namespace, }) if err != nil { - log.Fatal(err) + return err } for _, wfTmpl := range wftmplList.Items { delWFTmplNames = append(delWFTmplNames, wfTmpl.Name) @@ -51,17 +52,21 @@ func apiServerDeleteWorkflowTemplates(ctx context.Context, allWFs bool, wfTmplNa delWFTmplNames = wfTmplNames } for _, wfTmplNames := range delWFTmplNames { - apiServerDeleteWorkflowTemplate(serviceClient, ctx, namespace, wfTmplNames) + if err := apiServerDeleteWorkflowTemplate(serviceClient, ctx, namespace, wfTmplNames); err != nil { + return err + } } + return nil } -func apiServerDeleteWorkflowTemplate(client workflowtemplatepkg.WorkflowTemplateServiceClient, ctx context.Context, namespace, wftmplName string) { +func apiServerDeleteWorkflowTemplate(client workflowtemplatepkg.WorkflowTemplateServiceClient, ctx context.Context, namespace, wftmplName string) error { _, err := client.DeleteWorkflowTemplate(ctx, &workflowtemplatepkg.WorkflowTemplateDeleteRequest{ Name: wftmplName, Namespace: namespace, }) if err != nil { - errors.CheckError(err) + return err } fmt.Printf("WorkflowTemplate '%s' deleted\n", wftmplName) + return nil } diff --git a/cmd/argo/commands/template/get.go b/cmd/argo/commands/template/get.go index 49820ead8f78..2ed16f93c1ed 100644 --- a/cmd/argo/commands/template/get.go +++ b/cmd/argo/commands/template/get.go @@ -1,8 +1,6 @@ package template import ( - "log" - "github.com/spf13/cobra" "github.com/argoproj/argo-workflows/v3/cmd/argo/commands/client" @@ -15,11 +13,14 @@ func NewGetCommand() *cobra.Command { command := &cobra.Command{ Use: "get WORKFLOW_TEMPLATE...", Short: "display details about a workflow template", - Run: func(cmd *cobra.Command, args []string) { - ctx, apiClient := client.NewAPIClient(cmd.Context()) + RunE: func(cmd *cobra.Command, args []string) error { + ctx, apiClient, err := client.NewAPIClient(cmd.Context()) + if err != nil { + return err + } serviceClient, err := apiClient.NewWorkflowTemplateServiceClient() if err != nil { - log.Fatal(err) + return err } namespace := client.Namespace() for _, name := range args { @@ -28,10 +29,11 @@ func NewGetCommand() *cobra.Command { Namespace: namespace, }) if err != nil { - log.Fatal(err) + return err } printWorkflowTemplate(wftmpl, output) } + return nil }, } diff --git a/cmd/argo/commands/template/lint.go b/cmd/argo/commands/template/lint.go index c01e427217e0..29c7f849766c 100644 --- a/cmd/argo/commands/template/lint.go +++ b/cmd/argo/commands/template/lint.go @@ -19,19 +19,19 @@ func NewLintCommand() *cobra.Command { command := &cobra.Command{ Use: "lint (DIRECTORY | FILE1 FILE2 FILE3...)", Short: "validate a file or directory of workflow template manifests", - Run: func(cmd *cobra.Command, args []string) { - if len(args) == 0 { - cmd.HelpFunc()(cmd, args) - os.Exit(1) + Args: cobra.MinimumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + ctx, apiClient, err := client.NewAPIClient(cmd.Context()) + if err != nil { + return err } - ctx, apiClient := client.NewAPIClient(cmd.Context()) opts := lint.LintOptions{ Files: args, Strict: strict, DefaultNamespace: client.Namespace(), Printer: os.Stdout, } - lint.RunLint(ctx, apiClient, []string{wf.WorkflowTemplatePlural}, output, false, opts) + return lint.RunLint(ctx, apiClient, []string{wf.WorkflowTemplatePlural}, output, false, opts) }, } diff --git a/cmd/argo/commands/template/list.go b/cmd/argo/commands/template/list.go index 166712e650eb..42b4b21aa59c 100644 --- a/cmd/argo/commands/template/list.go +++ b/cmd/argo/commands/template/list.go @@ -2,11 +2,9 @@ package template import ( "fmt" - "log" "os" "text/tabwriter" - "github.com/argoproj/pkg/errors" "github.com/spf13/cobra" apiv1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -28,18 +26,23 @@ func NewListCommand() *cobra.Command { command := &cobra.Command{ Use: "list", Short: "list workflow templates", - Run: func(cmd *cobra.Command, args []string) { - ctx, apiClient := client.NewAPIClient(cmd.Context()) + RunE: func(cmd *cobra.Command, args []string) error { + ctx, apiClient, err := client.NewAPIClient(cmd.Context()) + if err != nil { + return err + } serviceClient, err := apiClient.NewWorkflowTemplateServiceClient() if err != nil { - log.Fatal(err) + return err } namespace := client.Namespace() if listArgs.allNamespaces { namespace = apiv1.NamespaceAll } labelSelector, err := labels.Parse(listArgs.labels) - errors.CheckError(err) + if err != nil { + return err + } wftmplList, err := serviceClient.ListWorkflowTemplates(ctx, &workflowtemplatepkg.WorkflowTemplateListRequest{ Namespace: namespace, @@ -48,7 +51,7 @@ func NewListCommand() *cobra.Command { }, }) if err != nil { - log.Fatal(err) + return err } switch listArgs.output { case "", "wide": @@ -58,8 +61,9 @@ func NewListCommand() *cobra.Command { fmt.Println(wftmp.ObjectMeta.Name) } default: - log.Fatalf("Unknown output mode: %s", listArgs.output) + return fmt.Errorf("Unknown output mode: %s", listArgs.output) } + return nil }, } command.Flags().BoolVarP(&listArgs.allNamespaces, "all-namespaces", "A", false, "Show workflows from all namespaces") diff --git a/cmd/argo/commands/template/root.go b/cmd/argo/commands/template/root.go index 2706a3b867fa..7f334602060e 100644 --- a/cmd/argo/commands/template/root.go +++ b/cmd/argo/commands/template/root.go @@ -8,8 +8,8 @@ func NewTemplateCommand() *cobra.Command { command := &cobra.Command{ Use: "template", Short: "manipulate workflow templates", - Run: func(cmd *cobra.Command, args []string) { - cmd.HelpFunc()(cmd, args) + RunE: func(cmd *cobra.Command, args []string) error { + return cmd.Help() }, } diff --git a/cmd/argo/commands/template/update.go b/cmd/argo/commands/template/update.go index 21db4445fc11..95e5c7617786 100644 --- a/cmd/argo/commands/template/update.go +++ b/cmd/argo/commands/template/update.go @@ -2,8 +2,7 @@ package template import ( "context" - "log" - "os" + "fmt" "github.com/spf13/cobra" @@ -30,13 +29,9 @@ func NewUpdateCommand() *cobra.Command { # Update a Workflow Template with relaxed validation: argo template update FILE1 --strict false `, - Run: func(cmd *cobra.Command, args []string) { - if len(args) == 0 { - cmd.HelpFunc()(cmd, args) - os.Exit(1) - } - - updateWorkflowTemplates(cmd.Context(), args, &cliUpdateOpts) + Args: cobra.MinimumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + return updateWorkflowTemplates(cmd.Context(), args, &cliUpdateOpts) }, } command.Flags().StringVarP(&cliUpdateOpts.output, "output", "o", "", "Output format. One of: name|json|yaml|wide") @@ -44,14 +39,17 @@ func NewUpdateCommand() *cobra.Command { return command } -func updateWorkflowTemplates(ctx context.Context, filePaths []string, cliOpts *cliUpdateOpts) { +func updateWorkflowTemplates(ctx context.Context, filePaths []string, cliOpts *cliUpdateOpts) error { if cliOpts == nil { cliOpts = &cliUpdateOpts{} } - ctx, apiClient := client.NewAPIClient(ctx) + ctx, apiClient, err := client.NewAPIClient(ctx) + if err != nil { + return err + } serviceClient, err := apiClient.NewWorkflowTemplateServiceClient() if err != nil { - log.Fatal(err) + return err } workflowTemplates := generateWorkflowTemplates(filePaths, cliOpts.strict) @@ -65,7 +63,7 @@ func updateWorkflowTemplates(ctx context.Context, filePaths []string, cliOpts *c Namespace: wftmpl.Namespace, }) if err != nil { - log.Fatalf("Failed to get existing workflow template %q to update: %v", wftmpl.Name, err) + return fmt.Errorf("Failed to get existing workflow template %q to update: %v", wftmpl.Name, err) } wftmpl.ResourceVersion = current.ResourceVersion updated, err := serviceClient.UpdateWorkflowTemplate(ctx, &workflowtemplatepkg.WorkflowTemplateUpdateRequest{ @@ -73,8 +71,9 @@ func updateWorkflowTemplates(ctx context.Context, filePaths []string, cliOpts *c Template: &wftmpl, }) if err != nil { - log.Fatalf("Failed to update workflow template: %v", err) + return fmt.Errorf("Failed to update workflow template: %v", err) } printWorkflowTemplate(updated, cliOpts.output) } + return nil } diff --git a/cmd/argo/commands/terminate.go b/cmd/argo/commands/terminate.go index c10e669bbae2..63ad051b60b9 100644 --- a/cmd/argo/commands/terminate.go +++ b/cmd/argo/commands/terminate.go @@ -1,10 +1,9 @@ package commands import ( + "errors" "fmt" - "os" - "github.com/argoproj/pkg/errors" "github.com/spf13/cobra" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -63,13 +62,17 @@ func NewTerminateCommand() *cobra.Command { argo terminate --field-selector metadata.namespace=argo `, - Run: func(cmd *cobra.Command, args []string) { + Args: func(cmd *cobra.Command, args []string) error { if len(args) == 0 && !t.isList() { - cmd.HelpFunc()(cmd, args) - os.Exit(1) + return errors.New("requires either selector or workflow") + } + return nil + }, + RunE: func(cmd *cobra.Command, args []string) error { + ctx, apiClient, err := client.NewAPIClient(cmd.Context()) + if err != nil { + return err } - - ctx, apiClient := client.NewAPIClient(cmd.Context()) serviceClient := apiClient.NewWorkflowServiceClient() t.namespace = client.Namespace() @@ -81,7 +84,9 @@ func NewTerminateCommand() *cobra.Command { fields: t.fields, labels: t.labels, }) - errors.CheckError(err) + if err != nil { + return err + } workflows = append(workflows, listed...) } else { workflows = t.convertToWorkflows(args) @@ -97,9 +102,12 @@ func NewTerminateCommand() *cobra.Command { Name: w.Name, Namespace: w.Namespace, }) - errors.CheckError(err) + if err != nil { + return err + } fmt.Printf("workflow %s terminated\n", wf.Name) } + return nil }, } diff --git a/cmd/argo/commands/version.go b/cmd/argo/commands/version.go index 9e923bf66b38..e40dd528cc24 100644 --- a/cmd/argo/commands/version.go +++ b/cmd/argo/commands/version.go @@ -3,7 +3,6 @@ package commands import ( "os" - "github.com/argoproj/pkg/errors" "github.com/spf13/cobra" "github.com/argoproj/argo-workflows/v3" @@ -18,16 +17,24 @@ func NewVersionCommand() *cobra.Command { cmd := cobra.Command{ Use: "version", Short: "print version information", - Run: func(cmd *cobra.Command, args []string) { + RunE: func(cmd *cobra.Command, args []string) error { cmdutil.PrintVersion(CLIName, argo.GetVersion(), short) if _, ok := os.LookupEnv("ARGO_SERVER"); ok { - ctx, apiClient := client.NewAPIClient(cmd.Context()) + ctx, apiClient, err := client.NewAPIClient(cmd.Context()) + if err != nil { + return err + } serviceClient, err := apiClient.NewInfoServiceClient() - errors.CheckError(err) + if err != nil { + return err + } serverVersion, err := serviceClient.GetVersion(ctx, &infopkg.GetVersionRequest{}) - errors.CheckError(err) + if err != nil { + return err + } cmdutil.PrintVersion("argo-server", *serverVersion, short) } + return nil }, } cmd.Flags().BoolVar(&short, "short", false, "print just the version number") diff --git a/cmd/argo/commands/wait.go b/cmd/argo/commands/wait.go index 64238918ab8a..e1bc762346b6 100644 --- a/cmd/argo/commands/wait.go +++ b/cmd/argo/commands/wait.go @@ -20,11 +20,15 @@ func NewWaitCommand() *cobra.Command { argo wait @latest `, - Run: func(cmd *cobra.Command, args []string) { - ctx, apiClient := client.NewAPIClient(cmd.Context()) + RunE: func(cmd *cobra.Command, args []string) error { + ctx, apiClient, err := client.NewAPIClient(cmd.Context()) + if err != nil { + return err + } serviceClient := apiClient.NewWorkflowServiceClient() namespace := client.Namespace() common.WaitWorkflows(ctx, serviceClient, namespace, args, ignoreNotFound, false) + return nil }, } command.Flags().BoolVar(&ignoreNotFound, "ignore-not-found", false, "Ignore the wait if the workflow is not found") diff --git a/cmd/argo/commands/watch.go b/cmd/argo/commands/watch.go index 488c3f57530d..23ef1411412c 100644 --- a/cmd/argo/commands/watch.go +++ b/cmd/argo/commands/watch.go @@ -1,8 +1,6 @@ package commands import ( - "os" - "github.com/spf13/cobra" "github.com/argoproj/argo-workflows/v3/cmd/argo/commands/client" @@ -23,15 +21,15 @@ func NewWatchCommand() *cobra.Command { argo watch @latest `, - Run: func(cmd *cobra.Command, args []string) { - if len(args) != 1 { - cmd.HelpFunc()(cmd, args) - os.Exit(1) + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + ctx, apiClient, err := client.NewAPIClient(cmd.Context()) + if err != nil { + return err } - ctx, apiClient := client.NewAPIClient(cmd.Context()) serviceClient := apiClient.NewWorkflowServiceClient() namespace := client.Namespace() - common.WatchWorkflow(ctx, serviceClient, namespace, args[0], getArgs) + return common.WatchWorkflow(ctx, serviceClient, namespace, args[0], getArgs) }, } command.Flags().StringVar(&getArgs.Status, "status", "", "Filter by status (Pending, Running, Succeeded, Skipped, Failed, Error)") diff --git a/cmd/argo/lint/lint.go b/cmd/argo/lint/lint.go index 8a749ce6161d..dc7c9c8d9d32 100644 --- a/cmd/argo/lint/lint.go +++ b/cmd/argo/lint/lint.go @@ -6,7 +6,6 @@ import ( "io" "strings" - "github.com/argoproj/pkg/errors" log "github.com/sirupsen/logrus" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -80,19 +79,26 @@ func GetFormatter(fmtr string) (Formatter, error) { // RunLint lints the specified kinds in the specified files and prints the results to os.Stdout. // If linting fails it will exit with status code 1. -func RunLint(ctx context.Context, client apiclient.Client, kinds []string, output string, offline bool, opts LintOptions) { +func RunLint(ctx context.Context, client apiclient.Client, kinds []string, output string, offline bool, opts LintOptions) error { fmtr, err := GetFormatter(output) - errors.CheckError(err) + if err != nil { + return err + } clients, err := getLintClients(client, kinds) - errors.CheckError(err) + if err != nil { + return err + } opts.ServiceClients = clients opts.Formatter = fmtr res, err := Lint(ctx, &opts) - errors.CheckError(err) + if err != nil { + return err + } if !res.Success { log.StandardLogger().Exit(1) } + return nil } // Lint reads all files, returns linting errors of all of the enitities of the specified kinds. diff --git a/cmd/argo/main.go b/cmd/argo/main.go index 81fc7721a211..22e33349fc87 100644 --- a/cmd/argo/main.go +++ b/cmd/argo/main.go @@ -1,7 +1,6 @@ package main import ( - "fmt" "os" // load authentication plugin for obtaining credentials from cloud providers. @@ -12,7 +11,6 @@ import ( func main() { if err := commands.NewCommand().Execute(); err != nil { - fmt.Println(err) os.Exit(1) } } diff --git a/cmd/argoexec/commands/agent.go b/cmd/argoexec/commands/agent.go index 4b796e91dbf1..88f970f741a7 100644 --- a/cmd/argoexec/commands/agent.go +++ b/cmd/argoexec/commands/agent.go @@ -34,20 +34,21 @@ func NewAgentCommand() *cobra.Command { func NewAgentInitCommand() *cobra.Command { return &cobra.Command{ Use: "init", - Run: func(cmd *cobra.Command, args []string) { + RunE: func(cmd *cobra.Command, args []string) error { for _, name := range getPluginNames() { filename := tokenFilename(name) log.WithField("plugin", name). WithField("filename", filename). Info("creating token file for plugin") if err := os.Mkdir(filepath.Dir(filename), 0o770); err != nil { - log.Fatal(err) + return err } token := rand.String(32) // this could have 26^32 ~= 2 x 10^45 possible values, not guessable in reasonable time if err := os.WriteFile(filename, []byte(token), 0o440); err != nil { - log.Fatal(err) + return err } } + return nil }, } } diff --git a/cmd/argoexec/commands/data.go b/cmd/argoexec/commands/data.go index a01bb70cd173..6e458ba9cfba 100644 --- a/cmd/argoexec/commands/data.go +++ b/cmd/argoexec/commands/data.go @@ -2,8 +2,8 @@ package commands import ( "context" + "fmt" - log "github.com/sirupsen/logrus" "github.com/spf13/cobra" ) @@ -11,12 +11,13 @@ func NewDataCommand() *cobra.Command { command := cobra.Command{ Use: "data", Short: "Process data", - Run: func(cmd *cobra.Command, args []string) { + RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() err := execData(ctx) if err != nil { - log.Fatalf("%+v", err) + return fmt.Errorf("%+v", err) } + return nil }, } return &command diff --git a/cmd/argoexec/commands/init.go b/cmd/argoexec/commands/init.go index 13d642ae2353..c7b9b1f99d7d 100644 --- a/cmd/argoexec/commands/init.go +++ b/cmd/argoexec/commands/init.go @@ -2,9 +2,9 @@ package commands import ( "context" + "fmt" "github.com/argoproj/pkg/stats" - log "github.com/sirupsen/logrus" "github.com/spf13/cobra" ) @@ -12,12 +12,13 @@ func NewInitCommand() *cobra.Command { command := cobra.Command{ Use: "init", Short: "Load artifacts", - Run: func(cmd *cobra.Command, args []string) { + RunE: func(cmd *cobra.Command, args []string) error { ctx := context.Background() err := loadArtifacts(ctx) if err != nil { - log.Fatalf("%+v", err) + return fmt.Errorf("%+v", err) } + return nil }, } return &command diff --git a/cmd/argoexec/commands/resource.go b/cmd/argoexec/commands/resource.go index 68b0583b6378..6bff68bf4bcc 100644 --- a/cmd/argoexec/commands/resource.go +++ b/cmd/argoexec/commands/resource.go @@ -3,9 +3,7 @@ package commands import ( "context" "fmt" - "os" - log "github.com/sirupsen/logrus" "github.com/spf13/cobra" "github.com/argoproj/argo-workflows/v3/workflow/common" @@ -15,17 +13,14 @@ func NewResourceCommand() *cobra.Command { command := cobra.Command{ Use: "resource (get|create|apply|delete) MANIFEST", Short: "update a resource and wait for resource conditions", - Run: func(cmd *cobra.Command, args []string) { - if len(args) != 1 { - cmd.HelpFunc()(cmd, args) - os.Exit(1) - } - + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() err := execResource(ctx, args[0]) if err != nil { - log.Fatalf("%+v", err) + return fmt.Errorf("%+v", err) } + return nil }, } return &command diff --git a/cmd/argoexec/commands/root.go b/cmd/argoexec/commands/root.go index 168d1cbc9d43..824734752c40 100644 --- a/cmd/argoexec/commands/root.go +++ b/cmd/argoexec/commands/root.go @@ -50,8 +50,8 @@ func NewRootCommand() *cobra.Command { command := cobra.Command{ Use: CLIName, Short: "argoexec is the executor sidecar to workflow containers", - Run: func(cmd *cobra.Command, args []string) { - cmd.HelpFunc()(cmd, args) + RunE: func(cmd *cobra.Command, args []string) error { + return cmd.Help() }, PersistentPreRun: func(cmd *cobra.Command, args []string) { initConfig() diff --git a/cmd/argoexec/commands/wait.go b/cmd/argoexec/commands/wait.go index 76896ea87a5b..2087673621c9 100644 --- a/cmd/argoexec/commands/wait.go +++ b/cmd/argoexec/commands/wait.go @@ -2,10 +2,10 @@ package commands import ( "context" + "fmt" "time" "github.com/argoproj/pkg/stats" - log "github.com/sirupsen/logrus" "github.com/spf13/cobra" ) @@ -13,12 +13,13 @@ func NewWaitCommand() *cobra.Command { command := cobra.Command{ Use: "wait", Short: "wait for main container to finish and save artifacts", - Run: func(cmd *cobra.Command, args []string) { + RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() err := waitContainer(ctx) if err != nil { - log.Fatalf("%+v", err) + return fmt.Errorf("%+v", err) } + return nil }, } return &command diff --git a/cmd/workflow-controller/main.go b/cmd/workflow-controller/main.go index a5edb978f374..7bf2d0984fc7 100644 --- a/cmd/workflow-controller/main.go +++ b/cmd/workflow-controller/main.go @@ -9,7 +9,6 @@ import ( "time" "github.com/argoproj/pkg/cli" - "github.com/argoproj/pkg/errors" kubecli "github.com/argoproj/pkg/kube/cli" "github.com/argoproj/pkg/stats" log "github.com/sirupsen/logrus" @@ -112,7 +111,9 @@ func NewRootCommand() *cobra.Command { } wfController, err := controller.NewWorkflowController(ctx, config, kubeclientset, wfclientset, namespace, managedNamespace, executorImage, executorImagePullPolicy, logFormat, configMap, executorPlugins) - errors.CheckError(err) + if err != nil { + return err + } leaderElectionOff := os.Getenv("LEADER_ELECTION_DISABLE") if leaderElectionOff == "true" { diff --git a/test/e2e/cli_test.go b/test/e2e/cli_test.go index b32ab06176c7..067a85af4d14 100644 --- a/test/e2e/cli_test.go +++ b/test/e2e/cli_test.go @@ -1462,7 +1462,7 @@ func (s *CLISuite) TestCronCommands() { s.Run("Get", func() { s.Given().RunCli([]string{"cron", "get", "not-found"}, func(t *testing.T, output string, err error) { require.EqualError(t, err, "exit status 1") - assert.Contains(t, output, `\"not-found\" not found`) + assert.Contains(t, output, `"not-found" not found`) }).RunCli([]string{"cron", "get", "test-cron-wf-basic"}, func(t *testing.T, output string, err error) { require.NoError(t, err) assert.Contains(t, output, "Name:") diff --git a/util/cmd/cmd.go b/util/cmd/cmd.go index b7e0bffc6884..82032528a3f3 100644 --- a/util/cmd/cmd.go +++ b/util/cmd/cmd.go @@ -20,9 +20,10 @@ func NewVersionCmd(cliName string) *cobra.Command { versionCmd := cobra.Command{ Use: "version", Short: "Print version information", - Run: func(cmd *cobra.Command, args []string) { + RunE: func(cmd *cobra.Command, args []string) error { version := argo.GetVersion() PrintVersion(cliName, version, short) + return nil }, } versionCmd.Flags().BoolVar(&short, "short", false, "print just the version number")