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

Open
stefanonardo wants to merge 1 commit into
base: main
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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,
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
8 changes: 8 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,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() {
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 {
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,
)
}
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