EVEREST-1673 | [CLI] namespaces command improvements (#895)

Signed-off-by: Mayank Shah <[email protected]>

cli-tests/tests/flow/namespaces.spec.ts

@@ -89,7 +89,7 @@ test.describe('Everest CLI install', async () => {
         `add everest --operator.mongodb=false --operator.postgresql=false --operator.xtradb-cluster=true`,
       );
       await out.outErrContainsNormalizedMany([
-          '× namespace (everest) already exists',
+          '× invalid namespace (everest): namespace already exists. HINT: set \'--take-ownership\' flag to use existing namespaces',
       ]);
     });
     await page.waitForTimeout(10_000);

commands/namespaces/add.go

@@ -2,6 +2,7 @@
 package namespaces
 
 import (
+	"errors"
 	"fmt"
 	"os"
 
@@ -15,6 +16,9 @@ import (
 	"github.com/percona/everest/pkg/output"
 )
 
+//nolint:gochecknoglobals
+var takeOwnershipHintMessage = fmt.Sprintf("HINT: set '--%s' flag to use existing namespaces", cli.FlagTakeNamespaceOwnership)
+
 // NewAddCommand returns a new command to add a new namespace.
 func NewAddCommand(l *zap.SugaredLogger) *cobra.Command {
 	cmd := &cobra.Command{
@@ -45,7 +49,10 @@ func NewAddCommand(l *zap.SugaredLogger) *cobra.Command {
 				cmd.Flags().Lookup("operator.postgresql").Changed ||
 				cmd.Flags().Lookup("operator.xtradb-cluster").Changed)
 
-			if err := c.Populate(false, askOperators); err != nil {
+			if err := c.Populate(cmd.Context(), false, askOperators); err != nil {
+				if errors.Is(err, namespaces.ErrNamespaceAlreadyExists) {
+					err = fmt.Errorf("%w. %s", err, takeOwnershipHintMessage)
+				}
 				output.PrintError(err, l, !enableLogging)
 				os.Exit(1)
 			}
@@ -69,7 +76,7 @@ func initAddFlags(cmd *cobra.Command) {
 	cmd.Flags().Bool(cli.FlagDisableTelemetry, false, "Disable telemetry")
 	cmd.Flags().MarkHidden(cli.FlagDisableTelemetry) //nolint:errcheck,gosec
 	cmd.Flags().Bool(cli.FlagSkipWizard, false, "Skip installation wizard")
-	cmd.Flags().Bool("take-ownership", false, "Take ownership of existing namespaces")
+	cmd.Flags().Bool(cli.FlagTakeNamespaceOwnership, false, "If the specified namespace already exists, take ownership of it")
 
 	cmd.Flags().String(helm.FlagChartDir, "", "Path to the chart directory. If not set, the chart will be downloaded from the repository")
 	cmd.Flags().MarkHidden(helm.FlagChartDir) //nolint:errcheck,gosec
@@ -83,9 +90,9 @@ func initAddFlags(cmd *cobra.Command) {
 }
 
 func initAddViperFlags(cmd *cobra.Command) {
-	viper.BindPFlag(cli.FlagSkipWizard, cmd.Flags().Lookup(cli.FlagSkipWizard))             //nolint:errcheck,gosec
-	viper.BindPFlag(cli.FlagDisableTelemetry, cmd.Flags().Lookup(cli.FlagDisableTelemetry)) //nolint:errcheck,gosec
-	viper.BindPFlag("take-ownership", cmd.Flags().Lookup("take-ownership"))                 //nolint:errcheck,gosec
+	viper.BindPFlag(cli.FlagSkipWizard, cmd.Flags().Lookup(cli.FlagSkipWizard))                         //nolint:errcheck,gosec
+	viper.BindPFlag(cli.FlagDisableTelemetry, cmd.Flags().Lookup(cli.FlagDisableTelemetry))             //nolint:errcheck,gosec
+	viper.BindPFlag(cli.FlagTakeNamespaceOwnership, cmd.Flags().Lookup(cli.FlagTakeNamespaceOwnership)) //nolint:errcheck,gosec
 
 	viper.BindPFlag(helm.FlagChartDir, cmd.Flags().Lookup(helm.FlagChartDir))     //nolint:errcheck,gosec
 	viper.BindPFlag(helm.FlagRepository, cmd.Flags().Lookup(helm.FlagRepository)) //nolint:errcheck,gosec

commands/namespaces/remove.go

@@ -2,6 +2,7 @@
 package namespaces
 
 import (
+	"errors"
 	"fmt"
 	"os"
 
@@ -14,6 +15,8 @@ import (
 	"github.com/percona/everest/pkg/output"
 )
 
+const forceUninstallHint = "HINT: use --force to remove the namespace and all its resources"
+
 // NewRemoveCommand returns a new command to remove an existing namespace.
 func NewRemoveCommand(l *zap.SugaredLogger) *cobra.Command {
 	cmd := &cobra.Command{
@@ -48,6 +51,9 @@ func NewRemoveCommand(l *zap.SugaredLogger) *cobra.Command {
 			}
 
 			if err := op.Run(cmd.Context()); err != nil {
+				if errors.Is(err, namespaces.ErrNamespaceNotEmpty) {
+					err = fmt.Errorf("%w. %s", err, forceUninstallHint)
+				}
 				output.PrintError(err, l, !enableLogging)
 				os.Exit(1)
 			}

commands/namespaces/update.go

@@ -46,7 +46,7 @@ func NewUpdateCommand(l *zap.SugaredLogger) *cobra.Command {
 				cmd.Flags().Lookup("operator.postgresql").Changed ||
 				cmd.Flags().Lookup("operator.xtradb-cluster").Changed)
 
-			if err := c.Populate(false, askOperators); err != nil {
+			if err := c.Populate(cmd.Context(), false, askOperators); err != nil {
 				output.PrintError(err, l, !enableLogging)
 				os.Exit(1)
 			}

pkg/cli/flags.go

@@ -39,4 +39,6 @@ const (
 	FlagSkipEnvDetection = "skip-env-detection"
 	// FlagDisableTelemetry disables telemetry.
 	FlagDisableTelemetry = "disable-telemetry"
+	// FlagTakeNamespaceOwnership is the name of the take-ownership flag.
+	FlagTakeNamespaceOwnership = "take-ownership"
 )

pkg/cli/install/install.go

@@ -21,7 +21,6 @@ import (
 	"errors"
 	"fmt"
 	"io"
-	"net/url"
 	"os"
 	"strings"
 	"time"
@@ -38,6 +37,7 @@ import (
 	helmutils "github.com/percona/everest/pkg/cli/helm/utils"
 	"github.com/percona/everest/pkg/cli/namespaces"
 	"github.com/percona/everest/pkg/cli/steps"
+	cliutils "github.com/percona/everest/pkg/cli/utils"
 	"github.com/percona/everest/pkg/common"
 	"github.com/percona/everest/pkg/kubernetes"
 	"github.com/percona/everest/pkg/output"
@@ -104,13 +104,8 @@ func NewInstall(c Config, l *zap.SugaredLogger, cmd *cobra.Command) (*Install, e
 	c.NamespaceAddConfig.DisableTelemetry = c.DisableTelemetry
 	cli.config = c
 
-	k, err := kubernetes.New(c.KubeconfigPath, cli.l)
+	k, err := cliutils.NewKubeclient(cli.l, c.KubeconfigPath)
 	if err != nil {
-		var u *url.Error
-		if errors.As(err, &u) {
-			l.Error("Could not connect to Kubernetes. " +
-				"Make sure Kubernetes is running and is accessible from this computer/server.")
-		}
 		return nil, err
 	}
 	cli.kubeClient = k
@@ -128,7 +123,7 @@ func (o *Install) Run(ctx context.Context) error {
 		return fmt.Errorf("everest is already installed. Version: %s", installedVersion)
 	}
 
-	dbInstallStep, err := o.installDBNamespacesStep()
+	dbInstallStep, err := o.installDBNamespacesStep(ctx)
 	if err != nil {
 		return fmt.Errorf("could not create db install step: %w", err)
 	}
@@ -172,13 +167,13 @@ func (o *Install) Run(ctx context.Context) error {
 	return o.printPostInstallMessage(ctx, out)
 }
 
-func (o *Install) installDBNamespacesStep() (*steps.Step, error) {
+func (o *Install) installDBNamespacesStep(ctx context.Context) (*steps.Step, error) {
 	askNamespaces := !o.cmd.Flags().Lookup(cli.FlagNamespaces).Changed
 	askOperators := !(o.cmd.Flags().Lookup(cli.FlagOperatorMongoDB).Changed ||
 		o.cmd.Flags().Lookup(cli.FlagOperatorPostgresql).Changed ||
 		o.cmd.Flags().Lookup(cli.FlagOperatorXtraDBCluster).Changed)
 
-	if err := o.config.Populate(askNamespaces, askOperators); err != nil {
+	if err := o.config.Populate(ctx, askNamespaces, askOperators); err != nil {
 		// not specifying a namespace in this context is allowed.
 		if errors.Is(err, namespaces.ErrNSEmpty) {
 			return nil, nil //nolint:nilnil

pkg/cli/namespaces/add.go

@@ -6,7 +6,6 @@ import (
 	"errors"
 	"fmt"
 	"io"
-	"net/url"
 	"os"
 	"path"
 	"regexp"
@@ -61,13 +60,8 @@ func NewNamespaceAdd(c NamespaceAddConfig, l *zap.SugaredLogger) (*NamespaceAdde
 		n.l = zap.NewNop().Sugar()
 	}
 
-	k, err := kubernetes.New(c.KubeconfigPath, n.l)
+	k, err := cliutils.NewKubeclient(n.l, c.KubeconfigPath)
 	if err != nil {
-		var u *url.Error
-		if errors.As(err, &u) {
-			l.Error("Could not connect to Kubernetes. " +
-				"Make sure Kubernetes is running and is accessible from this computer/server.")
-		}
 		return nil, err
 	}
 	n.kubeClient = k
@@ -177,27 +171,52 @@ func (n *NamespaceAdder) newStepInstallNamespace(version, namespace string) step
 	}
 }
 
-func (n *NamespaceAdder) provisionDBNamespace(
+var (
+	// ErrNsDoesNotExist appears when the namespace does not exist.
+	ErrNsDoesNotExist = errors.New("namespace does not exist")
+	// ErrNamespaceNotManagedByEverest appears when the namespace is not managed by Everest.
+	ErrNamespaceNotManagedByEverest = errors.New("namespace is not managed by Everest")
+	// ErrNamespaceAlreadyExists appears when the namespace already exists.
+	ErrNamespaceAlreadyExists = errors.New("namespace already exists")
+)
+
+func (cfg *NamespaceAddConfig) validateNamespaceOwnership(
 	ctx context.Context,
-	version string,
 	namespace string,
 ) error {
-	nsExists, ownedByEverest, err := n.namespaceExists(ctx, namespace)
+	k, err := cliutils.NewKubeclient(zap.NewNop().Sugar(), cfg.KubeconfigPath)
 	if err != nil {
 		return err
 	}
 
-	if n.cfg.Update {
+	nsExists, ownedByEverest, err := namespaceExists(ctx, namespace, k)
+	if err != nil {
+		return err
+	}
+
+	if cfg.Update {
 		if !nsExists {
-			return fmt.Errorf("namespace (%s) does not exist", namespace)
+			return ErrNsDoesNotExist
 		}
 		if !ownedByEverest {
-			return fmt.Errorf("namespace (%s) is not managed by Everest", namespace)
+			return ErrNamespaceNotManagedByEverest
 		}
-	} else if nsExists && !n.cfg.TakeOwnership {
-		return fmt.Errorf("namespace (%s) already exists", namespace)
+	} else if nsExists && !cfg.TakeOwnership {
+		return ErrNamespaceAlreadyExists
 	}
 
+	return nil
+}
+
+func (n *NamespaceAdder) provisionDBNamespace(
+	ctx context.Context,
+	version string,
+	namespace string,
+) error {
+	nsExists, _, err := namespaceExists(ctx, namespace, n.kubeClient)
+	if err != nil {
+		return err
+	}
 	chartDir := ""
 	if n.cfg.ChartDir != "" {
 		chartDir = path.Join(n.cfg.ChartDir, dbNamespaceSubChartPath)
@@ -221,8 +240,8 @@ func (n *NamespaceAdder) provisionDBNamespace(
 	return installer.Install(ctx)
 }
 
-func (n *NamespaceAdder) namespaceExists(ctx context.Context, namespace string) (bool, bool, error) {
-	ns, err := n.kubeClient.GetNamespace(ctx, namespace)
+func namespaceExists(ctx context.Context, namespace string, k kubernetes.KubernetesConnector) (bool, bool, error) {
+	ns, err := k.GetNamespace(ctx, namespace)
 	if err != nil {
 		if k8serrors.IsNotFound(err) {
 			return false, false, nil
@@ -238,11 +257,17 @@ func isManagedByEverest(ns *v1.Namespace) bool {
 }
 
 // Populate the configuration with the required values.
-func (cfg *NamespaceAddConfig) Populate(askNamespaces, askOperators bool) error {
+func (cfg *NamespaceAddConfig) Populate(ctx context.Context, askNamespaces, askOperators bool) error {
 	if err := cfg.populateNamespaces(askNamespaces); err != nil {
 		return err
 	}
 
+	for _, ns := range cfg.NamespaceList {
+		if err := cfg.validateNamespaceOwnership(ctx, ns); err != nil {
+			return fmt.Errorf("invalid namespace (%s): %w", ns, err)
+		}
+	}
+
 	if askOperators && len(cfg.NamespaceList) > 0 && !cfg.SkipWizard {
 		if err := cfg.populateOperators(); err != nil {
 			return err

pkg/cli/namespaces/remove.go

@@ -5,7 +5,6 @@ import (
 	"errors"
 	"fmt"
 	"io"
-	"net/url"
 	"os"
 	"time"
 
@@ -58,19 +57,17 @@ func NewNamespaceRemove(c NamespaceRemoveConfig, l *zap.SugaredLogger) (*Namespa
 		n.l = zap.NewNop().Sugar()
 	}
 
-	k, err := kubernetes.New(c.KubeconfigPath, n.l)
+	k, err := cliutils.NewKubeclient(n.l, n.config.KubeconfigPath)
 	if err != nil {
-		var u *url.Error
-		if errors.As(err, &u) {
-			l.Error("Could not connect to Kubernetes. " +
-				"Make sure Kubernetes is running and is accessible from this computer/server.")
-		}
 		return nil, err
 	}
 	n.kubeClient = k
 	return n, nil
 }
 
+// ErrNamespaceNotEmpty is returned when the namespace is not empty.
+var ErrNamespaceNotEmpty = errors.New("cannot remove namespace with running database clusters")
+
 // Run the namespace removal operation.
 func (r *NamespaceRemover) Run(ctx context.Context) error {
 	// This command expects a Helm based installation (< 1.4.0)
@@ -85,7 +82,7 @@ func (r *NamespaceRemover) Run(ctx context.Context) error {
 	}
 
 	if dbsExist && !r.config.Force {
-		return errors.New("databases exist in the namespaces. Please remove them first or use --force")
+		return ErrNamespaceNotEmpty
 	}
 
 	removalSteps := []steps.Step{}

pkg/cli/uninstall/uninstall.go

@@ -21,7 +21,6 @@ import (
 	"errors"
 	"fmt"
 	"io"
-	"net/url"
 	"os"
 	"time"
 
@@ -30,7 +29,7 @@ import (
 
 	"github.com/percona/everest/pkg/cli/namespaces"
 	"github.com/percona/everest/pkg/cli/steps"
-	"github.com/percona/everest/pkg/cli/utils"
+	cliutils "github.com/percona/everest/pkg/cli/utils"
 	"github.com/percona/everest/pkg/common"
 	"github.com/percona/everest/pkg/kubernetes"
 )
@@ -80,13 +79,8 @@ func NewUninstall(c Config, l *zap.SugaredLogger) (*Uninstall, error) {
 		cli.l = zap.NewNop().Sugar()
 	}
 
-	kubeClient, err := kubernetes.New(c.KubeconfigPath, cli.l)
+	kubeClient, err := cliutils.NewKubeclient(cli.l, c.KubeconfigPath)
 	if err != nil {
-		var u *url.Error
-		if errors.As(err, &u) {
-			l.Error("Could not connect to Kubernetes. " +
-				"Make sure Kubernetes is running and is accessible from this computer/server.")
-		}
 		return nil, err
 	}
 	cli.kubeClient = kubeClient
@@ -97,7 +91,7 @@ func NewUninstall(c Config, l *zap.SugaredLogger) (*Uninstall, error) {
 func (u *Uninstall) Run(ctx context.Context) error {
 	// This command expects a Helm based installation. Otherwise, we stop here.
 	// Older versions must use an older version of the CLI.
-	_, err := utils.CheckHelmInstallation(ctx, u.kubeClient)
+	_, err := cliutils.CheckHelmInstallation(ctx, u.kubeClient)
 	if err != nil {
 		return err
 	}

pkg/cli/upgrade/upgrade.go

@@ -21,7 +21,6 @@ import (
 	"errors"
 	"fmt"
 	"io"
-	"net/url"
 	"os"
 	"time"
 
@@ -33,6 +32,7 @@ import (
 	"github.com/percona/everest/pkg/cli/helm"
 	helmutils "github.com/percona/everest/pkg/cli/helm/utils"
 	"github.com/percona/everest/pkg/cli/steps"
+	cliutils "github.com/percona/everest/pkg/cli/utils"
 	"github.com/percona/everest/pkg/common"
 	"github.com/percona/everest/pkg/kubernetes"
 	"github.com/percona/everest/pkg/output"
@@ -112,13 +112,8 @@ func NewUpgrade(cfg *Config, l *zap.SugaredLogger) (*Upgrade, error) {
 		kubeClient = k
 	}
 	if cfg.KubeconfigPath != "" {
-		k, err := kubernetes.New(cfg.KubeconfigPath, cli.l)
+		k, err := cliutils.NewKubeclient(cli.l, cfg.KubeconfigPath)
 		if err != nil {
-			var u *url.Error
-			if errors.As(err, &u) {
-				l.Error("Could not connect to Kubernetes. " +
-					"Make sure Kubernetes is running and is accessible from this computer/server.")
-			}
 			return nil, err
 		}
 		kubeClient = k