Skip to content

Conversation

@HuijingHei
Copy link
Member

This is to verify (https://bugzilla.redhat.com/show_bug.cgi?id=1980679) for RHCOS

Notes:

  1. Ignition file is on the github in hhei-dev branch, if merged, should change the URL
  2. Ignore rhcos-afterburn-checkin.service dependencies check (azure only) because of run kola test failed, refer to Run kola test on Azure failed #2445

"config": {
"merge": [
{
"source": "https://raw.githubusercontent.com/HuijingHei/coreos-assembler/hhei-dev/mantle/kola/tests/rhcos/kargsfile.ign",
Copy link
Member

@miabbott miabbott Sep 21, 2021

Choose a reason for hiding this comment

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

We probably want to move this config under the coreos-assembler repo.

m := c.Machines()[0]

// Verify kernel arguments and /etc/testfile
c.RunCmdSync(m, "grep -q foobar /proc/cmdline")
Copy link
Member

Choose a reason for hiding this comment

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

This might be a good use for AssertCmdOutputContains; see #2418

@dustymabe
Copy link
Member

question for broader team:

Do we have an opinion on where we prefer to put tests in the future? This seems like something that would be good to put in our ext tests in the configs repo instead because it's very simple (all bash) etc.. We could also store the remote ignition config alongside the test in the same directory.

@saqibali-2k
Copy link
Contributor

saqibali-2k commented Sep 21, 2021

Do we have an opinion on where we prefer to put tests in the future? This seems like something that would be good to put in our ext tests in the configs repo instead because it's very simple (all bash) etc.. We could also store the remote ignition config alongside the test in the same directory.

I agree that putting this test in as an external test might be better since this test is simpler (maybe in openshift/os since it is an RHCOS test?). The ignition config used in the test can be added to a file named config.ign and the remote ignition config can be kept as is (and added to the same directory). The platforms and tags can be specified via kola.json.

Addressing the broader question: I recall reading in the docs that if a test could be run as an external test (for ex. it didnt need multiple nodes), then that approach is preferred. Not sure if that philosophy still stands.

@miabbott
Copy link
Member

question for broader team:

Do we have an opinion on where we prefer to put tests in the future? This seems like something that would be good to put in our ext tests in the configs repo instead because it's very simple (all bash) etc.. We could also store the remote ignition config alongside the test in the same directory.

Yeah, this makes sense. I had forgotten (or just didn't realize) that the ext tests supported custom Ignition configs and selectable arches/platforms.

In defense of @HuijingHei, I told her to create the test here.

Addressing the broader question: I recall reading in the docs that if a test could be run as an external test (for ex. it didnt need multiple nodes), then that approach is preferred. Not sure if that philosophy still stands.

I think it would be useful to ensure we have the philosophy documented somewhere; it seems like it makes sense.

@jlebon
Copy link
Member

jlebon commented Sep 21, 2021

Yup, this is documented in https://coreos.github.io/coreos-assembler/kola/adding-tests/#relationship-to-external-tests, but we can probably emphasize it more.

For background, the reason for this is that tests are more closely bound to the manifests describing the artifacts they're testing rather than the test harness. Then they're e.g. promoted together, and can be added/modified atomically with the feature implementation being tested. This isn't very different from, say, rpm-ostree tests living in the rpm-ostree repo for example.

@dustymabe
Copy link
Member

@HuijingHei - would you be willing to convert this to an exttest and open a PR against https://github.com/coreos/fedora-coreos-config.git ? If you have any questions please reach out to me and I'll be glad to help!

@HuijingHei
Copy link
Member Author

question for broader team:
Do we have an opinion on where we prefer to put tests in the future? This seems like something that would be good to put in our ext tests in the configs repo instead because it's very simple (all bash) etc.. We could also store the remote ignition config alongside the test in the same directory.

Yeah, this makes sense. I had forgotten (or just didn't realize) that the ext tests supported custom Ignition configs and selectable arches/platforms.

In defense of @HuijingHei, I told her to create the test here.

Addressing the broader question: I recall reading in the docs that if a test could be run as an external test (for ex. it didnt need multiple nodes), then that approach is preferred. Not sure if that philosophy still stands.

I think it would be useful to ensure we have the philosophy documented somewhere; it seems like it makes sense.

Thanks @miabbott ! Actually sometimes it is confused about where to add the tests. But I am still appreciated for your
original suggestion.

@HuijingHei
Copy link
Member Author

HuijingHei commented Sep 22, 2021

Thanks all for the review!

@HuijingHei - would you be willing to convert this to an exttest and open a PR against https://github.com/coreos/fedora-coreos-config.git ? If you have any questions please reach out to me and I'll be glad to help!

Hi, @dustymabe, if ignore rhcos-afterburn-checkin.service dependencies check (azure only), this is just general case for cloud. I think it is OK to me to add test to https://github.com/coreos/fedora-coreos-config.git

@HuijingHei
Copy link
Member Author

HuijingHei commented Sep 22, 2021

Move PR to coreos/fedora-coreos-config#1227

@HuijingHei HuijingHei closed this Sep 22, 2021
@HuijingHei HuijingHei deleted the hhei-dev branch May 10, 2022 03:37
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.

5 participants