Skip to content

ci: allow the installation of greenboot rpm packages from compose#178

Open
mcattamoredhat wants to merge 1 commit intofedora-iot:mainfrom
mcattamoredhat:rc-compose-tier1-testing
Open

ci: allow the installation of greenboot rpm packages from compose#178
mcattamoredhat wants to merge 1 commit intofedora-iot:mainfrom
mcattamoredhat:rc-compose-tier1-testing

Conversation

@mcattamoredhat
Copy link
Copy Markdown
Contributor

This PR suggest the changes to allow the installation of the greenboot rpm packages from a given compose specified by a local variable COMPOSE_ID.

If that variable is defined, qcow2, anaconda-iso, and ostree ci test cases will fetch the rpms from the RHEL composes available through DOWNLOAD_NODE url. So that local testing efforts are simpler and more automated.

Comment thread tests/greenboot-ostree.sh
BOOT_ARGS="uefi"
BOOT_LOCATION="http://${DOWNLOAD_NODE}/rhel-9/nightly/RHEL-9/latest-RHEL-9.8.0/compose/BaseOS/x86_64/os/"
if [[ "${USE_COMPOSE_RPMS}" == true ]]; then
GREENBOOT_PACKAGES_URL="https://${DOWNLOAD_NODE}/rhel-9/composes/RHEL-9/${COMPOSE_ID}/compose/AppStream/x86_64/os/Packages/"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that { set +x; } 2>/dev/null ... set -x is missing here.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, for sake of consistency with the other scripts we can have it here...

Comment thread tests/greenboot-ostree.sh
Comment on lines +117 to +135
greenprint "Downloading greenboot RPMs from compose: ${GREENBOOT_PACKAGES_URL}"

GREENBOOT_RPM="$(curl -kfsSL --retry 5 --retry-delay 2 --retry-all-errors \
"${GREENBOOT_PACKAGES_URL}" \
| grep -oE 'greenboot-[0-9][^"]*\.rpm' \
| grep -vE 'debug(info|source)' \
| sort -V | tail -n 1)"
echo "Selected greenboot RPM: ${GREENBOOT_RPM}"

GREENBOOT_DEFAULT_RPM="$(curl -kfsSL --retry 5 --retry-delay 2 --retry-all-errors \
"${GREENBOOT_PACKAGES_URL}" \
| grep -oE 'greenboot-default-health-checks-[0-9][^"]*\.rpm' \
| sort -V | tail -n 1)"
echo "Selected greenboot-default-health-checks RPM: ${GREENBOOT_DEFAULT_RPM}"

curl -kfsSL --retry 5 --retry-delay 2 --retry-all-errors \
"${GREENBOOT_PACKAGES_URL}${GREENBOOT_RPM}" -o "/var/www/html/packages/${GREENBOOT_RPM}"
curl -kfsSL --retry 5 --retry-delay 2 --retry-all-errors \
"${GREENBOOT_PACKAGES_URL}${GREENBOOT_DEFAULT_RPM}" -o "/var/www/html/packages/${GREENBOOT_DEFAULT_RPM}"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that { set +x; } 2>/dev/null ... set -x is missing here.

greenprint "Downloading greenboot RPMs from compose"
rm -f greenboot-*.rpm

GREENBOOT_RPM="$(curl -kfsSL --retry 5 --retry-delay 2 --retry-all-errors \
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would explain why the option -k is needed.

greenprint "Downloading greenboot RPMs from compose"
rm -f greenboot-*.rpm

GREENBOOT_RPM="$(curl -kfsSL --retry 5 --retry-delay 2 --retry-all-errors \
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would explain why the option -k is needed.

Comment thread tests/greenboot-ostree.sh
if [[ "${USE_COMPOSE_RPMS}" == true && -n "${GREENBOOT_PACKAGES_URL}" ]]; then
greenprint "Downloading greenboot RPMs from compose: ${GREENBOOT_PACKAGES_URL}"

GREENBOOT_RPM="$(curl -kfsSL --retry 5 --retry-delay 2 --retry-all-errors \
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would explain why the option -k is needed.

Copy link
Copy Markdown

@knecasov knecasov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a few comments.

Have you considered extracting the shared parts of the code (for example, an RPM download logic) into a common helper, please? The same block is repeated across all three scripts.

greenprint "Downloading greenboot RPMs from compose"
rm -f greenboot-*.rpm

GREENBOOT_RPM="$(curl -kfsSL --retry 5 --retry-delay 2 --retry-all-errors \
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The following command could be extracted into a variable to download the page once. It applies to all occurrences.

curl -kfsSL --retry 5 --retry-delay 2 --retry-all-errors \  
        "${GREENBOOT_PACKAGES_URL}"

Copy link
Copy Markdown
Member

@say-paul say-paul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Comment thread tests/greenboot-ostree.sh
BOOT_ARGS="uefi"
BOOT_LOCATION="http://${DOWNLOAD_NODE}/rhel-9/nightly/RHEL-9/latest-RHEL-9.8.0/compose/BaseOS/x86_64/os/"
if [[ "${USE_COMPOSE_RPMS}" == true ]]; then
GREENBOOT_PACKAGES_URL="https://${DOWNLOAD_NODE}/rhel-9/composes/RHEL-9/${COMPOSE_ID}/compose/AppStream/x86_64/os/Packages/"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, for sake of consistency with the other scripts we can have it here...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants