Add changefeed support for create/modify#6020
Open
deads2k wants to merge 10 commits into
Open
Conversation
With changefeeds, we react to every change and have more controlled resync, so we don't separate cooldowns.
The code in kubernetes informer/controller relies on presence of same resource version to indicate a resync. This confused event delivery, so adding a resourceversion was the easy solution.
|
[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 |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds Cosmos DB changefeed-backed support to the informer layer so create/modify updates can be observed near-real-time (instead of relying primarily on periodic relists), and updates the backend/kube-applier wiring and tests accordingly.
Changes:
- Introduces a changefeed-driven
ListWatchimplementation and threads aChangeFeedClientthrough informer constructors for resources and kube-applier desires. - Extends in-memory DB mocks to simulate Cosmos changefeed behavior and updates integration/unit tests to use the new informer signatures.
- Updates runtime object metadata to set
ResourceVersionfromInstanceVersion, and bumpsazcosmostov1.5.0-beta.7across modules.
Reviewed changes
Copilot reviewed 123 out of 134 changed files in this pull request and generated 16 comments.
Show a summary per file
| File | Description |
|---|---|
| tooling/hcpctl/cmd/datadump-to-git/cmd.go | Adds parsing support for changefeed log entries and instanceVersion gating when emitting git commits. |
| tooling/cleanup-sweeper/go.sum | Dependency checksum cleanup (removes legacy azure-sdk-for-go v68 entry). |
| test/go.sum | Updates Cosmos SDK checksums to azcosmos v1.5.0-beta.7. |
| test/go.mod | Drops legacy azure-sdk-for-go v68 and bumps azcosmos to beta for tests. |
| test-integration/kube-applier/framework/framework.go | Updates informer construction to pass a changefeed-capable client. |
| test-integration/go.sum | Updates Cosmos SDK checksums to azcosmos v1.5.0-beta.7. |
| test-integration/go.mod | Drops legacy azure-sdk-for-go v68 and bumps azcosmos to beta. |
| test-integration/frontend/informer_test.go | Updates frontend integration tests to pass DB client into informer constructors. |
| test-integration/backend/unionkubeapplierinformers/integration_test.go | Updates union kube-applier informer factory usage for changefeed client parameter. |
| test-integration/backend/launch/notification_test.go | Updates backend informer wiring to include ResourcesDBClient for changefeed. |
| test-integration/backend/controllers/mismatches/delete_orphaned_cosmos_test.go | Adds interface method to wrapper to match updated controller interface expectations. |
| sessiongate/go.sum | Dependency checksum cleanup (removes legacy azure-sdk-for-go v68 entry). |
| mgmt-agent/go.sum | Dependency checksum cleanup (removes legacy azure-sdk-for-go v68 entry). |
| kube-applier/pkg/controllers/read_desire_kubernetes/controller.go | Adds informer ObjectDescription to improve shared informer logging/diagnostics. |
| kube-applier/pkg/app/kube_applier.go | Passes DB client into desire informers (for changefeed support). |
| kube-applier/pkg/app/cosmos_wiring.go | Removes unused context parameter from kube-applier DB client wiring. |
| kube-applier/go.sum | Updates Cosmos SDK checksums to azcosmos v1.5.0-beta.7. |
| kube-applier/go.mod | Drops legacy azure-sdk-for-go v68 and bumps azcosmos to beta. |
| kube-applier/cmd/root.go | Removes ctx plumbing from option wiring and updates DB client constructor call. |
| internal/validation/validators.go | Adds explicit erroring when control-plane versions are unknown during nodepool validation. |
| internal/utils/context.go | Adds helper to add multiple resourceTypes to log values. |
| internal/go.sum | Updates Cosmos SDK checksums to azcosmos v1.5.0-beta.7. |
| internal/go.mod | Drops legacy azure-sdk-for-go v68 and bumps azcosmos to beta. |
| internal/databasetesting/mock_resources_db_client.go | Adds in-memory changefeed recording and exposes GetChangeFeed/GetFeedRanges on mock client. |
| internal/databasetesting/mock_resources_changefeed.go | Implements an in-memory changefeed log + continuation token simulation for mocks. |
| internal/databasetesting/mock_kube_applier_client.go | Adds changefeed support to kube-applier DB client mock. |
| internal/database/unioninformers/kubeapplier/union_informers_test.go | Updates informer construction to include changefeed client. |
| internal/database/unioninformers/kubeapplier/factory.go | Updates factory to pass DB client as changefeed client. |
| internal/database/unioninformers/kubeapplier/controller_test.go | Updates tests to pass DB client into informer wiring. |
| internal/database/kube_applier_client.go | Exposes changefeed methods on kube-applier Cosmos client and updates constructor signature. |
| internal/database/informers/types.go | Threads ChangeFeedClient through kube-applier informer constructors. |
| internal/database/informers/informers.go | Switches desire informers to use changefeed-backed list/watch and adjusts resync/relist settings. |
| internal/database/informers/informers_test.go | Updates informer tests to pass changefeed-capable mock client. |
| internal/database/informers/fleet_informers.go | Adds ObjectDescription for fleet informers. |
| internal/database/informers/changefeed_list_watch.go | Adds the core changefeed-backed ListWatch + watcher implementation. |
| internal/database/database.go | Adds ChangeFeedClient interface and forwards changefeed methods on the resources Cosmos client. |
| internal/api/types_runtime.go | Sets ResourceVersion from InstanceVersion for shared informer semantics. |
| internal/api/kubeapplier/types_runtime.go | Sets ResourceVersion from InstanceVersion for kube-applier runtime types. |
| internal/api/fleet/types_runtime.go | Sets ResourceVersion from InstanceVersion for fleet runtime types. |
| internal/api/arm/types_runtime.go | Sets ResourceVersion from InstanceVersion for ARM runtime types. |
| frontend/pkg/frontend/node_pool.go | Removes delete-time datadump logging hook. |
| frontend/pkg/frontend/external_auth.go | Removes delete-time datadump logging hook. |
| frontend/pkg/frontend/cluster.go | Removes delete-time datadump logging hook. |
| frontend/go.sum | Updates Cosmos SDK checksums to azcosmos v1.5.0-beta.7. |
| frontend/go.mod | Drops legacy azure-sdk-for-go v68 and bumps azcosmos to beta. |
| fleet/pkg/controllers/base/stamp_watching_controller.go | Adds handler logger into informer handler options. |
| fleet/go.sum | Updates Cosmos SDK checksums to azcosmos v1.5.0-beta.7. |
| fleet/go.mod | Drops legacy azure-sdk-for-go v68 and bumps azcosmos to beta. |
| backend/pkg/informers/types.go | Threads ResourcesDBClient into backend informer constructors (for changefeed support). |
| backend/pkg/informers/informers_test.go | Updates backend informer tests to pass mock DB client for changefeed. |
| backend/pkg/controllers/validationcontrollers/nodepool_validation_controller.go | Removes cooldown-checker plumbing from validation controller syncer. |
| backend/pkg/controllers/validationcontrollers/nodepool_validation_controller_test.go | Updates tests for validation controller syncer signature changes. |
| backend/pkg/controllers/validationcontrollers/cluster_validation_controller.go | Removes activeOperationLister dependency for cooldown gating. |
| backend/pkg/controllers/upgradecontrollers/trigger_node_pool_upgrade_controller.go | Removes cooldown-checker plumbing from trigger controller. |
| backend/pkg/controllers/upgradecontrollers/trigger_control_plane_upgrade_controller.go | Changes gating logic to use cluster ActiveOperationID lookup and adds additional logging. |
| backend/pkg/controllers/upgradecontrollers/trigger_control_plane_upgrade_controller_test.go | Updates tests to seed ActiveOperationID and new gating function signature. |
| backend/pkg/controllers/upgradecontrollers/nodepool_version_controller.go | Adds additional informer subscription (SPC) and updates NeedsWork signature to include SPC. |
| backend/pkg/controllers/upgradecontrollers/nodepool_version_controller_test.go | Updates tests for NeedsWork signature and removed cooldown checker. |
| backend/pkg/controllers/upgradecontrollers/nodepool_active_version_real_cosmos_test.go | Updates tests for removed cooldown checker. |
| backend/pkg/controllers/upgradecontrollers/nodepool_active_version_controller.go | Returns nil cooldown checker (effectively disables gating) after refactor. |
| backend/pkg/controllers/upgradecontrollers/nodepool_active_version_controller_test.go | Updates tests for removed cooldown checker. |
| backend/pkg/controllers/upgradecontrollers/control_plane_desired_version_controller.go | Switches create-operation gating to use ActiveOperationID lookup. |
| backend/pkg/controllers/upgradecontrollers/control_plane_desired_version_controller_test.go | Updates tests to implement new lister Get() and seed ActiveOperationID. |
| backend/pkg/controllers/upgradecontrollers/control_plane_active_version_controller.go | Removes cooldown-checker plumbing from controller. |
| backend/pkg/controllers/upgradecontrollers/control_plane_active_version_controller_test.go | Updates tests for removed cooldown checker. |
| backend/pkg/controllers/statuscontrollers/nodepool_degraded_aggregator.go | Removes cooldown-checker plumbing and treats NotFound on replace as non-fatal. |
| backend/pkg/controllers/statuscontrollers/nodepool_degraded_aggregator_test.go | Updates tests for removed cooldown checker. |
| backend/pkg/controllers/statuscontrollers/externalauth_degraded_aggregator.go | Removes cooldown-checker plumbing and treats NotFound on replace as non-fatal. |
| backend/pkg/controllers/statuscontrollers/externalauth_degraded_aggregator_test.go | Updates tests for removed cooldown checker. |
| backend/pkg/controllers/statuscontrollers/cluster_degraded_aggregator.go | Removes cooldown-checker plumbing and treats NotFound on replace as non-fatal. |
| backend/pkg/controllers/statuscontrollers/cluster_degraded_aggregator_test.go | Updates tests for removed cooldown checker. |
| backend/pkg/controllers/statuscontrollers/aggregator_testhelpers_test.go | Removes unused always-sync cooldown checker helper. |
| backend/pkg/controllers/operationcontrollers/generic_operation.go | Adds handler logger and introduces QueueForInformers method stub. |
| backend/pkg/controllers/nodepooldeletion/node_pool_deletion_controller.go | Removes cooldown-checker plumbing from deletion controller. |
| backend/pkg/controllers/nodepooldeletion/node_pool_deletion_controller_test.go | Updates tests for removed cooldown checker. |
| backend/pkg/controllers/nodepooldeletion/node_pool_cluster_service_id_clearer.go | Removes cooldown-checker plumbing from clearer controller. |
| backend/pkg/controllers/nodepooldeletion/node_pool_cluster_service_id_clearer_test.go | Updates tests for removed cooldown checker. |
| backend/pkg/controllers/nodepooldeletion/node_pool_cluster_service_delete_dispatch_controller.go | Removes cooldown-checker plumbing from delete-dispatch controller. |
| backend/pkg/controllers/nodepooldeletion/node_pool_cluster_service_delete_dispatch_controller_test.go | Updates tests for removed cooldown checker. |
| backend/pkg/controllers/nodepooldeletion/node_pool_child_resources_cleanup_controller.go | Removes cooldown-checker plumbing from cleanup controller. |
| backend/pkg/controllers/nodepooldeletion/node_pool_child_resources_cleanup_controller_test.go | Updates tests for removed cooldown checker. |
| backend/pkg/controllers/nodepoolcreationcontrollers/node_pool_cluster_service_create_controller.go | Removes cooldown-checker plumbing from nodepool create controller. |
| backend/pkg/controllers/nodepoolcreationcontrollers/node_pool_cluster_service_create_controller_test.go | Updates tests for removed cooldown checker. |
| backend/pkg/controllers/mismatchcontrollers/delete_orphaned_cosmos.go | Adds QueueForInformers stub method. |
| backend/pkg/controllers/mismatchcontrollers/cluster_service_cluster_matching.go | Adds error suppression for “no healthy upstream” and QueueForInformers stub. |
| backend/pkg/controllers/managementclustercontrollers/management_cluster_placement_sync.go | Removes cooldown-checker plumbing from placement syncer. |
| backend/pkg/controllers/managementclustercontrollers/management_cluster_placement_sync_test.go | Updates tests for removed cooldown checker. |
| backend/pkg/controllers/externalauthdeletion/external_auth_deletion_controller.go | Removes cooldown-checker plumbing from deletion controller. |
| backend/pkg/controllers/externalauthdeletion/external_auth_deletion_controller_test.go | Updates tests for removed cooldown checker. |
| backend/pkg/controllers/externalauthdeletion/external_auth_cluster_service_id_clearer.go | Removes cooldown-checker plumbing from clearer controller. |
| backend/pkg/controllers/externalauthdeletion/external_auth_cluster_service_id_clearer_test.go | Updates tests for removed cooldown checker. |
| backend/pkg/controllers/externalauthdeletion/external_auth_cluster_service_delete_dispatch_controller.go | Removes cooldown-checker plumbing from delete-dispatch controller. |
| backend/pkg/controllers/externalauthdeletion/external_auth_cluster_service_delete_dispatch_controller_test.go | Updates tests for removed cooldown checker. |
| backend/pkg/controllers/externalauthdeletion/external_auth_child_resources_cleanup_controller.go | Removes cooldown-checker plumbing from cleanup controller. |
| backend/pkg/controllers/externalauthdeletion/external_auth_child_resources_cleanup_controller_test.go | Updates tests for removed cooldown checker. |
| backend/pkg/controllers/do_nothing.go | Adds QueueForInformers stub method. |
| backend/pkg/controllers/datadumpcontrollers/dump_subscription_non_cluster.go | Uses time-based cooldown checker directly rather than active-op cooldown field. |
| backend/pkg/controllers/datadumpcontrollers/dump_cluster_recursive.go | Removes cooldown-checker plumbing from recursive dump controller. |
| backend/pkg/controllers/datadumpcontrollers/cs_state_dump.go | Removes redundant cooldown checker field and relies on nextDumpChecker. |
| backend/pkg/controllers/datadumpcontrollers/cs_state_dump_test.go | Updates tests for removed cooldown checker. |
| backend/pkg/controllers/datadumpcontrollers/billing_dump.go | Removes cooldown-checker plumbing from billing dump controller. |
| backend/pkg/controllers/datadumpcontrollers/billing_dump_test.go | Updates tests and removes now-unnecessary CooldownChecker test. |
| backend/pkg/controllers/create_nodepool_scoped_read_desires_controller.go | Removes cooldown-checker plumbing from read-desire creation controller. |
| backend/pkg/controllers/create_cluster_scoped_read_desires_controller.go | Removes cooldown-checker plumbing from read-desire creation controller. |
| backend/pkg/controllers/controllerutils/util.go | Extends Controller interface with QueueForInformers. |
| backend/pkg/controllers/controllerutils/nodepool_watching_controller.go | Removes syncer CooldownChecker requirement and returns nil for controller cooldown. |
| backend/pkg/controllers/controllerutils/generic_watching_controller.go | Adds per-handler logger and relaxes cooldown gating when nil. |
| backend/pkg/controllers/controllerutils/external_auth_watching_controller.go | Removes syncer CooldownChecker requirement and returns nil for controller cooldown. |
| backend/pkg/controllers/controllerutils/cluster_watching_controller.go | Removes syncer CooldownChecker requirement and returns nil for controller cooldown. |
| backend/pkg/controllers/clusterpropertiescontroller/identity_migration.go | Removes cooldown-checker plumbing from identity migration controller. |
| backend/pkg/controllers/clusterpropertiescontroller/identity_migration_test.go | Updates tests for removed cooldown checker. |
| backend/pkg/controllers/clusterpropertiescontroller/desired_control_plane_size_sync.go | Removes cooldown-checker plumbing from desired control plane size controller. |
| backend/pkg/controllers/clusterpropertiescontroller/desired_control_plane_size_sync_test.go | Updates tests for removed cooldown checker. |
| backend/pkg/controllers/clusterpropertiescontroller/cluster_properties_test_helpers_test.go | Removes unused always-sync cooldown checker helper. |
| backend/pkg/controllers/clusterpropertiescontroller/cluster_properties_sync.go | Removes cooldown-checker plumbing from cluster properties syncer. |
| backend/pkg/controllers/clusterpropertiescontroller/cluster_properties_sync_test.go | Updates tests for removed cooldown checker. |
| backend/pkg/controllers/clusterpropertiescontroller/cluster_base_domain_prefix_sync.go | Removes cooldown-checker plumbing from base domain prefix syncer. |
| backend/pkg/controllers/clusterpropertiescontroller/cluster_base_domain_prefix_sync_test.go | Updates tests for removed cooldown checker. |
| backend/pkg/controllers/clusterdeletion/cluster_deletion_controller.go | Removes cooldown-checker plumbing from cluster deletion controller. |
| backend/pkg/controllers/clusterdeletion/cluster_deletion_controller_test.go | Updates tests for removed cooldown checker. |
| backend/pkg/controllers/clusterdeletion/cluster_cluster_service_id_clearer.go | Removes cooldown-checker plumbing from clearer controller. |
| backend/pkg/controllers/clusterdeletion/cluster_cluster_service_id_clearer_test.go | Updates tests for removed cooldown checker. |
| backend/pkg/controllers/clusterdeletion/cluster_cluster_service_delete_dispatch_controller.go | Removes cooldown-checker plumbing from delete-dispatch controller. |
| backend/pkg/controllers/clusterdeletion/cluster_cluster_service_delete_dispatch_controller_test.go | Updates tests for removed cooldown checker. |
| backend/pkg/controllers/clusterdeletion/cluster_child_resources_cleanup_controller.go | Removes cooldown-checker plumbing from cleanup controller. |
| backend/pkg/controllers/clusterdeletion/cluster_child_resources_cleanup_controller_test.go | Updates tests for removed cooldown checker. |
| backend/pkg/controllers/billingcontrollers/orphaned_billing_cleanup.go | Adds QueueForInformers stub method. |
| backend/pkg/app/backend.go | Wires ResourcesDBClient into backend informers and updates controller constructors accordingly. |
| backend/go.sum | Updates Cosmos SDK checksums to azcosmos v1.5.0-beta.7. |
| backend/go.mod | Drops legacy azure-sdk-for-go v68 and bumps azcosmos to beta. |
| admin/server/go.sum | Updates Cosmos SDK checksums to azcosmos v1.5.0-beta.7. |
| admin/server/go.mod | Drops legacy azure-sdk-for-go v68 and bumps azcosmos to beta. |
Comment on lines
+231
to
234
| if cooldownChecker := c.syncer.CooldownChecker(); cooldownChecker != nil && !c.syncer.CooldownChecker().CanSync(ctx, key) { | ||
| logger.Info("Skipping notification") | ||
| return | ||
| } |
Comment on lines
+94
to
+99
| prevFeedWatcher := c.currentWatcher | ||
| c.currentWatcher = nil | ||
| if prevFeedWatcher != nil { | ||
| prevFeedWatcher.Stop() | ||
| } | ||
|
|
Comment on lines
+107
to
+110
| // cleanup the watcher we aren't going to use. This would happen when we re-enter anyway, but we can do slightly better here | ||
| c.currentWatcher.Stop() | ||
| c.currentWatcher = nil | ||
|
|
Comment on lines
133
to
+135
| // NewKubeApplierDBClient wraps a pre-opened Cosmos container client for a single | ||
| // management cluster. | ||
| func NewKubeApplierDBClient(container *azcosmos.ContainerClient, managementClusterResourceID *azcorearm.ResourceID) KubeApplierDBClient { | ||
| // management cluster. Feed ranges are fetched and cached at construction time. | ||
| func NewKubeApplierDBClient(container *azcosmos.ContainerClient, managementClusterResourceID *azcorearm.ResourceID) (KubeApplierDBClient, error) { |
Comment on lines
+212
to
+214
| // Cache feed ranges for the duration of the DBClient instance. | ||
| // Even if Cosmos DB splits a physical partition due to throughput | ||
| // or data size limits being exceeded, the feed ranges remain valid. |
Comment on lines
+119
to
+121
| func (c *orphanedBillingCleanup) QueueForInformers(resyncDuration time.Duration, notifiers ...controllerutils.Notifier) error { | ||
| panic("not implemented") | ||
| } |
Comment on lines
+277
to
+279
| func (c *deleteOrphanedCosmosResources) QueueForInformers(resyncDuration time.Duration, notifiers ...controllerutils.Notifier) error { | ||
| panic("not implemented") | ||
| } |
Comment on lines
+113
to
+115
| func (c *doNothingExample) QueueForInformers(resyncDuration time.Duration, notifiers ...controllerutils.Notifier) error { | ||
| panic("not implemented") | ||
| } |
| om.Name = strings.ToLower(o.ID.String()) | ||
| } | ||
| // shared_informer uses ResourceVersion to determine if an event is a sync | ||
| om.ResourceVersion = strconv.Itoa(int(o.InstanceVersion)) |
Comment on lines
+130
to
+133
| // cleanup the watcher we aren't going to use. This would happen when we re-enter anyway, but we can do slightly better here | ||
| c.currentWatcher.Stop() | ||
| c.currentWatcher = nil | ||
|
|
|
@deads2k: The following tests 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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a new PR to avoid the noise of the old PR since it had so many resolved, but unmarked bot comments.