diff --git a/pkg/controllers/configobservation/configobservercontroller/observe_config_controller.go b/pkg/controllers/configobservation/configobservercontroller/observe_config_controller.go index 76d77153f7..9abd2b2762 100644 --- a/pkg/controllers/configobservation/configobservercontroller/observe_config_controller.go +++ b/pkg/controllers/configobservation/configobservercontroller/observe_config_controller.go @@ -1,6 +1,7 @@ package configobservercontroller import ( + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/client-go/tools/cache" configinformers "github.com/openshift/client-go/config/informers/externalversions" @@ -24,6 +25,7 @@ func NewConfigObserver( kubeInformersForNamespaces v1helpers.KubeInformersForNamespaces, configInformer configinformers.SharedInformerFactory, resourceSyncer resourcesynccontroller.ResourceSyncer, + enabledClusterCapabilities sets.String, eventRecorder events.Recorder, ) factory.Controller { interestingNamespaces := []string{ @@ -35,19 +37,19 @@ func NewConfigObserver( preRunCacheSynced := []cache.InformerSynced{ operatorClient.Informer().HasSynced, configInformer.Config().V1().APIServers().Informer().HasSynced, - configInformer.Config().V1().Consoles().Informer().HasSynced, configInformer.Config().V1().Infrastructures().Informer().HasSynced, configInformer.Config().V1().OAuths().Informer().HasSynced, configInformer.Config().V1().Ingresses().Informer().HasSynced, + configInformer.Config().V1().ClusterVersions().Informer().HasSynced, } informers := []factory.Informer{ operatorClient.Informer(), configInformer.Config().V1().APIServers().Informer(), - configInformer.Config().V1().Consoles().Informer(), configInformer.Config().V1().Infrastructures().Informer(), configInformer.Config().V1().OAuths().Informer(), configInformer.Config().V1().Ingresses().Informer(), + configInformer.Config().V1().ClusterVersions().Informer(), } for _, ns := range interestingNamespaces { @@ -79,21 +81,30 @@ func NewConfigObserver( configobserver.WithPrefix(o, configobservation.OAuthServerConfigPrefix)) } + listers := configobservation.Listers{ + ConfigMapLister: kubeInformersForNamespaces.ConfigMapLister(), + SecretsLister: kubeInformersForNamespaces.SecretLister(), + IngressLister: configInformer.Config().V1().Ingresses().Lister(), + + APIServerLister_: configInformer.Config().V1().APIServers().Lister(), + ClusterVersionLister: configInformer.Config().V1().ClusterVersions().Lister(), + InfrastructureLister: configInformer.Config().V1().Infrastructures().Lister(), + OAuthLister_: configInformer.Config().V1().OAuths().Lister(), + ResourceSync: resourceSyncer, + PreRunCachesSynced: preRunCacheSynced, + } + + // Check if the Console capability is enabled on the cluster and sync and add its informer and lister. + if enabledClusterCapabilities.Has("Console") { + preRunCacheSynced = append(preRunCacheSynced, configInformer.Config().V1().Consoles().Informer().HasSynced) + informers = append(informers, configInformer.Config().V1().Consoles().Informer()) + listers.ConsoleLister = configInformer.Config().V1().Consoles().Lister() + } + return configobserver.NewNestedConfigObserver( operatorClient, eventRecorder, - configobservation.Listers{ - ConfigMapLister: kubeInformersForNamespaces.ConfigMapLister(), - SecretsLister: kubeInformersForNamespaces.SecretLister(), - IngressLister: configInformer.Config().V1().Ingresses().Lister(), - - APIServerLister_: configInformer.Config().V1().APIServers().Lister(), - ConsoleLister: configInformer.Config().V1().Consoles().Lister(), - InfrastructureLister: configInformer.Config().V1().Infrastructures().Lister(), - OAuthLister_: configInformer.Config().V1().OAuths().Lister(), - ResourceSync: resourceSyncer, - PreRunCachesSynced: preRunCacheSynced, - }, + listers, informers, []string{configobservation.OAuthServerConfigPrefix}, "OAuthServer", diff --git a/pkg/controllers/configobservation/console/observe_consoleurl.go b/pkg/controllers/configobservation/console/observe_consoleurl.go index 6464b21a35..140561a3fe 100644 --- a/pkg/controllers/configobservation/console/observe_consoleurl.go +++ b/pkg/controllers/configobservation/console/observe_consoleurl.go @@ -9,6 +9,8 @@ import ( "github.com/openshift/library-go/pkg/operator/configobserver" "github.com/openshift/library-go/pkg/operator/events" + configv1 "github.com/openshift/api/config/v1" + "github.com/openshift/cluster-authentication-operator/pkg/controllers/configobservation" ) @@ -20,12 +22,28 @@ func ObserveConsoleURL(genericlisters configobserver.Listers, recorder events.Re listers := genericlisters.(configobservation.Listers) errs := []error{} - consoleConfig, err := listers.ConsoleLister.Get("cluster") + clusterVersionConfig, err := listers.ClusterVersionLister.Get("version") if err != nil { return existingConfig, append(errs, err) } + isConsoleCapabilityEnabled := false + for _, capability := range clusterVersionConfig.Status.Capabilities.EnabledCapabilities { + if capability == configv1.ClusterVersionCapabilityConsole { + isConsoleCapabilityEnabled = true + break + } + } + if !isConsoleCapabilityEnabled { + return existingConfig, nil + } + + consoleConfig, err := listers.ConsoleLister.Get("cluster") + if err != nil { + return existingConfig, append(errs, err) + } observedAssetURL := consoleConfig.Status.ConsoleURL + if _, err := url.Parse(observedAssetURL); err != nil { // should never happen return existingConfig, append(errs, fmt.Errorf("failed to parse consoleURL %q: %w", observedAssetURL, err)) } diff --git a/pkg/controllers/configobservation/console/observe_consoleurl_test.go b/pkg/controllers/configobservation/console/observe_consoleurl_test.go index 07e85cd564..a2346eba93 100644 --- a/pkg/controllers/configobservation/console/observe_consoleurl_test.go +++ b/pkg/controllers/configobservation/console/observe_consoleurl_test.go @@ -17,31 +17,50 @@ import ( func TestObserveConsoleURL(t *testing.T) { existingConfig := configWithConsoleURL("https://teh.console.my") + noConfig := map[string]interface{}(nil) tests := []struct { name string consoleConfig *configv1.ConsoleStatus + clusterVersion *configv1.ClusterVersionStatus existingConfig map[string]interface{} expectedConfig map[string]interface{} expectedErrs []string expectedUpdateEvent bool }{ { - name: "NoConsoleConfig", + name: "NoConsoleConfigConsoleCapabilityEnabled", consoleConfig: nil, + clusterVersion: &configv1.ClusterVersionStatus{Capabilities: configv1.ClusterVersionCapabilitiesStatus{EnabledCapabilities: []configv1.ClusterVersionCapability{configv1.ClusterVersionCapabilityConsole}}}, existingConfig: existingConfig, expectedConfig: existingConfig, expectedErrs: []string{"\"cluster\" not found"}, }, + { + name: "NoConsoleConfigConsoleCapabilityDisabled", + consoleConfig: nil, + clusterVersion: &configv1.ClusterVersionStatus{Capabilities: configv1.ClusterVersionCapabilitiesStatus{EnabledCapabilities: []configv1.ClusterVersionCapability{}}}, + existingConfig: noConfig, + expectedConfig: noConfig, + }, + { + name: "ConsoleConfigConsoleCapabilityDisabled", + consoleConfig: &configv1.ConsoleStatus{ConsoleURL: "https://teh.console.my"}, + clusterVersion: &configv1.ClusterVersionStatus{Capabilities: configv1.ClusterVersionCapabilitiesStatus{EnabledCapabilities: []configv1.ClusterVersionCapability{}}}, + existingConfig: configWithConsoleURL(""), + expectedConfig: configWithConsoleURL(""), + }, { name: "SameConfig", consoleConfig: &configv1.ConsoleStatus{ConsoleURL: "https://teh.console.my"}, + clusterVersion: &configv1.ClusterVersionStatus{Capabilities: configv1.ClusterVersionCapabilitiesStatus{EnabledCapabilities: []configv1.ClusterVersionCapability{configv1.ClusterVersionCapabilityConsole}}}, existingConfig: existingConfig, expectedConfig: existingConfig, }, { name: "UpdatedConsoleConfig", consoleConfig: &configv1.ConsoleStatus{ConsoleURL: "https://my-new.console.url"}, + clusterVersion: &configv1.ClusterVersionStatus{Capabilities: configv1.ClusterVersionCapabilitiesStatus{EnabledCapabilities: []configv1.ClusterVersionCapability{configv1.ClusterVersionCapabilityConsole}}}, existingConfig: existingConfig, expectedConfig: configWithConsoleURL("https://my-new.console.url"), expectedUpdateEvent: true, @@ -49,6 +68,7 @@ func TestObserveConsoleURL(t *testing.T) { { name: "UnparsableConsoleURL", consoleConfig: &configv1.ConsoleStatus{ConsoleURL: "https://my-new.console.url:port"}, + clusterVersion: &configv1.ClusterVersionStatus{Capabilities: configv1.ClusterVersionCapabilitiesStatus{EnabledCapabilities: []configv1.ClusterVersionCapability{configv1.ClusterVersionCapabilityConsole}}}, existingConfig: existingConfig, expectedConfig: existingConfig, expectedErrs: []string{ @@ -58,9 +78,9 @@ func TestObserveConsoleURL(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - indexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{}) + consoleIndexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{}) if tt.consoleConfig != nil { - if err := indexer.Add(&configv1.Console{ + if err := consoleIndexer.Add(&configv1.Console{ ObjectMeta: metav1.ObjectMeta{ Name: "cluster", }, @@ -69,8 +89,20 @@ func TestObserveConsoleURL(t *testing.T) { t.Fatal(err) } } + clusterVersionIndexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{}) + if tt.clusterVersion != nil { + if err := clusterVersionIndexer.Add(&configv1.ClusterVersion{ + ObjectMeta: metav1.ObjectMeta{ + Name: "version", + }, + Status: *tt.clusterVersion, + }); err != nil { + t.Fatal(err) + } + } listers := configobservation.Listers{ - ConsoleLister: configlistersv1.NewConsoleLister(indexer), + ConsoleLister: configlistersv1.NewConsoleLister(consoleIndexer), + ClusterVersionLister: configlistersv1.NewClusterVersionLister(clusterVersionIndexer), } eventRecorder := events.NewInMemoryRecorder(tt.name) diff --git a/pkg/controllers/configobservation/interfaces.go b/pkg/controllers/configobservation/interfaces.go index b63a148aa7..781e96f802 100644 --- a/pkg/controllers/configobservation/interfaces.go +++ b/pkg/controllers/configobservation/interfaces.go @@ -21,6 +21,7 @@ type Listers struct { APIServerLister_ configlistersv1.APIServerLister ConsoleLister configlistersv1.ConsoleLister + ClusterVersionLister configlistersv1.ClusterVersionLister InfrastructureLister configlistersv1.InfrastructureLister OAuthLister_ configlistersv1.OAuthLister IngressLister configlistersv1.IngressLister diff --git a/pkg/operator/starter.go b/pkg/operator/starter.go index 28fbc9be8e..8ffac39802 100644 --- a/pkg/operator/starter.go +++ b/pkg/operator/starter.go @@ -16,8 +16,10 @@ import ( "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/selection" + "k8s.io/apimachinery/pkg/util/sets" certinformers "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes" + "k8s.io/client-go/tools/cache" "k8s.io/klog/v2" apiregistrationv1 "k8s.io/kube-aggregator/pkg/apis/apiregistration/v1" apiregistrationclient "k8s.io/kube-aggregator/pkg/client/clientset_generated/clientset" @@ -174,9 +176,22 @@ func RunOperator(ctx context.Context, controllerContext *controllercmd.Controlle operatorCtx.operatorInformer = operatorConfigInformers operatorCtx.operatorConfigInformer = configinformer.NewSharedInformerFactoryWithOptions(configClient, resync) - if err := prepareOauthOperator(controllerContext, operatorCtx); err != nil { + // this one needs to be started now, so prepareOauthOperator can get ClusterVersion + // operatorCtx.operatorConfigInformer.Start(ctx.Done()) + // allSynced := operatorCtx.operatorConfigInformer.WaitForCacheSync(ctx.Done()) + // for t, synced := range allSynced { + // if !synced { + // return fmt.Errorf("informer cache not synchronized for %v", t) + // } + // } + + if err := prepareOauthOperator(ctx, controllerContext, operatorCtx); err != nil { return err } + + // if err := prepareOauthOperator(controllerContext, operatorCtx); err != nil { + // return err + // } if err := prepareOauthAPIServerOperator(ctx, controllerContext, operatorCtx); err != nil { return err } @@ -194,6 +209,7 @@ func RunOperator(ctx context.Context, controllerContext *controllercmd.Controlle for _, informerToRunFn := range operatorCtx.informersToRunFunc { informerToRunFn(ctx.Done()) } + for _, controllerRunFn := range operatorCtx.controllersToRunFunc { go controllerRunFn(ctx, 1) } @@ -202,7 +218,7 @@ func RunOperator(ctx context.Context, controllerContext *controllercmd.Controlle return nil } -func prepareOauthOperator(controllerContext *controllercmd.ControllerContext, operatorCtx *operatorContext) error { +func prepareOauthOperator(ctx context.Context, controllerContext *controllercmd.ControllerContext, operatorCtx *operatorContext) error { routeClient, err := routeclient.NewForConfig(controllerContext.ProtoKubeConfig) if err != nil { return err @@ -224,6 +240,18 @@ func prepareOauthOperator(controllerContext *controllercmd.ControllerContext, op oauthInformers := oauthinformers.NewSharedInformerFactory(oauthClient, resync) + clusterVersionInformer := operatorCtx.operatorConfigInformer.Config().V1().ClusterVersions() + operatorCtx.operatorConfigInformer.Start(ctx.Done()) + if !cache.WaitForNamedCacheSync(configv1.GroupName, ctx.Done(), clusterVersionInformer.Informer().HasSynced) { + return fmt.Errorf("failed to synchronize cluster version informer") + } + + clusterVersionConfig, err := clusterVersionInformer.Lister().Get("version") + if err != nil { + return err + } + enabledClusterCapabilities := sets.StringKeySet(clusterVersionConfig.Status.Capabilities.EnabledCapabilities) + // add syncing for the OAuth metadata ConfigMap if err := operatorCtx.resourceSyncController.SyncConfigMap( resourcesynccontroller.ResourceLocation{Namespace: "openshift-config-managed", Name: "oauth-openshift"}, @@ -281,6 +309,7 @@ func prepareOauthOperator(controllerContext *controllercmd.ControllerContext, op operatorCtx.kubeInformersForNamespaces, operatorCtx.operatorConfigInformer, operatorCtx.resourceSyncController, + enabledClusterCapabilities, controllerContext.EventRecorder, )