add help_test & fix componentdef-controller

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
 }
 

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()))
+		})
+	})
 })

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
 }
 

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{