diff --git a/manifests/03-rbac-role-ns-openshift-ingress-operator.yaml b/manifests/03-rbac-role-ns-openshift-ingress-operator.yaml index dd6d3f5e98..812cc57686 100644 --- a/manifests/03-rbac-role-ns-openshift-ingress-operator.yaml +++ b/manifests/03-rbac-role-ns-openshift-ingress-operator.yaml @@ -8,7 +8,7 @@ metadata: include.release.openshift.io/ibm-cloud-managed: "true" include.release.openshift.io/self-managed-high-availability: "true" include.release.openshift.io/single-node-developer: "true" - capability.openshift.io/name: Console + capability.openshift.io/name: Console+Ingress rules: - apiGroups: - "" diff --git a/manifests/04-rbac-rolebinding.yaml b/manifests/04-rbac-rolebinding.yaml index 1189337d82..2d0f7f4bb2 100644 --- a/manifests/04-rbac-rolebinding.yaml +++ b/manifests/04-rbac-rolebinding.yaml @@ -87,7 +87,7 @@ metadata: include.release.openshift.io/ibm-cloud-managed: "true" include.release.openshift.io/self-managed-high-availability: "true" include.release.openshift.io/single-node-developer: "true" - capability.openshift.io/name: Console + capability.openshift.io/name: Console+Ingress roleRef: kind: Role name: console-operator diff --git a/pkg/console/controllers/clidownloads/controller.go b/pkg/console/controllers/clidownloads/controller.go index 75a3a271de..d135507d6e 100644 --- a/pkg/console/controllers/clidownloads/controller.go +++ b/pkg/console/controllers/clidownloads/controller.go @@ -47,11 +47,13 @@ import ( type CLIDownloadsSyncController struct { // clients - operatorClient v1helpers.OperatorClient - consoleCliDownloadsClient consoleclientv1.ConsoleCLIDownloadInterface - routeLister routev1listers.RouteLister - ingressConfigLister configlistersv1.IngressLister - operatorConfigLister operatorv1listers.ConsoleLister + operatorClient v1helpers.OperatorClient + consoleCliDownloadsClient consoleclientv1.ConsoleCLIDownloadInterface + routeLister routev1listers.RouteLister + ingressConfigLister configlistersv1.IngressLister + infrastructureConfigLister configlistersv1.InfrastructureLister + clusterVersionLister configlistersv1.ClusterVersionLister + operatorConfigLister operatorv1listers.ConsoleLister } func NewCLIDownloadsSyncController( @@ -71,11 +73,13 @@ func NewCLIDownloadsSyncController( ctrl := &CLIDownloadsSyncController{ // clients - operatorClient: operatorClient, - consoleCliDownloadsClient: cliDownloadsInterface, - routeLister: routeInformer.Lister(), - ingressConfigLister: configInformer.Config().V1().Ingresses().Lister(), - operatorConfigLister: operatorConfigInformer.Lister(), + operatorClient: operatorClient, + consoleCliDownloadsClient: cliDownloadsInterface, + routeLister: routeInformer.Lister(), + ingressConfigLister: configInformer.Config().V1().Ingresses().Lister(), + infrastructureConfigLister: configInformer.Config().V1().Infrastructures().Lister(), + clusterVersionLister: configInformer.Config().V1().ClusterVersions().Lister(), + operatorConfigLister: operatorConfigInformer.Lister(), } configV1Informers := configInformer.Config().V1() @@ -121,6 +125,19 @@ func (c *CLIDownloadsSyncController) Sync(ctx context.Context, controllerContext downloadsErr error ) if len(operatorConfig.Spec.Ingress.ClientDownloadsURL) == 0 { + infrastructureConfig, err := c.infrastructureConfigLister.Get(api.ConfigResourceName) + if err != nil { + return statusHandler.FlushAndReturn(err) + } + clusterVersionConfig, err := c.clusterVersionLister.Get(api.VersionResourceName) + if err != nil { + return statusHandler.FlushAndReturn(err) + } + if controllersutil.IsExternalControlPlaneWithIngressDisabled(infrastructureConfig, clusterVersionConfig) { + statusHandler.AddCondition(status.HandleDegraded("OCDownloadsSync", "", nil)) + return statusHandler.FlushAndReturn(nil) + } + ingressConfig, err := c.ingressConfigLister.Get(api.ConfigResourceName) if err != nil { return statusHandler.FlushAndReturn(err) diff --git a/pkg/console/controllers/oauthclients/oauthclients.go b/pkg/console/controllers/oauthclients/oauthclients.go index 39d1bb34f2..1c576216c6 100644 --- a/pkg/console/controllers/oauthclients/oauthclients.go +++ b/pkg/console/controllers/oauthclients/oauthclients.go @@ -57,6 +57,8 @@ type oauthClientsController struct { consoleOperatorLister operatorv1listers.ConsoleLister routesLister routev1listers.RouteLister ingressConfigLister configv1lister.IngressLister + infrastructureConfigLister configv1lister.InfrastructureLister + clusterVersionLister configv1lister.ClusterVersionLister targetNSSecretsLister corev1listers.SecretLister } @@ -67,6 +69,8 @@ func NewOAuthClientsController( consoleOperatorInformer operatorv1informers.ConsoleInformer, routeInformer routev1informers.RouteInformer, ingressConfigInformer configv1informers.IngressInformer, + infrastructureConfigInformer configv1informers.InfrastructureInformer, + clusterVersionInformer configv1informers.ClusterVersionInformer, targetNSsecretsInformer corev1informers.SecretInformer, oauthClientSwitchedInformer *util.InformerWithSwitch, recorder events.Recorder, @@ -81,6 +85,8 @@ func NewOAuthClientsController( consoleOperatorLister: consoleOperatorInformer.Lister(), routesLister: routeInformer.Lister(), ingressConfigLister: ingressConfigInformer.Lister(), + infrastructureConfigLister: infrastructureConfigInformer.Lister(), + clusterVersionLister: clusterVersionInformer.Lister(), targetNSSecretsLister: targetNSsecretsInformer.Lister(), } @@ -138,6 +144,19 @@ func (c *oauthClientsController) sync(ctx context.Context, controllerContext fac var consoleURL *url.URL if len(operatorConfig.Spec.Ingress.ConsoleURL) == 0 { + infrastructureConfig, err := c.infrastructureConfigLister.Get(api.ConfigResourceName) + if err != nil { + return err + } + clusterVersionConfig, err := c.clusterVersionLister.Get(api.VersionResourceName) + if err != nil { + return err + } + if util.IsExternalControlPlaneWithIngressDisabled(infrastructureConfig, clusterVersionConfig) { + statusHandler.AddConditions(status.HandleProgressingOrDegraded("OAuthClientSync", "", nil)) + return statusHandler.FlushAndReturn(nil) + } + routeName := api.OpenShiftConsoleRouteName routeConfig := routesub.NewRouteConfig(operatorConfig, ingressConfig, routeName) if routeConfig.IsCustomHostnameSet() { diff --git a/pkg/console/controllers/route/controller.go b/pkg/console/controllers/route/controller.go index c22dbc3a92..a61f7d6f7e 100644 --- a/pkg/console/controllers/route/controller.go +++ b/pkg/console/controllers/route/controller.go @@ -176,11 +176,6 @@ func (c *RouteSyncController) Sync(ctx context.Context, controllerContext factor return statusHandler.FlushAndReturn(err) } - ingressControllerConfig, err := c.ingressControllerLister.IngressControllers(api.IngressControllerNamespace).Get(api.DefaultIngressController) - if err != nil { - return statusHandler.FlushAndReturn(err) - } - clusterVersionConfig, err := c.clusterVersionLister.Get("version") if err != nil { return statusHandler.FlushAndReturn(err) @@ -193,6 +188,11 @@ func (c *RouteSyncController) Sync(ctx context.Context, controllerContext factor return statusHandler.FlushAndReturn(nil) } + ingressControllerConfig, err := c.ingressControllerLister.IngressControllers(api.IngressControllerNamespace).Get(api.DefaultIngressController) + if err != nil { + return statusHandler.FlushAndReturn(err) + } + ingressConfig, err := c.ingressConfigLister.Get(api.ConfigResourceName) if err != nil { return statusHandler.FlushAndReturn(err) diff --git a/pkg/console/operator/sync_v400.go b/pkg/console/operator/sync_v400.go index b0b1edb54e..d24def890f 100644 --- a/pkg/console/operator/sync_v400.go +++ b/pkg/console/operator/sync_v400.go @@ -61,6 +61,14 @@ func (co *consoleOperator) sync_v400(ctx context.Context, controllerContext fact ) if len(set.Operator.Spec.Ingress.ConsoleURL) == 0 { + clusterVersionConfig, err := co.clusterVersionLister.Get(api.VersionResourceName) + if err != nil { + return statusHandler.FlushAndReturn(err) + } + if controllersutil.IsExternalControlPlaneWithIngressDisabled(set.Infrastructure, clusterVersionConfig) { + return statusHandler.FlushAndReturn(nil) + } + routeName := api.OpenShiftConsoleRouteName routeConfig := routesub.NewRouteConfig(updatedOperatorConfig, set.Ingress, routeName) if routeConfig.IsCustomHostnameSet() { diff --git a/pkg/console/starter/starter.go b/pkg/console/starter/starter.go index c1cb7b2736..8f33a7e0c5 100644 --- a/pkg/console/starter/starter.go +++ b/pkg/console/starter/starter.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "os" + "syscall" "time" // kube @@ -15,6 +16,7 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/dynamic" "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes" @@ -246,6 +248,23 @@ func RunOperator(ctx context.Context, controllerContext *controllercmd.Controlle return err } + infrastructureConfig, err := configClient.ConfigV1().Infrastructures().Get(ctx, api.ConfigResourceName, metav1.GetOptions{}) + if err != nil { + return err + } + clusterVersionConfig, err := configClient.ConfigV1().ClusterVersions().Get(ctx, api.VersionResourceName, metav1.GetOptions{}) + if err != nil { + return err + } + ingressDisabled := util.IsExternalControlPlaneWithIngressDisabled(infrastructureConfig, clusterVersionConfig) + if ingressDisabled { + klog.Info("Ingress capability is disabled in external control plane topology, skipping route and health check controllers") + pollAndCallOnIngressEnabled(ctx, configClient, time.Minute*5, func() { + klog.Info("Ingress capability has been enabled, restarting to start route and health check controllers") + syscall.Kill(syscall.Getpid(), syscall.SIGINT) + }) + } + // TODO: rearrange these into informer,client pairs, NOT separated. consoleOperator := consoleoperator.NewConsoleOperator( ctx, @@ -296,6 +315,8 @@ func RunOperator(ctx context.Context, controllerContext *controllercmd.Controlle operatorConfigInformers.Operator().V1().Consoles(), routesInformersNamespaced.Route().V1().Routes(), configInformers.Config().V1().Ingresses(), + configInformers.Config().V1().Infrastructures(), + configInformers.Config().V1().ClusterVersions(), kubeInformersNamespaced.Core().V1().Secrets(), oauthClientsSwitchedInformer, recorder, @@ -424,61 +445,69 @@ func RunOperator(ctx context.Context, controllerContext *controllercmd.Controlle recorder, ) - consoleRouteController := route.NewRouteSyncController( - api.OpenShiftConsoleRouteName, - // enable health check for console route - true, - // top level config - configInformers, - // clients - operatorClient, - routesClient.RouteV1(), - kubeClient.CoreV1(), - // route - operatorConfigInformers.Operator().V1().Consoles(), - operatorConfigInformers.Operator().V1().IngressControllers(), - kubeInformersConfigNamespaced.Core().V1().Secrets(), // `openshift-config` namespace informers - kubeInformersNamespaced.Core().V1().Secrets(), // `openshift-console` namespace informers (for HTTP/2 cert) - routesInformersNamespaced.Route().V1().Routes(), - kubeInformersIngressOperatorNamespaced.Core().V1().Secrets(), // `openshift-ingress-operator` namespace informers (for router-ca) - // events - recorder, - ) - - downloadsRouteController := route.NewRouteSyncController( - api.OpenShiftConsoleDownloadsRouteName, - // disable health check for console route - false, - // top level config - configInformers, - // clients - operatorClient, - routesClient.RouteV1(), - nil, // no secret client needed for downloads route - // route - operatorConfigInformers.Operator().V1().Consoles(), - operatorConfigInformers.Operator().V1().IngressControllers(), - kubeInformersConfigNamespaced.Core().V1().Secrets(), // `openshift-config` namespace informers - nil, // no console secret lister needed for downloads route - routesInformersNamespaced.Route().V1().Routes(), - nil, // no ingress CA needed for downloads route - // events - recorder, - ) - - consoleRouteHealthCheckController := healthcheck.NewHealthCheckController( - // top level config - configClient.ConfigV1(), - // clients - operatorClient, - // route - operatorConfigInformers.Operator().V1().Consoles(), - configInformers, // Config - kubeInformersNamespaced.Core().V1(), // `openshift-console` namespace informers - routesInformersNamespaced.Route().V1().Routes(), - // events - recorder, - ) + var consoleRouteController, downloadsRouteController interface { + Run(ctx context.Context, workers int) + } + var consoleRouteHealthCheckController interface { + Run(ctx context.Context, workers int) + } + if !ingressDisabled { + consoleRouteController = route.NewRouteSyncController( + api.OpenShiftConsoleRouteName, + // enable health check for console route + true, + // top level config + configInformers, + // clients + operatorClient, + routesClient.RouteV1(), + kubeClient.CoreV1(), + // route + operatorConfigInformers.Operator().V1().Consoles(), + operatorConfigInformers.Operator().V1().IngressControllers(), + kubeInformersConfigNamespaced.Core().V1().Secrets(), // `openshift-config` namespace informers + kubeInformersNamespaced.Core().V1().Secrets(), // `openshift-console` namespace informers (for HTTP/2 cert) + routesInformersNamespaced.Route().V1().Routes(), + kubeInformersIngressOperatorNamespaced.Core().V1().Secrets(), // `openshift-ingress-operator` namespace informers (for router-ca) + // events + recorder, + ) + + downloadsRouteController = route.NewRouteSyncController( + api.OpenShiftConsoleDownloadsRouteName, + // disable health check for console route + false, + // top level config + configInformers, + // clients + operatorClient, + routesClient.RouteV1(), + nil, // no secret client needed for downloads route + // route + operatorConfigInformers.Operator().V1().Consoles(), + operatorConfigInformers.Operator().V1().IngressControllers(), + kubeInformersConfigNamespaced.Core().V1().Secrets(), // `openshift-config` namespace informers + nil, // no console secret lister needed for downloads route + routesInformersNamespaced.Route().V1().Routes(), + nil, // no ingress CA needed for downloads route + // events + recorder, + ) + + consoleRouteHealthCheckController = healthcheck.NewHealthCheckController( + // top level config + configClient.ConfigV1(), + // clients + operatorClient, + // route + operatorConfigInformers.Operator().V1().Consoles(), + configInformers, // Config + kubeInformersNamespaced.Core().V1(), // `openshift-console` namespace informers + routesInformersNamespaced.Route().V1().Routes(), + // events + recorder, + ) + } upgradeNotificationController := upgradenotification.NewUpgradeNotificationController( // top level config @@ -671,7 +700,7 @@ func RunOperator(ctx context.Context, controllerContext *controllercmd.Controlle informer.Start(ctx.Done()) } - for _, controller := range []interface { + controllers := []interface { Run(ctx context.Context, workers int) }{ migrationCleanupController, @@ -683,13 +712,10 @@ func RunOperator(ctx context.Context, controllerContext *controllercmd.Controlle consoleServiceAccountController, downloadsServiceAccountController, consoleServiceController, - consoleRouteController, downloadsServiceController, - downloadsRouteController, consoleOperator, cliDownloadsController, downloadsDeploymentController, - consoleRouteHealthCheckController, consolePDBController, downloadsPDBController, oauthClientController, @@ -699,7 +725,15 @@ func RunOperator(ctx context.Context, controllerContext *controllercmd.Controlle upgradeNotificationController, staleConditionsController, storageversionmigrationController, - } { + } + if !ingressDisabled { + controllers = append(controllers, + consoleRouteController, + downloadsRouteController, + consoleRouteHealthCheckController, + ) + } + for _, controller := range controllers { go controller.Run(ctx, 1) } @@ -747,6 +781,31 @@ func getResourceSyncer(controllerContext *controllercmd.ControllerContext, kubeC return resourceSyncerInformers, resourceSyncer } +func pollAndCallOnIngressEnabled(ctx context.Context, configClient configclient.Interface, interval time.Duration, onIngressEnabled func()) { + go func() { + err := wait.PollUntilContextCancel(ctx, interval, false, func(ctx context.Context) (done bool, err error) { + infrastructureConfig, err := configClient.ConfigV1().Infrastructures().Get(ctx, api.ConfigResourceName, metav1.GetOptions{}) + if err != nil { + klog.Errorf("failed to check infrastructure config for ingress capability, retrying: %v", err) + return false, nil + } + clusterVersionConfig, err := configClient.ConfigV1().ClusterVersions().Get(ctx, api.VersionResourceName, metav1.GetOptions{}) + if err != nil { + klog.Errorf("failed to check cluster version for ingress capability, retrying: %v", err) + return false, nil + } + ingressEnabled := !util.IsExternalControlPlaneWithIngressDisabled(infrastructureConfig, clusterVersionConfig) + return ingressEnabled, nil + }) + + if err != nil { + return + } + + onIngressEnabled() + }() +} + func extractStaticPodOperatorSpec(obj *unstructured.Unstructured, fieldManager string) (*applyoperatorv1.OperatorSpecApplyConfiguration, error) { castObj := &operatorv1.Console{} if err := runtime.DefaultUnstructuredConverter.FromUnstructured(obj.Object, castObj); err != nil { diff --git a/pkg/console/starter/starter_test.go b/pkg/console/starter/starter_test.go index bc806b802b..55a53a00ed 100644 --- a/pkg/console/starter/starter_test.go +++ b/pkg/console/starter/starter_test.go @@ -8,9 +8,12 @@ import ( configv1 "github.com/openshift/api/config/v1" operatorv1 "github.com/openshift/api/operator/v1" + fakeconfigclient "github.com/openshift/client-go/config/clientset/versioned/fake" + "github.com/openshift/console-operator/pkg/api" "github.com/openshift/library-go/pkg/controller/controllercmd" "github.com/openshift/library-go/pkg/operator/events" v1helpers "github.com/openshift/library-go/pkg/operator/v1helpers" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/kubernetes/fake" clocktesting "k8s.io/utils/clock/testing" @@ -95,3 +98,63 @@ func TestDeduplicateObjectReferences(t *testing.T) { }) } } + +func TestPollAndCallOnIngressEnabled(t *testing.T) { + infraConfig := &configv1.Infrastructure{ + ObjectMeta: metav1.ObjectMeta{Name: api.ConfigResourceName}, + Status: configv1.InfrastructureStatus{ + ControlPlaneTopology: configv1.ExternalTopologyMode, + }, + } + clusterVersionIngressDisabled := &configv1.ClusterVersion{ + ObjectMeta: metav1.ObjectMeta{Name: api.VersionResourceName}, + Status: configv1.ClusterVersionStatus{ + Capabilities: configv1.ClusterVersionCapabilitiesStatus{ + EnabledCapabilities: []configv1.ClusterVersionCapability{ + configv1.ClusterVersionCapabilityOpenShiftSamples, + }, + }, + }, + } + + t.Run("does not trigger when ingress stays disabled", func(t *testing.T) { + configClient := fakeconfigclient.NewClientset(infraConfig, clusterVersionIngressDisabled) + + triggered := make(chan struct{}) + pollAndCallOnIngressEnabled(t.Context(), configClient, 50*time.Millisecond, func() { + close(triggered) + }) + + select { + case <-triggered: + t.Fatal("callback should not have been called when ingress is disabled") + case <-time.After(500 * time.Millisecond): + } + }) + + t.Run("triggers when ingress becomes enabled", func(t *testing.T) { + configClient := fakeconfigclient.NewClientset(infraConfig, clusterVersionIngressDisabled) + + triggered := make(chan struct{}) + pollAndCallOnIngressEnabled(t.Context(), configClient, 50*time.Millisecond, func() { + close(triggered) + }) + + // Wait for at least one poll cycle, then enable ingress + time.Sleep(100 * time.Millisecond) + cv, err := configClient.ConfigV1().ClusterVersions().Get(context.Background(), api.VersionResourceName, metav1.GetOptions{}) + if err != nil { + t.Fatalf("failed to get ClusterVersion: %v", err) + } + cv.Status.Capabilities.EnabledCapabilities = append(cv.Status.Capabilities.EnabledCapabilities, configv1.ClusterVersionCapabilityIngress) + if _, err := configClient.ConfigV1().ClusterVersions().UpdateStatus(context.Background(), cv, metav1.UpdateOptions{}); err != nil { + t.Fatalf("failed to update ClusterVersion status: %v", err) + } + + select { + case <-triggered: + case <-time.After(3 * time.Second): + t.Fatal("timed out waiting for callback after ingress was enabled") + } + }) +}