From de37545a12d8f8bfcd0660fe535479d1e60a68de Mon Sep 17 00:00:00 2001 From: Somefive Date: Thu, 30 Jun 2022 16:22:46 +0800 Subject: [PATCH] Feat: disable component revision for component wo rollout (#4281) Signed-off-by: Somefive --- charts/vela-core/README.md | 1 + .../templates/kubevela-controller.yaml | 1 + charts/vela-core/values.yaml | 4 +++ .../v1alpha2/application/generator.go | 2 +- .../v1alpha2/application/revision.go | 32 +++++++++++++++++++ .../v1alpha2/application/suite_test.go | 3 ++ pkg/features/controller_features.go | 3 ++ .../multicluster_test.go | 8 ----- test/e2e-test/application_test.go | 22 ------------- 9 files changed, 45 insertions(+), 31 deletions(-) diff --git a/charts/vela-core/README.md b/charts/vela-core/README.md index ea8b718b5..235ef8537 100644 --- a/charts/vela-core/README.md +++ b/charts/vela-core/README.md @@ -93,6 +93,7 @@ helm install --create-namespace -n vela-system kubevela kubevela/vela-core --wai | `optimize.enableInMemoryWorkflowContext` | Optimize workflow by use in-memory context. | `false` | | `optimize.disableResourceApplyDoubleCheck` | Optimize workflow by ignoring resource double check after apply. | `false` | | `optimize.enableResourceTrackerDeleteOnlyTrigger` | Optimize resourcetracker by only trigger reconcile when resourcetracker is deleted. | `true` | +| `featureGates.enableLegacyComponentRevision` | if disabled, only component with rollout trait will create component revisions | `false` | ### MultiCluster parameters diff --git a/charts/vela-core/templates/kubevela-controller.yaml b/charts/vela-core/templates/kubevela-controller.yaml index 01b738a1c..263d4b0fb 100644 --- a/charts/vela-core/templates/kubevela-controller.yaml +++ b/charts/vela-core/templates/kubevela-controller.yaml @@ -217,6 +217,7 @@ spec: - "--max-workflow-step-error-retry-times={{ .Values.workflow.step.errorRetryTimes }}" - "--feature-gates=EnableSuspendOnFailure={{- .Values.workflow.enableSuspendOnFailure | toString -}}" - "--feature-gates=AuthenticateApplication={{- .Values.authentication.enabled | toString -}}" + - "--feature-gates=LegacyComponentRevision={{- .Values.featureGates.enableLegacyComponentRevision | toString -}}" {{ if .Values.authentication.enabled }} {{ if .Values.authentication.withUser }} - "--authentication-with-user" diff --git a/charts/vela-core/values.yaml b/charts/vela-core/values.yaml index 18c971969..c88a6edaa 100644 --- a/charts/vela-core/values.yaml +++ b/charts/vela-core/values.yaml @@ -109,6 +109,10 @@ optimize: disableResourceApplyDoubleCheck: false enableResourceTrackerDeleteOnlyTrigger: true +##@param featureGates.enableLegacyComponentRevision if disabled, only component with rollout trait will create component revisions +featureGates: + enableLegacyComponentRevision: false + ## @section MultiCluster parameters ## @param multicluster.enabled Whether to enable multi-cluster diff --git a/pkg/controller/core.oam.dev/v1alpha2/application/generator.go b/pkg/controller/core.oam.dev/v1alpha2/application/generator.go index 8e3b5243d..3c03da59f 100644 --- a/pkg/controller/core.oam.dev/v1alpha2/application/generator.go +++ b/pkg/controller/core.oam.dev/v1alpha2/application/generator.go @@ -334,7 +334,7 @@ func (h *AppHandler) prepareWorkloadAndManifests(ctx context.Context, if err := af.SetOAMContract(manifest); err != nil { return nil, nil, errors.WithMessage(err, "SetOAMContract") } - if err := h.HandleComponentsRevision(ctx, []*types.ComponentManifest{manifest}); err != nil { + if err := h.HandleComponentsRevision(contextWithComponent(ctx, &comp), []*types.ComponentManifest{manifest}); err != nil { return nil, nil, errors.WithMessage(err, "HandleComponentsRevision") } diff --git a/pkg/controller/core.oam.dev/v1alpha2/application/revision.go b/pkg/controller/core.oam.dev/v1alpha2/application/revision.go index a190632d2..85d4b277d 100644 --- a/pkg/controller/core.oam.dev/v1alpha2/application/revision.go +++ b/pkg/controller/core.oam.dev/v1alpha2/application/revision.go @@ -31,6 +31,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ktypes "k8s.io/apimachinery/pkg/types" + utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/klog/v2" "k8s.io/utils/pointer" "sigs.k8s.io/controller-runtime/pkg/client" @@ -46,6 +47,7 @@ import ( "github.com/oam-dev/kubevela/pkg/component" "github.com/oam-dev/kubevela/pkg/controller/utils" "github.com/oam-dev/kubevela/pkg/cue/model" + "github.com/oam-dev/kubevela/pkg/features" monitorContext "github.com/oam-dev/kubevela/pkg/monitor/context" "github.com/oam-dev/kubevela/pkg/monitor/metrics" "github.com/oam-dev/kubevela/pkg/multicluster" @@ -69,8 +71,35 @@ const ( ManifestKeyScopes = "Scopes" // ComponentRevisionNamespaceContextKey is the key in context that defines the override namespace of component revision ComponentRevisionNamespaceContextKey = contextKey("component-revision-namespace") + // ComponentContextKey is the key in context that records the component + ComponentContextKey = contextKey("component") ) +const rolloutTraitName = "rollout" + +// contextWithComponent records ApplicationComponent in context +func contextWithComponent(ctx context.Context, component *common.ApplicationComponent) context.Context { + return context.WithValue(ctx, ComponentContextKey, component) +} + +// componentInContext extract ApplicationComponent from context +func componentInContext(ctx context.Context) *common.ApplicationComponent { + comp, _ := ctx.Value(ComponentContextKey).(*common.ApplicationComponent) + return comp +} + +func _containsRolloutTrait(ctx context.Context) bool { + comp := componentInContext(ctx) + if comp != nil { + for _, trait := range comp.Traits { + if trait.Type == rolloutTraitName { + return true + } + } + } + return false +} + var ( // DisableAllComponentRevision disable component revision creation DisableAllComponentRevision = false @@ -709,6 +738,9 @@ func (h *AppHandler) createControllerRevision(ctx context.Context, cm *types.Com Data: *util.Object2RawExtension(comp), } common.NewOAMObjectReferenceFromObject(cm.StandardWorkload).AddLabelsToObject(cr) + if !utilfeature.DefaultMutableFeatureGate.Enabled(features.LegacyComponentRevision) && !_containsRolloutTrait(ctx) { + return nil + } return h.resourceKeeper.DispatchComponentRevision(ctx, cr) } diff --git a/pkg/controller/core.oam.dev/v1alpha2/application/suite_test.go b/pkg/controller/core.oam.dev/v1alpha2/application/suite_test.go index e0d31c27a..37b28faad 100644 --- a/pkg/controller/core.oam.dev/v1alpha2/application/suite_test.go +++ b/pkg/controller/core.oam.dev/v1alpha2/application/suite_test.go @@ -37,6 +37,7 @@ import ( "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" "k8s.io/utils/pointer" @@ -52,6 +53,7 @@ import ( "github.com/oam-dev/kubevela/apis/standard.oam.dev/v1alpha1" "github.com/oam-dev/kubevela/pkg/appfile" "github.com/oam-dev/kubevela/pkg/cue/packages" + "github.com/oam-dev/kubevela/pkg/features" "github.com/oam-dev/kubevela/pkg/oam/discoverymapper" // +kubebuilder:scaffold:imports ) @@ -165,6 +167,7 @@ var _ = BeforeSuite(func(done Done) { Expect(err).NotTo(HaveOccurred()) }() close(done) + Expect(utilfeature.DefaultMutableFeatureGate.Set(fmt.Sprintf("%s=true", features.LegacyComponentRevision))).Should(Succeed()) }, 120) var _ = AfterSuite(func() { diff --git a/pkg/features/controller_features.go b/pkg/features/controller_features.go index 540288613..7ab53d61e 100644 --- a/pkg/features/controller_features.go +++ b/pkg/features/controller_features.go @@ -35,6 +35,8 @@ const ( LegacyResourceTrackerGC featuregate.Feature = "LegacyResourceTrackerGC" // EnableSuspendOnFailure enable suspend on workflow failure EnableSuspendOnFailure featuregate.Feature = "EnableSuspendOnFailure" + // LegacyComponentRevision if enabled, create component revision even no rollout trait attached + LegacyComponentRevision featuregate.Feature = "LegacyComponentRevision" // Edge Features @@ -48,6 +50,7 @@ var defaultFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{ DeprecatedObjectLabelSelector: {Default: false, PreRelease: featuregate.Alpha}, LegacyResourceTrackerGC: {Default: false, PreRelease: featuregate.Beta}, EnableSuspendOnFailure: {Default: false, PreRelease: featuregate.Alpha}, + LegacyComponentRevision: {Default: false, PreRelease: featuregate.Alpha}, AuthenticateApplication: {Default: false, PreRelease: featuregate.Alpha}, } diff --git a/test/e2e-multicluster-test/multicluster_test.go b/test/e2e-multicluster-test/multicluster_test.go index 58ce8c480..299bf6128 100644 --- a/test/e2e-multicluster-test/multicluster_test.go +++ b/test/e2e-multicluster-test/multicluster_test.go @@ -255,10 +255,6 @@ var _ = Describe("Test multicluster scenario", func() { deploys = &appsv1.DeploymentList{} g.Expect(k8sClient.List(workerCtx, deploys, client.InNamespace(prodNamespace))).Should(Succeed()) g.Expect(len(deploys.Items)).Should(Equal(2)) - // check component revision - compRevs := &appsv1.ControllerRevisionList{} - g.Expect(k8sClient.List(workerCtx, compRevs, client.InNamespace(prodNamespace))).Should(Succeed()) - g.Expect(len(compRevs.Items)).Should(Equal(2)) }, time.Minute).Should(Succeed()) Expect(hubDeployName).Should(Equal("data-worker")) // delete application @@ -273,10 +269,6 @@ var _ = Describe("Test multicluster scenario", func() { deploys = &appsv1.DeploymentList{} g.Expect(k8sClient.List(workerCtx, deploys, client.InNamespace(namespace))).Should(Succeed()) g.Expect(len(deploys.Items)).Should(Equal(0)) - // check component revision - compRevs := &appsv1.ControllerRevisionList{} - g.Expect(k8sClient.List(workerCtx, compRevs, client.InNamespace(prodNamespace))).Should(Succeed()) - g.Expect(len(compRevs.Items)).Should(Equal(0)) }, time.Minute).Should(Succeed()) }) diff --git a/test/e2e-test/application_test.go b/test/e2e-test/application_test.go index 90a83a796..a42a156d8 100644 --- a/test/e2e-test/application_test.go +++ b/test/e2e-test/application_test.go @@ -208,23 +208,6 @@ var _ = Describe("Application Normal tests", func() { time.Second*60, time.Millisecond*500).Should(BeNil()) } - verifyComponentRevision := func(compName string, revisionNum int64) { - By("Verify Component revision") - expectCompRevName := fmt.Sprintf("%s-v%d", compName, revisionNum) - Eventually( - func() error { - gotCR := &v1.ControllerRevision{} - if err := k8sClient.Get(ctx, client.ObjectKey{Namespace: namespaceName, Name: expectCompRevName}, gotCR); err != nil { - return err - } - if gotCR.Revision != revisionNum { - return fmt.Errorf("expect revision %d != real %d", revisionNum, gotCR.Revision) - } - return nil - }, - time.Second*10, time.Millisecond*500).Should(BeNil()) - } - BeforeEach(func() { By("Start to run a test, clean up previous resources") namespaceName = "app-normal-e2e-test" + "-" + strconv.FormatInt(rand.Int63(), 16) @@ -243,25 +226,21 @@ var _ = Describe("Application Normal tests", func() { applyApp("app1.yaml") By("Apply the application rollout go directly to the target") verifyWorkloadRunningExpected("myweb", 1, "stefanprodan/podinfo:4.0.3") - verifyComponentRevision("myweb", 1) By("Update app with trait") updateApp("app2.yaml") By("Apply the application rollout go directly to the target") verifyWorkloadRunningExpected("myweb", 2, "stefanprodan/podinfo:4.0.3") - verifyComponentRevision("myweb", 2) By("Update app with trait updated") updateApp("app3.yaml") By("Apply the application rollout go directly to the target") verifyWorkloadRunningExpected("myweb", 3, "stefanprodan/podinfo:4.0.3") - verifyComponentRevision("myweb", 3) By("Update app with trait and workload image updated") updateApp("app4.yaml") By("Apply the application rollout go directly to the target") verifyWorkloadRunningExpected("myweb", 1, "stefanprodan/podinfo:5.0.2") - verifyComponentRevision("myweb", 4) }) It("Test app have component with multiple same type traits", func() { @@ -402,7 +381,6 @@ var _ = Describe("Application Normal tests", func() { By("Checking an application status") verifyWorkloadRunningExpected("myweb", 1, "stefanprodan/podinfo:4.0.3") - verifyComponentRevision("myweb", 1) Expect(k8sClient.Delete(ctx, &newApp)).Should(Succeed()) Eventually(func(g Gomega) {