From 9fdaef42a0663c4aa5d5cd32e40a46627c1d79fb Mon Sep 17 00:00:00 2001 From: David Eads Date: Tue, 17 Feb 2026 17:58:20 -0500 Subject: [PATCH 1/3] fire cluster watching controller when serviceprovidercluster changes --- backend/pkg/app/backend.go | 13 ++-- .../cluster_watching_controller.go | 67 +++++++++++++++---- .../control_plane_version_controller.go | 7 +- ...rigger_control_plane_upgrade_controller.go | 7 +- .../cluster_validation_controller.go | 7 +- .../backend/launch/notification_test.go | 5 +- 6 files changed, 70 insertions(+), 36 deletions(-) diff --git a/backend/pkg/app/backend.go b/backend/pkg/app/backend.go index 29fe585ebf..e18724230f 100644 --- a/backend/pkg/app/backend.go +++ b/backend/pkg/app/backend.go @@ -238,13 +238,12 @@ func (b *Backend) runBackendControllersUnderLeaderElection(ctx context.Context, _, subscriptionLister := backendInformers.Subscriptions() activeOperationInformer, activeOperationLister := backendInformers.ActiveOperations() - clusterInformer, _ := backendInformers.Clusters() startedLeading := atomic.Bool{} operationsScanner := oldoperationscanner.NewOperationsScanner( b.options.CosmosDBClient, b.options.ClustersServiceClient, b.options.AzureLocation, subscriptionLister) dataDumpController := controllerutils.NewClusterWatchingController( - "DataDump", b.options.CosmosDBClient, clusterInformer, 1*time.Minute, controllers.NewDataDumpController(activeOperationLister, b.options.CosmosDBClient)) + "DataDump", b.options.CosmosDBClient, backendInformers, 1*time.Minute, controllers.NewDataDumpController(activeOperationLister, b.options.CosmosDBClient)) doNothingController := controllers.NewDoNothingExampleController(b.options.CosmosDBClient, subscriptionLister) operationClusterCreateController := operationcontrollers.NewGenericOperationController( "OperationClusterCreate", @@ -304,26 +303,26 @@ func (b *Backend) runBackendControllersUnderLeaderElection(ctx context.Context, ) clusterServiceMatchingClusterController := mismatchcontrollers.NewClusterServiceClusterMatchingController(b.options.CosmosDBClient, subscriptionLister, b.options.ClustersServiceClient) cosmosMatchingNodePoolController := controllerutils.NewClusterWatchingController( - "CosmosMatchingNodePools", b.options.CosmosDBClient, clusterInformer, 60*time.Minute, + "CosmosMatchingNodePools", b.options.CosmosDBClient, backendInformers, 60*time.Minute, mismatchcontrollers.NewCosmosNodePoolMatchingController(b.options.CosmosDBClient, b.options.ClustersServiceClient)) cosmosMatchingExternalAuthController := controllerutils.NewClusterWatchingController( - "CosmosMatchingExternalAuths", b.options.CosmosDBClient, clusterInformer, 60*time.Minute, + "CosmosMatchingExternalAuths", b.options.CosmosDBClient, backendInformers, 60*time.Minute, mismatchcontrollers.NewCosmosExternalAuthMatchingController(b.options.CosmosDBClient, b.options.ClustersServiceClient)) cosmosMatchingClusterController := controllerutils.NewClusterWatchingController( - "CosmosMatchingClusters", b.options.CosmosDBClient, clusterInformer, 60*time.Minute, + "CosmosMatchingClusters", b.options.CosmosDBClient, backendInformers, 60*time.Minute, mismatchcontrollers.NewCosmosClusterMatchingController(utilsclock.RealClock{}, b.options.CosmosDBClient, b.options.ClustersServiceClient)) alwaysSuccessClusterValidationController := validationcontrollers.NewClusterValidationController( validations.NewAlwaysSuccessValidation(), activeOperationLister, b.options.CosmosDBClient, - clusterInformer, + backendInformers, ) deleteOrphanedCosmosResourcesController := mismatchcontrollers.NewDeleteOrphanedCosmosResourcesController(b.options.CosmosDBClient, subscriptionLister) triggerControlPlaneUpgradeController := upgradecontrollers.NewTriggerControlPlaneUpgradeController( b.options.CosmosDBClient, b.options.ClustersServiceClient, activeOperationLister, - clusterInformer, + backendInformers, ) le, err := leaderelection.NewLeaderElector(leaderelection.LeaderElectionConfig{ diff --git a/backend/pkg/controllers/controllerutils/cluster_watching_controller.go b/backend/pkg/controllers/controllerutils/cluster_watching_controller.go index 9ab950399f..250e70d2ea 100644 --- a/backend/pkg/controllers/controllerutils/cluster_watching_controller.go +++ b/backend/pkg/controllers/controllerutils/cluster_watching_controller.go @@ -27,9 +27,13 @@ import ( "k8s.io/client-go/util/workqueue" "k8s.io/utils/ptr" + azcorearm "github.com/Azure/azure-sdk-for-go/sdk/azcore/arm" + + "github.com/Azure/ARO-HCP/backend/pkg/informers" "github.com/Azure/ARO-HCP/internal/api" "github.com/Azure/ARO-HCP/internal/database" "github.com/Azure/ARO-HCP/internal/utils" + "github.com/Azure/ARO-HCP/internal/utils/apihelpers" ) type ClusterSyncer interface { @@ -56,7 +60,7 @@ type clusterWatchingController struct { func NewClusterWatchingController( name string, cosmosClient database.DBClient, - clusterInformer cache.SharedIndexInformer, + informers informers.BackendInformers, resyncDuration time.Duration, syncer ClusterSyncer, ) Controller { @@ -73,11 +77,24 @@ func NewClusterWatchingController( } // this happens when unit tests don't want triggering. This isn't beautiful, but fails to do nothing which is pretty safe. - if clusterInformer != nil { + if informers != nil { + clusterInformer, _ := informers.Clusters() _, err := clusterInformer.AddEventHandlerWithOptions( cache.ResourceEventHandlerFuncs{ - AddFunc: c.enqueueAdd, - UpdateFunc: c.enqueueUpdate, + AddFunc: c.enqueueClusterAdd, + UpdateFunc: c.enqueueClusterUpdate, + }, + cache.HandlerOptions{ + ResyncPeriod: ptr.To(resyncDuration), + }) + if err != nil { + panic(err) // coding error + } + serviceProviderInformer, _ := informers.ServiceProviderClusters() + _, err = serviceProviderInformer.AddEventHandlerWithOptions( + cache.ResourceEventHandlerFuncs{ + AddFunc: c.enqueueServiceProviderClusterAdd, + UpdateFunc: c.enqueueServiceProviderClusterUpdate, }, cache.HandlerOptions{ ResyncPeriod: ptr.To(resyncDuration), @@ -166,17 +183,26 @@ func (c *clusterWatchingController) processNextWorkItem(ctx context.Context) boo return true } -func (c *clusterWatchingController) enqueueAdd(newObj interface{}) { - logger := utils.DefaultLogger() - logger = logger.WithValues(utils.LogValues{}.AddControllerName(c.name)...) - ctx := logr.NewContext(context.TODO(), logger) +// EnqueueResourceIDAdd traverses to find a resourceID that is an hcpcluster and adds it if found. +// It is exposed so that individual controllers can add other items to requeue based on easily. +func (c *clusterWatchingController) EnqueueResourceIDAdd(resourceID *azcorearm.ResourceID) { + if resourceID == nil { + return + } + if !apihelpers.ResourceTypeEqual(resourceID.ResourceType, api.ClusterResourceType) { + c.EnqueueResourceIDAdd(resourceID.Parent) + return + } - castObj := newObj.(*api.HCPOpenShiftCluster) key := HCPClusterKey{ - SubscriptionID: castObj.ID.SubscriptionID, - ResourceGroupName: castObj.ID.ResourceGroupName, - HCPClusterName: castObj.ID.Name, + SubscriptionID: resourceID.SubscriptionID, + ResourceGroupName: resourceID.ResourceGroupName, + HCPClusterName: resourceID.Name, } + + logger := utils.DefaultLogger() + logger = logger.WithValues(utils.LogValues{}.AddControllerName(c.name)...) + ctx := logr.NewContext(context.TODO(), logger) logger = key.AddLoggerValues(logger) ctx = logr.NewContext(ctx, logger) @@ -187,6 +213,19 @@ func (c *clusterWatchingController) enqueueAdd(newObj interface{}) { c.queue.Add(key) } -func (c *clusterWatchingController) enqueueUpdate(_ interface{}, newObj interface{}) { - c.enqueueAdd(newObj) +// TODO once these share common metadata, recollapse +func (c *clusterWatchingController) enqueueClusterAdd(newObj interface{}) { + c.EnqueueResourceIDAdd(newObj.(*api.HCPOpenShiftCluster).ID) +} + +func (c *clusterWatchingController) enqueueClusterUpdate(_ interface{}, newObj interface{}) { + c.EnqueueResourceIDAdd(newObj.(*api.HCPOpenShiftCluster).ID) +} + +func (c *clusterWatchingController) enqueueServiceProviderClusterAdd(newObj interface{}) { + c.EnqueueResourceIDAdd(newObj.(*api.ServiceProviderCluster).CosmosMetadata.ResourceID) +} + +func (c *clusterWatchingController) enqueueServiceProviderClusterUpdate(_ interface{}, newObj interface{}) { + c.EnqueueResourceIDAdd(newObj.(*api.ServiceProviderCluster).CosmosMetadata.ResourceID) } diff --git a/backend/pkg/controllers/upgradecontrollers/control_plane_version_controller.go b/backend/pkg/controllers/upgradecontrollers/control_plane_version_controller.go index 6d5824ae3d..69ab14510b 100644 --- a/backend/pkg/controllers/upgradecontrollers/control_plane_version_controller.go +++ b/backend/pkg/controllers/upgradecontrollers/control_plane_version_controller.go @@ -26,11 +26,10 @@ import ( "github.com/golang/groupcache/lru" "github.com/google/uuid" - "k8s.io/client-go/tools/cache" - "github.com/openshift/cluster-version-operator/pkg/cincinnati" "github.com/Azure/ARO-HCP/backend/pkg/controllers/controllerutils" + "github.com/Azure/ARO-HCP/backend/pkg/informers" "github.com/Azure/ARO-HCP/backend/pkg/listers" "github.com/Azure/ARO-HCP/internal/api" "github.com/Azure/ARO-HCP/internal/cincinatti" @@ -60,7 +59,7 @@ func NewControlPlaneVersionController( cosmosClient database.DBClient, clusterServiceClient ocm.ClusterServiceClientSpec, activeOperationLister listers.ActiveOperationLister, - clusterInformer cache.SharedIndexInformer, + informers informers.BackendInformers, ) controllerutils.Controller { syncer := &controlPlaneVersionSyncer{ @@ -73,7 +72,7 @@ func NewControlPlaneVersionController( controller := controllerutils.NewClusterWatchingController( "ControlPlaneVersion", cosmosClient, - clusterInformer, + informers, 5*time.Minute, // Check for upgrades every 5 minutes syncer, ) 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 23bb3f659d..8c15043d13 100644 --- a/backend/pkg/controllers/upgradecontrollers/trigger_control_plane_upgrade_controller.go +++ b/backend/pkg/controllers/upgradecontrollers/trigger_control_plane_upgrade_controller.go @@ -22,11 +22,10 @@ import ( "github.com/blang/semver/v4" - "k8s.io/client-go/tools/cache" - arohcpv1alpha1 "github.com/openshift-online/ocm-sdk-go/arohcp/v1alpha1" "github.com/Azure/ARO-HCP/backend/pkg/controllers/controllerutils" + "github.com/Azure/ARO-HCP/backend/pkg/informers" "github.com/Azure/ARO-HCP/backend/pkg/listers" "github.com/Azure/ARO-HCP/internal/api" "github.com/Azure/ARO-HCP/internal/database" @@ -54,7 +53,7 @@ func NewTriggerControlPlaneUpgradeController( cosmosClient database.DBClient, clusterServiceClient ocm.ClusterServiceClientSpec, activeOperationLister listers.ActiveOperationLister, - clusterInformer cache.SharedIndexInformer, + informers informers.BackendInformers, ) controllerutils.Controller { syncer := &triggerControlPlaneUpgradeSyncer{ cooldownChecker: controllerutils.DefaultActiveOperationPrioritizingCooldown(activeOperationLister), @@ -65,7 +64,7 @@ func NewTriggerControlPlaneUpgradeController( controller := controllerutils.NewClusterWatchingController( "TriggerControlPlaneUpgrade", cosmosClient, - clusterInformer, + informers, 5*time.Minute, syncer, ) diff --git a/backend/pkg/controllers/validationcontrollers/cluster_validation_controller.go b/backend/pkg/controllers/validationcontrollers/cluster_validation_controller.go index 65b1fb1197..09026fbb42 100644 --- a/backend/pkg/controllers/validationcontrollers/cluster_validation_controller.go +++ b/backend/pkg/controllers/validationcontrollers/cluster_validation_controller.go @@ -20,10 +20,9 @@ import ( "net/http" "time" - "k8s.io/client-go/tools/cache" - "github.com/Azure/ARO-HCP/backend/pkg/controllers/controllerutils" "github.com/Azure/ARO-HCP/backend/pkg/controllers/validationcontrollers/validations" + "github.com/Azure/ARO-HCP/backend/pkg/informers" "github.com/Azure/ARO-HCP/backend/pkg/listers" "github.com/Azure/ARO-HCP/internal/api" "github.com/Azure/ARO-HCP/internal/database" @@ -48,7 +47,7 @@ func NewClusterValidationController( validation validations.ClusterValidation, activeOperationLister listers.ActiveOperationLister, cosmosClient database.DBClient, - clusterInformer cache.SharedIndexInformer, + informers informers.BackendInformers, ) controllerutils.Controller { syncer := &clusterValidationSyncer{ @@ -60,7 +59,7 @@ func NewClusterValidationController( controller := controllerutils.NewClusterWatchingController( fmt.Sprintf("ClusterValidation%s", validation.Name()), cosmosClient, - clusterInformer, + informers, 1*time.Minute, syncer, ) diff --git a/test-integration/backend/launch/notification_test.go b/test-integration/backend/launch/notification_test.go index 25201c36a4..62d7470007 100644 --- a/test-integration/backend/launch/notification_test.go +++ b/test-integration/backend/launch/notification_test.go @@ -83,10 +83,9 @@ func TestControllerNotifications(t *testing.T) { backendInformers := informers.NewBackendInformersWithRelistDuration(ctx, cosmosClient.GlobalListers(), ptr.To(100*time.Millisecond)) _, activeOperationLister := backendInformers.ActiveOperations() - clusterInformer, _ := backendInformers.Clusters() testSyncer := newTestController(activeOperationLister) testingController := controllerutils.NewClusterWatchingController( - "TestingController", cosmosClient, clusterInformer, 1*time.Minute, testSyncer) + "TestingController", cosmosClient, backendInformers, 1*time.Minute, testSyncer) go func() { backendStarted.Store(true) @@ -116,7 +115,7 @@ func TestControllerNotifications(t *testing.T) { case <-ctx.Done(): } - require.Equal(t, testSyncer.count.Load(), int32(1), "missing sync") + require.Equal(t, int32(1), testSyncer.count.Load(), "missing sync") testSyncer.observedKeys.Range(func(key, value interface{}) bool { t.Log(key) From aae15abe778597c35c959edb8d04b544eebaad72 Mon Sep 17 00:00:00 2001 From: David Eads Date: Fri, 20 Feb 2026 10:44:37 -0500 Subject: [PATCH 2/3] Create controller to pull serviceprovider data into cosmos from cluster-service We will use this for the read path of frontend since the information is already asynchronous, so some delay is ok. We must get the backend change to prod first though, becuase otherwise we can have a frontend updated and backend not updated and be unable to ever get these values. --- backend/pkg/app/backend.go | 15 ++ .../cluster_properties_sync.go | 139 ++++++++++ .../cluster_properties_sync_test.go | 245 ++++++++++++++++++ 3 files changed, 399 insertions(+) create mode 100644 backend/pkg/controllers/clusterpropertiescontroller/cluster_properties_sync.go create mode 100644 backend/pkg/controllers/clusterpropertiescontroller/cluster_properties_sync_test.go diff --git a/backend/pkg/app/backend.go b/backend/pkg/app/backend.go index e18724230f..0161b9ba07 100644 --- a/backend/pkg/app/backend.go +++ b/backend/pkg/app/backend.go @@ -33,6 +33,7 @@ import ( "github.com/Azure/ARO-HCP/backend/oldoperationscanner" "github.com/Azure/ARO-HCP/backend/pkg/controllers" + "github.com/Azure/ARO-HCP/backend/pkg/controllers/clusterpropertiescontroller" "github.com/Azure/ARO-HCP/backend/pkg/controllers/controllerutils" "github.com/Azure/ARO-HCP/backend/pkg/controllers/mismatchcontrollers" "github.com/Azure/ARO-HCP/backend/pkg/controllers/operationcontrollers" @@ -318,12 +319,24 @@ func (b *Backend) runBackendControllersUnderLeaderElection(ctx context.Context, backendInformers, ) deleteOrphanedCosmosResourcesController := mismatchcontrollers.NewDeleteOrphanedCosmosResourcesController(b.options.CosmosDBClient, subscriptionLister) + controlPlaneVersionController := upgradecontrollers.NewControlPlaneVersionController( + b.options.CosmosDBClient, + b.options.ClustersServiceClient, + activeOperationLister, + clusterInformer, + ) triggerControlPlaneUpgradeController := upgradecontrollers.NewTriggerControlPlaneUpgradeController( b.options.CosmosDBClient, b.options.ClustersServiceClient, activeOperationLister, backendInformers, ) + clusterPropertiesSyncController := clusterpropertiescontroller.NewClusterPropertiesSyncController( + b.options.CosmosDBClient, + b.options.ClustersServiceClient, + activeOperationLister, + clusterInformer, + ) le, err := leaderelection.NewLeaderElector(leaderelection.LeaderElectionConfig{ Lock: b.options.LeaderElectionLock, @@ -352,7 +365,9 @@ func (b *Backend) runBackendControllersUnderLeaderElection(ctx context.Context, go cosmosMatchingClusterController.Run(ctx, 20) go alwaysSuccessClusterValidationController.Run(ctx, 20) go deleteOrphanedCosmosResourcesController.Run(ctx, 20) + go controlPlaneVersionController.Run(ctx, 20) go triggerControlPlaneUpgradeController.Run(ctx, 20) + go clusterPropertiesSyncController.Run(ctx, 20) }, OnStoppedLeading: func() { operationsScanner.LeaderGauge.Set(0) diff --git a/backend/pkg/controllers/clusterpropertiescontroller/cluster_properties_sync.go b/backend/pkg/controllers/clusterpropertiescontroller/cluster_properties_sync.go new file mode 100644 index 0000000000..a756668fb0 --- /dev/null +++ b/backend/pkg/controllers/clusterpropertiescontroller/cluster_properties_sync.go @@ -0,0 +1,139 @@ +// 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 clusterpropertiescontroller + +import ( + "context" + "fmt" + "net/http" + "time" + + "k8s.io/apimachinery/pkg/api/equality" + "k8s.io/client-go/tools/cache" + + "github.com/Azure/ARO-HCP/backend/pkg/controllers/controllerutils" + "github.com/Azure/ARO-HCP/backend/pkg/listers" + "github.com/Azure/ARO-HCP/internal/database" + "github.com/Azure/ARO-HCP/internal/ocm" + "github.com/Azure/ARO-HCP/internal/utils" +) + +// clusterPropertiesSyncer is a Cluster syncer that synchronizes cluster properties +// from Cluster Service to Cosmos DB. It ensures that the following fields are populated: +// - ServiceProviderProperties.Console.URL +// - ServiceProviderProperties.DNS.BaseDomain +// - CustomerProperties.DNS.BaseDomainPrefix +type clusterPropertiesSyncer struct { + cooldownChecker controllerutils.CooldownChecker + cosmosClient database.DBClient + clusterServiceClient ocm.ClusterServiceClientSpec +} + +var _ controllerutils.ClusterSyncer = (*clusterPropertiesSyncer)(nil) + +// NewClusterPropertiesSyncController creates a new controller that synchronizes +// cluster properties from Cluster Service to Cosmos DB. +// It periodically checks each cluster and populates the Console.URL, DNS.BaseDomain, +// and DNS.BaseDomainPrefix fields if they are not set. +func NewClusterPropertiesSyncController( + cosmosClient database.DBClient, + clusterServiceClient ocm.ClusterServiceClientSpec, + activeOperationLister listers.ActiveOperationLister, + clusterInformer cache.SharedIndexInformer, +) controllerutils.Controller { + syncer := &clusterPropertiesSyncer{ + cooldownChecker: controllerutils.DefaultActiveOperationPrioritizingCooldown(activeOperationLister), + cosmosClient: cosmosClient, + clusterServiceClient: clusterServiceClient, + } + + controller := controllerutils.NewClusterWatchingController( + "ClusterPropertiesSync", + cosmosClient, + clusterInformer, + 5*time.Minute, // Check every 5 minutes + syncer, + ) + + return controller +} + +func (c *clusterPropertiesSyncer) CooldownChecker() controllerutils.CooldownChecker { + return c.cooldownChecker +} + +// SyncOnce performs a single reconciliation of cluster properties. +// It checks if the Console.URL, DNS.BaseDomain, or DNS.BaseDomainPrefix fields +// are unset, and if so, fetches the values from Cluster Service and updates Cosmos. +func (c *clusterPropertiesSyncer) SyncOnce(ctx context.Context, key controllerutils.HCPClusterKey) error { + logger := utils.LoggerFromContext(ctx) + + // Get the cluster from Cosmos + clusterCRUD := c.cosmosClient.HCPClusters(key.SubscriptionID, key.ResourceGroupName) + existingCluster, err := clusterCRUD.Get(ctx, key.HCPClusterName) + if database.IsResponseError(err, http.StatusNotFound) { + return nil // cluster doesn't exist, no work to do + } + if err != nil { + return utils.TrackError(fmt.Errorf("failed to get Cluster: %w", err)) + } + + // Check if we have a cluster service ID to query + if len(existingCluster.ServiceProviderProperties.ClusterServiceID.String()) == 0 { + return nil + } + + // Check if any of the properties need to be synced + needsConsoleURL := len(existingCluster.ServiceProviderProperties.Console.URL) == 0 + needsBaseDomain := len(existingCluster.ServiceProviderProperties.DNS.BaseDomain) == 0 + needsBaseDomainPrefix := len(existingCluster.CustomerProperties.DNS.BaseDomainPrefix) == 0 + + if !needsConsoleURL && !needsBaseDomain && !needsBaseDomainPrefix { + return nil + } + + // Fetch the cluster from Cluster Service + 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)) + } + + // Take a copy before making changes for comparison + originalCluster := existingCluster.DeepCopy() + + // Update the properties if they are not set + if needsConsoleURL { + existingCluster.ServiceProviderProperties.Console.URL = csCluster.Console().URL() + } + if needsBaseDomain { + existingCluster.ServiceProviderProperties.DNS.BaseDomain = csCluster.DNS().BaseDomain() + } + if needsBaseDomainPrefix { + existingCluster.CustomerProperties.DNS.BaseDomainPrefix = csCluster.DomainPrefix() + } + + // Only write back if something actually changed + if equality.Semantic.DeepEqual(originalCluster, existingCluster) { + return nil + } + + // Write the updated cluster back to Cosmos + if _, err := clusterCRUD.Replace(ctx, existingCluster, nil); err != nil { + return utils.TrackError(fmt.Errorf("failed to replace Cluster: %w", err)) + } + + logger.Info("synced cluster properties from Cluster Service") + return nil +} diff --git a/backend/pkg/controllers/clusterpropertiescontroller/cluster_properties_sync_test.go b/backend/pkg/controllers/clusterpropertiescontroller/cluster_properties_sync_test.go new file mode 100644 index 0000000000..627eee0df8 --- /dev/null +++ b/backend/pkg/controllers/clusterpropertiescontroller/cluster_properties_sync_test.go @@ -0,0 +1,245 @@ +// 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 clusterpropertiescontroller + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" + + azcorearm "github.com/Azure/azure-sdk-for-go/sdk/azcore/arm" + + arohcpv1alpha1 "github.com/openshift-online/ocm-sdk-go/arohcp/v1alpha1" + + "github.com/Azure/ARO-HCP/backend/pkg/controllers/controllerutils" + "github.com/Azure/ARO-HCP/internal/api" + "github.com/Azure/ARO-HCP/internal/api/arm" + "github.com/Azure/ARO-HCP/internal/databasetesting" + "github.com/Azure/ARO-HCP/internal/ocm" +) + +const ( + testSubscriptionID = "00000000-0000-0000-0000-000000000000" + testResourceGroupName = "test-rg" + testClusterName = "test-cluster" + testClusterServiceIDStr = "/api/clusters_mgmt/v1/clusters/abc123" + + testConsoleURL = "https://console.example.com" + testBaseDomain = "example.openshiftapps.com" + testBaseDomainPrefix = "my-cluster" +) + +func TestClusterPropertiesSyncer_SyncOnce(t *testing.T) { + testCases := []struct { + name string + existingCluster *api.HCPOpenShiftCluster + csCluster *arohcpv1alpha1.Cluster + expectCSCall bool + expectCosmosUpdate bool + expectedConsoleURL string + expectedBaseDomain string + expectedBaseDomainPrefix string + }{ + { + name: "short-circuit when all properties already set", + existingCluster: newTestCluster(func(c *api.HCPOpenShiftCluster) { + c.ServiceProviderProperties.Console.URL = testConsoleURL + c.ServiceProviderProperties.DNS.BaseDomain = testBaseDomain + c.CustomerProperties.DNS.BaseDomainPrefix = testBaseDomainPrefix + }), + expectCSCall: false, + expectCosmosUpdate: false, + expectedConsoleURL: testConsoleURL, + expectedBaseDomain: testBaseDomain, + expectedBaseDomainPrefix: testBaseDomainPrefix, + }, + { + name: "sync all properties from CS when all are missing", + existingCluster: newTestCluster(), + csCluster: buildCSCluster( + testConsoleURL, + testBaseDomain, + testBaseDomainPrefix, + ), + expectCSCall: true, + expectCosmosUpdate: true, + expectedConsoleURL: testConsoleURL, + expectedBaseDomain: testBaseDomain, + expectedBaseDomainPrefix: testBaseDomainPrefix, + }, + { + name: "sync only missing Console.URL", + existingCluster: newTestCluster(func(c *api.HCPOpenShiftCluster) { + c.ServiceProviderProperties.DNS.BaseDomain = testBaseDomain + c.CustomerProperties.DNS.BaseDomainPrefix = testBaseDomainPrefix + }), + csCluster: buildCSCluster( + testConsoleURL, + testBaseDomain, + testBaseDomainPrefix, + ), + expectCSCall: true, + expectCosmosUpdate: true, + expectedConsoleURL: testConsoleURL, + expectedBaseDomain: testBaseDomain, + expectedBaseDomainPrefix: testBaseDomainPrefix, + }, + { + name: "sync only missing DNS.BaseDomain", + existingCluster: newTestCluster(func(c *api.HCPOpenShiftCluster) { + c.ServiceProviderProperties.Console.URL = testConsoleURL + c.CustomerProperties.DNS.BaseDomainPrefix = testBaseDomainPrefix + }), + csCluster: buildCSCluster( + testConsoleURL, + testBaseDomain, + testBaseDomainPrefix, + ), + expectCSCall: true, + expectCosmosUpdate: true, + expectedConsoleURL: testConsoleURL, + expectedBaseDomain: testBaseDomain, + expectedBaseDomainPrefix: testBaseDomainPrefix, + }, + { + name: "sync only missing DNS.BaseDomainPrefix", + existingCluster: newTestCluster(func(c *api.HCPOpenShiftCluster) { + c.ServiceProviderProperties.Console.URL = testConsoleURL + c.ServiceProviderProperties.DNS.BaseDomain = testBaseDomain + }), + csCluster: buildCSCluster( + testConsoleURL, + testBaseDomain, + testBaseDomainPrefix, + ), + expectCSCall: true, + expectCosmosUpdate: true, + expectedConsoleURL: testConsoleURL, + expectedBaseDomain: testBaseDomain, + expectedBaseDomainPrefix: testBaseDomainPrefix, + }, + { + name: "no update when CS returns empty values", + existingCluster: newTestCluster(), + csCluster: buildCSCluster("", "", ""), + expectCSCall: true, + expectCosmosUpdate: false, + expectedConsoleURL: "", + expectedBaseDomain: "", + expectedBaseDomainPrefix: "", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + ctx := context.Background() + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + // Setup mock DB + mockDB := databasetesting.NewMockDBClient() + + // Create the cluster in the mock DB + clusterCRUD := mockDB.HCPClusters(testSubscriptionID, testResourceGroupName) + _, err := clusterCRUD.Create(ctx, tc.existingCluster, nil) + require.NoError(t, err) + + // Setup mock CS client + mockCSClient := ocm.NewMockClusterServiceClientSpec(ctrl) + + if tc.expectCSCall { + mockCSClient.EXPECT(). + GetCluster(gomock.Any(), api.Must(api.NewInternalID(testClusterServiceIDStr))). + Return(tc.csCluster, nil) + } + + // Create syncer + syncer := &clusterPropertiesSyncer{ + cooldownChecker: &alwaysSyncCooldownChecker{}, + cosmosClient: mockDB, + clusterServiceClient: mockCSClient, + } + + // Execute + key := controllerutils.HCPClusterKey{ + SubscriptionID: testSubscriptionID, + ResourceGroupName: testResourceGroupName, + HCPClusterName: testClusterName, + } + err = syncer.SyncOnce(ctx, key) + require.NoError(t, err) + + // Verify the cluster state in Cosmos + updatedCluster, err := clusterCRUD.Get(ctx, testClusterName) + require.NoError(t, err) + + assert.Equal(t, tc.expectedConsoleURL, updatedCluster.ServiceProviderProperties.Console.URL) + assert.Equal(t, tc.expectedBaseDomain, updatedCluster.ServiceProviderProperties.DNS.BaseDomain) + assert.Equal(t, tc.expectedBaseDomainPrefix, updatedCluster.CustomerProperties.DNS.BaseDomainPrefix) + }) + } +} + +// newTestCluster creates a test HCPOpenShiftCluster with default values. +// Options can be provided to customize the cluster. +func newTestCluster(opts ...func(*api.HCPOpenShiftCluster)) *api.HCPOpenShiftCluster { + resourceID := api.Must(azcorearm.ParseResourceID( + "/subscriptions/" + testSubscriptionID + + "/resourceGroups/" + testResourceGroupName + + "/providers/Microsoft.RedHatOpenShift/hcpOpenShiftClusters/" + testClusterName, + )) + + cluster := &api.HCPOpenShiftCluster{ + TrackedResource: arm.TrackedResource{ + Resource: arm.Resource{ + ID: resourceID, + Name: testClusterName, + Type: resourceID.ResourceType.String(), + }, + }, + ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{ + ClusterServiceID: api.Must(api.NewInternalID(testClusterServiceIDStr)), + }, + } + + for _, opt := range opts { + opt(cluster) + } + + return cluster +} + +// buildCSCluster creates a mock Cluster Service cluster with the given values. +func buildCSCluster(consoleURL, baseDomain, domainPrefix string) *arohcpv1alpha1.Cluster { + cluster, err := arohcpv1alpha1.NewCluster(). + Console(arohcpv1alpha1.NewClusterConsole().URL(consoleURL)). + DNS(arohcpv1alpha1.NewDNS().BaseDomain(baseDomain)). + DomainPrefix(domainPrefix). + Build() + if err != nil { + panic(err) + } + return cluster +} + +// alwaysSyncCooldownChecker always allows syncing +type alwaysSyncCooldownChecker struct{} + +func (c *alwaysSyncCooldownChecker) CanSync(ctx context.Context, key any) bool { + return true +} From 5702af3e8e3af1de0bdf3bdffd21e229ad7a2381 Mon Sep 17 00:00:00 2001 From: David Eads Date: Tue, 24 Feb 2026 15:23:36 -0500 Subject: [PATCH 3/3] adjust to new backend informers --- backend/pkg/app/backend.go | 4 ++-- .../clusterpropertiescontroller/cluster_properties_sync.go | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/backend/pkg/app/backend.go b/backend/pkg/app/backend.go index 0161b9ba07..6c1476f1a5 100644 --- a/backend/pkg/app/backend.go +++ b/backend/pkg/app/backend.go @@ -323,7 +323,7 @@ func (b *Backend) runBackendControllersUnderLeaderElection(ctx context.Context, b.options.CosmosDBClient, b.options.ClustersServiceClient, activeOperationLister, - clusterInformer, + backendInformers, ) triggerControlPlaneUpgradeController := upgradecontrollers.NewTriggerControlPlaneUpgradeController( b.options.CosmosDBClient, @@ -335,7 +335,7 @@ func (b *Backend) runBackendControllersUnderLeaderElection(ctx context.Context, b.options.CosmosDBClient, b.options.ClustersServiceClient, activeOperationLister, - clusterInformer, + backendInformers, ) le, err := leaderelection.NewLeaderElector(leaderelection.LeaderElectionConfig{ diff --git a/backend/pkg/controllers/clusterpropertiescontroller/cluster_properties_sync.go b/backend/pkg/controllers/clusterpropertiescontroller/cluster_properties_sync.go index a756668fb0..30bbca38b6 100644 --- a/backend/pkg/controllers/clusterpropertiescontroller/cluster_properties_sync.go +++ b/backend/pkg/controllers/clusterpropertiescontroller/cluster_properties_sync.go @@ -21,9 +21,9 @@ import ( "time" "k8s.io/apimachinery/pkg/api/equality" - "k8s.io/client-go/tools/cache" "github.com/Azure/ARO-HCP/backend/pkg/controllers/controllerutils" + "github.com/Azure/ARO-HCP/backend/pkg/informers" "github.com/Azure/ARO-HCP/backend/pkg/listers" "github.com/Azure/ARO-HCP/internal/database" "github.com/Azure/ARO-HCP/internal/ocm" @@ -51,7 +51,7 @@ func NewClusterPropertiesSyncController( cosmosClient database.DBClient, clusterServiceClient ocm.ClusterServiceClientSpec, activeOperationLister listers.ActiveOperationLister, - clusterInformer cache.SharedIndexInformer, + informers informers.BackendInformers, ) controllerutils.Controller { syncer := &clusterPropertiesSyncer{ cooldownChecker: controllerutils.DefaultActiveOperationPrioritizingCooldown(activeOperationLister), @@ -62,7 +62,7 @@ func NewClusterPropertiesSyncController( controller := controllerutils.NewClusterWatchingController( "ClusterPropertiesSync", cosmosClient, - clusterInformer, + informers, 5*time.Minute, // Check every 5 minutes syncer, )