-
Notifications
You must be signed in to change notification settings - Fork 155
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.
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
Conversation
|
Please do not merge this PR until https://gitlab.com/redhat/centos-stream/rpms/bootc/-/merge_requests/52 |
1b4bbf2 to
7b24e98
Compare
|
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
left a comment
There was a problem hiding this 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!
| esac | ||
|
|
||
| if [ "$TMT_REBOOT_COUNT" -eq 0 ]; then | ||
| # Fedora CI: https://github.com/fedora-ci/dist-git-pipeline/blob/master/Jenkinsfile#L145 |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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$') |
There was a problem hiding this comment.
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" \; ?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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.
No worried at all. Your comments are always welcome!! And helpful!! :-) Thank you! |
|
I'm debugging code change on Fedora CI now. |
cgwalters
left a comment
There was a problem hiding this 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!
d4b3ef2 to
27c5753
Compare
Also rename test-00-bootc-install to bootc-install-provision to make more sense Signed-off-by: Xiaofeng Wang <[email protected]>
|
Fedora CI test passed: |
This PR includes 0000-bootc-inistall.patch in PR https://src.fedoraproject.org/rpms/bootc/pull-request/18#.
The
0000-bootc-inistall.patchcan be removed in the next release.