ROX-35435: Support differing versions of Central and SecuredCluster#246
Open
vladbologa wants to merge 5 commits into
Open
ROX-35435: Support differing versions of Central and SecuredCluster#246vladbologa wants to merge 5 commits into
vladbologa wants to merge 5 commits into
Conversation
Contributor
📝 WalkthroughWalkthroughPer-component Central and SecuredCluster versions now drive image generation, operator instance planning, validation, Konflux variables, deployment orchestration, and deployment summaries. ChangesSplit Operator Support
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant DeployCommand
participant Config
participant ImageBuilder
participant OperatorDeployment
participant Kubernetes
DeployCommand->>Config: apply component version flags
Config->>Config: build OperatorInstances()
Config-->>ImageBuilder: effective versions and operator instances
ImageBuilder-->>DeployCommand: versioned image list
DeployCommand->>OperatorDeployment: ensure operator instances
OperatorDeployment->>Kubernetes: inspect, deploy, or tear down namespaces
Kubernetes-->>OperatorDeployment: deployment state
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
✅ Action performedReview finished.
|
![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: 6
🤖 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 `@internal/deployer/acs_images.go`:
- Around line 38-40: Update the image collection loop in the deployer flow
around OperatorInstances so single-operator configurations consistently use the
explicit Operator.Version override instead of mixing it with the resolved
instance version; alternatively, unify OperatorBundleImageForVersion and the OLM
Konflux environment-variable path on the same resolved version. Add a regression
test covering both Roxie.Version and Operator.Version being set, ensuring all
generated image references use one consistent operator version.
In `@internal/deployer/deploy_via_operator.go`:
- Around line 138-141: Update the early-return logic in the deployment flow
around isOperatorVersionCorrect so it also verifies the manager environment
matches instance.EnvVars, including component-specific reconciler flags and
Konflux/custom variables. Only return when both the image version and
environment are correct; otherwise reapply the desired Deployment using the
existing operator deployment path, preserving the split-mode flags defined by
the operator instance plan.
- Around line 110-125: The cleanup loop in deploy_via_operator.go around
AllOperatorNamespaces must attempt teardown for every undesired known namespace
without gating on operatorDeploymentExists or namespaceExists, allowing
teardownOperatorNonOLMInNamespace to remove namespaced and cluster-scoped
resources idempotently. In operator.go around the non-OLM cleanup flow, invoke
cleanup even when no Deployment is detected so partial resources are removed;
apply the change at both affected sites.
In `@internal/deployer/operator_instance.go`:
- Around line 78-82: Update the single-operator path in NeedsSplitOperators to
derive the operator version from the effective component version, honoring equal
Roxie and Operator overrides instead of using Roxie.Version directly. Preserve
the fallback behavior when no converted version is available, and extend
TestEffectiveVersions_BothOverridesSame_NoSplit with an assertion that the
deployed operator uses the override version.
In `@internal/deployer/operator.go`:
- Around line 575-598: Update the teardown flow around the kubectl deletion
calls and waitForNamespaceDeletion to propagate genuine deletion or timeout
errors instead of ignoring them or returning nil. Keep --ignore-not-found=true
for idempotent cleanup, but capture each runKubectl error and return it
immediately; likewise return the waitForNamespaceDeletion error before logging
success. Only log the success message and return nil after all cleanup steps
complete successfully.
- Around line 217-222: Update the EnvVars logging block in the operator
deployment flow to stop emitting environment values, since entries may contain
credentials or tokens. Preserve the existing count log and, if individual
entries are still needed, log only each variable’s name from envVarsToSortedList
without accessing or formatting ev["value"].
🪄 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: Path: .coderabbit.yml
Review profile: CHILL
Plan: Enterprise
Run ID: b2693e61-392e-405e-a4bb-6dd14e9c6436
📒 Files selected for processing (10)
cmd/deploy.gointernal/deployer/acs_images.gointernal/deployer/config.gointernal/deployer/deploy_via_operator.gointernal/deployer/deployer.gointernal/deployer/konflux.gointernal/deployer/konflux_test.gointernal/deployer/operator.gointernal/deployer/operator_instance.gointernal/deployer/operator_instance_test.go
vladbologa
force-pushed
the
vb/differing-central-sc-versions
branch
from
July 21, 2026 12:01
3c45e8d to
3776248
Compare
vladbologa
marked this pull request as ready for review
July 21, 2026 12:01
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
internal/deployer/operator_instance_test.go (1)
74-109: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake override fixtures distinct from the global version.
These cases can pass if downstream planning accidentally uses
Roxie.Versionfor a component, because it equals one expected component version.
internal/deployer/operator_instance_test.go#L74-L109: use a third Roxie version and verify the sensor instance still usesSecuredCluster.Version.internal/deployer/operator_instance_test.go#L123-L139: setRoxie.Versionbelow the Central override so this case proves Central participates in newest-version selection.internal/deployer/operator_instance_test.go#L151-L165: use a third Roxie version and assert no global-version-only images are collected when both components override it.🤖 Prompt for 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. In `@internal/deployer/operator_instance_test.go` around lines 74 - 109, The operator instance tests use Roxie versions that can mask incorrect global-version handling. In internal/deployer/operator_instance_test.go:74-109, choose a distinct third Roxie version and keep asserting the sensor uses SecuredCluster.Version; at 123-139, set Roxie.Version below the Central override to verify Central drives newest-version selection; at 151-165, use a distinct Roxie version and assert no images for the global-only version are collected when both components override it.
🤖 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.
Nitpick comments:
In `@internal/deployer/operator_instance_test.go`:
- Around line 74-109: The operator instance tests use Roxie versions that can
mask incorrect global-version handling. In
internal/deployer/operator_instance_test.go:74-109, choose a distinct third
Roxie version and keep asserting the sensor uses SecuredCluster.Version; at
123-139, set Roxie.Version below the Central override to verify Central drives
newest-version selection; at 151-165, use a distinct Roxie version and assert no
images for the global-only version are collected when both components override
it.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Enterprise
Run ID: 57f74974-e5ca-442d-b25f-d4c9e9bd1f49
📒 Files selected for processing (10)
cmd/deploy.gointernal/deployer/acs_images.gointernal/deployer/config.gointernal/deployer/deploy_via_operator.gointernal/deployer/deployer.gointernal/deployer/konflux.gointernal/deployer/konflux_test.gointernal/deployer/operator.gointernal/deployer/operator_instance.gointernal/deployer/operator_instance_test.go
🚧 Files skipped from review as they are similar to previous changes (9)
- internal/deployer/config.go
- internal/deployer/konflux.go
- internal/deployer/konflux_test.go
- internal/deployer/operator_instance.go
- internal/deployer/deployer.go
- cmd/deploy.go
- internal/deployer/acs_images.go
- internal/deployer/operator.go
- internal/deployer/deploy_via_operator.go
There was a problem hiding this comment.
Pull request overview
Adds mixed-version deployment support so Roxie can deploy Central and SecuredCluster at different versions on the same cluster, while preserving the existing single-version behavior.
Changes:
- Introduces per-component version overrides (
central.version/securedCluster.version, plus CLI flags) and resolves effective versions from config. - Enables dual non-OLM operator deployments (separate namespaces + per-instance reconciler toggles + distinct cluster-scoped RBAC names) and improves teardown/mode switching logic.
- Updates image planning (including operator/bundle images) and deployment summaries to reflect effective component versions.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/deployer/operator.go | Refactors non-OLM operator deployment/teardown to support multiple operator instances and per-instance RBAC names; gates CRD apply to the newest planned operator version. |
| internal/deployer/operator_instance.go | Adds effective version resolution, mixed-version detection, operator instance planning, and “newest operator version” selection. |
| internal/deployer/operator_instance_test.go | Adds unit tests for effective versions, operator instance planning, newest-version selection, and mixed-version image planning. |
| internal/deployer/konflux.go | Refactors Konflux env-var population to support per-instance operator versions (merge-style API). |
| internal/deployer/konflux_test.go | Updates tests for new Konflux APIs and adds coverage for per-instance version merging. |
| internal/deployer/deployer.go | Updates deployment summaries to display effective Central/SecuredCluster versions rather than the global Roxie version. |
| internal/deployer/deploy_via_operator.go | Extends operator deployment logic to deploy/validate multiple non-OLM operator instances and handle transitions between modes/namespaces. |
| internal/deployer/config.go | Adds version override fields to Central and SecuredCluster config structs. |
| internal/deployer/acs_images.go | Updates image selection to include unique effective main versions plus per-instance operator/bundle images (deduped). |
| cmd/deploy.go | Adds CLI flags for per-component tags, validates unsupported mode combinations for mixed versions, and adjusts version-gated checks/Konflux behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
vladbologa
force-pushed
the
vb/differing-central-sc-versions
branch
from
July 21, 2026 14:07
3776248 to
d63b2af
Compare
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
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.
Description
Roxie can now deploy Central and SecuredCluster at different versions on the same cluster (for compatibility testing), while keeping the single-version path unchanged.
Config and CLI
roxie.version/--tagremains the global default.central.version/--central-tagandsecuredCluster.version/--secured-cluster-tag.EffectiveCentralVersion()/EffectiveSecuredClusterVersion(). When they differ,HasMixedVersions()is true.--tag/roxie.version.Operators
When versions match (common case):
rhacs-operator-system(unchanged).When versions differ:
rhacs-operator-centralwithSECURED_CLUSTER_RECONCILER_ENABLED=false.rhacs-operator-sensorwithCENTRAL_RECONCILER_ENABLED=false.…-central/…-sensor).This is derived automatically from version mismatch; users do not configure reconciler toggles or operator namespaces.
Limitations
--olm/ OLM deploy are not supported (hard error).roxie deploy operator(operator-only) are not supported (hard error).Approaches considered
roxie deployinvocationsCENTRAL_LABEL_SELECTOR/ …)Manual testing on an infra cluster
4.11.1, SecuredCluster4.10.0. Confirmed dual operators, correct reconciler toggles, matching product images;rhacs-operator-systemunused.roxie teardownremoved both mixed operator namespaces and component CRs/workloads.--tag 4.11.1only. Single operator inrhacs-operator-system, no reconciler toggles, no double namespaces.4.11.1/ SecuredCluster4.10.0.rhacs-operator-systemremoved; mixed operators stood up correctly.TBD: Check switching from OLM to mixed versions, and back.
Summary by CodeRabbit
New Features
Bug Fixes
Tests