From 7609da212fde2b790db9404bb1d93de96ac719ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=A4=A9=E5=85=83?= Date: Mon, 21 Dec 2020 21:01:08 +0800 Subject: [PATCH] fix workload can not be update when revision enable trait is attached --- .../applicationconfiguration/render.go | 3 +- .../revision_enable_test.go | 340 ++++++++++++++++++ .../applicationconfiguration/suite_test.go | 17 + .../update_trait_test.go | 12 +- 4 files changed, 364 insertions(+), 8 deletions(-) create mode 100644 pkg/controller/core.oam.dev/v1alpha2/applicationconfiguration/revision_enable_test.go diff --git a/pkg/controller/core.oam.dev/v1alpha2/applicationconfiguration/render.go b/pkg/controller/core.oam.dev/v1alpha2/applicationconfiguration/render.go index d267f6a97..542475d3a 100644 --- a/pkg/controller/core.oam.dev/v1alpha2/applicationconfiguration/render.go +++ b/pkg/controller/core.oam.dev/v1alpha2/applicationconfiguration/render.go @@ -280,7 +280,8 @@ func SetWorkloadInstanceName(traitDefs []v1alpha2.TraitDefinition, w *unstructur componentLastRevision := c.Status.LatestRevision.Name // if workload exists, check the revision label, we will not change the name if workload exists and no revision changed if existingWorkload != nil && existingWorkload.GetLabels()[oam.LabelAppComponentRevision] == componentLastRevision { - return nil + // using the existing name + return errors.Wrapf(pv.SetString(instanceNamePath, existingWorkload.GetName()), errSetValueForField, instanceNamePath, c.Status.LatestRevision) } // if revisionEnabled and the running workload's revision isn't equal to the component's latest reversion, diff --git a/pkg/controller/core.oam.dev/v1alpha2/applicationconfiguration/revision_enable_test.go b/pkg/controller/core.oam.dev/v1alpha2/applicationconfiguration/revision_enable_test.go new file mode 100644 index 000000000..f908d16d3 --- /dev/null +++ b/pkg/controller/core.oam.dev/v1alpha2/applicationconfiguration/revision_enable_test.go @@ -0,0 +1,340 @@ +/* +Copyright 2020 The KubeVela Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package applicationconfiguration + +import ( + "context" + "fmt" + "strconv" + "time" + + v1 "k8s.io/api/apps/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + "github.com/oam-dev/kubevela/apis/core.oam.dev/v1alpha2" + "github.com/oam-dev/kubevela/pkg/oam/util" +) + +var _ = Describe("Test ApplicationConfiguration Component Revision Enabled trait", func() { + const ( + namespace = "revision-enable-test" + appName = "revision-test-app" + compName = "revision-test-comp" + ) + var ( + ctx = context.Background() + wr v1.Deployment + component v1alpha2.Component + appConfig v1alpha2.ApplicationConfiguration + appConfigKey = client.ObjectKey{ + Name: appName, + Namespace: namespace, + } + req = reconcile.Request{NamespacedName: appConfigKey} + ns = corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: namespace, + }, + } + ) + + BeforeEach(func() {}) + + AfterEach(func() { + // delete the namespace with all its resources + Expect(k8sClient.Delete(ctx, &ns, client.PropagationPolicy(metav1.DeletePropagationForeground))). + Should(SatisfyAny(BeNil(), &util.NotFoundMatcher{})) + }) + + It("revision enabled should create workload with revisionName and work upgrade with new revision successfully", func() { + + getDeploy := func(image string) *v1.Deployment { + return &v1.Deployment{ + TypeMeta: metav1.TypeMeta{ + Kind: "Deployment", + APIVersion: "apps/v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: namespace, + }, + Spec: v1.DeploymentSpec{ + Selector: &metav1.LabelSelector{MatchLabels: map[string]string{ + "app": compName, + }}, + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{ + "app": compName, + }}, + Spec: corev1.PodSpec{Containers: []corev1.Container{{ + Name: "wordpress", + Image: image, + Ports: []corev1.ContainerPort{ + { + Name: "wordpress", + ContainerPort: 80, + }, + }, + }, + }}}, + }, + } + } + component = v1alpha2.Component{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "core.oam.dev/v1alpha2", + Kind: "Component", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: compName, + Namespace: namespace, + }, + Spec: v1alpha2.ComponentSpec{ + Workload: runtime.RawExtension{ + Object: getDeploy("wordpress:4.6.1-apache"), + }, + }, + } + appConfig = v1alpha2.ApplicationConfiguration{ + ObjectMeta: metav1.ObjectMeta{ + Name: appName, + Namespace: namespace, + }, + } + + By("Create namespace") + Eventually( + func() error { + return k8sClient.Create(ctx, &ns) + }, + time.Second*3, time.Millisecond*300).Should(SatisfyAny(BeNil(), &util.AlreadyExistMatcher{})) + + By("Create Component") + Expect(k8sClient.Create(ctx, &component)).Should(Succeed()) + cmpV1 := &v1alpha2.Component{} + Expect(k8sClient.Get(ctx, client.ObjectKey{Namespace: namespace, Name: compName}, cmpV1)).Should(Succeed()) + + By("component handler will automatically create controller revision") + Expect(componentHandler.createControllerRevision(cmpV1, cmpV1)).Should(BeTrue()) + var crList v1.ControllerRevisionList + By("Check controller revision created successfully") + Eventually(func() error { + labels := &metav1.LabelSelector{ + MatchLabels: map[string]string{ + ControllerRevisionComponentLabel: compName, + }, + } + selector, err := metav1.LabelSelectorAsSelector(labels) + if err != nil { + return err + } + err = k8sClient.List(ctx, &crList, &client.ListOptions{ + LabelSelector: selector, + }) + if err != nil { + return err + } + if len(crList.Items) != 1 { + return fmt.Errorf("want only 1 revision created but got %d", len(crList.Items)) + } + return nil + }, time.Second, 300*time.Millisecond).Should(BeNil()) + + By("Create an ApplicationConfiguration") + appConfig = v1alpha2.ApplicationConfiguration{ + ObjectMeta: metav1.ObjectMeta{ + Name: appName, + Namespace: namespace, + }, + Spec: v1alpha2.ApplicationConfigurationSpec{Components: []v1alpha2.ApplicationConfigurationComponent{ + { + ComponentName: compName, + RevisionName: compName + "-v1", + Traits: []v1alpha2.ComponentTrait{ + { + Trait: runtime.RawExtension{Object: &unstructured.Unstructured{Object: map[string]interface{}{ + "apiVersion": "example.com/v1", + "kind": "Foo", + "metadata": map[string]interface{}{ + "labels": map[string]interface{}{ + "trait.oam.dev/type": "rollout-revision", + }, + }, + "spec": map[string]interface{}{ + "key": "test1", + }, + }}}, + }, + }, + }, + }}, + } + By("Creat appConfig & check successfully") + Expect(k8sClient.Create(ctx, &appConfig)).Should(Succeed()) + Eventually(func() error { + return k8sClient.Get(ctx, appConfigKey, &appConfig) + }, time.Second, 300*time.Millisecond).Should(BeNil()) + + By("Reconcile") + reconcileRetry(reconciler, req) + + By("Check workload created successfully") + Eventually(func() error { + var workloadKey = client.ObjectKey{Namespace: namespace, Name: compName + "-v1"} + return k8sClient.Get(ctx, workloadKey, &wr) + }, time.Second, 300*time.Millisecond).Should(BeNil()) + By("Check workload should only have 1 generation") + Expect(wr.GetGeneration()).Should(BeEquivalentTo(1)) + + By("Check reconcile again and no error will happen") + reconcileRetry(reconciler, req) + By("Check appconfig condition should not have error") + Eventually(func() string { + By("Reconcile again and should not have error") + reconcileRetry(reconciler, req) + err := k8sClient.Get(ctx, appConfigKey, &appConfig) + if err != nil { + return err.Error() + } + if len(appConfig.Status.Conditions) != 1 { + return "condition len should be 1 but now is " + strconv.Itoa(len(appConfig.Status.Conditions)) + } + return string(appConfig.Status.Conditions[0].Reason) + }, 3*time.Second, 300*time.Millisecond).Should(BeEquivalentTo("ReconcileSuccess")) + + By("Check workload will not update when reconcile again but no appconfig changed") + Eventually(func() error { + var workloadKey = client.ObjectKey{Namespace: namespace, Name: compName + "-v1"} + return k8sClient.Get(ctx, workloadKey, &wr) + }, time.Second, 300*time.Millisecond).Should(BeNil()) + By("Check workload should should still be 1 generation") + Expect(wr.GetGeneration()).Should(BeEquivalentTo(1)) + By("Check trait was created as expected") + var tr unstructured.Unstructured + Eventually(func() error { + tr.SetAPIVersion("example.com/v1") + tr.SetKind("Foo") + var traitKey = client.ObjectKey{Namespace: namespace, Name: appConfig.Status.Workloads[0].Traits[0].Reference.Name} + return k8sClient.Get(ctx, traitKey, &tr) + }, time.Second, 300*time.Millisecond).Should(BeNil()) + Expect(tr.Object["spec"]).Should(BeEquivalentTo(map[string]interface{}{"key": "test1"})) + + By("===================================== Start to Update =========================================") + cmpV2 := &v1alpha2.Component{} + Expect(k8sClient.Get(ctx, client.ObjectKey{Namespace: namespace, Name: compName}, cmpV2)).Should(Succeed()) + cmpV2.Spec.Workload = runtime.RawExtension{ + Object: getDeploy("wordpress:v2"), + } + By("Update Component") + Expect(k8sClient.Update(ctx, cmpV2)).Should(Succeed()) + By("component handler will automatically create a ne controller revision") + Expect(componentHandler.createControllerRevision(cmpV2, cmpV2)).Should(BeTrue()) + By("Check controller revision created successfully") + Eventually(func() error { + labels := &metav1.LabelSelector{ + MatchLabels: map[string]string{ + ControllerRevisionComponentLabel: compName, + }, + } + selector, err := metav1.LabelSelectorAsSelector(labels) + if err != nil { + return err + } + err = k8sClient.List(ctx, &crList, &client.ListOptions{ + LabelSelector: selector, + }) + if err != nil { + return err + } + if len(crList.Items) != 2 { + return fmt.Errorf("there should be exactly 2 revision created but got %d", len(crList.Items)) + } + return nil + }, time.Second, 300*time.Millisecond).Should(BeNil()) + + By("Update appConfig & check successfully") + appConfig.Spec.Components[0].RevisionName = compName + "-v2" + appConfig.Spec.Components[0].Traits[0].Trait = runtime.RawExtension{Object: &unstructured.Unstructured{Object: map[string]interface{}{ + "apiVersion": "example.com/v1", + "kind": "Foo", + "metadata": map[string]interface{}{ + "labels": map[string]interface{}{ + "trait.oam.dev/type": "rollout-revision", + }, + }, + "spec": map[string]interface{}{ + "key": "test2", + }, + }}} + Expect(k8sClient.Update(ctx, &appConfig)).Should(Succeed()) + Eventually(func() error { + return k8sClient.Get(ctx, appConfigKey, &appConfig) + }, time.Second, 300*time.Millisecond).Should(BeNil()) + By("Reconcile for new revision") + reconcileRetry(reconciler, req) + + By("Check new revision workload created successfully") + Eventually(func() error { + var workloadKey = client.ObjectKey{Namespace: namespace, Name: compName + "-v2"} + return k8sClient.Get(ctx, workloadKey, &wr) + }, time.Second, 300*time.Millisecond).Should(BeNil()) + By("Check the new workload should only have 1 generation") + Expect(wr.GetGeneration()).Should(BeEquivalentTo(1)) + Expect(wr.Spec.Template.Spec.Containers[0].Image).Should(BeEquivalentTo("wordpress:v2")) + + By("Check the old workload is still there with no change") + Eventually(func() error { + var workloadKey = client.ObjectKey{Namespace: namespace, Name: compName + "-v1"} + return k8sClient.Get(ctx, workloadKey, &wr) + }, time.Second, 300*time.Millisecond).Should(BeNil()) + By("Check the new workload should only have 1 generation") + Expect(wr.GetGeneration()).Should(BeEquivalentTo(1)) + + By("Check reconcile again and no error will happen") + reconcileRetry(reconciler, req) + By("Check appconfig condition should not have error") + Eventually(func() string { + By("Once more Reconcile and should not have error") + reconcileRetry(reconciler, req) + err := k8sClient.Get(ctx, appConfigKey, &appConfig) + if err != nil { + return err.Error() + } + if len(appConfig.Status.Conditions) != 1 { + return "condition len should be 1 but now is " + strconv.Itoa(len(appConfig.Status.Conditions)) + } + return string(appConfig.Status.Conditions[0].Reason) + }, 3*time.Second, 300*time.Millisecond).Should(BeEquivalentTo("ReconcileSuccess")) + By("Check trait was updated as expected") + Eventually(func() error { + tr.SetAPIVersion("example.com/v1") + tr.SetKind("Foo") + var traitKey = client.ObjectKey{Namespace: namespace, Name: appConfig.Status.Workloads[0].Traits[0].Reference.Name} + return k8sClient.Get(ctx, traitKey, &tr) + }, time.Second, 300*time.Millisecond).Should(BeNil()) + Expect(tr.Object["spec"]).Should(BeEquivalentTo(map[string]interface{}{"key": "test2"})) + }) + +}) diff --git a/pkg/controller/core.oam.dev/v1alpha2/applicationconfiguration/suite_test.go b/pkg/controller/core.oam.dev/v1alpha2/applicationconfiguration/suite_test.go index 1264b519f..e89044353 100644 --- a/pkg/controller/core.oam.dev/v1alpha2/applicationconfiguration/suite_test.go +++ b/pkg/controller/core.oam.dev/v1alpha2/applicationconfiguration/suite_test.go @@ -37,6 +37,7 @@ import ( ) var reconciler *OAMApplicationReconciler +var componentHandler *ComponentHandler var mgrclose chan struct{} var testEnv *envtest.Environment var cfg *rest.Config @@ -156,6 +157,7 @@ var _ = BeforeSuite(func(done Done) { Expect(mapping.Resource.Resource).Should(Equal("foo")) reconciler = NewReconciler(mgr, dm, WithLogger(logging.NewLogrLogger(ctrl.Log.WithName("suit-test-appconfig")))) + componentHandler = &ComponentHandler{Client: k8sClient, RevisionLimit: 100, Logger: logging.NewLogrLogger(ctrl.Log.WithName("component-handler"))} By("Creating workload definition and trait definition") wd := v1alpha2.WorkloadDefinition{ @@ -178,10 +180,25 @@ var _ = BeforeSuite(func(done Done) { }, }, } + + rollout := v1alpha2.TraitDefinition{ + ObjectMeta: metav1.ObjectMeta{ + Name: "rollout-revision", + }, + Spec: v1alpha2.TraitDefinitionSpec{ + Reference: v1alpha2.DefinitionReference{ + Name: "foo.example.com", + }, + RevisionEnabled: true, + }, + } + // For some reason, WorkloadDefinition is created as a Cluster scope object Expect(k8sClient.Create(ctx, &wd)).Should(SatisfyAny(BeNil(), &util.AlreadyExistMatcher{})) // For some reason, TraitDefinition is created as a Cluster scope object Expect(k8sClient.Create(ctx, &td)).Should(SatisfyAny(BeNil(), &util.AlreadyExistMatcher{})) + // rollout trait is used for revisionEnable case test + Expect(k8sClient.Create(ctx, &rollout)).Should(SatisfyAny(BeNil(), &util.AlreadyExistMatcher{})) close(done) }, 300) diff --git a/pkg/controller/core.oam.dev/v1alpha2/applicationconfiguration/update_trait_test.go b/pkg/controller/core.oam.dev/v1alpha2/applicationconfiguration/update_trait_test.go index 1819f4f03..3b6198977 100644 --- a/pkg/controller/core.oam.dev/v1alpha2/applicationconfiguration/update_trait_test.go +++ b/pkg/controller/core.oam.dev/v1alpha2/applicationconfiguration/update_trait_test.go @@ -13,14 +13,12 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" - - "github.com/oam-dev/kubevela/apis/core.oam.dev/v1alpha2" - - "github.com/oam-dev/kubevela/pkg/oam/util" - "sigs.k8s.io/controller-runtime/pkg/client" logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/reconcile" + + "github.com/oam-dev/kubevela/apis/core.oam.dev/v1alpha2" + "github.com/oam-dev/kubevela/pkg/oam/util" ) var _ = Describe("Test Updating-apply trait in an ApplicationConfiguration", func() { @@ -30,7 +28,7 @@ var _ = Describe("Test Updating-apply trait in an ApplicationConfiguration", fun compName = "example-comp" fakeTraitCRDName = "bars.example.com" fakeTraitGroup = "example.com" - fakeTraiitKind = "Bar" + fakeTraitKind = "Bar" ) var ( ctx = context.Background() @@ -124,7 +122,7 @@ var _ = Describe("Test Updating-apply trait in an ApplicationConfiguration", fun Spec: crdv1.CustomResourceDefinitionSpec{ Group: fakeTraitGroup, Names: crdv1.CustomResourceDefinitionNames{ - Kind: fakeTraiitKind, + Kind: fakeTraitKind, ListKind: "BarList", Plural: "bars", Singular: "bar",