stop serializing old nodepool fields#5208
Conversation
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
/hold for #5207 to get prod |
There was a problem hiding this comment.
Pull request overview
This PR is a follow-up step in the nodepool Cosmos storage migration plan (after the switch to generic storage), updating nodepool persistence and test fixtures so the on-disk nodepool document shape no longer includes the legacy intermediateResourceDoc / internalState.internalAPI fields and instead matches GenericDocument[api.HCPOpenShiftClusterNodePool].
Changes:
- Inline
api.HCPOpenShiftClusterNodePooldirectly into the nodepool Cosmos documentpropertiespayload and stop serializing legacy nodepool fields. - Move Cosmos persistence metadata (
resourceID,_etag, existing UID) ontocosmosMetadatawithin the internal nodepool type, and update conversions accordingly. - Update integration-test artifacts and unit tests to reflect the new stored JSON shape and required
cosmosMetadata.resourceIDpopulation.
Reviewed changes
Copilot reviewed 61 out of 61 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test-integration/utils/databasemutationhelpers/per_resource_comparer.go | Ignore redundant nested internalState.internalAPI.cosmosMetadata during comparisons. |
| test-integration/frontend/informer_test.go | Populate CosmosMetadata.ResourceID in nodepool test objects. |
| test-integration/frontend/artifacts/FrontendCRUD/NodePool/read-old-data/09-cosmosCompare-confirm-update/nodepool-basic-node-pool.json | Update expected Cosmos JSON shape (remove legacy fields, inline resource). |
| test-integration/frontend/artifacts/FrontendCRUD/NodePool/read-old-data/09-cosmosCompare-confirm-update/nodepool-02.json | Update expected Cosmos JSON shape (remove legacy fields, inline resource). |
| test-integration/frontend/artifacts/FrontendCRUD/NodePool/read-old-data/01-load-old-data/nodepool-node-pool-02.json | Update “old data” fixture to new inline nodepool shape. |
| test-integration/frontend/artifacts/FrontendCRUD/NodePool/read-old-data/01-load-old-data/nodepool-basic-node-pool.json | Update “old data” fixture to new inline nodepool shape. |
| test-integration/frontend/artifacts/FrontendCRUD/NodePool/create-current/06-cosmosCompare-confirm-content/nodepool-node-pool-02.json | Update expected Cosmos JSON shape for current create flow. |
| test-integration/frontend/artifacts/FrontendCRUD/NodePool/create-current/06-cosmosCompare-confirm-content/nodepool-basic-node-pool.json | Update expected Cosmos JSON shape for current create flow. |
| test-integration/frontend/artifacts/FrontendCRUD/Migration/read-new-data/18-cosmosCompare-confirm/nodepool-node-pool-02.json | Update migration test artifacts to new inline nodepool shape. |
| test-integration/frontend/artifacts/FrontendCRUD/Migration/read-new-data/18-cosmosCompare-confirm/nodepool-basic-node-pool.json | Update migration test artifacts to new inline nodepool shape. |
| test-integration/frontend/artifacts/FrontendCRUD/Migration/read-new-data/01-load-old-data/nodepool-node-pool-02.json | Update migration “load old data” artifact to new shape. |
| test-integration/frontend/artifacts/FrontendCRUD/Migration/read-new-data/01-load-old-data/nodepool-basic-node-pool.json | Update migration “load old data” artifact to new shape. |
| test-integration/frontend/artifacts/FrontendCRUD/Migration/migrate-old-data/99-cosmosCompare-confirm-migration/nodepool-node-pool-02.json | Update migration confirm artifacts to new shape. |
| test-integration/frontend/artifacts/FrontendCRUD/Migration/migrate-old-data/99-cosmosCompare-confirm-migration/nodepool-basic-node-pool.json | Update migration confirm artifacts to new shape. |
| test-integration/frontend/artifacts/FrontendCRUD/Migration/migrate-old-data/01-load-old-data/nodepool-node-pool-02.json | Update migration initial-load artifacts to new shape. |
| test-integration/frontend/artifacts/FrontendCRUD/Migration/migrate-old-data/01-load-old-data/nodepool-basic-node-pool.json | Update migration initial-load artifacts to new shape. |
| test-integration/frontend/artifacts/FrontendCRUD/Cluster/delete-with-pending-nodepool-operation/05-cosmosCompare-final-state/nodepool-test-nodepool.json | Update expected nodepool final-state Cosmos JSON shape. |
| test-integration/frontend/artifacts/DatabaseCRUD/UntypedCRUD/basic/09-untypedList-immutability-cluster/immutability-np.json | Remove legacy internalState from untyped list artifacts. |
| test-integration/frontend/artifacts/DatabaseCRUD/UntypedCRUD/basic/08-untypedListRecursive-immutability-cluster/immutability-np.json | Remove legacy internalState from untyped list artifacts. |
| test-integration/frontend/artifacts/DatabaseCRUD/UntypedCRUD/basic/07-untypedListRecursive-resourcegroup-via-child/immutability-np.json | Remove legacy internalState from untyped list artifacts. |
| test-integration/frontend/artifacts/DatabaseCRUD/UntypedCRUD/basic/05-untypedListRecursive-subscription/immutability-np.json | Remove legacy internalState from untyped list artifacts. |
| test-integration/frontend/artifacts/DatabaseCRUD/UntypedCRUD/basic/04-untypedListRecursive-resourcegroup/immutability-np.json | Remove legacy internalState from untyped list artifacts. |
| test-integration/frontend/artifacts/DatabaseCRUD/UntypedCRUD/basic/02-untypedListRecursive-immutability/immutability-np.json | Remove legacy internalState from untyped list artifacts. |
| test-integration/frontend/artifacts/DatabaseCRUD/UntypedCRUD/basic/01-load-initial/immutability-np.json | Remove legacy internalState from initial load artifact. |
| test-integration/backend/controllers/mismatches/artifacts/nodepool/remove_orphaned_nodepool_descendents/00-load-initial-state/nodepool-basic.json | Update backend mismatch artifacts to new inline nodepool shape. |
| test-integration/backend/controllers/mismatches/artifacts/nodepool/present_nodepool/99-cosmosCompare-end-state/nodepool-basic.json | Update backend mismatch artifacts to new inline nodepool shape. |
| test-integration/backend/controllers/mismatches/artifacts/nodepool/present_nodepool/00-load-initial-state/nodepool-basic.json | Update backend mismatch artifacts to new inline nodepool shape. |
| test-integration/backend/controllers/mismatches/artifacts/externalauth/remove_orphaned_externalauth_descendents/99-cosmosCompare-end-state/nodepool-basic.json | Update backend mismatch artifacts to new inline nodepool shape. |
| test-integration/backend/controllers/mismatches/artifacts/externalauth/remove_orphaned_externalauth_descendents/00-load-initial-state/nodepool-basic.json | Update backend mismatch artifacts to new inline nodepool shape. |
| test-integration/backend/controllers/mismatches/artifacts/delete_orphaned_cosmos/all_parents_exist/99-cosmosCompare-end-state/nodepool-basic.json | Update backend mismatch artifacts to new inline nodepool shape. |
| test-integration/backend/controllers/mismatches/artifacts/delete_orphaned_cosmos/all_parents_exist/00-load-initial-state/nodepool-basic.json | Update backend mismatch artifacts to new inline nodepool shape. |
| test-integration/backend/controllers/mismatches/artifacts/cluster/remove_orphaned_cluster_descendents/00-load-initial-state/nodepool-basic.json | Update backend mismatch artifacts to new inline nodepool shape. |
| internal/ocm/convert.go | Ensure converted nodepools include CosmosMetadata.ResourceID. |
| internal/databasetesting/mock_dbclient_test.go | Update mock DB client test fixtures to include CosmosMetadata.ResourceID. |
| internal/database/types_nodepool.go | Inline nodepool internal type directly into stored properties. |
| internal/database/convert_nodepool.go | Update internal↔cosmos conversions to use inlined nodepool + cosmosMetadata. |
| internal/database/convert_nodepool_test.go | Adjust nodepool round-trip tests for new metadata location and shape. |
| internal/database/convert_defaults_consistency_test.go | Update defaults/compat tests for nodepool provisioning state + metadata. |
| internal/database/convert_cluster_test.go | Update shared round-trip helper for nodepool ExistingCosmosUID location. |
| internal/conversion/readonly_nodepool.go | Preserve CosmosMetadata across replace flows; drop legacy CosmosETag field copy. |
| internal/api/zz_generated.deepcopy.go | Deep-copy CosmosMetadata on nodepool deepcopy. |
| internal/api/v20251223preview/zero_value_roundtrip_test.go | Seed baseline nodepool with CosmosMetadata.ResourceID for JSON round-trip. |
| internal/api/v20251223preview/nodepools_methods.go | Set internal ResourceID from external ID; emit external id from internal ResourceID. |
| internal/api/v20251223preview/nodepools_methods_test.go | Update nodepool tests to include CosmosMetadata.ResourceID. |
| internal/api/v20251223preview/conversion_fuzz_test.go | Update fuzz hooks for CosmosMetadata and remove nodepool SPP UID assumptions. |
| internal/api/v20240610preview/nodepools_methods.go | Same as v20251223preview: set internal ResourceID, emit external id from it. |
| internal/api/v20240610preview/nodepools_methods_test.go | Update nodepool tests to include CosmosMetadata.ResourceID. |
| internal/api/v20240610preview/conversion_fuzz_test.go | Update fuzz hooks for CosmosMetadata and remove nodepool SPP UID assumptions. |
| internal/api/types_nodepool.go | Add serialized cosmosMetadata; remove legacy CosmosETag field and SPP ExistingCosmosUID. |
| internal/admission/admit_cluster_test.go | Populate CosmosMetadata.ResourceID in admission test fixtures. |
| frontend/pkg/frontend/node_pool.go | Ensure created nodepools have cosmosMetadata.resourceID; validate stored docs use it. |
| backend/pkg/listertesting/slice_listers_test.go | Populate CosmosMetadata.ResourceID in lister test fixtures. |
| backend/pkg/informers/informers_test.go | Populate CosmosMetadata.ResourceID in informer test fixtures. |
| 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 test nodepool objects. |
| backend/pkg/controllers/read_and_persist_nodepool_scoped_maestro_readonly_bundles_content_controller_test.go | Populate CosmosMetadata.ResourceID in controller test fixtures. |
| 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 sync tests. |
| backend/pkg/controllers/metricscontrollers/resource_metrics_controller_test.go | Populate CosmosMetadata.ResourceID in metrics tests. |
| backend/pkg/controllers/datadumpcontrollers/cs_state_dump_test.go | Populate CosmosMetadata.ResourceID in dump tests. |
| backend/pkg/controllers/create_nodepool_scoped_maestro_readonly_bundles_controller_test.go | Populate CosmosMetadata.ResourceID in bundle controller tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| }, | ||
| Location: cosmosObj.InternalState.InternalAPI.Location, | ||
| Tags: resourceDoc.Tags, | ||
| internalObj := cosmosObj.DeepCopy() |
|
@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. |
|
/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 |
|
replaced by #5395 |
part of #5206, must merge after #5207 makes it to prod