feat: support OpenShift/CRC for local deploy - #967
Conversation
Signed-off-by: Mohamad Abo Ras <maboras@redhat.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughChangesThe deployment tooling now supports OpenShift clusters. It detects OpenShift domains, configures HTTPS Routes, manages internal registry images, validates existing contexts, and skips local cluster lifecycle operations. OpenShift deployment support
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant deploy_with_operator.sh
participant OpenShift cluster
participant OpenShift internal registry
participant operator deployment
deploy_with_operator.sh->>OpenShift cluster: set kubectl context
deploy_with_operator.sh->>OpenShift cluster: detect domain and configure Routes
deploy_with_operator.sh->>OpenShift internal registry: tag and push images
OpenShift internal registry-->>deploy_with_operator.sh: return internal image references
deploy_with_operator.sh->>operator deployment: patch operator image
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1⚔️ Resolve merge conflicts 💡
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@controller/hack/deploy_vars`:
- Around line 5-18: Update the BASEDOMAIN calculation around the else branch so
get_external_ip is only invoked when BASEDOMAIN is unset. Preserve a
user-supplied BASEDOMAIN unchanged, including for OpenShift, and retain the
existing nip.io derivation for cases where it is not provided.
In `@controller/hack/utils`:
- Around line 256-258: The podman login command on line 256 always uses
hardcoded podman, but the tag and push commands on lines 257-258 use the
CONTAINER_TOOL variable which may resolve to a different container tool.
Different tools have different credential stores and may not support the same
flags like --tls-verify. Update the login command to use the same
${CONTAINER_TOOL:-podman} variable as the tag and push operations, or require
podman explicitly for all three operations if that is the intended design.
- Around line 256-258: Update the registry authentication and push commands
around podman login and podman push to use the OpenShift registry CA certificate
and keep TLS verification enabled. Remove the unconditional --tls-verify=false
usage, or gate it behind an explicit local-development option for CRC-only
scenarios while preserving secure behavior by default.
- Line 256: Update the podman login command in the sourced utility flow to avoid
exposing the OpenShift token: temporarily disable shell xtrace while obtaining
and piping the token from oc whoami -t, restore the prior tracing state
afterward, and replace the -p argument with podman’s --password-stdin option.
- Around line 247-249: Update the image parsing logic near the name/tag
assignments to derive the repository name from the final path component before
splitting on its tag separator. Preserve registry ports and nested repository
paths, so images such as localhost:5000/team/controller:latest produce the
correct unique name and latest tag without collisions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f24b248b-fa2e-47fa-837c-9c5a3d207c3d
📒 Files selected for processing (4)
controller/Makefilecontroller/hack/deploy_varscontroller/hack/deploy_with_operator.shcontroller/hack/utils
|
I've created this PR for draft change. |
| # domain (e.g. apps-crc.testing), so we detect it instead of using a nip.io | ||
| # domain derived from the local outbound IP. | ||
| if [ -z "${BASEDOMAIN:-}" ] && [ "${CLUSTER_TYPE}" == "openshift" ]; then | ||
| OPENSHIFT_CLUSTER_DOMAIN=$(kubectl get ingresses.config.openshift.io cluster -o jsonpath='{.spec.domain}' 2>/dev/null || true) |
There was a problem hiding this comment.
nice, this can also be useful for real openshift clusters as well.
| export NETWORKING_MODE=${NETWORKING_MODE:-nodeport} | ||
| # OpenShift has no NodePort-friendly local port mapping, so Routes are the natural default there. | ||
| if [ "${CLUSTER_TYPE}" = "openshift" ]; then | ||
| export NETWORKING_MODE=${NETWORKING_MODE:-route} |
There was a problem hiding this comment.
👍 I was about to ask for this on the changes above, but it's handled here :-)
| openshift_registry_host() { | ||
| if [ -z "${_OPENSHIFT_REGISTRY_HOST:-}" ]; then | ||
| kubectl patch configs.imageregistry.operator.openshift.io/cluster \ | ||
| --type=merge -p '{"spec":{"defaultRoute":true}}' > /dev/null |
There was a problem hiding this comment.
We would enable the registry route even on a a real openshift, probably we should print a warning to let the user know. This would mostly be used in throw-away clusters... but I'd try to warn at least.
There was a problem hiding this comment.
ack, added a warning print before patching the registry route 4e8ff79
|
#967 (comment) That said, the image parsing fix is valid, it's about source image references with registry ports (e.g. |
|
@coderabbitai review |
✅ Action performedReview finished.
|
2 participants
Add OpenShift deploy support for controller and hack scripts
Summary
Add first-class OpenShift deployment support to the controller Makefile and hack deployment scripts so the operator and exporter-set controller can be deployed from a development workstation to OpenShift/CRC clusters.
Why
Local development on OpenShift/CRC requires a smooth deploy flow. Existing scripts assumed kind/k3s nodeport workflows and did not push images into the OpenShift internal registry or support OpenShift Routes.
Behavioral notes
Compatibility & risks
ocCLI and a container tool (podman/docker) with the locally-built image present and permissions to push to the cluster internal registry.