From 951506b79b8da0fcf3a95ccae381fec7fe7026ff Mon Sep 17 00:00:00 2001 From: chwetion <137953601@qq.com> Date: Tue, 18 Jan 2022 13:40:30 +0800 Subject: [PATCH] fix revision will change when add new trait with skiprevisionaffect to application (#3032) Signed-off-by: chwetion <chwetion@foxmail.com> Co-authored-by: chwetion <chwetion@foxmail.com> --- .../v1alpha2/application/revision.go | 25 ++++++++++++++++--- .../v1alpha2/application/revision_test.go | 10 ++++++-- 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/pkg/controller/core.oam.dev/v1alpha2/application/revision.go b/pkg/controller/core.oam.dev/v1alpha2/application/revision.go index f565ec0e..90910c62 100644 --- a/pkg/controller/core.oam.dev/v1alpha2/application/revision.go +++ b/pkg/controller/core.oam.dev/v1alpha2/application/revision.go @@ -311,7 +311,7 @@ func ComputeAppRevisionHash(appRevision *v1beta1.ApplicationRevision) (string, e } appRevisionHash.ComponentDefinitionHash[key] = hash } - for key, td := range appRevision.Spec.TraitDefinitions { + for key, td := range filterSkipAffectAppRevTraitDefinitions(appRevision.Spec.TraitDefinitions) { hash, err := utils.ComputeSpecHash(&td.Spec) if err != nil { return "", err @@ -376,7 +376,11 @@ func (h *AppHandler) currentAppRevIsNew(ctx context.Context) (bool, bool, error) // diff the latest revision first if h.app.Status.LatestRevision.RevisionHash == h.currentRevHash && DeepEqualRevision(h.latestAppRev, h.currentAppRev) { + appSpec := h.currentAppRev.Spec.Application.Spec + traitDef := h.currentAppRev.Spec.TraitDefinitions h.currentAppRev = h.latestAppRev.DeepCopy() + h.currentAppRev.Spec.Application.Spec = appSpec + h.currentAppRev.Spec.TraitDefinitions = traitDef return false, false, nil } @@ -409,7 +413,9 @@ func DeepEqualRevision(old, new *v1beta1.ApplicationRevision) bool { if len(old.Spec.WorkloadDefinitions) != len(new.Spec.WorkloadDefinitions) { return false } - if len(old.Spec.TraitDefinitions) != len(new.Spec.TraitDefinitions) { + oldTraitDefinitions := filterSkipAffectAppRevTraitDefinitions(old.Spec.TraitDefinitions) + newTraitDefinitions := filterSkipAffectAppRevTraitDefinitions(new.Spec.TraitDefinitions) + if len(oldTraitDefinitions) != len(newTraitDefinitions) { return false } if len(old.Spec.ComponentDefinitions) != len(new.Spec.ComponentDefinitions) { @@ -428,8 +434,8 @@ func DeepEqualRevision(old, new *v1beta1.ApplicationRevision) bool { return false } } - for key, td := range new.Spec.TraitDefinitions { - if !apiequality.Semantic.DeepEqual(old.Spec.TraitDefinitions[key].Spec, td.Spec) { + for key, td := range newTraitDefinitions { + if !apiequality.Semantic.DeepEqual(oldTraitDefinitions[key].Spec, td.Spec) { return false } } @@ -820,6 +826,17 @@ func filterSkipAffectAppRevTrait(appSpec v1beta1.ApplicationSpec, tds map[string 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 +} + type historiesByRevision []v1beta1.ApplicationRevision func (h historiesByRevision) Len() int { return len(h) } 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 7bb5f0ab..236330a9 100644 --- a/pkg/controller/core.oam.dev/v1alpha2/application/revision_test.go +++ b/pkg/controller/core.oam.dev/v1alpha2/application/revision_test.go @@ -151,7 +151,6 @@ var _ = Describe("test generate revision ", func() { appRevision1.Spec.ComponentDefinitions[cd.Name] = cd appRevision1.Spec.WorkloadDefinitions[wd.Name] = wd appRevision1.Spec.TraitDefinitions[td.Name] = td - appRevision1.Spec.TraitDefinitions[rolloutTd.Name] = rolloutTd appRevision1.Spec.ScopeDefinitions[sd.Name] = sd appRevision2 = *appRevision1.DeepCopy() @@ -168,6 +167,10 @@ 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()) @@ -228,7 +231,7 @@ var _ = Describe("test generate revision ", func() { verifyNotEqual() }) - It("Test appliction contain a SkipAppRevision tait will have same hash", func() { + It("Test appliction contain a SkipAppRevision tait will have same hash and revision will equal", func() { rolloutTrait := common.ApplicationTrait{ Type: "rollout", Properties: &runtime.RawExtension{ @@ -236,7 +239,10 @@ var _ = Describe("test generate revision ", func() { }, } 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 apply success for none rollout case", func() { -- GitLab