Add testing support for OpenSUSE Tumbleweed #21335
Conversation
Tumbleweed's file doesn't correctly recognise this as compressed kdump but instead just as regular data, so this check would always fail It also uses a date format for the file paths
This is due to zyppers default timeouts being really long since the VM's don't run with network access, we'll just disable them
This group exists by default in Tumbleweed, so we need to see if it exists first
There was a problem hiding this comment.
phew that's a mouthful :grin Thanks for working through them!
To be honest, this is too big to ever have a chance to land. Please don't try to fix everything at once -- trust me, it's going to be such a yak shave that both you and me will just give up in desperation.
I suggest that you split this up into multiple smaller stages and pull requests.
-
Start with a PR that clears away the generic stupid boring stuff, like ".gitignore: Ignore test results, the ruff fix, lease path to /run/dnsmasq, and similar. We can test and land that without an opensuse-tw image and test.
-
Sort out the image fixes in Add testing support for OpenSUSE Tumbleweed bots#7143
-
A minimal PR that fixes the spec, build system, PAM file, and
test/image-prepareto work at all. Afterbots/vm-run -s cockpit.socket opensuse-tumbleweedyou should be able to get a cockpit UI on http://1270.0.2:2201 which isn't completely broken. In that PR, take all commits which add test skips and squash them together. -
There will likely be more failures in the PR from 2., but that's fine -- at that point let's decide if we mark them as skipImage and enable opensuse-tumbleweed for all PRs (to avoid regressing it), or keep it as a manual test for the time being.
-
Discuss/fix some general mechanics/approach/helpers for the /usr write problem, in particular
sed_file()that checks if the file exists in /usr/etc instead and copies it first. (that'd be my recommendation, it mirrors what admins would do). -
See what's left, and split out the complicated bits into their own PR.
So let's keep this as a draft, and split out PRs step by step until this becomes empty or small enough to be reviewable and fixable.
Thanks!
tools/cockpit.spec
Outdated
| export NO_QUNIT=1 | ||
| %pytest | ||
| %endif | ||
|
|
||
| %install | ||
| %make_install | ||
| make install-tests DESTDIR=%{buildroot} | ||
| %if 0%{?suse_version} > 1500 | ||
| mkdir -p $RPM_BUILD_ROOT%{_pam_vendordir} | ||
| install -p -m 644 tools/cockpit.pam $RPM_BUILD_ROOT%{_pam_vendordir}/cockpit |
There was a problem hiding this comment.
OOI, Is that /usr/lib/pam.d/ ? Indeed I've wanted that for Fedora/RHEL as well, we just need a transition to remove an unmodified file in /etc.
| ls --hide={default,kubernetes,opensuse,registry,sle-micro,suse} | xargs rm -rv | ||
| popd | ||
| # need this in SUSE as post build checks dislike stale symlinks | ||
| install -m 644 -D /dev/null %{buildroot}/run/cockpit/motd |
| rm -r %{buildroot}%{_datadir}/cockpit/sosreport | ||
| rm -f %{buildroot}/%{_prefix}/share/metainfo/org.cockpit_project.cockpit_sosreport.metainfo.xml | ||
| rm -f %{buildroot}%{_datadir}/icons/hicolor/64x64/apps/cockpit-sosreport.png | ||
| %else |
There was a problem hiding this comment.
This "else" is weird. It combines two completely unrelated things, fixing symlinks and disabling the debug package. Please drop this else and keep the existing if ! 0%{?suse_version}, for the debug block.
| %if 0%{?suse_version} | ||
| Provides: group(cockpit-ws) | ||
| Provides: group(cockpit-wsinstance) | ||
| Provides: user(cockpit-ws) | ||
| Provides: user(cockpit-wsinstance) | ||
| %endif |
There was a problem hiding this comment.
This is all obsolete, current main doesn't use any static or even sysusers.d users any more. It's just DynamicUser= now.
| # setroubleshoot is available on SLE Micro starting with 5.5 | ||
| %if !0%{?is_smo} || ( 0%{?is_smo} && 0%{?sle_version} >= 150500 ) | ||
| Requires: setroubleshoot-server >= 3.3.3 | ||
| %endif |
There was a problem hiding this comment.
It IMHO doesn't make sense to build cockpit-selinux without setroubleshoot. Perhaps just skip the entire %package instead?
| if [ -f /etc/nsswitch.conf ]; then | ||
| sed -i '/^sudoers:/ s/files sss/sss files/' /etc/nsswitch.conf | ||
| else | ||
| sed -i '/^sudoers:/ s/files sss/sss files/' /usr/etc/nsswitch.conf |
There was a problem hiding this comment.
You know the pattern by now 😁
| if self.backend == "zypper": | ||
| cmd = """ |
There was a problem hiding this comment.
AH, here it is. About fifty commits ago I complained about an incomplete packagelib.py commit -- that "test: Add in handling for package cases using zypper backend" is the one with which it should be squashed 😁
| @@ -94,7 +94,9 @@ class TestSession(testlib.MachineCase): | |||
|
|
|||
| # this is enabled by default in tools/cockpit.debian.pam, as well as | |||
| # Debian/Ubuntu's /etc/pam.d/sshd; but not in Fedora/RHEL | |||
| if "debian" not in m.image and "ubuntu" not in m.image: | |||
| if "suse" in m.image: | |||
| self.write_file("/usr/lib/pam.d/cockpit", "session required pam_env.so user_readenv=1\n", append=True) | |||
| @@ -1284,6 +1284,7 @@ class TestMetricsPackages(packagelib.PackageCase): | |||
|
|
|||
| redis_service = redisService(m.image) | |||
| redis_package = "valkey" if redis_service == "valkey" else "redis" | |||
| redis_service = f"{redis_service}.target" if "suse" in m.image else f"{redis_service}.service" | |||
There was a problem hiding this comment.
Please squash this with the three or four other commits that fixed redis/valkey for suse.
test/verify/check-connection
Outdated
| m.execute("useradd -m user") | ||
| m.execute("groupadd user") | ||
| if "user" not in m.execute("cat /etc/group"): | ||
| m.execute("groupadd user") | ||
| m.execute("usermod -a -G user user") |
There was a problem hiding this comment.
Ah -- if suse has an "user" group by default, then let's just change the test to use a different name -- "john" or "jane" or "homer" or "marge" or anything else really..
|
Hi @martinpitt Thank you for taking the time to review the huge pile of commits. It's close to the end of the year and both me and @SludgeGirl are a bit busy trying to get urgent things done before we are "forced" to take our unspent PTO. Meaning that, unfortunately, we'll have to move continuing working on this to start of next year. Just letting you know in case you're wondering why there are no new PRs coming to your way. Thank you for always being so patient! |
|
@Nykseli Thanks for the heads-up! No worries, enjoy your holidays and talk to you next year! |
Hey!
First up, sorry for the size of this, I've tried to keep the commits as atomic as possible and provide as much information as I can, let me know if you need it improved or re-organized in any way. Secondly, sorry for the wait!
This looks to largely fix most of the issues with Tumbleweed experiences when running the tests for it. There will be another PR opened against bots shortly, I'll link both together once it's opened