Workflow to publish experimental MicroShift builds on COPR#208
Merged
pmtk merged 4 commits intomicroshift-io:mainfrom Mar 24, 2026
Merged
Workflow to publish experimental MicroShift builds on COPR#208pmtk merged 4 commits intomicroshift-io:mainfrom
pmtk merged 4 commits intomicroshift-io:mainfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (2)
📝 WalkthroughWalkthroughThe changes refactor the COPR build pipeline to support parameterized git sources and configurations. A new Makefile variable makes the git URL overridable, the core COPR workflow becomes reusable with typed inputs, and two new workflows (experimental and nightly) call this reusable workflow with different parameters. A minor URL update is made to the dependencies script. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (3)
src/copr/copr-cli.Containerfile (1)
9-10: Remove redundant spec file copy to root.
containernetworking-plugins.specis copied to both/(line 9) and/cni/(line 10). The spec is only used from/cni/bycni/build.sh. Remove it from line 9.♻️ Suggested fix
-COPY create-build.sh microshift-io-dependencies.sh cni/containernetworking-plugins.spec / +COPY create-build.sh microshift-io-dependencies.sh / COPY cni/build.sh cni/containernetworking-plugins.spec /cni/🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/copr/copr-cli.Containerfile` around lines 9 - 10, Remove the redundant copy of containernetworking-plugins.spec from the first COPY statement so the spec is only copied into /cni/ where cni/build.sh expects it; update the COPY that currently lists "create-build.sh microshift-io-dependencies.sh containernetworking-plugins.spec /" to omit containernetworking-plugins.spec, leaving the separate COPY that places containternetworking-plugins.spec into /cni/ unchanged.src/copr/cni/build.sh (2)
46-46: Temp directory not cleaned up on failure.If the COPR build fails (line 85), the script exits without removing
${temp_dir}. Consider using a trap for cleanup.Proposed fix
temp_dir="$(mktemp -d "/tmp/containernetworking-plugins-${version}.XXXXXX")" +trap 'rm -rf "${temp_dir}"' EXIT cp "${_scriptdir}/containernetworking-plugins.spec" "${temp_dir}/"Then remove the explicit cleanup at the end:
popd >/dev/null -rm -rf "${temp_dir}"Also applies to: 88-89
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/copr/cni/build.sh` at line 46, The temp_dir created in build.sh (variable temp_dir set by mktemp) is not removed on failure; add a trap to clean it up (e.g., trap 'rm -rf "${temp_dir}"' EXIT or trap on ERR/EXIT) immediately after temp_dir is created so the directory is removed on any exit, and then remove the explicit cleanup at the end of the script (the manual rm -rf of "${temp_dir}") to avoid double-deletion.
32-32: Shell variable inside single-quotedjqfilter.The
${_package_name}variable won't expand inside single quotes. This works by accident because the outer double quotes handle it, but it's fragile.Clearer approach
-cni_pkg="$(copr-cli list-packages "${COPR_REPO_NAME}" | jq -r '.[] | select(.name == "'${_package_name}'")')" +cni_pkg="$(copr-cli list-packages "${COPR_REPO_NAME}" | jq -r --arg name "${_package_name}" '.[] | select(.name == $name)')"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/copr/cni/build.sh` at line 32, The jq filter is using single quotes so ${_package_name} won't reliably expand; change the command that sets cni_pkg (the copr-cli list-packages | jq pipeline) to pass the shell variable into jq via --arg (e.g., --arg pkg "$_package_name") and use select(.name == $pkg) inside the jq expression so the package name comparison uses the passed-in argument rather than an interpolated shell variable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/copr-build-test.yaml:
- Line 202: There's a typo in the workflow step where the key is written as "run
: |" — remove the extra space so the key is properly "run: |"; locate the entry
containing the literal "run : |" and change it to "run: |" to fix YAML parsing
and align with GitHub Actions syntax.
- Around line 136-137: The job build-and-test-microshift currently only lists
needs: build-rpms causing it to run before build-dependencies-rpm finishes;
update the job declaration for build-and-test-microshift to include
build-dependencies-rpm in its needs (e.g., make needs an array containing both
build-rpms and build-dependencies-rpm) so the COPR build waits for dependency
and CNI RPMs to be available before running.
In @.github/workflows/experimental-copr.yaml:
- Around line 3-4: The workflow currently uses an unrestricted "push" trigger
which exposes the COPR_CONFIG secret and runs with empty inputs (e.g.,
inputs.fork-repo, inputs.branch, inputs.copr-repo) because those inputs are only
populated for workflow_dispatch; remove the bare push trigger or restrict it to
specific branches/tags (e.g., branches: [main, release/*]) and instead mirror
nightly-copr.yaml by using schedule plus workflow_dispatch, or add guards so
steps that use inputs.fork-repo, inputs.branch, and inputs.copr-repo only run
when github.event_name == 'workflow_dispatch' to avoid leaking secrets or
running with empty values.
In `@src/copr/cni/build.sh`:
- Around line 51-53: The two curl invocations in build.sh that download
amd64.tgz and arm64.tgz should add the fail-on-HTTP-error flag so downloads
error out on 4xx/5xx responses; update the two commands that call curl -L -o
amd64.tgz "https://...cni-plugins-linux-amd64-..." and curl -L -o arm64.tgz
"https://...cni-plugins-linux-arm64-..." to include -f (or --fail) alongside -L
and -o to ensure the script exits on HTTP errors.
In `@src/copr/copr.mk`:
- Around line 124-128: The podman run in the copr-cni target is using
interactive TTY flags (-ti) which can break non-interactive CI; update the
podman invocation in the copr-cni target by removing the -ti flags so it runs
non-interactively (adjust the line containing the podman run that references
COPR_CLI_IMAGE and /cni/build.sh "${COPR_REPO_NAME}"). Ensure no other
interactive flags remain (e.g., -t or -i) so the command is suitable for
automated builds.
- Around line 115-119: The podman run invocation in copr.mk uses the "-ti" flags
which allocate a TTY and keep stdin open and can fail in CI; remove the "-ti"
flags from the podman run command that calls "${COPR_CLI_IMAGE}"
/microshift-io-dependencies.sh "${OKD_VERSION_TAG}" "${COPR_REPO_NAME}" (and
only add "--interactive" or "--tty" conditionally if you detect an interactive
environment), keeping the other flags like --rm and --secret
(${COPR_SECRET_NAME},target=/root/.config/copr) unchanged.
In `@src/copr/microshift-io-dependencies.sh`:
- Around line 38-43: The loop that builds rhocp_versions uses
minor_version_start=$((minor - 2)) which can be negative when minor < 2 (e.g.,
OKD_VERSION_TAG 4.1.0) and produces invalid version strings; update the logic
around minor_version_start (and the seq loop that iterates from
minor_version_start to minor) to clamp the start to 0 (or the lowest valid
minor) before building rhocp_versions, e.g., compute a safe_start = max(0, minor
- 2) and iterate from safe_start to minor so functions/variables like
minor_version_start, minor, seq and rhocp_versions produce only valid X.Y
values.
- Line 58: The URL string currently set as
"https://github.com/microshift-io/microshift-io" is wrong; update the URL in
src/copr/microshift-io-dependencies.sh (the line starting with "URL:") to
"https://github.com/microshift-io/microshift" so it points to the microshift
repository instead of repeating "microshift-io".
---
Nitpick comments:
In `@src/copr/cni/build.sh`:
- Line 46: The temp_dir created in build.sh (variable temp_dir set by mktemp) is
not removed on failure; add a trap to clean it up (e.g., trap 'rm -rf
"${temp_dir}"' EXIT or trap on ERR/EXIT) immediately after temp_dir is created
so the directory is removed on any exit, and then remove the explicit cleanup at
the end of the script (the manual rm -rf of "${temp_dir}") to avoid
double-deletion.
- Line 32: The jq filter is using single quotes so ${_package_name} won't
reliably expand; change the command that sets cni_pkg (the copr-cli
list-packages | jq pipeline) to pass the shell variable into jq via --arg (e.g.,
--arg pkg "$_package_name") and use select(.name == $pkg) inside the jq
expression so the package name comparison uses the passed-in argument rather
than an interpolated shell variable.
In `@src/copr/copr-cli.Containerfile`:
- Around line 9-10: Remove the redundant copy of
containernetworking-plugins.spec from the first COPY statement so the spec is
only copied into /cni/ where cni/build.sh expects it; update the COPY that
currently lists "create-build.sh microshift-io-dependencies.sh
containernetworking-plugins.spec /" to omit containernetworking-plugins.spec,
leaving the separate COPY that places containternetworking-plugins.spec into
/cni/ unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 63a45a4f-48bf-4c0a-87c3-25f7a75e2b8b
📒 Files selected for processing (12)
.github/workflows/copr-build-test.yaml.github/workflows/experimental-copr.yaml.github/workflows/nightly-copr.yamlMakefiledocs/run.mdsrc/copr/cni/build.shsrc/copr/cni/containernetworking-plugins.specsrc/copr/copr-cli.Containerfilesrc/copr/copr.mksrc/copr/create-build.shsrc/copr/microshift-io-dependencies.shsrc/quickrpm.sh
pmtk
changed the title
ggiguash
reviewed
Mar 23, 2026
ggiguash
approved these changes
Mar 23, 2026
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
2 participants
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.
Summary by CodeRabbit
New Features
Chores