diff --git a/admin/server/handlers/hcp/breakglass/create.go b/admin/server/handlers/hcp/breakglass/create.go index c1337eb52fe..87b015ffc96 100644 --- a/admin/server/handlers/hcp/breakglass/create.go +++ b/admin/server/handlers/hcp/breakglass/create.go @@ -77,12 +77,15 @@ func (h *HCPBreakglassSessionCreationHandler) ServeHTTP(writer http.ResponseWrit return fmt.Errorf("failed to get HCP from database: %w", err) } - clusterHypershiftDetails, err := h.csClient.GetClusterHypershiftDetails(request.Context(), hcp.ServiceProviderProperties.ClusterServiceID) + if hcp.ServiceProviderProperties.ClusterServiceID == nil { + return fmt.Errorf("cluster has no ClusterServiceID") + } + clusterHypershiftDetails, err := h.csClient.GetClusterHypershiftDetails(request.Context(), *hcp.ServiceProviderProperties.ClusterServiceID) if err != nil { return hcphelpers.ClusterServiceError(err, "hypershift details") } - provisionShard, err := h.csClient.GetClusterProvisionShard(request.Context(), hcp.ServiceProviderProperties.ClusterServiceID) + provisionShard, err := h.csClient.GetClusterProvisionShard(request.Context(), *hcp.ServiceProviderProperties.ClusterServiceID) if err != nil { return hcphelpers.ClusterServiceError(err, "provision shard") } diff --git a/admin/server/handlers/hcp/helloworld.go b/admin/server/handlers/hcp/helloworld.go index 706ce57be6a..ee23c13f41b 100644 --- a/admin/server/handlers/hcp/helloworld.go +++ b/admin/server/handlers/hcp/helloworld.go @@ -65,7 +65,10 @@ func (h *HCPHelloWorldHandler) ServeHTTP(writer http.ResponseWriter, request *ht } // get CS cluster data - once the sync from CS to cosmos is in place, we should not need this anymore - csCluster, err := h.csClient.GetCluster(request.Context(), hcp.ServiceProviderProperties.ClusterServiceID) + if hcp.ServiceProviderProperties.ClusterServiceID == nil { + return fmt.Errorf("cluster has no ClusterServiceID") + } + csCluster, err := h.csClient.GetCluster(request.Context(), *hcp.ServiceProviderProperties.ClusterServiceID) if err != nil { return fmt.Errorf("failed to get CS cluster data: %w", err) } diff --git a/admin/server/handlers/hcp/serialconsole_test.go b/admin/server/handlers/hcp/serialconsole_test.go index 298e86f43ad..d16bbd71572 100644 --- a/admin/server/handlers/hcp/serialconsole_test.go +++ b/admin/server/handlers/hcp/serialconsole_test.go @@ -98,7 +98,7 @@ func TestSerialConsoleHandler(t *testing.T) { Resource: arm.Resource{ID: resourceID}, }, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ - ClusterServiceID: internalID, + ClusterServiceID: &internalID, }, } _, err = mockDB.HCPClusters(resourceID.SubscriptionID, resourceID.ResourceGroupName).Create(ctx, hcp, nil) @@ -121,7 +121,7 @@ func TestSerialConsoleHandler(t *testing.T) { Resource: arm.Resource{ID: resourceID}, }, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ - ClusterServiceID: internalID, + ClusterServiceID: &internalID, }, } _, err = mockDB.HCPClusters(resourceID.SubscriptionID, resourceID.ResourceGroupName).Create(ctx, hcp, nil) diff --git a/backend/pkg/controllers/billingcontrollers/create_billing_doc_test.go b/backend/pkg/controllers/billingcontrollers/create_billing_doc_test.go index 84ac884b9dc..aa03bddc59a 100644 --- a/backend/pkg/controllers/billingcontrollers/create_billing_doc_test.go +++ b/backend/pkg/controllers/billingcontrollers/create_billing_doc_test.go @@ -96,7 +96,7 @@ func newTestCluster(t *testing.T, clusterUID string, provisioningState arm.Provi ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ ProvisioningState: provisioningState, ClusterUID: clusterUID, - ClusterServiceID: api.Must(api.NewInternalID(testClusterServiceIDStr)), + ClusterServiceID: api.Ptr(api.Must(api.NewInternalID(testClusterServiceIDStr))), }, } } diff --git a/backend/pkg/controllers/billingcontrollers/orphaned_billing_cleanup_test.go b/backend/pkg/controllers/billingcontrollers/orphaned_billing_cleanup_test.go index 33e4332a1e8..38281dc6371 100644 --- a/backend/pkg/controllers/billingcontrollers/orphaned_billing_cleanup_test.go +++ b/backend/pkg/controllers/billingcontrollers/orphaned_billing_cleanup_test.go @@ -139,7 +139,7 @@ func TestOrphanedBillingCleanup_SyncOnce(t *testing.T) { ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ ProvisioningState: arm.ProvisioningStateSucceeded, ClusterUID: "billing-doc-2", - ClusterServiceID: api.Must(api.NewInternalID(testClusterServiceIDStr)), + ClusterServiceID: api.Ptr(api.Must(api.NewInternalID(testClusterServiceIDStr))), }, }, }, diff --git a/backend/pkg/controllers/clusterpropertiescontroller/cluster_customer_properties_migration.go b/backend/pkg/controllers/clusterpropertiescontroller/cluster_customer_properties_migration.go index 4528b73be9c..befa9686f16 100644 --- a/backend/pkg/controllers/clusterpropertiescontroller/cluster_customer_properties_migration.go +++ b/backend/pkg/controllers/clusterpropertiescontroller/cluster_customer_properties_migration.go @@ -75,7 +75,7 @@ func (c *clusterCustomerPropertiesMigrationController) CooldownChecker() control func (c *clusterCustomerPropertiesMigrationController) NeedsWork(ctx context.Context, existingCluster *api.HCPOpenShiftCluster) bool { // Check if we have a cluster service ID to query. We will lack this information for newly created records when we // transition to async cluster-service creation. - if len(existingCluster.ServiceProviderProperties.ClusterServiceID.String()) == 0 { + if existingCluster.ServiceProviderProperties.ClusterServiceID == nil || len(existingCluster.ServiceProviderProperties.ClusterServiceID.String()) == 0 { return false } @@ -124,7 +124,7 @@ func (c *clusterCustomerPropertiesMigrationController) SyncOnce(ctx context.Cont } // Fetch the cluster from Cluster Service - csCluster, err := c.clusterServiceClient.GetCluster(ctx, existingCluster.ServiceProviderProperties.ClusterServiceID) + csCluster, err := c.clusterServiceClient.GetCluster(ctx, *existingCluster.ServiceProviderProperties.ClusterServiceID) if err != nil { return utils.TrackError(fmt.Errorf("failed to get cluster from Cluster Service: %w", err)) } diff --git a/backend/pkg/controllers/clusterpropertiescontroller/cluster_properties_sync.go b/backend/pkg/controllers/clusterpropertiescontroller/cluster_properties_sync.go index c81e4fe4062..34f61b914b2 100644 --- a/backend/pkg/controllers/clusterpropertiescontroller/cluster_properties_sync.go +++ b/backend/pkg/controllers/clusterpropertiescontroller/cluster_properties_sync.go @@ -95,7 +95,7 @@ func (c *clusterPropertiesSyncer) SyncOnce(ctx context.Context, key controllerut } // Check if we have a cluster service ID to query - if len(existingCluster.ServiceProviderProperties.ClusterServiceID.String()) == 0 { + if existingCluster.ServiceProviderProperties.ClusterServiceID == nil || len(existingCluster.ServiceProviderProperties.ClusterServiceID.String()) == 0 { return nil } @@ -112,7 +112,7 @@ func (c *clusterPropertiesSyncer) SyncOnce(ctx context.Context, key controllerut } // Fetch the cluster from Cluster Service - csCluster, err := c.clusterServiceClient.GetCluster(ctx, existingCluster.ServiceProviderProperties.ClusterServiceID) + csCluster, err := c.clusterServiceClient.GetCluster(ctx, *existingCluster.ServiceProviderProperties.ClusterServiceID) if err != nil { return utils.TrackError(fmt.Errorf("failed to get cluster from Cluster Service: %w", err)) } diff --git a/backend/pkg/controllers/clusterpropertiescontroller/cluster_properties_sync_test.go b/backend/pkg/controllers/clusterpropertiescontroller/cluster_properties_sync_test.go index 7118a8f4c9a..cb3aec2a6bb 100644 --- a/backend/pkg/controllers/clusterpropertiescontroller/cluster_properties_sync_test.go +++ b/backend/pkg/controllers/clusterpropertiescontroller/cluster_properties_sync_test.go @@ -338,7 +338,7 @@ func newTestCluster(opts ...func(*api.HCPOpenShiftCluster)) *api.HCPOpenShiftClu }, }, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ - ClusterServiceID: api.Must(api.NewInternalID(testClusterServiceIDStr)), + ClusterServiceID: api.Ptr(api.Must(api.NewInternalID(testClusterServiceIDStr))), }, } diff --git a/backend/pkg/controllers/clusterpropertiescontroller/identity_migration.go b/backend/pkg/controllers/clusterpropertiescontroller/identity_migration.go index bd53738471b..02c5b6c03cb 100644 --- a/backend/pkg/controllers/clusterpropertiescontroller/identity_migration.go +++ b/backend/pkg/controllers/clusterpropertiescontroller/identity_migration.go @@ -79,7 +79,7 @@ func (c *identityMigrationSyncer) CooldownChecker() controllerutils.CooldownChec func (c *identityMigrationSyncer) NeedsWork(ctx context.Context, existingCluster *api.HCPOpenShiftCluster) bool { // Check if we have a cluster service ID to query - if len(existingCluster.ServiceProviderProperties.ClusterServiceID.String()) == 0 { + if existingCluster.ServiceProviderProperties.ClusterServiceID == nil || len(existingCluster.ServiceProviderProperties.ClusterServiceID.String()) == 0 { return false } @@ -134,7 +134,7 @@ func (c *identityMigrationSyncer) SyncOnce(ctx context.Context, key controllerut } // Fetch the cluster from Cluster Service - csCluster, err := c.clusterServiceClient.GetCluster(ctx, existingCluster.ServiceProviderProperties.ClusterServiceID) + csCluster, err := c.clusterServiceClient.GetCluster(ctx, *existingCluster.ServiceProviderProperties.ClusterServiceID) if err != nil { return utils.TrackError(fmt.Errorf("failed to get cluster from Cluster Service: %w", err)) } diff --git a/backend/pkg/controllers/create_cluster_scoped_maestro_readonly_bundles_controller.go b/backend/pkg/controllers/create_cluster_scoped_maestro_readonly_bundles_controller.go index b8e67f6fe1b..2e4698cd21e 100644 --- a/backend/pkg/controllers/create_cluster_scoped_maestro_readonly_bundles_controller.go +++ b/backend/pkg/controllers/create_cluster_scoped_maestro_readonly_bundles_controller.go @@ -107,6 +107,11 @@ func (c *createClusterScopedMaestroReadonlyBundlesSyncer) SyncOnce(ctx context.C if err != nil { return utils.TrackError(fmt.Errorf("failed to get Cluster: %w", err)) } + if existingCluster.ServiceProviderProperties.ClusterServiceID == nil { + // we don't have enough information to proceed. We will retrigger once the information is present. + // TODO remove this once we have the information all in cosmos. + return nil + } existingServiceProviderCluster, err := database.GetOrCreateServiceProviderCluster(ctx, c.cosmosClient, key.GetResourceID()) if err != nil { @@ -156,7 +161,7 @@ func (c *createClusterScopedMaestroReadonlyBundlesSyncer) SyncOnce(ctx context.C // we are guaranteed to have a shard allocated for the cluster. If this changes in the future // we would need to change the logic in controllers to check that the retrieved cluster has a // shard allocated. - clusterProvisionShard, err := c.clusterServiceClient.GetClusterProvisionShard(ctx, existingCluster.ServiceProviderProperties.ClusterServiceID) + clusterProvisionShard, err := c.clusterServiceClient.GetClusterProvisionShard(ctx, *existingCluster.ServiceProviderProperties.ClusterServiceID) if err != nil { return utils.TrackError(fmt.Errorf("failed to get Cluster Provision Shard from Cluster Service: %w", err)) } @@ -170,7 +175,7 @@ func (c *createClusterScopedMaestroReadonlyBundlesSyncer) SyncOnce(ctx context.C return utils.TrackError(fmt.Errorf("failed to create Maestro client: %w", err)) } - csCluster, err := c.clusterServiceClient.GetCluster(ctx, existingCluster.ServiceProviderProperties.ClusterServiceID) + csCluster, err := c.clusterServiceClient.GetCluster(ctx, *existingCluster.ServiceProviderProperties.ClusterServiceID) if err != nil { return utils.TrackError(fmt.Errorf("failed to get Cluster from Cluster Service: %w", err)) } diff --git a/backend/pkg/controllers/create_cluster_scoped_maestro_readonly_bundles_controller_test.go b/backend/pkg/controllers/create_cluster_scoped_maestro_readonly_bundles_controller_test.go index 8f2a70fb855..348fb2f792a 100644 --- a/backend/pkg/controllers/create_cluster_scoped_maestro_readonly_bundles_controller_test.go +++ b/backend/pkg/controllers/create_cluster_scoped_maestro_readonly_bundles_controller_test.go @@ -135,7 +135,7 @@ func TestBuildInitialReadonlyMaestroBundleForHostedCluster(t *testing.T) { }, }, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ - ClusterServiceID: api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/11111111111111111111111111111111")), + ClusterServiceID: api.Ptr(api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/11111111111111111111111111111111"))), }, } @@ -409,7 +409,7 @@ func TestCreateClusterScopedMaestroReadonlyBundlesSyncer_syncMaestroBundle(t *te Resource: arm.Resource{ID: clusterResourceID}, }, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ - ClusterServiceID: api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/11111111111111111111111111111111")), + ClusterServiceID: api.Ptr(api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/11111111111111111111111111111111"))), }, } @@ -545,7 +545,7 @@ func TestCreateClusterScopedMaestroReadonlyBundlesSyncer_SyncOnce_GetServiceProv cluster := &api.HCPOpenShiftCluster{ TrackedResource: arm.TrackedResource{Resource: arm.Resource{ID: clusterResourceID}}, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ - ClusterServiceID: api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/11111111111111111111111111111111")), + ClusterServiceID: api.Ptr(api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/11111111111111111111111111111111"))), }, } @@ -597,7 +597,7 @@ func TestCreateClusterScopedMaestroReadonlyBundlesSyncer_SyncOnce_AllBundlesAlre Resource: arm.Resource{ID: clusterResourceID}, }, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ - ClusterServiceID: api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/11111111111111111111111111111111")), + ClusterServiceID: api.Ptr(api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/11111111111111111111111111111111"))), }, } clustersCRUD := mockDBClient.HCPClusters(key.SubscriptionID, key.ResourceGroupName) @@ -669,7 +669,7 @@ func TestCreateClusterScopedMaestroReadonlyBundlesSyncer_SyncOnce_SyncLoopExecut }, }, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ - ClusterServiceID: api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/11111111111111111111111111111111")), + ClusterServiceID: api.Ptr(api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/11111111111111111111111111111111"))), }, } @@ -698,7 +698,7 @@ func TestCreateClusterScopedMaestroReadonlyBundlesSyncer_SyncOnce_SyncLoopExecut // Setup cluster service mocks provisionShard := buildTestProvisionShard("test-consumer") mockClusterService.EXPECT(). - GetClusterProvisionShard(gomock.Any(), cluster.ServiceProviderProperties.ClusterServiceID). + GetClusterProvisionShard(gomock.Any(), *cluster.ServiceProviderProperties.ClusterServiceID). Return(provisionShard, nil) csCluster, err := arohcpv1alpha1.NewCluster(). @@ -706,7 +706,7 @@ func TestCreateClusterScopedMaestroReadonlyBundlesSyncer_SyncOnce_SyncLoopExecut Build() require.NoError(t, err) mockClusterService.EXPECT(). - GetCluster(gomock.Any(), cluster.ServiceProviderProperties.ClusterServiceID). + GetCluster(gomock.Any(), *cluster.ServiceProviderProperties.ClusterServiceID). Return(csCluster, nil) // Setup maestro builder mock @@ -781,7 +781,7 @@ func TestCreateClusterScopedMaestroReadonlyBundlesSyncer_SyncOnce_ProcessesParti Resource: arm.Resource{ID: clusterResourceID}, }, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ - ClusterServiceID: api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/11111111111111111111111111111111")), + ClusterServiceID: api.Ptr(api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/11111111111111111111111111111111"))), }, } clustersCRUD := mockDBClient.HCPClusters(key.SubscriptionID, key.ResourceGroupName) @@ -815,12 +815,12 @@ func TestCreateClusterScopedMaestroReadonlyBundlesSyncer_SyncOnce_ProcessesParti provisionShard := buildTestProvisionShard("test-consumer") mockClusterService.EXPECT(). - GetClusterProvisionShard(gomock.Any(), cluster.ServiceProviderProperties.ClusterServiceID). + GetClusterProvisionShard(gomock.Any(), *cluster.ServiceProviderProperties.ClusterServiceID). Return(provisionShard, nil) csCluster, err := arohcpv1alpha1.NewCluster().DomainPrefix("test-domain").Build() require.NoError(t, err) mockClusterService.EXPECT(). - GetCluster(gomock.Any(), cluster.ServiceProviderProperties.ClusterServiceID). + GetCluster(gomock.Any(), *cluster.ServiceProviderProperties.ClusterServiceID). Return(csCluster, nil) restEndpoint := provisionShard.MaestroConfig().RestApiConfig().Url() diff --git a/backend/pkg/controllers/datadumpcontrollers/cs_state_dump.go b/backend/pkg/controllers/datadumpcontrollers/cs_state_dump.go index ceadaf4cc7c..35c0a3c811b 100644 --- a/backend/pkg/controllers/datadumpcontrollers/cs_state_dump.go +++ b/backend/pkg/controllers/datadumpcontrollers/cs_state_dump.go @@ -83,11 +83,11 @@ func (c *csStateDump) SyncOnce(ctx context.Context, key controllerutils.HCPClust return nil // best effort, don't fail } - csID := cluster.ServiceProviderProperties.ClusterServiceID - if len(csID.String()) == 0 { + if cluster.ServiceProviderProperties.ClusterServiceID == nil || len(cluster.ServiceProviderProperties.ClusterServiceID.String()) == 0 { // No ClusterServiceID yet, cluster hasn't been registered with CS return nil } + csID := *cluster.ServiceProviderProperties.ClusterServiceID // Fetch cluster state from cluster-service csCluster, err := c.csClient.GetCluster(ctx, csID) diff --git a/backend/pkg/controllers/datadumpcontrollers/cs_state_dump_test.go b/backend/pkg/controllers/datadumpcontrollers/cs_state_dump_test.go index 454667c1306..b2c2134bd89 100644 --- a/backend/pkg/controllers/datadumpcontrollers/cs_state_dump_test.go +++ b/backend/pkg/controllers/datadumpcontrollers/cs_state_dump_test.go @@ -177,7 +177,7 @@ func TestCSStateDump_SyncOnce(t *testing.T) { Resource: arm.Resource{ID: clusterResourceID}, }, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ - ClusterServiceID: csID, + ClusterServiceID: &csID, }, } diff --git a/backend/pkg/controllers/delete_orphaned_maestro_readonly_bundles_controller.go b/backend/pkg/controllers/delete_orphaned_maestro_readonly_bundles_controller.go index fc2ea05c54e..17a063a29c7 100644 --- a/backend/pkg/controllers/delete_orphaned_maestro_readonly_bundles_controller.go +++ b/backend/pkg/controllers/delete_orphaned_maestro_readonly_bundles_controller.go @@ -478,7 +478,7 @@ func (c *deleteOrphanedMaestroReadonlyBundles) clusterProvisionShardIDForService // provisionShardIDFromCluster resolves the provision shard for a Cosmos cluster document. skip is true when ClusterServiceID // is unset so the cluster is not yet registered with Cluster Service (same gate as create-*-scoped Maestro bundle controllers). func (c *deleteOrphanedMaestroReadonlyBundles) provisionShardIDFromCluster(ctx context.Context, cluster *api.HCPOpenShiftCluster) (shardID string, skip bool, err error) { - if len(cluster.ServiceProviderProperties.ClusterServiceID.String()) == 0 { + if cluster.ServiceProviderProperties.ClusterServiceID == nil || len(cluster.ServiceProviderProperties.ClusterServiceID.String()) == 0 { return "", true, nil } // TODO We get the provision shard ID from CS but at some point we should have @@ -486,7 +486,7 @@ func (c *deleteOrphanedMaestroReadonlyBundles) provisionShardIDFromCluster(ctx c // TODO should we take into account that at some point in the future we will implement migration between management // clusters, where a cluster could have bundles allocated to different provision shards at the same time? For now // we assume that the cluster is associated to a single provision shard at a time. - clusterCSShard, err := c.clusterServiceClient.GetClusterProvisionShard(ctx, cluster.ServiceProviderProperties.ClusterServiceID) + clusterCSShard, err := c.clusterServiceClient.GetClusterProvisionShard(ctx, *cluster.ServiceProviderProperties.ClusterServiceID) if err != nil { return "", false, utils.TrackError(fmt.Errorf("failed to get Cluster Provision Shard: %w", err)) } diff --git a/backend/pkg/controllers/delete_orphaned_maestro_readonly_bundles_controller_test.go b/backend/pkg/controllers/delete_orphaned_maestro_readonly_bundles_controller_test.go index d94d92ec828..b2c0f054012 100644 --- a/backend/pkg/controllers/delete_orphaned_maestro_readonly_bundles_controller_test.go +++ b/backend/pkg/controllers/delete_orphaned_maestro_readonly_bundles_controller_test.go @@ -27,6 +27,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/utils/ptr" azcorearm "github.com/Azure/azure-sdk-for-go/sdk/azcore/arm" @@ -770,7 +771,7 @@ func TestDeleteOrphanedMaestroReadonlyBundles_mapServiceProviderClustersByProvis cluster := &api.HCPOpenShiftCluster{ TrackedResource: arm.TrackedResource{Resource: arm.Resource{ID: clusterResourceID}}, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ - ClusterServiceID: api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/csid")), + ClusterServiceID: api.Ptr(api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/csid"))), }, } _, err := mockDB.HCPClusters("sub", "rg").Create(ctx, cluster, nil) @@ -779,7 +780,7 @@ func TestDeleteOrphanedMaestroReadonlyBundles_mapServiceProviderClustersByProvis CosmosMetadata: arm.CosmosMetadata{ResourceID: spcResourceID}, ResourceID: *spcResourceID, } - mockCS.EXPECT().GetClusterProvisionShard(gomock.Any(), cluster.ServiceProviderProperties.ClusterServiceID).Return(nil, fmt.Errorf("provision shard error")) + mockCS.EXPECT().GetClusterProvisionShard(gomock.Any(), *cluster.ServiceProviderProperties.ClusterServiceID).Return(nil, fmt.Errorf("provision shard error")) return &deleteOrphanedMaestroReadonlyBundles{cosmosClient: mockDB, clusterServiceClient: mockCS}, map[string]*shardMaestroClient{"unused-shard": noopMaestroShardClient}, []*api.ServiceProviderCluster{spc} @@ -795,7 +796,7 @@ func TestDeleteOrphanedMaestroReadonlyBundles_mapServiceProviderClustersByProvis cluster := &api.HCPOpenShiftCluster{ TrackedResource: arm.TrackedResource{Resource: arm.Resource{ID: clusterResourceID}}, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ - ClusterServiceID: api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/csid")), + ClusterServiceID: api.Ptr(api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/csid"))), }, } _, err := mockDB.HCPClusters("sub", "rg").Create(ctx, cluster, nil) @@ -816,7 +817,7 @@ func TestDeleteOrphanedMaestroReadonlyBundles_mapServiceProviderClustersByProvis ). Build() require.NoError(t, err) - mockCS.EXPECT().GetClusterProvisionShard(gomock.Any(), cluster.ServiceProviderProperties.ClusterServiceID).Return(shardReturnedByCS, nil) + mockCS.EXPECT().GetClusterProvisionShard(gomock.Any(), *cluster.ServiceProviderProperties.ClusterServiceID).Return(shardReturnedByCS, nil) clients := map[string]*shardMaestroClient{ shardInClientsMap.ID(): noopMaestroShardClient, } @@ -833,7 +834,7 @@ func TestDeleteOrphanedMaestroReadonlyBundles_mapServiceProviderClustersByProvis cluster := &api.HCPOpenShiftCluster{ TrackedResource: arm.TrackedResource{Resource: arm.Resource{ID: clusterResourceID}}, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ - ClusterServiceID: api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/csid")), + ClusterServiceID: api.Ptr(api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/csid"))), }, } _, err := mockDB.HCPClusters("sub", "rg").Create(ctx, cluster, nil) @@ -843,7 +844,7 @@ func TestDeleteOrphanedMaestroReadonlyBundles_mapServiceProviderClustersByProvis ResourceID: *spcResourceID, } provisionShard := buildTestProvisionShard("test-consumer") - mockCS.EXPECT().GetClusterProvisionShard(gomock.Any(), cluster.ServiceProviderProperties.ClusterServiceID).Return(provisionShard, nil) + mockCS.EXPECT().GetClusterProvisionShard(gomock.Any(), *cluster.ServiceProviderProperties.ClusterServiceID).Return(provisionShard, nil) clients := map[string]*shardMaestroClient{provisionShard.ID(): noopMaestroShardClient} return &deleteOrphanedMaestroReadonlyBundles{cosmosClient: mockDB, clusterServiceClient: mockCS}, clients, []*api.ServiceProviderCluster{spc} }, @@ -866,13 +867,13 @@ func TestDeleteOrphanedMaestroReadonlyBundles_mapServiceProviderClustersByProvis cluster1 := &api.HCPOpenShiftCluster{ TrackedResource: arm.TrackedResource{Resource: arm.Resource{ID: cluster1ResourceID}}, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ - ClusterServiceID: api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/csid1")), + ClusterServiceID: api.Ptr(api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/csid1"))), }, } cluster2 := &api.HCPOpenShiftCluster{ TrackedResource: arm.TrackedResource{Resource: arm.Resource{ID: cluster2ResourceID}}, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ - ClusterServiceID: api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/csid2")), + ClusterServiceID: api.Ptr(api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/csid2"))), }, } _, err := mockDB.HCPClusters("sub1", "rg1").Create(ctx, cluster1, nil) @@ -904,8 +905,8 @@ func TestDeleteOrphanedMaestroReadonlyBundles_mapServiceProviderClustersByProvis Build() require.NoError(t, err) - mockCS.EXPECT().GetClusterProvisionShard(gomock.Any(), cluster1.ServiceProviderProperties.ClusterServiceID).Return(shard1, nil) - mockCS.EXPECT().GetClusterProvisionShard(gomock.Any(), cluster2.ServiceProviderProperties.ClusterServiceID).Return(shard2, nil) + mockCS.EXPECT().GetClusterProvisionShard(gomock.Any(), *cluster1.ServiceProviderProperties.ClusterServiceID).Return(shard1, nil) + mockCS.EXPECT().GetClusterProvisionShard(gomock.Any(), *cluster2.ServiceProviderProperties.ClusterServiceID).Return(shard2, nil) clients := map[string]*shardMaestroClient{ shard1.ID(): noopMaestroShardClient, @@ -956,7 +957,7 @@ func TestDeleteOrphanedMaestroReadonlyBundles_provisionShardIDFromCluster(t *tes c := &deleteOrphanedMaestroReadonlyBundles{clusterServiceClient: mockCS} cluster := &api.HCPOpenShiftCluster{ ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ - ClusterServiceID: api.InternalID{}, + ClusterServiceID: nil, }, } shardID, skip, err := c.provisionShardIDFromCluster(ctx, cluster) @@ -972,7 +973,7 @@ func TestDeleteOrphanedMaestroReadonlyBundles_provisionShardIDFromCluster(t *tes csID := api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/csid")) cluster := &api.HCPOpenShiftCluster{ ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ - ClusterServiceID: csID, + ClusterServiceID: &csID, }, } provisionShard := buildTestProvisionShard("consumer") @@ -1063,7 +1064,7 @@ func TestDeleteOrphanedMaestroReadonlyBundles_mapServiceProviderNodePoolsByProvi cluster := &api.HCPOpenShiftCluster{ TrackedResource: arm.TrackedResource{Resource: arm.Resource{ID: clusterResourceID}}, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ - ClusterServiceID: api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/csid")), + ClusterServiceID: ptr.To(api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/csid"))), }, } _, err := mockDB.HCPClusters("sub", "rg").Create(ctx, cluster, nil) @@ -1072,7 +1073,7 @@ func TestDeleteOrphanedMaestroReadonlyBundles_mapServiceProviderNodePoolsByProvi CosmosMetadata: arm.CosmosMetadata{ResourceID: spnpResourceID}, ResourceID: *spnpResourceID, } - mockCS.EXPECT().GetClusterProvisionShard(gomock.Any(), cluster.ServiceProviderProperties.ClusterServiceID).Return(nil, fmt.Errorf("provision shard error")) + mockCS.EXPECT().GetClusterProvisionShard(gomock.Any(), *cluster.ServiceProviderProperties.ClusterServiceID).Return(nil, fmt.Errorf("provision shard error")) return &deleteOrphanedMaestroReadonlyBundles{cosmosClient: mockDB, clusterServiceClient: mockCS}, map[string]*shardMaestroClient{"unused-shard": noopMaestroShardClient}, []*api.ServiceProviderNodePool{spnp} @@ -1088,7 +1089,7 @@ func TestDeleteOrphanedMaestroReadonlyBundles_mapServiceProviderNodePoolsByProvi cluster := &api.HCPOpenShiftCluster{ TrackedResource: arm.TrackedResource{Resource: arm.Resource{ID: clusterResourceID}}, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ - ClusterServiceID: api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/csid")), + ClusterServiceID: ptr.To(api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/csid"))), }, } _, err := mockDB.HCPClusters("sub", "rg").Create(ctx, cluster, nil) @@ -1109,7 +1110,7 @@ func TestDeleteOrphanedMaestroReadonlyBundles_mapServiceProviderNodePoolsByProvi ). Build() require.NoError(t, err) - mockCS.EXPECT().GetClusterProvisionShard(gomock.Any(), cluster.ServiceProviderProperties.ClusterServiceID).Return(shardReturnedByCS, nil) + mockCS.EXPECT().GetClusterProvisionShard(gomock.Any(), *cluster.ServiceProviderProperties.ClusterServiceID).Return(shardReturnedByCS, nil) clients := map[string]*shardMaestroClient{ shardInClientsMap.ID(): noopMaestroShardClient, } @@ -1126,7 +1127,7 @@ func TestDeleteOrphanedMaestroReadonlyBundles_mapServiceProviderNodePoolsByProvi cluster := &api.HCPOpenShiftCluster{ TrackedResource: arm.TrackedResource{Resource: arm.Resource{ID: clusterResourceID}}, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ - ClusterServiceID: api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/csid")), + ClusterServiceID: ptr.To(api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/csid"))), }, } _, err := mockDB.HCPClusters("sub", "rg").Create(ctx, cluster, nil) @@ -1136,7 +1137,7 @@ func TestDeleteOrphanedMaestroReadonlyBundles_mapServiceProviderNodePoolsByProvi ResourceID: *spnpResourceID, } provisionShard := buildTestProvisionShard("test-consumer") - mockCS.EXPECT().GetClusterProvisionShard(gomock.Any(), cluster.ServiceProviderProperties.ClusterServiceID).Return(provisionShard, nil) + mockCS.EXPECT().GetClusterProvisionShard(gomock.Any(), *cluster.ServiceProviderProperties.ClusterServiceID).Return(provisionShard, nil) clients := map[string]*shardMaestroClient{provisionShard.ID(): noopMaestroShardClient} return &deleteOrphanedMaestroReadonlyBundles{cosmosClient: mockDB, clusterServiceClient: mockCS}, clients, []*api.ServiceProviderNodePool{spnp} }, @@ -1159,13 +1160,13 @@ func TestDeleteOrphanedMaestroReadonlyBundles_mapServiceProviderNodePoolsByProvi cluster1 := &api.HCPOpenShiftCluster{ TrackedResource: arm.TrackedResource{Resource: arm.Resource{ID: cluster1ResourceID}}, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ - ClusterServiceID: api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/csid1")), + ClusterServiceID: ptr.To(api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/csid1"))), }, } cluster2 := &api.HCPOpenShiftCluster{ TrackedResource: arm.TrackedResource{Resource: arm.Resource{ID: cluster2ResourceID}}, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ - ClusterServiceID: api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/csid2")), + ClusterServiceID: ptr.To(api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/csid2"))), }, } _, err := mockDB.HCPClusters("sub1", "rg1").Create(ctx, cluster1, nil) @@ -1197,8 +1198,8 @@ func TestDeleteOrphanedMaestroReadonlyBundles_mapServiceProviderNodePoolsByProvi Build() require.NoError(t, err) - mockCS.EXPECT().GetClusterProvisionShard(gomock.Any(), cluster1.ServiceProviderProperties.ClusterServiceID).Return(shard1, nil) - mockCS.EXPECT().GetClusterProvisionShard(gomock.Any(), cluster2.ServiceProviderProperties.ClusterServiceID).Return(shard2, nil) + mockCS.EXPECT().GetClusterProvisionShard(gomock.Any(), *cluster1.ServiceProviderProperties.ClusterServiceID).Return(shard1, nil) + mockCS.EXPECT().GetClusterProvisionShard(gomock.Any(), *cluster2.ServiceProviderProperties.ClusterServiceID).Return(shard2, nil) clients := map[string]*shardMaestroClient{ shard1.ID(): noopMaestroShardClient, @@ -1308,12 +1309,12 @@ func TestDeleteOrphanedMaestroReadonlyBundles_ensureClusterScopedOrphanedMaestro cluster := &api.HCPOpenShiftCluster{ TrackedResource: arm.TrackedResource{Resource: arm.Resource{ID: clusterRID}}, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ - ClusterServiceID: api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/csid")), + ClusterServiceID: ptr.To(api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/csid"))), }, } _, err := mockDB.HCPClusters(clusterRID.SubscriptionID, clusterRID.ResourceGroupName).Create(ctx, cluster, nil) require.NoError(t, err) - mockCS.EXPECT().GetClusterProvisionShard(gomock.Any(), cluster.ServiceProviderProperties.ClusterServiceID).Return(shard, nil).AnyTimes() + mockCS.EXPECT().GetClusterProvisionShard(gomock.Any(), *cluster.ServiceProviderProperties.ClusterServiceID).Return(shard, nil).AnyTimes() spc := &api.ServiceProviderCluster{ CosmosMetadata: arm.CosmosMetadata{ResourceID: spcResourceID}, ResourceID: *spcResourceID, @@ -1349,12 +1350,12 @@ func TestDeleteOrphanedMaestroReadonlyBundles_ensureClusterScopedOrphanedMaestro cluster := &api.HCPOpenShiftCluster{ TrackedResource: arm.TrackedResource{Resource: arm.Resource{ID: clusterRID}}, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ - ClusterServiceID: api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/csid")), + ClusterServiceID: ptr.To(api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/csid"))), }, } _, err := mockDB.HCPClusters(clusterRID.SubscriptionID, clusterRID.ResourceGroupName).Create(ctx, cluster, nil) require.NoError(t, err) - mockCS.EXPECT().GetClusterProvisionShard(gomock.Any(), cluster.ServiceProviderProperties.ClusterServiceID).Return(shard, nil).AnyTimes() + mockCS.EXPECT().GetClusterProvisionShard(gomock.Any(), *cluster.ServiceProviderProperties.ClusterServiceID).Return(shard, nil).AnyTimes() spc := &api.ServiceProviderCluster{ CosmosMetadata: arm.CosmosMetadata{ResourceID: spcResourceID}, ResourceID: *spcResourceID, @@ -1531,12 +1532,12 @@ func TestDeleteOrphanedMaestroReadonlyBundles_ensureOrphanedNodePoolScopedMaestr cluster := &api.HCPOpenShiftCluster{ TrackedResource: arm.TrackedResource{Resource: arm.Resource{ID: clusterRID}}, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ - ClusterServiceID: api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/csid")), + ClusterServiceID: ptr.To(api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/csid"))), }, } _, err := mockDB.HCPClusters(clusterRID.SubscriptionID, clusterRID.ResourceGroupName).Create(ctx, cluster, nil) require.NoError(t, err) - mockCS.EXPECT().GetClusterProvisionShard(gomock.Any(), cluster.ServiceProviderProperties.ClusterServiceID).Return(shard, nil).AnyTimes() + mockCS.EXPECT().GetClusterProvisionShard(gomock.Any(), *cluster.ServiceProviderProperties.ClusterServiceID).Return(shard, nil).AnyTimes() spnp := &api.ServiceProviderNodePool{ CosmosMetadata: arm.CosmosMetadata{ResourceID: spnpResourceID}, ResourceID: *spnpResourceID, @@ -1572,12 +1573,12 @@ func TestDeleteOrphanedMaestroReadonlyBundles_ensureOrphanedNodePoolScopedMaestr cluster := &api.HCPOpenShiftCluster{ TrackedResource: arm.TrackedResource{Resource: arm.Resource{ID: clusterRID}}, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ - ClusterServiceID: api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/csid")), + ClusterServiceID: ptr.To(api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/csid"))), }, } _, err := mockDB.HCPClusters(clusterRID.SubscriptionID, clusterRID.ResourceGroupName).Create(ctx, cluster, nil) require.NoError(t, err) - mockCS.EXPECT().GetClusterProvisionShard(gomock.Any(), cluster.ServiceProviderProperties.ClusterServiceID).Return(shard, nil).AnyTimes() + mockCS.EXPECT().GetClusterProvisionShard(gomock.Any(), *cluster.ServiceProviderProperties.ClusterServiceID).Return(shard, nil).AnyTimes() spnp := &api.ServiceProviderNodePool{ CosmosMetadata: arm.CosmosMetadata{ResourceID: spnpResourceID}, ResourceID: *spnpResourceID, @@ -1661,12 +1662,12 @@ func TestDeleteOrphanedMaestroReadonlyBundles_ensureClusterScopedOrphanedMaestro cluster := &api.HCPOpenShiftCluster{ TrackedResource: arm.TrackedResource{Resource: arm.Resource{ID: clusterRID}}, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ - ClusterServiceID: api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/csid")), + ClusterServiceID: ptr.To(api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/csid"))), }, } _, err = mockDB.HCPClusters(clusterRID.SubscriptionID, clusterRID.ResourceGroupName).Create(ctx, cluster, nil) require.NoError(t, err) - mockCS.EXPECT().GetClusterProvisionShard(gomock.Any(), cluster.ServiceProviderProperties.ClusterServiceID).Return(shard1, nil).AnyTimes() + mockCS.EXPECT().GetClusterProvisionShard(gomock.Any(), *cluster.ServiceProviderProperties.ClusterServiceID).Return(shard1, nil).AnyTimes() spcOnShard1 := &api.ServiceProviderCluster{ CosmosMetadata: arm.CosmosMetadata{ResourceID: spcOnShard1ResourceID}, @@ -1744,12 +1745,12 @@ func TestDeleteOrphanedMaestroReadonlyBundles_ensureClusterScopedOrphanedMaestro cluster := &api.HCPOpenShiftCluster{ TrackedResource: arm.TrackedResource{Resource: arm.Resource{ID: clusterRID}}, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ - ClusterServiceID: api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/csid")), + ClusterServiceID: ptr.To(api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/csid"))), }, } _, err = mockDB.HCPClusters(clusterRID.SubscriptionID, clusterRID.ResourceGroupName).Create(ctx, cluster, nil) require.NoError(t, err) - mockCS.EXPECT().GetClusterProvisionShard(gomock.Any(), cluster.ServiceProviderProperties.ClusterServiceID).Return(shardA, nil).AnyTimes() + mockCS.EXPECT().GetClusterProvisionShard(gomock.Any(), *cluster.ServiceProviderProperties.ClusterServiceID).Return(shardA, nil).AnyTimes() spc := &api.ServiceProviderCluster{ CosmosMetadata: arm.CosmosMetadata{ResourceID: spcResourceID}, @@ -1802,12 +1803,12 @@ func TestDeleteOrphanedMaestroReadonlyBundles_ensureClusterScopedOrphanedMaestro cluster := &api.HCPOpenShiftCluster{ TrackedResource: arm.TrackedResource{Resource: arm.Resource{ID: clusterRID}}, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ - ClusterServiceID: api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/csid")), + ClusterServiceID: ptr.To(api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/csid"))), }, } _, err := mockDB.HCPClusters(clusterRID.SubscriptionID, clusterRID.ResourceGroupName).Create(ctx, cluster, nil) require.NoError(t, err) - mockCS.EXPECT().GetClusterProvisionShard(gomock.Any(), cluster.ServiceProviderProperties.ClusterServiceID).Return(shard, nil).AnyTimes() + mockCS.EXPECT().GetClusterProvisionShard(gomock.Any(), *cluster.ServiceProviderProperties.ClusterServiceID).Return(shard, nil).AnyTimes() spc := &api.ServiceProviderCluster{ CosmosMetadata: arm.CosmosMetadata{ResourceID: spcResourceID}, @@ -1860,7 +1861,7 @@ func TestDeleteOrphanedMaestroReadonlyBundles_SyncOnce_FullFlow_DeletesOrphanedB cluster := &api.HCPOpenShiftCluster{ TrackedResource: arm.TrackedResource{Resource: arm.Resource{ID: clusterResourceID}}, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ - ClusterServiceID: api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/csid")), + ClusterServiceID: api.Ptr(api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/csid"))), }, } clustersCRUD := mockDB.HCPClusters(clusterResourceID.SubscriptionID, clusterResourceID.ResourceGroupName) @@ -1883,7 +1884,7 @@ func TestDeleteOrphanedMaestroReadonlyBundles_SyncOnce_FullFlow_DeletesOrphanedB provisionShard := buildTestProvisionShard("test-consumer") mockCS.EXPECT().ListProvisionShards().Return(ocm.NewSimpleProvisionShardListIterator([]*arohcpv1alpha1.ProvisionShard{provisionShard}, nil)) - mockCS.EXPECT().GetClusterProvisionShard(gomock.Any(), cluster.ServiceProviderProperties.ClusterServiceID).Return(provisionShard, nil).AnyTimes() + mockCS.EXPECT().GetClusterProvisionShard(gomock.Any(), *cluster.ServiceProviderProperties.ClusterServiceID).Return(provisionShard, nil).AnyTimes() restEndpoint := provisionShard.MaestroConfig().RestApiConfig().Url() grpcEndpoint := provisionShard.MaestroConfig().GrpcApiConfig().Url() consumerName := provisionShard.MaestroConfig().ConsumerName() diff --git a/backend/pkg/controllers/mismatchcontrollers/cluster_service_cluster_matching.go b/backend/pkg/controllers/mismatchcontrollers/cluster_service_cluster_matching.go index b3950811a2c..0ee5c5c33b6 100644 --- a/backend/pkg/controllers/mismatchcontrollers/cluster_service_cluster_matching.go +++ b/backend/pkg/controllers/mismatchcontrollers/cluster_service_cluster_matching.go @@ -80,6 +80,10 @@ func (c *clusterServiceClusterMatching) getAllCosmosObjs(ctx context.Context) (m for _, cluster := range allHCPClusters.Items(ctx) { ret = append(ret, cluster) + // we skip items without a clusterServiceID because they make be about to get them and shouldn't be deleted. + if cluster.ServiceProviderProperties.ClusterServiceID == nil { + continue + } existingCluster, exists := clusterServiceIDToCluster[cluster.ServiceProviderProperties.ClusterServiceID.String()] if exists { return nil, nil, utils.TrackError(fmt.Errorf("duplicate obj found: %s, owned by %q and %q", cluster.ID.String(), existingCluster.ID.String(), cluster.ID.String())) diff --git a/backend/pkg/controllers/mismatchcontrollers/cosmos_cluster_matching.go b/backend/pkg/controllers/mismatchcontrollers/cosmos_cluster_matching.go index a9450d48ed7..eec4fc0bd18 100644 --- a/backend/pkg/controllers/mismatchcontrollers/cosmos_cluster_matching.go +++ b/backend/pkg/controllers/mismatchcontrollers/cosmos_cluster_matching.go @@ -59,8 +59,12 @@ func (c *cosmosClusterMatching) synchronizeClusters(ctx context.Context, keyObj if err != nil { return utils.TrackError(err) } + if cosmosCluster.ServiceProviderProperties.ClusterServiceID == nil { + // no work to do because clusters start without clusterServiceIDs and that means we haven't got an orphan + return nil + } - _, err = c.clusterServiceClient.GetCluster(ctx, cosmosCluster.ServiceProviderProperties.ClusterServiceID) + _, err = c.clusterServiceClient.GetCluster(ctx, *cosmosCluster.ServiceProviderProperties.ClusterServiceID) var ocmGetClusterError *ocmerrors.Error isClusterServiceObjNotFound := errors.As(err, &ocmGetClusterError) && ocmGetClusterError.Status() == http.StatusNotFound if err != nil && !isClusterServiceObjNotFound { diff --git a/backend/pkg/controllers/mismatchcontrollers/cosmos_externalauth_matching.go b/backend/pkg/controllers/mismatchcontrollers/cosmos_externalauth_matching.go index 3a052297d6f..392e639b39b 100644 --- a/backend/pkg/controllers/mismatchcontrollers/cosmos_externalauth_matching.go +++ b/backend/pkg/controllers/mismatchcontrollers/cosmos_externalauth_matching.go @@ -100,13 +100,17 @@ func (c *cosmosExternalAuthMatching) synchronizeAllExternalAuths(ctx context.Con if err != nil { return utils.TrackError(err) } + if cluster.ServiceProviderProperties.ClusterServiceID == nil { + // no work to do because clusters start without clusterServiceIDs and that means we haven't got any child resources, so they haven't got an orphan. + return nil + } clusterServiceIDToCosmosExternalAuths, allCosmosExternalAuths, err := c.getAllCosmosObjs(ctx, keyObj) if err != nil { return utils.TrackError(err) } - clusterServiceIDToClusterServiceExternalAuths, allClusterServiceExternalAuths, err := c.getAllClusterServiceObjs(ctx, cluster.ServiceProviderProperties.ClusterServiceID) + clusterServiceIDToClusterServiceExternalAuths, allClusterServiceExternalAuths, err := c.getAllClusterServiceObjs(ctx, *cluster.ServiceProviderProperties.ClusterServiceID) if err != nil { return utils.TrackError(err) } diff --git a/backend/pkg/controllers/mismatchcontrollers/cosmos_nodepool_matching.go b/backend/pkg/controllers/mismatchcontrollers/cosmos_nodepool_matching.go index bef887ad33c..f6291e6d168 100644 --- a/backend/pkg/controllers/mismatchcontrollers/cosmos_nodepool_matching.go +++ b/backend/pkg/controllers/mismatchcontrollers/cosmos_nodepool_matching.go @@ -100,13 +100,17 @@ func (c *cosmosNodePoolMatching) synchronizeAllNodes(ctx context.Context, keyObj if err != nil { return utils.TrackError(err) } + if cluster.ServiceProviderProperties.ClusterServiceID == nil { + // no work to do because clusters start without clusterServiceIDs and that means we haven't got any child resources, so they haven't got an orphan. + return nil + } clusterServiceIDToCosmosNodePools, allCosmosNodePools, err := c.getAllCosmosObjs(ctx, keyObj) if err != nil { return utils.TrackError(err) } - clusterServiceIDToClusterServiceNodePools, allClusterServiceNodePools, err := c.getAllClusterServiceObjs(ctx, cluster.ServiceProviderProperties.ClusterServiceID) + clusterServiceIDToClusterServiceNodePools, allClusterServiceNodePools, err := c.getAllClusterServiceObjs(ctx, *cluster.ServiceProviderProperties.ClusterServiceID) if err != nil { return utils.TrackError(err) } diff --git a/backend/pkg/controllers/nodepoolpropertiescontroller/node_pool_properties_sync_test.go b/backend/pkg/controllers/nodepoolpropertiescontroller/node_pool_properties_sync_test.go index 1691e70aacc..0e7f7b62e4d 100644 --- a/backend/pkg/controllers/nodepoolpropertiescontroller/node_pool_properties_sync_test.go +++ b/backend/pkg/controllers/nodepoolpropertiescontroller/node_pool_properties_sync_test.go @@ -278,7 +278,7 @@ func newTestCluster(t *testing.T) *api.HCPOpenShiftCluster { Location: "eastus", }, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ - ClusterServiceID: clusterInternalID, + ClusterServiceID: &clusterInternalID, }, } } diff --git a/backend/pkg/controllers/operationcontrollers/operation_cluster_create.go b/backend/pkg/controllers/operationcontrollers/operation_cluster_create.go index 6d1046527c4..942871310bb 100644 --- a/backend/pkg/controllers/operationcontrollers/operation_cluster_create.go +++ b/backend/pkg/controllers/operationcontrollers/operation_cluster_create.go @@ -76,6 +76,12 @@ func (c *operationClusterCreate) SynchronizeOperation(ctx context.Context, key c return nil // no work to do } + if len(operation.InternalID.String()) == 0 { + // we cannot proceed: yet. + // TODO when we update to make clusterserice creation async, we need https://github.com/Azure/ARO-HCP/pull/4695 or similar + // and we need to wire up a fail-safe where if we have no ID and we time out, we report the best failure we can. + return nil + } clusterStatus, err := c.clusterServiceClient.GetClusterStatus(ctx, operation.InternalID) if err != nil { return utils.TrackError(err) diff --git a/backend/pkg/controllers/operationcontrollers/operation_cluster_delete.go b/backend/pkg/controllers/operationcontrollers/operation_cluster_delete.go index cd910da7b50..1d212fd657e 100644 --- a/backend/pkg/controllers/operationcontrollers/operation_cluster_delete.go +++ b/backend/pkg/controllers/operationcontrollers/operation_cluster_delete.go @@ -83,6 +83,11 @@ func (c *operationClusterDelete) SynchronizeOperation(ctx context.Context, key c return nil // no work to do } + if len(operation.InternalID.String()) == 0 { + // we cannot proceed: yet. + // TODO when we update to make clusterserice creation async, we need to handle this correctly. + return nil + } clusterStatus, err := c.clusterServiceClient.GetClusterStatus(ctx, operation.InternalID) var ocmGetClusterError *ocmerrors.Error if err != nil && errors.As(err, &ocmGetClusterError) && ocmGetClusterError.Status() == http.StatusNotFound { diff --git a/backend/pkg/controllers/operationcontrollers/operation_cluster_update.go b/backend/pkg/controllers/operationcontrollers/operation_cluster_update.go index 4eba3d1daaf..c59d4177667 100644 --- a/backend/pkg/controllers/operationcontrollers/operation_cluster_update.go +++ b/backend/pkg/controllers/operationcontrollers/operation_cluster_update.go @@ -75,6 +75,11 @@ func (c *operationClusterUpdate) SynchronizeOperation(ctx context.Context, key c if !c.ShouldProcess(ctx, operation) { return nil // no work to do } + if len(operation.InternalID.String()) == 0 { + // we cannot proceed: yet. + // TODO when we update to make clusterserice creation async, we need to handle this correctly. + return nil + } clusterStatus, err := c.clusterServiceClient.GetClusterStatus(ctx, operation.InternalID) if err != nil { diff --git a/backend/pkg/controllers/operationcontrollers/test_helpers_test.go b/backend/pkg/controllers/operationcontrollers/test_helpers_test.go index 788a4a8be62..f95fae5966d 100644 --- a/backend/pkg/controllers/operationcontrollers/test_helpers_test.go +++ b/backend/pkg/controllers/operationcontrollers/test_helpers_test.go @@ -92,7 +92,7 @@ func (f *clusterTestFixture) newCluster(createdAt *time.Time) *api.HCPOpenShiftC }, }, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ - ClusterServiceID: f.clusterInternalID, + ClusterServiceID: &f.clusterInternalID, ActiveOperationID: testOperationName, ClusterUID: testClusterUID, }, @@ -168,7 +168,7 @@ func (f *nodePoolTestFixture) newCluster() *api.HCPOpenShiftCluster { }, }, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ - ClusterServiceID: f.clusterInternalID, + ClusterServiceID: &f.clusterInternalID, }, } } @@ -261,7 +261,7 @@ func (f *externalAuthTestFixture) newCluster() *api.HCPOpenShiftCluster { }, }, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ - ClusterServiceID: f.clusterInternalID, + ClusterServiceID: &f.clusterInternalID, }, } } diff --git a/backend/pkg/controllers/read_and_persist_cluster_scoped_maestro_readonly_bundles_content_controller.go b/backend/pkg/controllers/read_and_persist_cluster_scoped_maestro_readonly_bundles_content_controller.go index c8fc6e3ad44..1e7cffee8e3 100644 --- a/backend/pkg/controllers/read_and_persist_cluster_scoped_maestro_readonly_bundles_content_controller.go +++ b/backend/pkg/controllers/read_and_persist_cluster_scoped_maestro_readonly_bundles_content_controller.go @@ -89,6 +89,11 @@ func (c *readAndPersistClusterScopedMaestroReadonlyBundlesContentSyncer) SyncOnc if err != nil { return utils.TrackError(fmt.Errorf("failed to get Cluster: %w", err)) } + if existingCluster.ServiceProviderProperties.ClusterServiceID == nil { + // we don't have enough information to proceed. We will retrigger once the information is present. + // TODO remove this once we have the information all in cosmos. + return nil + } existingServiceProviderCluster, err := database.GetOrCreateServiceProviderCluster(ctx, c.cosmosClient, key.GetResourceID()) if err != nil { @@ -105,7 +110,7 @@ func (c *readAndPersistClusterScopedMaestroReadonlyBundlesContentSyncer) SyncOnc // we are guaranteed to have a shard allocated for the cluster. If this changes in the future // we would need to change the logic in controllers to check that the retrieved cluster has a // shard allocated. - clusterProvisionShard, err := c.clusterServiceClient.GetClusterProvisionShard(ctx, existingCluster.ServiceProviderProperties.ClusterServiceID) + clusterProvisionShard, err := c.clusterServiceClient.GetClusterProvisionShard(ctx, *existingCluster.ServiceProviderProperties.ClusterServiceID) if err != nil { return utils.TrackError(fmt.Errorf("failed to get Cluster Provision Shard from Cluster Service: %w", err)) } diff --git a/backend/pkg/controllers/read_and_persist_cluster_scoped_maestro_readonly_bundles_content_controller_test.go b/backend/pkg/controllers/read_and_persist_cluster_scoped_maestro_readonly_bundles_content_controller_test.go index 42962c9cf55..77d79039a02 100644 --- a/backend/pkg/controllers/read_and_persist_cluster_scoped_maestro_readonly_bundles_content_controller_test.go +++ b/backend/pkg/controllers/read_and_persist_cluster_scoped_maestro_readonly_bundles_content_controller_test.go @@ -68,7 +68,7 @@ func TestReadAndPersistClusterScopedMaestroReadonlyBundlesContentSyncer_SyncOnce cluster := &api.HCPOpenShiftCluster{ TrackedResource: arm.TrackedResource{Resource: arm.Resource{ID: clusterResourceID}}, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ - ClusterServiceID: api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/11111111111111111111111111111111")), + ClusterServiceID: api.Ptr(api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/11111111111111111111111111111111"))), }, } @@ -114,7 +114,7 @@ func TestReadAndPersistClusterScopedMaestroReadonlyBundlesContentSyncer_SyncOnce cluster := &api.HCPOpenShiftCluster{ TrackedResource: arm.TrackedResource{Resource: arm.Resource{ID: clusterResourceID}}, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ - ClusterServiceID: api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/11111111111111111111111111111111")), + ClusterServiceID: api.Ptr(api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/11111111111111111111111111111111"))), }, } clustersCRUD := mockDBClient.HCPClusters(key.SubscriptionID, key.ResourceGroupName) @@ -157,7 +157,7 @@ func TestReadAndPersistClusterScopedMaestroReadonlyBundlesContentSyncer_SyncOnce cluster := &api.HCPOpenShiftCluster{ TrackedResource: arm.TrackedResource{Resource: arm.Resource{ID: clusterResourceID}}, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ - ClusterServiceID: api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/11111111111111111111111111111111")), + ClusterServiceID: api.Ptr(api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/11111111111111111111111111111111"))), }, } clustersCRUD := mockDBClient.HCPClusters(key.SubscriptionID, key.ResourceGroupName) @@ -179,7 +179,7 @@ func TestReadAndPersistClusterScopedMaestroReadonlyBundlesContentSyncer_SyncOnce require.NoError(t, err) mockClusterService.EXPECT(). - GetClusterProvisionShard(gomock.Any(), cluster.ServiceProviderProperties.ClusterServiceID). + GetClusterProvisionShard(gomock.Any(), *cluster.ServiceProviderProperties.ClusterServiceID). Return(nil, fmt.Errorf("provision shard error")) err = syncer.SyncOnce(ctx, key) @@ -214,7 +214,7 @@ func TestReadAndPersistClusterScopedMaestroReadonlyBundlesContentSyncer_SyncOnce cluster := &api.HCPOpenShiftCluster{ TrackedResource: arm.TrackedResource{Resource: arm.Resource{ID: clusterResourceID}}, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ - ClusterServiceID: api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/11111111111111111111111111111111")), + ClusterServiceID: api.Ptr(api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/11111111111111111111111111111111"))), }, } clustersCRUD := mockDBClient.HCPClusters(key.SubscriptionID, key.ResourceGroupName) @@ -237,7 +237,7 @@ func TestReadAndPersistClusterScopedMaestroReadonlyBundlesContentSyncer_SyncOnce provisionShard := buildTestProvisionShard("test-consumer") mockClusterService.EXPECT(). - GetClusterProvisionShard(gomock.Any(), cluster.ServiceProviderProperties.ClusterServiceID). + GetClusterProvisionShard(gomock.Any(), *cluster.ServiceProviderProperties.ClusterServiceID). Return(provisionShard, nil) restEndpoint := provisionShard.MaestroConfig().RestApiConfig().Url() diff --git a/backend/pkg/controllers/upgradecontrollers/control_plane_active_version_controller_test.go b/backend/pkg/controllers/upgradecontrollers/control_plane_active_version_controller_test.go index 3f247b4d6d3..056f3ca5e4d 100644 --- a/backend/pkg/controllers/upgradecontrollers/control_plane_active_version_controller_test.go +++ b/backend/pkg/controllers/upgradecontrollers/control_plane_active_version_controller_test.go @@ -260,7 +260,7 @@ func createTestHCPCluster(t *testing.T, ctx context.Context, mockDB *databasetes }, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ ProvisioningState: arm.ProvisioningStateSucceeded, - ClusterServiceID: clusterInternalID, + ClusterServiceID: &clusterInternalID, }, } _, err = mockDB.HCPClusters(testSubscriptionID, testResourceGroupName).Create(ctx, cluster, nil) diff --git a/backend/pkg/controllers/upgradecontrollers/control_plane_desired_version_controller.go b/backend/pkg/controllers/upgradecontrollers/control_plane_desired_version_controller.go index 08c7b6963a6..d4772f005dd 100644 --- a/backend/pkg/controllers/upgradecontrollers/control_plane_desired_version_controller.go +++ b/backend/pkg/controllers/upgradecontrollers/control_plane_desired_version_controller.go @@ -112,6 +112,11 @@ func (c *controlPlaneDesiredVersionSyncer) SyncOnce(ctx context.Context, key con if err != nil { return utils.TrackError(fmt.Errorf("failed to get Cluster: %w", err)) } + if existingCluster.ServiceProviderProperties.ClusterServiceID == nil { + // Currently, this is correct. We will likely refactor and change this to separate the read of active versions from the determination + // of the next desired version: we'll need to choose a desired version even if there are no active versions. + return nil + } existingServiceProviderCluster, err := database.GetOrCreateServiceProviderCluster(ctx, c.cosmosClient, key.GetResourceID()) if err != nil { @@ -119,7 +124,7 @@ func (c *controlPlaneDesiredVersionSyncer) SyncOnce(ctx context.Context, key con } // TODO bring the cluster uuid into serviceprovidercluster - clusterServiceCluster, err := c.clusterServiceClient.GetCluster(ctx, existingCluster.ServiceProviderProperties.ClusterServiceID) + clusterServiceCluster, err := c.clusterServiceClient.GetCluster(ctx, *existingCluster.ServiceProviderProperties.ClusterServiceID) if err != nil { return utils.TrackError(fmt.Errorf("failed to get cluster from Cluster Service: %w", err)) } 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 3b80bccd2a5..09a340e01a0 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 @@ -517,7 +517,7 @@ func testCosmosClusterWithWorkersNodePoolAtVersion(nodePoolVersionId string) []a }, }, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ - ClusterServiceID: api.Must(api.NewInternalID("/api/clusters_mgmt/v1/clusters/test-cluster")), + ClusterServiceID: api.Ptr(api.Must(api.NewInternalID("/api/clusters_mgmt/v1/clusters/test-cluster"))), }, } nodePoolResourceId := api.Must(azcorearm.ParseResourceID(clusterResourceId.String() + "/nodePools/workers")) diff --git a/backend/pkg/controllers/upgradecontrollers/nodepool_version_controller.go b/backend/pkg/controllers/upgradecontrollers/nodepool_version_controller.go index 5119608cb8e..a4a4f2b9214 100644 --- a/backend/pkg/controllers/upgradecontrollers/nodepool_version_controller.go +++ b/backend/pkg/controllers/upgradecontrollers/nodepool_version_controller.go @@ -138,9 +138,14 @@ func (c *nodePoolVersionSyncer) SyncOnce(ctx context.Context, key controllerutil if err != nil { return utils.TrackError(fmt.Errorf("failed to get cluster from cosmos: %w", err)) } + if cluster.ServiceProviderProperties.ClusterServiceID == nil { + // TODO this appears to only be used to look up a clusterservice cluster to get a UUID. Once the billing changes merge, + // we'll have UID to key by and won't need this. + return nil + } // Get the cluster from Cluster Service to obtain the cluster UUID for Cincinnati - csCluster, err := c.clusterServiceClient.GetCluster(ctx, cluster.ServiceProviderProperties.ClusterServiceID) + csCluster, err := c.clusterServiceClient.GetCluster(ctx, *cluster.ServiceProviderProperties.ClusterServiceID) if err != nil { return utils.TrackError(fmt.Errorf("failed to get cluster from Cluster Service: %w", err)) } diff --git a/backend/pkg/controllers/upgradecontrollers/nodepool_version_controller_test.go b/backend/pkg/controllers/upgradecontrollers/nodepool_version_controller_test.go index 8ced5eabe03..e7c1a690d89 100644 --- a/backend/pkg/controllers/upgradecontrollers/nodepool_version_controller_test.go +++ b/backend/pkg/controllers/upgradecontrollers/nodepool_version_controller_test.go @@ -105,7 +105,7 @@ func createTestNodePoolWithVersion(t *testing.T, ctx context.Context, mockDB *da }, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ ProvisioningState: arm.ProvisioningStateSucceeded, - ClusterServiceID: clusterInternalID, + ClusterServiceID: &clusterInternalID, }, } _, err = mockDB.HCPClusters(testSubscriptionID, testResourceGroupName).Create(ctx, cluster, nil) diff --git a/backend/pkg/controllers/upgradecontrollers/trigger_control_plane_upgrade_controller.go b/backend/pkg/controllers/upgradecontrollers/trigger_control_plane_upgrade_controller.go index 1c6038752bd..a52b9aca8fa 100644 --- a/backend/pkg/controllers/upgradecontrollers/trigger_control_plane_upgrade_controller.go +++ b/backend/pkg/controllers/upgradecontrollers/trigger_control_plane_upgrade_controller.go @@ -91,6 +91,10 @@ func (c *triggerControlPlaneUpgradeSyncer) SyncOnce(ctx context.Context, key con if err != nil { return utils.TrackError(fmt.Errorf("failed to get Cluster: %w", err)) } + if existingCluster.ServiceProviderProperties.ClusterServiceID == nil { + // if we have no clusterService cluster, we have nothing to trigger. + return nil + } existingServiceProviderCluster, err := database.GetOrCreateServiceProviderCluster(ctx, c.cosmosClient, key.GetResourceID()) if err != nil { @@ -113,7 +117,7 @@ func (c *triggerControlPlaneUpgradeSyncer) SyncOnce(ctx context.Context, key con return nil } - return c.createUpgradePolicyIfNeeded(ctx, desiredVersion, existingCluster.ServiceProviderProperties.ClusterServiceID) + return c.createUpgradePolicyIfNeeded(ctx, desiredVersion, *existingCluster.ServiceProviderProperties.ClusterServiceID) } // createUpgradePolicyIfNeeded ensures a control plane upgrade policy exists for the desired version. diff --git a/backend/pkg/informers/informers_test.go b/backend/pkg/informers/informers_test.go index 935bfe0e3b9..c0b26aabdf8 100644 --- a/backend/pkg/informers/informers_test.go +++ b/backend/pkg/informers/informers_test.go @@ -353,7 +353,7 @@ func clusterInformerTestCase() informerTestCase { }, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ ProvisioningState: state, - ClusterServiceID: internalID, + ClusterServiceID: &internalID, }, } } @@ -488,7 +488,7 @@ func nodePoolInformerTestCase() informerTestCase { }, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ ProvisioningState: arm.ProvisioningStateSucceeded, - ClusterServiceID: internalID, + ClusterServiceID: &internalID, }, } _, err = mockDB.HCPClusters(subscriptionID, resourceGroupName).Create(ctx, cluster, nil) @@ -723,7 +723,7 @@ func controllerInformerTestCase() informerTestCase { }, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ ProvisioningState: arm.ProvisioningStateSucceeded, - ClusterServiceID: internalID, + ClusterServiceID: &internalID, }, } _, err = mockDB.HCPClusters(subscriptionID, resourceGroupName).Create(ctx, cluster, nil) diff --git a/backend/pkg/listertesting/slice_listers_test.go b/backend/pkg/listertesting/slice_listers_test.go index 4052a273a9b..fab5b5c5ab6 100644 --- a/backend/pkg/listertesting/slice_listers_test.go +++ b/backend/pkg/listertesting/slice_listers_test.go @@ -332,7 +332,7 @@ func newTestCluster(subscriptionID, resourceGroupName, clusterName string) *api. }, }, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ - ClusterServiceID: api.Must(api.NewInternalID("/api/clusters_mgmt/v1/clusters/" + clusterName)), + ClusterServiceID: api.Ptr(api.Must(api.NewInternalID("/api/clusters_mgmt/v1/clusters/" + clusterName))), }, } } diff --git a/frontend/pkg/frontend/cluster.go b/frontend/pkg/frontend/cluster.go index 042f59d3d87..f44f01baa5f 100644 --- a/frontend/pkg/frontend/cluster.go +++ b/frontend/pkg/frontend/cluster.go @@ -101,6 +101,11 @@ func (f *Frontend) ArmResourceListClusters(writer http.ResponseWriter, request * } clustersByClusterServiceID := make(map[string]*api.HCPOpenShiftCluster) for _, internalCluster := range internalClusterIterator.Items(ctx) { + if internalCluster.ServiceProviderProperties.ClusterServiceID == nil { + // TODO this will be removed during our switch to read only from cosmos. + // we can still merge now since the value will never be nil until both the read path is fixed and this PR makes it to prod. + continue + } clustersByClusterServiceID[internalCluster.ServiceProviderProperties.ClusterServiceID.ID()] = internalCluster } err = internalClusterIterator.GetError() @@ -368,10 +373,11 @@ func (f *Frontend) createHCPCluster(writer http.ResponseWriter, request *http.Re return utils.TrackError(err) } - newInternalCluster.ServiceProviderProperties.ClusterServiceID, err = api.NewInternalID(resultingClusterServiceCluster.HREF()) + csID, err := api.NewInternalID(resultingClusterServiceCluster.HREF()) if err != nil { return utils.TrackError(err) } + newInternalCluster.ServiceProviderProperties.ClusterServiceID = &csID transaction := f.dbClient.NewTransaction(newInternalCluster.ID.SubscriptionID) @@ -379,7 +385,7 @@ func (f *Frontend) createHCPCluster(writer http.ResponseWriter, request *http.Re clusterCreateOperation := database.NewOperation( database.OperationRequestCreate, newInternalCluster.ID, - newInternalCluster.ServiceProviderProperties.ClusterServiceID, + ptr.Deref(newInternalCluster.ServiceProviderProperties.ClusterServiceID, api.InternalID{}), f.azureLocation, request.Header.Get(arm.HeaderNameHomeTenantID), request.Header.Get(arm.HeaderNameClientObjectID), @@ -635,39 +641,41 @@ func (f *Frontend) updateHCPClusterInCosmos(ctx context.Context, writer http.Res tenantID = *subscription.Properties.TenantId } - oldClusterServiceCluster, err := f.clusterServiceClient.GetCluster(ctx, oldInternalCluster.ServiceProviderProperties.ClusterServiceID) - if err != nil { - return utils.TrackError(err) - } - newClusterServiceClusterBuilder, newClusterServiceAutoscalerBuilder, err := ocm.BuildCSCluster(oldInternalCluster.ID, tenantID, newInternalCluster, nil, oldClusterServiceCluster) - if err != nil { - return utils.TrackError(err) - } - - logger.Info(fmt.Sprintf("updating resource %s", oldInternalCluster.ID)) - resultingClusterServiceAutoscaler, err := f.clusterServiceClient.UpdateClusterAutoscaler(ctx, oldInternalCluster.ServiceProviderProperties.ClusterServiceID, newClusterServiceAutoscalerBuilder) - if err != nil { - return utils.TrackError(err) - } - resultingClusterServiceCluster, err := f.clusterServiceClient.UpdateCluster(ctx, oldInternalCluster.ServiceProviderProperties.ClusterServiceID, newClusterServiceClusterBuilder) - if err != nil { - return utils.TrackError(err) - } + var resultingClusterServiceCluster *arohcpv1alpha1.Cluster + if oldInternalCluster.ServiceProviderProperties.ClusterServiceID != nil { + oldClusterServiceCluster, err := f.clusterServiceClient.GetCluster(ctx, *oldInternalCluster.ServiceProviderProperties.ClusterServiceID) + if err != nil { + return utils.TrackError(err) + } + newClusterServiceClusterBuilder, newClusterServiceAutoscalerBuilder, err := ocm.BuildCSCluster(oldInternalCluster.ID, tenantID, newInternalCluster, nil, oldClusterServiceCluster) + if err != nil { + return utils.TrackError(err) + } - // Merge the autoscaler model into the cluster model. - resultingClusterServiceCluster, err = arohcpv1alpha1.NewCluster(). - Copy(resultingClusterServiceCluster). - Autoscaler(arohcpv1alpha1.NewClusterAutoscaler().Copy(resultingClusterServiceAutoscaler)). - Build() - if err != nil { - return utils.TrackError(err) + logger.Info(fmt.Sprintf("updating resource %s", oldInternalCluster.ID)) + resultingClusterServiceAutoscaler, err := f.clusterServiceClient.UpdateClusterAutoscaler(ctx, *oldInternalCluster.ServiceProviderProperties.ClusterServiceID, newClusterServiceAutoscalerBuilder) + if err != nil { + return utils.TrackError(err) + } + resultingClusterServiceCluster, err = f.clusterServiceClient.UpdateCluster(ctx, *oldInternalCluster.ServiceProviderProperties.ClusterServiceID, newClusterServiceClusterBuilder) + if err != nil { + return utils.TrackError(err) + } + // Merge the autoscaler model into the cluster model. + resultingClusterServiceCluster, err = arohcpv1alpha1.NewCluster(). + Copy(resultingClusterServiceCluster). + Autoscaler(arohcpv1alpha1.NewClusterAutoscaler().Copy(resultingClusterServiceAutoscaler)). + Build() + if err != nil { + return utils.TrackError(err) + } } transaction := f.dbClient.NewTransaction(oldInternalCluster.ID.SubscriptionID) clusterUpdateOperation := database.NewOperation( database.OperationRequestUpdate, oldInternalCluster.ID, - oldInternalCluster.ServiceProviderProperties.ClusterServiceID, + ptr.Deref(oldInternalCluster.ServiceProviderProperties.ClusterServiceID, api.InternalID{}), f.azureLocation, request.Header.Get(arm.HeaderNameHomeTenantID), request.Header.Get(arm.HeaderNameClientObjectID), @@ -775,18 +783,20 @@ func (f *Frontend) addDeleteClusterToTransaction(ctx context.Context, writer htt return utils.TrackError(err) } - err = f.clusterServiceClient.DeleteCluster(ctx, cluster.ServiceProviderProperties.ClusterServiceID) - var ocmError *ocmerrors.Error - if errors.As(err, &ocmError) && ocmError.Status() == http.StatusNotFound { - // StatusNotFound means we have stale data in Cosmos DB. - // This can happen in test environments if a user bypasses - // the RP to delete a resource (e.g. "ocm delete"). It can - // also happen if an asynchronous deletion operation fails. - // we will fall through and cancel all operations and go through as normal a deletion flow as we can to avoid - // leaking data related to the resource, like controller status. - logger.Info("clusterService cluster missing, trying to clean up", "err", err) - } else if err != nil { - return utils.TrackError(err) + if cluster.ServiceProviderProperties.ClusterServiceID != nil { + err = f.clusterServiceClient.DeleteCluster(ctx, *cluster.ServiceProviderProperties.ClusterServiceID) + var ocmError *ocmerrors.Error + if errors.As(err, &ocmError) && ocmError.Status() == http.StatusNotFound { + // StatusNotFound means we have stale data in Cosmos DB. + // This can happen in test environments if a user bypasses + // the RP to delete a resource (e.g. "ocm delete"). It can + // also happen if an asynchronous deletion operation fails. + // we will fall through and cancel all operations and go through as normal a deletion flow as we can to avoid + // leaking data related to the resource, like controller status. + logger.Info("clusterService cluster missing, trying to clean up", "err", err) + } else if err != nil { + return utils.TrackError(err) + } } // Cluster Service will take care of canceling any ongoing operations @@ -804,10 +814,14 @@ func (f *Frontend) addDeleteClusterToTransaction(ctx context.Context, writer htt return utils.TrackError(err) } + clusterServiceID := api.InternalID{} + if cluster.ServiceProviderProperties.ClusterServiceID != nil { + clusterServiceID = *cluster.ServiceProviderProperties.ClusterServiceID + } operationDoc := database.NewOperation( database.OperationRequestDelete, cluster.ID, - cluster.ServiceProviderProperties.ClusterServiceID, + clusterServiceID, f.azureLocation, "", "", @@ -864,6 +878,9 @@ func (f *Frontend) addDeleteClusterToTransaction(ctx context.Context, writer htt // TODO this overwrite will transformed into a "set" function as we transition fields to ownership in cosmos // TODO remove the azure location once we have migrated every record to store the location func mergeToInternalCluster(csCluster *arohcpv1alpha1.Cluster, internalCluster *api.HCPOpenShiftCluster, azureLocation string) (*api.HCPOpenShiftCluster, error) { + if csCluster == nil { + return nil, utils.TrackError(fmt.Errorf("cannot merge nil cluster")) + } if len(internalCluster.CustomerProperties.Version.ChannelGroup) == 0 { // if we hit this branch, then we have old data that exists from before we stored all the content of the requested cluster return legacyMergeToInternalCluster(csCluster, internalCluster, azureLocation) @@ -905,7 +922,10 @@ func legacyMergeToInternalCluster(csCluster *arohcpv1alpha1.Cluster, internalClu // merges the states together, and returns the internal representation. // TODO remove the header it takes and collapse that to some general error handling. func (f *Frontend) readInternalClusterFromClusterService(ctx context.Context, oldInternalCluster *api.HCPOpenShiftCluster) (*api.HCPOpenShiftCluster, error) { - oldClusterServiceCluster, err := f.clusterServiceClient.GetCluster(ctx, oldInternalCluster.ServiceProviderProperties.ClusterServiceID) + if oldInternalCluster.ServiceProviderProperties.ClusterServiceID == nil { + return nil, utils.TrackError(errors.New("clusterServiceID is nil")) + } + oldClusterServiceCluster, err := f.clusterServiceClient.GetCluster(ctx, *oldInternalCluster.ServiceProviderProperties.ClusterServiceID) if err != nil { return nil, utils.TrackError(err) } diff --git a/frontend/pkg/frontend/external_auth.go b/frontend/pkg/frontend/external_auth.go index 54589491acf..81479ba1b39 100644 --- a/frontend/pkg/frontend/external_auth.go +++ b/frontend/pkg/frontend/external_auth.go @@ -86,6 +86,9 @@ func (f *Frontend) ArmResourceListExternalAuths(writer http.ResponseWriter, requ if err != nil { return utils.TrackError(err) } + if internalCluster.ServiceProviderProperties.ClusterServiceID == nil { + return utils.TrackError(fmt.Errorf("cluster %s has no ClusterServiceID", internalCluster.ID)) + } pagedResponse := arm.NewPagedResponse() @@ -117,7 +120,7 @@ func (f *Frontend) ArmResourceListExternalAuths(writer http.ResponseWriter, requ query := fmt.Sprintf("id in (%s)", strings.Join(queryIDs, ", ")) logger.Info(fmt.Sprintf("Searching Cluster Service for %q", query)) - csIterator := f.clusterServiceClient.ListExternalAuths(internalCluster.ServiceProviderProperties.ClusterServiceID, query) + csIterator := f.clusterServiceClient.ListExternalAuths(*internalCluster.ServiceProviderProperties.ClusterServiceID, query) for csExternalAuth := range csIterator.Items(ctx) { if internalExternalAuth, ok := externalAuthsByClusterServiceID[csExternalAuth.ID()]; ok { internalExternalAuth, err = mergeToInternalExternalAuth(csExternalAuth, internalExternalAuth) @@ -285,11 +288,15 @@ func (f *Frontend) createExternalAuth(writer http.ResponseWriter, request *http. if err := checkForProvisioningStateConflict(ctx, f.dbClient, database.OperationRequestCreate, newInternalExternalAuth.ID, newInternalExternalAuth.Properties.ProvisioningState); err != nil { return utils.TrackError(err) } + if cluster.ServiceProviderProperties.ClusterServiceID == nil { + return utils.TrackError(fmt.Errorf("cluster %s has no ClusterServiceID", cluster.ID)) + } + csExternalAuthBuilder, err := ocm.BuildCSExternalAuth(ctx, newInternalExternalAuth, false) if err != nil { return utils.TrackError(err) } - csExternalAuth, err := f.clusterServiceClient.PostExternalAuth(ctx, cluster.ServiceProviderProperties.ClusterServiceID, csExternalAuthBuilder) + csExternalAuth, err := f.clusterServiceClient.PostExternalAuth(ctx, *cluster.ServiceProviderProperties.ClusterServiceID, csExternalAuthBuilder) if err != nil { return utils.TrackError(err) } diff --git a/frontend/pkg/frontend/frontend.go b/frontend/pkg/frontend/frontend.go index e622e5764b8..4cdc3327092 100644 --- a/frontend/pkg/frontend/frontend.go +++ b/frontend/pkg/frontend/frontend.go @@ -48,6 +48,7 @@ import ( "github.com/Azure/ARO-HCP/internal/database" "github.com/Azure/ARO-HCP/internal/ocm" "github.com/Azure/ARO-HCP/internal/utils" + "github.com/Azure/ARO-HCP/internal/utils/armhelpers" "github.com/Azure/ARO-HCP/internal/validation" ) @@ -349,12 +350,14 @@ func (f *Frontend) ArmResourceActionRequestAdminCredential(writer http.ResponseW if err != nil { return utils.TrackError(err) } - // CheckForProvisioningStateConflict does not log conflict errors // but does log unexpected errors like database failures. if err := checkForProvisioningStateConflict(ctx, f.dbClient, operationRequest, cluster.ID, cluster.ServiceProviderProperties.ProvisioningState); err != nil { return utils.TrackError(err) } + if cluster.ServiceProviderProperties.ClusterServiceID == nil { + return utils.TrackError(fmt.Errorf("cluster %s has no ClusterServiceID", cluster.ID)) + } // New credential cannot be requested while credentials are being revoked. if len(cluster.ServiceProviderProperties.RevokeCredentialsOperationID) > 0 { @@ -362,7 +365,7 @@ func (f *Frontend) ArmResourceActionRequestAdminCredential(writer http.ResponseW return arm.NewConflictError(clusterResourceID, "Cannot request credential while credentials are being revoked") } - csCredential, err := f.clusterServiceClient.PostBreakGlassCredential(ctx, cluster.ServiceProviderProperties.ClusterServiceID) + csCredential, err := f.clusterServiceClient.PostBreakGlassCredential(ctx, *cluster.ServiceProviderProperties.ClusterServiceID) if err != nil { return utils.TrackError(err) } @@ -420,12 +423,14 @@ func (f *Frontend) ArmResourceActionRevokeCredentials(writer http.ResponseWriter if err != nil { return utils.TrackError(err) } - // CheckForProvisioningStateConflict does not log conflict errors // but does log unexpected errors like database failures. if err := checkForProvisioningStateConflict(ctx, f.dbClient, operationRequest, cluster.ID, cluster.ServiceProviderProperties.ProvisioningState); err != nil { return utils.TrackError(err) } + if cluster.ServiceProviderProperties.ClusterServiceID == nil { + return utils.TrackError(fmt.Errorf("cluster %s has no ClusterServiceID", cluster.ID)) + } subscription, err := f.dbClient.Subscriptions().Get(ctx, clusterResourceID.SubscriptionID) if err != nil { @@ -453,7 +458,7 @@ func (f *Frontend) ArmResourceActionRevokeCredentials(writer http.ResponseWriter return arm.NewConflictError(clusterResourceID, "Credentials are already being revoked") } - err = f.clusterServiceClient.DeleteBreakGlassCredentials(ctx, cluster.ServiceProviderProperties.ClusterServiceID) + err = f.clusterServiceClient.DeleteBreakGlassCredentials(ctx, *cluster.ServiceProviderProperties.ClusterServiceID) if err != nil { return utils.TrackError(err) } @@ -473,7 +478,7 @@ func (f *Frontend) ArmResourceActionRevokeCredentials(writer http.ResponseWriter operationDoc := database.NewOperation( operationRequest, clusterResourceID, - cluster.ServiceProviderProperties.ClusterServiceID, + *cluster.ServiceProviderProperties.ClusterServiceID, f.azureLocation, request.Header.Get(arm.HeaderNameHomeTenantID), request.Header.Get(arm.HeaderNameClientObjectID), @@ -1024,7 +1029,7 @@ func (f *Frontend) OperationResult(writer http.ResponseWriter, request *http.Req return utils.TrackError(err) } - case operation.InternalID.Kind() == arohcpv1alpha1.ClusterKind: + case armhelpers.ResourceTypeEqual(operation.ExternalID.ResourceType, api.ClusterResourceType): resultingInternalCluster, err := f.getInternalClusterFromStorage(ctx, operation.ExternalID) if err != nil { return utils.TrackError(err) diff --git a/frontend/pkg/frontend/frontend_test.go b/frontend/pkg/frontend/frontend_test.go index 7868a66b1db..c890374c032 100644 --- a/frontend/pkg/frontend/frontend_test.go +++ b/frontend/pkg/frontend/frontend_test.go @@ -605,7 +605,7 @@ func TestRequestAdminCredential(t *testing.T) { }, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ ProvisioningState: test.clusterProvisioningState, - ClusterServiceID: clusterInternalID, + ClusterServiceID: &clusterInternalID, RevokeCredentialsOperationID: test.revokeCredentialsOperationID, }, } @@ -724,7 +724,7 @@ func TestRevokeCredentials(t *testing.T) { }, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ ProvisioningState: test.clusterProvisioningState, - ClusterServiceID: clusterInternalID, + ClusterServiceID: &clusterInternalID, RevokeCredentialsOperationID: test.revokeCredentialsOperationID, }, } diff --git a/frontend/pkg/frontend/helpers_test.go b/frontend/pkg/frontend/helpers_test.go index 660bf8e0299..61a4af2e8f4 100644 --- a/frontend/pkg/frontend/helpers_test.go +++ b/frontend/pkg/frontend/helpers_test.go @@ -130,7 +130,7 @@ func TestCheckForProvisioningStateConflict(t *testing.T) { }, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ ProvisioningState: arm.ProvisioningStateSucceeded, - ClusterServiceID: clusterInternalID, + ClusterServiceID: &clusterInternalID, }, } _, _ = mockDBClient.HCPClusters(parentResourceID.SubscriptionID, parentResourceID.ResourceGroupName).Create(ctx, parentCluster, nil) @@ -177,7 +177,7 @@ func TestCheckForProvisioningStateConflict(t *testing.T) { }, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ ProvisioningState: provisioningState, - ClusterServiceID: clusterInternalID, + ClusterServiceID: &clusterInternalID, }, } _, _ = mockDBClient.HCPClusters(parentResourceID.SubscriptionID, parentResourceID.ResourceGroupName).Create(ctx, parentCluster, nil) diff --git a/frontend/pkg/frontend/node_pool.go b/frontend/pkg/frontend/node_pool.go index c1e924b5c1c..7854a6f9097 100644 --- a/frontend/pkg/frontend/node_pool.go +++ b/frontend/pkg/frontend/node_pool.go @@ -89,6 +89,9 @@ func (f *Frontend) ArmResourceListNodePools(writer http.ResponseWriter, request if err != nil { return utils.TrackError(err) } + if internalCluster.ServiceProviderProperties.ClusterServiceID == nil { + return utils.TrackError(fmt.Errorf("cluster %s has no ClusterServiceID", internalCluster.ID)) + } pagedResponse := arm.NewPagedResponse() @@ -120,7 +123,7 @@ func (f *Frontend) ArmResourceListNodePools(writer http.ResponseWriter, request query := fmt.Sprintf("id in (%s)", strings.Join(queryIDs, ", ")) logger.Info(fmt.Sprintf("Searching Cluster Service for %q", query)) - csIterator := f.clusterServiceClient.ListNodePools(internalCluster.ServiceProviderProperties.ClusterServiceID, query) + csIterator := f.clusterServiceClient.ListNodePools(*internalCluster.ServiceProviderProperties.ClusterServiceID, query) for csNodePool := range csIterator.Items(ctx) { if internalNodePool, ok := nodePoolsByClusterServiceID[csNodePool.ID()]; ok { internalNodePool, err = mergeToInternalNodePool(csNodePool, internalNodePool, f.azureLocation) @@ -284,6 +287,9 @@ func (f *Frontend) createNodePool(writer http.ResponseWriter, request *http.Requ if err != nil { return utils.TrackError(err) } + if cluster.ServiceProviderProperties.ClusterServiceID == nil { + return utils.TrackError(fmt.Errorf("cluster %s has no ClusterServiceID", cluster.ID)) + } validationOp := operation.Operation{ Type: operation.Create, @@ -304,7 +310,7 @@ func (f *Frontend) createNodePool(writer http.ResponseWriter, request *http.Requ if err != nil { return utils.TrackError(err) } - csNodePool, err := f.clusterServiceClient.PostNodePool(ctx, cluster.ServiceProviderProperties.ClusterServiceID, csNodePoolBuilder) + csNodePool, err := f.clusterServiceClient.PostNodePool(ctx, *cluster.ServiceProviderProperties.ClusterServiceID, csNodePoolBuilder) if err != nil { return utils.TrackError(err) } diff --git a/internal/api/types_cluster.go b/internal/api/types_cluster.go index 7b9c7907788..ba7c919e4cb 100644 --- a/internal/api/types_cluster.go +++ b/internal/api/types_cluster.go @@ -66,7 +66,7 @@ type HCPOpenShiftClusterCustomerProperties struct { type HCPOpenShiftClusterServiceProviderProperties struct { ExistingCosmosUID string `json:"-"` ProvisioningState arm.ProvisioningState `json:"provisioningState,omitempty"` - ClusterServiceID InternalID `json:"clusterServiceID,omitempty"` + ClusterServiceID *InternalID `json:"clusterServiceID,omitempty"` ActiveOperationID string `json:"activeOperationId,omitempty"` RevokeCredentialsOperationID string `json:"revokeCredentialsOperationId,omitempty"` DNS ServiceProviderDNSProfile `json:"dns,omitempty"` diff --git a/internal/api/types_runtime_test.go b/internal/api/types_runtime_test.go index 07f8fd7086b..a5cffe0b7a7 100644 --- a/internal/api/types_runtime_test.go +++ b/internal/api/types_runtime_test.go @@ -64,7 +64,7 @@ func deepCopyFuzzerFor(src rand.Source) *randfill.Filler { return } foo := Must(NewInternalID("/api/clusters_mgmt/v1/clusters/r" + strings.ReplaceAll(c.String(10), "/", "-"))) - j.ClusterServiceID = foo + j.ClusterServiceID = &foo }, func(j *HCPOpenShiftClusterNodePoolServiceProviderProperties, c randfill.Continue) { c.FillNoCustom(j) diff --git a/internal/api/v20240610preview/conversion_fuzz_test.go b/internal/api/v20240610preview/conversion_fuzz_test.go index f8232766cc9..0268cb43eb1 100644 --- a/internal/api/v20240610preview/conversion_fuzz_test.go +++ b/internal/api/v20240610preview/conversion_fuzz_test.go @@ -54,7 +54,7 @@ func TestRoundTripInternalExternalInternal(t *testing.T) { // RevokeCredentialsOperationID does not roundtrip through the external type because it is purely an internal detail j.RevokeCredentialsOperationID = "" // ClusterServiceID does not roundtrip through the external type because it is purely an internal detail - j.ClusterServiceID = ocm.InternalID{} + j.ClusterServiceID = nil j.ExistingCosmosUID = "" // ExperimentalFeatures does not roundtrip through the external type because it is purely an internal detail j.ExperimentalFeatures = api.ExperimentalFeatures{} diff --git a/internal/api/v20251223preview/conversion_fuzz_test.go b/internal/api/v20251223preview/conversion_fuzz_test.go index 31680541603..d2cccbc4823 100644 --- a/internal/api/v20251223preview/conversion_fuzz_test.go +++ b/internal/api/v20251223preview/conversion_fuzz_test.go @@ -52,7 +52,7 @@ func TestRoundTripInternalExternalInternal(t *testing.T) { // RevokeCredentialsOperationID does not roundtrip through the external type because it is purely an internal detail j.RevokeCredentialsOperationID = "" // ClusterServiceID does not roundtrip through the external type because it is purely an internal detail - j.ClusterServiceID = ocm.InternalID{} + j.ClusterServiceID = nil j.ExistingCosmosUID = "" // ExperimentalFeatures does not roundtrip through the external type because it is purely an internal detail j.ExperimentalFeatures = api.ExperimentalFeatures{} diff --git a/internal/api/zz_generated.deepcopy.go b/internal/api/zz_generated.deepcopy.go index bf8f9266d1d..72e8205ba13 100644 --- a/internal/api/zz_generated.deepcopy.go +++ b/internal/api/zz_generated.deepcopy.go @@ -449,7 +449,7 @@ func (in *HCPOpenShiftCluster) DeepCopyInto(out *HCPOpenShiftCluster) { *out = *in in.TrackedResource.DeepCopyInto(&out.TrackedResource) in.CustomerProperties.DeepCopyInto(&out.CustomerProperties) - out.ServiceProviderProperties = in.ServiceProviderProperties + in.ServiceProviderProperties.DeepCopyInto(&out.ServiceProviderProperties) if in.Identity != nil { in, out := &in.Identity, &out.Identity *out = new(arm.ManagedServiceIdentity) @@ -785,7 +785,11 @@ func (in *HCPOpenShiftClusterNodePoolServiceProviderProperties) DeepCopy() *HCPO // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *HCPOpenShiftClusterServiceProviderProperties) DeepCopyInto(out *HCPOpenShiftClusterServiceProviderProperties) { *out = *in - out.ClusterServiceID = in.ClusterServiceID + if in.ClusterServiceID != nil { + in, out := &in.ClusterServiceID, &out.ClusterServiceID + *out = new(InternalID) + **out = **in + } out.DNS = in.DNS out.Console = in.Console out.API = in.API diff --git a/internal/database/convert_cluster.go b/internal/database/convert_cluster.go index f59b3598981..0fb8f4466ba 100644 --- a/internal/database/convert_cluster.go +++ b/internal/database/convert_cluster.go @@ -18,9 +18,10 @@ import ( "fmt" "strings" + "k8s.io/utils/ptr" + "github.com/Azure/ARO-HCP/internal/api" "github.com/Azure/ARO-HCP/internal/api/arm" - "github.com/Azure/ARO-HCP/internal/ocm" ) func InternalToCosmosCluster(internalObj *api.HCPOpenShiftCluster) (*HCPCluster, error) { @@ -43,7 +44,7 @@ func InternalToCosmosCluster(internalObj *api.HCPOpenShiftCluster) (*HCPCluster, }, ResourceDocument: &ResourceDocument{ ResourceID: internalObj.ID, - InternalID: internalObj.ServiceProviderProperties.ClusterServiceID, + InternalID: ptr.Deref(internalObj.ServiceProviderProperties.ClusterServiceID, api.InternalID{}), ActiveOperationID: internalObj.ServiceProviderProperties.ActiveOperationID, ProvisioningState: internalObj.ServiceProviderProperties.ProvisioningState, Identity: toCosmosIdentity(internalObj.Identity), @@ -66,7 +67,9 @@ func InternalToCosmosCluster(internalObj *api.HCPOpenShiftCluster) (*HCPCluster, cosmosObj.InternalState.InternalAPI.SystemData = nil cosmosObj.InternalState.InternalAPI.Tags = nil cosmosObj.InternalState.InternalAPI.ServiceProviderProperties.ProvisioningState = "" - cosmosObj.InternalState.InternalAPI.ServiceProviderProperties.ClusterServiceID = ocm.InternalID{} + // we do this to keep serialization the same so that we can go to n-1 where this field isn't a pointer. + // on the reading side, we handle the pointer as expected. + cosmosObj.InternalState.InternalAPI.ServiceProviderProperties.ClusterServiceID = &api.InternalID{} cosmosObj.InternalState.InternalAPI.ServiceProviderProperties.ActiveOperationID = "" // This is not the place for validation, but during such a transition we need to ensure we fail quickly and certainly @@ -152,7 +155,13 @@ func CosmosToInternalCluster(cosmosObj *HCPCluster) (*api.HCPOpenShiftCluster, e internalObj.Tags = copyTags(resourceDoc.Tags) internalObj.ServiceProviderProperties.ExistingCosmosUID = cosmosObj.ID internalObj.ServiceProviderProperties.ProvisioningState = resourceDoc.ProvisioningState - internalObj.ServiceProviderProperties.ClusterServiceID = resourceDoc.InternalID + + 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_cluster_test.go b/internal/database/convert_cluster_test.go index ac39653f623..ab97111f231 100644 --- a/internal/database/convert_cluster_test.go +++ b/internal/database/convert_cluster_test.go @@ -66,7 +66,7 @@ func TestRoundTripClusterInternalCosmosInternal(t *testing.T) { } // we must always have an internal ID foo := api.Must(api.NewInternalID("/api/clusters_mgmt/v1/clusters/r" + strings.ReplaceAll(c.String(10), "/", "-"))) - j.ClusterServiceID = foo + j.ClusterServiceID = &foo }, func(j *api.HCPOpenShiftCluster, c randfill.Continue) { c.FillNoCustom(j) diff --git a/internal/databasetesting/mock_dbclient_test.go b/internal/databasetesting/mock_dbclient_test.go index 346725578f0..299ae729928 100644 --- a/internal/databasetesting/mock_dbclient_test.go +++ b/internal/databasetesting/mock_dbclient_test.go @@ -157,7 +157,7 @@ func TestMockDBClient_CRUD_Cluster(t *testing.T) { }, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ ProvisioningState: arm.ProvisioningStateSucceeded, - ClusterServiceID: internalID, + ClusterServiceID: &internalID, }, } @@ -395,7 +395,7 @@ func TestMockDBClient_Transaction(t *testing.T) { }, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ ProvisioningState: arm.ProvisioningStateSucceeded, - ClusterServiceID: internalID, + ClusterServiceID: &internalID, }, } @@ -453,7 +453,7 @@ func TestMockDBClient_UntypedCRUD(t *testing.T) { }, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ ProvisioningState: arm.ProvisioningStateSucceeded, - ClusterServiceID: internalID, + ClusterServiceID: &internalID, }, } @@ -808,7 +808,7 @@ func TestMockDBClient_addResource(t *testing.T) { }, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ ProvisioningState: arm.ProvisioningStateSucceeded, - ClusterServiceID: internalID, + ClusterServiceID: &internalID, }, } @@ -889,7 +889,7 @@ func TestNewMockDBClientWithResources(t *testing.T) { }, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ ProvisioningState: arm.ProvisioningStateSucceeded, - ClusterServiceID: internalID, + ClusterServiceID: &internalID, }, } diff --git a/internal/serverutils/dump_billing_test.go b/internal/serverutils/dump_billing_test.go index 4834d9ae42f..f62a3ee3226 100644 --- a/internal/serverutils/dump_billing_test.go +++ b/internal/serverutils/dump_billing_test.go @@ -52,7 +52,7 @@ func TestDumpBillingToLogger(t *testing.T) { }, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ ClusterUID: "billing-doc-1", - ClusterServiceID: api.Must(api.NewInternalID("/api/clusters_mgmt/v1/clusters/test-cluster-1")), + ClusterServiceID: api.Ptr(api.Must(api.NewInternalID("/api/clusters_mgmt/v1/clusters/test-cluster-1"))), }, } @@ -66,7 +66,7 @@ func TestDumpBillingToLogger(t *testing.T) { }, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ ClusterUID: "billing-doc-2", - ClusterServiceID: api.Must(api.NewInternalID("/api/clusters_mgmt/v1/clusters/test-cluster-2")), + ClusterServiceID: api.Ptr(api.Must(api.NewInternalID("/api/clusters_mgmt/v1/clusters/test-cluster-2"))), }, } @@ -128,7 +128,7 @@ func TestDumpBillingToLogger_PartitionScoping(t *testing.T) { }, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ ClusterUID: "cluster-1-billing-1", - ClusterServiceID: api.Must(api.NewInternalID("/api/clusters_mgmt/v1/clusters/test-cluster-1")), + ClusterServiceID: api.Ptr(api.Must(api.NewInternalID("/api/clusters_mgmt/v1/clusters/test-cluster-1"))), }, } @@ -142,7 +142,7 @@ func TestDumpBillingToLogger_PartitionScoping(t *testing.T) { }, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ ClusterUID: "cluster-2-billing-2", - ClusterServiceID: api.Must(api.NewInternalID("/api/clusters_mgmt/v1/clusters/test-cluster-2")), + ClusterServiceID: api.Ptr(api.Must(api.NewInternalID("/api/clusters_mgmt/v1/clusters/test-cluster-2"))), }, } @@ -156,7 +156,7 @@ func TestDumpBillingToLogger_PartitionScoping(t *testing.T) { }, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ ClusterUID: "cluster-3-billing-3", - ClusterServiceID: api.Must(api.NewInternalID("/api/clusters_mgmt/v1/clusters/test-cluster-3")), + ClusterServiceID: api.Ptr(api.Must(api.NewInternalID("/api/clusters_mgmt/v1/clusters/test-cluster-3"))), }, } diff --git a/internal/validation/validate_cluster.go b/internal/validation/validate_cluster.go index 2e2921b6fa1..7520e9e253c 100644 --- a/internal/validation/validate_cluster.go +++ b/internal/validation/validate_cluster.go @@ -258,7 +258,7 @@ var ( return &oldObj.DNS } toServiceProviderClusterServiceID = func(oldObj *api.HCPOpenShiftClusterServiceProviderProperties) *api.InternalID { - return &oldObj.ClusterServiceID + return oldObj.ClusterServiceID } toServiceProviderConsole = func(oldObj *api.HCPOpenShiftClusterServiceProviderProperties) *api.ServiceProviderConsoleProfile { return &oldObj.Console @@ -283,8 +283,8 @@ func validateClusterServiceProviderProperties(ctx context.Context, op operation. // ProvisioningState arm.ProvisioningState `json:"provisioningState,omitempty"` errs = append(errs, validate.ImmutableByCompare(ctx, op, fldPath.Child("provisioningState"), &newObj.ProvisioningState, safe.Field(oldObj, toHCPOpenShiftClusterServiceProviderPropertiesProvisioningState))...) - //ClusterServiceID InternalID `json:"clusterServiceID,omitempty"` - errs = append(errs, validate.ImmutableByReflect(ctx, op, fldPath.Child("clusterServiceID"), &newObj.ClusterServiceID, safe.Field(oldObj, toServiceProviderClusterServiceID))...) + //ClusterServiceID *InternalID `json:"clusterServiceID,omitempty"` + errs = append(errs, validate.ImmutableByReflect(ctx, op, fldPath.Child("clusterServiceID"), newObj.ClusterServiceID, safe.Field(oldObj, toServiceProviderClusterServiceID))...) // DNS CustomerDNSProfile `json:"dns,omitempty"` errs = append(errs, validateServiceProviderDNSProfile(ctx, op, fldPath.Child("dns"), &newObj.DNS, safe.Field(oldObj, toServiceProviderDNS))...) diff --git a/test-integration/backend/launch/metrics_test.go b/test-integration/backend/launch/metrics_test.go index 73621de8b53..d2e9fbfd251 100644 --- a/test-integration/backend/launch/metrics_test.go +++ b/test-integration/backend/launch/metrics_test.go @@ -155,7 +155,7 @@ func newMetricsTestCluster(resourceID *azcorearm.ResourceID, provisioningState a }, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ ProvisioningState: provisioningState, - ClusterServiceID: api.Must(api.NewInternalID("/api/clusters_mgmt/v1/clusters/test-cluster")), + ClusterServiceID: api.Ptr(api.Must(api.NewInternalID("/api/clusters_mgmt/v1/clusters/test-cluster"))), }, } } diff --git a/test-integration/frontend/informer_test.go b/test-integration/frontend/informer_test.go index 6fa3a8c5d67..14e3244ff57 100644 --- a/test-integration/frontend/informer_test.go +++ b/test-integration/frontend/informer_test.go @@ -369,7 +369,7 @@ func clusterInformerIntegrationTestCase() informerIntegrationTestCase { }, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ ProvisioningState: state, - ClusterServiceID: internalID, + ClusterServiceID: &internalID, }, } } @@ -497,7 +497,7 @@ func nodePoolInformerIntegrationTestCase() informerIntegrationTestCase { }, ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ ProvisioningState: arm.ProvisioningStateSucceeded, - ClusterServiceID: internalID, + ClusterServiceID: &internalID, }, } _, err = dbClient.HCPClusters(subscriptionID, resourceGroupName).Create(ctx, cluster, nil)