OCPBUGS-86059: Implement automatic LVMS rebase on the latest z-stream version#6940
OCPBUGS-86059: Implement automatic LVMS rebase on the latest z-stream version#6940ggiguash wants to merge 1 commit into
Conversation
openshift-ci-robot
added
jira/severity-important
|
@ggiguash: This pull request references Jira Issue OCPBUGS-86059, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this: Instructions 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 openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe PR adds a latest-tag LVMS rebase command, renames the generated wrapper script, updates asset recipe filenames, and refreshes LVMS rebase documentation. ChangesLVMS rebase flow
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error, 1 warning)
✅ Passed checks (13 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 |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ggiguash 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
|
/test ? |
|
@ggiguash: This pull request references Jira Issue OCPBUGS-86059, which is valid. 3 validation(s) were run on this bug
DetailsIn response to this:
Instructions 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 openshift-eng/jira-lifecycle-plugin repository. |
|
/test e2e-aws-tests |
|
/test ? |
|
/test test-rebase |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
assets/components/lvms/lvms-operator_rbac.authorization.k8s.io_v1_clusterrole.yaml (1)
47-52: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winSplit
podsout soupdatedoes not apply to them.Line 47 now shares the PVC verb set, so
podsalso getupdate. Keepupdatescoped to PVCs unless the operator has a proven pod-update path.Proposed RBAC split
- apiGroups: - "" resources: - persistentvolumeclaims - - pods verbs: - delete - get - list - update - watch + - apiGroups: + - "" + resources: + - pods + verbs: + - delete + - get + - list + - watchAs per path instructions, “RBAC: least privilege; no cluster-admin for workloads.”
🤖 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 `@assets/components/lvms/lvms-operator_rbac.authorization.k8s.io_v1_clusterrole.yaml` around lines 47 - 52, The ClusterRole rule currently groups pods with the same verb set as PVCs, which unintentionally grants pods the update verb. Split the RBAC rules in the ClusterRole manifest so the pods resource has only the intended verbs and update remains scoped to the PVC rule; use the existing pods and PVC entries in the role definition to separate them cleanly.Source: Path instructions
🤖 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 `@assets/components/lvms/lvm.topolvm.io_lvmclusters.yaml`:
- Around line 222-257: The immutability validations for additionalParameters,
reclaimPolicy, and volumeBindingMode are too strict for optional/defaulted
fields because oldSelf == self can reject a valid initial set when the previous
value is unset. Update the x-kubernetes-validations rules on these fields in the
LVMS schema to account for missing old values, using a pattern like
!has(oldSelf) || oldSelf == self so the fields can be set once and remain
immutable afterward. Locate the changes under the additionalParameters,
reclaimPolicy, and volumeBindingMode definitions in the device class schema and
apply the same rule consistently to each.
In
`@assets/components/lvms/lvms-operator_rbac.authorization.k8s.io_v1_clusterrole.yaml`:
- Around line 113-120: The RBAC rules for the operator still grant the core
events resource alongside the new events.k8s.io/events permissions, which
broadens access unnecessarily. Update the cluster role by removing the stale
core events grant from the RBAC manifest and keep only the permissions needed
for the target event API, using the existing events.k8s.io/events rule as the
reference point.
In `@assets/components/lvms/vg-manager_rbac.authorization.k8s.io_v1_role.yaml`:
- Around line 64-71: The Role currently grants event write permissions through
both the core and events.k8s.io APIs, so it is broader than necessary. In the
RBAC manifest for the vg-manager role, remove the core events rule if the
workload now uses events.k8s.io, and keep only the event API that is actually
referenced by the code so the Role remains least-privileged. Use the existing
events.k8s.io rule in this Role as the anchor when updating the permissions.
In `@scripts/auto-rebase/rebase-lvms.sh`:
- Around line 556-558: The latest case in the rebase dispatch currently forwards
$2 and $3 directly from the argument list, which can expand before usage() runs
when either argument is missing. Update the command handling around
rebase_lvms_latest in the latest branch to validate that both required arguments
are present before any expansion, and route invalid invocations to usage()
instead of calling the helper with unset parameters.
- Around line 44-51: The clean-tag lookup in rebase-lvms.sh is terminating early
under errexit/pipefail when grep finds no match, preventing the prerelease
fallback from running. Update the latest-tag lookup in the tag-selection logic
to tolerate an expected no-match before proceeding to the prerelease branch,
keeping the fallback reachable while preserving the existing sort/tail behavior
in the rebase-lvms.sh tag selection flow.
---
Outside diff comments:
In
`@assets/components/lvms/lvms-operator_rbac.authorization.k8s.io_v1_clusterrole.yaml`:
- Around line 47-52: The ClusterRole rule currently groups pods with the same
verb set as PVCs, which unintentionally grants pods the update verb. Split the
RBAC rules in the ClusterRole manifest so the pods resource has only the
intended verbs and update remains scoped to the PVC rule; use the existing pods
and PVC entries in the role definition to separate them cleanly.
🪄 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: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 6d970893-f8f8-4328-96bc-cc66ac8997d2
📒 Files selected for processing (12)
assets/components/lvms/lvm.topolvm.io_lvmclusters.yamlassets/components/lvms/lvm.topolvm.io_lvmvolumegroupnodestatuses.yamlassets/components/lvms/lvm.topolvm.io_lvmvolumegroups.yamlassets/components/lvms/lvms-operator_rbac.authorization.k8s.io_v1_clusterrole.yamlassets/components/lvms/topolvm-configmap_lvms-version.yamlassets/components/lvms/vg-manager_rbac.authorization.k8s.io_v1_clusterrole.yamlassets/components/lvms/vg-manager_rbac.authorization.k8s.io_v1_role.yamlassets/release/release-aarch64.jsonassets/release/release-x86_64.jsonscripts/auto-rebase/last_lvms_rebase.shscripts/auto-rebase/rebase-lvms.shscripts/auto-rebase/rebase_job_entrypoint.sh
|
/test e2e-aws-tests |
ggiguash
force-pushed
the
lvms-rebase-latest-z-version
branch
from
2301014 to
d04ec5d
Compare
ggiguash
changed the title
ggiguash
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@scripts/auto-rebase/rebase_job_entrypoint.sh`:
- Around line 91-94: The LVMS rebase invocation in rebase_job_entrypoint.sh is
using a mismatched release version, with opm_release set to v4.22 but
rebase_lvms.sh still receiving 4.21. Update the version argument in the
rebase_lvms.sh call so it matches the job’s target release (the same X.Y used by
opm_release) and keep the release value consistent in this script.
🪄 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: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 17ee17ed-320c-48c0-89d1-8f03e467f8e0
📒 Files selected for processing (2)
scripts/auto-rebase/last_rebase_lvms.shscripts/auto-rebase/rebase_job_entrypoint.sh
✅ Files skipped from review due to trivial changes (1)
- scripts/auto-rebase/last_rebase_lvms.sh
coderabbitai
Bot
added
the
ready-for-human-review
|
@ggiguash: This pull request references Jira Issue OCPBUGS-86059, which is valid. 3 validation(s) were run on this bug
The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
Instructions 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 openshift-eng/jira-lifecycle-plugin repository. |
|
/test e2e-aws-tests |
ggiguash
force-pushed
the
lvms-rebase-latest-z-version
branch
from
d04ec5d to
0b34cb6
Compare
|
/test test-rebase |
coderabbitai
Bot
removed
the
ready-for-human-review
|
@ggiguash: This pull request references Jira Issue OCPBUGS-86059, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
DetailsIn response to this:
Instructions 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 openshift-eng/jira-lifecycle-plugin repository. |
ggiguash
force-pushed
the
lvms-rebase-latest-z-version
branch
from
0b34cb6 to
14fecbb
Compare
|
/test test-rebase |
|
@ggiguash: This pull request references Jira Issue OCPBUGS-86059, which is valid. 3 validation(s) were run on this bug
DetailsIn response to this:
Instructions 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 openshift-eng/jira-lifecycle-plugin repository. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@scripts/auto-rebase/rebase_lvms.sh`:
- Around line 38-41: The comment above resolve_latest_z_tag currently describes
prerelease fallback that no longer exists. Update the docstring in
resolve_latest_z_tag to state that it only searches for clean semver tags
matching vX.Y.Z for the given X.Y stream and fails if none are found, removing
any mention of pre-release fallback.
🪄 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: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 8f76bbb6-e841-46e2-8a20-5b4130d4c85e
📒 Files selected for processing (9)
docs/contributor/rebase.mdscripts/auto-rebase/assets_lvms.yamlscripts/auto-rebase/assets_ossm.yamlscripts/auto-rebase/last_lvms_rebase.shscripts/auto-rebase/last_rebase_lvms.shscripts/auto-rebase/presubmit.pyscripts/auto-rebase/rebase_gateway_api.shscripts/auto-rebase/rebase_job_entrypoint.shscripts/auto-rebase/rebase_lvms.sh
💤 Files with no reviewable changes (1)
- scripts/auto-rebase/last_lvms_rebase.sh
✅ Files skipped from review due to trivial changes (1)
- docs/contributor/rebase.md
🚧 Files skipped from review as they are similar to previous changes (4)
- scripts/auto-rebase/last_rebase_lvms.sh
- scripts/auto-rebase/rebase_gateway_api.sh
- scripts/auto-rebase/presubmit.py
- scripts/auto-rebase/rebase_job_entrypoint.sh
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
ggiguash
force-pushed
the
lvms-rebase-latest-z-version
branch
from
14fecbb to
a070c7d
Compare
2 participants
Summary by CodeRabbit
latest REGISTRY XY_VERSIONto automatically resolve and rebase to the newest matching LVMS z-stream tag.