Add machinery to make errors human-readable (#131)

We face four different kinds of errors in the k8s-snap.
This commit adds machinery to handle all of them:

On the server-side there are:

1. microcluster errors: errors that directly come from microcluster. They are
   untyped and we need to rely on the error string to handle them.

2. server-side errors: error that happen in our system and that we can
   formally define.

The errors are not preserved on the client. Microcluster simply sends
the error message and creates a new error on the client.
This means we cannot rely on error types by default.
But we can define custom errors in `api/v1/errors.go` that support a direct
string comparison by implementing the `Error` and `Is` interfaces.

For server-side errors we can simply use the exposed error types and the
string comparison will always work. The error is basically just
serialized on the server and deserialized on the client.

For the microcluster errors we need to be careful on library upgrades.
The error messages are not recognized anymore, if microcluster changes the error messages.
We can avoid this by adding strict integration tests that validate the
explicit CLI output.

On the client-side there are:

3. generic client-side errors: errors that pop up on the client/CLI,
   e.g. cannot connect to k8sd and apply for all commands.

4. custom client-side errors: errors that are unique for a specific
   command, e.g. ErrAlreadyBootstrapped.

This commit generically resolve those error by defining an error wrapper
(`cmd/k8s/errors/wrapper.go`) that is injected at the root `k8s` CLI command and
tries to resolve any error to a human-readable message.
Generic errors translations are defined in the wrapper module, custom
error messages can be defined in the command (see e.g. `k8s_bootstrap.go`)

Also, puts each CLI command into a function that expects a k8s client.
Provides a mock implementation of the client so that unit tests for the
CLI can be implemented.

src/k8s/api/v1/errors.go

@@ -0,0 +1,25 @@
+package v1
+
+import "errors"
+
+// ErrNotBootstrapped indicates that the cluster is not yet bootstrapped.
+var ErrNotBootstrapped = errors.New("daemon not yet initialized")
+
+// ErrAlreadyBootstrapped indicates that there is already a cluster bootstrapped on this node.
+var ErrAlreadyBootstrapped = errors.New("cluster already bootstrapped")
+
+// ErrInvalidJoinToken indicates that a node tried to join the cluster with an invalid token.
+var ErrInvalidJoinToken = errors.New("failed to join cluster with the given join token")
+
+// ErrTokenAlreadyCreated indicates that a token for this node was already created.
+// TODO: Instead, return the already existing token.
+var ErrTokenAlreadyCreated = errors.New("UNIQUE constraint failed: internal_token_records.name")
+
+// ErrTimeout indicates that the action on the server took too long.
+var ErrTimeout = errors.New("context deadline exceeded")
+
+// ErrConnectionFailed indicates that a connection to the k8sd daemon could not be established.
+var ErrConnectionFailed = errors.New("dial unix")
+
+// ErrUnknown indicates that the server returns an unknown error.
+var ErrUnknown = errors.New("unknown error")

src/k8s/cmd/k8s/errors/errors.go

@@ -0,0 +1,53 @@
+package errors
+
+import (
+	"errors"
+	"fmt"
+	"strings"
+
+	v1 "github.com/canonical/k8s/api/v1"
+)
+
+var genericErrorMsgs = map[error]string{
+	v1.ErrNotBootstrapped: fmt.Sprintln("The cluster has not been initialized yet. Please call:\n\n    sudo k8s bootstrap"),
+	v1.ErrConnectionFailed: "Unable to connect to the cluster. Verify that that the services are running:\n\n sudo snap services k8s\n\n" +
+		"and see logs for more details:\n\n sudo journalctl -n 300 -u snap.k8s.k8sd\n\n",
+	v1.ErrTimeout: "Command timed out. See logs for more details:\n\n" +
+		" sudo journalctl -n 300 -u snap.k8s.k8sd\n\n" +
+		"You may increase the timeout with `--timeout 3m`.",
+}
+
+// Transform checks if the error returned by the server contains a known error message
+// and transforms it into an user-friendly error message. The error type is lost when sending it over the wire,
+// thus the error type cannot be checked.
+// Transform is intended to be used with `defer`, therefore changing error in place.
+func Transform(err *error, extraErrorMessages map[error]string) {
+	if *err == nil {
+		return
+	}
+
+	// Unknown error occured. Append the full error message to the result.
+	if strings.Contains(strings.ToLower((*err).Error()), strings.ToLower(v1.ErrUnknown.Error())) {
+		var prefix string
+		if msg, ok := extraErrorMessages[v1.ErrUnknown]; ok {
+			prefix = msg
+		} else {
+			prefix = genericErrorMsgs[v1.ErrUnknown]
+		}
+		*err = fmt.Errorf("%s%s", prefix, (*err).Error())
+		return
+	}
+
+	for errorType, msg := range extraErrorMessages {
+		if strings.Contains(strings.ToLower((*err).Error()), strings.ToLower(errorType.Error())) {
+			*err = errors.New(msg)
+			return
+		}
+	}
+
+	for errorType, msg := range genericErrorMsgs {
+		if strings.Contains(strings.ToLower((*err).Error()), strings.ToLower(errorType.Error())) {
+			*err = errors.New(msg)
+		}
+	}
+}

src/k8s/cmd/k8s/errors/errors_test.go

@@ -0,0 +1,66 @@
+package errors
+
+import (
+	"errors"
+	"fmt"
+	"testing"
+
+	v1 "github.com/canonical/k8s/api/v1"
+	. "github.com/onsi/gomega"
+)
+
+func TestResolve(t *testing.T) {
+	g := NewGomegaWithT(t)
+
+	mockErrorMsg := "unknown error: "
+	mockExtraErrorMsgs := map[error]string{
+		v1.ErrUnknown:         mockErrorMsg,
+		v1.ErrNotBootstrapped: "not bootstrapped",
+	}
+
+	testCases := []struct {
+		name        string
+		err         error
+		extraErrMsg map[error]string
+		expected    error
+	}{
+		{
+			name:     "NilError",
+			err:      nil,
+			expected: nil,
+		},
+		{
+			name:     "UnknownError",
+			err:      errors.New("unknown error"),
+			expected: fmt.Errorf("%s%s", genericErrorMsgs[v1.ErrUnknown], "unknown error"),
+		},
+		{
+			name:        "UnknownErrorWithCustomMsg",
+			err:         errors.New("unknown error"),
+			extraErrMsg: mockExtraErrorMsgs,
+			expected:    fmt.Errorf("%s%s", mockExtraErrorMsgs[v1.ErrUnknown], "unknown error"),
+		},
+		{
+			name:        "KnownErrorWithCustomMsg",
+			err:         errors.New(v1.ErrNotBootstrapped.Error()),
+			extraErrMsg: mockExtraErrorMsgs,
+			expected:    errors.New("not bootstrapped"),
+		},
+		{
+			name:     "UnknownErrorNoMatch",
+			err:      errors.New("some other error occurred"),
+			expected: errors.New("some other error occurred"),
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			Transform(&tc.err, tc.extraErrMsg)
+			if tc.expected == nil {
+				g.Expect(tc.err).To(BeNil())
+			} else {
+				g.Expect(tc.err).To(MatchError(tc.expected))
+			}
+		})
+	}
+}

src/k8s/cmd/k8s/hooks.go

@@ -0,0 +1,33 @@
+package k8s
+
+import (
+	"fmt"
+
+	"github.com/canonical/k8s/pkg/k8s/client"
+	"github.com/spf13/cobra"
+)
+
+func chainPreRunHooks(hooks ...func(*cobra.Command, []string) error) func(*cobra.Command, []string) error {
+	return func(cmd *cobra.Command, args []string) error {
+		for _, hook := range hooks {
+			err := hook(cmd, args)
+			if err != nil {
+				return err
+			}
+		}
+		return nil
+	}
+}
+
+func hookSetupClient(cmd *cobra.Command, args []string) error {
+	var err error
+	k8sdClient, err = client.NewClient(cmd.Context(), client.ClusterOpts{
+		StateDir: rootCmdOpts.stateDir,
+		Verbose:  rootCmdOpts.logVerbose,
+		Debug:    rootCmdOpts.logDebug,
+	})
+	if err != nil {
+		return fmt.Errorf("failed to create client: %w", err)
+	}
+	return nil
+}

src/k8s/cmd/k8s/k8s.go

@@ -3,6 +3,7 @@ package k8s
 import (
 	"fmt"
 
+	"github.com/canonical/k8s/pkg/k8s/client"
 	"github.com/canonical/k8s/pkg/utils"
 	"github.com/spf13/cobra"
 )
@@ -11,9 +12,13 @@ var (
 	rootCmdOpts struct {
 		logDebug   bool
 		logVerbose bool
+		stateDir   string
 	}
+	k8sdClient client.Client
+)
 
-	rootCmd = &cobra.Command{
+func NewRootCmd() *cobra.Command {
+	rootCmd := &cobra.Command{
 		Use:   "k8s",
 		Short: "Canonical Kubernetes CLI",
 		PersistentPreRunE: func(cmd *cobra.Command, args []string) error {
@@ -22,15 +27,42 @@ var (
 				return fmt.Errorf("failed to check if command runs as root: %w", err)
 			}
 			if !withRoot {
-				return fmt.Errorf("k8s CLI needs to run with root priviledge.")
+				return fmt.Errorf("You do not have enough permissions. Please run the command with sudo.")
 			}
 			return nil
 		},
-		SilenceUsage: true,
+		SilenceUsage:  true,
+		SilenceErrors: true,
 	}
-)
 
-func init() {
+	rootCmd.PersistentFlags().StringVar(&rootCmdOpts.stateDir, "state-dir", "", "Directory with the dqlite datastore")
 	rootCmd.PersistentFlags().BoolVarP(&rootCmdOpts.logDebug, "debug", "d", false, "Show all debug messages")
 	rootCmd.PersistentFlags().BoolVarP(&rootCmdOpts.logVerbose, "verbose", "v", true, "Show all information messages")
+
+	// By default, the state dir is set to a fixed directory in the snap.
+	// This shouldn't be overwritten by the user.
+	rootCmd.PersistentFlags().MarkHidden("state-dir")
+
+	// General
+	rootCmd.AddCommand(newStatusCmd())
+
+	// Clustering
+	rootCmd.AddCommand(newBootstrapCmd())
+	rootCmd.AddCommand(newAddNodeCmd())
+	rootCmd.AddCommand(newJoinNodeCmd())
+	rootCmd.AddCommand(newRemoveNodeCmd())
+
+	// Components
+	rootCmd.AddCommand(newEnableCmd())
+	rootCmd.AddCommand(newDisableCmd())
+
+	// internal
+	rootCmd.AddCommand(newGenerateAuthTokenCmd())
+	rootCmd.AddCommand(newKubeConfigCmd())
+
+	// Those commands replace the executable - no need for error wrapping.
+	rootCmd.AddCommand(newHelmCmd())
+	rootCmd.AddCommand(newKubectlCmd())
+
+	return rootCmd
 }

src/k8s/cmd/k8s/k8s_add_node.go

@@ -3,44 +3,49 @@ package k8s
 import (
 	"fmt"
 
-	"github.com/canonical/k8s/pkg/k8s/client"
+	apiv1 "github.com/canonical/k8s/api/v1"
+	"github.com/canonical/k8s/cmd/k8s/errors"
 	"github.com/spf13/cobra"
 )
 
 var (
 	addNodeCmdOpts struct {
 		worker bool
 	}
-	addNodeCmd = &cobra.Command{
-		Use:   "add-node <name>",
-		Short: "Create a connection token for a node to join the cluster",
-		Args:  cobra.ExactArgs(1),
-		RunE: func(cmd *cobra.Command, args []string) error {
-			name := args[0]
+	addNodeCmdErrorMsgs = map[error]string{
+		apiv1.ErrTokenAlreadyCreated: "A token for this node was already created and the node did not join.",
+	}
+)
 
-			c, err := client.NewClient(cmd.Context(), client.ClusterOpts{
-				StateDir: clusterCmdOpts.stateDir,
-				Verbose:  rootCmdOpts.logVerbose,
-				Debug:    rootCmdOpts.logDebug,
-			})
-			if err != nil {
-				return fmt.Errorf("failed to create client: %w", err)
+func newAddNodeCmd() *cobra.Command {
+	addNodeCmd := &cobra.Command{
+		Use:               "add-node <name>",
+		Short:             "Create a connection token for a node to join the cluster",
+		PersistentPreRunE: chainPreRunHooks(hookSetupClient),
+		RunE: func(cmd *cobra.Command, args []string) (err error) {
+			if len(args) > 1 {
+				return fmt.Errorf("Too many arguments. Please, only provide the node name to add.")
+			}
+			if len(args) < 1 {
+				return fmt.Errorf("Not enough arguments. Please, provide the node name to add.")
 			}
 
+			defer errors.Transform(&err, addNodeCmdErrorMsgs)
+			name := args[0]
+
 			// Create a token that will be used by the joining node to join the cluster.
-			token, err := c.CreateJoinToken(cmd.Context(), name, addNodeCmdOpts.worker)
+			token, err := k8sdClient.CreateJoinToken(cmd.Context(), name, addNodeCmdOpts.worker)
 			if err != nil {
 				return fmt.Errorf("failed to retrieve token: %w", err)
 			}
 
+			// TODO: Print guidance on what to do with the token.
+			//       This requires a --format flag first as we still need some machine readable output for the integration tests.
 			fmt.Println(token)
 			return nil
 		},
 	}
-)
 
-func init() {
 	addNodeCmd.Flags().BoolVar(&addNodeCmdOpts.worker, "worker", false, "generate a token for a worker node")
-
-	rootCmd.AddCommand(addNodeCmd)
+	return addNodeCmd
 }