From 41c26ed1ca0d6ce4c740803d691750184bf64af0 Mon Sep 17 00:00:00 2001 From: a le <101848970+1aal@users.noreply.github.com> Date: Tue, 17 Oct 2023 09:21:32 +0800 Subject: [PATCH] fix: orioledb create fail by `kbcli cluster create --cluster-definition` (#5422) Co-authored-by: 1aal <1aal@users.noreply.github.com> (cherry picked from commit 111733811bf792086fc1ae2c1ffc9bf17e90e578) --- docs/user_docs/cli/kbcli_cluster_create.md | 10 ++ internal/cli/cluster/helper.go | 23 +++ internal/cli/cluster/helper_test.go | 21 +++ internal/cli/cmd/cluster/create.go | 160 ++++++++++++++++++++- internal/cli/cmd/cluster/create_test.go | 95 ++++++++++++ internal/cli/testing/fake.go | 16 +++ 6 files changed, 323 insertions(+), 2 deletions(-) diff --git a/docs/user_docs/cli/kbcli_cluster_create.md b/docs/user_docs/cli/kbcli_cluster_create.md index 25dff24ec2f..8c7a008ed34 100644 --- a/docs/user_docs/cli/kbcli_cluster_create.md +++ b/docs/user_docs/cli/kbcli_cluster_create.md @@ -84,6 +84,15 @@ kbcli cluster create [NAME] [flags] # Create a cluster with auto backup kbcli cluster create --cluster-definition apecloud-mysql --backup-enabled + + # Create a cluster with default component having multiple storage volumes + kbcli cluster create --cluster-definition oceanbase --pvc name=data-file,size=50Gi --pvc name=data-log,size=50Gi --pvc name=log,size=20Gi + + # Create a cluster with specifying a component having multiple storage volumes + kbcli cluster create --cluster-definition pulsar --pvc type=bookies,name=ledgers,size=20Gi --pvc type=bookies,name=journal,size=20Gi + + # Create a cluster with using a service reference to another KubeBlocks cluster + cluster create --cluster-definition pulsar --service-reference name=pulsarZookeeper,cluster=zookeeper,namespace=default ``` ### Options @@ -110,6 +119,7 @@ kbcli cluster create [NAME] [flags] --pvc stringArray Set the cluster detail persistent volume claim, each '--pvc' corresponds to a component, and will override the simple configurations about storage by --set (e.g. --pvc type=mysql,name=data,mode=ReadWriteOnce,size=20Gi --pvc type=mysql,name=log,mode=ReadWriteOnce,size=1Gi) --rbac-enabled Specify whether rbac resources will be created by kbcli, otherwise KubeBlocks server will try to create rbac resources --restore-to-time string Set a time for point in time recovery + --service-reference stringArray Set the other KubeBlocks cluster dependencies, each '--service-reference' corresponds to a cluster service. (e.g --service-reference name=pulsarZookeeper,cluster=zookeeper,namespace=default) --set stringArray Set the cluster resource including cpu, memory, replicas and storage, each set corresponds to a component.(e.g. --set cpu=1,memory=1Gi,replicas=3,storage=20Gi or --set class=general-1c1g) -f, --set-file string Use yaml file, URL, or stdin to set the cluster resource --tenancy string Tenancy options, one of: (SharedNode, DedicatedNode) (default "SharedNode") diff --git a/internal/cli/cluster/helper.go b/internal/cli/cluster/helper.go index 459dbe45374..92789123e9f 100644 --- a/internal/cli/cluster/helper.go +++ b/internal/cli/cluster/helper.go @@ -431,3 +431,26 @@ func GetConfigConstraintByName(dynamic dynamic.Interface, name string) (*appsv1a } return ccObj, nil } + +func GetServiceRefs(cd *appsv1alpha1.ClusterDefinition) []string { + var serviceRefs []string + for _, compDef := range cd.Spec.ComponentDefs { + if compDef.ServiceRefDeclarations == nil { + continue + } + + for _, ref := range compDef.ServiceRefDeclarations { + serviceRefs = append(serviceRefs, ref.Name) + } + } + return serviceRefs +} + +// GetDefaultServiceRef will return the ServiceRefDeclarations in cluster-definition when the cluster-definition contains only one ServiceRefDeclaration +func GetDefaultServiceRef(cd *appsv1alpha1.ClusterDefinition) (string, error) { + serviceRefs := GetServiceRefs(cd) + if len(serviceRefs) != 1 { + return "", fmt.Errorf("failed to get the cluster default service reference name") + } + return serviceRefs[0], nil +} diff --git a/internal/cli/cluster/helper_test.go b/internal/cli/cluster/helper_test.go index aa22666e1a8..71eb4c5a12e 100644 --- a/internal/cli/cluster/helper_test.go +++ b/internal/cli/cluster/helper_test.go @@ -125,4 +125,25 @@ var _ = Describe("helper", func() { Expect(err).Should(Succeed()) Expect(cm).ShouldNot(BeNil()) }) + + It("get all ServiceRefs from cluster-definition", func() { + Expect(GetServiceRefs(testing.FakeClusterDef())).Should(Equal([]string{testing.ServiceRefName})) + }) + + It("get default ServiceRef from cluster-definition", func() { + cd := testing.FakeClusterDef() + ref, err := GetDefaultServiceRef(cd) + Expect(ref).Should(Equal(testing.ServiceRefName)) + Expect(err).Should(Succeed()) + + deepCopyCD := cd.DeepCopy() + deepCopyCD.Spec.ComponentDefs[0].ServiceRefDeclarations = append(deepCopyCD.Spec.ComponentDefs[0].ServiceRefDeclarations, testing.FakeServiceRef("other-serviceRef")) + _, err = GetDefaultServiceRef(deepCopyCD) + Expect(err).Should(HaveOccurred()) + + deepCopyCD = cd.DeepCopy() + deepCopyCD.Spec.ComponentDefs[0].ServiceRefDeclarations = nil + _, err = GetDefaultServiceRef(deepCopyCD) + Expect(err).Should(HaveOccurred()) + }) }) diff --git a/internal/cli/cmd/cluster/create.go b/internal/cli/cmd/cluster/create.go index c8590c9e860..4cec421105e 100755 --- a/internal/cli/cmd/cluster/create.go +++ b/internal/cli/cmd/cluster/create.go @@ -140,6 +140,15 @@ var clusterCreateExample = templates.Examples(` # Create a cluster with auto backup kbcli cluster create --cluster-definition apecloud-mysql --backup-enabled + + # Create a cluster with default component having multiple storage volumes + kbcli cluster create --cluster-definition oceanbase --pvc name=data-file,size=50Gi --pvc name=data-log,size=50Gi --pvc name=log,size=20Gi + + # Create a cluster with specifying a component having multiple storage volumes + kbcli cluster create --cluster-definition pulsar --pvc type=bookies,name=ledgers,size=20Gi --pvc type=bookies,name=journal,size=20Gi + + # Create a cluster with using a service reference to another KubeBlocks cluster + cluster create --cluster-definition pulsar --service-reference name=pulsarZookeeper,cluster=zookeeper,namespace=default `) const ( @@ -228,6 +237,7 @@ type CreateOptions struct { Values []string `json:"-"` RBACEnabled bool `json:"-"` Storages []string `json:"-"` + ServiceRef []string `json:"-"` // backup name to restore in creation Backup string `json:"backup,omitempty"` RestoreTime string `json:"restoreTime,omitempty"` @@ -262,6 +272,8 @@ func NewCreateCmd(f cmdutil.Factory, streams genericiooptions.IOStreams) *cobra. cmd.Flags().StringVarP(&o.SetFile, "set-file", "f", "", "Use yaml file, URL, or stdin to set the cluster resource") cmd.Flags().StringArrayVar(&o.Values, "set", []string{}, "Set the cluster resource including cpu, memory, replicas and storage, each set corresponds to a component.(e.g. --set cpu=1,memory=1Gi,replicas=3,storage=20Gi or --set class=general-1c1g)") cmd.Flags().StringArrayVar(&o.Storages, "pvc", []string{}, "Set the cluster detail persistent volume claim, each '--pvc' corresponds to a component, and will override the simple configurations about storage by --set (e.g. --pvc type=mysql,name=data,mode=ReadWriteOnce,size=20Gi --pvc type=mysql,name=log,mode=ReadWriteOnce,size=1Gi)") + cmd.Flags().StringArrayVar(&o.ServiceRef, "service-reference", []string{}, "Set the other KubeBlocks cluster dependencies, each '--service-reference' corresponds to a cluster service. (e.g --service-reference name=pulsarZookeeper,cluster=zookeeper,namespace=default)") + cmd.Flags().StringVar(&o.Backup, "backup", "", "Set a source backup to restore data") cmd.Flags().StringVar(&o.RestoreTime, "restore-to-time", "", "Set a time for point in time recovery") cmd.Flags().StringVar(&o.RestoreManagementPolicy, "volume-restore-policy", "Parallel", "the volume claim restore policy, supported values: [Serial, Parallel]") @@ -633,6 +645,14 @@ func (o *CreateOptions) buildComponents(clusterCompSpecs []appsv1alpha1.ClusterC compSpecs = rebuildCompStorage(storages, compSpecs) } + // build service reference if --service-reference not empty + if len(o.ServiceRef) != 0 { + compSpecs, err = buildServiceRefs(o.ServiceRef, cd, compSpecs) + if err != nil { + return nil, err + } + } + var comps []map[string]interface{} for _, compSpec := range compSpecs { // validate component classes @@ -1591,13 +1611,13 @@ func buildCompStorages(pvcs []string, cd *appsv1alpha1.ClusterDefinition) (map[s for _, set := range sets { kv := strings.Split(set, "=") if len(kv) != 2 { - return nil, fmt.Errorf("unknown set format \"%s\", should be like key1=value1", set) + return nil, fmt.Errorf("unknown --pvc format \"%s\", should be like key1=value1", set) } // only record the supported key k := parseKey(kv[0]) if k == storageKeyUnknown { - return nil, fmt.Errorf("unknown set key \"%s\", should be one of [%s]", kv[0], strings.Join(storageSetKey(), ",")) + return nil, fmt.Errorf("unknown --pvc key \"%s\", should be one of [%s]", kv[0], strings.Join(storageSetKey(), ",")) } res[k] = kv[1] } @@ -1704,3 +1724,139 @@ func rebuildCompStorage(pvcMaps map[string][]map[storageKey]string, specs []*app } return specs } + +// serviceRefKey declares --service-reference validate keyword +type serviceRefKey string + +const ( + serviceRefKeyName serviceRefKey = "name" + serviceRefKeyCluster serviceRefKey = "cluster" + serviceRefKeyNamespace serviceRefKey = "namespace" + serviceRefKeyUnknown serviceRefKey = "unknown" +) + +func serviceRefSetKey() []string { + return []string{ + string(serviceRefKeyName), + string(serviceRefKeyCluster), + string(serviceRefKeyNamespace), + } +} + +// getServiceRefs parses the serviceRef from flag --service-reference and performs basic validation, then return all valid serviceRefs +func getServiceRefs(serviceRef []string, cd *appsv1alpha1.ClusterDefinition) ([]map[serviceRefKey]string, error) { + var serviceRefSets []map[serviceRefKey]string + + parseKey := func(key string) serviceRefKey { + for _, k := range serviceRefSetKey() { + if strings.EqualFold(k, key) { + return serviceRefKey(k) + } + } + return serviceRefKeyUnknown + } + + buildServiceRefMap := func(sets []string) (map[serviceRefKey]string, error) { + res := map[serviceRefKey]string{} + for _, set := range sets { + kv := strings.Split(set, "=") + if len(kv) != 2 { + return nil, fmt.Errorf("unknown --service-reference format \"%s\", should be like key1=value1", set) + } + + // only record the supported key + k := parseKey(kv[0]) + if k == serviceRefKeyUnknown { + return nil, fmt.Errorf("unknown --service-reference key \"%s\", should be one of [%s]", kv[0], strings.Join(serviceRefSetKey(), ",")) + } + res[k] = kv[1] + } + return res, nil + } + + for _, ref := range serviceRef { + refMap, err := buildServiceRefMap(strings.Split(ref, ",")) + if err != nil { + return nil, err + } + if len(refMap) == 0 { + continue + } + serviceRefName := refMap[serviceRefKeyName] + + if len(serviceRefName) == 0 { + name, err := cluster.GetDefaultServiceRef(cd) + if err != nil { + return nil, err + } + refMap[serviceRefKeyName] = name + } else { + // check if the serviceRefName is defined in the cluster-definition + valid := false + for _, c := range cluster.GetServiceRefs(cd) { + if c == serviceRefName { + valid = true + break + } + } + if !valid { + // todo: kbcli cluster list-serviceRef + return nil, fmt.Errorf("the service reference name \"%s\" is not declared in the cluster components,use `kbcli cluster list-serviceRef %s` to show all available service reference names", serviceRefName, cd.Name) + } + } + serviceRefSets = append(serviceRefSets, refMap) + } + return serviceRefSets, nil +} + +// buildServiceRefs supplements the serviceRef content for compSpecs based on the input flags --service-reference +func buildServiceRefs(serviceRef []string, cd *appsv1alpha1.ClusterDefinition, compSpecs []*appsv1alpha1.ClusterComponentSpec) ([]*appsv1alpha1.ClusterComponentSpec, error) { + refSet, err := getServiceRefs(serviceRef, cd) + if err != nil { + return nil, err + } + // Check if the ServiceRefDeclarations for the current refName have been declared in the cluster-definition corresponding to comp + check := func(comp *appsv1alpha1.ClusterComponentSpec, refName string) bool { + valid := false + for _, cdSpec := range cd.Spec.ComponentDefs { + if cdSpec.Name != comp.Name { + continue + } + for i := range cdSpec.ServiceRefDeclarations { + if cdSpec.ServiceRefDeclarations[i].Name == refName { + // serviceRefName has been declared in cluster-definition + valid = true + break + } + } + } + return valid + } + + for _, ref := range refSet { + name := ref[serviceRefKeyName] + for i, comp := range compSpecs { + if !check(comp, name) { + continue + } + // if cluster-definition ComponentDef have the correct ServiceRefDeclarations，add the ServiceRefs to the cluster compSpecs + if compSpecs[i].ServiceRefs == nil { + compSpecs[i].ServiceRefs = []appsv1alpha1.ServiceRef{ + { + Name: ref[serviceRefKeyName], + Namespace: ref[serviceRefKeyNamespace], + Cluster: ref[serviceRefKeyCluster], + }, + } + } else { + compSpecs[i].ServiceRefs = append(compSpecs[i].ServiceRefs, + appsv1alpha1.ServiceRef{ + Name: ref[serviceRefKeyName], + Namespace: ref[serviceRefKeyNamespace], + Cluster: ref[serviceRefKeyCluster], + }) + } + } + } + return compSpecs, nil +} diff --git a/internal/cli/cmd/cluster/create_test.go b/internal/cli/cmd/cluster/create_test.go index eb3f05d7a05..96713ff1c15 100644 --- a/internal/cli/cmd/cluster/create_test.go +++ b/internal/cli/cmd/cluster/create_test.go @@ -20,6 +20,7 @@ along with this program. If not, see . package cluster import ( + "fmt" "net/http" "net/http/httptest" "os" @@ -758,4 +759,98 @@ var _ = Describe("create", func() { }) + It("test getServiceRefs", func() { + testCase := []struct { + input []string + success bool + expected []map[serviceRefKey]string + }{ + { + []string{fmt.Sprintf("name=%s,cluster=mysql,namespace=default", testing.ServiceRefName)}, + true, + []map[serviceRefKey]string{ + { + serviceRefKeyName: testing.ServiceRefName, + serviceRefKeyCluster: "mysql", + serviceRefKeyNamespace: "default", + }, + }, + }, + { + []string{"name=invalid,cluster=mysql,namespace=default"}, + false, + nil, + }, + { + []string{"invalidKey=test"}, + false, + nil, + }, + } + for i := range testCase { + refs, err := getServiceRefs(testCase[i].input, testing.FakeClusterDef()) + if testCase[i].success { + Expect(err).Should(Succeed()) + Expect(refs).Should(Equal(testCase[i].expected)) + } else { + Expect(err).Should(HaveOccurred()) + } + } + + }) + + It("test build ServiceRefs for ClusterComponentSpec", func() { + testCase := []struct { + input []string + before []*appsv1alpha1.ClusterComponentSpec + cd *appsv1alpha1.ClusterDefinition + success bool + expected []*appsv1alpha1.ClusterComponentSpec + }{ + {[]string{fmt.Sprintf("name=%s,cluster=%s,namespace=%s", testing.ServiceRefName, testing.ClusterName, testing.Namespace)}, + []*appsv1alpha1.ClusterComponentSpec{ + { + Name: testing.ComponentDefName, + }, + }, + testing.FakeClusterDef(), + true, + []*appsv1alpha1.ClusterComponentSpec{ + { + Name: testing.ComponentDefName, + ServiceRefs: []appsv1alpha1.ServiceRef{ + {Name: testing.ServiceRefName, Cluster: testing.ClusterName, Namespace: testing.Namespace}, + }, + }, + }, + }, {[]string{fmt.Sprintf("name=%s,cluster=%s,namespace=%s", testing.ServiceRefName, testing.ClusterName, testing.Namespace)}, + []*appsv1alpha1.ClusterComponentSpec{ + { + Name: testing.ComponentDefName, + }, + }, + testing.FakeClusterDef(), + true, + []*appsv1alpha1.ClusterComponentSpec{ + { + Name: testing.ComponentDefName, + ServiceRefs: []appsv1alpha1.ServiceRef{ + {Name: testing.ServiceRefName, Cluster: testing.ClusterName, Namespace: testing.Namespace}, + }, + }, + }, + }, + } + + for i := range testCase { + compSpec, err := buildServiceRefs(testCase[i].input, testCase[i].cd, testCase[i].before) + if testCase[i].success { + Expect(err).Should(Succeed()) + Expect(compSpec).Should(Equal(testCase[i].expected)) + } else { + Expect(err).Should(HaveOccurred()) + } + } + + }) }) diff --git a/internal/cli/testing/fake.go b/internal/cli/testing/fake.go index c02cb798701..f71362aee00 100644 --- a/internal/cli/testing/fake.go +++ b/internal/cli/testing/fake.go @@ -60,6 +60,7 @@ const ( SecretName = "fake-secret-conn-credential" StorageClassName = "fake-storage-class" PVCName = "fake-pvc" + ServiceRefName = "fake-serviceRef" KubeBlocksRepoName = "fake-kubeblocks-repo" KubeBlocksChartName = "fake-kubeblocks" @@ -300,6 +301,9 @@ func FakeClusterDef() *appsv1alpha1.ClusterDefinition { ConfigConstraintRef: "mysql8.0-config-constraints", }, }, + ServiceRefDeclarations: []appsv1alpha1.ServiceRefDeclaration{ + FakeServiceRef(ServiceRefName), + }, }, { Name: ExtraComponentDefName, @@ -1081,3 +1085,15 @@ func FakeClusterList() *appsv1alpha1.ClusterList { clusters.Items = append(clusters.Items, *FakeCluster(ClusterName, Namespace)) return clusters } + +func FakeServiceRef(serviceRefName string) appsv1alpha1.ServiceRefDeclaration { + return appsv1alpha1.ServiceRefDeclaration{ + Name: serviceRefName, + ServiceRefDeclarationSpecs: []appsv1alpha1.ServiceRefDeclarationSpec{ + { + ServiceKind: "mysql", + ServiceVersion: "8.0.\\d{1,2}$", + }, + }, + } +}