-
Notifications
You must be signed in to change notification settings - Fork 192
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
upgrade-test: add 2 upgrades test #4806
Conversation
dc6b0a1
to
97657b0
Compare
The basic tests include
|
d9ed6f7
to
07f72de
Compare
885cab9
to
0586cc0
Compare
e11c68e
to
fb38b73
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 really great! This kind of testing was sorely needed.
.cci.jenkinsfile
Outdated
make -C tests/kolainst upgrades-install | ||
mv rpm-ostree-[0-9]*.rpm rpm-ostree-libs-[0-9]*.rpm /usr/lib/coreos-assembler/tests/kola/rpm-ostree/upgrades/data/ | ||
""") | ||
kola(cosaDir: "${env.WORKSPACE}", extraArgs: "--qemu-image ./fedora-coreos*.qcow2 ext.rpm-ostree.upgrades.*", parallel: 2) |
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 was thinking more of having the OS rollback to an earlier version on its first boot, but yeah this is more proper even if more wordy. :) (Kola also has support for downloading the previous image, but as part of the run-upgrade
suite; I think we should really fold that back into kola run
proper and support external tests.)
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.
(Kola also has support for downloading the previous image, but as part of the run-upgrade suite; I think we should really fold that back into kola run proper and support external tests.)
Good suggestion, then this will make upgrade tests more easier, will change that later.
tests/kolainst/Makefile
Outdated
@@ -22,3 +22,9 @@ install: all | |||
localinstall: all | |||
rm -rf ../kola | |||
make install KOLA_TESTDIR=../kola | |||
|
|||
upgrades-install: all |
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.
Instead of this, we could make it part of the destructive
suite, but add a e.g. requiredTag: rpm-ostree-upgrade
to it so that it doesn't run in the default testsuite run, and will only run if you pass --tag rpm-ostree-upgrade
.
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.
SGTM, thanks!
## minMemory: 2048 | ||
## tags: needs-internet | ||
## description: Start old build, upgrade rpm-ostree to new version, | ||
## do client-side layering tests, then upgrade to another new update. |
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.
Sorry, can you expand on #4784 (comment) ? Isn't fedora-silverblue/issue-tracker#523 covered by your first test where we do an override remove
on the old version and then upgrade to the new version and then upgrade again from the new version to another version (with the same cached RPM content)?
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.
Yes, I meant to add there like that, but then I thought if we add such tests in kolainst that can cover more tests instead of just remove, no need to change in e2e-upgrades
.
But agree with you that it would be better to add the tests to e2e-upgrades
instead of kolainst.
/test kola-upgrade |
Seems |
OK yeah, I think I understand better now. Basically we want to test new rpm-ostree on a system composed by old rpm-ostree. There's a subtle difference from the scenario in fedora-silverblue/issue-tracker#523 though: the override remove was done with the old rpm-ostree first and then upgraded to a compose with the new rpm-ostree (but that's still composed by old rpm-ostree). I don't think it matters for catching #4746, but does for #4727 since the pkgcache was generated by old rpm-ostree in one case but new rpm-ostree in the other. Do you agree? I think it does make sense to test both cases, which is what you're doing with the Prow version and the kolainst version of these tests. But man... this stuff is complex. Maybe let's add a link to this PR (or maybe even this comment directly) in the tests? |
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.
Some comments, but LGTM overall! Thank you so much for working on this.
Before, even though a test had a required tag, explicitly passing a pattern which matched that test name would still include it. I thought that would be convenient, but it makes the logic less predictable. Since we don't use many required tags today, let's instead opt for being more explicit and always actually require the tag. Motivated by coreos/rpm-ostree#4806 (comment).
Should be fixed by coreos/coreos-assembler#3710! |
Before, even though a test had a required tag, explicitly passing a pattern which matched that test name would still include it. I thought that would be convenient, but it makes the logic less predictable. Since we don't use many required tags today, let's instead opt for being more explicit and always actually require the tag. Motivated by coreos/rpm-ostree#4806 (comment).
Yes, yes, you are totally correct. So we should have tests:
Is my understanding is correct? |
08ff36f
to
667cf69
Compare
- `kolainst`: Do client-side layering tests on old build, then upgrade rpm-ostree to new version, then upgrade to another update. - `prow/e2e-upgrades`: Start old build with upgraded rpm-ostree, add layering tests, then upgrade to another new update.
coreos-installer download -p qemu -f qcow2.xz -s testing --decompress | ||
mv rpm-ostree-[0-9]*.rpm rpm-ostree-libs-[0-9]*.rpm /usr/lib/coreos-assembler/tests/kola/rpm-ostree/destructive/data/ | ||
""") | ||
kola(cosaDir: "${env.WORKSPACE}", extraArgs: "--qemu-image ./fedora-coreos*.qcow2 --tag rpm-ostree-upgrade ext.rpm-ostree.destructive.client-layering-upgrade") |
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.
BTW, this could now just be
kola(cosaDir: "${env.WORKSPACE}", extraArgs: "--qemu-image ./fedora-coreos*.qcow2 --tag rpm-ostree-upgrade")
since it's the only test with that tag.
Also, we probably want skipUpgrade: true
here.
And finally, it doesn't matter much, though re. --qemu-image ....qcow2
, that would belong better as a platformArgs
argument. (It would matter if we e.g. didn't disable the default upgrade tests.)
|
||
/tmp/autopkgtest-reboot 2 | ||
;; | ||
"2") |
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.
We should verify here that the booted ref is kola
to make sure e.g. stage finalization didn't fail.
Xerf to Jonathan's comment coreos#4806 (comment)
Xerf to Jonathan's comment coreos#4806 (comment)
Enhance 2 upgrade tests:
kolainst
: Do client-side layering tests on old build, then upgraderpm-ostree to new version, then upgrade to another new update.
prow/e2e-upgrades
: Start old build with upgraded rpm-ostree,do layering tests, then upgrade to another new update.
Fixes: #4776