read new cluster serialization#5171
Conversation
There was a problem hiding this comment.
Pull request overview
This PR continues the migration toward using GenericDocument[api.HCPOpenShiftCluster] for cluster storage by moving CosmosMetadata into the internal HCPOpenShiftCluster representation and updating DB conversion paths and tests/artifacts accordingly.
Changes:
- Inline
api.HCPOpenShiftClusterinto the Cosmos DBHCPClusterPropertiesshape and treatCosmosMetadata.ResourceIDas the canonical Cosmos-side identifier. - Update cluster conversion/helpers to preserve/copy
CosmosMetadata(including ETag/ResourceID) across read/replace flows. - Refresh a large set of integration test fixtures (“cosmosCompare” artifacts) to reflect the updated serialized cluster document shape.
Reviewed changes
Copilot reviewed 91 out of 91 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| test-integration/utils/databasemutationhelpers/per_resource_comparer.go | Ignores additional variable/UUID fields introduced by updated cluster serialization. |
| test-integration/frontend/informer_test.go | Populates CosmosMetadata.ResourceID in informer integration test clusters. |
| test-integration/frontend/artifacts/FrontendCRUD/NodePool/read-old-data/09-cosmosCompare-confirm-update/cluster-create-with-tags.json | Updates expected stored cluster doc shape to include inlined cluster fields and cosmos metadata. |
| test-integration/frontend/artifacts/FrontendCRUD/NodePool/read-old-data/01-load-old-data/cluster-create-with-tags.json | Updates loaded “old data” fixture to include inlined cluster fields/cosmos metadata. |
| test-integration/frontend/artifacts/FrontendCRUD/NodePool/create-current/06-cosmosCompare-confirm-content/cluster-create-with-tags.json | Updates expected cluster doc content for current-create flow. |
| test-integration/frontend/artifacts/FrontendCRUD/NodePool/cluster-deleting/02-loadCosmos-cluster/cosmos-01-cluster.json | Updates “cluster deleting” cosmos fixture for new inlined fields. |
| test-integration/frontend/artifacts/FrontendCRUD/NodePool/cluster-creating/02-loadCosmos-cluster/cosmos-01-cluster.json | Updates “cluster creating” cosmos fixture for new inlined fields. |
| test-integration/frontend/artifacts/FrontendCRUD/Migration/read-new-data/18-cosmosCompare-confirm/cluster-create-with-tags.json | Updates migration test expected doc with inlined fields. |
| test-integration/frontend/artifacts/FrontendCRUD/Migration/read-new-data/01-load-old-data/cluster-create-with-tags.json | Updates migration “load old data” fixture to match new doc shape. |
| test-integration/frontend/artifacts/FrontendCRUD/Migration/migrate-old-data/99-cosmosCompare-confirm-migration/cluster-create-with-tags.json | Updates expected migrated doc shape for new inlined fields. |
| test-integration/frontend/artifacts/FrontendCRUD/Migration/migrate-old-data/01-load-old-data/cluster-create-with-tags.json | Updates migration old-data fixture to include inlined fields. |
| test-integration/frontend/artifacts/FrontendCRUD/ExternalAuth/read-old-data/09-cosmosCompare-confirm-update/cluster-create-with-tags.json | Updates ExternalAuth flow expected cluster doc for new inlined fields. |
| test-integration/frontend/artifacts/FrontendCRUD/ExternalAuth/read-old-data/01-load-old-data/cluster-create-with-tags.json | Updates ExternalAuth flow old-data fixture for new inlined fields. |
| test-integration/frontend/artifacts/FrontendCRUD/ExternalAuth/create-current/06-cosmosCompare-ending-content/cluster-create-with-tags.json | Updates expected final cluster doc content for ExternalAuth create-current flow. |
| test-integration/frontend/artifacts/FrontendCRUD/ExternalAuth/cluster-deleting/02-loadCosmos-cluster/cosmos-01-cluster.json | Updates ExternalAuth “cluster deleting” cosmos fixture for new inlined fields. |
| test-integration/frontend/artifacts/FrontendCRUD/ExternalAuth/cluster-creating/02-loadCosmos-cluster/cosmos-01-cluster.json | Updates ExternalAuth “cluster creating” cosmos fixture for new inlined fields. |
| test-integration/frontend/artifacts/FrontendCRUD/Cluster/read-old-data/09-cosmosCompare-confirm-update/cluster-create-with-tags.json | Updates Cluster CRUD expected doc for new inlined fields. |
| test-integration/frontend/artifacts/FrontendCRUD/Cluster/read-old-data/01-load-old-data/create-with-tags.json | Updates Cluster CRUD old-data fixture for new inlined fields. |
| test-integration/frontend/artifacts/FrontendCRUD/Cluster/delete-with-pending-nodepool-operation/05-cosmosCompare-final-state/cluster-test-cluster.json | Updates delete-with-pending-nodepool-operation expected final doc for new inlined fields. |
| test-integration/frontend/artifacts/FrontendCRUD/Cluster/delete-with-pending-cluster-operation/03-cosmosCompare-final-state/cluster-test-cluster.json | Updates delete-with-pending-cluster-operation expected final doc for new inlined fields. |
| test-integration/frontend/artifacts/FrontendCRUD/Cluster/create-current/09-cosmosCompare-confirm-update/cluster-create-with-tags.json | Updates create-current confirm-update expected doc for new inlined fields. |
| test-integration/frontend/artifacts/FrontendCRUD/Cluster/create-current/02-cosmosCompare-confirm-content/cluster-create-with-tags.json | Updates create-current confirm-content expected doc for new inlined fields. |
| test-integration/frontend/artifacts/FrontendCRUD/AdminCredentials/revoke/01-load-initial-cosmos-state/01-cluster.json | Updates AdminCredentials revoke initial cosmos fixture for new inlined fields. |
| test-integration/frontend/artifacts/FrontendCRUD/AdminCredentials/revoke-provisioning-conflict/01-load-initial-cosmos-state/01-cluster.json | Updates provisioning-conflict fixture with inlined fields/cosmos metadata. |
| test-integration/frontend/artifacts/FrontendCRUD/AdminCredentials/revoke-forbidden-afec-pending/01-load-initial-cosmos-state/01-cluster.json | Updates forbidden-afec-pending fixture with inlined fields/cosmos metadata. |
| test-integration/frontend/artifacts/FrontendCRUD/AdminCredentials/revoke-forbidden-afec-missing/01-load-initial-cosmos-state/01-cluster.json | Updates forbidden-afec-missing fixture with inlined fields/cosmos metadata. |
| test-integration/frontend/artifacts/FrontendCRUD/AdminCredentials/request/01-load-initial-cosmos-state/01-cluster.json | Updates AdminCredentials request initial cosmos fixture with inlined fields/cosmos metadata. |
| test-integration/frontend/artifacts/FrontendCRUD/AdminCredentials/request-provisioning-conflict/01-load-initial-cosmos-state/01-cluster.json | Updates request provisioning-conflict fixture with inlined fields/cosmos metadata. |
| test-integration/frontend/artifacts/DatabaseCRUD/UntypedCRUD/basic/12-untypedList-resourcegroup/create-with-tags-cluster.json | Updates untyped CRUD fixture to include inlined cluster fields/cosmos metadata. |
| test-integration/frontend/artifacts/DatabaseCRUD/UntypedCRUD/basic/10-untypedList-resourcegroup/create-with-tags-cluster.json | Updates untyped CRUD fixture to include inlined cluster fields/cosmos metadata. |
| test-integration/frontend/artifacts/DatabaseCRUD/UntypedCRUD/basic/07-untypedListRecursive-resourcegroup-via-child/create-with-tags-cluster.json | Updates recursive list fixture with inlined fields/cosmos metadata. |
| test-integration/frontend/artifacts/DatabaseCRUD/UntypedCRUD/basic/05-untypedListRecursive-subscription/create-with-tags-cluster.json | Updates recursive subscription list fixture with inlined fields/cosmos metadata. |
| test-integration/frontend/artifacts/DatabaseCRUD/UntypedCRUD/basic/04-untypedListRecursive-resourcegroup/create-with-tags-cluster.json | Updates recursive resource-group list fixture with inlined fields/cosmos metadata. |
| test-integration/frontend/artifacts/DatabaseCRUD/UntypedCRUD/basic/01-load-initial/create-with-tags-cluster.json | Updates initial untyped CRUD fixture with inlined fields/cosmos metadata. |
| test-integration/backend/launch/metrics_test.go | Populates CosmosMetadata.ResourceID in metrics integration test cluster. |
| test-integration/backend/controllers/mismatches/artifacts/nodepool/remove_orphaned_nodepool_descendents/99-cosmosCompare-end-state/cluster.json | Updates mismatch-controller expected end-state cluster doc. |
| test-integration/backend/controllers/mismatches/artifacts/nodepool/remove_orphaned_nodepool_descendents/00-load-initial-state/cluster.json | Updates mismatch-controller initial-state cluster doc. |
| test-integration/backend/controllers/mismatches/artifacts/nodepool/present_nodepool/99-cosmosCompare-end-state/cluster.json | Updates present-nodepool expected end-state cluster doc. |
| test-integration/backend/controllers/mismatches/artifacts/nodepool/present_nodepool/00-load-initial-state/cluster.json | Updates present-nodepool initial-state cluster doc. |
| test-integration/backend/controllers/mismatches/artifacts/externalauth/remove_orphaned_externalauth_descendents/99-cosmosCompare-end-state/cluster.json | Updates external auth mismatch expected end-state cluster doc. |
| test-integration/backend/controllers/mismatches/artifacts/externalauth/remove_orphaned_externalauth_descendents/00-load-initial-state/cluster.json | Updates external auth mismatch initial-state cluster doc. |
| test-integration/backend/controllers/mismatches/artifacts/externalauth/present_externalauth/99-cosmosCompare-end-state/cluster.json | Updates present-external-auth expected end-state cluster doc. |
| test-integration/backend/controllers/mismatches/artifacts/externalauth/present_externalauth/00-load-initial-state/cluster.json | Updates present-external-auth initial-state cluster doc. |
| test-integration/backend/controllers/mismatches/artifacts/delete_orphaned_cosmos/controller_under_missing_nodepool_deleted/99-cosmosCompare-end-state/cluster.json | Updates delete-orphaned-cosmos expected end-state cluster doc. |
| test-integration/backend/controllers/mismatches/artifacts/delete_orphaned_cosmos/controller_under_missing_nodepool_deleted/00-load-initial-state/cluster.json | Updates delete-orphaned-cosmos initial-state cluster doc. |
| test-integration/backend/controllers/mismatches/artifacts/delete_orphaned_cosmos/all_parents_exist/99-cosmosCompare-end-state/cluster.json | Updates all-parents-exist expected end-state cluster doc. |
| test-integration/backend/controllers/mismatches/artifacts/delete_orphaned_cosmos/all_parents_exist/00-load-initial-state/cluster.json | Updates all-parents-exist initial-state cluster doc. |
| test-integration/backend/controllers/mismatches/artifacts/cluster/remove_orphaned_cluster_descendents/99-cosmosCompare-end-state/cluster-safe.json | Updates cluster-descendents expected safe cluster doc. |
| test-integration/backend/controllers/mismatches/artifacts/cluster/remove_orphaned_cluster_descendents/00-load-initial-state/cluster.json | Updates cluster-descendents initial cluster doc. |
| test-integration/backend/controllers/mismatches/artifacts/cluster/remove_orphaned_cluster_descendents/00-load-initial-state/cluster-safe.json | Updates cluster-descendents initial safe cluster doc. |
| test-integration/backend/controllers/mismatches/artifacts/cluster/present_cluster/99-cosmosCompare-end-state/cluster.json | Updates present-cluster expected end-state cluster doc. |
| test-integration/backend/controllers/mismatches/artifacts/cluster/present_cluster/00-load-initial-state/cluster.json | Updates present-cluster initial-state cluster doc. |
| test-integration/backend/controllers/do_nothing/artifacts/sync_cluster/00-load-initial-state/cluster.json | Updates do-nothing controller fixture cluster doc. |
| test-integration/admin/artifacts/AdminCRUD/HCP/lowercase-middleware/00-load-initial-cosmos-state/01-cluster.json | Updates admin CRUD fixture with inlined cluster fields/cosmos metadata. |
| test-integration/admin/artifacts/AdminCRUD/HCP/hello-world/00-load-initial-cosmos-state/01-cluster.json | Updates admin CRUD fixture with inlined cluster fields/cosmos metadata. |
| test-integration/admin/artifacts/AdminCRUD/HCP/cosmosdump/00-load-initial-cosmos-state/01-cluster.json | Updates admin cosmos dump fixture with inlined cluster fields/cosmos metadata. |
| test-integration/admin/artifacts/AdminCRUD/HCP/breakglass/00-load-initial-cosmos-state/01-cluster.json | Updates admin breakglass fixture with inlined cluster fields/cosmos metadata. |
| internal/serverutils/dump_billing_test.go | Populates CosmosMetadata.ResourceID on test clusters used for billing dump. |
| internal/databasetesting/mock_dbclient_test.go | Ensures mock DB tests populate CosmosMetadata.ResourceID on clusters. |
| internal/database/types_hcpcluster.go | Inlines api.HCPOpenShiftCluster in HCPClusterProperties to align with generic storage. |
| internal/database/convert_defaults_consistency_test.go | Updates defaults tests to build clusters with embedded CosmosMetadata. |
| internal/database/convert_cluster.go | Switches Cosmos conversions to use CosmosMetadata.ResourceID as canonical; adjusts ETag/UID handling. |
| internal/database/convert_cluster_test.go | Updates conversion fuzz/roundtrip tests for new CosmosMetadata handling and ExistingCosmosUID placement. |
| internal/conversion/readonly_cluster.go | Copies CosmosMetadata as part of read-only value preservation on replace. |
| internal/api/zz_generated.deepcopy.go | Ensures HCPOpenShiftCluster.DeepCopyInto deep-copies CosmosMetadata. |
| internal/api/v20251223preview/zero_value_roundtrip_test.go | Baseline internal cluster now includes CosmosMetadata.ResourceID. |
| internal/api/v20251223preview/hcpopenshiftclusters_methods.go | Uses ResourceID when producing external ID; sets internal ResourceID on convert-to-internal. |
| internal/api/v20251223preview/conversion_fuzz_test.go | Adds fuzzer customization for CosmosMetadata fields. |
| internal/api/v20240610preview/hcpopenshiftclusters_methods.go | Uses ResourceID when producing external ID; sets internal ResourceID on convert-to-internal. |
| internal/api/v20240610preview/conversion_fuzz_test.go | Adds fuzzer customization for CosmosMetadata fields. |
| internal/api/types_cluster.go | Embeds CosmosMetadata into HCPOpenShiftCluster and removes prior CosmosETag/ExistingCosmosUID locations. |
| frontend/pkg/frontend/helpers_test.go | Populates cluster CosmosMetadata.ResourceID in provisioning conflict tests. |
| frontend/pkg/frontend/frontend_test.go | Populates cluster CosmosMetadata.ResourceID in admin credential tests. |
| frontend/pkg/frontend/cluster.go | Sets ResourceID during create decode; validates requested resourceID against stored CosmosMetadata.ResourceID. |
| backend/pkg/listertesting/slice_listers_test.go | Populates cluster CosmosMetadata.ResourceID in lister tests. |
| backend/pkg/informers/informers_test.go | Populates cluster CosmosMetadata.ResourceID in informer tests. |
| backend/pkg/controllers/upgradecontrollers/nodepool_version_controller_test.go | Populates cluster CosmosMetadata.ResourceID in upgrade controller tests. |
| backend/pkg/controllers/upgradecontrollers/control_plane_desired_version_controller_test.go | Populates cluster CosmosMetadata.ResourceID in desired version controller tests. |
| backend/pkg/controllers/upgradecontrollers/control_plane_active_version_controller_test.go | Populates cluster CosmosMetadata.ResourceID in active version controller tests. |
| backend/pkg/controllers/read_and_persist_cluster_scoped_maestro_readonly_bundles_content_controller_test.go | Populates cluster CosmosMetadata.ResourceID in bundle-content tests. |
| backend/pkg/controllers/operationcontrollers/test_helpers_test.go | Populates cluster CosmosMetadata.ResourceID in operation controller fixtures. |
| backend/pkg/controllers/nodepoolpropertiescontroller/node_pool_properties_sync_test.go | Populates cluster CosmosMetadata.ResourceID in nodepool properties sync tests. |
| backend/pkg/controllers/metricscontrollers/resource_metrics_controller_test.go | Populates cluster CosmosMetadata.ResourceID in metrics controller tests. |
| backend/pkg/controllers/maestro_readonly_bundle_helpers_test.go | Adjusts maestro bundle helper tests for new cluster shape/metadata. |
| backend/pkg/controllers/delete_orphaned_maestro_readonly_bundles_controller_test.go | Populates cluster CosmosMetadata.ResourceID across many orphaned bundle tests. |
| backend/pkg/controllers/datadumpcontrollers/cs_state_dump_test.go | Populates cluster CosmosMetadata.ResourceID in CS state dump tests. |
| backend/pkg/controllers/create_cluster_scoped_maestro_readonly_bundles_controller_test.go | Populates cluster CosmosMetadata.ResourceID in create-bundle controller tests. |
| backend/pkg/controllers/clusterpropertiescontroller/cluster_properties_sync_test.go | Populates cluster CosmosMetadata.ResourceID in cluster properties sync tests. |
| backend/pkg/controllers/billingcontrollers/orphaned_billing_cleanup_test.go | Populates cluster CosmosMetadata.ResourceID in billing cleanup tests. |
| backend/pkg/controllers/billingcontrollers/create_billing_doc_test.go | Populates cluster CosmosMetadata.ResourceID in billing doc creation tests. |
| admin/server/handlers/hcp/serialconsole_test.go | Populates cluster CosmosMetadata.ResourceID in serial console handler tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
f31e689 to
6114a62
Compare
6114a62 to
e4a3d2e
Compare
|
This probably won't survive til it's ready to merge, but for the record... /lgtm |
|
/hold until #5095 makes it to prod |
e4a3d2e to
cdc66cc
Compare
|
/hold cancel it made it to prod |
|
/lgtm |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 35 out of 36 changed files in this pull request and generated 1 comment.
Files not reviewed (1)
- internal/api/zz_generated.deepcopy.go: Language not supported
Comments suppressed due to low confidence (1)
internal/database/convert_cluster.go:99
CosmosToInternalClusterno longer restoresTrackedResource(ID/Name/Type/SystemData/Location/Tags),Identity, orServiceProviderProperties.{ProvisioningState,ClusterServiceID,ActiveOperationID}fromIntermediateResourceDoc— these now come solely from the inlinedHCPOpenShiftCluster. Cosmos documents that were written before the inline serialization began (i.e., before PR #5095) and have not been rewritten yet will not have these inline fields populated, so reads of such legacy documents will silently return a cluster with zeroed TrackedResource/Identity/ProvisioningState/etc. Please confirm that all existing documents have been re-written since #5095 deployed, or retain a fallback toIntermediateResourceDocuntil step 3/4 of the migration plan.
internalObj := cosmosObj.DeepCopy()
internalObj.ExistingCosmosUID = cosmosObj.ID
internalObj.SetEtag(cosmosObj.CosmosETag)
if internalObj.GetResourceID() == nil {
if cosmosObj.ResourceID != nil {
internalObj.SetResourceID(cosmosObj.ResourceID)
} else {
return nil, fmt.Errorf("internalObj is missing a resourceID: %T: %q", cosmosObj, cosmosObj.ID)
}
}
internalObj.EnsureDefaults()
return internalObj, nil
| if cosmosObj.ResourceID != nil { | ||
| internalObj.SetResourceID(cosmosObj.ResourceID) | ||
| } else { | ||
| return nil, fmt.Errorf("internalObj is missing a resourceID: %T: %q", cosmosObj, cosmosObj.ID) |
|
query confirms all data is ready for this |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: deads2k, mbarnes, stevekuznetsov 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 |
|
/hold Revision 00e6304 was retested 3 times: holding |
00e6304 to
134aaba
Compare
|
New changes are detected. LGTM label has been removed. |
|
simple rebase |
|
/hold cancel |
|
/hold non-conflicting conflicts |
|
/hold we believe this has non-merge-conflict issues with a recently-merged PR, need to remove from batch |
|
merged in another PR. |
builds on #5095 to move
api.ComsosMetadatainto the internal representation ofHCPOpenshiftCluster