ci: support testing from brew builds and compose repositories - #239
ci: support testing from brew builds and compose repositories#239mcattamoredhat wants to merge 5 commits into
Conversation
|
Opened as draft PR for testing purposes only. |
There was a problem hiding this comment.
Code Review
This pull request adds support for RHEL 10 and introduces features to install go-fdo-client and go-fdo-server from specific Brew build URLs or compose repositories. The review feedback identifies a critical bug where calling parse_brew_url in a subshell prevents its global variables from propagating back to the parent shell, which breaks the RPM installation paths. Additionally, the feedback recommends removing the redundant static files/rhel-10-2.repo file and stripping trailing slashes from COMPOSE_BASE_URL to avoid malformed repository URLs.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| # Parse a brew build base URL into its components. | ||
| # The URL must point to the version/release directory of the package in brew. | ||
| # Sets _brew_ver, _brew_rel, _brew_arch and returns the trailing-slash-stripped | ||
| # URL on stdout so callers can capture it with: local url; url=$(parse_brew_url …) | ||
| parse_brew_url() { | ||
| local url="${1%/}" # strip any trailing slash to prevent empty basename | ||
| _brew_ver=$(basename "$(dirname "${url}")") | ||
| _brew_rel=$(basename "${url}") | ||
| _brew_arch=$(uname -m) | ||
| echo "${url}" | ||
| } |
There was a problem hiding this comment.
The parse_brew_url function is designed to set global variables (_brew_ver, _brew_rel, _brew_arch) as side effects. However, because it is called using command substitution (url=$(parse_brew_url ...)), it runs in a subshell. In Bash, variables set inside a subshell do not propagate back to the parent shell, leaving these variables empty when used later.
To fix this, redefine parse_brew_url to set a global _brew_url variable directly and avoid command substitution entirely.
| # Parse a brew build base URL into its components. | |
| # The URL must point to the version/release directory of the package in brew. | |
| # Sets _brew_ver, _brew_rel, _brew_arch and returns the trailing-slash-stripped | |
| # URL on stdout so callers can capture it with: local url; url=$(parse_brew_url …) | |
| parse_brew_url() { | |
| local url="${1%/}" # strip any trailing slash to prevent empty basename | |
| _brew_ver=$(basename "$(dirname "${url}")") | |
| _brew_rel=$(basename "${url}") | |
| _brew_arch=$(uname -m) | |
| echo "${url}" | |
| } | |
| # Parse a brew build base URL into its components. | |
| # The URL must point to the version/release directory of the package in brew. | |
| # Sets _brew_url, _brew_ver, _brew_rel, and _brew_arch globally. | |
| parse_brew_url() { | |
| _brew_url="${1%/}" # strip any trailing slash to prevent empty basename | |
| _brew_ver=$(basename "$(dirname "${_brew_url}")") | |
| _brew_rel=$(basename "${_brew_url}") | |
| _brew_arch=$(uname -m) | |
| } |
| install_client() { | ||
| go install github.com/fido-device-onboard/go-fdo-client@main | ||
| if [ -n "${CLIENT_RPM_URL:-}" ]; then | ||
| # Install from a specific brew build base path. | ||
| # CLIENT_RPM_URL should point to the version/release directory of the package in brew. | ||
| local url | ||
| url=$(parse_brew_url "${CLIENT_RPM_URL}") | ||
| # --nogpgcheck and sslverify=false are intentional: internal brew servers | ||
| # use self-signed certificates and builds may not be GPG-signed. | ||
| sudo dnf install -y --nogpgcheck --setopt=sslverify=false \ | ||
| "${url}/${_brew_arch}/go-fdo-client-${_brew_ver}-${_brew_rel}.${_brew_arch}.rpm" | ||
| else | ||
| go install github.com/fido-device-onboard/go-fdo-client@main | ||
| fi | ||
| } |
There was a problem hiding this comment.
Since parse_brew_url runs in a subshell when called via $(parse_brew_url ...), the global variables _brew_ver, _brew_rel, and _brew_arch are not set in the parent shell, causing the RPM filename construction to fail. Call parse_brew_url directly and use the global _brew_url variable instead.
| install_client() { | |
| go install github.com/fido-device-onboard/go-fdo-client@main | |
| if [ -n "${CLIENT_RPM_URL:-}" ]; then | |
| # Install from a specific brew build base path. | |
| # CLIENT_RPM_URL should point to the version/release directory of the package in brew. | |
| local url | |
| url=$(parse_brew_url "${CLIENT_RPM_URL}") | |
| # --nogpgcheck and sslverify=false are intentional: internal brew servers | |
| # use self-signed certificates and builds may not be GPG-signed. | |
| sudo dnf install -y --nogpgcheck --setopt=sslverify=false \ | |
| "${url}/${_brew_arch}/go-fdo-client-${_brew_ver}-${_brew_rel}.${_brew_arch}.rpm" | |
| else | |
| go install github.com/fido-device-onboard/go-fdo-client@main | |
| fi | |
| } | |
| install_client() { | |
| if [ -n "${CLIENT_RPM_URL:-}" ]; then | |
| # Install from a specific brew build base path. | |
| # CLIENT_RPM_URL should point to the version/release directory of the package in brew. | |
| parse_brew_url "${CLIENT_RPM_URL}" | |
| # --nogpgcheck and sslverify=false are intentional: internal brew servers | |
| # use self-signed certificates and builds may not be GPG-signed. | |
| sudo dnf install -y --nogpgcheck --setopt=sslverify=false \ | |
| "${_brew_url}/${_brew_arch}/go-fdo-client-${_brew_ver}-${_brew_rel}.${_brew_arch}.rpm" | |
| else | |
| go install github.com/fido-device-onboard/go-fdo-client@main | |
| fi | |
| } |
| if [ -n "${CLIENT_RPM_URL:-}" ]; then | ||
| # Install go-fdo-client from a specific brew build base path. | ||
| # CLIENT_RPM_URL should point to the version/release directory of the package in brew. | ||
| local url | ||
| url=$(parse_brew_url "${CLIENT_RPM_URL}") | ||
| tee Containerfile >/dev/null <<EOF | ||
| FROM ${base_image_url} | ||
| # --nogpgcheck and sslverify=false are intentional: internal brew servers | ||
| # use self-signed certificates and builds may not be GPG-signed. | ||
| RUN dnf install -y --nogpgcheck --setopt=sslverify=false \ | ||
| "${url}/${_brew_arch}/go-fdo-client-${_brew_ver}-${_brew_rel}.${_brew_arch}.rpm" | ||
| EOF |
There was a problem hiding this comment.
Since parse_brew_url runs in a subshell when called via $(parse_brew_url ...), the global variables _brew_ver, _brew_rel, and _brew_arch are not set in the parent shell, causing the RPM filename construction to fail. Call parse_brew_url directly and use the global _brew_url variable instead.
| if [ -n "${CLIENT_RPM_URL:-}" ]; then | |
| # Install go-fdo-client from a specific brew build base path. | |
| # CLIENT_RPM_URL should point to the version/release directory of the package in brew. | |
| local url | |
| url=$(parse_brew_url "${CLIENT_RPM_URL}") | |
| tee Containerfile >/dev/null <<EOF | |
| FROM ${base_image_url} | |
| # --nogpgcheck and sslverify=false are intentional: internal brew servers | |
| # use self-signed certificates and builds may not be GPG-signed. | |
| RUN dnf install -y --nogpgcheck --setopt=sslverify=false \ | |
| "${url}/${_brew_arch}/go-fdo-client-${_brew_ver}-${_brew_rel}.${_brew_arch}.rpm" | |
| EOF | |
| if [ -n "${CLIENT_RPM_URL:-}" ]; then | |
| # Install go-fdo-client from a specific brew build base path. | |
| # CLIENT_RPM_URL should point to the version/release directory of the package in brew. | |
| parse_brew_url "${CLIENT_RPM_URL}" | |
| tee Containerfile >/dev/null <<EOF | |
| FROM ${base_image_url} | |
| # --nogpgcheck and sslverify=false are intentional: internal brew servers | |
| # use self-signed certificates and builds may not be GPG-signed. | |
| RUN dnf install -y --nogpgcheck --setopt=sslverify=false \ | |
| "${_brew_url}/${_brew_arch}/go-fdo-client-${_brew_ver}-${_brew_rel}.${_brew_arch}.rpm" | |
| EOF |
| elif [ -n "${SERVER_RPM_URL:-}" ]; then | ||
| # Install from a specific brew build base path. | ||
| # SERVER_RPM_URL should point to the version/release directory of the package in brew. | ||
| local url | ||
| url=$(parse_brew_url "${SERVER_RPM_URL}") | ||
| # --nogpgcheck and sslverify=false are intentional: internal brew servers | ||
| # use self-signed certificates and builds may not be GPG-signed. | ||
| sudo dnf install -y --nogpgcheck --setopt=sslverify=false \ | ||
| "${url}/${_brew_arch}/go-fdo-server-${_brew_ver}-${_brew_rel}.${_brew_arch}.rpm" \ | ||
| "${url}/noarch/go-fdo-server-manufacturer-${_brew_ver}-${_brew_rel}.noarch.rpm" \ | ||
| "${url}/noarch/go-fdo-server-owner-${_brew_ver}-${_brew_rel}.noarch.rpm" \ | ||
| "${url}/noarch/go-fdo-server-rendezvous-${_brew_ver}-${_brew_rel}.noarch.rpm" |
There was a problem hiding this comment.
Since parse_brew_url runs in a subshell when called via $(parse_brew_url ...), the global variables _brew_ver, _brew_rel, and _brew_arch are not set in the parent shell, causing the RPM filename construction to fail. Call parse_brew_url directly and use the global _brew_url variable instead.
| elif [ -n "${SERVER_RPM_URL:-}" ]; then | |
| # Install from a specific brew build base path. | |
| # SERVER_RPM_URL should point to the version/release directory of the package in brew. | |
| local url | |
| url=$(parse_brew_url "${SERVER_RPM_URL}") | |
| # --nogpgcheck and sslverify=false are intentional: internal brew servers | |
| # use self-signed certificates and builds may not be GPG-signed. | |
| sudo dnf install -y --nogpgcheck --setopt=sslverify=false \ | |
| "${url}/${_brew_arch}/go-fdo-server-${_brew_ver}-${_brew_rel}.${_brew_arch}.rpm" \ | |
| "${url}/noarch/go-fdo-server-manufacturer-${_brew_ver}-${_brew_rel}.noarch.rpm" \ | |
| "${url}/noarch/go-fdo-server-owner-${_brew_ver}-${_brew_rel}.noarch.rpm" \ | |
| "${url}/noarch/go-fdo-server-rendezvous-${_brew_ver}-${_brew_rel}.noarch.rpm" | |
| elif [ -n "${SERVER_RPM_URL:-}" ]; then | |
| # Install from a specific brew build base path. | |
| # SERVER_RPM_URL should point to the version/release directory of the package in brew. | |
| parse_brew_url "${SERVER_RPM_URL}" | |
| # --nogpgcheck and sslverify=false are intentional: internal brew servers | |
| # use self-signed certificates and builds may not be GPG-signed. | |
| sudo dnf install -y --nogpgcheck --setopt=sslverify=false \ | |
| "${_brew_url}/${_brew_arch}/go-fdo-server-${_brew_ver}-${_brew_rel}.${_brew_arch}.rpm" \ | |
| "${_brew_url}/noarch/go-fdo-server-manufacturer-${_brew_ver}-${_brew_rel}.noarch.rpm" \ | |
| "${_brew_url}/noarch/go-fdo-server-owner-${_brew_ver}-${_brew_rel}.noarch.rpm" \ | |
| "${_brew_url}/noarch/go-fdo-server-rendezvous-${_brew_ver}-${_brew_rel}.noarch.rpm" |
| elif [ -n "${CLIENT_RPM_URL:-}" ]; then | ||
| # Install from a specific brew build base path. | ||
| # CLIENT_RPM_URL should point to the version/release directory of the package in brew. | ||
| local url | ||
| url=$(parse_brew_url "${CLIENT_RPM_URL}") | ||
| # --nogpgcheck and sslverify=false are intentional: internal brew servers | ||
| # use self-signed certificates and builds may not be GPG-signed. | ||
| sudo dnf install -y --nogpgcheck --setopt=sslverify=false \ | ||
| "${url}/${_brew_arch}/go-fdo-client-${_brew_ver}-${_brew_rel}.${_brew_arch}.rpm" |
There was a problem hiding this comment.
Since parse_brew_url runs in a subshell when called via $(parse_brew_url ...), the global variables _brew_ver, _brew_rel, and _brew_arch are not set in the parent shell, causing the RPM filename construction to fail. Call parse_brew_url directly and use the global _brew_url variable instead.
| elif [ -n "${CLIENT_RPM_URL:-}" ]; then | |
| # Install from a specific brew build base path. | |
| # CLIENT_RPM_URL should point to the version/release directory of the package in brew. | |
| local url | |
| url=$(parse_brew_url "${CLIENT_RPM_URL}") | |
| # --nogpgcheck and sslverify=false are intentional: internal brew servers | |
| # use self-signed certificates and builds may not be GPG-signed. | |
| sudo dnf install -y --nogpgcheck --setopt=sslverify=false \ | |
| "${url}/${_brew_arch}/go-fdo-client-${_brew_ver}-${_brew_rel}.${_brew_arch}.rpm" | |
| elif [ -n "${CLIENT_RPM_URL:-}" ]; then | |
| # Install from a specific brew build base path. | |
| # CLIENT_RPM_URL should point to the version/release directory of the package in brew. | |
| parse_brew_url "${CLIENT_RPM_URL}" | |
| # --nogpgcheck and sslverify=false are intentional: internal brew servers | |
| # use self-signed certificates and builds may not be GPG-signed. | |
| sudo dnf install -y --nogpgcheck --setopt=sslverify=false \ | |
| "${_brew_url}/${_brew_arch}/go-fdo-client-${_brew_ver}-${_brew_rel}.${_brew_arch}.rpm" |
| elif [ -n "${SERVER_RPM_URL:-}" ]; then | ||
| # Install from a specific brew build base path. | ||
| # SERVER_RPM_URL should point to the version/release directory of the package in brew. | ||
| local url | ||
| url=$(parse_brew_url "${SERVER_RPM_URL}") | ||
| # --nogpgcheck and sslverify=false are intentional: internal brew servers | ||
| # use self-signed certificates and builds may not be GPG-signed. | ||
| sudo dnf install -y --nogpgcheck --setopt=sslverify=false \ | ||
| "${url}/${_brew_arch}/go-fdo-server-${_brew_ver}-${_brew_rel}.${_brew_arch}.rpm" \ | ||
| "${url}/noarch/go-fdo-server-manufacturer-${_brew_ver}-${_brew_rel}.noarch.rpm" \ | ||
| "${url}/noarch/go-fdo-server-owner-${_brew_ver}-${_brew_rel}.noarch.rpm" \ | ||
| "${url}/noarch/go-fdo-server-rendezvous-${_brew_ver}-${_brew_rel}.noarch.rpm" |
There was a problem hiding this comment.
Since parse_brew_url runs in a subshell when called via $(parse_brew_url ...), the global variables _brew_ver, _brew_rel, and _brew_arch are not set in the parent shell, causing the RPM filename construction to fail. Call parse_brew_url directly and use the global _brew_url variable instead.
| elif [ -n "${SERVER_RPM_URL:-}" ]; then | |
| # Install from a specific brew build base path. | |
| # SERVER_RPM_URL should point to the version/release directory of the package in brew. | |
| local url | |
| url=$(parse_brew_url "${SERVER_RPM_URL}") | |
| # --nogpgcheck and sslverify=false are intentional: internal brew servers | |
| # use self-signed certificates and builds may not be GPG-signed. | |
| sudo dnf install -y --nogpgcheck --setopt=sslverify=false \ | |
| "${url}/${_brew_arch}/go-fdo-server-${_brew_ver}-${_brew_rel}.${_brew_arch}.rpm" \ | |
| "${url}/noarch/go-fdo-server-manufacturer-${_brew_ver}-${_brew_rel}.noarch.rpm" \ | |
| "${url}/noarch/go-fdo-server-owner-${_brew_ver}-${_brew_rel}.noarch.rpm" \ | |
| "${url}/noarch/go-fdo-server-rendezvous-${_brew_ver}-${_brew_rel}.noarch.rpm" | |
| elif [ -n "${SERVER_RPM_URL:-}" ]; then | |
| # Install from a specific brew build base path. | |
| # SERVER_RPM_URL should point to the version/release directory of the package in brew. | |
| parse_brew_url "${SERVER_RPM_URL}" | |
| # --nogpgcheck and sslverify=false are intentional: internal brew servers | |
| # use self-signed certificates and builds may not be GPG-signed. | |
| sudo dnf install -y --nogpgcheck --setopt=sslverify=false \ | |
| "${_brew_url}/${_brew_arch}/go-fdo-server-${_brew_ver}-${_brew_rel}.${_brew_arch}.rpm" \ | |
| "${_brew_url}/noarch/go-fdo-server-manufacturer-${_brew_ver}-${_brew_rel}.noarch.rpm" \ | |
| "${_brew_url}/noarch/go-fdo-server-owner-${_brew_ver}-${_brew_rel}.noarch.rpm" \ | |
| "${_brew_url}/noarch/go-fdo-server-rendezvous-${_brew_ver}-${_brew_rel}.noarch.rpm" |
| [RHEL-10.2-NIGHTLY-BaseOS] | ||
| name=baseos | ||
| baseurl=http://REPLACE_ME_HERE/rhel-10/nightly/RHEL-10/latest-RHEL-10.2/compose/BaseOS/$basearch/os | ||
| enabled=1 | ||
| # Nightly compose builds are not GPG-signed; gpgcheck=0 is intentional. | ||
| gpgcheck=0 | ||
| [RHEL-10.2-NIGHTLY-AppStream] | ||
| name=appstream | ||
| baseurl=http://REPLACE_ME_HERE/rhel-10/nightly/RHEL-10/latest-RHEL-10.2/compose/AppStream/$basearch/os/ | ||
| enabled=1 | ||
| # Nightly compose builds are not GPG-signed; gpgcheck=0 is intentional. | ||
| gpgcheck=0 |
There was a problem hiding this comment.
The file files/rhel-10-2.repo is redundant and unused. In test/bootc/utils.sh, the repository configuration is dynamically generated at runtime as files/rhel-${VERSION_ID}.repo (which resolves to files/rhel-10.2.repo with a dot, whereas this file uses a hyphen rhel-10-2.repo). Furthermore, this file contains unreplaced placeholders (REPLACE_ME_HERE). This file should be removed from the repository.
| local arch | ||
| arch=$(uname -m) | ||
| for stream in ${compose_streams}; do | ||
| local repo_name="compose-${ID}-${VERSION_ID}-${stream}" | ||
| sudo tee "/etc/yum.repos.d/${repo_name}.repo" >/dev/null <<EOF | ||
| [${repo_name}] | ||
| name=${repo_name} | ||
| baseurl=${compose_base_url}/${stream}/${arch}/os/ | ||
| enabled=1 | ||
| gpgcheck=0 | ||
| sslverify=0 | ||
| EOF | ||
| done |
There was a problem hiding this comment.
If COMPOSE_BASE_URL is provided with a trailing slash, constructing the baseurl as ${compose_base_url}/${stream}/${arch}/os/ will result in double slashes (e.g., .../compose//BaseOS/...). It is safer to strip any trailing slash from compose_base_url before constructing the repository configuration.
| local arch | |
| arch=$(uname -m) | |
| for stream in ${compose_streams}; do | |
| local repo_name="compose-${ID}-${VERSION_ID}-${stream}" | |
| sudo tee "/etc/yum.repos.d/${repo_name}.repo" >/dev/null <<EOF | |
| [${repo_name}] | |
| name=${repo_name} | |
| baseurl=${compose_base_url}/${stream}/${arch}/os/ | |
| enabled=1 | |
| gpgcheck=0 | |
| sslverify=0 | |
| EOF | |
| done | |
| local arch | |
| arch=$(uname -m) | |
| compose_base_url="${compose_base_url%/}" | |
| for stream in ${compose_streams}; do | |
| local repo_name="compose-${ID}-${VERSION_ID}-${stream}" | |
| sudo tee "/etc/yum.repos.d/${repo_name}.repo" >/dev/null <<EOF | |
| [${repo_name}] | |
| name=${repo_name} | |
| baseurl=${compose_base_url}/${stream}/${arch}/os/ | |
| enabled=1 | |
| gpgcheck=0 | |
| sslverify=0 | |
| EOF | |
| done |
| elif [ -n "${COMPOSE_BASE_URL:-}" ]; then | ||
| # Install go-fdo-client from a compose repository. | ||
| # Generate per-stream repo files, copy them into the image, install, then remove them. | ||
| local compose_streams="${COMPOSE_STREAMS:-BaseOS AppStream}" | ||
| local arch | ||
| arch=$(uname -m) | ||
| mkdir -p files | ||
| local repo_args="" | ||
| for stream in ${compose_streams}; do | ||
| local repo_name="compose-${ID}-${VERSION_ID}-${stream}" | ||
| local repo_file="files/${repo_name}.repo" | ||
| cat > "${repo_file}" <<EOF | ||
| [${repo_name}] | ||
| name=${repo_name} | ||
| baseurl=${COMPOSE_BASE_URL}/${stream}/${arch}/os/ | ||
| enabled=1 | ||
| gpgcheck=0 | ||
| sslverify=0 | ||
| EOF | ||
| repo_args+="COPY ${repo_file} /etc/yum.repos.d/${repo_name}.repo"$'\n' | ||
| done |
There was a problem hiding this comment.
If COMPOSE_BASE_URL is provided with a trailing slash, constructing the baseurl as ${COMPOSE_BASE_URL}/${stream}/${arch}/os/ will result in double slashes. It is safer to strip any trailing slash from COMPOSE_BASE_URL before constructing the repository configuration.
elif [ -n "${COMPOSE_BASE_URL:-}" ]; then
# Install go-fdo-client from a compose repository.
# Generate per-stream repo files, copy them into the image, install, then remove them.
local compose_streams="${COMPOSE_STREAMS:-BaseOS AppStream}"
local arch
arch=$(uname -m)
local compose_base_url="${COMPOSE_BASE_URL%/}"
mkdir -p files
local repo_args=""
for stream in ${compose_streams}; do
local repo_name="compose-${ID}-${VERSION_ID}-${stream}"
local repo_file="files/${repo_name}.repo"
cat > "${repo_file}" <<EOF
[${repo_name}]
name=${repo_name}
baseurl=${compose_base_url}/${stream}/${arch}/os/
enabled=1
gpgcheck=0
sslverify=0
EOF
repo_args+="COPY ${repo_file} /etc/yum.repos.d/${repo_name}.repo"$'\n'
doneaa88912 to
64cd690
Compare
Signed-off-by: Mario Cattamo <mcattamo@redhat.com>
Signed-off-by: Mario Cattamo <mcattamo@redhat.com>
64cd690 to
a34fce9
Compare
`parse_brew_url()` set `_brew_ver`/`_brew_rel`/`_brew_arch` as side effects, but every caller captured it via `url=$(parse_brew_url ...)`. Command substitution runs the function in a subshell, so those assignments never reached the calling scope — under `set -u`, the first reference to `_brew_arch` in install_client()/install_server() (test/ci, test/rpm, test/bootc) crashed with "unbound variable" whenever SERVER_RPM_URL or CLIENT_RPM_URL was actually set. Fix: have parse_brew_url() set _brew_url directly instead of echoing it, and call it as a plain statement (no command substitution) so all four variables land in the caller's scope. No behavior change for any path that doesn't set SERVER_RPM_URL / CLIENT_RPM_URL: PACKIT_COPR_RPMS-driven Packit/Testing Farm runs and the GitHub Actions test/ci matrix never touch parse_brew_url() at all (they short-circuit on PACKIT_COPR_RPMS or don't set these vars), so this only affects the manual brew-build testing path introduced in daf355b. Assisted-by: Claude (claude-sonnet-5) Signed-off-by: Mario Cattamo <mcattamo@redhat.com>
- install_from_compose() (test/rpm/utils.sh) and the bootc client
compose branch (test/bootc/utils.sh) now strip a trailing slash
from the compose base URL before building baseurl=..., avoiding
a double slash if the caller passes COMPOSE_BASE_URL with one.
- Remove files/rhel-10-2.repo: unused static template with
unreplaced REPLACE_ME_HERE placeholders. The real RHEL repo file
is generated at runtime as files/rhel-${VERSION_ID}.repo
(test/bootc/utils.sh), so this static file was dead weight.
Assisted-by: Claude (claude-sonnet-5)
Signed-off-by: Mario Cattamo <mcattamo@redhat.com>
configure_service_firewalld() returned right after `systemctl start firewalld`, but firewalld >= 2.4.1 (e.g. on Fedora 44) no longer blocks until its D-Bus interface is ready. configure_service_libvirtd() runs immediately afterward and calls `virsh net-start integration` on a network with zone='trusted', which requires firewalld's D-Bus API — racing it produces: error: internal error: zone trusted requested for network integration but firewalld is not active Poll `firewall-cmd --state` (up to 30s) after starting firewalld, bailing out with diagnostics if the unit dies or the timeout is hit. Assisted-by: Claude (claude-sonnet-5) Signed-off-by: Mario Cattamo <mcattamo@redhat.com>
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
Port brew-build and compose-repository install support from fido-device-onboard/go-fdo-server#239 into the shared test/{ci,rpm,bootc}/utils.sh, so test/rpm and test/bootc can install go-fdo-client/go-fdo-server from a brew build (CLIENT_RPM_URL/SERVER_RPM_URL) or a compose snapshot (COMPOSE_BASE_URL/COMPOSE_STREAMS), on top of the existing COPR/Packit/local-build paths. Also carries over the upstream follow-up fixes: parse_brew_url() sets its output vars directly to survive subshell calls, trailing slashes are stripped from COMPOSE_BASE_URL, and test/bootc waits for firewalld's D-Bus interface before starting the libvirt network (firewalld >= 2.4.1 no longer blocks on it at startup). parse_brew_url() is added to test/ci/utils.sh since both test/rpm/utils.sh and test/bootc/utils.sh source down to it; it isn't used by ci/utils.sh's own git-source-based install functions. No behavior change for PACKIT_COPR_RPMS-driven or local-build runs. Ports: fido-device-onboard/go-fdo-server#239 Assisted-by: Claude (claude-sonnet-5) Signed-off-by: Mario Cattamo <mcattamo@redhat.com>
|
Closing this PR and opening fido-device-onboard/go-fdo-ci#20 |
Port brew-build and compose-repository install support from fido-device-onboard/go-fdo-server#239 into the shared test/{ci,rpm,bootc}/utils.sh, so test/rpm and test/bootc can install go-fdo-client/go-fdo-server from a brew build (CLIENT_RPM_URL/SERVER_RPM_URL) or a compose snapshot (COMPOSE_BASE_URL/COMPOSE_STREAMS), on top of the existing COPR/Packit/local-build paths. Also carries over the upstream follow-up fixes: parse_brew_url() sets its output vars directly to survive subshell calls, trailing slashes are stripped from COMPOSE_BASE_URL, and test/bootc waits for firewalld's D-Bus interface before starting the libvirt network (firewalld >= 2.4.1 no longer blocks on it at startup). parse_brew_url() is added to test/ci/utils.sh since both test/rpm/utils.sh and test/bootc/utils.sh source down to it; it isn't used by ci/utils.sh's own git-source-based install functions. No behavior change for PACKIT_COPR_RPMS-driven or local-build runs. Ports: fido-device-onboard/go-fdo-server#239 Assisted-by: Claude (claude-sonnet-5) Signed-off-by: Mario Cattamo <mcattamo@redhat.com>
Port brew-build and compose-repository install support from fido-device-onboard/go-fdo-server#239 into the shared test/{ci,rpm,bootc}/utils.sh, so test/rpm and test/bootc can install go-fdo-client/go-fdo-server from a brew build (BREW_CLIENT_RPMS_URL/BREW_SERVER_RPMS_URL) or a compose snapshot (COMPOSE_BASE_URL/COMPOSE_STREAMS), on top of the existing COPR/Packit/local-build paths. rpms_from_brew_url() (test/ci/utils.sh) lists the RPM URLs actually published under a brew build's ${arch}/ and noarch/ directories, rather than hardcoding each sub-package's name, so it doesn't need to be kept in sync whenever brew's package layout changes. Also carries over the upstream follow-up fixes: trailing slashes are stripped from COMPOSE_BASE_URL, and test/bootc waits for firewalld's D-Bus interface before starting the libvirt network (firewalld >= 2.4.1 no longer blocks on it at startup). rpms_from_brew_url() is added to test/ci/utils.sh since both test/rpm/utils.sh and test/bootc/utils.sh source down to it; it isn't used by ci/utils.sh's own git-source-based install functions. No behavior change for PACKIT_COPR_RPMS-driven or local-build runs. Assisted-by: Claude (claude-sonnet-5) Signed-off-by: Mario Cattamo <mcattamo@redhat.com>
Summary
This PR extends the test infrastructure to allow installing RPM packages from two new sources — internal brew builds and compose repositories — for
test/rpmandtest/bootcdeployments. It also includes three follow-up fixes for bugs found during review and manual/CI testing of that new support.Commit 1 —
test: allow testing brew buildsIntroduces support for installing pre-built RPMs from an internal brew server by pointing at a specific build's version/release directory.
Usage:
Commit 2 —
ci: add compose repository support for RPM and bootc testsIntroduces support for installing RPM packages from a compose repository URL, targeting a specific compose snapshot.
Usage:
Commit 3 —
fix: avoid unbound _brew_* vars in parse_brew_url callersparse_brew_url()set_brew_ver/_brew_rel/_brew_archas side effects, but every caller captured it viaurl=$(parse_brew_url ...). Command substitution runs the function in a subshell, so those assignments never reached the calling scope — underset -u, the first reference to_brew_archininstall_client()/install_server()(test/ci,test/rpm,test/bootc) crashed with"unbound variable"wheneverSERVER_RPM_URLorCLIENT_RPM_URLwas actually set.Fix: have
parse_brew_url()set_brew_urldirectly instead of echoing it, and call it as a plain statement (no command substitution) so all four variables land in the caller's scope.No behavior change for any path that doesn't set
SERVER_RPM_URL/CLIENT_RPM_URL:PACKIT_COPR_RPMS-driven Packit/Testing Farm runs and the GitHub Actionstest/cimatrix never touchparse_brew_url()at all — this only affects the manual brew-build testing path introduced in Commit 1.Commit 4 —
fix: strip trailing slash from COMPOSE_BASE_URL and drop dead repo fileinstall_from_compose()(test/rpm/utils.sh) and the bootc client compose branch (test/bootc/utils.sh) now strip a trailing slash from the compose base URL before buildingbaseurl=..., avoiding a double slash if the caller passesCOMPOSE_BASE_URLwith one.files/rhel-10-2.repo: an unused static template with unreplacedREPLACE_ME_HEREplaceholders. The real RHEL repo file is generated at runtime asfiles/rhel-${VERSION_ID}.repo(test/bootc/utils.sh), so this static file was dead weight.Commit 5 —
fix(bootc): wait for firewalld D-Bus before starting libvirt networkconfigure_service_firewalld()returned right aftersystemctl start firewalld, but firewalld ≥ 2.4.1 (e.g. on Fedora 44) no longer blocks until its D-Bus interface is ready.configure_service_libvirtd()runs immediately afterward and callsvirsh net-start integrationon a network withzone='trusted', which requires firewalld's D-Bus API — racing it produces:Fix: poll
firewall-cmd --state(up to 30s) after starting firewalld, bailing out with diagnostics if the unit dies or the timeout is hit. Same root cause/fix asvirt-s1/rhel-edge#12261.