Remove cluster-service to cosmos nodepool migration - #5059
Conversation
| @@ -1,142 +0,0 @@ | |||
| // Copyright 2026 Microsoft Corporation | |||
There was a problem hiding this comment.
Should we keep this one for now because of defaulting on CS side occurring right now? we are getting it on nodepool creation on the frontend but if at some point we remove the reads from there and we haven't moved defaulting to the RP we would stop getting it
|
/hold #5058 must go to prod before merging |
5d4aae6 to
2478750
Compare
|
/hold cancel |
There was a problem hiding this comment.
Pull request overview
This PR removes the now-obsolete “Cluster Service → Cosmos DB” nodepool property migration/sync path, relying on the frontend/Cosmos to be the source of truth for existing nodepool data (per the direction in #5058).
Changes:
- Removed CS→RP nodepool conversion helpers (including disk/persistence default mapping) from
internal/ocm. - Deleted backend controllers (and tests) that periodically migrated/synced nodepool properties from Cluster Service into Cosmos.
- Unregistered the removed controllers from backend startup.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| internal/ocm/convert.go | Removes CS→RP nodepool conversion and CS-string-to-RP-enum mapping helpers tied to the migration path. |
| internal/database/convert_defaults_consistency_test.go | Drops tests that validated CS→RP default behavior for nodepools (conversion no longer exists). |
| backend/pkg/controllers/nodepoolpropertiescontroller/node_pool_properties_sync.go | Removes controller that synced nodepool version/channel from Cluster Service into Cosmos. |
| backend/pkg/controllers/nodepoolpropertiescontroller/node_pool_properties_sync_test.go | Removes tests for the nodepool properties sync controller. |
| backend/pkg/controllers/nodepoolpropertiescontroller/node_pool_customer_properties_migration.go | Removes controller that migrated “customer properties” (e.g., VMSize) from Cluster Service into Cosmos. |
| backend/pkg/controllers/nodepoolpropertiescontroller/node_pool_customer_properties_migration_test.go | Removes tests for the customer properties migration controller. |
| backend/pkg/app/backend.go | Stops constructing/running the removed nodepool migration/sync controllers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
/lgtm |
|
/hold Revision 2478750 was retested 3 times: holding |
|
/retest |
2478750 to
66e0090
Compare
|
/hold cancel |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: deads2k, miguelsorianod 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 |
These controllers have done their work and the frontend keeps track of all existing data.
66e0090 to
08c1836
Compare
|
New changes are detected. LGTM label has been removed. |
|
simple rebase /label lgtm |
These controllers have done their work and the frontend keeps track of all existing data.
/hold
#5058 must go to prod before merging