USHIFT-5397: [QE] Replace MicroShift build images phase with dev scripts #4900

agullon · 2025-05-13T13:19:04Z

No description provided.

openshift-ci-robot · 2025-05-13T13:20:45Z

@agullon: This pull request references USHIFT-5397 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target either version "4.20." or "openshift-4.20.", but it targets "openshift-4.19" instead.

In response to this:

Instructions 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 openshift-eng/jira-lifecycle-plugin repository.

openshift-ci · 2025-05-13T13:20:55Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

agullon · 2025-06-13T11:09:14Z

test/bin/build_rpms.sh

@@ -119,13 +120,22 @@ download_brew_rpms() {
    mkdir -p "${BREW_RPM_SOURCE}"
    if "${SCRIPTDIR}/manage_brew_rpms.sh" access ; then


@ggiguash This block is not accessible from CI on cache-nightly runs, see this log:

+ 00:55:44.906969816 ./bin/build_rpms.sh:130 echo 'WARNING: The Brew Hub site is not accessible, skipping the download'

For that reason, I also implemented this logic in the CI script, see openshift/release#66024 PR

I suggest we should extract the block to download RPMs from brew into a separate function and use it here and on release repo instead of duplicating the code.

You are right. Sorry for misleading you.

There is no need to extract it into separate function.
Just add the necessary version downloads in the release repo

I'm sorry, I don't understand what you mean by necessary version downloads

I meant adding the following commands here - pseudo code:

bash -x ./test/bin/manage_brew_rpms.sh download "${ocpversion}" "${out_path}" bash -x ./test/bin/manage_brew_rpms.sh download "${ocpversion}" - 1 "${out_path}" bash -x ./test/bin/manage_brew_rpms.sh download "${ocpversion}" - 2 "${out_path}"

done here: https://github.com/openshift/microshift/pull/4900/files#diff-c5558625fd237b08c74b6c9a3a55f7d994792bebf3c4a9ee2548a96d18810db4R130-R136

… into AWS S3 bucket

…FT-5397

… into AWS S3 bucket

…FT-5397

agullon · 2025-06-17T11:40:46Z

/retest-required

agullon · 2025-06-17T12:49:55Z

/retest-required

agullon · 2025-06-17T13:10:01Z

/retest-required

test/bin/manage_brew_rpms.sh

ggiguash · 2025-06-17T15:03:59Z

test/bin/manage_brew_rpms.sh

-        echo "ERROR: Cannot find MicroShift '${ver}' packages in brew"
-        exit 1
+        echo "WARNING: Cannot find MicroShift '${ver}' packages in brew"
+        exit 0


Why do we need this to be a non-fatal exit?

Because of the automation from CI job when running for latest non released versions like 4.20. For example, when ER or RC don't exist, the script must continue running.

I would like to make this conditional then. We should not allow failure to fetch main packages.
Let's issue a warning and skip for specific package types (exception), or fail as a rule?

done it in 2de69c5 commit
Also, added skip for released brew version here: 1b9f044

ggiguash · 2025-06-17T15:05:06Z

test/bin/manage_brew_rpms.sh

+        if [ -z "${ver_type}" ] ; then
+            adir="${dir}/${arch}"
+        else
+            adir="${dir}/$(echo "${package}" | sed 's/.*microshift-\([^-]*\).*/\1/')/${arch}"


Can we split this expression into another var?
Please, explain in the comment what we're trying to extract.

Split into an extra var in c05d860 commit.
The reason I extract it is to have this dir structure:

$ tree -L 2 _output/test-images/brew-rpms-released _output/test-images/brew-rpms-released ├── 4.16.41 │ ├── aarch64 │ └── x86_64 ├── 4.17.27 │ ├── aarch64 │ └── x86_64 ├── 4.18.0~0.nightly_2025_06_16_132618 │ ├── aarch64 │ └── x86_64 ├── 4.18.0~ec.4 │ ├── aarch64 │ └── x86_64 ├── 4.18.0~rc.10 │ ├── aarch64 │ └── x86_64 └── 4.18.11 ├── aarch64 └── x86_64

instead of this:

$ tree -L 2 _output/test-images/brew-rpms-released _output/test-images/brew-rpms-released ├── microshift-4.16.41-202505260636.p0.g789ff62.assembly.4.16.41.el9 │ ├── aarch64 │ └── x86_64 ├── microshift-4.17.27-202504251637.p0.ge4822bf.assembly.4.17.27.el9 │ ├── aarch64 │ └── x86_64 ├── microshift-4.18.0~0.nightly_2025_06_16_132618-202506170128.p0.gbc96f1d.assembly.microshift.el9 │ ├── aarch64 │ └── x86_64 ├── microshift-4.18.0~ec.4-202411221636.p0.gbfd3ba1.assembly.ec.4.el9 │ ├── aarch64 │ └── x86_64 ├── microshift-4.18.0~rc.10-202502170426.p0.g84f67ec.assembly.rc.10.el9 │ ├── aarch64 │ └── x86_64 └── microshift-4.18.11-202504281317.p0.g48a03d6.assembly.4.18.11.el9 ├── aarch64 └── x86_64

What do you think? to be honest not sure if it's worth it.

test/bin/manage_brew_rpms.sh

ggiguash · 2025-06-17T15:12:57Z

test/bin/build_rpms.sh

@@ -126,6 +126,14 @@ download_brew_rpms() {
        # Run the download procedure
        bash -x "${SCRIPTDIR}/../../scripts/fetch_tools.sh" brew
        bash -x "${SCRIPTDIR}/manage_brew_rpms.sh" download "4.${MINOR_VERSION}" "${BREW_RPM_SOURCE}"
+
+        out_path="${IMAGEDIR}/released-brew-rpms"


Do we have to put those in a separate directory?
Nit: if we have to, can we use brew-rpms-released name so that they are sorted together in the output?

We do not have to, no needed it. But I did it this way to change as less as possible the rest of the code on manage_build_cache.sh script when uploading the RPMs into S3 bucket.
I agree to rename it to brew-rpms-released, done in 615fc4e commit.

Co-authored-by: Gregory Giguashvili <[email protected]>

test/bin/manage_brew_rpms.sh

ggiguash · 2025-06-18T15:17:07Z

test/bin/manage_brew_rpms.sh

+        else
+            # remove date and commit id from package name and use it in dir name
+            version=$(echo "${package}" | sed 's/.*microshift-\([^-]*\).*/\1/')
+            adir="${dir}/${version}/${arch}"


Can we simplify this and just use ${version_type} instead of ${version}?

I think it's not a good idea. I prefer a directory explicitly pointing to the version:

$ ls -1d * 4.16.41 4.17.27 4.18.0~0.nightly_2025_06_18_163729 4.18.0~ec.4 4.18.0~rc.10 4.18.11

instead of:

$ ls -1d * ec nightly rc zstream

or

$ ls -1d * 4.16-zstream 4.17-zstream 4.18-ec 4.18-nightly 4.18-rc 4.18-zstream

Can you explain why?
Having floating top-level names also makes is more difficult to automate the usage of those.

With the <version>-<version_type> naming, you can just point to a version and its type in the automation and run with whatever is there. Note that the actual versions are still present, but inside the directory.

Not sure if we are both in the same page here. I'll try to explain myself better.

For me, the final goal of this PR is to have a database/repository (S3 bucket in this case, which is perfect) from where I can fetch any given MicroShift version (RPMs, images or refs) to deploy a MicroShift cluster and run release regression testing.

So my idea/proposal is to put all those artifacts (RPMs, images and refs) in a S3 bucket directory to point to them when triggering the release regression testing. By using the naming approach, 4.18-zstream I find it difficult to know beforehand which version is that: 4.18.2? 4.18.11?

For what you were trying to achieve, the versioning makes sense. I missed that point, so thank you for explaining.

As I commented in another thread, changing release-specific storage of artifacts is not what we do and I'm not sure it's a way to go.

I would suggest to continue with storing the artifacts under a given release because all the system is written to work like this. Then, I suppose, we can use more generic names, like 4.18-zstream, etc.

I'm thinking about this regarding the images, how to build them pointing to the right parent version for upgrades, and I convinced myself we should follow the current approach. Otherwise, I foresee another complete and different approach for parent images, which I think it's not a good idea at all.

So, I change it to follow current approach.

ggiguash · 2025-06-18T15:19:34Z

test/bin/manage_build_cache.sh

+
+    # Upload released RPM from brew
+    pushd "${src_base}"
+    released_brew_rpms_dirs="$(find "brew-rpms-released" -maxdepth 1 -type d | tail -n +2)"


Why can't we add brew-rpms-released to the loop above?

We also need to implement download operation

Because I think it's better to put the artifacts (RPMs, images and refs) in a separate S3 bucket directory. With this approach, we won't have them duplicated for every directory branch, and we save resources by uploading them for every directory branch.

Yes, my idea was to do it on another PR. Because this PR should only focus on the build phase.

This requires a discussion. We do not store version collection of data without a release tag at this time. I do not think we should change this approach.

The disk space (duplication) is not a problem. We need to manage release-specific information in the buckets. This script is release specific and it should only manipulate release-specific data.

I see, no problem then.

Co-authored-by: Gregory Giguashvili <[email protected]>

… brew by default

agullon · 2025-06-19T09:30:54Z

test/bin/manage_build_cache.sh

@@ -102,7 +102,10 @@ action_upload() {
    run_aws_cli s3 sync --quiet --include '*.iso' "${iso_base}" "${iso_dest}"

    # Upload mirror-registry, brew-rpms and repo archives
-    for dir in mirror-registry brew-rpms repo ; do
+    pushd "${src_base}"
+    brew_rpms_dirs="$(find "brew-rpms" -maxdepth 1 -type d | grep '4..[0-9]-')"


Output example from these lines to understand dir structure:

$ pushd _output/test-images/ ~/microshift/_output/test-images ~/microshift $ find "brew-rpms" -maxdepth 1 -type d | grep '4..[0-9]-' brew-rpms/4.18-nightly brew-rpms/4.18-zstream brew-rpms/4.17-zstream brew-rpms/4.16-zstream brew-rpms/4.18-rc brew-rpms/4.18-ec

The download should be relatively fast for RPMs. Let's try not to optimize upfront. We can start from a simple on tar and optimize if necessary in the future.

This way, we should also get upload and download without changes in this script.

I agree, remove changes from manage_build_cache.sh on this PR.

ggiguash · 2025-06-19T12:57:16Z

test/bin/manage_brew_rpms.sh

+        rc|ec)
+            package=$(brew list-builds --quiet --package=microshift --state=COMPLETE | grep "^microshift-${ver}.0~${ver_type}." | tail -1) || true
+            ;;
+        *)


If version_type is not specified, let's use nightly by default.
We also need to produce an error in case ver_type is not one of the supported strings.

done in c54d483 commit

ggiguash · 2025-06-19T14:49:29Z

/lgtm

openshift-ci · 2025-06-19T14:49:52Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: agullon, ggiguash

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [agullon,ggiguash]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

agullon · 2025-06-19T15:00:54Z

/retest-required

openshift-ci-robot · 2025-06-19T15:26:10Z

/retest-required

Remaining retests: 0 against base HEAD 1fdfec0 and 2 for PR HEAD 86b9afe in total

openshift-ci-robot · 2025-06-19T17:26:10Z

/retest-required

Remaining retests: 0 against base HEAD b72066d and 1 for PR HEAD 86b9afe in total

agullon · 2025-06-20T05:46:56Z

/retest-required

agullon · 2025-06-20T07:39:59Z

/retest-required

agullon · 2025-06-20T13:50:21Z

/retest-required

openshift-ci-robot · 2025-06-20T17:45:36Z

/retest-required

Remaining retests: 0 against base HEAD b72066d and 2 for PR HEAD 86b9afe in total

ggiguash · 2025-06-21T05:37:58Z

The failures have nothing to do with the changes. Saving on CI cycles.
/override ci/prow/e2e-aws-tests-bootc-periodic ci/prow/test-unit ci/prow/verify

openshift-ci · 2025-06-21T05:38:15Z

@ggiguash: Overrode contexts on behalf of ggiguash: ci/prow/e2e-aws-tests-bootc-periodic, ci/prow/test-unit, ci/prow/verify

In response to this:

The failures have nothing to do with the changes. Saving on CI cycles.
/override ci/prow/e2e-aws-tests-bootc-periodic ci/prow/test-unit ci/prow/verify

Instructions 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.

openshift-ci · 2025-06-21T05:43:43Z

@agullon: all tests passed!

Full PR test history. Your PR dashboard.

Instructions 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.

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label May 13, 2025

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 13, 2025

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 13, 2025

agullon commented Jun 13, 2025

View reviewed changes

Add support to fetch different RPM versions from brew and upload them…

a254eca

… into AWS S3 bucket

agullon force-pushed the USHIFT-5397 branch from 962146d to a254eca Compare June 17, 2025 10:58

agullon added 6 commits June 17, 2025 12:59

Merge branch 'openshift:main' into USHIFT-5397

1e46318

Merge branch 'main' into USHIFT-5397

61bc4c9

Merge branch 'USHIFT-5397' of github.com:agullon/microshift into USHI…

4b442ef

…FT-5397

fix merge conflict from manual merge

bca2b46

Add support to fetch different RPM versions from brew and upload them…

b63ab4e

… into AWS S3 bucket

Merge branch 'USHIFT-5397' of github.com:agullon/microshift into USHI…

0accb65

…FT-5397

agullon marked this pull request as ready for review June 17, 2025 11:12

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 17, 2025

openshift-ci bot requested review from ggiguash and pmtk June 17, 2025 11:13

ggiguash reviewed Jun 17, 2025

View reviewed changes

agullon and others added 5 commits June 18, 2025 10:51

improve usage message description

4597529

use case block

6c39fc7

Update test/bin/manage_brew_rpms.sh

cc0b959

Co-authored-by: Gregory Giguashvili <[email protected]>

rename output dir for release brew RPMs

615fc4e

split version name into another var

c05d860

ggiguash reviewed Jun 18, 2025

View reviewed changes

test/bin/manage_brew_rpms.sh Outdated Show resolved Hide resolved

ggiguash reviewed Jun 18, 2025

View reviewed changes

test/bin/manage_brew_rpms.sh Outdated Show resolved Hide resolved

ggiguash reviewed Jun 18, 2025

View reviewed changes

agullon and others added 7 commits June 18, 2025 17:23

fix usage message

7e537af

Co-authored-by: Gregory Giguashvili <[email protected]>

fixe case block formatting

c815ab9

Co-authored-by: Gregory Giguashvili <[email protected]>

fix case block format

13c7437

exit when error when brew package can not be found or downloaded from…

2de69c5

… brew by default

skip exist with error if released brew version not found

1b9f044

chaange dir structure for all RPMs fetched from brew

b0bf2a6

set version_type option mandatory

50d9f97

agullon commented Jun 19, 2025

View reviewed changes

ggiguash reviewed Jun 19, 2025

View reviewed changes

agullon added 5 commits June 19, 2025 15:14

set nightly version type by default when not set

5a9c20b

add error handling for not supported version types

c54d483

add missing line

3b1bd23

no need to make any change here

a16b680

fix

86b9afe

openshift-ci bot assigned ggiguash Jun 19, 2025

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jun 19, 2025

openshift-merge-bot bot merged commit 844791b into openshift:main Jun 21, 2025
13 checks passed

		@@ -119,13 +120,22 @@ download_brew_rpms() {
		mkdir -p "${BREW_RPM_SOURCE}"
		if "${SCRIPTDIR}/manage_brew_rpms.sh" access ; then

USHIFT-5397: [QE] Replace MicroShift build images phase with dev scripts #4900

USHIFT-5397: [QE] Replace MicroShift build images phase with dev scripts #4900

Conversation

agullon commented May 13, 2025

Uh oh!

openshift-ci-robot commented May 13, 2025 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci bot commented May 13, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ggiguash Jun 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

agullon commented Jun 17, 2025

Uh oh!

agullon commented Jun 17, 2025

Uh oh!

agullon commented Jun 17, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ggiguash Jun 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

openshift-ci-robot commented May 13, 2025 •

edited by openshift-ci bot

Loading

ggiguash Jun 13, 2025 •

edited

Loading

ggiguash Jun 19, 2025 •

edited

Loading