From 3345846f78f212450b39c4783d4e5b336ca69bef Mon Sep 17 00:00:00 2001 From: yangsoon Date: Tue, 16 Mar 2021 15:37:16 +0800 Subject: [PATCH] add help_test & fix componentdef-controller --- .../componentdefinition_controller.go | 58 ++++--- .../componentdefinition_controller_test.go | 164 +++++++++++++++++- pkg/oam/util/helper.go | 8 +- pkg/oam/util/helper_test.go | 41 +++++ 4 files changed, 240 insertions(+), 31 deletions(-) diff --git a/pkg/controller/core.oam.dev/v1alpha2/core/components/componentdefinition/componentdefinition_controller.go b/pkg/controller/core.oam.dev/v1alpha2/core/components/componentdefinition/componentdefinition_controller.go index 7c6ed9e72..7c5dcb508 100644 --- a/pkg/controller/core.oam.dev/v1alpha2/core/components/componentdefinition/componentdefinition_controller.go +++ b/pkg/controller/core.oam.dev/v1alpha2/core/components/componentdefinition/componentdefinition_controller.go @@ -67,30 +67,9 @@ func (r *Reconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { return ctrl.Result{}, nil } - // if Workload.Type is empty, we need create a WorkloadDefinition - if componentDefinition.Spec.Workload.Type == "" { - workloadDefinition := new(v1alpha2.WorkloadDefinition) - newCd := componentDefinition.DeepCopy() - if err := util.ConvertComponentDef2WorkloadDef(newCd, workloadDefinition); err != nil { - klog.ErrorS(err, "cannot convert ComponentDefinition") - r.record.Event(&componentDefinition, event.Warning("cannot convert ComponentDefinition", err)) - return ctrl.Result{}, util.PatchCondition(ctx, r, &componentDefinition, - cpv1alpha1.ReconcileError(fmt.Errorf(util.ErrConvertComponentDefinition, componentDefinition.Name, err))) - } - owners := []metav1.OwnerReference{{ - APIVersion: v1alpha2.SchemeGroupVersion.String(), - Kind: v1alpha2.ComponentDefinitionKind, - Name: componentDefinition.Name, - UID: componentDefinition.UID, - Controller: pointer.BoolPtr(true), - }} - workloadDefinition.SetOwnerReferences(owners) - if err := r.Create(ctx, workloadDefinition); err != nil { - klog.ErrorS(err, "cannot create converted WorkloadDefinition") - r.record.Event(&componentDefinition, event.Warning("cannot create converted Workload", err)) - return ctrl.Result{}, util.PatchCondition(ctx, r, &componentDefinition, - cpv1alpha1.ReconcileError(fmt.Errorf(util.ErrCreateConvertedWorklaodDefinition, workloadDefinition.Name, err))) - } + // if Workload.Type is not empty, means componentdefinition refer to an already existing workloaddefinition + if componentDefinition.Spec.Workload.Type != "" { + return ctrl.Result{}, nil } var def utils.CapabilityComponentDefinition @@ -109,6 +88,37 @@ func (r *Reconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { return ctrl.Result{}, nil } klog.Info("Successfully stored Capability Schema in ConfigMap") + + // if Workload.Type is empty, we need create a WorkloadDefinition + if err := r.Get(ctx, req.NamespacedName, &v1alpha2.WorkloadDefinition{}); err == nil { + klog.Infof("WorkloadDefinition: %s already exists", componentDefinition.Name) + return ctrl.Result{}, nil + } + + workloadDefinition := new(v1alpha2.WorkloadDefinition) + newCd := componentDefinition.DeepCopy() + if err := util.ConvertComponentDef2WorkloadDef(newCd, workloadDefinition); err != nil { + klog.ErrorS(err, "cannot convert ComponentDefinition") + r.record.Event(&componentDefinition, event.Warning("cannot convert ComponentDefinition", err)) + return ctrl.Result{}, util.PatchCondition(ctx, r, &componentDefinition, + cpv1alpha1.ReconcileError(fmt.Errorf(util.ErrConvertComponentDefinition, componentDefinition.Name, err))) + } + owners := []metav1.OwnerReference{{ + APIVersion: v1alpha2.SchemeGroupVersion.String(), + Kind: v1alpha2.ComponentDefinitionKind, + Name: componentDefinition.Name, + UID: componentDefinition.UID, + Controller: pointer.BoolPtr(true), + }} + workloadDefinition.SetOwnerReferences(owners) + if err := r.Create(ctx, workloadDefinition); err != nil { + klog.ErrorS(err, "cannot create converted WorkloadDefinition") + r.record.Event(&componentDefinition, event.Warning("cannot create converted Workload", err)) + return ctrl.Result{}, util.PatchCondition(ctx, r, &componentDefinition, + cpv1alpha1.ReconcileError(fmt.Errorf(util.ErrCreateConvertedWorklaodDefinition, workloadDefinition.Name, err))) + } + + klog.InfoS("Successfully create WorkloadDefinition", "name", workloadDefinition.Name) return ctrl.Result{}, nil } diff --git a/pkg/controller/core.oam.dev/v1alpha2/core/components/componentdefinition/componentdefinition_controller_test.go b/pkg/controller/core.oam.dev/v1alpha2/core/components/componentdefinition/componentdefinition_controller_test.go index 1cb719c12..dc52dca85 100644 --- a/pkg/controller/core.oam.dev/v1alpha2/core/components/componentdefinition/componentdefinition_controller_test.go +++ b/pkg/controller/core.oam.dev/v1alpha2/core/components/componentdefinition/componentdefinition_controller_test.go @@ -35,7 +35,7 @@ import ( "github.com/oam-dev/kubevela/pkg/oam/util" ) -var _ = Describe("Apply ComponentDefinition to store its schema to ConfigMap Test", func() { +var _ = Describe("Test ComponentDefinition Controller", func() { ctx := context.Background() var ns corev1.Namespace @@ -52,7 +52,9 @@ metadata: definition.oam.dev/description: "test" spec: workload: - type: deployments.app + definition: + apiVersion: apps/v1 + kind: Deployment schematic: cue: template: | @@ -114,7 +116,9 @@ metadata: definition.oam.dev/description: "test" spec: workload: - type: deployments.app + definition: + apiVersion: apps/v1 + kind: Deployment schematic: cue: template: | @@ -205,7 +209,9 @@ metadata: definition.oam.dev/description: "test" spec: workload: - type: deployments.app + definition: + apiVersion: apps/v1 + kind: Deployment schematic: cue: template: | @@ -292,7 +298,9 @@ metadata: definition.oam.dev/description: "test" spec: workload: - type: deployments.app + definition: + apiVersion: apps/v1 + kind: Deployment schematic: cue: template: | @@ -327,4 +335,150 @@ spec: Expect(k8sClient.Get(ctx, client.ObjectKey{Name: invalidComponentDefinitionName, Namespace: namespace}, gotComponentDefinition)).Should(BeNil()) }) }) + + Context("When the ComponentDefinition only container Workload.Definition, should create a WorkloadDefinition", func() { + var componentDefinitionName = "cd-with-workload-definition" + var namespace = "default" + req := reconcile.Request{NamespacedName: client.ObjectKey{Name: componentDefinitionName, Namespace: namespace}} + + It("Applying ComponentDefinition with Workload.Definition", func() { + By("Apply ComponentDefinition") + var validComponentDefinition = ` +apiVersion: core.oam.dev/v1alpha2 +kind: ComponentDefinition +metadata: + name: cd-with-workload-definition + annotations: + definition.oam.dev/description: "test" +spec: + workload: + definition: + apiVersion: apps/v1 + kind: Deployment + schematic: + cue: + template: | + output: { + apiVersion: "apps/v1" + kind: "Deployment" + spec: { + selector: matchLabels: { + "app.oam.dev/component": context.name + } + + template: { + metadata: labels: { + "app.oam.dev/component": context.name + } + + spec: { + containers: [{ + name: context.name + image: parameter.image + + if parameter["cmd"] != _|_ { + command: parameter.cmd + } + }] + } + } + } + } + parameter: { + // +usage=Which image would you like to use for your service + // +short=i + image: string + + // +usage=Commands to run in the container + cmd?: [...string] + } +` + var def v1alpha2.ComponentDefinition + Expect(yaml.Unmarshal([]byte(validComponentDefinition), &def)).Should(BeNil()) + def.Namespace = namespace + Expect(k8sClient.Create(ctx, &def)).Should(Succeed()) + + By("Check whether WorkloadDefinition is created") + reconcileRetry(&r, req) + var wd v1alpha2.WorkloadDefinition + var wdName = componentDefinitionName + Eventually(func() bool { + err := k8sClient.Get(ctx, client.ObjectKey{Namespace: namespace, Name: wdName}, &wd) + return err == nil + }, 10*time.Second, time.Second).Should(BeTrue()) + Expect(wd.Name).Should(Equal(def.Name)) + Expect(wd.Spec.Schematic.CUE).Should(Equal(def.Spec.Schematic.CUE)) + convertRef, err := util.ConvertWorkloadGVK2Definition(def.Spec.Workload.Definition) + Expect(err).Should(BeNil()) + Expect(wd.Spec.Reference).Should(Equal(convertRef)) + }) + }) + + Context("When the ComponentDefinition container Workload.Type, shouldn't create a WorkloadDefinition", func() { + var componentDefinitionName = "cd-with-workload-type" + var namespace = "default" + req := reconcile.Request{NamespacedName: client.ObjectKey{Name: componentDefinitionName, Namespace: namespace}} + + It("Applying ComponentDefinition with Workload.Type", func() { + By("Apply ComponentDefinition") + var validComponentDefinition = ` +apiVersion: core.oam.dev/v1alpha2 +kind: ComponentDefinition +metadata: + name: cd-with-workload-type + annotations: + definition.oam.dev/description: "test" +spec: + workload: + type: deployments.app + schematic: + cue: + template: | + output: { + apiVersion: "apps/v1" + kind: "Deployment" + spec: { + selector: matchLabels: { + "app.oam.dev/component": context.name + } + + template: { + metadata: labels: { + "app.oam.dev/component": context.name + } + + spec: { + containers: [{ + name: context.name + image: parameter.image + + if parameter["cmd"] != _|_ { + command: parameter.cmd + } + }] + } + } + } + } + parameter: { + // +usage=Which image would you like to use for your service + // +short=i + image: string + + // +usage=Commands to run in the container + cmd?: [...string] + } +` + var def v1alpha2.ComponentDefinition + Expect(yaml.Unmarshal([]byte(validComponentDefinition), &def)).Should(BeNil()) + def.Namespace = namespace + Expect(k8sClient.Create(ctx, &def)).Should(Succeed()) + + By("Check whether WorkloadDefinition is created") + reconcileRetry(&r, req) + var wd v1alpha2.WorkloadDefinition + var wdName = componentDefinitionName + Expect(k8sClient.Get(ctx, client.ObjectKey{Namespace: namespace, Name: wdName}, &wd)).Should(Not(Succeed())) + }) + }) }) diff --git a/pkg/oam/util/helper.go b/pkg/oam/util/helper.go index 8fac3912c..192741b80 100644 --- a/pkg/oam/util/helper.go +++ b/pkg/oam/util/helper.go @@ -428,8 +428,12 @@ func ConvertWorkloadGVK2Definition(def v1alpha2.WorkloadGVK) (v1alpha2.Definitio return reference, err } resource := strings.ToLower(def.Kind) + "s" - reference.Name = resource + "." + gv.Group - reference.Version = gv.Version + if gv.Group == "" { + reference.Name = resource + "." + gv.Version + } else { + reference.Name = resource + "." + gv.Group + reference.Version = gv.Version + } return reference, nil } diff --git a/pkg/oam/util/helper_test.go b/pkg/oam/util/helper_test.go index 5d7187aa7..903f04084 100644 --- a/pkg/oam/util/helper_test.go +++ b/pkg/oam/util/helper_test.go @@ -811,6 +811,47 @@ func TestGetGVKFromDef(t *testing.T) { }, gvk) } +func TestConvertWorkloadGVK2Def(t *testing.T) { + type want struct { + ref v1alpha2.DefinitionReference + err error + } + convertErr := fmt.Errorf("unexpected GroupVersion string: %v", "apps/v1/") + + cases := []struct { + testName string + workloadGVK v1alpha2.WorkloadGVK + want want + }{{ + testName: "expected GVK with version", + workloadGVK: v1alpha2.WorkloadGVK{APIVersion: "apps/v1", Kind: "Deployment"}, + want: want{ + ref: v1alpha2.DefinitionReference{Name: "deployments.apps", Version: "v1"}, + err: nil, + }, + }, { + testName: "expected GVK without version", + workloadGVK: v1alpha2.WorkloadGVK{APIVersion: "apps", Kind: "Deployment"}, + want: want{ + ref: v1alpha2.DefinitionReference{Name: "deployments.apps", Version: ""}, + err: nil, + }, + }, { + testName: "unexpected GVK", + workloadGVK: v1alpha2.WorkloadGVK{APIVersion: "apps/v1/", Kind: "Deployment"}, + want: want{ + ref: v1alpha2.DefinitionReference{}, + err: convertErr, + }, + }} + for _, tc := range cases { + ref, err := util.ConvertWorkloadGVK2Definition(tc.workloadGVK) + t.Log(fmt.Sprint("Running test: ", tc.testName)) + assert.Equal(t, tc.want.err, err) + assert.Equal(t, tc.want.ref, ref) + } +} + func TestGenTraitName(t *testing.T) { mts := v1alpha2.ManualScalerTrait{ ObjectMeta: metav1.ObjectMeta{