Skip to content
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

[testing] Add testing via Cirrus CI #2305

Closed
wants to merge 4 commits into from

Conversation

TurboTurtle
Copy link
Member

@TurboTurtle TurboTurtle commented Nov 10, 2020

This patchset enables automated PR testing via Cirrus on GCP.

The first patch adds the necessary Cirrus configuration, and the second overhauls the simple.sh test suite.

Finally, drop .travis.yml to stop testing on Travis entirely.

Signed-off-by: Jake Hunsaker [email protected]


Please place an 'X' inside each '[]' to confirm you adhere to our Contributor Guidelines

  • Is the commit message split over multiple lines and hard-wrapped at 72 characters?
  • Is the subject and message clear and concise?
  • Does the subject start with [plugin_name] if submitting a plugin patch or a [section_name] if part of the core sosreport code?
  • Does the commit contain a Signed-off-by: First Lastname [email protected]?
  • If this commit closes an existing issue, is the line Closes: #ISSUENUMBER included in an independent line?
  • If this commit resolves an existing pull request, is the line Resolves: #PRNUMBER included in an independent line?

@TurboTurtle TurboTurtle force-pushed the cirrus-ci-tests branch 5 times, most recently from b607834 to 58e284d Compare November 11, 2020 21:47
@TurboTurtle
Copy link
Member Author

Ok, definitely made some headway here.

We have functional Fedora testing from this. It'll just be a maintenance task on me to build and push new images with each subsequent Fedora release. My initial thought is to test against the current latest and latest-1 release. I am working out if we can do RHEL testing as well here, if not CentOS definitely looks like an option.

I had initially thought that we could use the pre-existing Ubuntu images on GCE as well, but there appears to be an access issue - I'm digging into that now.

I've brought the different python version checks forward as well, using containers rather than full VMs. This should serve as a decent smoke test to safe guard against changes in python modules. One thing I'd like to explore later on is running sos inside a container that grabs data from the host - like the support-tools image does for CoreOS hosts.

There is one thing we'd sacrifice by moving to Cirrus from Travis though, and that is non x86 test runs. GCE (and pretty much every cloud platform I know of) only offer x86_64 as a VM arch. We've had reliability issues in the past with Travis generally, and non-x86 arches specifically, so I'm not sure if we honestly care all that much about that though.

Thoughts?

@pmoravec @BryanQuigley @slashdd @bmr-cymru

@BryanQuigley
Copy link
Contributor

./tests/simple.sh: line 61: tar: command not found
I guess the test script should be checking for tar existing. Never thought of that..

And Travis isn't posting again? If Cirrus works more reliably, I'm all for it.

I think it's we've caught arch issues once, but testing on both Fedora/RH/CentOS and Ubuntu is worth much more. It is a feature request they are tracking and if Cirrus get's more "real" VMs then we could theoretically use qemu to do the test.

I also do prefer how we see the results so far and apparently we can annotate results even more if we wanted to.

If we can test all supported RH/Fedora and Ubuntu LTS, I think we can just go down to one 3.9 python forward looking test.

@TurboTurtle
Copy link
Member Author

./tests/simple.sh: line 61: tar: command not found
I guess the test script should be checking for tar existing. Never thought of that..

Oh wow...never would have thought of that either. I'll rebuild the GCP images to include tar. And here I was thinking it was a default install. :|

And Travis isn't posting again? If Cirrus works more reliably, I'm all for it.

I think it's we've caught arch issues once, but testing on both Fedora/RH/CentOS and Ubuntu is worth much more. It is a feature request they are tracking and if Cirrus get's more "real" VMs then we could theoretically use qemu to do the test.

Yeah - however that would require us to upgrade to a paid service from Cirrus. Right now, this is using the free-for-open-source Cirrus offering and leveraging GCP resources with some funding from RH. Not saying it's out of the question, but its a bridge I'll need to cross internally whenever that support becomes available.

The reason we aren't entirely on the free tier for Cirrus is that we need to bring our own images for RH-family instances. I don't see that changing if Cirrus expands arch support though ;-)

I also do prefer how we see the results so far and apparently we can annotate results even more if we wanted to.

Yeah, I've been playing with this for just a short while but I'm really liking it. I imagine we could get quite complex with the results as we build it out over time.

If we can test all supported RH/Fedora and Ubuntu LTS, I think we can just go down to one 3.9 python forward looking test.

Ack.

@TurboTurtle
Copy link
Member Author

I've spent the day working out the Ubuntu image issue, and then overhauling simple.sh to give us more granular details. Will have something up shortly.

In the meantime, is there a desire to run Ubuntu LTS and some later version, or is just LTS sufficient?

@BryanQuigley
Copy link
Contributor

IMHO testing Ubuntu 18.04 LTS and 20.04 LTS should be enough.

TurboTurtle added a commit to TurboTurtle/sos that referenced this pull request Nov 12, 2020
Adds functional CI testing via Cirrus in conjunction with GCP. Tests can
now be run on Fedora as well as Ubuntu hosts, and minor tasks can be run
in containers. This will allow us to easily expand our testing base
across more distributions provided those distributions can be run on GCP
instances.

As new releases of supported distributions are made available, the
maintainers will need to build and push updated Fedora (or more
generally, RH-family) images to the GCP project. Ubuntu has cloud images
on GCP already, so when new releases of Ubuntu are pushed we will simply
need to update .cirrus.yml to point to the new public images.

Resolves: sosreport#2305

Signed-off-by: Jake Hunsaker <[email protected]>
TurboTurtle added a commit to TurboTurtle/sos that referenced this pull request Nov 12, 2020
This patch overhauls tests/simple.sh to provide more granular details on
why a test run may have failed, beyond checking for a non-zero exit code
or if output was written to stderr.

This should also serve as another step towards more easily extendible
tests for our automated processes.

Resolves: sosreport#2305

Signed-off-by: Jake Hunsaker <[email protected]>
TurboTurtle added a commit to TurboTurtle/sos that referenced this pull request Nov 12, 2020
Adds functional CI testing via Cirrus in conjunction with GCP. Tests can
now be run on Fedora as well as Ubuntu hosts, and minor tasks can be run
in containers. This will allow us to easily expand our testing base
across more distributions provided those distributions can be run on GCP
instances.

As new releases of supported distributions are made available, the
maintainers will need to build and push updated Fedora (or more
generally, RH-family) images to the GCP project. Ubuntu has cloud images
on GCP already, so when new releases of Ubuntu are pushed we will simply
need to update .cirrus.yml to point to the new public images.

Resolves: sosreport#2305

Signed-off-by: Jake Hunsaker <[email protected]>
TurboTurtle added a commit to TurboTurtle/sos that referenced this pull request Nov 12, 2020
This patch overhauls tests/simple.sh to provide more granular details on
why a test run may have failed, beyond checking for a non-zero exit code
or if output was written to stderr.

This should also serve as another step towards more easily extendible
tests for our automated processes.

Resolves: sosreport#2305

Signed-off-by: Jake Hunsaker <[email protected]>
@TurboTurtle
Copy link
Member Author

IMHO testing Ubuntu 18.04 LTS and 20.04 LTS should be enough.

Oh, I didn't realize 18.04 was still supported. I'll get that added.

As for the current tests failing on Ubuntu, it seems the test suite is picking up --tmp-dir from the pre-existing /etc/sos/sos.conf. I think the easiest way forward is to hardcode a --tmp-dir override to conform with the rest of simple.sh (I didn't bother changing the extraction steps).

@BryanQuigley
Copy link
Contributor

Oh right, I guess the Ubuntu GCE image has sos already installed. I believe getting rid of the package sudo apt purge sosreport - should remove the config file as well.

@TurboTurtle
Copy link
Member Author

TurboTurtle commented Nov 12, 2020

Oh right, I guess the Ubuntu GCE image has sos already installed. I believe getting rid of the package sudo apt purge sosreport - should remove the config file as well.

I had thought about mucking with the config file at first, but then realized this would also potentially be running on contributor's systems. So we don't want to go changing people's actual sos installs on them when they try to commit, but at the same time we need simple.sh to be runnable locally.

I'm all ears for a better solution than setting --tmp-dir in the sos commands run by simple.sh, but for now for testing this PR that's what I'm going to do for the short-term.

TurboTurtle added a commit to TurboTurtle/sos that referenced this pull request Nov 12, 2020
Adds functional CI testing via Cirrus in conjunction with GCP. Tests can
now be run on Fedora as well as Ubuntu hosts, and minor tasks can be run
in containers. This will allow us to easily expand our testing base
across more distributions provided those distributions can be run on GCP
instances.

As new releases of supported distributions are made available, the
maintainers will need to build and push updated Fedora (or more
generally, RH-family) images to the GCP project. Ubuntu has cloud images
on GCP already, so when new releases of Ubuntu are pushed we will simply
need to update .cirrus.yml to point to the new public images.

Resolves: sosreport#2305

Signed-off-by: Jake Hunsaker <[email protected]>
TurboTurtle added a commit to TurboTurtle/sos that referenced this pull request Nov 12, 2020
This patch overhauls tests/simple.sh to provide more granular details on
why a test run may have failed, beyond checking for a non-zero exit code
or if output was written to stderr.

This should also serve as another step towards more easily extendible
tests for our automated processes.

Resolves: sosreport#2305

Signed-off-by: Jake Hunsaker <[email protected]>
@TurboTurtle
Copy link
Member Author

So with that change, all the jobs are currently passing. I still need to work out running on actual RHEL but I don't think that's a hard requirement for this first move to Cirrus. I'll keep plugging away at it though.

The other question is do we want to hold on to Travis for any amount of time with this, or just move straight to Cirrus and drop .travis.yml right away?

@BryanQuigley
Copy link
Contributor

+1 on the new structure to simple.sh. And wow, is it faster and seems to cover as much.

I did look into why Fedora takes longer and collects more - but it seems fine - just more things installed by default AFAICT. Good test diving into Cirrus and I like it.

My last thoughts before +1 and fully replacing Travis:

  • From a "native" test perspective I think it would be preferably to not install dependencies using pip3. Use apt and dnf directly? This seems especially important for older releases.
  • Add nosetests
  • Be OK with not testing python3-pexpect not installed case. (I am, or we could always add another small test just for it).

Awesome work!

@TurboTurtle
Copy link
Member Author

  • From a "native" test perspective I think it would be preferably to not install dependencies using pip3. Use apt and dnf directly? This seems especially important for older releases.

So the only module being install by pip that's for actual functionality is pexpect. We don't have any collect tests currently, and it'll likely be a while before we have something for it (I'm honestly not sure if we're even able to spin up instances with Cirrus in a way where they could reliably reference each other).

So for now, I'm onboard with dropping the pip install step. If/when we get collect tests we'll likely split them out into a different task anyways.

  • Add nosetests

I knew I was forgetting something obvious!

TurboTurtle added a commit to TurboTurtle/sos that referenced this pull request Nov 16, 2020
Adds functional CI testing via Cirrus in conjunction with GCP. Tests can
now be run on Fedora as well as Ubuntu hosts, and minor tasks can be run
in containers. This will allow us to easily expand our testing base
across more distributions provided those distributions can be run on GCP
instances.

As new releases of supported distributions are made available, the
maintainers will need to build and push updated Fedora (or more
generally, RH-family) images to the GCP project. Ubuntu has cloud images
on GCP already, so when new releases of Ubuntu are pushed we will simply
need to update .cirrus.yml to point to the new public images.

Resolves: sosreport#2305

Signed-off-by: Jake Hunsaker <[email protected]>
TurboTurtle added a commit to TurboTurtle/sos that referenced this pull request Nov 16, 2020
This patch overhauls tests/simple.sh to provide more granular details on
why a test run may have failed, beyond checking for a non-zero exit code
or if output was written to stderr.

This should also serve as another step towards more easily extendible
tests for our automated processes.

Resolves: sosreport#2305

Signed-off-by: Jake Hunsaker <[email protected]>
@TurboTurtle
Copy link
Member Author

I noticed the --mask test was a bit lacking. Building that out a bit more, then I can prep this to merge with a handful of others I have staged.

@TurboTurtle
Copy link
Member Author

...and what do you know, found a bug with mac obfuscation. I'll get a PR up for that, then update this with the expanded --mask tests once it's settled.

TurboTurtle added a commit to TurboTurtle/sos that referenced this pull request Nov 18, 2020
Adds functional CI testing via Cirrus in conjunction with GCP. Tests can
now be run on Fedora as well as Ubuntu hosts, and minor tasks can be run
in containers. This will allow us to easily expand our testing base
across more distributions provided those distributions can be run on GCP
instances.

As new releases of supported distributions are made available, the
maintainers will need to build and push updated Fedora (or more
generally, RH-family) images to the GCP project. Ubuntu has cloud images
on GCP already, so when new releases of Ubuntu are pushed we will simply
need to update .cirrus.yml to point to the new public images.

Closes: sosreport#1885
Resolves: sosreport#2305

Signed-off-by: Jake Hunsaker <[email protected]>
TurboTurtle added a commit to TurboTurtle/sos that referenced this pull request Nov 18, 2020
This patch overhauls tests/simple.sh to provide more granular details on
why a test run may have failed, beyond checking for a non-zero exit code
or if output was written to stderr.

This should also serve as another step towards more easily extendible
tests for our automated processes.

Resolves: sosreport#2305

Signed-off-by: Jake Hunsaker <[email protected]>
TurboTurtle added a commit to TurboTurtle/sos that referenced this pull request Nov 18, 2020
We are dropping the use of Travis for our CI testing, and moving to
Cirrus-CI.

Resolves: sosreport#2305
Closes: sosreport#2246
Closes: sosreport#2048

Signed-off-by: Jake Hunsaker <[email protected]>
@TurboTurtle
Copy link
Member Author

Hmmm. I had thought #2311 had covered the known problem areas, but I guess there's more.

I think this deserves another round at looking at the regex used for IPv4 MAC addrs that avoids an IPV6 substring rather than just chasing characters here.

That being said, I would if replace (\.|,|!)? with the simpler (.)? would work?

@BryanQuigley
Copy link
Contributor

I have not looked at the code in-depth - but is it possible to ensure the IPv6 masking always happens before the MAC one?

@TurboTurtle
Copy link
Member Author

That was my original approach, however the issue was/is that we end up breaking everything out into its own loop which becomes fairly inefficient and can massively slow down the obfuscation times. I think I've got it worked out now, and the tests are passing locally with me injecting a few purpose-built lines. A bit of cleanup left, then I'll re-push.

TurboTurtle added a commit to TurboTurtle/sos that referenced this pull request Nov 18, 2020
This patch overhauls tests/simple.sh to provide more granular details on
why a test run may have failed, beyond checking for a non-zero exit code
or if output was written to stderr.

This should also serve as another step towards more easily extendible
tests for our automated processes.

Resolves: sosreport#2305

Signed-off-by: Jake Hunsaker <[email protected]>
TurboTurtle added a commit to TurboTurtle/sos that referenced this pull request Nov 18, 2020
We are dropping the use of Travis for our CI testing, and moving to
Cirrus-CI.

Resolves: sosreport#2305
Closes: sosreport#2246
Closes: sosreport#2048

Signed-off-by: Jake Hunsaker <[email protected]>
TurboTurtle added a commit to TurboTurtle/sos that referenced this pull request Nov 18, 2020
The previously merged sosreport#2311 was thought to be sufficient for remaining
cases where MAC addrs were followed by punctuation characters, however
it was found to be too restrictive in that it would leave quoted and
other forms of MAC addrs unobfuscated when those addrs were immediately
followed by other characters.

Simply the regex match to catch all of these, and further update the
`parse_lines()` override to properly trim down the match to just the
address substring.

Related: sosreport#2311
Related: sosreport#2305

Signed-off-by: Jake Hunsaker <[email protected]>
TurboTurtle added a commit to TurboTurtle/sos that referenced this pull request Nov 18, 2020
This patch overhauls tests/simple.sh to provide more granular details on
why a test run may have failed, beyond checking for a non-zero exit code
or if output was written to stderr.

This should also serve as another step towards more easily extendible
tests for our automated processes.

Resolves: sosreport#2305

Signed-off-by: Jake Hunsaker <[email protected]>
TurboTurtle added a commit to TurboTurtle/sos that referenced this pull request Nov 18, 2020
We are dropping the use of Travis for our CI testing, and moving to
Cirrus-CI.

Resolves: sosreport#2305
Closes: sosreport#2246
Closes: sosreport#2048

Signed-off-by: Jake Hunsaker <[email protected]>
TurboTurtle added a commit to TurboTurtle/sos that referenced this pull request Nov 18, 2020
The previously merged sosreport#2311 was thought to be sufficient for remaining
cases where MAC addrs were followed by punctuation characters, however
it was found to be too restrictive in that it would leave quoted and
other forms of MAC addrs unobfuscated when those addrs were immediately
followed by other characters.

Simply the regex match to catch all of these, and further update the
`parse_lines()` override to properly trim down the match to just the
address substring.

Related: sosreport#2311
Related: sosreport#2305

Signed-off-by: Jake Hunsaker <[email protected]>
@TurboTurtle
Copy link
Member Author

\o/

There we go!

That was a bit of a pain, but the mask test now has pretty decent coverage. With all that, I think this is finally in the right state for merging.

@BryanQuigley @pmoravec @bmr-cymru

@TurboTurtle
Copy link
Member Author

Noticed a typo or two in the commit messages. Fixing, then will merge.

Adds functional CI testing via Cirrus in conjunction with GCE. Tests can
now be run on Fedora as well as Ubuntu hosts, and minor tasks can be run
in containers. This will allow us to easily expand our testing base
across more distributions provided those distributions can be run on GCP
instances.

As new releases of supported distributions are made available, the
maintainers will need to build and push updated Fedora (or more
generally, RH-family) images to the GCP project. Ubuntu has cloud images
on GCE already, so when new releases of Ubuntu are pushed we will simply
need to update .cirrus.yml to point to the new public images.

Note that at the moment testing on RHEL is not enabled, though it should
follow the same framework as the Fedora tests and should hopefully be
coming before too long.

Closes: sosreport#1885
Resolves: sosreport#2305

Signed-off-by: Jake Hunsaker <[email protected]>
This patch overhauls tests/simple.sh to provide more granular details on
why a test run may have failed, beyond checking for a non-zero exit code
or if output was written to stderr.

This should also serve as another step towards more easily extendible
tests for our automated processes.

Resolves: sosreport#2305

Signed-off-by: Jake Hunsaker <[email protected]>
We are dropping the use of Travis for our CI testing, and moving to
Cirrus-CI.

Resolves: sosreport#2305
Closes: sosreport#2246
Closes: sosreport#2048

Signed-off-by: Jake Hunsaker <[email protected]>
The previously merged sosreport#2311 was thought to be sufficient for remaining
cases where MAC addrs were followed by punctuation characters, however
it was found to be too restrictive in that it would leave quoted and
other forms of MAC addrs unobfuscated when those addrs were immediately
followed by other characters.

Simply the regex match to catch all of these, and further update the
`parse_lines()` override to properly trim down the match to just the
address substring.

Related: sosreport#2311
Related: sosreport#2305

Signed-off-by: Jake Hunsaker <[email protected]>
TurboTurtle added a commit that referenced this pull request Nov 19, 2020
This patch overhauls tests/simple.sh to provide more granular details on
why a test run may have failed, beyond checking for a non-zero exit code
or if output was written to stderr.

This should also serve as another step towards more easily extendible
tests for our automated processes.

Resolves: #2305

Signed-off-by: Jake Hunsaker <[email protected]>
TurboTurtle added a commit that referenced this pull request Nov 19, 2020
We are dropping the use of Travis for our CI testing, and moving to
Cirrus-CI.

Resolves: #2305
Closes: #2246
Closes: #2048

Signed-off-by: Jake Hunsaker <[email protected]>
TurboTurtle added a commit that referenced this pull request Nov 19, 2020
The previously merged #2311 was thought to be sufficient for remaining
cases where MAC addrs were followed by punctuation characters, however
it was found to be too restrictive in that it would leave quoted and
other forms of MAC addrs unobfuscated when those addrs were immediately
followed by other characters.

Simply the regex match to catch all of these, and further update the
`parse_lines()` override to properly trim down the match to just the
address substring.

Related: #2311
Related: #2305

Signed-off-by: Jake Hunsaker <[email protected]>
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