Feat: remove skip revision affect (#4774)

Signed-off-by: Somefive <yd219913@alibaba-inc.com>

Signed-off-by: Somefive <yd219913@alibaba-inc.com>

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"`

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

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

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

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

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

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

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

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

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

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"))

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

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