stop serializing old cluster fields#5174
Conversation
|
[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 |
There was a problem hiding this comment.
Pull request overview
This PR is the third step in the cluster storage migration plan (per #5095 / #5171) and updates Cosmos DB serialization to stop writing the legacy cluster fields, relying on the new inline api.HCPOpenShiftCluster shape (with cosmosMetadata) instead.
Changes:
- Simplifies cluster Cosmos conversion by writing/reading the inlined
api.HCPOpenShiftClusterrather than legacyintermediateResourceDoc/internalStatefields. - Updates internal cluster model to embed
CosmosMetadata, and adjusts conversion/copy logic and API-version converters accordingly. - Updates integration/unit test fixtures and Cosmos-compare artifacts to match the new stored document shape.
Reviewed changes
Copilot reviewed 91 out of 91 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| test-integration/utils/databasemutationhelpers/per_resource_comparer.go | Ignore newly-variable/relocated fields in Cosmos comparison. |
| test-integration/frontend/informer_test.go | Populate CosmosMetadata.ResourceID in informer fixtures. |
| test-integration/frontend/artifacts/FrontendCRUD/NodePool/read-old-data/09-cosmosCompare-confirm-update/cluster-create-with-tags.json | Update stored cluster artifact to new inline shape. |
| test-integration/frontend/artifacts/FrontendCRUD/NodePool/read-old-data/01-load-old-data/cluster-create-with-tags.json | Update stored cluster artifact to new inline shape. |
| test-integration/frontend/artifacts/FrontendCRUD/NodePool/create-current/06-cosmosCompare-confirm-content/cluster-create-with-tags.json | Update expected Cosmos content for new shape. |
| test-integration/frontend/artifacts/FrontendCRUD/NodePool/cluster-deleting/02-loadCosmos-cluster/cosmos-01-cluster.json | Update deleting cluster artifact to new shape. |
| test-integration/frontend/artifacts/FrontendCRUD/NodePool/cluster-creating/02-loadCosmos-cluster/cosmos-01-cluster.json | Update creating cluster artifact to new shape. |
| test-integration/frontend/artifacts/FrontendCRUD/Migration/read-new-data/18-cosmosCompare-confirm/cluster-create-with-tags.json | Update migration artifact to new shape. |
| test-integration/frontend/artifacts/FrontendCRUD/Migration/read-new-data/01-load-old-data/cluster-create-with-tags.json | Update migration artifact to new shape. |
| test-integration/frontend/artifacts/FrontendCRUD/Migration/migrate-old-data/99-cosmosCompare-confirm-migration/cluster-create-with-tags.json | Update migration artifact to new shape. |
| test-integration/frontend/artifacts/FrontendCRUD/Migration/migrate-old-data/01-load-old-data/cluster-create-with-tags.json | Update migration artifact to new shape. |
| test-integration/frontend/artifacts/FrontendCRUD/ExternalAuth/read-old-data/09-cosmosCompare-confirm-update/cluster-create-with-tags.json | Update ExternalAuth flow cluster artifact to new shape. |
| test-integration/frontend/artifacts/FrontendCRUD/ExternalAuth/read-old-data/01-load-old-data/cluster-create-with-tags.json | Update ExternalAuth flow cluster artifact to new shape. |
| test-integration/frontend/artifacts/FrontendCRUD/ExternalAuth/create-current/06-cosmosCompare-ending-content/cluster-create-with-tags.json | Update expected Cosmos content for new shape. |
| test-integration/frontend/artifacts/FrontendCRUD/ExternalAuth/cluster-deleting/02-loadCosmos-cluster/cosmos-01-cluster.json | Update deleting cluster artifact to new shape. |
| test-integration/frontend/artifacts/FrontendCRUD/ExternalAuth/cluster-creating/02-loadCosmos-cluster/cosmos-01-cluster.json | Update creating cluster artifact to new shape. |
| test-integration/frontend/artifacts/FrontendCRUD/Cluster/read-old-data/09-cosmosCompare-confirm-update/cluster-create-with-tags.json | Update Cluster CRUD artifact to new shape. |
| test-integration/frontend/artifacts/FrontendCRUD/Cluster/read-old-data/01-load-old-data/create-with-tags.json | Update Cluster CRUD artifact to new shape. |
| test-integration/frontend/artifacts/FrontendCRUD/Cluster/delete-with-pending-nodepool-operation/05-cosmosCompare-final-state/cluster-test-cluster.json | Update delete scenario artifact to new shape. |
| test-integration/frontend/artifacts/FrontendCRUD/Cluster/delete-with-pending-cluster-operation/03-cosmosCompare-final-state/cluster-test-cluster.json | Update delete scenario artifact to new shape. |
| test-integration/frontend/artifacts/FrontendCRUD/Cluster/create-current/09-cosmosCompare-confirm-update/cluster-create-with-tags.json | Update create scenario artifact to new shape. |
| test-integration/frontend/artifacts/FrontendCRUD/Cluster/create-current/02-cosmosCompare-confirm-content/cluster-create-with-tags.json | Update create scenario artifact to new shape. |
| test-integration/frontend/artifacts/FrontendCRUD/AdminCredentials/revoke/01-load-initial-cosmos-state/01-cluster.json | Update AdminCredentials cluster artifact to new shape. |
| test-integration/frontend/artifacts/FrontendCRUD/AdminCredentials/revoke-provisioning-conflict/01-load-initial-cosmos-state/01-cluster.json | Update conflict scenario artifact to new shape. |
| test-integration/frontend/artifacts/FrontendCRUD/AdminCredentials/revoke-forbidden-afec-pending/01-load-initial-cosmos-state/01-cluster.json | Update AFEC pending scenario artifact to new shape. |
| test-integration/frontend/artifacts/FrontendCRUD/AdminCredentials/revoke-forbidden-afec-missing/01-load-initial-cosmos-state/01-cluster.json | Update AFEC missing scenario artifact to new shape. |
| test-integration/frontend/artifacts/FrontendCRUD/AdminCredentials/request/01-load-initial-cosmos-state/01-cluster.json | Update request scenario artifact to new shape. |
| test-integration/frontend/artifacts/FrontendCRUD/AdminCredentials/request-provisioning-conflict/01-load-initial-cosmos-state/01-cluster.json | Update conflict scenario artifact to new shape. |
| test-integration/frontend/artifacts/DatabaseCRUD/UntypedCRUD/basic/12-untypedList-resourcegroup/create-with-tags-cluster.json | Update untyped list artifact for new stored shape. |
| test-integration/frontend/artifacts/DatabaseCRUD/UntypedCRUD/basic/10-untypedList-resourcegroup/create-with-tags-cluster.json | Update untyped list artifact for new stored shape. |
| test-integration/frontend/artifacts/DatabaseCRUD/UntypedCRUD/basic/07-untypedListRecursive-resourcegroup-via-child/create-with-tags-cluster.json | Update untyped recursive list artifact for new stored shape. |
| test-integration/frontend/artifacts/DatabaseCRUD/UntypedCRUD/basic/05-untypedListRecursive-subscription/create-with-tags-cluster.json | Update untyped recursive list artifact for new stored shape. |
| test-integration/frontend/artifacts/DatabaseCRUD/UntypedCRUD/basic/04-untypedListRecursive-resourcegroup/create-with-tags-cluster.json | Update untyped recursive list artifact for new stored shape. |
| test-integration/frontend/artifacts/DatabaseCRUD/UntypedCRUD/basic/01-load-initial/create-with-tags-cluster.json | Update initial load artifact for new stored shape. |
| test-integration/backend/launch/metrics_test.go | Populate CosmosMetadata.ResourceID in metrics fixture. |
| test-integration/backend/controllers/mismatches/artifacts/nodepool/present_nodepool/99-cosmosCompare-end-state/cluster.json | Update mismatch test artifacts for new stored shape. |
| test-integration/backend/controllers/mismatches/artifacts/nodepool/present_nodepool/00-load-initial-state/cluster.json | Update mismatch test artifacts for new stored shape. |
| test-integration/backend/controllers/mismatches/artifacts/delete_orphaned_cosmos/controller_under_missing_nodepool_deleted/99-cosmosCompare-end-state/cluster.json | Update mismatch test artifacts for new stored shape. |
| test-integration/backend/controllers/mismatches/artifacts/delete_orphaned_cosmos/controller_under_missing_nodepool_deleted/00-load-initial-state/cluster.json | Update mismatch test artifacts for new stored shape. |
| test-integration/backend/controllers/mismatches/artifacts/delete_orphaned_cosmos/all_parents_exist/99-cosmosCompare-end-state/cluster.json | Update mismatch test artifacts for new stored shape. |
| test-integration/backend/controllers/mismatches/artifacts/delete_orphaned_cosmos/all_parents_exist/00-load-initial-state/cluster.json | Update mismatch test artifacts for new stored shape. |
| test-integration/backend/controllers/mismatches/artifacts/cluster/remove_orphaned_cluster_descendents/00-load-initial-state/cluster-safe.json | Update mismatch test artifacts for new stored shape. |
| test-integration/backend/controllers/mismatches/artifacts/cluster/present_cluster/99-cosmosCompare-end-state/cluster.json | Update mismatch test artifacts for new stored shape. |
| test-integration/backend/controllers/mismatches/artifacts/cluster/present_cluster/00-load-initial-state/cluster.json | Update mismatch test artifacts for new stored shape. |
| test-integration/backend/controllers/do_nothing/artifacts/sync_cluster/00-load-initial-state/cluster.json | Update do-nothing controller artifacts for new stored shape. |
| test-integration/admin/artifacts/AdminCRUD/HCP/lowercase-middleware/00-load-initial-cosmos-state/01-cluster.json | Update admin artifacts for new stored shape. |
| test-integration/admin/artifacts/AdminCRUD/HCP/hello-world/00-load-initial-cosmos-state/01-cluster.json | Update admin artifacts for new stored shape. |
| test-integration/admin/artifacts/AdminCRUD/HCP/cosmosdump/00-load-initial-cosmos-state/01-cluster.json | Update admin artifacts for new stored shape. |
| test-integration/admin/artifacts/AdminCRUD/HCP/breakglass/00-load-initial-cosmos-state/01-cluster.json | Update admin artifacts for new stored shape. |
| internal/serverutils/dump_billing_test.go | Populate CosmosMetadata.ResourceID in billing dump fixtures. |
| internal/databasetesting/mock_dbclient_test.go | Populate CosmosMetadata.ResourceID in mock DB tests. |
| internal/database/types_hcpcluster.go | Remove legacy serialized fields; inline api.HCPOpenShiftCluster. |
| internal/database/convert_defaults_consistency_test.go | Update tests to build clusters using new inline fields. |
| internal/database/convert_cluster.go | Update cluster Cosmos<->internal conversion for new schema. |
| internal/database/convert_cluster_test.go | Update round-trip tests for new CosmosMetadata/UID handling. |
| internal/conversion/readonly_cluster.go | Preserve CosmosMetadata across replace-round-trip copies. |
| internal/api/zz_generated.deepcopy.go | Ensure CosmosMetadata deep-copies correctly. |
| internal/api/v20251223preview/zero_value_roundtrip_test.go | Provide CosmosMetadata in baseline internal cluster fixture. |
| internal/api/v20251223preview/hcpopenshiftclusters_methods.go | Use ResourceID when serializing external ID; set internal ResourceID. |
| internal/api/v20251223preview/conversion_fuzz_test.go | Add fuzz customizations for CosmosMetadata fields. |
| internal/api/v20240610preview/hcpopenshiftclusters_methods.go | Use ResourceID when serializing external ID; set internal ResourceID. |
| internal/api/v20240610preview/conversion_fuzz_test.go | Add fuzz customizations for CosmosMetadata fields. |
| internal/api/types_cluster.go | Embed CosmosMetadata in internal cluster; remove legacy CosmosETag plumbing. |
| frontend/pkg/frontend/helpers_test.go | Populate CosmosMetadata.ResourceID in frontend helper tests. |
| frontend/pkg/frontend/frontend_test.go | Populate CosmosMetadata.ResourceID in frontend tests. |
| frontend/pkg/frontend/cluster.go | Set/validate new canonical ResourceID and adjust equality check. |
| backend/pkg/listertesting/slice_listers_test.go | Populate CosmosMetadata.ResourceID in lister tests. |
| backend/pkg/informers/informers_test.go | Populate CosmosMetadata.ResourceID in informer tests. |
| backend/pkg/controllers/upgradecontrollers/nodepool_version_controller_test.go | Populate CosmosMetadata.ResourceID in upgrade controller tests. |
| backend/pkg/controllers/upgradecontrollers/control_plane_desired_version_controller_test.go | Populate CosmosMetadata.ResourceID in upgrade controller tests. |
| backend/pkg/controllers/upgradecontrollers/control_plane_active_version_controller_test.go | Populate CosmosMetadata.ResourceID in upgrade controller tests. |
| backend/pkg/controllers/read_and_persist_cluster_scoped_maestro_readonly_bundles_content_controller_test.go | Populate CosmosMetadata.ResourceID in maestro bundle tests. |
| backend/pkg/controllers/operationcontrollers/test_helpers_test.go | Populate CosmosMetadata.ResourceID in operation controller fixtures. |
| backend/pkg/controllers/nodepoolpropertiescontroller/node_pool_properties_sync_test.go | Populate CosmosMetadata.ResourceID in nodepool properties tests. |
| backend/pkg/controllers/metricscontrollers/resource_metrics_controller_test.go | Populate CosmosMetadata.ResourceID in resource metrics tests. |
| backend/pkg/controllers/maestro_readonly_bundle_helpers_test.go | Populate CosmosMetadata.ResourceID in maestro helper tests. |
| backend/pkg/controllers/datadumpcontrollers/cs_state_dump_test.go | Populate CosmosMetadata.ResourceID in state dump tests. |
| backend/pkg/controllers/create_cluster_scoped_maestro_readonly_bundles_controller_test.go | Populate CosmosMetadata.ResourceID in bundle creation tests. |
| backend/pkg/controllers/clusterpropertiescontroller/cluster_properties_sync_test.go | Populate CosmosMetadata.ResourceID in cluster properties tests. |
| backend/pkg/controllers/billingcontrollers/orphaned_billing_cleanup_test.go | Populate CosmosMetadata.ResourceID in billing cleanup tests. |
| backend/pkg/controllers/billingcontrollers/create_billing_doc_test.go | Populate CosmosMetadata.ResourceID in billing doc creation tests. |
| admin/server/handlers/hcp/serialconsole_test.go | Populate CosmosMetadata.ResourceID in admin handler tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
f0a4c21 to
496d801
Compare
|
/retest |
496d801 to
0216e8b
Compare
| internalObj := cosmosObj.DeepCopy() | ||
| internalObj.ExistingCosmosUID = cosmosObj.ID | ||
| internalObj.SetEtag(cosmosObj.CosmosETag) | ||
| if internalObj.GetResourceID() == nil { | ||
| if cosmosObj.ResourceID != nil { | ||
| internalObj.SetResourceID(cosmosObj.ResourceID) | ||
| } else { |
|
I should be back before this merges, but cursory read of the last commit LGTM. |
|
/hold until #5171 makes it to prod |
0216e8b to
40747ff
Compare
|
@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. |
|
PR needs rebase. 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. |
|
replaced by #5395 |
This is part three of of the plan in #5095 and it stops serializing unnecessary fields. Cannot merge until #5171 goes to prod