diff --git a/backend/pkg/controllers/create_nodepool_scoped_maestro_readonly_bundles_controller_test.go b/backend/pkg/controllers/create_nodepool_scoped_maestro_readonly_bundles_controller_test.go index 3abda265771..c50f3db4e5d 100644 --- a/backend/pkg/controllers/create_nodepool_scoped_maestro_readonly_bundles_controller_test.go +++ b/backend/pkg/controllers/create_nodepool_scoped_maestro_readonly_bundles_controller_test.go @@ -146,6 +146,7 @@ func TestBuildInitialReadonlyMaestroBundleForNodePool(t *testing.T) { nodepoolResourceID := api.Must(azcorearm.ParseResourceID("/subscriptions/test-sub/resourceGroups/test-rg/providers/Microsoft.RedHatOpenShift/hcpOpenShiftClusters/test-cluster/nodePools/test-nodepool")) nodepool := &api.HCPOpenShiftClusterNodePool{ + CosmosMetadata: arm.CosmosMetadata{ResourceID: nodepoolResourceID}, TrackedResource: arm.TrackedResource{ Resource: arm.Resource{ ID: nodepoolResourceID, @@ -465,6 +466,7 @@ func TestCreateNodePoolScopedMaestroReadonlyBundlesSyncer_syncMaestroBundle(t *t } ctx := context.Background() nodepool := &api.HCPOpenShiftClusterNodePool{ + CosmosMetadata: arm.CosmosMetadata{ResourceID: nodepoolResourceID}, TrackedResource: arm.TrackedResource{ Resource: arm.Resource{ ID: nodepoolResourceID, @@ -560,6 +562,7 @@ func TestCreateNodePoolScopedMaestroReadonlyBundlesSyncer_SyncOnce_EmptyClusterS nodepoolResourceID := api.Must(azcorearm.ParseResourceID("/subscriptions/test-sub/resourceGroups/test-rg/providers/Microsoft.RedHatOpenShift/hcpOpenShiftClusters/test-cluster/nodePools/test-nodepool")) nodepool := &api.HCPOpenShiftClusterNodePool{ + CosmosMetadata: arm.CosmosMetadata{ResourceID: nodepoolResourceID}, TrackedResource: arm.TrackedResource{ Resource: arm.Resource{ ID: nodepoolResourceID, @@ -612,6 +615,7 @@ func TestCreateNodePoolScopedMaestroReadonlyBundlesSyncer_SyncOnce_GetServicePro nodepoolResourceID := api.Must(azcorearm.ParseResourceID("/subscriptions/test-sub/resourceGroups/test-rg/providers/Microsoft.RedHatOpenShift/hcpOpenShiftClusters/test-cluster/nodePools/test-nodepool")) nodepool := &api.HCPOpenShiftClusterNodePool{ + CosmosMetadata: arm.CosmosMetadata{ResourceID: nodepoolResourceID}, TrackedResource: arm.TrackedResource{ Resource: arm.Resource{ ID: nodepoolResourceID, @@ -666,6 +670,7 @@ func TestCreateNodePoolScopedMaestroReadonlyBundlesSyncer_SyncOnce_AllBundlesAlr nodepoolResourceID := api.Must(azcorearm.ParseResourceID("/subscriptions/test-sub/resourceGroups/test-rg/providers/Microsoft.RedHatOpenShift/hcpOpenShiftClusters/test-cluster/nodePools/test-nodepool")) nodepool := &api.HCPOpenShiftClusterNodePool{ + CosmosMetadata: arm.CosmosMetadata{ResourceID: nodepoolResourceID}, TrackedResource: arm.TrackedResource{ Resource: arm.Resource{ ID: nodepoolResourceID, @@ -730,6 +735,7 @@ func TestCreateNodePoolScopedMaestroReadonlyBundlesSyncer_SyncOnce_SyncLoopExecu nodepoolResourceID := api.Must(azcorearm.ParseResourceID("/subscriptions/test-sub/resourceGroups/test-rg/providers/Microsoft.RedHatOpenShift/hcpOpenShiftClusters/test-cluster/nodePools/test-nodepool")) nodepool := &api.HCPOpenShiftClusterNodePool{ + CosmosMetadata: arm.CosmosMetadata{ResourceID: nodepoolResourceID}, TrackedResource: arm.TrackedResource{ Resource: arm.Resource{ ID: nodepoolResourceID, @@ -830,6 +836,7 @@ func TestCreateNodePoolScopedMaestroReadonlyBundlesSyncer_SyncOnce_ProcessesPart nodepoolResourceID := api.Must(azcorearm.ParseResourceID("/subscriptions/test-sub/resourceGroups/test-rg/providers/Microsoft.RedHatOpenShift/hcpOpenShiftClusters/test-cluster/nodePools/test-nodepool")) nodepool := &api.HCPOpenShiftClusterNodePool{ + CosmosMetadata: arm.CosmosMetadata{ResourceID: nodepoolResourceID}, TrackedResource: arm.TrackedResource{ Resource: arm.Resource{ ID: nodepoolResourceID, diff --git a/backend/pkg/controllers/datadumpcontrollers/cs_state_dump_test.go b/backend/pkg/controllers/datadumpcontrollers/cs_state_dump_test.go index d93368c1599..8cf271831b1 100644 --- a/backend/pkg/controllers/datadumpcontrollers/cs_state_dump_test.go +++ b/backend/pkg/controllers/datadumpcontrollers/cs_state_dump_test.go @@ -211,6 +211,7 @@ func newTestNodePool(name, clusterServiceIDStr string) *api.HCPOpenShiftClusterN nodePoolResourceID := api.Must(azcorearm.ParseResourceID( "/subscriptions/test-sub/resourceGroups/test-rg/providers/Microsoft.RedHatOpenShift/hcpOpenShiftClusters/test-cluster/nodePools/" + name)) np := &api.HCPOpenShiftClusterNodePool{ + CosmosMetadata: arm.CosmosMetadata{ResourceID: nodePoolResourceID}, TrackedResource: arm.TrackedResource{ Resource: arm.Resource{ ID: nodePoolResourceID, diff --git a/backend/pkg/controllers/metricscontrollers/resource_metrics_controller_test.go b/backend/pkg/controllers/metricscontrollers/resource_metrics_controller_test.go index 73c44690aff..349af2f0bc6 100644 --- a/backend/pkg/controllers/metricscontrollers/resource_metrics_controller_test.go +++ b/backend/pkg/controllers/metricscontrollers/resource_metrics_controller_test.go @@ -133,6 +133,7 @@ func TestNodePoolMetricsHandler_SetsMetrics(t *testing.T) { handler := NewNodePoolMetricsHandler(reg) nodePool := &api.HCPOpenShiftClusterNodePool{ + CosmosMetadata: arm.CosmosMetadata{ResourceID: api.Must(azcorearm.ParseResourceID("/subscriptions/sub-1/resourceGroups/rg/providers/Microsoft.RedHatOpenShift/hcpOpenShiftClusters/cluster-1/nodePools/np-1"))}, TrackedResource: arm.TrackedResource{ Resource: arm.Resource{ ID: api.Must(azcorearm.ParseResourceID("/subscriptions/sub-1/resourceGroups/rg/providers/Microsoft.RedHatOpenShift/hcpOpenShiftClusters/cluster-1/nodePools/np-1")), diff --git a/backend/pkg/controllers/operationcontrollers/test_helpers_test.go b/backend/pkg/controllers/operationcontrollers/test_helpers_test.go index f6c43ee56a6..f0e14923d03 100644 --- a/backend/pkg/controllers/operationcontrollers/test_helpers_test.go +++ b/backend/pkg/controllers/operationcontrollers/test_helpers_test.go @@ -177,6 +177,7 @@ func (f *nodePoolTestFixture) newCluster() *api.HCPOpenShiftCluster { func (f *nodePoolTestFixture) newNodePool() *api.HCPOpenShiftClusterNodePool { return &api.HCPOpenShiftClusterNodePool{ + CosmosMetadata: arm.CosmosMetadata{ResourceID: f.nodePoolResourceID}, TrackedResource: arm.TrackedResource{ Resource: arm.Resource{ ID: f.nodePoolResourceID, diff --git a/backend/pkg/controllers/read_and_persist_nodepool_scoped_maestro_readonly_bundles_content_controller_test.go b/backend/pkg/controllers/read_and_persist_nodepool_scoped_maestro_readonly_bundles_content_controller_test.go index f5ac3564fae..4940ab4ee9d 100644 --- a/backend/pkg/controllers/read_and_persist_nodepool_scoped_maestro_readonly_bundles_content_controller_test.go +++ b/backend/pkg/controllers/read_and_persist_nodepool_scoped_maestro_readonly_bundles_content_controller_test.go @@ -106,6 +106,7 @@ func TestReadAndPersistNodePoolScopedMaestroReadonlyBundlesContentSyncer_SyncOnc nodepoolResourceID := api.Must(azcorearm.ParseResourceID("/subscriptions/test-sub/resourceGroups/test-rg/providers/Microsoft.RedHatOpenShift/hcpOpenShiftClusters/test-cluster/nodePools/test-nodepool")) nodepool := &api.HCPOpenShiftClusterNodePool{ + CosmosMetadata: arm.CosmosMetadata{ResourceID: nodepoolResourceID}, TrackedResource: arm.TrackedResource{ Resource: arm.Resource{ ID: nodepoolResourceID, @@ -153,6 +154,7 @@ func TestReadAndPersistNodePoolScopedMaestroReadonlyBundlesContentSyncer_SyncOnc nodepoolResourceID := api.Must(azcorearm.ParseResourceID("/subscriptions/test-sub/resourceGroups/test-rg/providers/Microsoft.RedHatOpenShift/hcpOpenShiftClusters/test-cluster/nodePools/test-nodepool")) nodepool := &api.HCPOpenShiftClusterNodePool{ + CosmosMetadata: arm.CosmosMetadata{ResourceID: nodepoolResourceID}, TrackedResource: arm.TrackedResource{ Resource: arm.Resource{ ID: nodepoolResourceID, @@ -202,6 +204,7 @@ func TestReadAndPersistNodePoolScopedMaestroReadonlyBundlesContentSyncer_SyncOnc nodepoolResourceID := api.Must(azcorearm.ParseResourceID("/subscriptions/test-sub/resourceGroups/test-rg/providers/Microsoft.RedHatOpenShift/hcpOpenShiftClusters/test-cluster/nodePools/test-nodepool")) nodepool := &api.HCPOpenShiftClusterNodePool{ + CosmosMetadata: arm.CosmosMetadata{ResourceID: nodepoolResourceID}, TrackedResource: arm.TrackedResource{ Resource: arm.Resource{ ID: nodepoolResourceID, @@ -251,6 +254,7 @@ func TestReadAndPersistNodePoolScopedMaestroReadonlyBundlesContentSyncer_SyncOnc nodepoolResourceID := api.Must(azcorearm.ParseResourceID("/subscriptions/test-sub/resourceGroups/test-rg/providers/Microsoft.RedHatOpenShift/hcpOpenShiftClusters/test-cluster/nodePools/test-nodepool")) nodepool := &api.HCPOpenShiftClusterNodePool{ + CosmosMetadata: arm.CosmosMetadata{ResourceID: nodepoolResourceID}, TrackedResource: arm.TrackedResource{ Resource: arm.Resource{ ID: nodepoolResourceID, @@ -314,6 +318,7 @@ func TestReadAndPersistNodePoolScopedMaestroReadonlyBundlesContentSyncer_SyncOnc nodepoolResourceID := api.Must(azcorearm.ParseResourceID("/subscriptions/test-sub/resourceGroups/test-rg/providers/Microsoft.RedHatOpenShift/hcpOpenShiftClusters/test-cluster/nodePools/test-nodepool")) nodepool := &api.HCPOpenShiftClusterNodePool{ + CosmosMetadata: arm.CosmosMetadata{ResourceID: nodepoolResourceID}, TrackedResource: arm.TrackedResource{ Resource: arm.Resource{ ID: nodepoolResourceID, diff --git a/backend/pkg/controllers/upgradecontrollers/control_plane_desired_version_controller_test.go b/backend/pkg/controllers/upgradecontrollers/control_plane_desired_version_controller_test.go index 6134e0cd62a..ca092f4d15c 100644 --- a/backend/pkg/controllers/upgradecontrollers/control_plane_desired_version_controller_test.go +++ b/backend/pkg/controllers/upgradecontrollers/control_plane_desired_version_controller_test.go @@ -593,6 +593,9 @@ func testCosmosClusterWithWorkersNodePoolAtVersion(nodePoolVersionId string) []a return []any{ cluster, &api.HCPOpenShiftClusterNodePool{ + CosmosMetadata: arm.CosmosMetadata{ + ResourceID: nodePoolResourceId, + }, TrackedResource: arm.NewTrackedResource(nodePoolResourceId, "eastus"), Properties: api.HCPOpenShiftClusterNodePoolProperties{ Version: api.NodePoolVersionProfile{ID: nodePoolVersionId}, diff --git a/backend/pkg/controllers/upgradecontrollers/nodepool_version_controller_test.go b/backend/pkg/controllers/upgradecontrollers/nodepool_version_controller_test.go index d614d4ee20d..e123014afb4 100644 --- a/backend/pkg/controllers/upgradecontrollers/nodepool_version_controller_test.go +++ b/backend/pkg/controllers/upgradecontrollers/nodepool_version_controller_test.go @@ -125,6 +125,7 @@ func createTestNodePoolWithVersion(t *testing.T, ctx context.Context, mockResour nodePoolInternalID := api.Ptr(api.Must(api.NewInternalID(testCSNodePoolIDStr))) nodePool := &api.HCPOpenShiftClusterNodePool{ + CosmosMetadata: arm.CosmosMetadata{ResourceID: nodePoolResourceID}, TrackedResource: arm.TrackedResource{ Resource: arm.Resource{ ID: nodePoolResourceID, @@ -263,6 +264,9 @@ func TestNodePoolVersionSyncer_SyncOnce(t *testing.T) { "/providers/Microsoft.RedHatOpenShift/hcpOpenShiftClusters/" + testClusterName + "/nodePools/" + testNodePoolName)) nodePool := &api.HCPOpenShiftClusterNodePool{ + CosmosMetadata: arm.CosmosMetadata{ + ResourceID: nodePoolResourceID, + }, TrackedResource: arm.TrackedResource{ Resource: arm.Resource{ ID: nodePoolResourceID, diff --git a/backend/pkg/informers/informers_test.go b/backend/pkg/informers/informers_test.go index 7d6f706336f..dc3ecbc8aa0 100644 --- a/backend/pkg/informers/informers_test.go +++ b/backend/pkg/informers/informers_test.go @@ -448,6 +448,7 @@ func nodePoolInformerTestCase() informerTestCase { "/nodePools/"+name) internalID := api.Ptr(api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/" + clusterName + "/node_pools/" + name))) return &api.HCPOpenShiftClusterNodePool{ + CosmosMetadata: arm.CosmosMetadata{ResourceID: npResourceID}, TrackedResource: arm.TrackedResource{ Resource: arm.Resource{ ID: npResourceID, @@ -732,6 +733,7 @@ func controllerInformerTestCase() informerTestCase { "/providers/Microsoft.RedHatOpenShift/hcpOpenShiftClusters/"+clusterName+ "/nodePools/"+nodePoolName) np := &api.HCPOpenShiftClusterNodePool{ + CosmosMetadata: arm.CosmosMetadata{ResourceID: npResourceID}, TrackedResource: arm.TrackedResource{ Resource: arm.Resource{ ID: npResourceID, diff --git a/backend/pkg/listertesting/slice_listers_test.go b/backend/pkg/listertesting/slice_listers_test.go index eb4dd051bac..c8528c540e9 100644 --- a/backend/pkg/listertesting/slice_listers_test.go +++ b/backend/pkg/listertesting/slice_listers_test.go @@ -344,6 +344,7 @@ func newTestNodePool(subscriptionID, resourceGroupName, clusterName, nodePoolNam "/nodePools/" + nodePoolName, )) return &api.HCPOpenShiftClusterNodePool{ + CosmosMetadata: arm.CosmosMetadata{ResourceID: resourceID}, TrackedResource: arm.TrackedResource{ Resource: arm.Resource{ ID: resourceID, diff --git a/frontend/pkg/frontend/node_pool.go b/frontend/pkg/frontend/node_pool.go index b50d9792ced..f27fa066d9c 100644 --- a/frontend/pkg/frontend/node_pool.go +++ b/frontend/pkg/frontend/node_pool.go @@ -212,6 +212,7 @@ func decodeDesiredNodePoolCreate(ctx context.Context, azureLocation string) (*ap return nil, nameResourceIDMismatch(resourceID, newInternalNodePool.Name) } conversion.CopyReadOnlyTrackedResourceValues(&newInternalNodePool.TrackedResource, ptr.To(arm.NewTrackedResource(resourceID, azureLocation))) + newInternalNodePool.SetResourceID(resourceID) // set fields that were not included during the conversion, because the user does not provide them or because the // data is determined live on read. @@ -799,8 +800,11 @@ func (f *Frontend) getInternalNodePoolFromStorage(ctx context.Context, resourceI // normalize or return a toupper or tolower form of the resource // group or resource name. The resource group name and resource // name must come from the URL and not the request body. - if !strings.EqualFold(internalNodePool.ID.String(), resourceID.String()) { - return nil, fmt.Errorf("unexpected resourceID: %s", internalNodePool.ID.String()) + if internalNodePool.ResourceID == nil { + return nil, fmt.Errorf("stored nodepool document is missing cosmosMetadata.resourceID") + } + if !strings.EqualFold(internalNodePool.ResourceID.String(), resourceID.String()) { + return nil, fmt.Errorf("unexpected resourceID: %s", internalNodePool.ResourceID.String()) } internalNodePool.ID = resourceID diff --git a/internal/admission/admit_cluster_test.go b/internal/admission/admit_cluster_test.go index 5fed1675fed..468d02babbd 100644 --- a/internal/admission/admit_cluster_test.go +++ b/internal/admission/admit_cluster_test.go @@ -262,6 +262,9 @@ func TestAdmitClusterOnUpdate(t *testing.T) { nodePoolResourceID := api.Must(azcorearm.ParseResourceID( clusterResourceID.String() + "/nodePools/" + name)) return &api.HCPOpenShiftClusterNodePool{ + CosmosMetadata: arm.CosmosMetadata{ + ResourceID: nodePoolResourceID, + }, TrackedResource: arm.NewTrackedResource(nodePoolResourceID, "eastus"), Properties: api.HCPOpenShiftClusterNodePoolProperties{ Version: api.NodePoolVersionProfile{ID: versionID}, diff --git a/internal/api/types_nodepool.go b/internal/api/types_nodepool.go index 0dea1e56739..91aaad0a03e 100644 --- a/internal/api/types_nodepool.go +++ b/internal/api/types_nodepool.go @@ -20,7 +20,6 @@ import ( "k8s.io/utils/ptr" - "github.com/Azure/azure-sdk-for-go/sdk/azcore" azcorearm "github.com/Azure/azure-sdk-for-go/sdk/azcore/arm" "github.com/Azure/ARO-HCP/internal/api/arm" @@ -30,29 +29,16 @@ import ( // OpenShift clusters. // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object type HCPOpenShiftClusterNodePool struct { + CosmosMetadata `json:"cosmosMetadata"` + arm.TrackedResource Properties HCPOpenShiftClusterNodePoolProperties `json:"properties,omitempty"` ServiceProviderProperties HCPOpenShiftClusterNodePoolServiceProviderProperties `json:"serviceProviderProperties,omitempty"` Identity *arm.ManagedServiceIdentity `json:"identity,omitempty"` - // CosmosETag is an in-memory copy of the _etag field read from the Cosmos DB document (BaseDocument) and - // populated on DB read via the CosmosToInternalNodePool() conversion function. - // We carry it across the API boundary between NodePool (the direct cosmos db type) and HCPOpenShiftClusterNodePool (this) - // so we can populate the CosmosETag in GetCosmosData() so that we can do conditional replaces in cosmos. - // This can be removed once we have inlined and serialized CosmosMetadata in - // HCPOpenShiftClusterNodePool. - CosmosETag azcore.ETag `json:"-"` } var _ arm.CosmosPersistable = &HCPOpenShiftClusterNodePool{} -func (o *HCPOpenShiftClusterNodePool) GetCosmosData() *arm.CosmosMetadata { - return &arm.CosmosMetadata{ - CosmosETag: o.CosmosETag, - ResourceID: o.ID, - ExistingCosmosUID: o.ServiceProviderProperties.ExistingCosmosUID, - } -} - // HCPOpenShiftClusterNodePoolProperties represents the property bag of a // HCPOpenShiftClusterNodePool resource. type HCPOpenShiftClusterNodePoolProperties struct { @@ -68,7 +54,6 @@ type HCPOpenShiftClusterNodePoolProperties struct { } type HCPOpenShiftClusterNodePoolServiceProviderProperties struct { - ExistingCosmosUID string `json:"-"` ClusterServiceID *InternalID `json:"clusterServiceID,omitempty"` ActiveOperationID string `json:"activeOperationId,omitempty"` } diff --git a/internal/api/v20240610preview/conversion_fuzz_test.go b/internal/api/v20240610preview/conversion_fuzz_test.go index 7c17643c5f2..cf2cae217f5 100644 --- a/internal/api/v20240610preview/conversion_fuzz_test.go +++ b/internal/api/v20240610preview/conversion_fuzz_test.go @@ -39,6 +39,14 @@ func TestRoundTripInternalExternalInternal(t *testing.T) { func(j *azcorearm.ResourceID, c randfill.Continue) { *j = *api.Must(azcorearm.ParseResourceID("/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myRg")) }, + func(j *api.CosmosMetadata, c randfill.Continue) { + c.FillNoCustom(j) + // ConvertToInternal lowercases the ID, so use the lowercased canonical form + // here so the round-trip comparison succeeds. + j.ResourceID = api.Must(azcorearm.ParseResourceID("/subscriptions/00000000-0000-0000-0000-000000000000/resourcegroups/myrg")) + j.ExistingCosmosUID = "" + j.CosmosETag = "" + }, func(j *api.HCPOpenShiftClusterCustomerProperties, c randfill.Continue) { c.FillNoCustom(j) // ImageDigestMirrors is a v20251223preview field that does not exist in v20240610preview. @@ -72,7 +80,6 @@ func TestRoundTripInternalExternalInternal(t *testing.T) { j.ActiveOperationID = "" // ClusterServiceID does not roundtrip through the external type because it is purely an internal detail j.ClusterServiceID = nil - j.ExistingCosmosUID = "" }, func(j *api.OSDiskProfile, c randfill.Continue) { c.FillNoCustom(j) diff --git a/internal/api/v20240610preview/nodepools_methods.go b/internal/api/v20240610preview/nodepools_methods.go index b8f7fd4e8c8..5b0c43f1803 100644 --- a/internal/api/v20240610preview/nodepools_methods.go +++ b/internal/api/v20240610preview/nodepools_methods.go @@ -74,6 +74,7 @@ func (h *NodePool) ConvertToInternal(existing *api.HCPOpenShiftClusterNodePool) if h.ID != nil { out.ID = api.Must(azcorearm.ParseResourceID(strings.ToLower(*h.ID))) + out.ResourceID = api.Must(azcorearm.ParseResourceID(strings.ToLower(*h.ID))) } if h.Name != nil { out.Name = *h.Name @@ -309,8 +310,8 @@ func (v version) NewHCPOpenShiftClusterNodePool(from *api.HCPOpenShiftClusterNod } idString := "" - if from.ID != nil { - idString = from.ID.String() + if from.ResourceID != nil { + idString = from.ResourceID.String() } out := &NodePool{ diff --git a/internal/api/v20240610preview/nodepools_methods_test.go b/internal/api/v20240610preview/nodepools_methods_test.go index 5f3d53b06d0..6176528b931 100644 --- a/internal/api/v20240610preview/nodepools_methods_test.go +++ b/internal/api/v20240610preview/nodepools_methods_test.go @@ -40,6 +40,7 @@ func TestSizeGiBRoundTrip(t *testing.T) { { name: "SizeGiB with explicit value should round-trip", original: &api.HCPOpenShiftClusterNodePool{ + CosmosMetadata: arm.CosmosMetadata{ResourceID: api.Must(azcorearm.ParseResourceID(strings.ToLower("/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myRg/providers/Microsoft.RedHatOpenShift/hcpOpenShiftClusters/myCluster/nodePools/myNodePool")))}, TrackedResource: arm.TrackedResource{ Resource: arm.Resource{ ID: api.Must(azcorearm.ParseResourceID(strings.ToLower("/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myRg/providers/Microsoft.RedHatOpenShift/hcpOpenShiftClusters/myCluster/nodePools/myNodePool"))), diff --git a/internal/api/v20251223preview/conversion_fuzz_test.go b/internal/api/v20251223preview/conversion_fuzz_test.go index a144b0f4fd5..fdec57204ff 100644 --- a/internal/api/v20251223preview/conversion_fuzz_test.go +++ b/internal/api/v20251223preview/conversion_fuzz_test.go @@ -39,6 +39,14 @@ func TestRoundTripInternalExternalInternal(t *testing.T) { func(j *azcorearm.ResourceID, c randfill.Continue) { *j = *api.Must(azcorearm.ParseResourceID("/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myRg")) }, + func(j *api.CosmosMetadata, c randfill.Continue) { + c.FillNoCustom(j) + // ConvertToInternal lowercases the ID, so use the lowercased canonical form + // here so the round-trip comparison succeeds. + j.ResourceID = api.Must(azcorearm.ParseResourceID("/subscriptions/00000000-0000-0000-0000-000000000000/resourcegroups/myrg")) + j.ExistingCosmosUID = "" + j.CosmosETag = "" + }, func(j *api.ImageDigestMirror, c randfill.Continue) { c.FillNoCustom(j) // MirrorSourcePolicy does not roundtrip through the external type because it is purely an internal detail @@ -70,7 +78,6 @@ func TestRoundTripInternalExternalInternal(t *testing.T) { j.ActiveOperationID = "" // ClusterServiceID does not roundtrip through the external type because it is purely an internal detail j.ClusterServiceID = nil - j.ExistingCosmosUID = "" }, func(j *api.HCPOpenShiftClusterExternalAuthServiceProviderProperties, c randfill.Continue) { c.FillNoCustom(j) diff --git a/internal/api/v20251223preview/nodepools_methods.go b/internal/api/v20251223preview/nodepools_methods.go index ffeb6536290..0cdfba01116 100644 --- a/internal/api/v20251223preview/nodepools_methods.go +++ b/internal/api/v20251223preview/nodepools_methods.go @@ -87,6 +87,7 @@ func (h *NodePool) ConvertToInternal(existing *api.HCPOpenShiftClusterNodePool) if h.ID != nil { out.ID = api.Must(azcorearm.ParseResourceID(strings.ToLower(*h.ID))) + out.ResourceID = api.Must(azcorearm.ParseResourceID(strings.ToLower(*h.ID))) } if h.Name != nil { out.Name = *h.Name @@ -301,8 +302,8 @@ func (v version) NewHCPOpenShiftClusterNodePool(from *api.HCPOpenShiftClusterNod } idString := "" - if from.ID != nil { - idString = from.ID.String() + if from.ResourceID != nil { + idString = from.ResourceID.String() } out := &NodePool{ diff --git a/internal/api/v20251223preview/nodepools_methods_test.go b/internal/api/v20251223preview/nodepools_methods_test.go index 9777529565e..5d5580f151c 100644 --- a/internal/api/v20251223preview/nodepools_methods_test.go +++ b/internal/api/v20251223preview/nodepools_methods_test.go @@ -40,6 +40,7 @@ func TestSizeGiBRoundTrip(t *testing.T) { { name: "SizeGiB with explicit value should round-trip", original: &api.HCPOpenShiftClusterNodePool{ + CosmosMetadata: arm.CosmosMetadata{ResourceID: api.Must(azcorearm.ParseResourceID(strings.ToLower("/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myRg/providers/Microsoft.RedHatOpenShift/hcpOpenShiftClusters/myCluster/nodePools/myNodePool")))}, TrackedResource: arm.TrackedResource{ Resource: arm.Resource{ ID: api.Must(azcorearm.ParseResourceID(strings.ToLower("/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myRg/providers/Microsoft.RedHatOpenShift/hcpOpenShiftClusters/myCluster/nodePools/myNodePool"))), diff --git a/internal/api/v20251223preview/zero_value_roundtrip_test.go b/internal/api/v20251223preview/zero_value_roundtrip_test.go index 305f44de779..44d79a737fb 100644 --- a/internal/api/v20251223preview/zero_value_roundtrip_test.go +++ b/internal/api/v20251223preview/zero_value_roundtrip_test.go @@ -261,6 +261,7 @@ func jsonRoundTripCluster(t *testing.T, original *api.HCPOpenShiftCluster) *api. // to zero before round-tripping. func newBaselineInternalNodePool() *api.HCPOpenShiftClusterNodePool { return &api.HCPOpenShiftClusterNodePool{ + CosmosMetadata: arm.CosmosMetadata{ResourceID: api.Must(azcorearm.ParseResourceID(strings.ToLower("/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myRg/providers/Microsoft.RedHatOpenShift/hcpOpenShiftClusters/myCluster/nodePools/myNodePool")))}, TrackedResource: arm.TrackedResource{ Resource: arm.Resource{ ID: api.Must(azcorearm.ParseResourceID(strings.ToLower("/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myRg/providers/Microsoft.RedHatOpenShift/hcpOpenShiftClusters/myCluster/nodePools/myNodePool"))), diff --git a/internal/api/zz_generated.deepcopy.go b/internal/api/zz_generated.deepcopy.go index 0080672fdd1..9f2cd0b6b60 100644 --- a/internal/api/zz_generated.deepcopy.go +++ b/internal/api/zz_generated.deepcopy.go @@ -646,6 +646,7 @@ func (in *HCPOpenShiftClusterList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *HCPOpenShiftClusterNodePool) DeepCopyInto(out *HCPOpenShiftClusterNodePool) { *out = *in + in.CosmosMetadata.DeepCopyInto(&out.CosmosMetadata) in.TrackedResource.DeepCopyInto(&out.TrackedResource) in.Properties.DeepCopyInto(&out.Properties) in.ServiceProviderProperties.DeepCopyInto(&out.ServiceProviderProperties) diff --git a/internal/conversion/readonly_nodepool.go b/internal/conversion/readonly_nodepool.go index 6636409f1eb..4887beaae68 100644 --- a/internal/conversion/readonly_nodepool.go +++ b/internal/conversion/readonly_nodepool.go @@ -22,6 +22,10 @@ import ( func CopyReadOnlyNodePoolValues(dest, src *api.HCPOpenShiftClusterNodePool) { CopyReadOnlyTrackedResourceValues(&dest.TrackedResource, &src.TrackedResource) + // CosmosMetadata is read-only on the API surface; carry over so the + // case-preserving ResourceID and CosmosETag survive the replace round-trip. + dest.CosmosMetadata = *src.CosmosMetadata.DeepCopy() + switch { case hasClusterIdentityToSet(src.Identity) && dest.Identity == nil: dest.Identity = &arm.ManagedServiceIdentity{} @@ -34,5 +38,4 @@ func CopyReadOnlyNodePoolValues(dest, src *api.HCPOpenShiftClusterNodePool) { dest.Properties.ProvisioningState = src.Properties.ProvisioningState dest.ServiceProviderProperties = *src.ServiceProviderProperties.DeepCopy() - dest.CosmosETag = src.CosmosETag } diff --git a/internal/database/convert_cluster_test.go b/internal/database/convert_cluster_test.go index 2c886285514..82205ef2594 100644 --- a/internal/database/convert_cluster_test.go +++ b/internal/database/convert_cluster_test.go @@ -138,7 +138,7 @@ func roundTripInternalToCosmosToInternal[InternalAPIType, CosmosAPIType any](t * case *api.HCPOpenShiftCluster: cast.ServiceProviderProperties.ExistingCosmosUID = "" case *api.HCPOpenShiftClusterNodePool: - cast.ServiceProviderProperties.ExistingCosmosUID = "" + cast.ExistingCosmosUID = "" case *api.HCPOpenShiftClusterExternalAuth: cast.ServiceProviderProperties.ExistingCosmosUID = "" } diff --git a/internal/database/convert_nodepool.go b/internal/database/convert_nodepool.go index b39b3d445a0..c02dc62cefc 100644 --- a/internal/database/convert_nodepool.go +++ b/internal/database/convert_nodepool.go @@ -21,7 +21,6 @@ import ( "k8s.io/utils/ptr" "github.com/Azure/ARO-HCP/internal/api" - "github.com/Azure/ARO-HCP/internal/api/arm" ) func InternalToCosmosNodePool(internalObj *api.HCPOpenShiftClusterNodePool) (*NodePool, error) { @@ -29,22 +28,26 @@ func InternalToCosmosNodePool(internalObj *api.HCPOpenShiftClusterNodePool) (*No return nil, nil } + // CosmosMetadata.ResourceID is the canonical identifier for cosmos-side + // concerns (partitioning, document UID, resource-type indexing). Use it + // instead of the TrackedResource.ID, which is an ARM-surface concern. + cosmosResourceID := internalObj.GetCosmosData().ResourceID + if cosmosResourceID == nil { + return nil, fmt.Errorf("internalObj is missing CosmosMetadata.ResourceID") + } cosmosObj := &NodePool{ TypedDocument: TypedDocument{ BaseDocument: BaseDocument{ ID: internalObj.GetCosmosData().GetCosmosUID(), }, - PartitionKey: strings.ToLower(internalObj.ID.SubscriptionID), - ResourceID: internalObj.ID, - ResourceType: internalObj.ID.ResourceType.String(), + PartitionKey: strings.ToLower(cosmosResourceID.SubscriptionID), + ResourceID: cosmosResourceID, + ResourceType: cosmosResourceID.ResourceType.String(), }, NodePoolProperties: NodePoolProperties{ HCPOpenShiftClusterNodePool: *internalObj, - CosmosMetadata: api.CosmosMetadata{ - ResourceID: internalObj.ID, - }, IntermediateResourceDoc: &ResourceDocument{ - ResourceID: internalObj.ID, + ResourceID: cosmosResourceID, InternalID: ptr.Deref(internalObj.ServiceProviderProperties.ClusterServiceID, api.InternalID{}), ActiveOperationID: internalObj.ServiceProviderProperties.ActiveOperationID, ProvisioningState: internalObj.Properties.ProvisioningState, @@ -58,6 +61,8 @@ func InternalToCosmosNodePool(internalObj *api.HCPOpenShiftClusterNodePool) (*No }, } + cosmosObj.InternalState.InternalAPI.CosmosMetadata = api.CosmosMetadata{} + return cosmosObj, nil } @@ -65,42 +70,17 @@ func CosmosToInternalNodePool(cosmosObj *NodePool) (*api.HCPOpenShiftClusterNode if cosmosObj == nil { return nil, nil } - resourceDoc := cosmosObj.IntermediateResourceDoc - if resourceDoc == nil { - return nil, fmt.Errorf("resource document cannot be nil") - } - tempInternalAPI := cosmosObj.InternalState.InternalAPI - internalObj := &tempInternalAPI - - // some pieces of data are stored on the ResourceDocument, so we need to restore that data - internalObj.TrackedResource = arm.TrackedResource{ - Resource: arm.Resource{ - ID: resourceDoc.ResourceID, - Name: resourceDoc.ResourceID.Name, - Type: resourceDoc.ResourceID.ResourceType.String(), - SystemData: resourceDoc.SystemData, - }, - Location: cosmosObj.InternalState.InternalAPI.Location, - Tags: resourceDoc.Tags, + 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) + } } - // we carry over the CosmosETag from the cosmos object to the internal object into a - // temporary field until we have inlined and serialized CosmosMetadata in - // HCPOpenShiftClusterNodePool. - internalObj.CosmosETag = cosmosObj.BaseDocument.CosmosETag - internalObj.Identity = resourceDoc.Identity.DeepCopy() - internalObj.Properties.ProvisioningState = resourceDoc.ProvisioningState - internalObj.SystemData = resourceDoc.SystemData - internalObj.Tags = copyTags(resourceDoc.Tags) - internalObj.ServiceProviderProperties.ExistingCosmosUID = cosmosObj.ID - if len(resourceDoc.InternalID.String()) == 0 { - // preserve the nil on read - internalObj.ServiceProviderProperties.ClusterServiceID = nil - } else { - internalObj.ServiceProviderProperties.ClusterServiceID = &resourceDoc.InternalID - } - - internalObj.ServiceProviderProperties.ActiveOperationID = resourceDoc.ActiveOperationID internalObj.EnsureDefaults() diff --git a/internal/database/convert_nodepool_test.go b/internal/database/convert_nodepool_test.go index 7bf40c9a2b0..35d4ed0fc33 100644 --- a/internal/database/convert_nodepool_test.go +++ b/internal/database/convert_nodepool_test.go @@ -56,13 +56,17 @@ func TestRoundTripNodePoolInternalCosmosInternal(t *testing.T) { foo := api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/" + clusterID + "/node_pools/" + nodePoolID)) j.ClusterServiceID = &foo }, + func(j *api.CosmosMetadata, c randfill.Continue) { + c.FillNoCustom(j) + j.ResourceID = api.Must(azcorearm.ParseResourceID("/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myRg")) + j.ExistingCosmosUID = "" + j.CosmosETag = "" + }, func(j *api.HCPOpenShiftClusterNodePool, c randfill.Continue) { c.FillNoCustom(j) if j == nil { return } - j.ServiceProviderProperties.ExistingCosmosUID = "" - j.CosmosETag = "" // Canonical defaults are applied on Cosmos read, so ensure // defaulted fields are never zero during round-trip testing. if len(j.Properties.Platform.OSDisk.DiskStorageAccountType) == 0 { @@ -103,6 +107,11 @@ func TestCosmosToInternalNodePoolPreservesETag(t *testing.T) { }, }, NodePoolProperties: NodePoolProperties{ + HCPOpenShiftClusterNodePool: api.HCPOpenShiftClusterNodePool{ + CosmosMetadata: arm.CosmosMetadata{ + ResourceID: resourceID, + }, + }, IntermediateResourceDoc: &ResourceDocument{ ResourceID: resourceID, }, diff --git a/internal/database/types_nodepool.go b/internal/database/types_nodepool.go index 16948b40ef5..4c24ac8e084 100644 --- a/internal/database/types_nodepool.go +++ b/internal/database/types_nodepool.go @@ -25,16 +25,12 @@ type NodePool struct { } type NodePoolProperties struct { - // HCPOpenShiftClusterNodePool is where we're migrating to. It is compatible with a GenericDocument[api.HCPOpenShiftClusterNodePool] - // which is where we want to end up. - // * to be compatible with prior versions, we must continue writing all previous fields and this new field - // * to be compatible with prior versions, we must continue reading only from previous fields + // HCPOpenShiftClusterNodePool is the inline serialization that mirrors GenericDocument[api.HCPOpenShiftClusterNodePool] + // (the destination shape for nodepool documents). The reading path now uses these inline fields as the + // source of truth; the IntermediateResourceDoc/InternalState siblings are still written for compatibility + // with old readers, but will be removed once all readers have migrated. api.HCPOpenShiftClusterNodePool `json:",inline"` - // when we switch to inlining the internalObj, this will be in the right spot. We add it now so that we can switch our - // queries to select on cosmosMetadata.ResourceID instead of resourceId - CosmosMetadata api.CosmosMetadata `json:"cosmosMetadata"` - // IntermediateResourceDoc exists so that we can stop inlining the resource document so that we can directly // embed the InternalAPIType which has colliding serialization fields. IntermediateResourceDoc *ResourceDocument `json:"intermediateResourceDoc"` diff --git a/internal/databasetesting/mock_dbclient_test.go b/internal/databasetesting/mock_dbclient_test.go index 8ab938a974b..b9513c617bf 100644 --- a/internal/databasetesting/mock_dbclient_test.go +++ b/internal/databasetesting/mock_dbclient_test.go @@ -900,6 +900,7 @@ func TestNewMockResourcesDBClientWithResources(t *testing.T) { "/nodePools/" + nodePoolName)) nodePool := &api.HCPOpenShiftClusterNodePool{ + CosmosMetadata: arm.CosmosMetadata{ResourceID: nodePoolResourceID}, TrackedResource: arm.TrackedResource{ Resource: arm.Resource{ ID: nodePoolResourceID, diff --git a/test-integration/frontend/informer_test.go b/test-integration/frontend/informer_test.go index 04847beafcc..f0d10cad79d 100644 --- a/test-integration/frontend/informer_test.go +++ b/test-integration/frontend/informer_test.go @@ -456,6 +456,7 @@ func nodePoolInformerIntegrationTestCase() informerIntegrationTestCase { "/nodePools/"+name) internalID := api.Ptr(api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/" + clusterName + "/node_pools/" + name))) return &api.HCPOpenShiftClusterNodePool{ + CosmosMetadata: arm.CosmosMetadata{ResourceID: npResourceID}, TrackedResource: arm.TrackedResource{ Resource: arm.Resource{ ID: npResourceID, diff --git a/test-integration/utils/databasemutationhelpers/per_resource_comparer.go b/test-integration/utils/databasemutationhelpers/per_resource_comparer.go index 579174dd6c2..4541401a9ee 100644 --- a/test-integration/utils/databasemutationhelpers/per_resource_comparer.go +++ b/test-integration/utils/databasemutationhelpers/per_resource_comparer.go @@ -120,6 +120,7 @@ func ResourceInstanceEquals(t *testing.T, expected, actual any) (string, bool) { unstructured.RemoveNestedField(currMap, prepend(possiblePrepend, "internalState", "internalAPI", "serviceProviderProperties", "clusterServiceID")...) // cluster, nodepool, externalauth - randomly generated by cluster-service mock unstructured.RemoveNestedField(currMap, prepend(possiblePrepend, "internalState", "internalAPI", "systemData", "createdAt")...) // cluster, nodepool, externalauth - varies on every run unstructured.RemoveNestedField(currMap, prepend(possiblePrepend, "internalState", "internalAPI", "systemData", "lastModifiedAt")...) // cluster, nodepool, externalauth - varies on every run + unstructured.RemoveNestedField(currMap, prepend(possiblePrepend, "internalState", "internalAPI", "cosmosMetadata")...) // cluster, nodepool - redundant copy of inline cosmosMetadata unstructured.RemoveNestedField(currMap, prepend(possiblePrepend, "cosmosMetadata", "etag")...) // inline serialization (cluster, nodepool, externalauth) also exposes these UUID-generated / variable fields directly under properties