Skip to content

OCPBUGS-58422: Handle disabled Ingress capability in HyperShift #1182

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
openshift-merge-bot merged 2 commits into from
Aug 24, 2026
Merged

OCPBUGS-58422: Handle disabled Ingress capability in HyperShift #1182

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
848ba33
OCPBUGS-58422: Handle disabled Ingress capability in HyperShift
stefanonardo Jul 6, 2026
844b8c5
Update pkg/console/operator/sync_v400.go
stefanonardo Aug 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 27 additions & 10 deletions pkg/console/controllers/clidownloads/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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()
Expand Down Expand Up @@ -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)
}

Comment thread
stefanonardo marked this conversation as resolved.
ingressConfig, err := c.ingressConfigLister.Get(api.ConfigResourceName)
if err != nil {
return statusHandler.FlushAndReturn(err)
Expand Down
19 changes: 19 additions & 0 deletions pkg/console/controllers/oauthclients/oauthclients.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ type oauthClientsController struct {
consoleOperatorLister operatorv1listers.ConsoleLister
routesLister routev1listers.RouteLister
ingressConfigLister configv1lister.IngressLister
infrastructureConfigLister configv1lister.InfrastructureLister
clusterVersionLister configv1lister.ClusterVersionLister
Comment thread
stefanonardo marked this conversation as resolved.
targetNSSecretsLister corev1listers.SecretLister
}

Expand All @@ -67,6 +69,8 @@ func NewOAuthClientsController(
consoleOperatorInformer operatorv1informers.ConsoleInformer,
routeInformer routev1informers.RouteInformer,
ingressConfigInformer configv1informers.IngressInformer,
infrastructureConfigInformer configv1informers.InfrastructureInformer,
clusterVersionInformer configv1informers.ClusterVersionInformer,

@jhadvig jhadvig Aug 24, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both of these informares we need to add to the WithInformers() method in

@stefanonardo stefanonardo Aug 24, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These informers are intentionally not in WithInformers(). They're only used for the IsExternalControlPlaneWithIngressDisabled gating check which is a static condition at runtime. Adding them to WithInformers() would trigger a resync on every ClusterVersion/Infrastructure update event, which combined with WithSyncDegradedOnError writing back to operator status can cause unnecessary resync storms. I actually tried adding them during development and had to revert because of resync storms.

@jhadvig jhadvig Aug 24, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, so we will stick to to 1minut resync

targetNSsecretsInformer corev1informers.SecretInformer,
oauthClientSwitchedInformer *util.InformerWithSwitch,
recorder events.Recorder,
Expand All @@ -81,6 +85,8 @@ func NewOAuthClientsController(
consoleOperatorLister: consoleOperatorInformer.Lister(),
routesLister: routeInformer.Lister(),
ingressConfigLister: ingressConfigInformer.Lister(),
infrastructureConfigLister: infrastructureConfigInformer.Lister(),
clusterVersionLister: clusterVersionInformer.Lister(),
targetNSSecretsLister: targetNSsecretsInformer.Lister(),
}

Expand Down Expand Up @@ -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() {
Expand Down
10 changes: 5 additions & 5 deletions pkg/console/controllers/route/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,11 +145,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)
Expand All @@ -162,6 +157,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)
Expand Down
9 changes: 9 additions & 0 deletions pkg/console/operator/sync_v400.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,15 @@ 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) {
statusHandler.AddConditions(status.HandleProgressingOrDegraded("WaitingForConsoleURL", "", nil))
return statusHandler.FlushAndReturn(nil)
}

@jhadvig jhadvig Aug 21, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The oauthclients.go and clidownloads/controller.go early-returns explicitly clear their conditions (HandleProgressingOrDegraded("OAuthClientSync", "", nil) and HandleDegraded("OCDownloadsSync", "", nil)).... this one just returns nil without clearing anything. Probably fine since ingress-disabled is a creation-time state on HyperShift, but worth keeping consistent with the other controllers.

@stefanonardo stefanonardo Aug 21, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll add it to make it consistent, however it's not a real bug as you explained

routeName := api.OpenShiftConsoleRouteName
routeConfig := routesub.NewRouteConfig(updatedOperatorConfig, set.Ingress, routeName)
if routeConfig.IsCustomHostnameSet() {
Expand Down
167 changes: 113 additions & 54 deletions pkg/console/starter/starter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"fmt"
"os"
"syscall"
"time"

// kube
Expand All @@ -14,6 +15,7 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"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"
Expand Down Expand Up @@ -237,6 +239,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 {

@jhadvig jhadvig Aug 21, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR description mentions "manifests: Gate openshift-ingress-operator namespace Role and RoleBinding with Console+Ingress capability annotation" but I don't see any manifest changes in this PR. I also couldn't find any console-operator manifests that target the openshift-ingress-operator namespace in the repo. Is the description stale, or are those changes still needed?

@stefanonardo stefanonardo Aug 21, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh yeah, I need to update the description. That role was deleted in the meanwhile in another commit.

@stefanonardo stefanonardo Aug 21, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

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)
})
}

Comment thread
coderabbitai[bot] marked this conversation as resolved.
// TODO: rearrange these into informer,client pairs, NOT separated.
consoleOperator := consoleoperator.NewConsoleOperator(
ctx,
Expand Down Expand Up @@ -288,6 +307,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,
Expand Down Expand Up @@ -416,55 +437,63 @@ 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(),
// route
operatorConfigInformers.Operator().V1().Consoles(),
operatorConfigInformers.Operator().V1().IngressControllers(),
kubeInformersConfigNamespaced.Core().V1().Secrets(), // `openshift-config` namespace informers
routesInformersNamespaced.Route().V1().Routes(),
// events
recorder,
)

downloadsRouteController := route.NewRouteSyncController(
api.OpenShiftConsoleDownloadsRouteName,
// disable health check for console route
false,
// top level config
configInformers,
// clients
operatorClient,
routesClient.RouteV1(),
// route
operatorConfigInformers.Operator().V1().Consoles(),
operatorConfigInformers.Operator().V1().IngressControllers(),
kubeInformersConfigNamespaced.Core().V1().Secrets(), // `openshift-config` namespace informers
routesInformersNamespaced.Route().V1().Routes(),
// 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(),
// route
operatorConfigInformers.Operator().V1().Consoles(),
operatorConfigInformers.Operator().V1().IngressControllers(),
kubeInformersConfigNamespaced.Core().V1().Secrets(), // `openshift-config` namespace informers
routesInformersNamespaced.Route().V1().Routes(),
// events
recorder,
)

downloadsRouteController = route.NewRouteSyncController(
api.OpenShiftConsoleDownloadsRouteName,
// disable health check for console route
false,
// top level config
configInformers,
// clients
operatorClient,
routesClient.RouteV1(),
// route
operatorConfigInformers.Operator().V1().Consoles(),
operatorConfigInformers.Operator().V1().IngressControllers(),
kubeInformersConfigNamespaced.Core().V1().Secrets(), // `openshift-config` namespace informers
routesInformersNamespaced.Route().V1().Routes(),
// 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
Expand Down Expand Up @@ -664,7 +693,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,
Expand All @@ -677,13 +706,10 @@ func RunOperator(ctx context.Context, controllerContext *controllercmd.Controlle
consoleServiceAccountController,
downloadsServiceAccountController,
consoleServiceController,
consoleRouteController,
downloadsServiceController,
downloadsRouteController,
consoleOperator,
cliDownloadsController,
downloadsDeploymentController,
consoleRouteHealthCheckController,
consolePDBController,
downloadsPDBController,
oauthClientController,
Expand All @@ -693,7 +719,15 @@ func RunOperator(ctx context.Context, controllerContext *controllercmd.Controlle
upgradeNotificationController,
staleConditionsController,
storageversionmigrationController,
} {
}
if !ingressDisabled {
controllers = append(controllers,
consoleRouteController,
downloadsRouteController,
consoleRouteHealthCheckController,
)
}

@logonoff logonoff Aug 20, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you check my AI slop and verify that those controllers don't need to be in the list

Slop report # console-operator: Network & Ingress Dependency Summary

Direct Network Calls

Only one controller makes real outbound network connections:

Controller What It Does
consoleRouteHealthCheckController HTTPS GET to the live console route URL. Custom http.Client with cluster CA pool. Up to 10 retries, 5s timeout per request, 1s between retries.

All others interact exclusively through the kube-apiserver (informers, listers, typed clients). Several import net/url for URL parsing only. resourceSyncer imports net/http but only to serve an inbound debug handler — it makes no outbound calls.

Controllers That Break If Ingress Is Disabled

"Ingress disabled" means the cluster router is gone (Routes never reach Admitted=True) and/or config.openshift.io/v1/ingresses/cluster is missing.

Controller What Breaks Condition Set
consoleRouteHealthCheckController Probes console route URL over HTTPS; if the route is never admitted, all 10 retries fail → ClusterOperator Available=False (most visible user-facing failure) RouteHealthDegraded=True
RouteHealthAvailable=False
reason: RouteNotAdmitted
consoleOperator (sync_v400) Calls GetActiveRouteInfo → IngressURI; on failure aborts the entire sync loop — ConfigMap, Deployment, console.config.openshift.io Status.ConsoleURL, and console-public ConfigMap all go stale or unset SyncLoopRefreshDegraded=True
reason: FailedIngress
consoleRouteController Console Route never transitions to Admitted; hostname never derivable downstream ConsoleDefaultRouteSyncDegraded=True
reason: FailedAdmitDefaultRoute
downloadsRouteController Downloads Route never admitted DownloadsDefaultRouteSyncDegraded=True
reason: FailedAdmitDefaultRoute
oauthClientController redirectURIs on OAuthClient/console never updated with the admitted console URL → OAuth login to the console fails OAuthClientSyncDegraded=True
(via WithSyncDegradedOnError)
cliDownloadsController ConsoleCLIDownload/oc-cli-downloads CR never created/updated; oc CLI download links broken No named condition — bare error, sync retries silently
consoleServiceController If Ingresses/cluster object is entirely missing, service creation fails ServiceSyncDegraded=True
downloadsServiceController Same as above ServiceSyncDegraded=True

Controllers Not Affected by Ingress

Controller Reason Unaffected
migrationCleanupController Deletes stale Deployment/Service/Secret only
resourceSyncer Syncs ConfigMaps/Secrets via kube-apiserver only
clusterOperatorStatus Aggregates conditions written by other controllers
logLevelController Manages operator log level only
managementStateController Manages operator management state only
configUpgradeableController Checks unsupported config overrides only
configObserver Watches FeatureGate/ClusterVersion via informers only
consoleServiceAccountController Applies ServiceAccount objects only
downloadsServiceAccountController Applies ServiceAccount objects only
consolePDBController Applies PodDisruptionBudget only
downloadsPDBController Applies PodDisruptionBudget only
oauthClientSecretController Generates and writes OAuth client secret only
oidcSetupController Reads authentication/cluster, syncs ConfigMaps only
cliOIDCClientStatusController Updates authentication status conditions only
upgradeNotificationController Reads ClusterVersion, manages ConsoleNotification only
staleConditionsController Removes stale operator status conditions only
storageversionmigrationController Manages StorageVersionMigration CRDs only
downloadsDeploymentController Manages the downloads Deployment only (reads infra config, not ingress)

Designed Escape Hatches

1. Explicit URL overrides (bypasses IngressURI calls)

Setting these fields on the Console operator config skips route admission checks entirely:

Field Bypasses IngressURI in
operatorConfig.Spec.Ingress.ConsoleURL consoleOperator, oauthClientController, consoleRouteHealthCheckController
operatorConfig.Spec.Ingress.ClientDownloadsURL cliDownloadsController

2. IsExternalControlPlaneWithIngressDisabled() (HyperShift)

When the cluster is HyperShift (ExternalTopologyMode) and the Ingress capability is disabled in ClusterVersion.Status.Capabilities:

  • consoleRouteController / downloadsRouteController — short-circuit with nil (no route created, no error set)
  • consoleServiceController / downloadsServiceController — switch service type from ClusterIP → NodePort so pods are reachable without a router

@stefanonardo stefanonardo Aug 21, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

everything looks in place

for _, controller := range controllers {
go controller.Run(ctx, 1)
}

Expand Down Expand Up @@ -741,6 +775,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 {
Expand Down
Loading