diff --git a/apis/core.oam.dev/v1beta1/core_types.go b/apis/core.oam.dev/v1beta1/core_types.go index ed83ae00b..521d9a417 100644 --- a/apis/core.oam.dev/v1beta1/core_types.go +++ b/apis/core.oam.dev/v1beta1/core_types.go @@ -155,9 +155,6 @@ type TraitDefinitionSpec struct { // ManageWorkload defines the trait would be responsible for creating the workload // +optional ManageWorkload bool `json:"manageWorkload,omitempty"` - // SkipRevisionAffect defines the update this trait will not generate a new application Revision - // +optional - SkipRevisionAffect bool `json:"skipRevisionAffect,omitempty"` // ControlPlaneOnly defines which cluster is dispatched to // +optional ControlPlaneOnly bool `json:"controlPlaneOnly,omitempty"` diff --git a/charts/vela-core/crds/core.oam.dev_applicationrevisions.yaml b/charts/vela-core/crds/core.oam.dev_applicationrevisions.yaml index dabd31714..f0438a218 100644 --- a/charts/vela-core/crds/core.oam.dev_applicationrevisions.yaml +++ b/charts/vela-core/crds/core.oam.dev_applicationrevisions.yaml @@ -3887,10 +3887,6 @@ spec: - configuration type: object type: object - skipRevisionAffect: - description: SkipRevisionAffect defines the update this - trait will not generate a new application Revision - type: boolean status: description: Status defines the custom health policy and status message for trait diff --git a/charts/vela-core/crds/core.oam.dev_definitionrevisions.yaml b/charts/vela-core/crds/core.oam.dev_definitionrevisions.yaml index 30a4969c1..abd422493 100644 --- a/charts/vela-core/crds/core.oam.dev_definitionrevisions.yaml +++ b/charts/vela-core/crds/core.oam.dev_definitionrevisions.yaml @@ -916,10 +916,6 @@ spec: - configuration type: object type: object - skipRevisionAffect: - description: SkipRevisionAffect defines the update this trait - will not generate a new application Revision - type: boolean status: description: Status defines the custom health policy and status message for trait diff --git a/charts/vela-core/crds/core.oam.dev_traitdefinitions.yaml b/charts/vela-core/crds/core.oam.dev_traitdefinitions.yaml index 0e808978b..f4b21a273 100644 --- a/charts/vela-core/crds/core.oam.dev_traitdefinitions.yaml +++ b/charts/vela-core/crds/core.oam.dev_traitdefinitions.yaml @@ -558,10 +558,6 @@ spec: - configuration type: object type: object - skipRevisionAffect: - description: SkipRevisionAffect defines the update this trait will - not generate a new application Revision - type: boolean status: description: Status defines the custom health policy and status message for trait diff --git a/charts/vela-minimal/crds/core.oam.dev_applicationrevisions.yaml b/charts/vela-minimal/crds/core.oam.dev_applicationrevisions.yaml index dabd31714..f0438a218 100644 --- a/charts/vela-minimal/crds/core.oam.dev_applicationrevisions.yaml +++ b/charts/vela-minimal/crds/core.oam.dev_applicationrevisions.yaml @@ -3887,10 +3887,6 @@ spec: - configuration type: object type: object - skipRevisionAffect: - description: SkipRevisionAffect defines the update this - trait will not generate a new application Revision - type: boolean status: description: Status defines the custom health policy and status message for trait diff --git a/charts/vela-minimal/crds/core.oam.dev_definitionrevisions.yaml b/charts/vela-minimal/crds/core.oam.dev_definitionrevisions.yaml index 30a4969c1..abd422493 100644 --- a/charts/vela-minimal/crds/core.oam.dev_definitionrevisions.yaml +++ b/charts/vela-minimal/crds/core.oam.dev_definitionrevisions.yaml @@ -916,10 +916,6 @@ spec: - configuration type: object type: object - skipRevisionAffect: - description: SkipRevisionAffect defines the update this trait - will not generate a new application Revision - type: boolean status: description: Status defines the custom health policy and status message for trait diff --git a/charts/vela-minimal/crds/core.oam.dev_traitdefinitions.yaml b/charts/vela-minimal/crds/core.oam.dev_traitdefinitions.yaml index 0e808978b..f4b21a273 100644 --- a/charts/vela-minimal/crds/core.oam.dev_traitdefinitions.yaml +++ b/charts/vela-minimal/crds/core.oam.dev_traitdefinitions.yaml @@ -558,10 +558,6 @@ spec: - configuration type: object type: object - skipRevisionAffect: - description: SkipRevisionAffect defines the update this trait will - not generate a new application Revision - type: boolean status: description: Status defines the custom health policy and status message for trait diff --git a/legacy/charts/vela-core-legacy/crds/core.oam.dev_applicationrevisions.yaml b/legacy/charts/vela-core-legacy/crds/core.oam.dev_applicationrevisions.yaml index d47a0effb..2a0ac5120 100644 --- a/legacy/charts/vela-core-legacy/crds/core.oam.dev_applicationrevisions.yaml +++ b/legacy/charts/vela-core-legacy/crds/core.oam.dev_applicationrevisions.yaml @@ -3887,10 +3887,6 @@ spec: - configuration type: object type: object - skipRevisionAffect: - description: SkipRevisionAffect defines the update this - trait will not generate a new application Revision - type: boolean status: description: Status defines the custom health policy and status message for trait diff --git a/legacy/charts/vela-core-legacy/crds/core.oam.dev_definitionrevisions.yaml b/legacy/charts/vela-core-legacy/crds/core.oam.dev_definitionrevisions.yaml index 6c39c58fc..0009f9b51 100644 --- a/legacy/charts/vela-core-legacy/crds/core.oam.dev_definitionrevisions.yaml +++ b/legacy/charts/vela-core-legacy/crds/core.oam.dev_definitionrevisions.yaml @@ -916,10 +916,6 @@ spec: - configuration type: object type: object - skipRevisionAffect: - description: SkipRevisionAffect defines the update this trait - will not generate a new application Revision - type: boolean status: description: Status defines the custom health policy and status message for trait diff --git a/legacy/charts/vela-core-legacy/crds/core.oam.dev_traitdefinitions.yaml b/legacy/charts/vela-core-legacy/crds/core.oam.dev_traitdefinitions.yaml index 3e7a0046e..35566396a 100644 --- a/legacy/charts/vela-core-legacy/crds/core.oam.dev_traitdefinitions.yaml +++ b/legacy/charts/vela-core-legacy/crds/core.oam.dev_traitdefinitions.yaml @@ -558,10 +558,6 @@ spec: - configuration type: object type: object - skipRevisionAffect: - description: SkipRevisionAffect defines the update this trait will - not generate a new application Revision - type: boolean status: description: Status defines the custom health policy and status message for trait diff --git a/pkg/controller/core.oam.dev/v1alpha2/application/application_controller_test.go b/pkg/controller/core.oam.dev/v1alpha2/application/application_controller_test.go index 50fd4ba66..60cab2eda 100644 --- a/pkg/controller/core.oam.dev/v1alpha2/application/application_controller_test.go +++ b/pkg/controller/core.oam.dev/v1alpha2/application/application_controller_test.go @@ -925,13 +925,12 @@ var _ = Describe("Test Application Controller", func() { Expect(k8sClient.Get(ctx, types.NamespacedName{Name: "myweb1", Namespace: ns.Name}, deploy)).Should(util.NotFoundMatcher{}) By("check update rollout trait won't generate new appRevision") - appRevName := checkApp.Status.LatestRevision.Name checkApp.Spec.Components[0].Traits[0].Properties = &runtime.RawExtension{Raw: []byte(`{"targetRevision":"myweb1-v3"}`)} Expect(k8sClient.Update(ctx, checkApp)).Should(BeNil()) testutil.ReconcileOnce(reconciler, reconcile.Request{NamespacedName: appKey}) checkApp = &v1beta1.Application{} Expect(k8sClient.Get(ctx, appKey, checkApp)).Should(BeNil()) - Expect(checkApp.Status.LatestRevision.Name).Should(BeEquivalentTo(appRevName)) + Expect(checkApp.Status.LatestRevision.Name).Should(BeEquivalentTo("app-with-rollout-v3")) checkRollout = &stdv1alpha1.Rollout{} Expect(k8sClient.Get(ctx, types.NamespacedName{Name: "myweb1", Namespace: ns.Name}, checkRollout)).Should(BeNil()) Expect(checkRollout.Spec.TargetRevisionName).Should(BeEquivalentTo("myweb1-v3")) diff --git a/pkg/controller/core.oam.dev/v1alpha2/application/revision.go b/pkg/controller/core.oam.dev/v1alpha2/application/revision.go index 7ccd219d7..8c43f343f 100644 --- a/pkg/controller/core.oam.dev/v1alpha2/application/revision.go +++ b/pkg/controller/core.oam.dev/v1alpha2/application/revision.go @@ -374,7 +374,7 @@ func ComputeAppRevisionHash(appRevision *v1beta1.ApplicationRevision) (string, e PolicyHash: make(map[string]string), } var err error - revHash.ApplicationSpecHash, err = utils.ComputeSpecHash(filterSkipAffectAppRevTrait(appRevision.Spec.Application.Spec, appRevision.Spec.TraitDefinitions)) + revHash.ApplicationSpecHash, err = utils.ComputeSpecHash(appRevision.Spec.Application.Spec) if err != nil { return "", err } @@ -392,7 +392,7 @@ func ComputeAppRevisionHash(appRevision *v1beta1.ApplicationRevision) (string, e } revHash.ComponentDefinitionHash[key] = hash } - for key, td := range filterSkipAffectAppRevTraitDefinitions(appRevision.Spec.TraitDefinitions) { + for key, td := range appRevision.Spec.TraitDefinitions { hash, err := utils.ComputeSpecHash(&td.Spec) if err != nil { return "", err @@ -496,8 +496,8 @@ func DeepEqualRevision(old, new *v1beta1.ApplicationRevision) bool { if len(old.Spec.WorkloadDefinitions) != len(new.Spec.WorkloadDefinitions) { return false } - oldTraitDefinitions := filterSkipAffectAppRevTraitDefinitions(old.Spec.TraitDefinitions) - newTraitDefinitions := filterSkipAffectAppRevTraitDefinitions(new.Spec.TraitDefinitions) + oldTraitDefinitions := old.Spec.TraitDefinitions + newTraitDefinitions := new.Spec.TraitDefinitions if len(oldTraitDefinitions) != len(newTraitDefinitions) { return false } @@ -555,8 +555,7 @@ func deepEqualAppInRevision(old, new *v1beta1.ApplicationRevision) bool { return false } } - return apiequality.Semantic.DeepEqual(filterSkipAffectAppRevTrait(old.Spec.Application.Spec, old.Spec.TraitDefinitions), - filterSkipAffectAppRevTrait(new.Spec.Application.Spec, new.Spec.TraitDefinitions)) + return apiequality.Semantic.DeepEqual(old.Spec.Application.Spec, new.Spec.Application.Spec) } // HandleComponentsRevision manages Component revisions @@ -915,34 +914,6 @@ func replaceComponentRevisionContext(u *unstructured.Unstructured, compRevName s return nil } -// before computing hash or deepEqual, filterSkipAffectAppRevTrait filter can remove `SkipAffectAppRevTrait` trait from appSpec -func filterSkipAffectAppRevTrait(appSpec v1beta1.ApplicationSpec, tds map[string]v1beta1.TraitDefinition) v1beta1.ApplicationSpec { - // deepCopy avoid modify origin appSpec - res := appSpec.DeepCopy() - for index, comp := range res.Components { - i := 0 - for _, trait := range comp.Traits { - if !tds[trait.Type].Spec.SkipRevisionAffect { - comp.Traits[i] = trait - i++ - } - } - res.Components[index].Traits = res.Components[index].Traits[:i] - } - return *res -} - -// before computing hash or deepEqual, filterSkipAffectAppRevTraitDefinitions filter can ignore `SkipAffectRevision` trait definition from appRev -func filterSkipAffectAppRevTraitDefinitions(tds map[string]v1beta1.TraitDefinition) map[string]v1beta1.TraitDefinition { - res := make(map[string]v1beta1.TraitDefinition) - for key, td := range tds { - if !td.Spec.SkipRevisionAffect { - res[key] = td - } - } - return res -} - func cleanUpWorkflowComponentRevision(ctx context.Context, h *AppHandler) error { if DisableAllComponentRevision { return nil diff --git a/pkg/controller/core.oam.dev/v1alpha2/application/revision_test.go b/pkg/controller/core.oam.dev/v1alpha2/application/revision_test.go index aa89a3ca6..5cb4130e0 100644 --- a/pkg/controller/core.oam.dev/v1alpha2/application/revision_test.go +++ b/pkg/controller/core.oam.dev/v1alpha2/application/revision_test.go @@ -154,10 +154,6 @@ var _ = Describe("test generate revision ", func() { Expect(k8sClient.Delete(context.TODO(), &ns)).Should(Succeed()) }) - verifyDeepEqualRevision := func() { - Expect(DeepEqualRevision(&appRevision1, &appRevision2)).Should(BeTrue()) - } - verifyEqual := func() { appHash1, err := ComputeAppRevisionHash(&appRevision1) Expect(err).Should(Succeed()) @@ -207,20 +203,6 @@ var _ = Describe("test generate revision ", func() { verifyNotEqual() }) - It("Test appliction contain a SkipAppRevision tait will have same hash and revision will equal", func() { - rolloutTrait := common.ApplicationTrait{ - Type: "rollout", - Properties: &runtime.RawExtension{ - Raw: []byte(`{"targetRevision":"myrev-v1"}`), - }, - } - appRevision2.Spec.Application.Spec.Components[0].Traits = append(appRevision2.Spec.Application.Spec.Components[0].Traits, rolloutTrait) - // appRevision will have no traitDefinition of rollout - appRevision2.Spec.TraitDefinitions[rolloutTd.Name] = rolloutTd - verifyEqual() - verifyDeepEqualRevision() - }) - It("Test application revision compare", func() { By("Apply the application") appParser := appfile.NewApplicationParser(reconciler.Client, reconciler.dm, reconciler.pd) @@ -760,49 +742,6 @@ var _ = Describe("Test ReplaceComponentRevisionContext func", func() { }) }) -var _ = Describe("Test remove SkipAppRev func", func() { - It("Test remove spec", func() { - appSpec := v1beta1.ApplicationSpec{ - Components: []common.ApplicationComponent{ - { - Traits: []common.ApplicationTrait{ - { - Type: "rollout", - }, - { - Type: "ingress", - }, - { - Type: "service", - }, - }, - }, - }, - } - tds := map[string]v1beta1.TraitDefinition{ - "rollout": { - Spec: v1beta1.TraitDefinitionSpec{ - SkipRevisionAffect: true, - }, - }, - "ingress": { - Spec: v1beta1.TraitDefinitionSpec{ - SkipRevisionAffect: false, - }, - }, - "service": { - Spec: v1beta1.TraitDefinitionSpec{ - SkipRevisionAffect: false, - }, - }, - } - res := filterSkipAffectAppRevTrait(appSpec, tds) - Expect(len(res.Components[0].Traits)).Should(BeEquivalentTo(2)) - Expect(res.Components[0].Traits[0].Type).Should(BeEquivalentTo("ingress")) - Expect(res.Components[0].Traits[1].Type).Should(BeEquivalentTo("service")) - }) -}) - var _ = Describe("Test PrepareCurrentAppRevision", func() { var app v1beta1.Application var apprev v1beta1.ApplicationRevision