[release-controller] ARM64 support for OKD releases - #83116
Conversation
rh-pre-commit.version: 2.4.0 rh-pre-commit.check-secrets: ENABLED
openshift-merge-bot
Bot
added
the
rehearsals-ack
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe release-controller generator now supports OCP and OKD products across configured architectures and privacy modes. It generates product-specific RBAC, namespaces, deployments, service accounts, ARM64 resources, and legacy compatibility deployments. ChangesRelease-controller productization
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 14 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (14 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
hack/generators/release-controllers/content/art_namespaces_rbac.py (1)
229-231: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winThe prowjob
Rolename stays product-independent and now collides across products.Line 229 builds the name from
context.suffixonly. Line 230 places the Role in the sharedcideployment namespace. OCP and OKD now run for the same architectures, so both products emit a Role namedrelease-controller-prowjobinciforx86_64andrelease-controller-arm64-prowjobinciforarm64. The generated fileclusters/app.ci/release-controller/admin_deploy-origin-controller.yamlat lines 125-142 shows the OKD copy. Two applied files then own one cluster object.The rules are identical today, so behavior does not change. The ownership is still ambiguous. Add the product prefix, as done for the sibling names on lines 211 and 249. Update the
roleRefon line 333 in the same change.🔧 Proposed fix
- 'name': f'release-controller{context.suffix}-prowjob', + 'name': f'release-controller-{context.product.deployment_prefix}{context.suffix}-prowjob', 'namespace': context.config.rc_deployment_namespace,Apply the matching change to the
roleRefat line 333:- 'name': f'release-controller{context.suffix}-prowjob' + 'name': f'release-controller-{context.product.deployment_prefix}{context.suffix}-prowjob'🤖 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 `@hack/generators/release-controllers/content/art_namespaces_rbac.py` around lines 229 - 231, Update the prowjob Role name in the relevant generator block to include the product prefix, matching the sibling naming patterns near lines 211 and 249, while preserving the existing suffix and namespace. Update the corresponding roleRef target near line 333 to reference the exact same product-qualified Role name.clusters/app.ci/release-controller/deploy-origin-controller.yaml (1)
14-23: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick winDelete the old Route during the cutover.
The previous manifest defines
Route/release-controllerwith the same host. A normal apply does not delete it. Delete the old Route, or use an applier that prunes removed objects, before applyingRoute/release-controller-okd. Otherwise, OpenShift rejects the new Route withHostAlreadyClaimed, and the old Route targets the zero-replicarelease-controller-api.🤖 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 `@clusters/app.ci/release-controller/deploy-origin-controller.yaml` around lines 14 - 23, Remove the legacy Route/release-controller as part of the cutover before applying Route/release-controller-okd, or configure the deployment applier to prune removed objects. Ensure the old Route no longer claims the shared host and does not continue targeting release-controller-api.
🧹 Nitpick comments (3)
hack/generators/release-controllers/generate-release-controllers.py (1)
44-53: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOptional: move the per-product generator selection onto
Product.The loop dispatches on
product.namewith string comparison. A third product requires a newelifbranch here and in each RBAC generator. Consider storing the deployment generator callables on theProductinstance instead.🤖 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 `@hack/generators/release-controllers/generate-release-controllers.py` around lines 44 - 53, Optionally refactor the per-product deployment dispatch in the generator loop to use generator callables stored on the Product instance instead of comparing product.name. Add the appropriate deployment generator selection to Product and invoke it from this flow, preserving the existing OCP and OKD generation behavior and enabling additional products without new branches.hack/generators/release-controllers/content/development_rbac.py (1)
139-142: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winEmit the
cimonitoring RBAC explicitly instead of attaching it to the first OKD file.The
ci_monitoring_generatedflag places thecinamespace monitoring Role and binding in whichever file the firstokdcontext produces. That file isclusters/app.ci/release-controller/admin-origin-rbac.yamltoday. If theokdproduct is renamed or removed, thecimonitoring RBAC disappears from all generated output and no error is raised.Generate the
cimonitoring resources in a dedicated one-off document, in the same waygenerate_app_ci_contenthandles other cluster-wide resources.🤖 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 `@hack/generators/release-controllers/content/development_rbac.py` around lines 139 - 142, Replace the ci_monitoring_generated condition in the release-controller generation flow with a dedicated one-off document for the ci monitoring Role and binding. Follow the generate_app_ci_content pattern for emitting cluster-wide resources, and remove the dependency on product.name == 'okd' so the ci monitoring RBAC is generated explicitly even when OKD contexts are renamed or absent.hack/generators/release-controllers/content/origin_resources.py (1)
4-84: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePlan the removal of the legacy scaled-down Deployments.
These two Deployments exist only to drain the pre-rename workloads. After the rollout completes, they become permanent dead objects in the generated output. Add a tracking issue or a TODO with the removal condition.
The Keel annotations also have no effect while
replicasis 0. Keel continues to poll and patch the image on objects that never run pods. Consider dropping the fourkeel.sh/*annotations from these two objects.♻️ Proposed simplification of the legacy objects
gendoc.append({ "apiVersion": "apps/v1", "kind": "Deployment", "metadata": { - "annotations": { - 'keel.sh/policy': 'force', - 'keel.sh/matchTag': 'true', - 'keel.sh/trigger': 'poll', - 'keel.sh/pollSchedule': '`@every` 5m' - }, "name": "release-controller", "namespace": context.config.rc_deployment_namespace, },🤖 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 `@hack/generators/release-controllers/content/origin_resources.py` around lines 4 - 84, Update add_legacy_origin_deployments_scaled_down to document a removal condition with a TODO or tracking issue, and remove the four keel.sh annotations from both legacy scaled-down Deployment metadata blocks while preserving their drain-only replicas: 0 behavior.
🤖 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
`@clusters/app.ci/release-controller/admin_deploy-origin-arm64-controller.yaml`:
- Around line 202-205: Add a Kubernetes NetworkPolicy resource targeting the
ci-release-arm64 namespace, alongside the existing Namespace manifest. Configure
it according to the repository’s established NetworkPolicy conventions so the
namespace is covered without changing the existing Namespace definition.
In `@clusters/app.ci/release-controller/admin-origin-arm64-rbac.yaml`:
- Around line 65-72: Update the Jobs RBAC rule in development_rbac.py to use the
batch API group instead of batch/v1, then regenerate the affected
release-controller RBAC manifests so the generated files grant the intended Jobs
permissions.
In `@clusters/app.ci/release-controller/deploy-origin-arm64-controller.yaml`:
- Around line 61-206: Update add_okd_deployments in origin_resources.py to add
pod-level runAsNonRoot: true, container securityContext settings, and CPU and
memory limits for every generated container, then regenerate the manifests.
Apply the changes to deploy-origin-arm64-controller.yaml lines 61-206 for
git-sync-init, git-sync, and controller, and lines 211-307 for controller; both
sites require the same updates.
In `@hack/generators/release-controllers/config/__init__.py`:
- Around line 136-151: Make release-controller resource names unique per
product: in hack/generators/release-controllers/config/__init__.py:136-151,
include product.deployment_prefix in jobs_namespace, secret_name_tls, and
secret_name_tls_api. In
hack/generators/release-controllers/content/art_namespaces_rbac.py:229-231,
apply the same product prefix to the prowjob Role name and matching roleRef.
Regenerate clusters/app.ci/release-controller/admin-origin-rbac.yaml:99-158 from
the updated generator so the resulting resources no longer overlap.
---
Outside diff comments:
In `@clusters/app.ci/release-controller/deploy-origin-controller.yaml`:
- Around line 14-23: Remove the legacy Route/release-controller as part of the
cutover before applying Route/release-controller-okd, or configure the
deployment applier to prune removed objects. Ensure the old Route no longer
claims the shared host and does not continue targeting release-controller-api.
In `@hack/generators/release-controllers/content/art_namespaces_rbac.py`:
- Around line 229-231: Update the prowjob Role name in the relevant generator
block to include the product prefix, matching the sibling naming patterns near
lines 211 and 249, while preserving the existing suffix and namespace. Update
the corresponding roleRef target near line 333 to reference the exact same
product-qualified Role name.
---
Nitpick comments:
In `@hack/generators/release-controllers/content/development_rbac.py`:
- Around line 139-142: Replace the ci_monitoring_generated condition in the
release-controller generation flow with a dedicated one-off document for the ci
monitoring Role and binding. Follow the generate_app_ci_content pattern for
emitting cluster-wide resources, and remove the dependency on product.name ==
'okd' so the ci monitoring RBAC is generated explicitly even when OKD contexts
are renamed or absent.
In `@hack/generators/release-controllers/content/origin_resources.py`:
- Around line 4-84: Update add_legacy_origin_deployments_scaled_down to document
a removal condition with a TODO or tracking issue, and remove the four keel.sh
annotations from both legacy scaled-down Deployment metadata blocks while
preserving their drain-only replicas: 0 behavior.
In `@hack/generators/release-controllers/generate-release-controllers.py`:
- Around line 44-53: Optionally refactor the per-product deployment dispatch in
the generator loop to use generator callables stored on the Product instance
instead of comparing product.name. Add the appropriate deployment generator
selection to Product and invoke it from this flow, preserving the existing OCP
and OKD generation behavior and enabling additional products without new
branches.
🪄 Autofix
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: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 0e84363c-86fa-4908-96a7-d76e404f58df
📒 Files selected for processing (16)
clusters/app.ci/crt/admin_generated_rbac.yamlclusters/app.ci/release-controller/admin-origin-arm64-rbac.yamlclusters/app.ci/release-controller/admin-origin-rbac.yamlclusters/app.ci/release-controller/admin_deploy-origin-arm64-controller.yamlclusters/app.ci/release-controller/admin_deploy-origin-controller.yamlclusters/app.ci/release-controller/deploy-origin-arm64-controller.yamlclusters/app.ci/release-controller/deploy-origin-controller.yamlclusters/app.ci/trt/admin_generated_rbac.yamlhack/generators/release-controllers/config/__init__.pyhack/generators/release-controllers/content/__init__.pyhack/generators/release-controllers/content/art_namespaces_rbac.pyhack/generators/release-controllers/content/development_rbac.pyhack/generators/release-controllers/content/origin_resources.pyhack/generators/release-controllers/content/rbac_resources.pyhack/generators/release-controllers/content/trt_rbac.pyhack/generators/release-controllers/generate-release-controllers.py
rh-pre-commit.version: 2.4.0 rh-pre-commit.check-secrets: ENABLED
rh-pre-commit.version: 2.4.0 rh-pre-commit.check-secrets: ENABLED
|
/label tide/merge-method-squash |
openshift-ci
Bot
added
the
tide/merge-method-squash
|
[REHEARSALNOTIFIER] Note: If this PR includes changes to step registry files ( Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
/hold |
openshift-ci
Bot
added
the
do-not-merge/hold
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bradmwilliams, jupierce The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
|
/unhold |
openshift-ci
Bot
removed
the
do-not-merge/hold
|
@bradmwilliams: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
2 participants
This PR adds a product abstraction to release controller generator and adds support for a brand new release-controller for ARM64 OKD releases.
Summary
rh-pre-commit.version: 2.4.0
rh-pre-commit.check-secrets: ENABLED
Summary by CodeRabbit
origin-arm64namespace.originandorigin-arm64.batchAPI group.