microshift-io/microshift-nightly COPR: RPM with rhocp-mirror-beta config & CNIs#203
microshift-io/microshift-nightly COPR: RPM with rhocp-mirror-beta config & CNIs#203pmtk wants to merge 5 commits intomicroshift-io:mainfrom
Conversation
📝 WalkthroughWalkthroughThis PR automates the building and publishing of MicroShift dependencies and CNI plugins RPMs through COPR, introduces a new parameterized GitHub workflow to support dynamic COPR repository selection, and updates the installation process to use a dedicated dependency repository package instead of repository creation scripts. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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: 7
🤖 Fix all issues with AI agents
In `@docs/run.md`:
- Around line 44-47: Replace the incorrect package name
"microshift-io-dependencies-repo" in the installation command with the correct
package "microshift-io-dependencies" so the docs match the spec and quickrpm.sh
usage; update the line that currently runs sudo dnf install -y
microshift-io-dependencies-repo to sudo dnf install -y
microshift-io-dependencies in docs/run.md.
In `@src/copr/cni/build.sh`:
- Around line 50-59: The unquoted ${version} variable is used in several path
constructions (mkdir, tar -C, cp, tar czf, mv) in build.sh and triggers
shellcheck SC2086; update every occurrence to use quoted expansions like
"${version}" (e.g., containernetworking-plugins-"${version}"/x86_64) so paths
with spaces or glob chars are safe, and ensure all related commands (mkdir, tar
-C, cp, tar czf, mv) reference the quoted form consistently.
In `@src/copr/copr-cli.Containerfile`:
- Around line 3-7: The RUN installation line installs the wrong package name
"rpmbuild"; replace it with the correct Fedora package "rpm-build" in the RUN
command that installs "copr-cli jq rpmbuild" (so the token "rpmbuild" in that
RUN should be changed to "rpm-build") and keep the rest of the flags and cleanup
unchanged.
In `@src/copr/copr.mk`:
- Around line 120-126: The copr-cni make target is missing a .PHONY declaration,
uses the interactive -ti flags in the podman run, and the file lacks a final
newline; update the Makefile to add copr-cni to the .PHONY list (alongside other
phony targets), remove the -ti flags from the podman run invocation in the
copr-cni recipe so it runs non-interactively, and ensure the file ends with a
trailing newline character so POSIX tools behave correctly.
- Around line 112-118: Add a .PHONY declaration for the copr-dependencies
target, remove the interactive TTY flags (-ti) from the podman run invocation
(use --rm only, or gate -t behind a tty check if colored output is needed), and
document or provide a default for OKD_VERSION_TAG in the Makefile variables/help
section so the copr-dependencies target has a defined value; update references
to COPR_SECRET_NAME and COPR_CLI_IMAGE only as needed to match the existing
variable names.
In `@src/copr/microshift-io-dependencies.sh`:
- Around line 26-37: The if condition incorrectly uses command substitution
causing SC2091 (if $(...) executes stdout rather than the pipeline exit status);
change the condition to test the pipeline directly—invoke the pipeline starting
with copr-cli list-packages "${COPR_REPO_NAME}" | jq -r '.[].name' | grep -q
"${_package_name}" in the if statement (without $()), so the exit code controls
the branch; keep the inner logic that sets existing_package_version (from
copr-cli get-package ... | jq -r
'.latest_succeeded_build.source_package.version') and compares it to
"${pkg_version}-1" as before.
- Around line 88-91: The script currently calls copr-cli build with a local
.spec path (copr-cli build "${COPR_REPO_NAME}" "${dest}/${_package_name}.spec"),
but copr-cli expects an SRPM or URL; change the flow to create an SRPM from the
spec (e.g., run rpmbuild -ba /path/to/"${_package_name}.spec" or the project's
SRPM build command to produce an .src.rpm) and then call copr-cli build with
that SRPM path (copr-cli build "${COPR_REPO_NAME}"
"/path/to/${_package_name}-*.src.rpm"); keep the subsequent copr-cli
regenerate-repos call the same. Ensure variables (COPR_REPO_NAME, dest,
_package_name) are used to locate the produced SRPM.
🧹 Nitpick comments (3)
src/copr/copr-cli.Containerfile (1)
9-10:containernetworking-plugins.specis copied to both/and/cni/.Line 9 copies the spec to
/and line 10 copies it again to/cni/. Thebuild.shscript references it relative to its own directory ($_scriptdir), which is/cni/. The copy to/on line 9 appears redundant.Proposed 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/src/copr/microshift-io-dependencies.sh (1)
73-74:gpgcheck=0disables signature verification on the repo.The generated
.repofiles disable GPG checking. If this is intentional for beta mirrors, consider adding a comment in the spec explaining why.src/copr/cni/build.sh (1)
41-42: No cleanup trap fortemp_diron early exit.If the script fails between
mktempand the finalrm -rf, the temp directory persists. Not critical in CI containers, but a trap would be more robust.Suggested addition after line 41
temp_dir="$(mktemp -d "/tmp/containernetworking-plugins-${version}.XXXXXX")" +trap 'rm -rf "${temp_dir}"' EXIT cp "${_scriptdir}/containernetworking-plugins.spec" "${temp_dir}/"
| # microshift-io-dependencies must be installed | ||
| # before microshift in order to setup dependencies repositories. | ||
| sudo dnf install -y microshift-io-dependencies-repo | ||
| sudo dnf install -y microshift microshift-kindnet |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check the current state of docs/run.md around lines 44-47
head -n 50 docs/run.md | tail -n 20Repository: microshift-io/microshift
Length of output: 680
🏁 Script executed:
# Search for microshift-io-dependencies references across the codebase
rg "microshift-io-dependencies" -n --max-count=20Repository: microshift-io/microshift
Length of output: 565
🏁 Script executed:
# Check quickrpm.sh for the package name being installed
rg -A 5 -B 5 "microshift-io-dependencies" src/quickrpm.shRepository: microshift-io/microshift
Length of output: 288
🏁 Script executed:
# Look for any -repo suffix variants
rg "microshift-io-dependencies-repo" -nRepository: microshift-io/microshift
Length of output: 134
Fix package name in installation command.
Line 46 references microshift-io-dependencies-repo, but the actual package is microshift-io-dependencies (as defined in the spec and used in quickrpm.sh line 101).
Change required
-sudo dnf install -y microshift-io-dependencies-repo
+sudo dnf install -y microshift-io-dependencies📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # microshift-io-dependencies must be installed | |
| # before microshift in order to setup dependencies repositories. | |
| sudo dnf install -y microshift-io-dependencies-repo | |
| sudo dnf install -y microshift microshift-kindnet | |
| # microshift-io-dependencies must be installed | |
| # before microshift in order to setup dependencies repositories. | |
| sudo dnf install -y microshift-io-dependencies | |
| sudo dnf install -y microshift microshift-kindnet |
🤖 Prompt for AI Agents
In `@docs/run.md` around lines 44 - 47, Replace the incorrect package name
"microshift-io-dependencies-repo" in the installation command with the correct
package "microshift-io-dependencies" so the docs match the spec and quickrpm.sh
usage; update the line that currently runs sudo dnf install -y
microshift-io-dependencies-repo to sudo dnf install -y
microshift-io-dependencies in docs/run.md.
| mkdir -p containernetworking-plugins-${version}/{x86_64,aarch64} | ||
|
|
||
| tar xf amd64.tgz -C containernetworking-plugins-${version}/x86_64 | ||
| tar xf arm64.tgz -C containernetworking-plugins-${version}/aarch64 | ||
| cp containernetworking-plugins-${version}/x86_64/LICENSE containernetworking-plugins-${version}/x86_64/README.md containernetworking-plugins-${version}/ | ||
|
|
||
| tar czf containernetworking-plugins-${version}.tar.gz -C containernetworking-plugins-${version} . | ||
|
|
||
| mkdir -p buildroot/{RPMS,SRPMS,SOURCES,SPECS,BUILD} | ||
| mv containernetworking-plugins-${version}.tar.gz buildroot/SOURCES/ |
There was a problem hiding this comment.
Quote ${version} to fix SC2086 shellcheck warnings.
Multiple unquoted uses of ${version} in path constructions.
Proposed fix
-mkdir -p containernetworking-plugins-${version}/{x86_64,aarch64}
+mkdir -p "containernetworking-plugins-${version}"/{x86_64,aarch64}
-tar xf amd64.tgz -C containernetworking-plugins-${version}/x86_64
-tar xf arm64.tgz -C containernetworking-plugins-${version}/aarch64
-cp containernetworking-plugins-${version}/x86_64/LICENSE containernetworking-plugins-${version}/x86_64/README.md containernetworking-plugins-${version}/
+tar xf amd64.tgz -C "containernetworking-plugins-${version}/x86_64"
+tar xf arm64.tgz -C "containernetworking-plugins-${version}/aarch64"
+cp "containernetworking-plugins-${version}/x86_64/LICENSE" "containernetworking-plugins-${version}/x86_64/README.md" "containernetworking-plugins-${version}/"
-tar czf containernetworking-plugins-${version}.tar.gz -C containernetworking-plugins-${version} .
+tar czf "containernetworking-plugins-${version}.tar.gz" -C "containernetworking-plugins-${version}" .
mkdir -p buildroot/{RPMS,SRPMS,SOURCES,SPECS,BUILD}
-mv containernetworking-plugins-${version}.tar.gz buildroot/SOURCES/
+mv "containernetworking-plugins-${version}.tar.gz" buildroot/SOURCES/As per coding guidelines, shell scripts should pass linting with shellcheck.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| mkdir -p containernetworking-plugins-${version}/{x86_64,aarch64} | |
| tar xf amd64.tgz -C containernetworking-plugins-${version}/x86_64 | |
| tar xf arm64.tgz -C containernetworking-plugins-${version}/aarch64 | |
| cp containernetworking-plugins-${version}/x86_64/LICENSE containernetworking-plugins-${version}/x86_64/README.md containernetworking-plugins-${version}/ | |
| tar czf containernetworking-plugins-${version}.tar.gz -C containernetworking-plugins-${version} . | |
| mkdir -p buildroot/{RPMS,SRPMS,SOURCES,SPECS,BUILD} | |
| mv containernetworking-plugins-${version}.tar.gz buildroot/SOURCES/ | |
| mkdir -p "containernetworking-plugins-${version}"/{x86_64,aarch64} | |
| tar xf amd64.tgz -C "containernetworking-plugins-${version}/x86_64" | |
| tar xf arm64.tgz -C "containernetworking-plugins-${version}/aarch64" | |
| cp "containernetworking-plugins-${version}/x86_64/LICENSE" "containernetworking-plugins-${version}/x86_64/README.md" "containernetworking-plugins-${version}/" | |
| tar czf "containernetworking-plugins-${version}.tar.gz" -C "containernetworking-plugins-${version}" . | |
| mkdir -p buildroot/{RPMS,SRPMS,SOURCES,SPECS,BUILD} | |
| mv "containernetworking-plugins-${version}.tar.gz" buildroot/SOURCES/ |
🧰 Tools
🪛 GitHub Actions: linters
[warning] 50-50: SC2086: Double quote to prevent globbing and word splitting.
[warning] 52-52: SC2086: Double quote to prevent globbing and word splitting.
[warning] 53-53: SC2086: Double quote to prevent globbing and word splitting.
[warning] 54-54: SC2086: Double quote to prevent globbing and word splitting.
[warning] 54-54: SC2086: Double quote to prevent globbing and word splitting.
[warning] 56-56: SC2086: Double quote to prevent globbing and word splitting.
[warning] 56-56: SC2086: Double quote to prevent globbing and word splitting.
[warning] 59-59: SC2086: Double quote to prevent globbing and word splitting.
🤖 Prompt for AI Agents
In `@src/copr/cni/build.sh` around lines 50 - 59, The unquoted ${version} variable
is used in several path constructions (mkdir, tar -C, cp, tar czf, mv) in
build.sh and triggers shellcheck SC2086; update every occurrence to use quoted
expansions like "${version}" (e.g.,
containernetworking-plugins-"${version}"/x86_64) so paths with spaces or glob
chars are safe, and ensure all related commands (mkdir, tar -C, cp, tar czf, mv)
reference the quoted form consistently.
| RUN dnf install \ | ||
| --setopt=install_weak_deps=False \ | ||
| -y \ | ||
| copr-cli jq rpmbuild \ | ||
| && dnf clean all |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Fedora dnf package rpmbuild rpm-build
💡 Result:
On Fedora, the rpmbuild command is provided by the rpm-build package. [1]
sudo dnf install rpm-buildTo confirm on your system:
dnf provides '*/rpmbuild'Use rpm-build instead of rpmbuild.
On Fedora, the rpmbuild binary is provided by the rpm-build package. dnf install rpmbuild will fail.
Proposed fix
RUN dnf install \
--setopt=install_weak_deps=False \
-y \
- copr-cli jq rpmbuild \
+ copr-cli jq rpm-build \
&& dnf clean all📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| RUN dnf install \ | |
| --setopt=install_weak_deps=False \ | |
| -y \ | |
| copr-cli jq rpmbuild \ | |
| && dnf clean all | |
| RUN dnf install \ | |
| --setopt=install_weak_deps=False \ | |
| -y \ | |
| copr-cli jq rpm-build \ | |
| && dnf clean all |
🤖 Prompt for AI Agents
In `@src/copr/copr-cli.Containerfile` around lines 3 - 7, The RUN installation
line installs the wrong package name "rpmbuild"; replace it with the correct
Fedora package "rpm-build" in the RUN command that installs "copr-cli jq
rpmbuild" (so the token "rpmbuild" in that RUN should be changed to "rpm-build")
and keep the rest of the flags and cleanup unchanged.
| copr-dependencies: copr-cfg-ensure-podman-secret copr-cli | ||
| @echo "Building RPM with MicroShift dependencies repositories configuration" | ||
| sudo podman run \ | ||
| --rm -ti \ | ||
| --secret ${COPR_SECRET_NAME},target=/root/.config/copr \ | ||
| "${COPR_CLI_IMAGE}" \ | ||
| /microshift-io-dependencies.sh "${OKD_VERSION_TAG}" "${COPR_REPO_NAME}" |
There was a problem hiding this comment.
Missing .PHONY and -ti will break in CI.
- Add
.PHONY: copr-dependencies— every other target in this file declares it. -tiallocates a TTY + interactive stdin. GitHub Actions runners have no TTY, so this will fail or emit warnings. The other targets in this file use only--rm. Drop-ti(or use-tonly if truly needed for colored output, gated on[ -t 0 ]).OKD_VERSION_TAGis undocumented in the help target and has no default — consider adding it to the variables section.
Proposed fix
+.PHONY: copr-dependencies
copr-dependencies: copr-cfg-ensure-podman-secret copr-cli
`@echo` "Building RPM with MicroShift dependencies repositories configuration"
sudo podman run \
- --rm -ti \
+ --rm \
--secret ${COPR_SECRET_NAME},target=/root/.config/copr \
"${COPR_CLI_IMAGE}" \
/microshift-io-dependencies.sh "${OKD_VERSION_TAG}" "${COPR_REPO_NAME}"🤖 Prompt for AI Agents
In `@src/copr/copr.mk` around lines 112 - 118, Add a .PHONY declaration for the
copr-dependencies target, remove the interactive TTY flags (-ti) from the podman
run invocation (use --rm only, or gate -t behind a tty check if colored output
is needed), and document or provide a default for OKD_VERSION_TAG in the
Makefile variables/help section so the copr-dependencies target has a defined
value; update references to COPR_SECRET_NAME and COPR_CLI_IMAGE only as needed
to match the existing variable names.
| copr-cni: copr-cfg-ensure-podman-secret copr-cli | ||
| @echo "Building RPM with CNI plugins" | ||
| sudo podman run \ | ||
| --rm -ti \ | ||
| --secret ${COPR_SECRET_NAME},target=/root/.config/copr \ | ||
| "${COPR_CLI_IMAGE}" \ | ||
| /cni/build.sh "${COPR_REPO_NAME}" |
There was a problem hiding this comment.
Same issues: missing .PHONY and -ti; also no trailing newline.
Same fixes as copr-dependencies above. Additionally, the file lacks a final newline which can cause POSIX tools to misbehave.
Proposed fix
+.PHONY: copr-cni
copr-cni: copr-cfg-ensure-podman-secret copr-cli
`@echo` "Building RPM with CNI plugins"
sudo podman run \
- --rm -ti \
+ --rm \
--secret ${COPR_SECRET_NAME},target=/root/.config/copr \
"${COPR_CLI_IMAGE}" \
/cni/build.sh "${COPR_REPO_NAME}"
+🤖 Prompt for AI Agents
In `@src/copr/copr.mk` around lines 120 - 126, The copr-cni make target is missing
a .PHONY declaration, uses the interactive -ti flags in the podman run, and the
file lacks a final newline; update the Makefile to add copr-cni to the .PHONY
list (alongside other phony targets), remove the -ti flags from the podman run
invocation in the copr-cni recipe so it runs non-interactively, and ensure the
file ends with a trailing newline character so POSIX tools behave correctly.
| if $(copr-cli list-packages "${COPR_REPO_NAME}" | jq -r '.[].name' | grep -q "${_package_name}"); then | ||
| existing_package_version=$(copr-cli get-package \ | ||
| --name "${_package_name}" \ | ||
| --with-latest-succeeded-build \ | ||
| "${COPR_REPO_NAME}" \ | ||
| | jq -r '.latest_succeeded_build.source_package.version') | ||
|
|
||
| if [[ "${existing_package_version}" == "${pkg_version}-1" ]]; then | ||
| echo "Package ${_package_name} ${pkg_version} already exists in the COPR repository" | ||
| exit 0 | ||
| fi | ||
| fi |
There was a problem hiding this comment.
Bug: if $(...) always evaluates to true (SC2091).
The $() captures stdout (empty from grep -q) and tries to execute it as a command. An empty command succeeds, so this if is always true — the version-existence check never gates anything.
Remove the $() wrapper so if tests the pipeline's exit code directly:
Proposed fix
-if $(copr-cli list-packages "${COPR_REPO_NAME}" | jq -r '.[].name' | grep -q "${_package_name}"); then
+if copr-cli list-packages "${COPR_REPO_NAME}" | jq -r '.[].name' | grep -q "${_package_name}"; thenAs per coding guidelines, shell scripts should pass linting with shellcheck.
🧰 Tools
🪛 GitHub Actions: linters
[warning] 26-26: SC2091: Remove surrounding $() to avoid executing output (or use eval if intentional).
🤖 Prompt for AI Agents
In `@src/copr/microshift-io-dependencies.sh` around lines 26 - 37, The if
condition incorrectly uses command substitution causing SC2091 (if $(...)
executes stdout rather than the pipeline exit status); change the condition to
test the pipeline directly—invoke the pipeline starting with copr-cli
list-packages "${COPR_REPO_NAME}" | jq -r '.[].name' | grep -q
"${_package_name}" in the if statement (without $()), so the exit code controls
the branch; keep the inner logic that sets existing_package_version (from
copr-cli get-package ... | jq -r
'.latest_succeeded_build.source_package.version') and compares it to
"${pkg_version}-1" as before.
| if copr-cli build "${COPR_REPO_NAME}" "${dest}/${_package_name}.spec"; then | ||
| copr-cli regenerate-repos "${COPR_REPO_NAME}" | ||
| else | ||
| exit 1 |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
copr-cli build .spec file SRPM input format documentation
💡 Result:
copr-cli build primarily takes an SRPM as its input (either a local file path or an HTTP(S) URL): copr-cli build <project> <path-or-url-to>.src.rpm. [1]
For a .spec file, COPR supports it as a build source too:
- URL to a
.specfile: you can pass a public HTTPS URL to a.spec; COPR will then download the referenced sources (from public HTTPS locations) and build an SRPM first, then build RPMs from that SRPM. [2] - Direct upload of a local
.spec: COPR also supports uploading a local.spec(or SRPM) viacopr-cli/ web UI (“Direct Upload” source type). [2]
If what you actually have is a Git repo with the spec+sources, use the SCM source type (build from Git/SVN/DistGit and have COPR generate the SRPM using rpkg/tito/make srpm). [2]
Docs to read (most relevant sections):
- Fedora Developer Portal “Copr command line interface” (shows
copr-cli buildwith SRPM path/URL). [1] - COPR “User Documentation → Build Source Types” (URLs / Direct Upload / SCM; includes
.specvia URL and local.specupload). [2]
Sources: [1] [2]
🏁 Script executed:
# Check the context around line 88-91 in the file
sed -n '80,100p' src/copr/microshift-io-dependencies.shRepository: microshift-io/microshift
Length of output: 426
🏁 Script executed:
# Check if there's any SRPM generation earlier in the script
head -n 87 src/copr/microshift-io-dependencies.sh | tail -n 20Repository: microshift-io/microshift
Length of output: 602
copr-cli build cannot accept a local .spec file directly.
According to COPR documentation, copr-cli build expects an SRPM (local path or HTTP(S) URL), not a raw .spec file. Passing a local .spec file path will fail at runtime. Either build an SRPM first and pass that, or provide a URL to the .spec file so COPR can download sources and generate the SRPM itself.
🤖 Prompt for AI Agents
In `@src/copr/microshift-io-dependencies.sh` around lines 88 - 91, The script
currently calls copr-cli build with a local .spec path (copr-cli build
"${COPR_REPO_NAME}" "${dest}/${_package_name}.spec"), but copr-cli expects an
SRPM or URL; change the flow to create an SRPM from the spec (e.g., run rpmbuild
-ba /path/to/"${_package_name}.spec" or the project's SRPM build command to
produce an .src.rpm) and then call copr-cli build with that SRPM path (copr-cli
build "${COPR_REPO_NAME}" "/path/to/${_package_name}-*.src.rpm"); keep the
subsequent copr-cli regenerate-repos call the same. Ensure variables
(COPR_REPO_NAME, dest, _package_name) are used to locate the produced SRPM.
1 participant
Addresses: #165
Summary by CodeRabbit
New Features
Documentation
Chores