-
Notifications
You must be signed in to change notification settings - Fork 6.6k
fix: cross namespace hierarchy traversal from cluster-scoped parents to namespaced children (fixes #24379) #24847
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
fix: cross namespace hierarchy traversal from cluster-scoped parents to namespaced children (fixes #24379) #24847
Conversation
❌ Preview Environment undeployed from BunnyshellAvailable commands (reply to this comment):
|
gitops-engine/pkg/cache/cluster.go
Outdated
| if !c.disableClusterScopedParentRefs { | ||
| // If this is a namespaced resource that was tracked as an orphaned child | ||
| if key.Namespace != "" { | ||
| if parents, exists := c.childToClusterParents[key]; exists { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe rename to namespacedChildToClusterParents? Wordy, but hella clear :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done!
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #24847 +/- ##
==========================================
- Coverage 62.58% 62.57% -0.01%
==========================================
Files 352 352
Lines 49746 49757 +11
==========================================
+ Hits 31135 31137 +2
- Misses 15634 15643 +9
Partials 2977 2977 ☔ View full report in Codecov by Sentry. |
45679ba to
7be9d10
Compare
gitops-engine/pkg/cache/cluster.go
Outdated
| } | ||
| } | ||
| } else { | ||
| // Inferred owner reference - construct the key |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this can actually occur in practice, but should we keep it out of an abundance of safety?
d7027b7 to
70e1828
Compare
ab727de to
80055fb
Compare
Cross-Namespace Hierarchy TraversalOverviewThis branch adds support for traversing resource hierarchies from cluster-scoped parents to their namespaced children Scope: This feature specifically handles traversing from cluster-scoped parents (e.g., Crossplane Provider) to their ImplementationData Structure// parentUIDToChildren maps a resource's UID to all its direct children
// Maintained for ALL resources that have owner references
// Used during cross-namespace hierarchy traversal for O(1) lookups
parentUIDToChildren map[types.UID][]kube.ResourceKeyCache Lifecycle IntegrationThe index is maintained alongside existing cache structures and follows the same lifecycle: Incremental Updates (Continuous)During normal operation, Kubernetes watch events trigger incremental cache updates:
Full Sync (Rare)During cluster invalidation (
Normal Refresh (Frequent, ~3 min)Standard ArgoCD refresh operations do not touch the cache at all - they only re-run health/sync assessments on existing Index Maintenance Functions
Traversal Algorithm
Design RationaleThe index-based approach was chosen over graph-based traversal because: Graph-based approach (considered but not implemented):
Index-based approach (current implementation):
Scaling Characteristics ComparisonThe index-based approach fundamentally changes what dimensions drive performance:
Key insight: The index-based approach makes the feature's performance predictable based solely on the total number Performance CharacteristicsBenchmark ResultsTesting with 50 namespaces, 100 resources per namespace, varying percentages of cross-namespace resources:
Performance ScalingThe implementation scales with the total number of cross-namespace resources, not the percentage of namespaces
Key difference from graph-based approaches: The index-based implementation scales with the total count of
This means performance is largely independent of namespace distribution - whether 10 cross-namespace resources are Memory EfficiencyMemory usage remains efficient across workload types:
Memory usage grows modestly because the index is maintained persistently - there's no repeated allocation of temporary Complexity AnalysisTime Complexity: O(k) where k = number of resources with cross-namespace parents
Space Complexity: O(r) where r = total resources in cluster
Real-World ImpactTypical Crossplane Workload (1-2% cross-namespace)
Multi-Provider Setup (5-10% cross-namespace)
Heavy Cross-Namespace Usage (20% cross-namespace)
Extreme Cross-Namespace (100% cross-namespace)
All overhead figures are still in the microsecond range and represent acceptable performance for the added Operational CharacteristicsMemory OverheadThe For a typical cluster with 5,000 resources and 10% having owner refs: This is negligible in the context of ArgoCD's overall memory footprint. CPU ImpactIndex maintenance: Near-zero overhead
Traversal: Efficient O(k) traversal
ScalabilityThe implementation scales well with cluster size: Namespace scaling: Performance is independent of total namespace count
Resource scaling: Linear with cross-namespace resources
TestingUnit Test CoverageAll cross-namespace hierarchy tests pass:
Benchmark ScenariosBenchmarks test various real-world patterns:
Code LocationFile: Key sections:
SummaryThis implementation adds cross-namespace hierarchy traversal to ArgoCD by: ✅ Maintaining a persistent parent-to-children index for O(1) lookups The index-based approach was chosen over graph-based traversal to avoid O(n×m) complexity and repeated memory |
4a40e83 to
7f6752f
Compare
7f6752f to
b1d6a83
Compare
gitops-engine/pkg/cache/cluster.go
Outdated
| parentKey = key | ||
| } | ||
| } else { | ||
| // Inferred owner reference - construct the key |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is some defensive coding that I'm not sure would actually be allowed to occur in production.
93326fd to
68cfab0
Compare
68cfab0 to
1c09e9b
Compare
| visited := make(map[kube.ResourceKey]int) | ||
| for _, key := range namespaceKeys { | ||
| visited[key] = 0 | ||
| c.processNamespaceHierarchy(namespaceKeys, nsNodes, graph, visited, action) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The diff is a little gnarly here, but the contents of this loop now live in c.processNamespaceHierarchy and the logic hasn't changed.
01bfb2b to
1c2673d
Compare
4533780 to
ff2a071
Compare
… of namespaced children Signed-off-by: Jonathan Ogilvie <[email protected]>
Signed-off-by: Jonathan Ogilvie <[email protected]> # Conflicts: # controller/appcontroller_test.go # gitops-engine/pkg/cache/mocks/ClusterCache.go
Signed-off-by: Jonathan Ogilvie <[email protected]>
Signed-off-by: Jonathan Ogilvie <[email protected]>
Signed-off-by: Jonathan Ogilvie <[email protected]>
Signed-off-by: Jonathan Ogilvie <[email protected]>
Signed-off-by: Jonathan Ogilvie <[email protected]>
…ionships based on input keys; defer to namespace scan Signed-off-by: Jonathan Ogilvie <[email protected]>
…sualizes correctly in UI Signed-off-by: Jonathan Ogilvie <[email protected]>
Signed-off-by: Jonathan Ogilvie <[email protected]>
Signed-off-by: Jonathan Ogilvie <[email protected]>
Signed-off-by: Jonathan Ogilvie <[email protected]>
…mment Signed-off-by: Jonathan Ogilvie <[email protected]>
Signed-off-by: Jonathan Ogilvie <[email protected]>
Signed-off-by: Jonathan Ogilvie <[email protected]>
Signed-off-by: Jonathan Ogilvie <[email protected]>
Signed-off-by: Jonathan Ogilvie <[email protected]>
Signed-off-by: Jonathan Ogilvie <[email protected]>
Signed-off-by: Jonathan Ogilvie <[email protected]>
…eration (cluster-parent/namespace-child only) Signed-off-by: Jonathan Ogilvie <[email protected]>
Signed-off-by: Jonathan Ogilvie <[email protected]>
Signed-off-by: Jonathan Ogilvie <[email protected]>
Signed-off-by: Jonathan Ogilvie <[email protected]>
Signed-off-by: Jonathan Ogilvie <[email protected]>
Signed-off-by: Jonathan Ogilvie <[email protected]>
Signed-off-by: Jonathan Ogilvie <[email protected]>
6e7f966 to
5e6a34b
Compare
Fixes #24379.
Fixes #7733
Fixes #8222
This PR adds resource-tree (including UI) support for cluster-scoped parents owning namespaced children. This is a valid kube owner relationship which is sometimes manifested in projects like Crossplane, but in the current version we don't actually capture the children in the resource tree.
Checklist: