Applier 05 reactive informers#5311
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR shifts kube-applier storage and watch semantics to a per-management-cluster Cosmos container model and adds union listers/informers so backend controllers can react to *Desire changes across all management clusters.
Changes:
- Introduces
database.KubeApplierDBClients(plural) registry + per-containerKubeApplierDBClientAPI (Listers(),UntypedCRUD, direct*DesiresCRUD). - Adds union listers + union informers to aggregate per-MC informer/lister surfaces into a single backend-facing watch surface.
- Updates kube-applier runtime wiring, backend wiring, mismatch orphan-sweep controller, tests, and documentation for the per-MC container model and new required fleet status field.
Reviewed changes
Copilot reviewed 50 out of 50 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| test-integration/utils/controllertesthelpers/basic_controller.go | Adds KubeApplierDBClients to controller test initialization with a default empty registry. |
| test-integration/kube-applier/framework/step_load_desire.go | Adapts integration harness desire loading to new per-container CRUD accessors. |
| test-integration/kube-applier/framework/step_desire_eventually.go | Updates desire getters to use direct CRUD accessors. |
| test-integration/kube-applier/framework/framework.go | Switches informer wiring to KubeApplierDBClient.Listers() and passes kac into controllers. |
| test-integration/backend/controllers/mismatches/delete_orphaned_cosmos_test.go | Updates integration tests to pass KubeApplierDBClients into the orphan controller constructor. |
| kube-applier/readme.md | Documents per-MC container model and new client/registry concepts. |
| kube-applier/pkg/controllers/read_desire_manager/controller_test.go | Updates tests for new DB client surface (no KubeApplier(mgmt) accessor). |
| kube-applier/pkg/controllers/read_desire_kubernetes/controller_test.go | Updates tests to pass the per-container DB client directly. |
| kube-applier/pkg/app/kube_applier.go | Uses Listers() and passes KubeApplierDBClient directly to controllers. |
| kube-applier/pkg/app/cosmos_wiring.go | Changes kube-applier Cosmos wiring to open a specific container + partition key. |
| kube-applier/docs/README.md | Updates design docs index and storage layer description for per-MC containers + registry. |
| kube-applier/docs/08-rollout.md | Updates rollout steps for per-MC containers and new registry approach. |
| kube-applier/docs/04-informers-listers.md | Updates informer/lister scoping explanation for per-MC containers. |
| kube-applier/docs/03-database.md | Rewrites database design doc around per-MC containers + KubeApplierDBClients. |
| kube-applier/docs/01-overview.md | Reframes cross-MC access as coming from KubeApplierDBClients. |
| kube-applier/cmd/root.go | Adds required --cosmos-container flag and wires it into kube-applier DB client construction. |
| internal/validation/validate_management_cluster_test.go | Adds required+immutable validation and migration behavior test for KubeApplierCosmosContainerName. |
| internal/validation/validate_management_cluster.go | Implements create-required + “empty→set allowed, otherwise immutable” validation for container name. |
| internal/ocm/convert_test.go | Asserts kube-applier container name is set during CS→internal conversion. |
| internal/ocm/convert.go | Populates KubeApplierCosmosContainerName during CS→internal conversion. |
| internal/databasetesting/mock_kube_applier_client_test.go | Updates mock client tests to new Listers() and direct CRUD accessors. |
| internal/databasetesting/mock_kube_applier_client.go | Reworks mock kube-applier DB client to per-container model; adds untyped CRUD + plural registry mock. |
| internal/database/unionlisters/kubeapplier/union_listers_test.go | Adds test coverage for union listers across management clusters. |
| internal/database/unionlisters/kubeapplier/read_desire_lister.go | Introduces union ReadDesire lister. |
| internal/database/unionlisters/kubeapplier/delete_desire_lister.go | Introduces union DeleteDesire lister. |
| internal/database/unionlisters/kubeapplier/apply_desire_lister.go | Introduces union ApplyDesire lister. |
| internal/database/unioninformers/kubeapplier/union_kube_applier_informers.go | Adds union aggregator for per-MC informers/listers. |
| internal/database/unioninformers/kubeapplier/union_informers_test.go | Adds extensive tests for union desire informer and union kube-applier informers end-to-end. |
| internal/database/unioninformers/kubeapplier/desire_informer.go | Implements union informer that propagates handlers to sub-informers and tracks sync. |
| internal/database/listertesting/slice_listers_test.go | Updates tests for ListForManagementCluster signature change to resourceID. |
| internal/database/listertesting/slice_listers.go | Updates slice listers to take *azcorearm.ResourceID for management cluster filtering. |
| internal/database/listertesting/db_listers_test.go | Updates DB-backed listers test to use KubeApplierDBClients registry. |
| internal/database/listertesting/db_listers.go | Refactors DB-backed desire listers to aggregate across KubeApplierDBClients. |
| internal/database/listers/read_desire_lister.go | Updates management-cluster listing to accept *azcorearm.ResourceID. |
| internal/database/listers/delete_desire_lister.go | Updates management-cluster listing to accept *azcorearm.ResourceID. |
| internal/database/listers/apply_desire_lister.go | Updates management-cluster listing to accept *azcorearm.ResourceID. |
| internal/database/kube_applier_clients_test.go | Adds tests for plural registry behavior and thread-safety. |
| internal/database/kube_applier_client.go | Implements per-container KubeApplierDBClient, KubeApplierListers, untyped CRUD wiring, and plural registry. |
| internal/database/informers/types.go | Switches informer factory to accept database.KubeApplierListers (per-container). |
| internal/database/informers/list_watch.go | Updates package docs for per-container listers model. |
| internal/database/informers/informers_test.go | Updates informer tests to use mock.Listers() and resourceID management cluster filtering. |
| internal/database/crud_untyped_kube_applier.go | Adds kube-applier-specific untyped CRUD for cleanup by cosmosID. |
| internal/database/crud_kube_applier.go | Removes the old shared-container constant (now per-MC containers). |
| internal/api/fleet/types_management_cluster.go | Adds Status.KubeApplierCosmosContainerName field. |
| backend/pkg/controllers/mismatchcontrollers/delete_orphaned_cosmos_test.go | Adds unit test for orphan desire sweeping across multiple per-MC mock containers. |
| backend/pkg/controllers/mismatchcontrollers/delete_orphaned_cosmos.go | Extends orphan sweep to walk kube-applier containers via KubeApplierDBClients and delete orphaned desires. |
| backend/pkg/controllers/managementclustercontrollers/management_cluster_migration_test.go | Updates expected migrated ManagementCluster status to include kube-applier container name. |
| backend/pkg/app/cosmos_wiring.go | Adds backend wiring helper to build KubeApplierDBClients. |
| backend/pkg/app/backend.go | Wires KubeApplierDBClients into backend options and orphan controller construction. |
| backend/cmd/root.go | Constructs KubeApplierDBClients registry using a DB-backed management cluster lister. |
Comments suppressed due to low confidence (1)
internal/ocm/convert.go:1
- This comment includes unprofessional phrasing (“i hate this a lot”) and a typo (“his will resolve”). Please reword to a neutral explanation of the temporary naming replication and the intended source of truth in phase 2.
| func NewKubeApplierDBClients(database *azcosmos.DatabaseClient, mcLister ManagementClusterLister) KubeApplierDBClients { | ||
| return &kubeApplierDBClients{ | ||
| database: database, |
| func (d *kubeApplierUntypedCRUD) List(ctx context.Context, options *DBClientListResourceDocsOptions) (DBClientIterator[TypedDocument], error) { | ||
| // Empty partitionKey → cross-partition query in list(). | ||
| return list[TypedDocument, TypedDocument](ctx, d.containerClient, "", nil, &d.parentResourceID, options, true) | ||
| } |
| delete, _ := info.DeleteDesires() | ||
| read, _ := info.ReadDesires() | ||
| waitForSync(t, ctx, apply.HasSynced, delete.HasSynced, read.HasSynced) |
26d3ef3 to
c6a5fbd
Compare
The backend talks to many per-management-cluster kube-applier Cosmos containers but wants a single read surface across them all. This adds two new packages: internal/database/unionlisters/kubeapplier — UnionApplyDesireLister, UnionDeleteDesireLister, and UnionReadDesireLister. Each holds a thread-safe set of sublisters keyed by management cluster resourceID with Add/Remove keyed by *azcorearm.ResourceID. List/ListForCluster/ ListForNodePool aggregate across sublisters; GetForCluster/GetForNodePool are first-hit-wins with NotFound treated as "try next"; ListForManagementCluster delegates to the single registered sublister. internal/database/unioninformers/kubeapplier — UnionDesireInformer primitive plus a UnionKubeApplierInformers aggregator that mirrors informers.KubeApplierInformers. AddEventHandler propagates handlers to every current sub-informer and auto-attaches them to subs Added later; the returned cache.ResourceEventHandlerRegistration's HasSynced ANDs over every sub's per-handler registration. The aggregator does not own sub-informer lifecycle — callers start each per-MC bundle on its own ctx and pair cancellation with Remove. That keeps these types pure registries, ready for a higher-level reactor driven by a ManagementClusterInformer. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
c6a5fbd to
c3e1522
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: deads2k The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@deads2k: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Add union informers for our types so we can have controllers fire on changes read desire content.