From 7a92fd0d57f9e34439cf5ced74e3f92e66737413 Mon Sep 17 00:00:00 2001 From: FakePlasticTree <58327725+lizzy-0323@users.noreply.github.com> Date: Fri, 18 Oct 2024 09:36:11 +0800 Subject: [PATCH] Fix: bugs in cluster and tenant creating (#590) --- internal/cli/cluster/create.go | 26 +++++++++++++++++--------- internal/cli/cluster/enter.go | 7 ++----- internal/cli/tenant/create.go | 34 +++++++++++++++++----------------- internal/cli/utils/utils.go | 2 +- 4 files changed, 37 insertions(+), 32 deletions(-) diff --git a/internal/cli/cluster/create.go b/internal/cli/cluster/create.go index 35f468903..d8dca6c57 100644 --- a/internal/cli/cluster/create.go +++ b/internal/cli/cluster/create.go @@ -55,9 +55,10 @@ func NewCreateOptions() *CreateOptions { OBServer: ¶m.OBServerSpec{ Storage: ¶m.OBServerStorageSpec{}, }, - Parameters: make([]modelcommon.KVPair, 0), - Zones: make(map[string]string), - Topology: make([]param.ZoneTopology, 0), + BackupVolume: ¶m.NFSVolumeSpec{}, + Parameters: make([]modelcommon.KVPair, 0), + Zones: make(map[string]string), + Topology: make([]param.ZoneTopology, 0), } } @@ -75,14 +76,20 @@ func (o *CreateOptions) Validate() error { } func (o *CreateOptions) Parse(_ *cobra.Command, args []string) error { + // Parse the zone topology topology, err := utils.MapZonesToTopology(o.Zones) if err != nil { return err } + // Parse the parameters parameters, err := utils.MapParameters(o.KvParameters) if err != nil { return err } + // Parse the BackupVolume related flags + if o.BackupVolume.Address == "" || o.BackupVolume.Path == "" { + o.BackupVolume = nil + } o.Parameters = parameters o.Topology = topology o.Name = args[0] @@ -256,7 +263,7 @@ func buildMonitorTemplate(monitorSpec *param.MonitorSpec) *apitypes.MonitorTempl return monitorTemplate } -// Create an OBClusterInstance +// CreateOBClusterInstance creates an OBClusterInstance func CreateOBClusterInstance(param *CreateOptions) *v1alpha1.OBCluster { observerTemplate := buildOBServerTemplate(param.OBServer) monitorTemplate := buildMonitorTemplate(param.Monitor) @@ -296,6 +303,7 @@ func (o *CreateOptions) AddFlags(cmd *cobra.Command) { o.AddObserverFlags(cmd) o.AddZoneFlags(cmd) o.AddParameterFlags(cmd) + o.AddBackupVolumeFlags(cmd) } // AddZoneFlags adds the zone-related flags to the command. @@ -319,8 +327,8 @@ func (o *CreateOptions) AddBaseFlags(cmd *cobra.Command) { func (o *CreateOptions) AddObserverFlags(cmd *cobra.Command) { observerFlags := pflag.NewFlagSet(FLAGSET_OBSERVER, pflag.ContinueOnError) observerFlags.StringVar(&o.OBServer.Image, FLAG_OBSERVER_IMAGE, DEFAULT_OBSERVER_IMAGE, "The image of the observer") - observerFlags.Int64Var(&o.OBServer.Resource.Cpu, FLAG_OBSERVER_CPU, DEFAULT_CPU_NUM, "The cpu of the observer") - observerFlags.Int64Var(&o.OBServer.Resource.MemoryGB, FLAG_MONITOR_MEMORY, DEFAULT_MONITOR_MEMORY, "The memory of the observer") + observerFlags.Int64Var(&o.OBServer.Resource.Cpu, FLAG_OBSERVER_CPU, DEFAULT_OBSERVER_CPU, "The cpu of the observer") + observerFlags.Int64Var(&o.OBServer.Resource.MemoryGB, FLAG_OBSERVER_MEMORY, DEFAULT_OBSERVER_MEMORY, "The memory of the observer") observerFlags.StringVar(&o.OBServer.Storage.Data.StorageClass, FLAG_DATA_STORAGE_CLASS, DEFAULT_DATA_STORAGE_CLASS, "The storage class of the data storage") observerFlags.StringVar(&o.OBServer.Storage.RedoLog.StorageClass, FLAG_REDO_LOG_STORAGE_CLASS, DEFAULT_REDO_LOG_STORAGE_CLASS, "The storage class of the redo log storage") observerFlags.StringVar(&o.OBServer.Storage.Log.StorageClass, FLAG_LOG_STORAGE_CLASS, DEFAULT_LOG_STORAGE_CLASS, "The storage class of the log storage") @@ -342,14 +350,14 @@ func (o *CreateOptions) AddMonitorFlags(cmd *cobra.Command) { // AddBackupVolumeFlags adds the backup-volume-related flags to the command. func (o *CreateOptions) AddBackupVolumeFlags(cmd *cobra.Command) { backupVolumeFlags := pflag.NewFlagSet(FLAGSET_BACKUP_VOLUME, pflag.ContinueOnError) - backupVolumeFlags.StringVar(&o.BackupVolume.Address, FLAG_BACKUP_ADDRESS, DEFAULT_BACKUP_ADDRESS, "The storage class of the backup storage") - backupVolumeFlags.StringVar(&o.BackupVolume.Path, FLAG_BACKUP_PATH, DEFAULT_BACKUP_PATH, "The size of the backup storage") + backupVolumeFlags.StringVar(&o.BackupVolume.Address, FLAG_BACKUP_ADDRESS, "", "The storage class of the backup storage") + backupVolumeFlags.StringVar(&o.BackupVolume.Path, FLAG_BACKUP_PATH, "", "The size of the backup storage") cmd.Flags().AddFlagSet(backupVolumeFlags) } // AddParameterFlags adds the parameter-related flags, e.g. __min_full_resource_pool_memory, to the command func (o *CreateOptions) AddParameterFlags(cmd *cobra.Command) { parameterFlags := pflag.NewFlagSet(FLAGSET_PARAMETERS, pflag.ContinueOnError) - parameterFlags.StringToStringVar(&o.KvParameters, FLAG_PARAMETERS, map[string]string{"__min_full_resource_pool_memory": DEFAULT_MIN_FULL_RESOURCE_POOL_MEMORY, "system_memory": DEFAULT_SYSTEM_MEMORY}, "Other parameter settings in OBCluster, e.g., __min_full_resource_pool_memory") + parameterFlags.StringToStringVar(&o.KvParameters, FLAG_PARAMETERS, map[string]string{"__min_full_resource_pool_memory": DEFAULT_MIN_FULL_RESOURCE_POOL_MEMORY, "system_memory": DEFAULT_MIN_SYSTEM_MEMORY}, "Other parameter settings in OBCluster, e.g., __min_full_resource_pool_memory") cmd.Flags().AddFlagSet(parameterFlags) } diff --git a/internal/cli/cluster/enter.go b/internal/cli/cluster/enter.go index c4ca50f30..dcfd1e85b 100644 --- a/internal/cli/cluster/enter.go +++ b/internal/cli/cluster/enter.go @@ -61,8 +61,7 @@ const ( // Default values for int and string flags DEFAULT_NAMESPACE = "default" DEFAULT_ID = 0 - DEFAULT_CPU_NUM = 2 - DEFAULT_OBSERVER_IMAGE = "oceanbase/oceanbase-cloud-native:4.2.1.6-106000012024042515" + DEFAULT_OBSERVER_IMAGE = "quay.io/oceanbase/oceanbase-cloud-native:4.2.1.6-106000012024042515" DEFAULT_OBSERVER_CPU = 2 DEFAULT_OBSERVER_MEMORY = 10 DEFAULT_DATA_STORAGE_CLASS = "local-path" @@ -74,10 +73,8 @@ const ( DEFAULT_MONITOR_IMAGE = "oceanbase/obagent:4.2.1-100000092023101717" DEFAULT_MONITOR_CPU = 1 DEFAULT_MONITOR_MEMORY = 1 - DEFAULT_BACKUP_ADDRESS = "local-path" - DEFAULT_BACKUP_PATH = "/opt/nfs" // Default values for Parameter flag DEFAULT_MIN_FULL_RESOURCE_POOL_MEMORY = "2147483648" - DEFAULT_SYSTEM_MEMORY = "1G" + DEFAULT_MIN_SYSTEM_MEMORY = "1G" ) diff --git a/internal/cli/tenant/create.go b/internal/cli/tenant/create.go index 9abaed10c..00bfeac8d 100644 --- a/internal/cli/tenant/create.go +++ b/internal/cli/tenant/create.go @@ -78,22 +78,22 @@ type CreateOptions struct { Timestamp string `json:"timestamp"` } -func (o *CreateOptions) Parse(_ *cobra.Command, args []string) error { +func (o *CreateOptions) Parse(cmd *cobra.Command, args []string) error { pools, err := utils.MapZonesToPools(o.ZonePriority) if err != nil { return err } o.Pools = pools o.Name = args[0] + o.Cmd = cmd + o.TenantRole = string(apiconst.TenantRolePrimary) if o.CheckIfFlagChanged("from") { o.Source.Tenant = &o.From - o.TenantRole = string(apiconst.TenantRolePrimary) - } else { - o.TenantRole = string(apiconst.TenantRoleStandby) - } - // create empty standby tenant - if !o.Restore { - o.Source.Restore = nil + // If flag `restore` is specified, restore a tenant, otherwise create an empty standby tenant. + if !o.Restore { + o.TenantRole = string(apiconst.TenantRoleStandby) + o.Source.Restore = nil + } } return nil } @@ -162,7 +162,7 @@ func CreateOBTenant(ctx context.Context, p *CreateOptions) (*v1alpha1.OBTenant, if strings.Contains(*p.Source.Tenant, "/") { splits := strings.Split(*p.Source.Tenant, "/") if len(splits) != 2 { - return nil, oberr.NewBadRequest("invalid tenant name") + return nil, fmt.Errorf("invalid tenant name") } ns, tenantCR = splits[0], splits[1] } @@ -172,21 +172,21 @@ func CreateOBTenant(ctx context.Context, p *CreateOptions) (*v1alpha1.OBTenant, }) if err != nil { if kubeerrors.IsNotFound(err) { - return nil, oberr.NewBadRequest("primary tenant not found") + return nil, fmt.Errorf("primary tenant not found") } - return nil, oberr.NewInternal(err.Error()) + return nil, fmt.Errorf(err.Error()) } if existing.Status.TenantRole != apiconst.TenantRolePrimary { - return nil, oberr.NewBadRequest("the target tenant is not primary tenant") + return nil, fmt.Errorf("the target tenant is not primary tenant") } // Match root password rootSecret, err := k8sclient.ClientSet.CoreV1().Secrets(existing.Namespace).Get(ctx, existing.Status.Credentials.Root, v1.GetOptions{}) if err != nil { - return nil, oberr.NewInternal(err.Error()) + return nil, fmt.Errorf(err.Error()) } if pwd, ok := rootSecret.Data["password"]; ok { if p.RootPassword != string(pwd) { - return nil, oberr.NewBadRequest("root password not match") + return nil, fmt.Errorf("root password not match") } if t.Spec.Credentials.Root != "" { err = createPasswordSecret(ctx, types.NamespacedName{ @@ -194,7 +194,7 @@ func CreateOBTenant(ctx context.Context, p *CreateOptions) (*v1alpha1.OBTenant, Name: t.Spec.Credentials.Root, }, p.RootPassword) if err != nil { - return nil, oberr.NewInternal(err.Error()) + return nil, fmt.Errorf(err.Error()) } } } @@ -414,8 +414,8 @@ func (o *CreateOptions) AddBaseFlags(cmd *cobra.Command) { baseFlags.StringVarP(&o.TenantName, FLAG_TENANT_NAME, "n", "", "Tenant name, if not specified, use name in k8s instead") baseFlags.StringVar(&o.ClusterName, FLAG_CLUSTER_NAME, "", "The cluster name tenant belonged to in k8s") baseFlags.StringVar(&o.Namespace, FLAG_NAMESPACE, DEFAULT_NAMESPACE, "The namespace of the tenant") - baseFlags.StringVarP(&o.RootPassword, FLAG_ROOTPASSWD, "p", "", "The root password of the cluster") - baseFlags.StringVar(&o.Charset, FLAG_CHARSET, DEFAULT_CHARSET, "The charset using in ob tenant") + baseFlags.StringVarP(&o.RootPassword, FLAG_ROOTPASSWD, "p", "", "The root password of the primary tenant, if not specified, generate a random password") + baseFlags.StringVarP(&o.Charset, FLAG_CHARSET, "c", DEFAULT_CHARSET, "The charset using in ob tenant") baseFlags.StringVar(&o.ConnectWhiteList, FLAG_CONNECT_WHITE_LIST, DEFAULT_CONNECT_WHITE_LIST, "The connect white list using in ob tenant") baseFlags.StringVar(&o.From, FLAG_FROM, "", "restore from data source") } diff --git a/internal/cli/utils/utils.go b/internal/cli/utils/utils.go index 79aa7a2ea..0105035ca 100644 --- a/internal/cli/utils/utils.go +++ b/internal/cli/utils/utils.go @@ -35,7 +35,7 @@ import ( ) const ( - characters = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789~#%^&*_-+|(){}[]:;,.?/\"" + characters = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789~#%^&*_-+|(){}[]:,.?/" factor = 4294901759 )