-
Notifications
You must be signed in to change notification settings - Fork 295
Add podman upstream e2e tests #23161
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
base: master
Are you sure you want to change the base?
Conversation
ed1b810
to
4c10924
Compare
19cde9a
to
927db44
Compare
574c4a7
to
910a845
Compare
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.
just one nitpick, rest is fine.
910a845
to
d5f1b05
Compare
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 interesting. In general I like the idea to add this to Tumbleweed, but there are some concerns the initial test design.
I added some questions that I would like to have discussed before proceeding further.
@@ -0,0 +1,84 @@ | |||
From 18b5b2e2d699551dab8fb25c9969d231d5662513 Mon Sep 17 00:00:00 2001 |
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 don't see the approach of applying patches to the test cases as sustainable, especially not if we want to run this on multiple systems (Tumbleweed and SLES).
Is there not a way of allowing those tests to fail and mark them as softfailures, instead of patching the test run?
This looks very weird to me and I have concerns regarding the long-time usability of this approach. Let's try to avoid this if possible.
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 sustainable. I use them extensively in the bats tests that otherwise couldn't be reliably run in SLES. They fix flakes and other stuff. Any concern you may have about patches can be solved with tooling and documentation. Patches can be tracked in an automated way. I care about removing dead code and that's why I created tooling for tracking soft-failures, etc.
I'm also not a fan of skipping tests and we must test the PR's that we submit anyway.
To sum it up:
- Patches are needed because they fix flakes. We're not applying patches that increase test coverage.
- The way we apply patches with
git apply -3 --ours
make the whole operation idempotent. - We don't have to worry about the lifetime of patches as they can be tracked with the most trivial tooling.
- Patches that are no longer needed in openSUSE Tumbleweed will eventually be needed in SLES.
- We must test the patches we submit to upstream and also test the patches suggested by upstream following the Factory First philosophy. The only way to test them is by incorporating them.
my $oci_runtime; | ||
|
||
# mapping of known expected failures | ||
my %xfails = ( |
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.
Wouldn't this be better suited as a json
file like e.g. https://github.com/os-autoinst/os-autoinst-distri-opensuse/blob/master/data/journal_check/bug_refs.json?
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.
Wouldn't this be better suited as a
json
file like e.g. https://github.com/os-autoinst/os-autoinst-distri-opensuse/blob/master/data/journal_check/bug_refs.json?
Will look into this but for now I'd rather stick with a separate mapping, given that we need to differentiate between OCI runtimes. Also bug_refs.json is used more for soft-failures and I want to explicitly avoid this concept in favour of xfail or expected failures.
sub setup { | ||
my @pkgs = qw(aardvark-dns apache2-utils buildah catatonit glibc-devel-static go1.24 gpg2 jq libgpgme-devel | ||
libseccomp-devel make netavark openssl podman podman-remote skopeo socat sudo systemd-container xfsprogs); | ||
push @pkgs, qw(criu libcriu2) if is_tumbleweed; |
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.
Since the target system is Tumbleweed at the moment, I don't understand why there is an explicit is_tumbleweed
and those are not just added to the list above?
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.
Since the target system is Tumbleweed at the moment, I don't understand why there is an explicit
is_tumbleweed
and those are not just added to the list above?
Because I ran this test against SLES 16.0 in build validation and want to be able to do this.
For reference: https://openqa.suse.de/tests/19007305#external
push @pkgs, $oci_runtime; | ||
|
||
install_packages(@pkgs); | ||
install_git; |
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 not simply add it to the list above?
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 not simply add it to the list above?
We need the latest git 2.47.0+ (not available in SLES 16.0) to use --ours
with git apply -3
. This makes the test runnable on SLES 16.
run_command "cat ~/.ssh/id_$algo.pub >> ~/.ssh/authorized_keys"; | ||
run_command "ssh-keyscan localhost 127.0.0.1 ::1 | tee -a ~/.ssh/known_hosts"; | ||
|
||
# Download podman sources |
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 have the feeling that we would be better off in mirroring the required files to data/containers/podman/e2e
(or something similar) than downloading it on every test run.
Is there an argument to be made why this should be always fresh?
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 have the feeling that we would be better off in mirroring the required files to
data/containers/podman/e2e
(or something similar) than downloading it on every test run.
There are 489 files.
Is there an argument to be made why this should be always fresh?
Because it depends on the podman version. Also git apply -3
needs a git clone
. It may fail with a --depth 1
clone because it must compare indexes.
d5f1b05
to
ef4b498
Compare
Add podman upstream e2e tests.
Verification runs:
Bugs & PR's opened:
NOTES
QEMUCPUS=4
TODO