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

exttests: New container image with upstream tests #1745

Merged
merged 1 commit into from
Oct 9, 2020

Conversation

cgwalters
Copy link
Member

This is aiming to finally, finally close the loop around
running upstream tests as part of our CI.

We landed support for "exttests" in here for a while and
since then we are successfully running upstream test suites
using kola on upstream pull requests.

But we also want the inverse: run upstream tests on builds outside of PRs to those repos.
For example,
I really want to run ostree's "transactionality" test on FCOS builds so that if a kernel change breaks it, we know that.

In practice, it's likely that down the line we
will need to version at least some of our tests along
with the OS content - by building them as RPMs or
sticking them in containers that are versioned with the OS
or something.

But for now, building fixed upstream releases will
give us a lot of reproducibility/control, at the
cost of needing to periodically bump them.

@cgwalters
Copy link
Member Author

Moving this over here from https://github.com/cgwalters/exttests

@cgwalters
Copy link
Member Author

The main test I want to run is ostreedev/ostree#2127
But this also would tend to obsolete some of the other ostree/rpm-ostree tests that live in cosa today.

repos[coreos/rpm-ostree]=v2020.5
for repo in "${!repos[@]}"; do
bn=$(basename ${repo})
git clone --depth=1 https://github.com/${repo} ${bn}
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we check out the specific tag here?

### Alternative: Make RPMs of tests and install those in coreos-assembler

We have an `ostree-tests` RPM...but I'm not sure I want to deal with
packaging for it.
Copy link
Member

Choose a reason for hiding this comment

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

Will throw out my proposal too here.

I think we should try to address the binding problem with this the same way we did with tests/kola in f-c-c. IOW, the test version should match the app version. And since lockfiles live in f-c-c, this is what dictates what tests to use.

WDYT about something like this:

  • have a YAML file in f-c-c containing a list of upstream repos which support kola tests
  • have e.g. a cosa buildextend-exttests which read this YAML file, and for each repo/app, have it check the NEVRA in the build, clone the repo at that tag, build the exttests, and bundle them in a tarball artifact that becomes part of meta.json
  • have pipelines use the buildroot container to run cosa buildextend-exttests; this artifact can then be used by any system which wants to run the tests against the FCOS build (obviously the QEMU kola run invocation, but also e.g. AWS/GCP/$cloud tests or whatever else wants to validate the build)

Copy link
Member

Choose a reason for hiding this comment

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

To clarify, this is fine to start as is too; had one comment about the code. From your commit message, I think we're aligned high-level on how we should change the approach here in the future.

Copy link
Member Author

Choose a reason for hiding this comment

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

A whole lot going on there. Trying to wrap my head around it. ⏳ time passes ⏳

OK I have done so and I mostly like it.

  • Note: It's elevating the buildroot container, which I think makes sense...we definitely need to "rationalize" the quay.io/coreos-assembler vs api.ci thing still though.

I think a big negative of this is that building the tests becomes a critical path thing done synchronously, whereas I would really like to avoid hard blocking on test build failures. For example, today ostree's exttests just use crates.io to build, but as is this proposal would mean we couldn't ship FCOS if crates.io is down.

Ultimately I think as a general rule we should at least support running old tests against new code. So the FCOS pipeline could use the previous tests to start (perhaps a subset of them) and we can optionally choose whether or not to block on the new tests being built. Also an important optimization here I think is avoiding rebuilding tests if they didn't change - we'll need to debate how that works.

Copy link
Member Author

Choose a reason for hiding this comment

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

To clarify, this is fine to start as is too;

Fixed the tag fetching. I think I'd like to propose trying this (this="exttests container") out and see how things go, and we can always revisit for a second phase with something like "exttests artifact" (your proposal).

Copy link
Member Author

Choose a reason for hiding this comment

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

Though a definite downside of this is it requires scheduling a new pod, which will force us to either use the exttests container for the build too, or go to multi-phase (which we know we need anyways though).

@cgwalters cgwalters changed the title exttests: New container image with upstream tests WIP: exttests: New container image with upstream tests Oct 1, 2020
This is aiming to finally, finally close the loop around
running upstream tests as part of our CI.

We landed support for "exttests" in here for a while and
since then we are successfully running upstream test suites
using kola on upstream pull requests.

But we also want the inverse: run upstream tests on builds outside of PRs to those repos.
For example,
I really want to run ostree's "transactionality" test on FCOS builds so that if a kernel change breaks it, we know that.

In practice, it's likely that down the line we
will need to version at least some of our tests along
with the OS content - by building them as RPMs or
sticking them in containers that are versioned with the OS
or something.

But for now, building fixed upstream releases will
give us a lot of reproducibility/control, at the
cost of needing to periodically bump them.
@cgwalters cgwalters changed the title WIP: exttests: New container image with upstream tests exttests: New container image with upstream tests Oct 7, 2020
@cgwalters
Copy link
Member Author

Lifting WIP on this - I think this design is a low-impact way to make some progress here, though clearly we will need to grow a lot more sophistication in the future.

@jlebon
Copy link
Member

jlebon commented Oct 7, 2020

Yup, SGTM!
/lgtm

@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cgwalters, jlebon

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-robot openshift-merge-robot merged commit 4a168f5 into coreos:master Oct 9, 2020
@cgwalters
Copy link
Member Author

Followup openshift/release#12632

@cgwalters
Copy link
Member Author

Followup coreos/fedora-coreos-config#677

@cgwalters
Copy link
Member Author

OK a few ideas here.

First I'd like to get better support in our existing CI for running optional tests. If we can add a magic Prow-like comment
/test kola-ext
that would also run tests using this optionally, that could help a lot.
Maybe...hmm, if we just stuck it in the commit messages, we could add some wrapper glue to coreos-ci-lib that parsed it and that would trigger conditional code in the main fcosKola perhaps?

And related to that, how hard would it be to add a periodic "informing" job to the FCOS pipeline that triggered off new builds but didn't block them?

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

Successfully merging this pull request may close these issues.

4 participants