operator: implement routes and api detection #177
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
WalkthroughThis PR adds OpenShift Route support to the Jumpstarter controller by introducing API discovery to detect available Kubernetes resources, extending the endpoints reconciler to conditionally create Route objects as alternatives to Ingress, and refactoring pointer handling in deployment specifications using utility functions. Changes
Sequence DiagramsequenceDiagram
participant Ctrl as Controller
participant Recon as Reconciler
participant Disc as Discovery
participant API as Kubernetes API
Ctrl->>Recon: NewReconciler(client, scheme, config)
activate Recon
Recon->>Disc: discoverAPIResource(..., "networking.k8s.io/v1", "Ingress")
activate Disc
Disc->>API: Query ServerResourcesForGroupVersion
API-->>Disc: APIResourceList
Disc-->>Recon: IngressAvailable=true/false
deactivate Disc
Recon->>Disc: discoverAPIResource(..., "route.openshift.io/v1", "Route")
activate Disc
Disc->>API: Query ServerResourcesForGroupVersion
API-->>Disc: APIResourceList
Disc-->>Recon: RouteAvailable=true/false
deactivate Disc
Recon-->>Ctrl: Reconciler initialized
deactivate Recon
Note over Ctrl,Recon: During Reconciliation
Ctrl->>Recon: Reconcile(endpoint)
activate Recon
alt IngressAvailable
Recon->>API: Create/Update Ingress
end
alt RouteAvailable
Recon->>API: Create/Update Route
end
Recon-->>Ctrl: Reconciliation complete
deactivate Recon
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
bennyz
reviewed
Oct 29, 2025
| } | ||
|
|
||
| // int32Ptr returns a pointer to an int32 value | ||
| func int32Ptr(i int32) *int32 { |
Member
There was a problem hiding this comment.
can we use https://pkg.go.dev/k8s.io/utils/pointer for this?
Member
There was a problem hiding this comment.
(we should already have this dependency through all the k8s stuff)
Member
Author
There was a problem hiding this comment.
Done, also across some other instances in the operator.
bkhizgiy
reviewed
Oct 29, 2025
deploy/operator/internal/controller/jumpstarter/endpoints/discovery.go
Outdated
Show resolved
Hide resolved
bkhizgiy
reviewed
Oct 29, 2025
mangelajo
force-pushed
the
operator-ingress
branch
2 times, most recently
from
c1f42c4 to
3e595c8
Compare
mangelajo
force-pushed
the
autodetect-and-route-support
branch
from
dc075cd to
be25dc3
Compare
bkhizgiy
approved these changes
Oct 30, 2025
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
deploy/operator/internal/controller/jumpstarter/endpoints/route.go (1)
96-103: Clarify the route log messageThis branch logs “Skipping ingress creation” even though we’re handling Routes here. It’s confusing during debugging—please adjust the message to say “route” so operators know which path was skipped.
deploy/operator/internal/controller/jumpstarter/endpoints/endpoints.go (2)
44-60: Handle nil REST config to keep API discovery safeRight now we defer straight to
discoverAPIResource, so any caller that passes a nil or fake*rest.Config(common in unit tests) will panic inside the discovery client. A tiny guard that treats a missing config as “APIs unavailable” preserves the old behavior and avoids a crash.Apply this diff (or similar):
func NewReconciler(client client.Client, scheme *runtime.Scheme, config *rest.Config) *Reconciler { log := logf.Log.WithName("endpoints-reconciler") + if config == nil { + log.Info("REST config not provided; skipping API discovery", "ingressAvailable", false, "routeAvailable", false) + return &Reconciler{ + Client: client, + Scheme: scheme, + } + } + // Discover API availability at initialization ingressAvailable := discoverAPIResource(config, "networking.k8s.io/v1", "Ingress") routeAvailable := discoverAPIResource(config, "route.openshift.io/v1", "Route")
301-302: Please refresh the stale commentThis note still says “Route resources still need to be implemented,” but the Route code now exists just above. Updating or dropping it will prevent future confusion.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
deploy/operator/go.sumis excluded by!**/*.sum
📒 Files selected for processing (9)
deploy/operator/cmd/main.go(1 hunks)deploy/operator/go.mod(1 hunks)deploy/operator/internal/controller/jumpstarter/endpoints/discovery.go(1 hunks)deploy/operator/internal/controller/jumpstarter/endpoints/endpoints.go(4 hunks)deploy/operator/internal/controller/jumpstarter/endpoints/endpoints_test.go(2 hunks)deploy/operator/internal/controller/jumpstarter/endpoints/ingress.go(1 hunks)deploy/operator/internal/controller/jumpstarter/endpoints/route.go(1 hunks)deploy/operator/internal/controller/jumpstarter/jumpstarter_controller.go(5 hunks)deploy/operator/internal/controller/jumpstarter/jumpstarter_controller_test.go(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: NickCao
PR: jumpstarter-dev/jumpstarter-controller#137
File: deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/router-deployment.yaml:23-26
Timestamp: 2025-05-13T19:56:27.924Z
Learning: In the jumpstarter-controller project, the router service uses the same ConfigMap as the controller service (controller-cm.yaml) even though it has been moved to its own separate deployment.
Learnt from: mangelajo
PR: jumpstarter-dev/jumpstarter-controller#170
File: deploy/operator/internal/controller/jumpstarter/jumpstarter_controller.go:328-333
Timestamp: 2025-10-24T11:57:23.796Z
Learning: In the jumpstarter-controller operator (deploy/operator/), the design allows only one Jumpstarter CR per namespace, which will be enforced by a validation webhook. This constraint eliminates concerns about resource name collisions within a namespace.
Learnt from: NickCao
PR: jumpstarter-dev/jumpstarter-controller#137
File: hack/deploy_with_helm.sh:26-34
Timestamp: 2025-05-13T19:57:56.811Z
Learning: The jumpstarter project uses a custom kind cluster configuration with an expanded NodePort range (3000-32767) and explicit port mappings for ingress (5080/5443) and gRPC services (30010/30011 mapped to 8082/8083).
Learnt from: NickCao
PR: jumpstarter-dev/jumpstarter-controller#137
File: hack/deploy_with_helm.sh:26-34
Timestamp: 2025-05-13T19:57:56.811Z
Learning: The jumpstarter-dev repository uses a custom kind cluster configuration that allows NodePort services to use non-standard ports 5080 and 5443, outside the default Kubernetes NodePort range (30000-32767).
📚 Learning: 2025-10-24T11:57:23.796Z
Learnt from: mangelajo
PR: jumpstarter-dev/jumpstarter-controller#170
File: deploy/operator/internal/controller/jumpstarter/jumpstarter_controller.go:328-333
Timestamp: 2025-10-24T11:57:23.796Z
Learning: In the jumpstarter-controller operator (deploy/operator/), the design allows only one Jumpstarter CR per namespace, which will be enforced by a validation webhook. This constraint eliminates concerns about resource name collisions within a namespace.
Applied to files:
deploy/operator/cmd/main.godeploy/operator/internal/controller/jumpstarter/endpoints/endpoints.go
🧬 Code graph analysis (6)
deploy/operator/cmd/main.go (1)
deploy/operator/internal/controller/jumpstarter/endpoints/endpoints.go (1)
NewReconciler(44-61)
deploy/operator/internal/controller/jumpstarter/endpoints/endpoints_test.go (1)
deploy/operator/internal/controller/jumpstarter/endpoints/endpoints.go (1)
NewReconciler(44-61)
deploy/operator/internal/controller/jumpstarter/endpoints/route.go (3)
deploy/operator/internal/controller/jumpstarter/endpoints/endpoints.go (1)
Reconciler(36-41)deploy/helm/jumpstarter/charts/jumpstarter-controller/model.py (1)
Route(85-90)deploy/operator/internal/utils/utils.go (1)
MergeMaps(5-19)
deploy/operator/internal/controller/jumpstarter/jumpstarter_controller_test.go (1)
deploy/operator/internal/controller/jumpstarter/endpoints/endpoints.go (1)
NewReconciler(44-61)
deploy/operator/internal/controller/jumpstarter/endpoints/endpoints.go (1)
deploy/helm/jumpstarter/charts/jumpstarter-controller/model.py (1)
Route(85-90)
deploy/operator/internal/controller/jumpstarter/endpoints/discovery.go (1)
internal/log/levels.go (1)
Error(51-53)
🪛 OSV Scanner (2.2.3)
deploy/operator/go.mod
[HIGH] 10-10: github.com/golang-jwt/jwt/v5 5.2.1: Excessive memory allocation during header parsing in github.com/golang-jwt/jwt
(GO-2025-3553)
[HIGH] 10-10: github.com/golang-jwt/jwt/v5 5.2.1: jwt-go allows excessive memory allocation during header parsing
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: deploy-kind
- GitHub Check: deploy-with-operator
- GitHub Check: e2e-tests (ubuntu-24.04-arm)
- GitHub Check: e2e-tests-release-0-7
- GitHub Check: e2e-tests (ubuntu-24.04)
- GitHub Check: lint-go
- GitHub Check: tests
🔇 Additional comments (2)
deploy/operator/cmd/main.go (1)
218-223: Config wiring confirmedPassing
mgr.GetConfig()keeps the reconciler aligned with the new signature and ensures discovery has the right REST config context. Thanks for updating the constructor call accordingly.deploy/operator/internal/controller/jumpstarter/endpoints/endpoints.go (1)
155-162: Route creation path looks solidThanks for mirroring the ingress flow for Routes; reusing the ClusterIP service keeps things consistent and should make OpenShift users happy.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit implements route detection
requires #176 (builds on top)
Summary by CodeRabbit
New Features
Chores