test/system: Don't assume that no container will have /etc/kernel#1409
test/system: Don't assume that no container will have /etc/kernel#1409debarshiray merged 1 commit intocontainers:mainfrom
Conversation
03be9a5 to
b95e018
Compare
|
Build failed. ✔️ unit-test SUCCESS in 6m 58s |
There was a problem hiding this comment.
Thanks for your contribution!
We should definitely try to get the tests ready to be run on Ubuntu hosts, so that we can enable them in our CI. The tests do have lots of assumptions about running on Fedora hosts at the moment, which isn't good. So thanks for looking into that.
However, do Ubuntu hosts really not have /etc/kernel? So far, I tried Ubuntu 22.04, 22.10, 23.04 & 23.10 Desktop, and they all had `/etc/kernel.
I am not really opposed to the change, other than someone wanting to share /var/tmp between the host and container in future, but I want to understand the problem that you found.
debarshiray
left a comment
There was a problem hiding this comment.
The commit message could be slightly better. Something like:
"test/system: Don't assume that all host OSes will have /etc/kernel"
test/system/104-run.bats
Outdated
|
|
||
| pushd /etc/kernel | ||
| local testdir | ||
| testdir="$(mktemp -d '/var/tmp/toolbox-test-XXXX')" |
There was a problem hiding this comment.
Nitpick: the single quotes aren't necessary.
containers#1409 Signed-off-by: Penn Bauman <me@pennbauman.com>
b95e018 to
ad2e69e
Compare
|
I took the liberty to address the nits and updated the branch by rebasing against |
|
Ubuntu hosts do have the You can simulate this on fedora if you temporarily add the following to |
|
Build succeeded. ✔️ unit-test SUCCESS in 9m 11s |
Any image or container that has APT or systemd may have /etc/kernel. eg., the arch-toolbox and ubuntu-toolbox images. containers#1409 Signed-off-by: Penn Bauman <me@pennbauman.com>
ad2e69e to
2f3ee0e
Compare
Aha, okay, you are right. I didn't read your initial comment carefully.
Thanks for the detailed example. Let's merge this once the CI finishes. |
|
@pennbauman Since you spent some time running the test on non-Fedora hosts, would you mind submitting a new PR that runs this "fallback working directory" test on other images, and not just the default one? eg., Arch Linux, RHEL, Ubuntu, etc. like we do elsewhere. |
|
Build succeeded. ✔️ unit-test SUCCESS in 8m 54s |
|
@debarshiray I was thinking that would be a good next step. I will look into that when I have some time. |
Thanks! |
Any image or container that has APT or systemd may have /etc/kernel. eg., the arch-toolbox and ubuntu-toolbox images. containers#1409 containers#1750 Signed-off-by: Penn Bauman <me@pennbauman.com> (backported from commit 2f3ee0e)
The test "run: Ensure that $HOME is used as a fallback working directory" assumes
/etc/kernelwill not be present in containers, but this is not the case for Ubuntu (and Debian) containers. This patch switches the test to using a temporary directory (under/var/tmpwhich is not mounted by toolboxes) to ensure fallback is activated.