-
Notifications
You must be signed in to change notification settings - Fork 74
feat: implement Gateway API routing class for DevWorkspaceRouting #1680
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
base: main
Are you sure you want to change the base?
Changes from all commits
1f5688d
e6409ee
9b06672
5c9e528
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 | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -119,6 +119,23 @@ type BackupCronJobConfig struct { | |||||||||
| BackoffLimit *int32 `json:"backoffLimit,omitempty"` | ||||||||||
| } | ||||||||||
|
|
||||||||||
| // GatewayReference defines a reference to a Gateway API Gateway resource | ||||||||||
| // that HTTPRoutes should attach to via parentRefs. | ||||||||||
| type GatewayReference struct { | ||||||||||
| // Name is the name of the Gateway resource | ||||||||||
| // +kubebuilder:validation:Required | ||||||||||
| // +kubebuilder:validation:MinLength=1 | ||||||||||
| // +kubebuilder:validation:MaxLength=253 | ||||||||||
| Name string `json:"name"` | ||||||||||
| // Namespace is the namespace of the Gateway resource. | ||||||||||
| // If not specified, HTTPRoutes will reference a Gateway in the same namespace | ||||||||||
| // as the DevWorkspace. | ||||||||||
| // +kubebuilder:validation:Optional | ||||||||||
|
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. nit, Perhaps add DNS-1123 label bounds here too
Suggested change
Collaborator
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. Agreed, adding bounds. |
||||||||||
| // +kubebuilder:validation:MinLength=1 | ||||||||||
| // +kubebuilder:validation:MaxLength=63 | ||||||||||
| Namespace *string `json:"namespace,omitempty"` | ||||||||||
| } | ||||||||||
|
|
||||||||||
| type RoutingConfig struct { | ||||||||||
| // DefaultRoutingClass specifies the routingClass to be used when a DevWorkspace | ||||||||||
| // specifies an empty `.spec.routingClass`. Supported routingClasses can be defined | ||||||||||
|
|
@@ -144,6 +161,12 @@ type RoutingConfig struct { | |||||||||
| // TLSCertificateConfigmapRef defines the name and namespace of the configmap with a certificate to inject into the | ||||||||||
| // HTTP client. | ||||||||||
| TLSCertificateConfigmapRef *ConfigmapReference `json:"tlsCertificateConfigmapRef,omitempty"` | ||||||||||
| // GatewayRef defines a reference to a Gateway API Gateway resource that HTTPRoutes | ||||||||||
| // should attach to when using the 'gateway-api' routing class. This field is required | ||||||||||
| // when routingClass is set to 'gateway-api'. The referenced Gateway must be provisioned | ||||||||||
| // by the cluster administrator or Che Operator before workspaces can use Gateway API routing. | ||||||||||
|
Collaborator
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.
Suggested change
Collaborator
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. To provide context, in DWO we try to minimize any mention of Eclipse Che, since technically, Che is not a dependency of DWO |
||||||||||
| // +kubebuilder:validation:Optional | ||||||||||
| GatewayRef *GatewayReference `json:"gatewayRef,omitempty"` | ||||||||||
| } | ||||||||||
|
|
||||||||||
| // OverrideConfig defines configuration options for controlling which fields are restricted | ||||||||||
|
|
||||||||||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -41,6 +41,7 @@ import ( | |
| "sigs.k8s.io/controller-runtime/pkg/controller" | ||
| "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" | ||
| "sigs.k8s.io/controller-runtime/pkg/reconcile" | ||
| gwapiv1 "sigs.k8s.io/gateway-api/apis/v1" | ||
|
|
||
| controllerv1alpha1 "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1" | ||
| ) | ||
|
|
@@ -69,6 +70,7 @@ type DevWorkspaceRoutingReconciler struct { | |
| // +kubebuilder:rbac:groups=route.openshift.io,resources=routes,verbs=* | ||
| // +kubebuidler:rbac:groups=route.openshift.io,resources=routes/status,verbs=get,list,watch | ||
| // +kubebuilder:rbac:groups=route.openshift.io,resources=routes/custom-host,verbs=create | ||
| // +kubebuilder:rbac:groups=gateway.networking.k8s.io,resources=httproutes,verbs=get;list;watch;create;update;patch;delete | ||
|
|
||
| func (r *DevWorkspaceRoutingReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { | ||
| reqLogger := r.Log.WithValues("Request.Namespace", req.Namespace, "Request.Name", req.Name) | ||
|
|
@@ -184,6 +186,16 @@ func (r *DevWorkspaceRoutingReconciler) Reconcile(ctx context.Context, req ctrl. | |
| routes[idx].Annotations = maputils.Append(routes[idx].Annotations, constants.DevWorkspaceRestrictedAccessAnnotation, restrictedAccess) | ||
| } | ||
| } | ||
| httpRoutes := routingObjects.HTTPRoutes | ||
| for idx := range httpRoutes { | ||
| err := controllerutil.SetControllerReference(instance, &httpRoutes[idx], r.Scheme) | ||
| if err != nil { | ||
| return reconcile.Result{}, err | ||
| } | ||
| if setRestrictedAccess { | ||
| httpRoutes[idx].Annotations = maputils.Append(httpRoutes[idx].Annotations, constants.DevWorkspaceRestrictedAccessAnnotation, restrictedAccess) | ||
| } | ||
| } | ||
|
|
||
| servicesInSync, clusterServices, err := r.syncServices(instance, services) | ||
| if err != nil { | ||
|
|
@@ -232,12 +244,34 @@ func (r *DevWorkspaceRoutingReconciler) Reconcile(ctx context.Context, req ctrl. | |
| clusterRoutingObj.Ingresses = clusterIngresses | ||
| } | ||
|
|
||
| // Sync HTTPRoutes if using gateway-api routing class | ||
| if len(httpRoutes) > 0 { | ||
| httpRoutesInSync, clusterHTTPRoutes, err := r.syncHTTPRoutes(instance, httpRoutes) | ||
| if err != nil { | ||
| failError := &sync.UnrecoverableSyncError{} | ||
| if errors.As(err, &failError) { | ||
| return reconcile.Result{}, r.markRoutingFailed(instance, err.Error()) | ||
| } | ||
| reqLogger.Error(err, "Error syncing HTTPRoutes") | ||
| return reconcile.Result{Requeue: true}, r.reconcileStatus(instance, nil, nil, false, "Preparing HTTPRoutes") | ||
| } else if !httpRoutesInSync { | ||
| reqLogger.Info("HTTPRoutes not in sync") | ||
| return reconcile.Result{Requeue: true}, r.reconcileStatus(instance, nil, nil, false, "Preparing HTTPRoutes") | ||
| } | ||
| clusterRoutingObj.HTTPRoutes = clusterHTTPRoutes | ||
| } | ||
|
|
||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| exposedEndpoints, endpointsAreReady, err := solver.GetExposedEndpoints(instance.Spec.Endpoints, clusterRoutingObj) | ||
| if err != nil { | ||
| reqLogger.Error(err, "Could not get exposed endpoints for devworkspace") | ||
| return reconcile.Result{}, r.markRoutingFailed(instance, fmt.Sprintf("Could not get exposed endpoints for DevWorkspace: %s", err)) | ||
| } | ||
|
|
||
| if !endpointsAreReady { | ||
| reqLogger.Info("Endpoints not ready, requeuing") | ||
| return reconcile.Result{RequeueAfter: 3 * time.Second}, r.reconcileStatus(instance, nil, nil, false, "Waiting for endpoints to be ready") | ||
| } | ||
|
Collaborator
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. I'm not sure if a requeue is necessary here, since there nothing in the reconciliation loop that should happen after endpointsAreReady goes from false to true? I could be missing something but is there a reason to requeue? |
||
|
|
||
| return reconcile.Result{}, r.reconcileStatus(instance, &routingObjects, exposedEndpoints, endpointsAreReady, "") | ||
| } | ||
|
|
||
|
|
@@ -351,6 +385,9 @@ func (r *DevWorkspaceRoutingReconciler) SetupWithManager(mgr ctrl.Manager) error | |
| if infrastructure.IsOpenShift() { | ||
| bld.Owns(&routeV1.Route{}) | ||
| } | ||
| if infrastructure.IsGatewayAPIInstalled() { | ||
| bld.Owns(&gwapiv1.HTTPRoute{}) | ||
| } | ||
| if r.SolverGetter == nil { | ||
| return NoSolversEnabled | ||
| } | ||
|
|
||
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.
Required only checks presence; maybe add length bounds?
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.
Good catch, I will add the length bounds.