-
Notifications
You must be signed in to change notification settings - Fork 150
OCPBUGS-33787: Tolerate the absence of ingress capability on HyperShift clusters #886
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| # This 'console' service manifest is used when the ingress cluster capability is disabled. | ||
| # Service will be exposed using a NodePort to enable the alternative ingress. | ||
| apiVersion: v1 | ||
| kind: Service | ||
| metadata: | ||
| name: console | ||
| namespace: openshift-console | ||
| labels: | ||
| app: console | ||
| spec: | ||
| ports: | ||
| - name: https | ||
| protocol: TCP | ||
| port: 443 | ||
| targetPort: 8443 | ||
| selector: | ||
| app: console | ||
| component: ui | ||
| type: NodePort | ||
| sessionAffinity: None |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| # This 'downloads' service manifest is used when the ingress cluster capability is disabled. | ||
| # Service will be exposed using a NodePort to enable the alternative ingress. | ||
| apiVersion: v1 | ||
| kind: Service | ||
| metadata: | ||
| namespace: openshift-console | ||
| name: downloads | ||
| spec: | ||
| ports: | ||
| - name: http | ||
| port: 80 | ||
| protocol: TCP | ||
| targetPort: 8080 | ||
| selector: | ||
| app: console | ||
| component: downloads | ||
| type: NodePort | ||
| sessionAffinity: None |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,115 @@ | ||
| # Use AWS ALB as alternative ingress on ROSA HCP | ||
|
|
||
| This doc aims at showing the effort needed to expose the OpenShift console via AWS ALB on a ROSA HCP cluster. | ||
| The use case in mind is [HyperShift hosted clusters where the Ingress capability is disabled](https://github.com/openshift/enhancements/pull/1415). | ||
|
|
||
| ## Requirements | ||
|
|
||
| - ROSA HCP OpenShift cluster. | ||
| - [AWS Load Balancer Operator installed and its controller created](https://docs.openshift.com/rosa/networking/aws-load-balancer-operator.html). | ||
| - User logged as a cluster admin. | ||
|
|
||
| ## Procedure | ||
|
|
||
| ### Create certificate in AWS Certificate Manager | ||
|
|
||
| In order to configure an HTTPS listener on AWS ALB you need to have a certificate created in AWS Certificate Manager. | ||
| You can import an existing certificate or request a new one. Make sure the certificate is created in the same region as your cluster. | ||
| Note the certificate ARN, you will need it later. | ||
|
|
||
| ### Create Ingress resources for the NodePort services | ||
|
|
||
| To provision ALBs create the following resources: | ||
| ```bash | ||
| cat <<EOF | oc apply -f - | ||
| apiVersion: networking.k8s.io/v1 | ||
| kind: Ingress | ||
| metadata: | ||
| annotations: | ||
| alb.ingress.kubernetes.io/scheme: internet-facing | ||
| alb.ingress.kubernetes.io/target-type: instance | ||
| alb.ingress.kubernetes.io/backend-protocol: HTTPS | ||
| alb.ingress.kubernetes.io/certificate-arn: ${CERTIFICATE_ARN} | ||
| name: console | ||
| namespace: openshift-console | ||
| spec: | ||
| ingressClassName: alb | ||
| rules: | ||
| - http: | ||
| paths: | ||
| - path: / | ||
| pathType: Prefix | ||
| backend: | ||
| service: | ||
| name: console | ||
| port: | ||
| number: 443 | ||
| --- | ||
| apiVersion: networking.k8s.io/v1 | ||
| kind: Ingress | ||
| metadata: | ||
| annotations: | ||
| alb.ingress.kubernetes.io/scheme: internet-facing | ||
| alb.ingress.kubernetes.io/target-type: instance | ||
| alb.ingress.kubernetes.io/backend-protocol: HTTP | ||
| alb.ingress.kubernetes.io/certificate-arn: ${CERTIFICATE_ARN} | ||
| name: downloads | ||
| namespace: openshift-console | ||
| spec: | ||
| ingressClassName: alb | ||
| rules: | ||
| - http: | ||
| paths: | ||
| - path: / | ||
| pathType: Prefix | ||
| backend: | ||
| service: | ||
| name: downloads | ||
| port: | ||
| number: 80 | ||
| EOF | ||
| ``` | ||
|
|
||
| ### Update console config | ||
|
|
||
| Once the console ALBs are ready you need to let the console operator know which urls to use. | ||
| Update the console operator config providing the custom urls: | ||
| ```bash | ||
| $ CONSOLE_ALB_HOST=$(oc -n openshift-console get ing console -o yaml | yq .status.loadBalancer.ingress[0].hostname) | ||
| $ DOWNLOADS_ALB_HOST=$(oc -n openshift-console get ing downloads -o yaml | yq .status.loadBalancer.ingress[0].hostname) | ||
| $ oc patch console.operator.openshift.io cluster --type=merge -p "{\"spec\":{\"ingress\":{\"consoleURL\":\"https://${CONSOLE_ALB_HOST}\",\"clientDownloadsURL\":\"https://${DOWNLOADS_ALB_HOST}\"}}}" | ||
| ``` | ||
|
|
||
| ## Notes | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
|
|
||
| 1. ROSA HCP does not have the authentication operator, the authentication server is managed centrally by the HyperShift layer: | ||
| ```bash | ||
| $ oc -n openshift-authentication-operator get deploy,route | ||
| No resources found | ||
|
|
||
| $ oc -n openshift-authentication get pods,routes | ||
| No resources found | ||
|
|
||
| $ oc get oauthclient | grep -v console | ||
| NAME SECRET WWW-CHALLENGE TOKEN-MAX-AGE REDIRECT URIS | ||
| openshift-browser-client false default https://oauth.mytestcluster.5199.s3.devshift.org:443/oauth/token/display | ||
| openshift-challenging-client true default https://oauth.mytestcluster.5199.s3.devshift.org:443/oauth/token/implicit | ||
|
|
||
| $ oc -n openshift-console rsh deploy/console curl -k https://openshift.default.svc/.well-known/oauth-authorization-server | ||
| { | ||
| "issuer": "https://oauth.mytestcluster.5199.s3.devshift.org:443", | ||
| "authorization_endpoint": "https://oauth.mytestcluster.5199.s3.devshift.org:443/oauth/authorize", | ||
| "token_endpoint": "https://oauth.mytestcluster.5199.s3.devshift.org:443/oauth/token", | ||
| ``` | ||
|
|
||
| 2. When the ingress capability is disabled, the console operator relies on the end user to provide the console and download URLs (using the operator API) for health checks and oauthclient. | ||
|
|
||
| 3. When the ingress capability is disabled, the console operator skips the implementation of the component route customization. | ||
|
|
||
| 4. To simulate the absence of ingress connectivity when the ingress capability is disabled, set the desired replicas to zero in the default ingress controller: | ||
| ```bash | ||
| $ oc -n openshift-ingress-operator patch ingresscontroller default --type='json' -p='[{"op": "replace", "path": "/spec/replicas", "value":0}]' | ||
| ``` | ||
|
|
||
| ## Links | ||
| - [Demo of ALB ingress for the console on ROSA HCP](https://drive.google.com/file/d/1uWZgFbSeZTlDzlFyPW7QcH-625JsbSbw/view) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -40,11 +40,13 @@ type RouteSyncController struct { | |
| routeName string | ||
| isHealthCheckEnabled bool | ||
| // clients | ||
| operatorClient v1helpers.OperatorClient | ||
| routeClient routeclientv1.RoutesGetter | ||
| operatorConfigLister operatorv1listers.ConsoleLister | ||
| ingressConfigLister configlistersv1.IngressLister | ||
| secretLister corev1listers.SecretLister | ||
| operatorClient v1helpers.OperatorClient | ||
| routeClient routeclientv1.RoutesGetter | ||
| operatorConfigLister operatorv1listers.ConsoleLister | ||
| ingressConfigLister configlistersv1.IngressLister | ||
| secretLister corev1listers.SecretLister | ||
| infrastructureConfigLister configlistersv1.InfrastructureLister | ||
| clusterVersionLister configlistersv1.ClusterVersionLister | ||
| } | ||
|
|
||
| func NewRouteSyncController( | ||
|
|
@@ -63,13 +65,15 @@ func NewRouteSyncController( | |
| recorder events.Recorder, | ||
| ) factory.Controller { | ||
| ctrl := &RouteSyncController{ | ||
| routeName: routeName, | ||
| isHealthCheckEnabled: isHealthCheckEnabled, | ||
| operatorClient: operatorClient, | ||
| operatorConfigLister: operatorConfigInformer.Lister(), | ||
| ingressConfigLister: configInformer.Config().V1().Ingresses().Lister(), | ||
| routeClient: routev1Client, | ||
| secretLister: secretInformer.Lister(), | ||
| routeName: routeName, | ||
| isHealthCheckEnabled: isHealthCheckEnabled, | ||
| operatorClient: operatorClient, | ||
| operatorConfigLister: operatorConfigInformer.Lister(), | ||
| ingressConfigLister: configInformer.Config().V1().Ingresses().Lister(), | ||
| routeClient: routev1Client, | ||
| secretLister: secretInformer.Lister(), | ||
| infrastructureConfigLister: configInformer.Config().V1().Infrastructures().Lister(), | ||
| clusterVersionLister: configInformer.Config().V1().ClusterVersions().Lister(), | ||
| } | ||
|
|
||
| configV1Informers := configInformer.Config().V1() | ||
|
|
@@ -114,6 +118,35 @@ func (c *RouteSyncController) Sync(ctx context.Context, controllerContext factor | |
|
|
||
| statusHandler := status.NewStatusHandler(c.operatorClient) | ||
|
|
||
| // Do not proceed to the route checks if alternative ingress is requested. | ||
| switch c.routeName { | ||
| case api.OpenShiftConsoleRouteName: | ||
| if len(operatorConfig.Spec.Ingress.ConsoleURL) != 0 { | ||
| return statusHandler.FlushAndReturn(nil) | ||
| } | ||
| case api.OpenShiftConsoleDownloadsRouteName: | ||
| if len(operatorConfig.Spec.Ingress.ClientDownloadsURL) != 0 { | ||
| return statusHandler.FlushAndReturn(nil) | ||
| } | ||
| } | ||
|
|
||
| infrastructureConfig, err := c.infrastructureConfigLister.Get(api.ConfigResourceName) | ||
| if err != nil { | ||
| return statusHandler.FlushAndReturn(err) | ||
| } | ||
|
|
||
| clusterVersionConfig, err := c.clusterVersionLister.Get("version") | ||
| if err != nil { | ||
| return statusHandler.FlushAndReturn(err) | ||
| } | ||
|
|
||
| // Disable the route check for external control plane topology (hypershift) if the ingress capability is disabled. | ||
| // The components will miss the required RBAC to implement the custom hostname or TLS. | ||
| // Link: https://github.com/openshift/enhancements/blob/f5290a98ea4f23f8e76621806b656a3849c74a17/enhancements/ingress/optional-ingress-hypershift.md#component-routes. | ||
| if util.IsExternalControlPlaneWithIngressDisabled(infrastructureConfig, clusterVersionConfig) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. by doing this you are basically disabling the operator from doing its work by syncing the console routes.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jhadvig: From what I see both of the functions used by the route controller ( @csrwng: can you please confirm that the component routes are not supported on HyperShift?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That is correct. If you've disabled ingress you are also indirectly disabling component routes, no?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right, I put my followup comment into a wrong conversation. Here it is: Also, in the absence of the ingress operator the required RBAC won't be created for the components (including the console operator): Component routes chapter of the EP. |
||
| return statusHandler.FlushAndReturn(nil) | ||
| } | ||
|
|
||
| ingressConfig, err := c.ingressConfigLister.Get(api.ConfigResourceName) | ||
| if err != nil { | ||
| return statusHandler.FlushAndReturn(err) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wondering if this could be also done automatically by the console-operator if ALB setup is detected? I see that
CERTIFICATE_ARNneeds to be provided though. Could we fetch it from some CM or Secret on the cluster?Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So far, we didn't see much automation around the certificate creation from the ALBO users. Probably because this requires quite some some effort to manage the cloud credentials. We can think about it as a followup enhancement.