diff --git a/backend/pkg/app/backend.go b/backend/pkg/app/backend.go index db32f83d5db..41f4583a03f 100644 --- a/backend/pkg/app/backend.go +++ b/backend/pkg/app/backend.go @@ -327,6 +327,11 @@ func (b *Backend) runBackendControllersUnderLeaderElection(ctx context.Context, b.options.ClustersServiceClient, activeOperationInformer, ) + dispatchClusterCreateController := operationcontrollers.NewDispatchClusterCreateController( + b.options.CosmosDBClient, + b.options.ClustersServiceClient, + activeOperationInformer, + ) operationClusterCreateController := operationcontrollers.NewOperationClusterCreateController( b.options.CosmosDBClient, b.options.ClustersServiceClient, @@ -545,6 +550,7 @@ func (b *Backend) runBackendControllersUnderLeaderElection(ctx context.Context, go doNothingController.Run(ctx, 20) go dispatchRequestCredentialController.Run(ctx, 20) go dispatchRevokeCredentialsController.Run(ctx, 20) + go dispatchClusterCreateController.Run(ctx, 20) go operationClusterCreateController.Run(ctx, 20) go operationClusterUpdateController.Run(ctx, 20) go operationClusterDeleteController.Run(ctx, 20) diff --git a/backend/pkg/controllers/operationcontrollers/dispatch_cluster_create.go b/backend/pkg/controllers/operationcontrollers/dispatch_cluster_create.go new file mode 100644 index 00000000000..1a4fdc1dd69 --- /dev/null +++ b/backend/pkg/controllers/operationcontrollers/dispatch_cluster_create.go @@ -0,0 +1,241 @@ +// Copyright 2026 Microsoft Corporation +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package operationcontrollers + +import ( + "context" + "fmt" + "strings" + "time" + + arohcpv1alpha1 "github.com/openshift-online/ocm-sdk-go/arohcp/v1alpha1" + "k8s.io/client-go/tools/cache" + + "github.com/Azure/ARO-HCP/backend/pkg/controllers/controllerutils" + "github.com/Azure/ARO-HCP/internal/api" + "github.com/Azure/ARO-HCP/internal/database" + "github.com/Azure/ARO-HCP/internal/ocm" + "github.com/Azure/ARO-HCP/internal/utils" +) + +type dispatchClusterCreate struct { + cosmosClient database.DBClient + clustersServiceClient ocm.ClusterServiceClientSpec +} + +func NewDispatchClusterCreateController( + cosmosClient database.DBClient, + clustersServiceClient ocm.ClusterServiceClientSpec, + activeOperationInformer cache.SharedIndexInformer, +) controllerutils.Controller { + syncer := &dispatchClusterCreate{ + cosmosClient: cosmosClient, + clustersServiceClient: clustersServiceClient, + } + + return NewGenericOperationController( + "DispatchClusterCreate", + syncer, + 10*time.Second, + activeOperationInformer, + cosmosClient, + ) +} + +func (c *dispatchClusterCreate) ShouldProcess(ctx context.Context, operation *api.Operation) bool { + if operation.Status.IsTerminal() { + return false + } + if operation.Request != database.OperationRequestCreate { + return false + } + if operation.ExternalID == nil || !strings.EqualFold(operation.ExternalID.ResourceType.String(), api.ClusterResourceType.String()) { + return false + } + if len(operation.InternalID.String()) > 0 { + return false + } + return true +} + +func (c *dispatchClusterCreate) SynchronizeOperation(ctx context.Context, key controllerutils.OperationKey) error { + logger := utils.LoggerFromContext(ctx) + logger.Info("checking operation") + + operation, err := c.cosmosClient.Operations(key.SubscriptionID).Get(ctx, key.OperationName) + if database.IsNotFoundError(err) { + return nil + } + if err != nil { + return fmt.Errorf("failed to get active operation: %w", err) + } + if !c.ShouldProcess(ctx, operation) { + return nil + } + + cluster, err := c.cosmosClient.HCPClusters(operation.ExternalID.SubscriptionID, operation.ExternalID.ResourceGroupName).Get(ctx, operation.ExternalID.Name) + if err != nil { + return utils.TrackError(err) + } + + if cluster.ServiceProviderProperties.ActiveOperationID != "" && + cluster.ServiceProviderProperties.ActiveOperationID != operation.OperationID.Name { + logger.Info("skipping cluster create dispatch: active operation mismatch", + "cluster_active_operation_id", cluster.ServiceProviderProperties.ActiveOperationID, + "operation_name", operation.OperationID.Name) + return nil + } + + csInternalIDFromCluster := cluster.ServiceProviderProperties.ClusterServiceID + if csInternalIDFromCluster != nil && len(csInternalIDFromCluster.String()) > 0 { + // Recovery: cluster document was updated with ClusterServiceID but the operation + // write failed or lagged. Only patch the operation when it still has no InternalID. + if len(operation.InternalID.String()) > 0 { + if strings.EqualFold(operation.InternalID.String(), csInternalIDFromCluster.String()) { + return nil + } + return fmt.Errorf("cluster create dispatch: operation internalId %q does not match cluster clusterServiceID %q", + operation.InternalID.String(), csInternalIDFromCluster.String()) + } + operation.InternalID = *csInternalIDFromCluster + _, err = c.cosmosClient.Operations(key.SubscriptionID).Replace(ctx, operation, nil) + if err != nil { + return utils.TrackError(err) + } + return nil + } + + subscription, err := c.cosmosClient.Subscriptions().Get(ctx, operation.ExternalID.SubscriptionID) + if err != nil { + return utils.TrackError(err) + } + if subscription.Properties == nil || subscription.Properties.TenantId == nil || *subscription.Properties.TenantId == "" { + return utils.TrackError(fmt.Errorf("subscription %s has no tenant id", operation.ExternalID.SubscriptionID)) + } + tenantID := *subscription.Properties.TenantId + + mrg := cluster.CustomerProperties.Platform.ManagedResourceGroup + if mrg == "" { + return utils.TrackError(fmt.Errorf("cluster %s has no managed resource group", cluster.Name)) + } + existing, err := c.findAROHCPClusterByAzureInfo(ctx, + operation.ExternalID.SubscriptionID, + operation.ExternalID.ResourceGroupName, + operation.ExternalID.Name, + tenantID, + mrg, + ) + if err != nil { + return utils.TrackError(err) + } + + var csCluster *arohcpv1alpha1.Cluster + if existing != nil { + csCluster = existing + logger.Info("adopting existing Cluster Service cluster for Azure resource") + } else { + clusterBuilder, autoscalerBuilder, err := ocm.BuildCSCluster(cluster.ID, tenantID, cluster, nil, nil) + if err != nil { + return utils.TrackError(err) + } + logger.Info("dispatching POST clusters to Cluster Service") + csCluster, err = c.clustersServiceClient.PostCluster(ctx, clusterBuilder, autoscalerBuilder) + if err != nil { + return utils.TrackError(err) + } + } + + csInternalID, err := api.NewInternalID(csCluster.HREF()) + if err != nil { + return utils.TrackError(err) + } + + cluster.ServiceProviderProperties.ClusterServiceID = &csInternalID + _, err = c.cosmosClient.HCPClusters(operation.ExternalID.SubscriptionID, operation.ExternalID.ResourceGroupName).Replace(ctx, cluster, nil) + if err != nil { + return utils.TrackError(err) + } + + operation.InternalID = csInternalID + _, err = c.cosmosClient.Operations(key.SubscriptionID).Replace(ctx, operation, nil) + if err != nil { + return utils.TrackError(err) + } + + return nil +} + +// findAROHCPClusterByAzureInfo returns the Cluster Service cluster whose Azure +// metadata matches the given subscription, resource group, ARM resource name, +// tenant ID, and managed resource group name (MRG). +// It returns (nil, nil) when no such cluster exists. +// An error is returned if more than one cluster is returned matching the azure metadata, as it should be unique. +func (c *dispatchClusterCreate) findAROHCPClusterByAzureInfo(ctx context.Context, subscriptionID, resourceGroupName, resourceName, tenantID, managedResourceGroupName string) (*arohcpv1alpha1.Cluster, error) { + // Subscription ID, resource group, and cluster name are lowercased when building the Cluster Service + // cluster (see withImmutableAttributes in convert.go). + wantSub := strings.ToLower(subscriptionID) + wantRG := strings.ToLower(resourceGroupName) + wantName := strings.ToLower(resourceName) + // Tenant ID and managed resource group are not lowercased in the OCM CS + // builder (see withImmutableAttributes in convert.go)), we keep the casing as it is. + wantTenant := tenantID + wantMRG := managedResourceGroupName + search := c.clustersServiceClusterByAzureInfoSearchString(wantSub, wantRG, wantName, wantTenant, wantMRG) + matches, err := c.csClustersMatchingClusterByAzureInfo(ctx, c.clustersServiceClient.ListClusters(search), wantSub, wantRG, wantName, wantTenant, wantMRG) + if err != nil { + return nil, err + } + if len(matches) > 1 { + return nil, fmt.Errorf( + "cluster service returned %d clusters for one Azure resource (expected exactly 1): "+ + "subscription_id=%q resource_group=%q resource_name=%q tenant_id=%q managed_resource_group=%q", + len(matches), wantSub, wantRG, wantName, wantTenant, wantMRG, + ) + } + if len(matches) == 1 { + return matches[0], nil + } + return nil, nil +} + +func (c *dispatchClusterCreate) clustersServiceClusterByAzureInfoSearchString(wantSub, wantRG, wantName, wantTenant, wantMRG string) string { + return fmt.Sprintf( + "azure.subscription_id = '%s' and azure.resource_group_name = '%s' and azure.resource_name = '%s' and "+ + "azure.tenant_id = '%s' and azure.managed_resource_group_name = '%s'", + wantSub, wantRG, wantName, wantTenant, wantMRG, + ) +} + +func (c *dispatchClusterCreate) csClustersMatchingClusterByAzureInfo(ctx context.Context, it ocm.ClusterListIterator, wantSub, wantRG, wantName, wantTenant, wantMRG string) ([]*arohcpv1alpha1.Cluster, error) { + var res []*arohcpv1alpha1.Cluster + for csCluster := range it.Items(ctx) { + az := csCluster.Azure() + if az == nil { + continue + } + if az.SubscriptionID() != wantSub || + az.ResourceGroupName() != wantRG || + az.ResourceName() != wantName || + az.TenantID() != wantTenant || + az.ManagedResourceGroupName() != wantMRG { + continue + } + res = append(res, csCluster) + } + if err := it.GetError(); err != nil { + return nil, err + } + return res, nil +} diff --git a/backend/pkg/controllers/operationcontrollers/dispatch_cluster_create_test.go b/backend/pkg/controllers/operationcontrollers/dispatch_cluster_create_test.go new file mode 100644 index 00000000000..ddad239041a --- /dev/null +++ b/backend/pkg/controllers/operationcontrollers/dispatch_cluster_create_test.go @@ -0,0 +1,261 @@ +// Copyright 2026 Microsoft Corporation +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package operationcontrollers + +import ( + "context" + "strings" + "testing" + + "github.com/go-logr/logr/testr" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" + "k8s.io/utils/ptr" + + arohcpv1alpha1 "github.com/openshift-online/ocm-sdk-go/arohcp/v1alpha1" + + "github.com/Azure/ARO-HCP/internal/api" + "github.com/Azure/ARO-HCP/internal/api/arm" + "github.com/Azure/ARO-HCP/internal/database" + "github.com/Azure/ARO-HCP/internal/databasetesting" + "github.com/Azure/ARO-HCP/internal/ocm" + "github.com/Azure/ARO-HCP/internal/utils" +) + +func TestDispatchClusterCreate_SynchronizeOperation(t *testing.T) { + tests := []struct { + name string + setup func(*clusterTestFixture) (*api.HCPOpenShiftCluster, *api.Operation) + expectError bool + verify func(t *testing.T, ctx context.Context, db *databasetesting.MockDBClient, fixture *clusterTestFixture, mockCS *ocm.MockClusterServiceClientSpec) + }{ + { + name: "successful dispatch records cluster service ID on cluster and operation", + setup: func(f *clusterTestFixture) (*api.HCPOpenShiftCluster, *api.Operation) { + cluster := api.MinimumValidClusterTestCase() + cluster.ID = f.clusterResourceID + cluster.Name = testClusterName + cluster.Type = f.clusterResourceID.ResourceType.String() + cluster.ServiceProviderProperties.ClusterServiceID = nil + cluster.ServiceProviderProperties.ActiveOperationID = testOperationName + cluster.ServiceProviderProperties.ClusterUID = testClusterUID + op := f.newOperation(database.OperationRequestCreate) + op.InternalID = api.InternalID{} + return cluster, op + }, + expectError: false, + verify: func(t *testing.T, ctx context.Context, db *databasetesting.MockDBClient, fixture *clusterTestFixture, _ *ocm.MockClusterServiceClientSpec) { + op, err := db.Operations(testSubscriptionID).Get(ctx, testOperationName) + require.NoError(t, err) + assert.Equal(t, testClusterServiceIDStr, op.InternalID.String()) + cluster, err := db.HCPClusters(testSubscriptionID, testResourceGroupName).Get(ctx, testClusterName) + require.NoError(t, err) + require.NotNil(t, cluster.ServiceProviderProperties.ClusterServiceID) + assert.Equal(t, testClusterServiceIDStr, cluster.ServiceProviderProperties.ClusterServiceID.String()) + }, + }, + { + name: "recovery when cluster document already has ClusterServiceID", + setup: func(f *clusterTestFixture) (*api.HCPOpenShiftCluster, *api.Operation) { + cluster := f.newCluster(nil) + op := f.newOperation(database.OperationRequestCreate) + op.InternalID = api.InternalID{} + return cluster, op + }, + expectError: false, + verify: func(t *testing.T, ctx context.Context, db *databasetesting.MockDBClient, fixture *clusterTestFixture, _ *ocm.MockClusterServiceClientSpec) { + op, err := db.Operations(testSubscriptionID).Get(ctx, testOperationName) + require.NoError(t, err) + assert.Equal(t, testClusterServiceIDStr, op.InternalID.String()) + }, + }, + { + name: "active operation mismatch skips dispatch", + setup: func(f *clusterTestFixture) (*api.HCPOpenShiftCluster, *api.Operation) { + cluster := api.MinimumValidClusterTestCase() + cluster.ID = f.clusterResourceID + cluster.Name = testClusterName + cluster.Type = f.clusterResourceID.ResourceType.String() + cluster.ServiceProviderProperties.ClusterServiceID = nil + cluster.ServiceProviderProperties.ActiveOperationID = "other-op" + cluster.ServiceProviderProperties.ClusterUID = testClusterUID + op := f.newOperation(database.OperationRequestCreate) + op.InternalID = api.InternalID{} + return cluster, op + }, + expectError: false, + verify: func(t *testing.T, ctx context.Context, db *databasetesting.MockDBClient, fixture *clusterTestFixture, _ *ocm.MockClusterServiceClientSpec) { + op, err := db.Operations(testSubscriptionID).Get(ctx, testOperationName) + require.NoError(t, err) + assert.Equal(t, "", op.InternalID.String()) + }, + }, + { + name: "missing managed resource group returns error", + setup: func(f *clusterTestFixture) (*api.HCPOpenShiftCluster, *api.Operation) { + cluster := api.MinimumValidClusterTestCase() + cluster.ID = f.clusterResourceID + cluster.Name = testClusterName + cluster.Type = f.clusterResourceID.ResourceType.String() + cluster.ServiceProviderProperties.ClusterServiceID = nil + cluster.ServiceProviderProperties.ActiveOperationID = testOperationName + cluster.ServiceProviderProperties.ClusterUID = testClusterUID + cluster.CustomerProperties.Platform.ManagedResourceGroup = "" + op := f.newOperation(database.OperationRequestCreate) + op.InternalID = api.InternalID{} + return cluster, op + }, + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := context.Background() + ctx = utils.ContextWithLogger(ctx, testr.New(t)) + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + fixture := newClusterTestFixture() + cluster, operation := tt.setup(fixture) + + subscriptionResourceID := api.Must(arm.ToSubscriptionResourceID(testSubscriptionID)) + subscription := &arm.Subscription{ + CosmosMetadata: api.CosmosMetadata{ + ResourceID: subscriptionResourceID, + }, + ResourceID: subscriptionResourceID, + Properties: &arm.SubscriptionProperties{ + TenantId: ptr.To(testTenantID), + }, + } + + mockDB, err := databasetesting.NewMockDBClientWithResources(ctx, []any{subscription, cluster, operation}) + require.NoError(t, err) + + mockCS := ocm.NewMockClusterServiceClientSpec(ctrl) + + switch tt.name { + case "successful dispatch records cluster service ID on cluster and operation": + mockCS.EXPECT(). + ListClusters(gomock.Any()). + Return(ocm.NewSimpleClusterListIterator(nil, nil)) + csCluster, err := arohcpv1alpha1.NewCluster(). + HREF(testClusterServiceIDStr). + Build() + require.NoError(t, err) + mockCS.EXPECT(). + PostCluster(gomock.Any(), gomock.Any(), gomock.Any()). + Return(csCluster, nil) + case "recovery when cluster document already has ClusterServiceID": + // no Cluster Service calls + case "active operation mismatch skips dispatch": + // no Cluster Service calls + case "missing managed resource group returns error": + // no Cluster Service calls + } + + dispatcher := &dispatchClusterCreate{ + cosmosClient: mockDB, + clustersServiceClient: mockCS, + } + + err = dispatcher.SynchronizeOperation(ctx, fixture.operationKey()) + + if tt.expectError { + require.Error(t, err) + } else { + require.NoError(t, err) + } + + if tt.verify != nil { + tt.verify(t, ctx, mockDB, fixture, mockCS) + } + }) + } +} + +func TestDispatchClusterCreate_findAROHCPClusterByAzureInfo(t *testing.T) { + azureTestCluster := func(t *testing.T, sub, rg, name, tenant, mrg string) *arohcpv1alpha1.Cluster { + t.Helper() + c, err := arohcpv1alpha1.NewCluster(). + Name(name). + Azure(arohcpv1alpha1.NewAzure(). + SubscriptionID(sub). + ResourceGroupName(rg). + ResourceName(name). + TenantID(tenant). + ManagedResourceGroupName(mrg)). + Build() + require.NoError(t, err) + return c + } + + ctx := context.Background() + sub := "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee" + rg := "my-rg" + resName := "MyCluster" + tenant := "bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb" + mrg := "arohcp-mycluster-uuid" + + wantSearch := "azure.subscription_id = '" + strings.ToLower(sub) + "' and azure.resource_group_name = '" + strings.ToLower(rg) + "' and azure.resource_name = '" + strings.ToLower(resName) + "'" + + " and azure.tenant_id = '" + tenant + "'" + + " and azure.managed_resource_group_name = '" + mrg + "'" + + t.Run("found on primary search", func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + match := azureTestCluster(t, strings.ToLower(sub), strings.ToLower(rg), strings.ToLower(resName), tenant, mrg) + mock := ocm.NewMockClusterServiceClientSpec(ctrl) + mock.EXPECT(). + ListClusters(wantSearch). + Return(ocm.NewSimpleClusterListIterator([]*arohcpv1alpha1.Cluster{match}, nil)) + + d := &dispatchClusterCreate{clustersServiceClient: mock} + got, err := d.findAROHCPClusterByAzureInfo(ctx, sub, rg, resName, tenant, mrg) + require.NoError(t, err) + require.Same(t, match, got) + }) + + t.Run("not found", func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + mock := ocm.NewMockClusterServiceClientSpec(ctrl) + mock.EXPECT(). + ListClusters(wantSearch). + Return(ocm.NewSimpleClusterListIterator(nil, nil)) + + d := &dispatchClusterCreate{clustersServiceClient: mock} + got, err := d.findAROHCPClusterByAzureInfo(ctx, sub, rg, resName, tenant, mrg) + require.NoError(t, err) + require.Nil(t, got) + }) + + t.Run("multiple matches error", func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + a := azureTestCluster(t, strings.ToLower(sub), strings.ToLower(rg), strings.ToLower(resName), tenant, mrg) + b := azureTestCluster(t, strings.ToLower(sub), strings.ToLower(rg), strings.ToLower(resName), tenant, mrg) + mock := ocm.NewMockClusterServiceClientSpec(ctrl) + mock.EXPECT(). + ListClusters(wantSearch). + Return(ocm.NewSimpleClusterListIterator([]*arohcpv1alpha1.Cluster{a, b}, nil)) + + d := &dispatchClusterCreate{clustersServiceClient: mock} + _, err := d.findAROHCPClusterByAzureInfo(ctx, sub, rg, resName, tenant, mrg) + require.Error(t, err) + }) +} diff --git a/backend/pkg/controllers/operationcontrollers/operation_cluster_create.go b/backend/pkg/controllers/operationcontrollers/operation_cluster_create.go index 97e55c29481..6b3eab8c9e8 100644 --- a/backend/pkg/controllers/operationcontrollers/operation_cluster_create.go +++ b/backend/pkg/controllers/operationcontrollers/operation_cluster_create.go @@ -89,6 +89,9 @@ func (c *operationClusterCreate) ShouldProcess(ctx context.Context, operation *a if operation.ExternalID == nil || !strings.EqualFold(operation.ExternalID.ResourceType.String(), api.ClusterResourceType.String()) { return false } + if len(operation.InternalID.String()) == 0 { + return false + } return true } @@ -107,12 +110,8 @@ 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 - } + // TODO 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. + clusterStatus, err := c.clusterServiceClient.GetClusterStatus(ctx, operation.InternalID) if err != nil { return utils.TrackError(err) diff --git a/frontend/cmd/cmd.go b/frontend/cmd/cmd.go index 21067e5d8fc..f7bd04387f9 100644 --- a/frontend/cmd/cmd.go +++ b/frontend/cmd/cmd.go @@ -54,11 +54,8 @@ type FrontendOpts struct { auditLogQueueSize int auditConnectSocket bool - clustersServiceURL string - clusterServiceProvisionShard string - clusterServiceNoopProvision bool - clusterServiceNoopDeprovision bool - insecure bool + clustersServiceURL string + insecure bool location string metricsPort int @@ -101,9 +98,6 @@ func NewRootCmd() *cobra.Command { rootCmd.Flags().StringVar(&opts.clustersServiceURL, "clusters-service-url", "https://api.openshift.com", "URL of the OCM API gateway.") rootCmd.Flags().BoolVar(&opts.insecure, "insecure", false, "Skip validating TLS for clusters-service.") - rootCmd.Flags().StringVar(&opts.clusterServiceProvisionShard, "cluster-service-provision-shard", "", "Manually specify provision shard for all requests to cluster service") - rootCmd.Flags().BoolVar(&opts.clusterServiceNoopProvision, "cluster-service-noop-provision", false, "Skip cluster service provisioning steps for development purposes") - rootCmd.Flags().BoolVar(&opts.clusterServiceNoopDeprovision, "cluster-service-noop-deprovision", false, "Skip cluster service deprovisioning steps for development purposes") rootCmd.Flags().BoolVar(&opts.exitOnPanic, "exit-on-panic", opts.exitOnPanic, "If set, frontend will exit the process if a panic occurs. As of now it only controls the setting of k8s.io/apimachinery/pkg/util/runtime.ReallyCrash", @@ -242,8 +236,7 @@ func (opts *FrontendOpts) Run() error { f := frontend.NewFrontend( logger, listener, metricsListener, legacyregistry.Registerer(), legacyregistry.DefaultGatherer, - dbClient, csClient, auditClient, opts.location, opts.clusterServiceProvisionShard, - opts.clusterServiceNoopProvision, opts.clusterServiceNoopDeprovision, opts.exitOnPanic, + dbClient, csClient, auditClient, opts.location, opts.exitOnPanic, ) runErrCh := make(chan error, 1) diff --git a/frontend/pkg/frontend/cluster.go b/frontend/pkg/frontend/cluster.go index 1d944dc3cb9..af3fe8704ff 100644 --- a/frontend/pkg/frontend/cluster.go +++ b/frontend/pkg/frontend/cluster.go @@ -303,7 +303,6 @@ func (f *Frontend) createHCPCluster(writer http.ResponseWriter, request *http.Re // that represents an existing resource to be updated. ctx := request.Context() - logger := utils.LoggerFromContext(ctx) subscription, err := SubscriptionFromContext(ctx) if err != nil { @@ -343,44 +342,13 @@ func (f *Frontend) createHCPCluster(writer http.ResponseWriter, request *http.Re // TODO this is bad, see above TODOs. We want to validate what we store. newInternalCluster.Identity.UserAssignedIdentities = nil - var tenantID string - if subscription.Properties != nil && subscription.Properties.TenantId != nil { - tenantID = *subscription.Properties.TenantId - } - - initialClusterProperties := map[string]string{} - if len(f.clusterServiceProvisionShard) != 0 { - initialClusterProperties[ocm.CSPropertyProvisionShardID] = f.clusterServiceProvisionShard - } - if f.clusterServiceNoopProvision { - initialClusterProperties[ocm.CSPropertyNoopProvision] = ocm.CSPropertyEnabled - } - if f.clusterServiceNoopDeprovision { - initialClusterProperties[ocm.CSPropertyNoopDeprovision] = ocm.CSPropertyEnabled - } - newClusterServiceClusterBuilder, newClusterServiceAutoscalerBuilder, err := ocm.BuildCSCluster(newInternalCluster.ID, tenantID, newInternalCluster, initialClusterProperties, nil) - if err != nil { - return utils.TrackError(err) - } - logger.Info(fmt.Sprintf("creating resource %s", newInternalCluster.ID)) - resultingClusterServiceCluster, err := f.clusterServiceClient.PostCluster(ctx, newClusterServiceClusterBuilder, newClusterServiceAutoscalerBuilder) - if err != nil { - return utils.TrackError(err) - } - - csID, err := api.NewInternalID(resultingClusterServiceCluster.HREF()) - if err != nil { - return utils.TrackError(err) - } - newInternalCluster.ServiceProviderProperties.ClusterServiceID = &csID - transaction := f.dbClient.NewTransaction(newInternalCluster.ID.SubscriptionID) // TODO extract to straight instance creation and then validation. clusterCreateOperation := database.NewOperation( database.OperationRequestCreate, newInternalCluster.ID, - ptr.Deref(newInternalCluster.ServiceProviderProperties.ClusterServiceID, api.InternalID{}), + api.InternalID{}, f.azureLocation, request.Header.Get(arm.HeaderNameHomeTenantID), request.Header.Get(arm.HeaderNameClientObjectID), @@ -419,6 +387,7 @@ func (f *Frontend) createHCPCluster(writer http.ResponseWriter, request *http.Re return fmt.Errorf("unexpected type %T", resultingUncastInternalCluster) } + var resultingClusterServiceCluster *arohcpv1alpha1.Cluster // TODO remove this once we moved read from CS // TODO this overwrite will transformed into a "set" function as we transition fields to ownership in cosmos resultingInternalCluster, err = mergeToInternalCluster(resultingClusterServiceCluster, resultingInternalCluster, f.azureLocation) if err != nil { diff --git a/frontend/pkg/frontend/frontend.go b/frontend/pkg/frontend/frontend.go index 7cbab612b07..f2a66bc8559 100644 --- a/frontend/pkg/frontend/frontend.go +++ b/frontend/pkg/frontend/frontend.go @@ -65,16 +65,6 @@ type Frontend struct { // this is the azure location for this instance of the frontend azureLocation string - // clusterServiceProvisionShard pins cluster requests to a specific - // Cluster Service provision shard during testing. - clusterServiceProvisionShard string - // clusterServiceNoopProvision short-circuits the full provision flow - // during testing. - clusterServiceNoopProvision bool - // clusterServiceNoopDeprovision short-circuits the full deprovision flow - // during testing. - clusterServiceNoopDeprovision bool - apiRegistry api.APIRegistry exitOnPanic bool @@ -90,9 +80,6 @@ func NewFrontend( csClient ocm.ClusterServiceClientSpec, auditClient audit.Client, azureLocation string, - clusterServiceProvisionShard string, - clusterServiceNoopProvision bool, - clusterServiceNoopDeprovision bool, exitOnPanic bool, ) *Frontend { // zero side-effect registration path @@ -116,12 +103,9 @@ func NewFrontend( return utils.ContextWithLogger(context.Background(), logger) }, }, - auditClient: auditClient, - dbClient: dbClient, - collector: metrics.NewSubscriptionCollector(registerer, dbClient, azureLocation), - clusterServiceProvisionShard: clusterServiceProvisionShard, - clusterServiceNoopProvision: clusterServiceNoopProvision, - clusterServiceNoopDeprovision: clusterServiceNoopDeprovision, + auditClient: auditClient, + dbClient: dbClient, + collector: metrics.NewSubscriptionCollector(registerer, dbClient, azureLocation), healthGauge: promauto.With(registerer).NewGauge( prometheus.GaugeOpts{ Name: healthGaugeName, diff --git a/frontend/pkg/frontend/frontend_test.go b/frontend/pkg/frontend/frontend_test.go index f93cccdaa07..15c17fa0777 100644 --- a/frontend/pkg/frontend/frontend_test.go +++ b/frontend/pkg/frontend/frontend_test.go @@ -103,7 +103,7 @@ func TestSubscriptionsGET(t *testing.T) { nil, newNoopAuditClient(t), api.TestLocation, - "", false, false, true, + true, ) // Pre-populate subscription in the mock database @@ -252,7 +252,7 @@ func TestSubscriptionsPUT(t *testing.T) { nil, newNoopAuditClient(t), api.TestLocation, - "", false, false, true, + true, ) body, err := json.Marshal(&test.subscription) @@ -467,7 +467,7 @@ func TestDeploymentPreflight(t *testing.T) { nil, newNoopAuditClient(t), api.TestLocation, - "", false, false, true, + true, ) subs := map[string]*arm.Subscription{ @@ -592,7 +592,7 @@ func TestRequestAdminCredential(t *testing.T) { nil, newNoopAuditClient(t), api.TestLocation, - "", false, false, true, + true, ) // Pre-populate the mock database with cluster and subscription @@ -702,7 +702,7 @@ func TestRevokeCredentials(t *testing.T) { nil, newNoopAuditClient(t), api.TestLocation, - "", false, false, true, + true, ) // Pre-populate the mock database with cluster diff --git a/frontend/pkg/frontend/testhelpers.go b/frontend/pkg/frontend/testhelpers.go index d373639a459..d7e8fac4373 100644 --- a/frontend/pkg/frontend/testhelpers.go +++ b/frontend/pkg/frontend/testhelpers.go @@ -48,7 +48,7 @@ func NewTestFrontend(t *testing.T) *Frontend { nil, newNoopAuditClient(t), api.TestLocation, - "", false, false, true, + true, ) return f } diff --git a/test-integration/utils/integrationutils/utils.go b/test-integration/utils/integrationutils/utils.go index c097727625e..bbbd024718c 100644 --- a/test-integration/utils/integrationutils/utils.go +++ b/test-integration/utils/integrationutils/utils.go @@ -131,7 +131,7 @@ func NewIntegrationTestInfoFromEnv(ctx context.Context, t *testing.T, withMock b } fakeAuditClient := &FakeOTELClient{} metricsRegistry := prometheus.NewRegistry() - aroHCPFrontend := frontend.NewFrontend(logger, frontendListener, frontendMetricsListener, metricsRegistry, metricsRegistry, storageIntegrationTestInfo.CosmosClient(), clusterServiceMockInfo.MockClusterServiceClient, fakeAuditClient, "fake-location", "", false, false, true) + aroHCPFrontend := frontend.NewFrontend(logger, frontendListener, frontendMetricsListener, metricsRegistry, metricsRegistry, storageIntegrationTestInfo.CosmosClient(), clusterServiceMockInfo.MockClusterServiceClient, fakeAuditClient, "fake-location", true) // admin api setup adminListener, err := net.Listen("tcp4", "127.0.0.1:0")