Improve versionned resources deletion in a single apply

[Misfits09]

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #1119

Does this PR introduce a user-facing change?

Obsolete versioned resources are now deleted at the same time the newest version is created

Additional Notes for your reviewer:

Review Checklist:

• Follows the developer guidelines
• Relevant tests are added or updated
• Relevant docs in this repo added or updated
• N/A Relevant carvel.dev docs added or updated in a separate PR and there's
  a link to that PR
• Code is at least as readable and maintainable as it was before this
  change

Additional documentation e.g., Proposal, usage docs, etc.:

N/A

[Copilot]

Pull request overview

This PR addresses kapp’s versioned-resource retention behavior so that obsolete versions are deleted in the same apply run that creates a new version, reducing confusing “delayed deletes” on subsequent no-op applies (Fixes #1119).

Changes:

• Track when a new versioned resource version is actually created, and delete now-obsolete versions during the same change calculation.
• Add a regression test covering “delete obsolete versions in same apply” for versioned ConfigMaps with kapp.k14s.io/num-versions.
• Add cluster apply graph ordering intended to defer deletion of versioned resources until after other operations complete.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
pkg/kapp/diff/change_set_with_versioned_rs.go	Detects when a new version is created and adjusts retention deletion calculation to delete obsolete versions immediately.
pkg/kapp/diff/change_set_with_versioned_rs_test.go	Adds a test asserting obsolete versions are deleted in the same apply when a new version is created.
pkg/kapp/clusterapply/cluster_change_set.go	Adds apply-graph ordering logic to make versioned deletes wait for other operations.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

configVersionedDeleteOrdering() appends dependencies after NewChangeGraph() has already run cycle detection/dedup. Because otherOperations currently includes delete operations too, this can easily invert existing delete-ordering rules and can introduce new dependency cycles (e.g., if some other delete is already ordered after this versioned delete via change-rules/bindings). Consider restricting the “wait for everything” set to non-delete operations (upserts/noops), and/or only appending a dependency when it would not create a cycle (e.g., skip if the candidate op already transitively waits on del, and skip if del already directly waits on it).

Suggested change

	} else {
	otherOperations = append(otherOperations, change)
	}
	}
	for _, del := range versionedDeletes {
	del.WaitingFor = append(del.WaitingFor, otherOperations...)
	}
	}
	} else if cc.ApplyOp() != ClusterChangeApplyOpDelete {
	// Only consider non-delete operations as "other operations" to wait on.
	otherOperations = append(otherOperations, change)
	}
	}
	for _, del := range versionedDeletes {
	for _, op := range otherOperations {
	// Avoid self-dependency, redundant edges, and cycles.
	if op == del {
	continue
	}
	if c.directlyWaitsOn(del, op) {
	continue
	}
	if c.waitsTransitivelyOn(op, del, map[*ctldgraph.Change]bool{}) {
	// Adding del -> op would create a cycle because op already waits on del.
	continue
	}
	del.WaitingFor = append(del.WaitingFor, op)
	}
	}
	}
	func (c ClusterChangeSet) directlyWaitsOn(change, candidate *ctldgraph.Change) bool {
	for _, dep := range change.WaitingFor {
	if dep == candidate {
	return true
	}
	}
	return false
	}
	func (c ClusterChangeSet) waitsTransitivelyOn(start, target *ctldgraph.Change, visited map[*ctldgraph.Change]bool) bool {
	if start == target {
	return true
	}
	if visited[start] {
	return false
	}
	visited[start] = true
	for _, dep := range start.WaitingFor {
	if c.waitsTransitivelyOn(dep, target, visited) {
	return true
	}
	}
	return false
	}

[Misfits09]

Is added a check on op.IsTransitivelyWaitingFor(del) to ensure there is not deadlocks

[Copilot]

configVersionedDeleteOrdering() creates an all-to-all dependency from each versioned delete to every other operation. In cases with many obsolete versions (and therefore many deletes), this grows quadratically and can substantially increase graph size and apply scheduling overhead. If possible, reduce the number of edges (e.g., only depend on upsert operations, and avoid adding duplicates by checking whether the dependency is already present).

Suggested change

	otherOperations = append(otherOperations, change)
	}
	}
	for _, del := range versionedDeletes {
	del.WaitingFor = append(del.WaitingFor, otherOperations...)
	// Only consider non-delete, non-noop operations as dependencies (i.e. upserts).
	if cc.ApplyOp() != ClusterChangeApplyOpDelete && cc.ApplyOp() != ClusterChangeApplyOpNoop {
	otherOperations = append(otherOperations, change)
	}
	}
	}
	for _, del := range versionedDeletes {
	// Build a set of existing dependencies to avoid adding duplicates.
	existing := make(map[*ctldgraph.Change]struct{}, len(del.WaitingFor))
	for _, dep := range del.WaitingFor {
	existing[dep] = struct{}{}
	}
	for _, op := range otherOperations {
	if op == del {
	continue
	}
	if _, found := existing[op]; found {
	continue
	}
	del.WaitingFor = append(del.WaitingFor, op)
	}

[Misfits09]

I excluded delete/noop operations from WaitingFor