Skip to content

cockpit tests: fix testing on RHEL >= 8.5#2840

Merged
jirihnidek merged 4 commits intomainfrom
ptoscano/cockpit-ci-fixes
Oct 19, 2021
Merged

cockpit tests: fix testing on RHEL >= 8.5#2840
jirihnidek merged 4 commits intomainfrom
ptoscano/cockpit-ci-fixes

Conversation

@ptoscano
Copy link
Copy Markdown
Contributor

@ptoscano ptoscano commented Oct 15, 2021

These are few commits to improve the cockpit tests, and make it work on RHEL >= 8.5:

  • properly setup SSL in the mock-insights tool, with the right key according to the cockpit version
  • be less aggressive in waiting for candlepin to startup
  • properly install subscription-manager in the system, so it overrides the system version
  • disable on RHEL 9 two tests that use insights-client, as it seems to be currently broken

Inspired by #2778, credits to both @marusak and @martinpitt (of the cockpit team) for the hints on this.

Make sure that the mock-insights tool properly uses also the self-signed
key created by cockpit, not only the certificate. The only gotcha is due
to a behaviour change in cockpit: v242 switched from a single file
including both key and certificate to separate files for them.

Credits to both Matej Marusak and Martin Pitt (of the cockpit team) for
the hints on this.
tomcat is not exactly fast at starting, and I have always seen at least
5/6 lines of:
  Waiting for https://10.111.112.100:8443/candlepin
during the cockpit tests, either triggered for GitHub PRs or in own runs
on baremetal. Hence, directly wait 5 seconds as initial wait time to
ease the traffic/load.
@cnsnyder cnsnyder requested review from a team and wottop and removed request for a team October 15, 2021 12:50
@martinpitt
Copy link
Copy Markdown
Contributor

Thanks for fixing the certificate handling! LGTM. 👍

@ptoscano
Copy link
Copy Markdown
Contributor Author

Finally! rhel-8-5 and rhel-8-6 now work fine, together with rhel-8-4 :)
Let's see if I can make rhel-9-0 usable too...

It seems that newer versions of Python or setuptools create a different
layout in the Python site-package depending whether the installation is
direct to the specified prefix, or there is temporary location (a local
destdir). Because of this, force "--root /" when installing directly to
/usr for testing: this seems to use the same layout of the system
installation (via distribution packages), and thus the existing files
will be properly overwritten.

Another change is the location of the "subscription-manager" executable:
setuptools installs it in $prefix/bin, while our Makefile moves it to
$prefix/sbin; hence, move it to the desired place, so it properly
overwrites the package version.
@ptoscano
Copy link
Copy Markdown
Contributor Author

Almost there! Now the testing of rhel-9-0 works fine, as the sub-man installation for it properly overrides the system version.

The two failing tests are testInsights and testSubAndInAndFail: both of them use insights-client, which seems to be broken:

Traceback (most recent call last):
  File "/usr/lib/python3.9/site-packages/insights_client/run.py", line 4, in <module>
    from insights import package_info
ModuleNotFoundError: No module named 'insights'

The last change for this: skip these two tests when testing on RHEL 9.

It seems that currently insights-client on RHEL 9 is broken:

  Traceback (most recent call last):
    File "/usr/lib/python3.9/site-packages/insights_client/run.py", line 4, in <module>
      from insights import package_info
  ModuleNotFoundError: No module named 'insights'

Hence, disable the two tests using it when running the test suite on a
rhel-9-X image.

This is mostly a temporary solution to get the cockpit test suite
running on RHEL 9; this commit can be reverted once insights-client in
RHEL 9 is functional.
@ptoscano ptoscano changed the title cockpit tests: fix mock-insights keys, wait better for candlepin cockpit tests: fix testing on RHEL >= 8.5 Oct 15, 2021
Comment on lines +317 to +318
if m.image.startswith('rhel-9-'):
self.skipTest("insights-client is currently broken in RHEL 9")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can do this, but one tends to forget about these.

but for these kinds of OS regressions we have the naughty patterns mechanism in bots: (1) file a bugzilla against the affected OS, so that it's reported and gets fixed; (2) file a knownissue bots bug with refering to the downstream bug, and (3) add a naughty pattern for the test failure to bots.

Then the failure will be converted to a "# skip known issue #1234" automatically, the test is green. The big advantage is that our bots regularly check which naughty patterns are still being used, and files PRs such as cockpit-project/bots#2530 to automatically clean them up.

Advantages: You don't need to change your tests once the regression gets fixed, tracking happens automatically.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well TBH my hope is to get this fixed "relatively soon"™, and this is more like a (hopefully) short-term band-aid to let at least the rest of the tests run.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, that's our hope for all these knownissues 😁

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got feedback that registering a RHEL 9 system to RHSM and connecting it to Insights works fine. Hence, I think that the problem could likely be in our mock-insights mini-server that does not fully emulate what the newer insights-client does; in this case, I will not create a BZ for it, as it is most likely not an issue impacting the cockpit plugin.

Copy link
Copy Markdown
Contributor

@jirihnidek jirihnidek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants