Skip to content

test: update bootc install script to support Fedora CI gating test #1282

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
May 3, 2025
Merged

test: update bootc install script to support Fedora CI gating test #1282

merged 1 commit into from
May 3, 2025

Conversation

henrywang
Copy link
Collaborator

This PR includes 0000-bootc-inistall.patch in PR https://src.fedoraproject.org/rpms/bootc/pull-request/18#.

The 0000-bootc-inistall.patch can be removed in the next release.

@henrywang henrywang changed the title test: update bootc install script to support Fedora CI gating test WIP: test: update bootc install script to support Fedora CI gating test Apr 24, 2025
@henrywang
Copy link
Collaborator Author

Please do not merge this PR until https://gitlab.com/redhat/centos-stream/rpms/bootc/-/merge_requests/52 tier-0 test passed.

@henrywang henrywang force-pushed the fedora_ci branch 4 times, most recently from 1b4bbf2 to 7b24e98 Compare April 30, 2025 14:03
@henrywang henrywang changed the title WIP: test: update bootc install script to support Fedora CI gating test test: update bootc install script to support Fedora CI gating test Apr 30, 2025
@henrywang
Copy link
Collaborator Author

Hi @cgwalters, I think the right flow should be land this PR first, then make a bootc release, update Packit release PR in https://src.fedoraproject.org/rpms/bootc to include gating test settings in https://src.fedoraproject.org/rpms/bootc/pull-request/23 and https://src.fedoraproject.org/rpms/bootc/pull-request/22.

BTW, I'll close https://src.fedoraproject.org/rpms/bootc/pull-request/23 and https://src.fedoraproject.org/rpms/bootc/pull-request/22 and waiting for Packit release PR to add gating in it.

Any thoughts on this flow? Thanks.

cgwalters
cgwalters reviewed Apr 30, 2025
View reviewed changes
Copy link
Collaborator

@cgwalters cgwalters left a comment

Choose a reason for hiding this comment

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

Thanks for working on this! Some comments here, but to be clear I'm overall fine to merge as is to keep testing work unblocked and continue to iterate on improvements post merge!

tmt/tests/bootc-install-provision.sh
@@ -19,7 +19,27 @@ case "$ID" in
esac

if [ "$TMT_REBOOT_COUNT" -eq 0 ]; then
# Fedora CI: https://github.com/fedora-ci/dist-git-pipeline/blob/master/Jenkinsfile#L145
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is something I keep getting hung up on. So AIUI today, Testing Farm basically does not handle the job of injecting (or building) the artifact-under-test, it's the reponsibility of a different tool. And commonly today, that different tool is Packit (which is very RPM oriented).

Would it be accurate then to say this is basically doing something similar to what Packit is doing?

Is there hookup/alignment between the fedora-ci/osci and packit or do they just overlap in this sense?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

AFAIK, the fedora ci and osci don't use Packit yet.

tmt/tests/bootc-install-provision.sh Outdated
cat /etc/yum.repos.d/test-artifacts.repo
ls -al /var/share/test-artifacts
# get bootc and system-reinstall-bootc rpm package name
BOOTC_RPM_FULL_PATH=$(find /var/share/test-artifacts/ -type f | grep -E '/bootc-[0-9]{1,2}.[0-9]{1,2}.[0-9]{1,2}-[0-9]{1,2}.(fc|el)[0-9]{1,2}.(x86_64|aarch64).rpm$')
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why can't we just find /var/share/test-artifacts -type f -name '*.rpm' -exec cp {} "$BOOTC_TEMPDIR" \; ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There're src and debug rpms in the same folder. So I have to filter two rpms with regex.

Copy link
Collaborator

Choose a reason for hiding this comment

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

OK. At least if we do the bind mount we won't need to filter here. And I think rpm -Fvh --oldpackage should ensure we only upgrade already installed packages.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I removed all those regrex and use bind mount and dnf -y upgrade /rpms/*.rpm instead.

tmt/tests/bootc-install-provision.sh Outdated

COPY $BOOTC_RPM_FILE_NAME /$BOOTC_RPM_FILE_NAME
COPY $SYSTEM_REINSTALL_BOOTC_RPM_FILE_NAME /$SYSTEM_REINSTALL_BOOTC_RPM_FILE_NAME
RUN dnf remove -y bootc system-reinstall-bootc && dnf install -y /$BOOTC_RPM_FILE_NAME /$SYSTEM_REINSTALL_BOOTC_RPM_FILE_NAME rpm-ostree
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is rpm-ostree mentioned here?

It'd also seem cleaner again to me to operate on *.rpm so adding a new subpackage wouldn't require changing a bunch of code. If we do the bind mount above I think this is could be
RUN rpm -Uvh --oldpackage /rpms/*.rpm or so

tmt/tests/bootc-install-provision.sh Outdated
FROM $TIER1_IMAGE_URL

COPY $BOOTC_RPM_FILE_NAME /$BOOTC_RPM_FILE_NAME
COPY $SYSTEM_REINSTALL_BOOTC_RPM_FILE_NAME /$SYSTEM_REINSTALL_BOOTC_RPM_FILE_NAME
Copy link
Collaborator

Choose a reason for hiding this comment

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

How about just bind mounting in /var/share/test-artifacts like podman build -v /var/share/test-artifacts:/rpms:ro ? That would involve a lot less copying and filename manipulation.

@henrywang
Copy link
Collaborator Author

Thanks for working on this! Some comments here, but to be clear I'm overall fine to merge as is to keep testing work unblocked and continue to iterate on improvements post merge!

No worried at all. Your comments are always welcome!! And helpful!! :-) Thank you!

@henrywang
Copy link
Collaborator Author

I'm debugging code change on Fedora CI now.

cgwalters
cgwalters approved these changes May 1, 2025
View reviewed changes
Copy link
Collaborator

@cgwalters cgwalters left a comment

Choose a reason for hiding this comment

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

Feel free to merge when ready!

@henrywang henrywang force-pushed the fedora_ci branch 2 times, most recently from d4b3ef2 to 27c5753 Compare May 1, 2025 17:42
@henrywang
test: update bootc install script to support Fedora CI gating test
ad936b7 
Also rename test-00-bootc-install to bootc-install-provision to
make more sense

Signed-off-by: Xiaofeng Wang <henrywangxf@me.com>
@henrywang
Copy link
Collaborator Author

Fedora CI test passed:
f41: https://src.fedoraproject.org/rpms/bootc/pull-request/23
f42: https://src.fedoraproject.org/rpms/bootc/pull-request/22

@henrywang henrywang merged commit 578063b into bootc-dev:main May 3, 2025
30 of 31 checks passed
@henrywang henrywang deleted the fedora_ci branch May 3, 2025 03:07
@jmarrero jmarrero mentioned this pull request May 9, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cgwalters cgwalters cgwalters approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@henrywang @cgwalters