Fix(cli): bugs in cli first round testing (#664)

* fix: version flag

* fix: bugs in demo cmd and add addtional check when creating cluster and tenant

* fix: update all the helm repos when helm<3.7.0

* fix: log cmd to show the passwd with specified namespace

* fix: duplicate item in backup policy show cmd

* fix: remove `Complete` function when creating a backup policy

When using the same path for each tenant, will raise an NFS error "Error 9081 (HY000): format file does not match, try to set a new directory". This commit fixes the issue by ensuring the destination configuration correctly matches the current cluster/tenant/backup type.

* fix: remove path complete in tenant 's `create` cmd

* fix: lint

internal/cli/backup/create.go

@@ -16,7 +16,6 @@ package backup
 import (
 	"context"
 	"errors"
-	"fmt"
 
 	"github.com/robfig/cron/v3"
 	"github.com/spf13/cobra"
@@ -178,17 +177,6 @@ func CreateTenantBackupPolicy(ctx context.Context, o *CreateOptions) (*v1alpha1.
 	return policy, nil
 }
 
-func (o *CreateOptions) Complete() error {
-	// set default values for archive path and backup data path
-	if o.DestType == "NFS" && o.ArchivePath == "" {
-		o.ArchivePath = fmt.Sprintf("%s/%s", "archive", o.Name)
-	}
-	if o.DestType == "NFS" && o.BakDataPath == "" {
-		o.BakDataPath = fmt.Sprintf("%s/%s", "backup", o.Name)
-	}
-	return nil
-}
-
 func (o *CreateOptions) Validate() error {
 	if o.Namespace == "" {
 		return errors.New("Namespace is required")
@@ -224,6 +212,8 @@ func (o *CreateOptions) AddFlags(cmd *cobra.Command) {
 
 func (o *CreateOptions) SetRequiredFlags(cmd *cobra.Command) {
 	_ = cmd.MarkFlagRequired(FLAG_FULL)
+	_ = cmd.MarkFlagRequired(FLAG_ARCHIVE_PATH)
+	_ = cmd.MarkFlagRequired(FLAG_BAK_DATA_PATH)
 }
 
 // AddBaseFlags adds the base flags for the create command

internal/cli/cli.go

@@ -32,13 +32,12 @@ func NewCliCmd() *cobra.Command {
 		Use:   "obocli",
 		Short: "OceanBase Operator Cli",
 		Long:  "OceanBase Operator Cli tool to manage OceanBase clusters, tenants, and backup policies.",
-		Run: func(cmd *cobra.Command, args []string) {
+		RunE: func(cmd *cobra.Command, args []string) error {
 			if cmd.Flags().Changed("version") {
 				versionCmd := version.NewCmd()
-				versionCmd.Run(cmd, args)
-			} else {
-				_ = cmd.Help()
+				return versionCmd.RunE(cmd, args)
 			}
+			return cmd.Help()
 		},
 	}
 	cmd.AddCommand(version.NewCmd())

internal/cli/cmd/backup/create.go

@@ -31,9 +31,6 @@ func NewCreateCmd() *cobra.Command {
 		Args:    cobra.ExactArgs(1),
 		PreRunE: o.Parse,
 		Run: func(cmd *cobra.Command, args []string) {
-			if err := o.Complete(); err != nil {
-				logger.Fatalln(err)
-			}
 			if err := o.Validate(); err != nil {
 				logger.Fatalln(err)
 			}

internal/cli/cmd/backup/show.go

@@ -45,7 +45,7 @@ func NewShowCmd() *cobra.Command {
 				logger.Fatalln(err, "failed to get backup policy")
 			}
 			if backupPolicy == nil || backupPolicy.Name == "" {
-				logger.Fatalln("no backup policy found")
+				logger.Fatalln("no backup policies found")
 			}
 			backupJobList, err := backup.ListBackupJobs(cmd.Context(), backupPolicy.Name, o)
 			if err != nil {
@@ -60,9 +60,9 @@ func NewShowCmd() *cobra.Command {
 				sort.Slice(backupJobList.Items, func(i, j int) bool {
 					return backupJobList.Items[i].CreationTimestamp.Before(&backupJobList.Items[j].CreationTimestamp)
 				})
+				tbLog.Println("JOBNAME \t STATUS \t TYPE \t STARTAT \t ENDAT \t")
 			}
 			for _, job := range backupJobList.Items {
-				tbLog.Println("JOBNAME \t STATUS \t TYPE \t STARTAT \t ENDAT \t")
 				tbLog.Printf("%s \t %s \t %s \t %s \t %s \n", job.Name, job.Status.Status, job.Spec.Type, job.Status.StartedAt, job.Status.EndedAt)
 			}
 			if err = tbw.Flush(); err != nil {

internal/cli/cmd/cluster/create.go

@@ -41,7 +41,7 @@ func NewCreateCmd() *cobra.Command {
 				logger.Fatalln(err)
 			}
 			logger.Printf("Create OBCluster instance: %s", o.ClusterName)
-			logger.Printf("Run `echo $(kubectl get secret %s -o jsonpath='{.data.password}'|base64 --decode)` to get the secrets", obcluster.Spec.UserSecrets.Root)
+			logger.Printf("Run `echo $(kubectl get secret %s -n %s -o jsonpath='{.data.password}'|base64 --decode)` to get the secrets", obcluster.Spec.UserSecrets.Root, obcluster.Namespace)
 		},
 	}
 	o.AddFlags(cmd)

internal/cli/cmd/demo/demo.go

@@ -40,20 +40,27 @@ func NewCmd() *cobra.Command {
 	var clusterType string
 	var wait bool
 	var err error
+	var prompt any
 	cmd := &cobra.Command{
 		Use:   "demo <subcommand>",
 		Short: "deploy demo ob cluster and tenant in easier way",
 		Long:  `deploy demo ob cluster and tenant in easier way, currently support single node and three node cluster, with corresponding tenant`,
 		Args:  cobra.NoArgs,
 		PreRun: func(cmd *cobra.Command, args []string) {
 			// prompt for cluster create options
-			prompt := pf.CreatePrompt(cluster.FLAG_NAME)
-			if clusterOptions.Name, err = pf.RunPromptE(prompt); err != nil {
-				logger.Fatalln(err)
-			}
-			prompt = pf.CreatePrompt(cluster.FLAG_NAMESPACE)
-			if clusterOptions.Namespace, err = pf.RunPromptE(prompt); err != nil {
-				logger.Fatalln(err)
+			for {
+				prompt = pf.CreatePrompt(cluster.FLAG_NAME)
+				if clusterOptions.Name, err = pf.RunPromptE(prompt); err != nil {
+					logger.Fatalln(err)
+				}
+				prompt = pf.CreatePrompt(cluster.FLAG_NAMESPACE)
+				if clusterOptions.Namespace, err = pf.RunPromptE(prompt); err != nil {
+					logger.Fatalln(err)
+				}
+				if !utils.CheckIfClusterExists(cmd.Context(), clusterOptions.Name, clusterOptions.Namespace) {
+					break
+				}
+				logger.Printf("Cluster %s already exists in namespace %s, please input another cluster name", clusterOptions.Name, clusterOptions.Namespace)
 			}
 			prompt = pf.CreatePrompt(cluster.CLUSTER_TYPE)
 			if clusterType, err = pf.RunPromptE(prompt); err != nil {
@@ -63,9 +70,17 @@ func NewCmd() *cobra.Command {
 			if clusterOptions.RootPassword, err = pf.RunPromptE(prompt); err != nil {
 				logger.Fatalln(err)
 			}
-			prompt = pf.CreatePrompt(tenant.FLAG_TENANT_NAME_IN_K8S)
-			if tenantOptions.Name, err = pf.RunPromptE(prompt); err != nil {
-				logger.Fatalln(err)
+
+			// prompt for tenant create options
+			for {
+				prompt = pf.CreatePrompt(tenant.FLAG_TENANT_NAME_IN_K8S)
+				if tenantOptions.Name, err = pf.RunPromptE(prompt); err != nil {
+					logger.Fatalln(err)
+				}
+				if !utils.CheckIfTenantExists(cmd.Context(), tenantOptions.Name, clusterOptions.Namespace) {
+					break
+				}
+				logger.Printf("Tenant %s already exists in namespace %s, please input another tenant name", tenantOptions.Name, clusterOptions.Namespace)
 			}
 			prompt = pf.CreatePrompt(tenant.FLAG_TENANT_NAME)
 			if tenantOptions.TenantName, err = pf.RunPromptE(prompt); err != nil {
@@ -91,7 +106,7 @@ func NewCmd() *cobra.Command {
 			}
 			logger.Printf("Creating OBCluster instance: %s", clusterOptions.ClusterName)
 			waitForClusterReady(cmd.Context(), obcluster, logger, 2*time.Second)
-			logger.Printf("Run `echo $(kubectl get secret %s -clusterOptions jsonpath='{.data.password}'|base64 --decode)` to get clsuter secrets", obcluster.Spec.UserSecrets.Root)
+			logger.Printf("Run `echo $(kubectl get secret %s -n %s -o jsonpath='{.data.password}'|base64 --decode)` to get cluster secrets", obcluster.Spec.UserSecrets.Root, obcluster.Namespace)
 		},
 		PostRun: func(cmd *cobra.Command, args []string) {
 			// create tenant after cluster ready
@@ -101,7 +116,7 @@ func NewCmd() *cobra.Command {
 			}
 			logger.Printf("Creating OBTenant instance: %s", tenantOptions.TenantName)
 			waitForTenantReady(cmd.Context(), obtenant, logger, 1*time.Second)
-			logger.Printf("Run `echo $(kubectl get secret %s -o jsonpath='{.data.password}'|base64 --decode)` to get tenant secrets", obtenant.Spec.Credentials.Root)
+			logger.Printf("Run `echo $(kubectl get secret %s -n %s -o jsonpath='{.data.password}'|base64 --decode)` to get tenant secrets", obtenant.Spec.Credentials.Root, obtenant.Namespace)
 		},
 	}
 	// TODO: if w is set, wait for the cluster and tenant ready

internal/cli/cmd/tenant/create.go

@@ -41,7 +41,7 @@ func NewCreateCmd() *cobra.Command {
 				logger.Fatalln(err)
 			}
 			logger.Printf("Create OBTenant instance: %s", o.TenantName)
-			logger.Printf("Run `echo $(kubectl get secret %s -o jsonpath='{.data.password}'|base64 --decode)` to get the secrets", obtenant.Spec.Credentials.Root)
+			logger.Printf("Run `echo $(kubectl get secret %s -n %s -o jsonpath='{.data.password}'|base64 --decode)` to get the secrets", obtenant.Spec.Credentials.Root, obtenant.Namespace)
 		},
 	}
 	o.AddFlags(cmd)

internal/cli/demo/prompt.go

@@ -39,6 +39,7 @@ var (
 	}
 )
 
+// PromptFactory is a factory to create prompt
 type PromptFactory struct {
 	promptTepl *promptui.PromptTemplates
 	selectTepl *promptui.SelectTemplates
@@ -114,7 +115,7 @@ func (pf *PromptFactory) CreatePrompt(promptType string) any {
 				if input == "" {
 					return nil
 				}
-				if utils.CheckPassword(input) {
+				if !utils.CheckPassword(input) {
 					return errors.New("invalid password")
 				}
 				return nil

internal/cli/install/install.go

@@ -129,7 +129,7 @@ func addHelmRepo() error {
 
 // updateHelmRepo update ob-operator helm repo
 func updateHelmRepo() error {
-	cmdUpdateRepo := exec.Command("helm", "repo", "update", "ob-operator")
+	cmdUpdateRepo := exec.Command("helm", "repo", "update")
 	output, err := cmdUpdateRepo.CombinedOutput()
 	if err != nil {
 		return fmt.Errorf("updating repo failed: %s, %s", err, output)

internal/cli/tenant/create.go

@@ -116,12 +116,6 @@ func (o *CreateOptions) Complete() error {
 	if o.RestoreType != "" {
 		o.RestoreType = strings.ToUpper(o.RestoreType)
 	}
-	if o.RestoreType == "NFS" && o.Source.Restore.BakDataSource == "" {
-		o.Source.Restore.BakDataSource = fmt.Sprintf("%s/%s", "backup", o.From)
-	}
-	if o.RestoreType == "NFS" && o.Source.Restore.ArchiveSource == "" {
-		o.Source.Restore.ArchiveSource = fmt.Sprintf("%s/%s", "archive", o.From)
-	}
 	return nil
 }
 
@@ -141,14 +135,17 @@ func (o *CreateOptions) Validate() error {
 	if !utils.CheckTenantName(o.TenantName) {
 		return fmt.Errorf("invalid tenant name: %s, the first letter must be a letter or an underscore and cannot contain -", o.TenantName)
 	}
+	if o.Restore && o.Source == nil {
+		return errors.New("source tenant is not specified")
+	}
 	if o.Restore && o.RestoreType != "OSS" && o.RestoreType != "NFS" {
-		return errors.New("Restore Type not supported")
+		return errors.New("restore Type is not supported")
 	}
 	if o.Restore && o.RestoreType == "OSS" && o.Source.Restore.OSSAccessKey == "" {
-		return errors.New("oss access key not specified")
+		return errors.New("oss access key is not specified")
 	}
 	if o.Restore && o.RestoreType == "NFS" && o.Source.Restore.BakEncryptionPassword == "" {
-		return errors.New("back encryption password not specified")
+		return errors.New("back encryption password is not specified")
 	}
 	return nil
 }
@@ -401,6 +398,7 @@ func (o *CreateOptions) AddFlags(cmd *cobra.Command) {
 func (o *CreateOptions) SetRequiredFlags(cmd *cobra.Command) {
 	_ = cmd.MarkFlagRequired(FLAG_CLUSTER_NAME)
 	_ = cmd.MarkFlagRequired(FLAG_ZONE_PRIORITY)
+	cmd.MarkFlagsRequiredTogether(FLAG_RESTORE, FLAG_ARCHIVE_SOURCE, FLAG_BAK_DATA_SOURCE)
 }
 
 // AddBaseFlags add base flags

internal/cli/utils/checker.go

@@ -15,11 +15,15 @@ package utils
 
 import (
 	"bytes"
+	"context"
 	"fmt"
 	"os/exec"
 
+	"k8s.io/apimachinery/pkg/types"
+
 	apitypes "github.com/oceanbase/ob-operator/api/types"
 	"github.com/oceanbase/ob-operator/api/v1alpha1"
+	"github.com/oceanbase/ob-operator/internal/clients"
 	clusterstatus "github.com/oceanbase/ob-operator/internal/const/status/obcluster"
 	"github.com/oceanbase/ob-operator/internal/const/status/tenantstatus"
 )
@@ -59,6 +63,22 @@ var (
 	localPathProvisionerResources = "local-path-provisioner"
 )
 
+// CheckIfClusterExists checks if cluster exists in the environment
+func CheckIfClusterExists(ctx context.Context, name string, namespace string) bool {
+	cluster, _ := clients.GetOBCluster(ctx, namespace, name)
+	return cluster != nil
+}
+
+// CheckIfTenantExists checks if tenant exists in the environment
+func CheckIfTenantExists(ctx context.Context, name string, namespace string) bool {
+	nn := types.NamespacedName{
+		Name:      name,
+		Namespace: namespace,
+	}
+	tenant, _ := clients.GetOBTenant(ctx, nn)
+	return tenant != nil
+}
+
 // CheckTenantStatus checks running status of obtenant
 func CheckTenantStatus(tenant *v1alpha1.OBTenant) error {
 	if tenant.Status.Status != tenantstatus.Running {

internal/cli/utils/validator.go

@@ -16,6 +16,7 @@ package utils
 import (
 	"regexp"
 	"strings"
+	"unicode"
 )
 
 const (
@@ -42,19 +43,19 @@ func CheckPassword(password string) bool {
 	for _, char := range password {
 		if strings.ContainsRune(characters, char) {
 			switch {
-			case strings.ContainsRune("ABCDEFGHIJKLMNOPQRSTUVWXYZ", char):
+			case unicode.IsUpper(char):
 				countUppercase++
-			case strings.ContainsRune("abcdefghijklmnopqrstuvwxyz", char):
+			case unicode.IsLower(char):
 				countLowercase++
-			case strings.ContainsRune("0123456789", char):
+			case unicode.IsNumber(char):
 				countNumber++
 			default:
 				countSpecialChar++
 			}
 		} else {
 			return false
 		}
-		// if satisfied
+		// if satisfied, early return
 		if countUppercase >= 2 && countLowercase >= 2 && countNumber >= 2 && countSpecialChar >= 2 {
 			return true
 		}

internal/dashboard/business/oceanbase/obcluster.go

@@ -20,7 +20,6 @@ import (
 
 	"github.com/google/uuid"
 	"github.com/pkg/errors"
-	"github.com/sirupsen/logrus"
 	logger "github.com/sirupsen/logrus"
 	corev1 "k8s.io/api/core/v1"
 	apiresource "k8s.io/apimachinery/pkg/api/resource"
@@ -896,17 +895,17 @@ func ListOBClusterParameters(ctx context.Context, nn *param.K8sObjectIdentity) (
 		LabelSelector: fmt.Sprintf("%s=%s", oceanbaseconst.LabelRefOBCluster, nn.Name),
 	})
 	if err != nil {
-		logrus.WithError(err).Error("Failed to list observers")
+		logger.WithError(err).Error("Failed to list observers")
 		return nil, errors.Wrap(err, "List observers")
 	}
 	conn, err := utils.GetOBConnection(ctx, obcluster, "root", "sys", obcluster.Spec.UserSecrets.Root)
 	if err != nil {
-		logrus.Info("Failed to get OceanBase database connection")
+		logger.Info("Failed to get OceanBase database connection")
 		return nil, errors.Wrap(err, "Get OceanBase database connection")
 	}
 	parameters, err := conn.ListParametersWithTenantID(ctx, 1)
 	if err != nil {
-		logrus.WithError(err).Error("Failed to query parameters")
+		logger.WithError(err).Error("Failed to query parameters")
 		return nil, errors.Wrap(err, "Query parameters")
 	}
 	return parameters, nil