Reimplemented a common delete tink-cli command

client/main.go

@@ -25,7 +25,7 @@ var (
 	EventsClient   events.EventsServiceClient
 )
 
-// FullClient aggregates all the grpc clients available from Tinkerbell Server
+// FullClient aggregates all the gRPC clients available from Tinkerbell Server
 type FullClient struct {
 	TemplateClient template.TemplateServiceClient
 	WorkflowClient workflow.WorkflowServiceClient
@@ -35,8 +35,8 @@ type FullClient struct {
 
 // NewFullClientFromGlobal is a dirty hack that returns a FullClient using the
 // global variables exposed by the client package. Globals should be avoided
-// and we will deprecated them at some point replacing this function with
-// NewFullClient. If you are strating a new project please use the last one
+// and we will deprecate them at some point replacing this function with
+// NewFullClient. If you are starting a new project please use NewFullClient instead.
 func NewFullClientFromGlobal() (*FullClient, error) {
 	// This is required because we use init() too often, even more in the
 	// CLI and based on where you are sometime the clients are not initialised

cmd/tink-cli/cmd/delete/delete.go

@@ -0,0 +1,123 @@
+package delete
+
+import (
+	"context"
+	"fmt"
+
+	"github.com/pkg/errors"
+	"github.com/spf13/cobra"
+	"github.com/tinkerbell/tink/client"
+	"google.golang.org/grpc"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
+)
+
+type Options struct {
+	// DeleteByID is used to delete a resource
+	DeleteByID func(context.Context, *client.FullClient, string) (interface{}, error)
+
+	clientConnOpt *client.ConnOptions
+	fullClient    *client.FullClient
+}
+
+func (o *Options) SetClientConnOpt(co *client.ConnOptions) {
+	o.clientConnOpt = co
+}
+
+func (o *Options) SetFullClient(cl *client.FullClient) {
+	o.fullClient = cl
+}
+
+const shortDescr = "delete one or more resources"
+
+const longDescr = `Deletes one or more resources and prints the status of
+the deleted resource.
+
+# Delete template resource (success)
+tink template delete 8ae1cc24-6a9c-11eb-a0fc-0242ac120005
+Deleted	8ae1cc24-6a9c-11eb-a0fc-0242ac120005
+
+# Delete template resource (not found)
+tink template delete 8ae1cc24-6a9c-11eb-a0fc-0242ac120005
+Error	8ae1cc24-6a9c-11eb-a0fc-0242ac120005	not found
+
+# Delete template resources (one not found)
+tink template delete 8ae1cc24-6a9c-11eb-a0fc-0242ac120005 e4115856-4358-429d-a8f6-9e1b7d794b72
+Deleted	8ae1cc24-6a9c-11eb-a0fc-0242ac120005
+Error	e4115856-4358-429d-a8f6-9e1b7d794b72	not found
+
+# Delete resources and extract resource ID with awk
+tink template delete 8ae1cc24-6a9c-11eb-a0fc-0242ac120005 e4115856-4358-429d-a8f6-9e1b7d794b72 | awk {print $2} > result
+cat result
+8ae1cc24-6a9c-11eb-a0fc-0242ac120005
+e4115856-4358-429d-a8f6-9e1b7d794b72
+`
+
+const exampleDescr = `# Delete template resource
+tink template delete [id]
+
+# Delete hardware resource
+tink hardware delete [id]
+
+# Delete workflow resource
+tink workflow delete [id]
+
+# Delete multiple workflow resources
+tink workflow delete [id_1] [id_2] [id_3]
+`
+
+func NewDeleteCommand(opt Options) *cobra.Command {
+	cmd := &cobra.Command{
+		Use:                   "delete",
+		Short:                 shortDescr,
+		Long:                  longDescr,
+		Example:               exampleDescr,
+		DisableFlagsInUseLine: true,
+		PersistentPreRunE: func(cmd *cobra.Command, args []string) error {
+			if opt.fullClient != nil {
+				return nil
+			}
+			if opt.clientConnOpt == nil {
+				opt.SetClientConnOpt(&client.ConnOptions{})
+			}
+			opt.clientConnOpt.SetFlags(cmd.PersistentFlags())
+			return nil
+		},
+		PreRunE: func(cmd *cobra.Command, args []string) error {
+			if opt.fullClient == nil {
+				var err error
+				var conn *grpc.ClientConn
+				conn, err = client.NewClientConn(opt.clientConnOpt)
+				if err != nil {
+					println("Flag based client configuration failed with err: %s. Trying with env var legacy method...", err)
+					// Fallback to legacy Setup via env var
+					conn, err = client.GetConnection()
+					if err != nil {
+						return errors.Wrap(err, "failed to setup connection to tink-server")
+					}
+				}
+				opt.SetFullClient(client.NewFullClient(conn))
+			}
+			return nil
+		},
+		RunE: func(cmd *cobra.Command, args []string) error {
+			if opt.DeleteByID == nil {
+				return errors.New("DeleteByID is not implemented for this resource yet. Please have a look at the issue in GitHub or open a new one.")
+			}
+			for _, requestedID := range args {
+				_, err := opt.DeleteByID(cmd.Context(), opt.fullClient, requestedID)
+				if err != nil {
+					if s, ok := status.FromError(err); ok && s.Code() == codes.NotFound {
+						fmt.Fprintf(cmd.ErrOrStderr(), "Error\t%s\tnot found\n", requestedID)
+						continue
+					} else {
+						return err
+					}
+				}
+				fmt.Fprintf(cmd.OutOrStdout(), "Deleted\t%s\n", requestedID)
+			}
+			return nil
+		},
+	}
+	return cmd
+}

cmd/tink-cli/cmd/delete/delete_test.go

@@ -0,0 +1,98 @@
+package delete
+
+import (
+	"bytes"
+	"context"
+	"io/ioutil"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/tinkerbell/tink/client"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
+)
+
+func TestNewDeleteCommand(t *testing.T) {
+	table := []struct {
+		name           string
+		expectedOutput string
+		args           []string
+		opt            Options
+	}{
+		{
+			name:           "happy-path",
+			expectedOutput: "Deleted\tbeeb5c79\n",
+			args:           []string{"beeb5c79"},
+			opt: Options{
+				DeleteByID: func(c context.Context, fc *client.FullClient, s string) (interface{}, error) {
+					return struct{}{}, nil
+				},
+			},
+		},
+		{
+			name:           "happy-path-multiple-resources",
+			expectedOutput: "Deleted\tbeeb5c79\nDeleted\t14810952\nDeleted\te7a91fe9\n",
+			args:           []string{"beeb5c79", "14810952", "e7a91fe9"},
+			opt: Options{
+				DeleteByID: func(c context.Context, fc *client.FullClient, s string) (interface{}, error) {
+					return struct{}{}, nil
+				},
+			},
+		},
+		{
+			name:           "resource-not-found",
+			expectedOutput: "Error\tbeeb5c79\tnot found\n",
+			args:           []string{"beeb5c79"},
+			opt: Options{
+				DeleteByID: func(c context.Context, fc *client.FullClient, s string) (interface{}, error) {
+					return struct{}{}, status.Error(codes.NotFound, "")
+				},
+			},
+		},
+		{
+			name:           "multiple-resources-not-found",
+			expectedOutput: "Error\tbeeb5c79\tnot found\nError\t14810952\tnot found\n",
+			args:           []string{"beeb5c79", "14810952"},
+			opt: Options{
+				DeleteByID: func(c context.Context, fc *client.FullClient, s string) (interface{}, error) {
+					return struct{}{}, status.Error(codes.NotFound, "")
+				},
+			},
+		},
+		{
+			name:           "only-one-resource-of-two-was-deleted",
+			expectedOutput: "Deleted\tbeeb5c79\nError\t14810952\tnot found\n",
+			args:           []string{"beeb5c79", "14810952"},
+			opt: Options{
+				DeleteByID: func(c context.Context, fc *client.FullClient, s string) (interface{}, error) {
+					if s == "beeb5c79" {
+						return struct{}{}, nil
+					}
+					return struct{}{}, status.Error(codes.NotFound, "")
+				},
+			},
+		},
+	}
+
+	for _, test := range table {
+		t.Run(test.name, func(t *testing.T) {
+			stdout := bytes.NewBufferString("")
+			test.opt.SetFullClient(&client.FullClient{})
+			cmd := NewDeleteCommand(test.opt)
+			cmd.SetOut(stdout)
+			cmd.SetErr(stdout)
+			cmd.SetArgs(test.args)
+			err := cmd.Execute()
+			if err != nil {
+				t.Error(err)
+			}
+			out, err := ioutil.ReadAll(stdout)
+			if err != nil {
+				t.Error(err)
+			}
+			if diff := cmp.Diff(string(out), test.expectedOutput); diff != "" {
+				t.Fatal(diff)
+			}
+		})
+	}
+}

cmd/tink-cli/cmd/delete/doc.go

@@ -0,0 +1,4 @@
+// Package delete provides a reusable implementation of the Delete command
+// for the tink cli. The Delete command deletes resources. It is designed
+// to be extendible and usable across resources.
+package delete

cmd/tink-cli/cmd/get/get_test.go

@@ -124,7 +124,7 @@ func TestNewGetCommand(t *testing.T) {
 		},
 		{
 			Name: "happy-path-json-no-headers",
-			Skip: "The JSON format is rusty and custom because we table library we use do not support JSON right now. This feature is not implemented",
+			Skip: "The JSON format is rusty and custom because the table library we use does not support JSON right now. This feature is not implemented.",
 		},
 		{
 			Name: "happy-path-csv-no-headers",
@@ -198,5 +198,4 @@ func TestNewGetCommand(t *testing.T) {
 			}
 		})
 	}
-
 }

cmd/tink-cli/cmd/hardware.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 
 	"github.com/spf13/cobra"
+	"github.com/tinkerbell/tink/cmd/tink-cli/cmd/delete"
 	"github.com/tinkerbell/tink/cmd/tink-cli/cmd/get"
 	"github.com/tinkerbell/tink/cmd/tink-cli/cmd/hardware"
 )
@@ -22,7 +23,7 @@ func NewHardwareCommand() *cobra.Command {
 	}
 
 	cmd.AddCommand(get.NewGetCommand(hardware.NewGetOptions()))
-	cmd.AddCommand(hardware.NewDeleteCmd())
+	cmd.AddCommand(delete.NewDeleteCommand(hardware.NewDeleteOptions()))
 	cmd.AddCommand(hardware.NewGetByIDCmd())
 	cmd.AddCommand(hardware.NewGetByIPCmd())
 	cmd.AddCommand(hardware.NewListCmd())

cmd/tink-cli/cmd/hardware/delete.go

@@ -4,30 +4,23 @@ package hardware
 
 import (
 	"context"
-	"log"
 
-	"github.com/spf13/cobra"
 	"github.com/tinkerbell/tink/client"
+	"github.com/tinkerbell/tink/cmd/tink-cli/cmd/delete"
 	"github.com/tinkerbell/tink/protos/hardware"
 )
 
-func NewDeleteCmd() *cobra.Command {
-	return &cobra.Command{
-		Use:     "delete",
-		Short:   "delete hardware by id",
-		Example: "tink hardware delete 224ee6ab-ad62-4070-a900-ed816444cec0 cb76ae54-93e9-401c-a5b2-d455bb3800b1",
-		Args: func(_ *cobra.Command, args []string) error {
-			return verifyUUIDs(args)
-		},
-		Run: func(cmd *cobra.Command, args []string) {
-			for _, id := range args {
-				_, err := client.HardwareClient.Delete(context.Background(), &hardware.DeleteRequest{Id: id})
-				if err != nil {
-					log.Fatal(err)
-				}
-				log.Println("Hardware data with id", id, "deleted successfully")
-			}
-		},
-	}
+type deleteHardware struct {
+	delete.Options
+}
 
+func (h *deleteHardware) DeleteByID(ctx context.Context, cl *client.FullClient, requestedID string) (interface{}, error) {
+	return cl.HardwareClient.Delete(ctx, &hardware.DeleteRequest{Id: requestedID})
+}
+
+func NewDeleteOptions() delete.Options {
+	h := deleteHardware{}
+	return delete.Options{
+		DeleteByID: h.DeleteByID,
+	}
 }

cmd/tink-cli/cmd/hardware_test.go

@@ -2,6 +2,7 @@ package cmd
 
 import (
 	"bytes"
+	"fmt"
 	"strings"
 	"testing"
 
@@ -112,8 +113,9 @@ func Test_NewHardwareCommand(t *testing.T) {
 				if err := root.Execute(); err != nil {
 					t.Error(err)
 				}
-				if !strings.Contains(out.String(), "delete hardware by id") {
-					t.Error("expected output should include delete hardware by id")
+				want := "Deletes one or more resources"
+				if !strings.Contains(out.String(), want) {
+					t.Error(fmt.Errorf("unexpected output, looking for %q as a substring in %q", want, out.String()))
 				}
 			},
 		},

cmd/tink-cli/cmd/template.go

@@ -5,6 +5,7 @@ import (
 	"os"
 
 	"github.com/spf13/cobra"
+	"github.com/tinkerbell/tink/cmd/tink-cli/cmd/delete"
 	"github.com/tinkerbell/tink/cmd/tink-cli/cmd/get"
 	"github.com/tinkerbell/tink/cmd/tink-cli/cmd/template"
 )
@@ -23,12 +24,12 @@ func NewTemplateCommand() *cobra.Command {
 	}
 
 	cmd.AddCommand(template.NewCreateCommand())
-	cmd.AddCommand(template.NewDeleteCommand())
+	cmd.AddCommand(delete.NewDeleteCommand(template.NewDeleteOptions()))
 	cmd.AddCommand(template.NewListCommand())
 	cmd.AddCommand(template.NewUpdateCommand())
 
-	// If the variable TINK_CLI_VERSION is not set to 0.0.0 use the old get
-	// command. This is a way to keep retro-compatibility with the old get command.
+	// If the variable TINK_CLI_VERSION is set to 0.0.0 use the old get command.
+	// This is a way to keep retro-compatibility with the old get command.
 	getCmd := template.GetCmd
 	if v := os.Getenv("TINK_CLI_VERSION"); v != "0.0.0" {
 		getCmd = get.NewGetCommand(template.NewGetOptions())