feat: add ACM fast-forwarding flow#77987
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: dhaiducek The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
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 updates ownership/approver lists across multiple OCM CI step registry metadata files (removing 5 users, adding 1), introduces a new multi-repository fast-forward CI workflow with step definitions and implementation script, integrates a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 10✅ Passed checks (10 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
openshift-ci
Bot
added
the
do-not-merge/invalid-owners-file
|
@dhaiducek, Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, 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: 7
🧹 Nitpick comments (6)
ci-operator/step-registry/ocm/update-owners.sh (2)
1-1: Nit: shebang has a space.
#! /bin/bashis accepted by Linux, but the conventional form is#!/bin/bashand some linters/tools are stricter.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ci-operator/step-registry/ocm/update-owners.sh` at line 1, The shebang in update-owners.sh uses a nonstandard space ("#! /bin/bash"); change it to the conventional form without the space ("#!/bin/bash") at the top of the script so linters/tools accept it and execution behaves consistently; update the first line of the file (the existing shebang) accordingly.
1-8: Add strict-mode flags to fail fast on errors.Without
set -euo pipefail, if theyqread on line 5 fails (missing/malformedOWNERS, yq not installed)ownersbecomes empty and line 8 expands toyq -i -o json '.owners.approvers = ' …, which is a yq syntax error per file — or worse, in some yq versions could silently writenull, clobbering every metadata file's approvers.🛡️ Suggested hardening
#! /bin/bash +set -euo pipefail script_dir=$(cd "$(dirname "${BASH_SOURCE[0]}")" >/dev/null 2>&1 && pwd) owners=$(yq '.approvers' -I=0 -o=json "${script_dir}/OWNERS") +if [[ -z "${owners}" || "${owners}" == "null" ]]; then + echo "ERROR: failed to read approvers from ${script_dir}/OWNERS" >&2 + exit 1 +fi find "${script_dir}" -type f -name "*.metadata.json" \ -exec yq -i -o json '.owners.approvers = '"${owners}" {} \;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ci-operator/step-registry/ocm/update-owners.sh` around lines 1 - 8, Add strict-mode and a pre-flight validation: insert "set -euo pipefail" immediately after the shebang, ensure yq exists (or fail early) and verify the owners variable (from owners=$(yq '.approvers' ...)) is non-empty before running the find/... -exec yq -i ... command so the find loop never executes with an empty/invalid assignment; reference the owners variable, the yq read call that populates it, and the find ... -exec yq -i -o json '.owners.approvers = '"${owners}" {} \; invocation when implementing these checks.ci-operator/config/stolostron/acm-infra/OWNERS (1)
1-11: LGTM. OWNERS file satisfieshack/validate-owners.shrequirements for the newacm-infraconfig directory, and the approvers/reviewers mirror well-known stolostron maintainers.Optional: several sibling
ci-operator/config/stolostron/<component>/OWNERSfiles are auto-generated from the upstream repo's root OWNERS (e.g.,acm-cli). Ifstolostron/acm-infrahas an upstream OWNERS, consider keeping this in sync via the same auto-generation flow to avoid drift.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ci-operator/config/stolostron/acm-infra/OWNERS` around lines 1 - 11, OWNERS file is valid but optionally should be kept in sync with an upstream repo to avoid drift; implement an automated sync/generation step that sources the upstream root OWNERS and regenerates the approvers and reviewers entries (the approvers and reviewers blocks) for acm-infra so the entries (e.g., dhaiducek, gparvin, xuezhaojun, zhujian7) are updated automatically rather than manually edited.ci-operator/step-registry/ocm/ci/fastforward-multiple/ocm-ci-fastforward-multiple-commands.sh (2)
1-3: Add strict bash flags.Without
set -euo pipefail, typos (e.g., unset vars likeARTIFACT_DIRonce the commented-out block is enabled) and mid-script failures will pass silently and the script will stillexit 0via${exit_code}. Recommended:#!/bin/bash +set -euo pipefail🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ci-operator/step-registry/ocm/ci/fastforward-multiple/ocm-ci-fastforward-multiple-commands.sh` around lines 1 - 3, The script lacks strict Bash options so failures or unset vars (e.g., ARTIFACT_DIR when that block is enabled) can be masked by the explicit exit_code=0; add strict flags at the top of the script by calling set -euo pipefail (and consider IFS=$'\n\t') so errors and unset variables abort the run, and update usages of ARTIFACT_DIR and other variables to be initialized/quoted; keep the existing exit_code logic but ensure failures actually set exit_code before the final exit.
45-56: Large block of commented-out execution logic.The actual fast-forward invocation is fully commented out, so this job will only validate inputs and log intent — it will never fast-forward anything. If that’s intentional (dry-run during bring-up), please add a
DRY_RUN-style flag and a TODO/issue reference; otherwise, enable the block before merging.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ci-operator/step-registry/ocm/ci/fastforward-multiple/ocm-ci-fastforward-multiple-commands.sh` around lines 45 - 56, The commented-out fast-forward invocation (the block that defines log_file and calls ../fastforward/ocm-ci-fastforward-commands.sh with REPO_OWNER/REPO_NAME/SOURCE_BRANCH/DESTINATION_BRANCH) prevents any real fast-forward; either re-enable that block so the script runs, or implement a DRY_RUN flag (e.g., FASTFORWARD_DRY_RUN or DRY_RUN) around it: when DRY_RUN is true only echo intent and keep the block commented/disabled, otherwise execute the call and preserve the existing error handling that captures exit_code, prints the error and sed-formatted logs; also add a TODO/issue reference comment indicating intentional dry-run behavior if you choose the flag route.ci-operator/step-registry/ocm/ci/fastforward-multiple/ocm-ci-fastforward-multiple-ref.yaml (1)
26-37: Make required inputs explicit.
REPO_MAP_PATHandDESTINATION_VERSIONSare effectively required (the command script exits 1 when empty/missing), but they are declared withdefault: "". Consider documenting that they are required, or omitting the empty default, so callers get a clearer contract at config-validation time.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ci-operator/step-registry/ocm/ci/fastforward-multiple/ocm-ci-fastforward-multiple-ref.yaml` around lines 26 - 37, The inputs REPO_MAP_PATH and DESTINATION_VERSIONS are treated as required but are declared with empty defaults; update their parameter declarations (the REPO_MAP_PATH and DESTINATION_VERSIONS entries) to make them explicit required inputs—either remove the default: "" so validation will enforce presence or add a clear "required: true" (and update the documentation blocks to state "Required: true" and provide example values) so callers see the contract at config-validation time.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ci-operator/config/stolostron/acm-infra/acm-infra-main.yaml`:
- Around line 11-19: The test config points REPO_MAP_PATH to a non-existent path
"acm-config/product/component-registry.yaml" used by the
ocm-ci-fastforward-multiple workflow; either change REPO_MAP_PATH to a valid
file path inside this repo or add the external acm-config repository as an
extra_refs entry so the workflow can access that file, and also fix the logic
bug in the workflow commands script ocm-ci-fastforward-multiple-commands.sh by
inverting the file existence check (replace the inverted if [[ -f ... ]] with if
[[ ! -f ... ]]) so the script correctly errors when the file is missing.
In
`@ci-operator/step-registry/ocm/ci/fastforward-multiple/ocm-ci-fastforward-multiple-commands.sh`:
- Around line 10-13: The file-existence test for REPO_MAP_PATH is inverted:
change the conditional in the block that references REPO_MAP_PATH so it checks
for non-existence (negate [[ -f "${REPO_MAP_PATH}" ]]) and only emits the
error/exit when the file is missing; keep the existing error message and exit
behavior but ensure the negated test (e.g., using ! -f) is used in the if that
currently wraps the echo/exit.
- Around line 27-43: The branch construction leaks and reuses the stale variable
named version from the earlier validation loop, causing branch_prefix
(release-${version}/backplane-${version}) to use the wrong value and then append
the inner-loop version again; fix by scoping the per-target version correctly:
remove reliance on the outer version variable and ensure the inner-loop iterator
(the one from DESTINATION_VERSIONS) is the only version used when building
branch and logging; specifically update the loops around
component_repos/owner_repo/branch_prefix and the inner for loop so branch_prefix
is computed using the inner-loop variable (or compute branch directly per
DESTINATION_VERSIONS) and change the top-level echo to not print the ambiguous
${version} variable (use a list or per-version messages instead).
- Around line 29-31: The yq filter for component_repos uses an equality
comparison against the literal string with a wildcard, so select(.repository ==
"https://github.com/stolostron/*") never matches; change the predicate to a
regex/test or startswith check (e.g., replace select(.repository ==
"https://github.com/stolostron/*") with select(.repository |
test("^https://github.com/stolostron/")) or select(.repository |
startswith("https://github.com/stolostron/")) in the yq invocation that produces
component_repos so real stolostron repo URLs are matched.
In
`@ci-operator/step-registry/ocm/ci/fastforward-multiple/ocm-ci-fastforward-multiple-ref.metadata.json`:
- Line 2: The metadata's "path" value is incorrect (points to
"ocm/ci/fastforward/ocm-ci-fastforward-multiple-ref.yaml") and should reference
the actual directory "fastforward-multiple"; update the "path" string in
ocm-ci-fastforward-multiple-ref.metadata.json to
"ocm/ci/fastforward-multiple/ocm-ci-fastforward-multiple-ref.yaml" so it matches
the ref YAML filename and directory, ensuring the "path" key is the only changed
token.
In
`@ci-operator/step-registry/ocm/ci/fastforward-multiple/ocm-ci-fastforward-multiple-workflow.metadata.json`:
- Line 2: The metadata file sets an incorrect "path" value pointing to
"ocm/ci/fastforward/ocm-ci-fastforward-multiple-workflow.yaml"; update the
"path" string in the metadata to the correct location
"ocm/ci/fastforward-multiple/ocm-ci-fastforward-multiple-workflow.yaml" so the
metadata correctly associates with the workflow file referenced by the
repository (edit the "path" key in the metadata file).
In `@ci-operator/step-registry/ocm/OWNERS`:
- Line 7: Replace the misspelled emeritus approver username "gurben" with the
correct GitHub handle "gurnben" in the OWNERS entry so the emeritus_approvers
list correctly references Gurney Buchanan; update the line that currently
contains "gurben" to "gurnben".
---
Nitpick comments:
In `@ci-operator/config/stolostron/acm-infra/OWNERS`:
- Around line 1-11: OWNERS file is valid but optionally should be kept in sync
with an upstream repo to avoid drift; implement an automated sync/generation
step that sources the upstream root OWNERS and regenerates the approvers and
reviewers entries (the approvers and reviewers blocks) for acm-infra so the
entries (e.g., dhaiducek, gparvin, xuezhaojun, zhujian7) are updated
automatically rather than manually edited.
In
`@ci-operator/step-registry/ocm/ci/fastforward-multiple/ocm-ci-fastforward-multiple-commands.sh`:
- Around line 1-3: The script lacks strict Bash options so failures or unset
vars (e.g., ARTIFACT_DIR when that block is enabled) can be masked by the
explicit exit_code=0; add strict flags at the top of the script by calling set
-euo pipefail (and consider IFS=$'\n\t') so errors and unset variables abort the
run, and update usages of ARTIFACT_DIR and other variables to be
initialized/quoted; keep the existing exit_code logic but ensure failures
actually set exit_code before the final exit.
- Around line 45-56: The commented-out fast-forward invocation (the block that
defines log_file and calls ../fastforward/ocm-ci-fastforward-commands.sh with
REPO_OWNER/REPO_NAME/SOURCE_BRANCH/DESTINATION_BRANCH) prevents any real
fast-forward; either re-enable that block so the script runs, or implement a
DRY_RUN flag (e.g., FASTFORWARD_DRY_RUN or DRY_RUN) around it: when DRY_RUN is
true only echo intent and keep the block commented/disabled, otherwise execute
the call and preserve the existing error handling that captures exit_code,
prints the error and sed-formatted logs; also add a TODO/issue reference comment
indicating intentional dry-run behavior if you choose the flag route.
In
`@ci-operator/step-registry/ocm/ci/fastforward-multiple/ocm-ci-fastforward-multiple-ref.yaml`:
- Around line 26-37: The inputs REPO_MAP_PATH and DESTINATION_VERSIONS are
treated as required but are declared with empty defaults; update their parameter
declarations (the REPO_MAP_PATH and DESTINATION_VERSIONS entries) to make them
explicit required inputs—either remove the default: "" so validation will
enforce presence or add a clear "required: true" (and update the documentation
blocks to state "Required: true" and provide example values) so callers see the
contract at config-validation time.
In `@ci-operator/step-registry/ocm/update-owners.sh`:
- Line 1: The shebang in update-owners.sh uses a nonstandard space ("#!
/bin/bash"); change it to the conventional form without the space
("#!/bin/bash") at the top of the script so linters/tools accept it and
execution behaves consistently; update the first line of the file (the existing
shebang) accordingly.
- Around line 1-8: Add strict-mode and a pre-flight validation: insert "set -euo
pipefail" immediately after the shebang, ensure yq exists (or fail early) and
verify the owners variable (from owners=$(yq '.approvers' ...)) is non-empty
before running the find/... -exec yq -i ... command so the find loop never
executes with an empty/invalid assignment; reference the owners variable, the yq
read call that populates it, and the find ... -exec yq -i -o json
'.owners.approvers = '"${owners}" {} \; invocation when implementing these
checks.
🪄 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: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 90de8da1-74af-4134-b8d4-197e0f9bec0a
📒 Files selected for processing (31)
ci-operator/config/stolostron/acm-infra/OWNERSci-operator/config/stolostron/acm-infra/acm-infra-main.yamlci-operator/jobs/stolostron/acm-infra/stolostron-acm-infra-acm-infra-main-periodics.yamlci-operator/step-registry/ocm/OWNERSci-operator/step-registry/ocm/ci/fastforward-multiple/OWNERSci-operator/step-registry/ocm/ci/fastforward-multiple/ocm-ci-fastforward-multiple-commands.shci-operator/step-registry/ocm/ci/fastforward-multiple/ocm-ci-fastforward-multiple-ref.metadata.jsonci-operator/step-registry/ocm/ci/fastforward-multiple/ocm-ci-fastforward-multiple-ref.yamlci-operator/step-registry/ocm/ci/fastforward-multiple/ocm-ci-fastforward-multiple-workflow.metadata.jsonci-operator/step-registry/ocm/ci/fastforward-multiple/ocm-ci-fastforward-multiple-workflow.yamlci-operator/step-registry/ocm/ci/fastforward/ocm-ci-fastforward-ref.metadata.jsonci-operator/step-registry/ocm/ci/fastforward/ocm-ci-fastforward-workflow.metadata.jsonci-operator/step-registry/ocm/ci/image-mirror-periodic/ocm-ci-image-mirror-periodic-ref.metadata.jsonci-operator/step-registry/ocm/ci/image-mirror-periodic/ocm-ci-image-mirror-periodic-workflow.metadata.jsonci-operator/step-registry/ocm/ci/image-mirror/ocm-ci-image-mirror-ref.metadata.jsonci-operator/step-registry/ocm/ci/image-mirror/ocm-ci-image-mirror-workflow.metadata.jsonci-operator/step-registry/ocm/ci/manifest-update/ocm-ci-manifest-update-ref.metadata.jsonci-operator/step-registry/ocm/ci/manifest-update/ocm-ci-manifest-update-workflow.metadata.jsonci-operator/step-registry/ocm/ci/rbac/ocm-ci-rbac-ref.metadata.jsonci-operator/step-registry/ocm/ci/rbac/ocm-ci-rbac-workflow.metadata.jsonci-operator/step-registry/ocm/e2e/clusterpool/checkin/ocm-e2e-clusterpool-checkin-ref.metadata.jsonci-operator/step-registry/ocm/e2e/clusterpool/checkout/ocm-e2e-clusterpool-checkout-chain.metadata.jsonci-operator/step-registry/ocm/e2e/clusterpool/checkout/ocm-e2e-clusterpool-checkout-ref.metadata.jsonci-operator/step-registry/ocm/e2e/clusterpool/cluster/deploy/ocm-e2e-clusterpool-cluster-deploy-ref.metadata.jsonci-operator/step-registry/ocm/e2e/clusterpool/cluster/getcreds/ocm-e2e-clusterpool-cluster-getcreds-ref.metadata.jsonci-operator/step-registry/ocm/e2e/clusterpool/list/ocm-e2e-clusterpool-list-ref.metadata.jsonci-operator/step-registry/ocm/e2e/clusterpool/ocm-e2e-clusterpool-workflow.metadata.jsonci-operator/step-registry/ocm/e2e/kind/create/ocm-e2e-kind-create-ref.metadata.jsonci-operator/step-registry/ocm/e2e/kind/destroy/ocm-e2e-kind-destroy-ref.metadata.jsonci-operator/step-registry/ocm/e2e/kind/ocm-e2e-kind-workflow.metadata.jsonci-operator/step-registry/ocm/update-owners.sh
Also adds a script to manage it more easily. Signed-off-by: Dale Haiducek <19750917+dhaiducek@users.noreply.github.com>
dhaiducek
force-pushed
the
add-fast-forwarding
branch
2 times, most recently
from
9949a7f to
81694c1
Compare
openshift-ci
Bot
removed
the
do-not-merge/invalid-owners-file
|
/pj-rehearse periodic-ci-stolostron-acm-infra-acm-infra-main-fast-forward |
|
@dhaiducek: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
dhaiducek
force-pushed
the
add-fast-forwarding
branch
from
81694c1 to
34d0151
Compare
|
/pj-rehearse periodic-ci-stolostron-acm-infra-acm-infra-main-fast-forward |
|
@dhaiducek: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
dhaiducek
force-pushed
the
add-fast-forwarding
branch
from
34d0151 to
e1a396b
Compare
|
/pj-rehearse periodic-ci-stolostron-acm-infra-acm-infra-main-fast-forward |
|
@dhaiducek: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@dhaiducek: job(s): periodic-ci-stolostron-acm-infra-acm-infra-main-fast-forward either don't exist or were not found to be affected, and cannot be rehearsed |
|
/pj-rehearse periodic-ci-stolostron-acm-infra-main-fast-forward |
|
@dhaiducek: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
dhaiducek
force-pushed
the
add-fast-forwarding
branch
from
e1a396b to
ebba4b8
Compare
|
/pj-rehearse periodic-ci-stolostron-acm-infra-main-fast-forward |
|
@dhaiducek: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
dhaiducek
force-pushed
the
add-fast-forwarding
branch
from
ebba4b8 to
e45933e
Compare
|
/pj-rehearse periodic-ci-stolostron-acm-infra-main-fast-forward |
|
@dhaiducek: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@dhaiducek, If the problem persists, please contact Test Platform. |
|
/pj-rehearse periodic-ci-stolostron-acm-infra-main-fast-forward |
|
@dhaiducek: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
dhaiducek
force-pushed
the
add-fast-forwarding
branch
from
e45933e to
90aaf7e
Compare
|
/pj-rehearse periodic-ci-stolostron-acm-infra-main-fast-forward |
|
@dhaiducek: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ci-stolostron-acm-infra-main-fast-forward |
|
@dhaiducek: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
dhaiducek
force-pushed
the
add-fast-forwarding
branch
from
90aaf7e to
940e865
Compare
|
/pj-rehearse periodic-ci-stolostron-acm-infra-main-fast-forward |
|
@dhaiducek: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@ci-operator/step-registry/ocm/ci/fastforward-multiple/ocm-ci-fastforward-multiple-commands.sh`:
- Around line 31-57: The loop currently only echoes actions; restore the real
fast-forward invocation by uncommenting the block that sets REPO_OWNER/REPO_NAME
and calls ../fastforward/ocm-ci-fastforward-commands.sh, and re-enable
writing/reading the log_file and exit_code handling so failures are detected and
logged. Also restore the owner extraction (uncomment the owner=${owner_repo%/*}
line) so REPO_OWNER is correctly populated before invoking the fast-forward
helper; ensure variables used are owner, repo (from owner_repo), branch,
ARTIFACT_DIR, SOURCE_BRANCH and DESTINATION_BRANCH remain consistent and the
exit_code check and sed log dump run in the failure branch.
🪄 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: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 691aa2bd-0293-4345-85f3-75478cc4020a
📒 Files selected for processing (10)
ci-operator/config/stolostron/acm-infra/.config.prowgenci-operator/config/stolostron/acm-infra/OWNERSci-operator/config/stolostron/acm-infra/stolostron-acm-infra-main.yamlci-operator/jobs/stolostron/acm-infra/stolostron-acm-infra-main-periodics.yamlci-operator/step-registry/ocm/ci/fastforward-multiple/OWNERSci-operator/step-registry/ocm/ci/fastforward-multiple/ocm-ci-fastforward-multiple-commands.shci-operator/step-registry/ocm/ci/fastforward-multiple/ocm-ci-fastforward-multiple-ref.metadata.jsonci-operator/step-registry/ocm/ci/fastforward-multiple/ocm-ci-fastforward-multiple-ref.yamlci-operator/step-registry/ocm/ci/fastforward-multiple/ocm-ci-fastforward-multiple-workflow.metadata.jsonci-operator/step-registry/ocm/ci/fastforward-multiple/ocm-ci-fastforward-multiple-workflow.yaml
✅ Files skipped from review due to trivial changes (7)
- ci-operator/config/stolostron/acm-infra/.config.prowgen
- ci-operator/step-registry/ocm/ci/fastforward-multiple/OWNERS
- ci-operator/step-registry/ocm/ci/fastforward-multiple/ocm-ci-fastforward-multiple-workflow.yaml
- ci-operator/step-registry/ocm/ci/fastforward-multiple/ocm-ci-fastforward-multiple-ref.metadata.json
- ci-operator/step-registry/ocm/ci/fastforward-multiple/ocm-ci-fastforward-multiple-workflow.metadata.json
- ci-operator/config/stolostron/acm-infra/OWNERS
- ci-operator/config/stolostron/acm-infra/stolostron-acm-infra-main.yaml
🚧 Files skipped from review as they are similar to previous changes (2)
- ci-operator/step-registry/ocm/ci/fastforward-multiple/ocm-ci-fastforward-multiple-ref.yaml
- ci-operator/jobs/stolostron/acm-infra/stolostron-acm-infra-main-periodics.yaml
| for repo in ${component_repos}; do | ||
| owner_repo=${repo#https://github.com/} | ||
| # owner=${owner_repo%/*} | ||
| repo=${owner_repo#*/} | ||
|
|
||
| echo "INFO: Handling ${owner_repo}" | ||
|
|
||
| branch_prefix="release" | ||
| if [[ ${product} == "mce" ]]; then | ||
| branch_prefix="backplane" | ||
| fi | ||
|
|
||
| for version in ${DESTINATION_VERSIONS}; do | ||
| branch="${branch_prefix}-${version}" | ||
| echo "INFO: Fast-forwarding ${owner_repo} main to branch: ${branch}" | ||
| # log_file="${ARTIFACT_DIR}/fastforward-${owner_repo}-${branch}.log" | ||
| # REPO_OWNER=${owner} \ | ||
| # REPO_NAME=${repo} \ | ||
| # SOURCE_BRANCH=main \ | ||
| # DESTINATION_BRANCH=${branch} \ | ||
| # ../fastforward/ocm-ci-fastforward-commands.sh >"${log_file}" 2>&1 || | ||
| # { | ||
| # exit_code=$? | ||
| # echo "ERROR: Failed to fast-forward ${owner_repo} to branch: ${branch}" | ||
| # echo "Logs:" | ||
| # sed 's/^/ /' "${log_file}" | ||
| # } |
There was a problem hiding this comment.
Enable the actual fast-forward call before shipping this flow.
Right now the loop only prints the intended action: lines 46-57 are commented out, so exit_code never changes and the periodic job will pass without fast-forwarding any repository. Also restore owner extraction on Line 33; the intended invocation needs it for REPO_OWNER.
🐛 Proposed fix
owner_repo=${repo#https://github.com/}
- # owner=${owner_repo%/*}
+ owner=${owner_repo%/*}
repo=${owner_repo#*/}
@@
branch="${branch_prefix}-${version}"
echo "INFO: Fast-forwarding ${owner_repo} main to branch: ${branch}"
- # log_file="${ARTIFACT_DIR}/fastforward-${owner_repo}-${branch}.log"
- # REPO_OWNER=${owner} \
- # REPO_NAME=${repo} \
- # SOURCE_BRANCH=main \
- # DESTINATION_BRANCH=${branch} \
- # ../fastforward/ocm-ci-fastforward-commands.sh >"${log_file}" 2>&1 ||
- # {
- # exit_code=$?
- # echo "ERROR: Failed to fast-forward ${owner_repo} to branch: ${branch}"
- # echo "Logs:"
- # sed 's/^/ /' "${log_file}"
- # }
+ log_file="${ARTIFACT_DIR}/fastforward-${owner_repo//\//-}-${branch}.log"
+ REPO_OWNER=${owner} \
+ REPO_NAME=${repo} \
+ SOURCE_BRANCH=main \
+ DESTINATION_BRANCH=${branch} \
+ ../fastforward/ocm-ci-fastforward-commands.sh >"${log_file}" 2>&1 ||
+ {
+ exit_code=$?
+ echo "ERROR: Failed to fast-forward ${owner_repo} to branch: ${branch}"
+ echo "Logs:"
+ sed 's/^/ /' "${log_file}"
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@ci-operator/step-registry/ocm/ci/fastforward-multiple/ocm-ci-fastforward-multiple-commands.sh`
around lines 31 - 57, The loop currently only echoes actions; restore the real
fast-forward invocation by uncommenting the block that sets REPO_OWNER/REPO_NAME
and calls ../fastforward/ocm-ci-fastforward-commands.sh, and re-enable
writing/reading the log_file and exit_code handling so failures are detected and
logged. Also restore the owner extraction (uncomment the owner=${owner_repo%/*}
line) so REPO_OWNER is correctly populated before invoking the fast-forward
helper; ensure variables used are owner, repo (from owner_repo), branch,
ARTIFACT_DIR, SOURCE_BRANCH and DESTINATION_BRANCH remain consistent and the
exit_code check and sed log dump run in the failure branch.
940e865 to
852e203
Compare
|
/pj-rehearse periodic-ci-stolostron-acm-infra-main-fast-forward |
|
@dhaiducek: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@dhaiducek: job(s): periodic-ci-stolostron-acm-infra-main-fast-forward either don't exist or were not found to be affected, and cannot be rehearsed |
|
/pj-rehearse periodic-ci-stolostron-acm-config-main-fast-forward |
|
@dhaiducek: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
dhaiducek
force-pushed
the
add-fast-forwarding
branch
from
852e203 to
6dc2bd1
Compare
|
/pj-rehearse periodic-ci-stolostron-acm-config-main-fast-forward |
|
@dhaiducek: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@ci-operator/jobs/stolostron/acm-config/stolostron-acm-config-main-periodics.yaml`:
- Around line 45-47: The pod spec mounts a volume named "gcs-credentials" at
/secrets/gcs but no corresponding entry exists in spec.volumes; add a volume
object with name: "gcs-credentials" under spec.volumes that sources the GCS
credentials (e.g., secret or configMap) and set its secretName/configMap name to
the existing credentials resource (and optional:false if required) so the volume
name exactly matches the mount name used in the container spec.
🪄 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: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 19b21898-be85-45e2-952d-14af0ce854c1
📒 Files selected for processing (10)
ci-operator/config/stolostron/acm-config/.config.prowgenci-operator/config/stolostron/acm-config/OWNERSci-operator/config/stolostron/acm-config/stolostron-acm-config-main.yamlci-operator/jobs/stolostron/acm-config/stolostron-acm-config-main-periodics.yamlci-operator/step-registry/ocm/ci/fastforward-multiple/OWNERSci-operator/step-registry/ocm/ci/fastforward-multiple/ocm-ci-fastforward-multiple-commands.shci-operator/step-registry/ocm/ci/fastforward-multiple/ocm-ci-fastforward-multiple-ref.metadata.jsonci-operator/step-registry/ocm/ci/fastforward-multiple/ocm-ci-fastforward-multiple-ref.yamlci-operator/step-registry/ocm/ci/fastforward-multiple/ocm-ci-fastforward-multiple-workflow.metadata.jsonci-operator/step-registry/ocm/ci/fastforward-multiple/ocm-ci-fastforward-multiple-workflow.yaml
✅ Files skipped from review due to trivial changes (8)
- ci-operator/step-registry/ocm/ci/fastforward-multiple/OWNERS
- ci-operator/config/stolostron/acm-config/.config.prowgen
- ci-operator/config/stolostron/acm-config/OWNERS
- ci-operator/step-registry/ocm/ci/fastforward-multiple/ocm-ci-fastforward-multiple-workflow.yaml
- ci-operator/config/stolostron/acm-config/stolostron-acm-config-main.yaml
- ci-operator/step-registry/ocm/ci/fastforward-multiple/ocm-ci-fastforward-multiple-workflow.metadata.json
- ci-operator/step-registry/ocm/ci/fastforward-multiple/ocm-ci-fastforward-multiple-commands.sh
- ci-operator/step-registry/ocm/ci/fastforward-multiple/ocm-ci-fastforward-multiple-ref.metadata.json
🚧 Files skipped from review as they are similar to previous changes (1)
- ci-operator/step-registry/ocm/ci/fastforward-multiple/ocm-ci-fastforward-multiple-ref.yaml
| - mountPath: /secrets/gcs | ||
| name: gcs-credentials | ||
| readOnly: true |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file='ci-operator/jobs/stolostron/acm-config/stolostron-acm-config-main-periodics.yaml'
python - <<'PY'
from pathlib import Path
import re
path = Path("ci-operator/jobs/stolostron/acm-config/stolostron-acm-config-main-periodics.yaml")
text = path.read_text()
mount = re.search(r'- mountPath: /secrets/gcs\n\s+name: (\S+)', text)
if not mount:
raise SystemExit("No /secrets/gcs volumeMount found")
mount_name = mount.group(1)
volumes = set(re.findall(r'(?m)^ - name: (\S+)$', text))
print(f"/secrets/gcs volumeMount name: {mount_name}")
print(f"declared pod volumes: {sorted(volumes)}")
if mount_name not in volumes:
raise SystemExit(f"ERROR: volumeMount {mount_name!r} has no matching spec.volumes entry")
PY
echo
echo "Existing examples of /secrets/gcs wiring:"
rg -n -C 6 'mountPath: /secrets/gcs|name: gcs-credentials|secretName: gcs-upload' ci-operator/jobs | head -120Repository: openshift/release
Length of output: 355
Add the missing gcs-credentials volume to fix pod spec validation error.
Line 46 mounts volume gcs-credentials at /secrets/gcs, but this volume is never declared in spec.volumes (lines 61-79). Kubernetes will reject the pod spec at creation time.
Proposed fix
- name: boskos
secret:
items:
- key: credentials
path: credentials
secretName: boskos-credentials
+ - name: gcs-credentials
+ secret:
+ secretName: gcs-upload
- name: github-credentials-openshift-ci-robot-private-git-cloner
secret:
secretName: github-credentials-openshift-ci-robot-private-git-cloner🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@ci-operator/jobs/stolostron/acm-config/stolostron-acm-config-main-periodics.yaml`
around lines 45 - 47, The pod spec mounts a volume named "gcs-credentials" at
/secrets/gcs but no corresponding entry exists in spec.volumes; add a volume
object with name: "gcs-credentials" under spec.volumes that sources the GCS
credentials (e.g., secret or configMap) and set its secretName/configMap name to
the existing credentials resource (and optional:false if required) so the volume
name exactly matches the mount name used in the container spec.
6dc2bd1 to
2cb9673
Compare
Signed-off-by: Dale Haiducek <19750917+dhaiducek@users.noreply.github.com>
dhaiducek
force-pushed
the
add-fast-forwarding
branch
from
2cb9673 to
0c9e32c
Compare
|
[REHEARSALNOTIFIER]
Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
/pj-rehearse periodic-ci-stolostron-acm-config-main-fast-forward |
|
@dhaiducek: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
[REHEARSALNOTIFIER]
Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
@dhaiducek: The following tests failed, say
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. |
| ### Swap out this code with the commented out code after testing | ||
| echo "INFO: Fast-forwarding ${owner_repo} main to branch: ${branch}" > "${log_file}" | ||
| # REPO_OWNER=${owner} \ | ||
| # REPO_NAME=${repo} \ | ||
| # SOURCE_BRANCH=main \ | ||
| # DESTINATION_BRANCH=${branch} \ | ||
| # ../fastforward/ocm-ci-fastforward-commands.sh >"${log_file}" 2>&1 || | ||
| # { | ||
| # exit_code=$? | ||
| # echo "ERROR: Failed to fast-forward ${owner_repo} to branch: ${branch}" | ||
| # echo "Logs:" | ||
| # sed 's/^/ /' "${log_file}" | ||
| # } |
There was a problem hiding this comment.
Like the Rabbit says, this is the chunk of code that would need to be swapped out/un-commented in order to activate the fast-forwarding logic (and assuming it works--I'm not even sure whether referencing a script in a different directory is valid).
| interval: 2h | ||
| steps: | ||
| env: | ||
| DESTINATION_VERSIONS: 2.17 5.0 |
There was a problem hiding this comment.
This is where the fast-forwarded versions are specified.
1 participant
The flow is a 2h periodic that pulls the YAML from
acm-configand iterates over the repos there leveraging the pre-existing fast-forwarding script each time.Also updates OWNERS and adds script to manage OWNERS updates more easily.
Summary by CodeRabbit
Release Notes
New Features
Documentation
Chores