From 7115c64c4247ee47502c09f4a320ee951a251a5b Mon Sep 17 00:00:00 2001 From: OlofKalufs Date: Mon, 12 Aug 2024 08:57:55 +0200 Subject: [PATCH 1/4] Made Noop match upserting rule targets --- pkg/kapp/diffgraph/change.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/kapp/diffgraph/change.go b/pkg/kapp/diffgraph/change.go index 47914a277..20cd4d2ff 100644 --- a/pkg/kapp/diffgraph/change.go +++ b/pkg/kapp/diffgraph/change.go @@ -231,6 +231,8 @@ func (cs Changes) MatchesRule(rule ChangeRule, _ *Change) ([]*Change, error) { op := change.Change.Op() switch op { + case ActualChangeOpNoop: + // Fall through since we want noop to be treated as upsert case ActualChangeOpUpsert: if rule.TargetAction == ChangeRuleTargetActionUpserting { result = append(result, change) @@ -239,7 +241,6 @@ func (cs Changes) MatchesRule(rule ChangeRule, _ *Change) ([]*Change, error) { if rule.TargetAction == ChangeRuleTargetActionDeleting { result = append(result, change) } - case ActualChangeOpNoop: default: panic(fmt.Sprintf("Unknown change operation: %s", op)) } From 97814dac76732d61ca9e352c750e3735c9f699cf Mon Sep 17 00:00:00 2001 From: Olof Skyttner Date: Mon, 12 Aug 2024 15:56:40 +0200 Subject: [PATCH 2/4] Reverted this change, since it didn't help --- pkg/kapp/diffgraph/change.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/kapp/diffgraph/change.go b/pkg/kapp/diffgraph/change.go index 20cd4d2ff..47914a277 100644 --- a/pkg/kapp/diffgraph/change.go +++ b/pkg/kapp/diffgraph/change.go @@ -231,8 +231,6 @@ func (cs Changes) MatchesRule(rule ChangeRule, _ *Change) ([]*Change, error) { op := change.Change.Op() switch op { - case ActualChangeOpNoop: - // Fall through since we want noop to be treated as upsert case ActualChangeOpUpsert: if rule.TargetAction == ChangeRuleTargetActionUpserting { result = append(result, change) @@ -241,6 +239,7 @@ func (cs Changes) MatchesRule(rule ChangeRule, _ *Change) ([]*Change, error) { if rule.TargetAction == ChangeRuleTargetActionDeleting { result = append(result, change) } + case ActualChangeOpNoop: default: panic(fmt.Sprintf("Unknown change operation: %s", op)) } From 258b8976d480f3725a6099b1ec28fa2919090426 Mon Sep 17 00:00:00 2001 From: Olof Skyttner Date: Mon, 12 Aug 2024 15:56:59 +0200 Subject: [PATCH 3/4] Made a change so that noop-operations are performed last --- pkg/kapp/clusterapply/applying_changes.go | 46 ++++++++++++++--------- 1 file changed, 29 insertions(+), 17 deletions(-) diff --git a/pkg/kapp/clusterapply/applying_changes.go b/pkg/kapp/clusterapply/applying_changes.go index 0487c0b06..001ca7564 100644 --- a/pkg/kapp/clusterapply/applying_changes.go +++ b/pkg/kapp/clusterapply/applying_changes.go @@ -60,25 +60,28 @@ func (c *ApplyingChanges) Apply(allChanges []*ctldgraph.Change) ([]WaitingChange // - "...: context canceled (reason: )" applyThrottle := util.NewThrottle(c.opts.Concurrency) applyCh := make(chan applyResult, len(nonAppliedChanges)) + allChangesAreNoop := c.allChangesAreNoop(nonAppliedChanges) for _, change := range nonAppliedChanges { - change := change // copy - - go func() { - applyThrottle.Take() - defer applyThrottle.Done() - - clusterChange := change.Change.(wrappedClusterChange).ClusterChange - retryable, descMsgs, err := clusterChange.Apply() - - applyCh <- applyResult{ - Change: change, - ClusterChange: clusterChange, - DescMsgs: descMsgs, - Retryable: retryable, - Err: err, - } - }() + if allChangesAreNoop || (change.Change.Op() != ctldgraph.ActualChangeOpNoop) { + change := change // copy + + go func() { + applyThrottle.Take() + defer applyThrottle.Done() + + clusterChange := change.Change.(wrappedClusterChange).ClusterChange + retryable, descMsgs, err := clusterChange.Apply() + + applyCh <- applyResult{ + Change: change, + ClusterChange: clusterChange, + DescMsgs: descMsgs, + Retryable: retryable, + Err: err, + } + }() + } } var appliedChanges []WaitingChange @@ -138,6 +141,15 @@ func (c *ApplyingChanges) nonAppliedChanges(allChanges []*ctldgraph.Change) []*c return result } +func (c *ApplyingChanges) allChangesAreNoop(allChanges []*ctldgraph.Change) bool { + for _, change := range allChanges { + if change.Change.Op() != ctldgraph.ActualChangeOpNoop { + return false + } + } + return true +} + func (c *ApplyingChanges) isApplied(change *ctldgraph.Change) bool { _, found := c.applied[change] return found From 0bb7be6dffbe25e94833484721c45ec92460f5d8 Mon Sep 17 00:00:00 2001 From: Olof Skyttner Date: Tue, 13 Aug 2024 06:29:37 +0200 Subject: [PATCH 4/4] Added comment Signed-off-by: Olof Skyttner --- pkg/kapp/clusterapply/applying_changes.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/kapp/clusterapply/applying_changes.go b/pkg/kapp/clusterapply/applying_changes.go index 001ca7564..d29c93b78 100644 --- a/pkg/kapp/clusterapply/applying_changes.go +++ b/pkg/kapp/clusterapply/applying_changes.go @@ -60,6 +60,8 @@ func (c *ApplyingChanges) Apply(allChanges []*ctldgraph.Change) ([]WaitingChange // - "...: context canceled (reason: )" applyThrottle := util.NewThrottle(c.opts.Concurrency) applyCh := make(chan applyResult, len(nonAppliedChanges)) + + // Perform noop changes after all other changes are applied allChangesAreNoop := c.allChangesAreNoop(nonAppliedChanges) for _, change := range nonAppliedChanges {