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

tests cleanup/improvements #543

Closed
cgwalters opened this issue May 16, 2024 · 12 comments · Fixed by #625
Closed

tests cleanup/improvements #543

cgwalters opened this issue May 16, 2024 · 12 comments · Fixed by #625
Assignees
Labels
enhancement New feature or request triaged This looks like a valid issue
Milestone

Comments

@cgwalters
Copy link
Collaborator

Our testing story is of course scattered. It's time to improve that.
Right now we have important tests being run via github actions bridging to testing farm, which itself is mainly an executor for tmt.

Then there's some github actions usage, plus some tests maintained in the rust sources.

(We also have rust unit tests, but not much can be unit tested in an interesting way here)

The most important thing here is:

Create a streamlined workflow for "build bootc and run key tests locally"

And we have a few avenues to pursue for this. I'd like to make this viable with tmt. We have the hack/Containerfile which builds a bootc-based container image. I'd like to streamline a flow where tmt can do something like run podman-bootc automatically and make a disk image, and then run tests against it with the "classic" model of puppeting over ssh.

@cgwalters cgwalters added enhancement New feature or request triaged This looks like a valid issue labels May 16, 2024
@cgwalters cgwalters self-assigned this May 16, 2024
@mvo5
Copy link
Contributor

mvo5 commented May 17, 2024

It's probably self-evident but I will throw it out anyway. I think it would be great too if the testing can be equally easy run locally as it is in the GH runners/testingfarm so that the barrier for new contributors is low. So being able to e.g. run in qemu too, ideally via a single command (once the right test dependencies are installed).

@cgwalters
Copy link
Collaborator Author

@henrywang says that packit is the main flow here that builds RPMs and has code to inject them into the test environment. But surely other projects have a flow for this?

We have the hack/Containerfile which builds a bootc-based container image. I'd like to streamline a flow where tmt can do something like run podman-bootc automatically and make a disk image, and then run tests against it with the "classic" model of puppeting over ssh.

Related to this, today the existing tmt code basically implements its own provisioning; it can talk to AWS or libvirt. But tmt's provisioner can do that too! As can podman-bootc.

So in theory I could run tmt --how local but it feels quite expensive; among other things it'll rebuild an RPM from scratch each time, also redownload a c9s qcow2. Whereas with tmt at least the qcow2 is cached.

The containerfile we use for development has incremental builds set up which is really useful. I wouldn't be opposed to giving that up in the short term to gain better tmt integration though.

Another related thing here is I'd like to try to move some of the implementation of testing to one where the guest is driving it, not being driven by the host. This is how we do things in coreos-assembler/kola - the host just polls a systemd unit. Notably, there's good support for having the test do reboots because we implement the debian autopkgtest https://manpages.debian.org/bookworm/autopkgtest/autopkgtest.1.en.html API to allow the testbed to reboot. This is super useful for bootc.

@henrywang
Copy link
Contributor

For example, tmt --root . -c arch=x86_64 run --all --verbose -e TEST_OS=fedora-40 -e ARCH=x86_64 -e QUAY_USERNAME=abc -e QUAY_PASSWORD="foobar" -e QUAY_SECRET="quayiouserpassbase64" provision --how local plans --name install-upgrade/to-disk command will run all test in local machine, no addition vm deployment and provision needed.

In this case building rpm, bootc install-to-disk, runing disk image with virt-install are all run in local machine. All test required package will be install into local machine by tmt plan.

@henrywang
Copy link
Contributor

henrywang commented May 31, 2024

Packit can be one of solutions for bootc testing. There're some benefits we can get from Packit solution:

  1. It does not need github runner, that can save github runner
  2. It can build bootc RPM package with copr build and only a spec file need
  3. No Testing Farm API Token configured in Packit
  4. Some of bootc tests can be moved to TMT to avoid github runner.

Downsides:

  1. Packit does not support secret. I think we can drop bootc install-to-existing-root test on AWS instance, but use libvirt vm instead. That avoid AWS secrets configured. For quay.io secrets required for pushing, localhost can be used for upgrade test, local folder (/mnt for example) can be used for switch test.
  2. No RHEL RPM build and no RHEL environment.

@cgwalters
Copy link
Collaborator Author

For example, tmt --root . -c arch=x86_64 run --all --verbose -e TEST_OS=fedora-40 -e ARCH=x86_64 -e QUAY_USERNAME=abc -e QUAY_PASSWORD="foobar" -e QUAY_SECRET="quayiouserpassbase64" provision --how local plans --name install-upgrade/to-disk command will run all test in local machine, no addition vm deployment and provision needed.

The QUAY_USERNAME here strongly shows that's not local; we're relying on an external infrastructure. I think we actually do need to fix that at some point even in general because isn't it racy today to have two concurrent test runs, both pushing staging images to quay?

This is a super complex topic of course...how to handle registries in a generic way especially that works locally is messy. One really nice thing about Prow (as used by Kube) is that each CI job runs by default in its own Kubernetes namespace and gets the free ability to push to the internal registry in a nicely scoped/safe way. We don't have that by default with GHA or Testing Farm.

We should probably make the registry usage in the tmt tests parameterized, so one can use ghcr.io or whatever too, and probably default to auto-synthesizing tags. Also btw when pushing to quay.io one can use tag expiration to get reliable GC of images.

Anyways, it's quite important of course to have coverage with "real" registry pushes, but I think we can do a lot of testing in a fully hermetic/local way with a VMs that don't even have networking, or at least not Internet routing (we do this in coreos-assembler/kola; see e.g. https://github.com/coreos/coreos-assembler/blob/main/docs/kola/external-tests.md#kolajson which includes the needs-internet tag which was taken from Debian autopkgtest. tmt definitely needs this too.

In the short term...man, I really want to reuse TMT (or more generally, something someone else has written and maintained) but...argh.

I'm still looking at this problem domain; in the short term I did #576 to just clean up that whole framework, but it doesn't cover the "local hermetic VM testing" that I'm getting at here.

@henrywang
Copy link
Contributor

henrywang commented Jun 1, 2024

I'm still looking at this problem domain; in the short term I did #576 to just clean up that whole framework, but it doesn't cover the "local hermetic VM testing" that I'm getting at here.

I'm working on test update to add localhost scenario (no push to registry required) with an argv controlled.

@vrothberg vrothberg added this to the 1.0 milestone Jun 7, 2024
cgwalters added a commit to cgwalters/bootc that referenced this issue Jun 9, 2024
These are things that https://tmt.readthedocs.io/en/stable/
wants, and the goal is to support running under tmt more
directly.

Part of containers#543

Signed-off-by: Colin Walters <[email protected]>
cgwalters added a commit to cgwalters/bootc that referenced this issue Jun 9, 2024
These are things that https://tmt.readthedocs.io/en/stable/
wants, and the goal is to support running under tmt more
directly.

Part of containers#543

With this I can run:

```
provision:
  how: virtual
  image: /home/walters/src/github/containers/bootc/target/testbootc-cloud.qcow2
summary: Basic smoke test
execute:
    how: tmt
    script: bootc status
```

Signed-off-by: Colin Walters <[email protected]>
@cgwalters
Copy link
Collaborator Author

OK #590 pushes things forward here a bit, getting to the point where we have a container image that can be turned into a disk image, that works directly with a simple tmt test.

However, the workflow of "build a disk image to pass to tmt" needs automating. I still don't largely understand how the expected workflow of "build code locally to pass to tmt" is expected to generally work. The way e.g. bib builds the container as part of the tests feels...wrong.

cgwalters added a commit to cgwalters/bootc that referenced this issue Jun 10, 2024
These are things that https://tmt.readthedocs.io/en/stable/
wants, and the goal is to support running under tmt more
directly.

Part of containers#543

Signed-off-by: Colin Walters <[email protected]>
cgwalters added a commit to cgwalters/bootc that referenced this issue Jun 10, 2024
Part of containers#543

I'm not totally happy with this, but it does demonstrate the basic
wiring flow of:

- `cargo xtask test-tmt`

That will do a container build, make a disk image from it,
and run a "hello world" tmt test.

A lot more to do here including wiring up our existing tests
into this, and deduplicating with the other integration tests.

A key aspect too will be exploring workflows that e.g. expose
a registry locally.

Signed-off-by: Colin Walters <[email protected]>
cgwalters added a commit to cgwalters/bootc that referenced this issue Jun 17, 2024
Part of containers#543

I'm not totally happy with this, but it does demonstrate the basic
wiring flow of:

- `cargo xtask test-tmt`

That will do a container build, make a disk image from it,
and run a "hello world" tmt test.

A lot more to do here including wiring up our existing tests
into this, and deduplicating with the other integration tests.

A key aspect too will be exploring workflows that e.g. expose
a registry locally.

Signed-off-by: Colin Walters <[email protected]>
henrywang pushed a commit to henrywang/bootc that referenced this issue Jun 18, 2024
Part of containers#543

I'm not totally happy with this, but it does demonstrate the basic
wiring flow of:

- `cargo xtask test-tmt`

That will do a container build, make a disk image from it,
and run a "hello world" tmt test.

A lot more to do here including wiring up our existing tests
into this, and deduplicating with the other integration tests.

A key aspect too will be exploring workflows that e.g. expose
a registry locally.

Signed-off-by: Colin Walters <[email protected]>
@cgwalters
Copy link
Collaborator Author

cgwalters commented Jun 20, 2024

  • Chatting with @martinpitt we determined it would probably make sense to have packit (as the "glue") support something like building a container image with the code into temporary registry (it could of course use the COPR or whatever it provisions and install into the container)
  • A good toplevel goal would be generic support sufficient to e2e verify centos-bootc - cockpit-ws does not work - selinux problems #571 in cockpit's CI, but also other projects (e.g. freeipa) could do very similar things

@martinpitt
Copy link

Note, I'm @martinpitt; @pitti exists, but is someone else 😁

Another approach which we discussed would be to design this "human/local first" (always good): build an OCI container locally with the modified code, turn that into a qcow, and boot that with qemu (or virt-install if you prefer). This is what you'd want to run tests locally without unnecessary faff like AWS credentials or large uploads/downloads.

However, that's hard to deploy to GitHub workflows and standard TF machines, but: @thrix mentioned last week that they offer /dev/kvm support in TF machines now, and even real-iron (I don't have details).

Another option is Cirrus CI, they also offer /dev/kvm. Our starter-kit project uses this, mostly as a demo -- but it does boot our standard cockpit bots VM images and runs stuff in them just fine. TF is obviously preferable both in terms of "use our standard tools" and also rate limiting, but maybe it's useful for something.

@cgwalters
Copy link
Collaborator Author

teemtee/tmt#3037 makes running tmt locally for me actually bearable. I was really surprised to run out of disk space on this workstation with 2T of local storage. Also auditing this stack, I did https://pagure.io/testcloud/pull-request/174

@henrywang
Copy link
Contributor

henrywang commented Jun 21, 2024

According to @cgwalters and @martinpitt (Hi, long time no see 😊) chatting, we can move integration test (shell + ansible) part to packit but with some changes:

  1. rename this test to end to end test, e2e is more reasonable in this case.
  2. run all e2e tests locally by dropping aws and quay.io dependence, but use vm and local registry instead. That avoids secrets required.
  3. e2e tests are for bootc install to-existing-root, bootc install to-disk, bootc upgrade/switch command test and their args tests.

What do you think @cgwalters? Thanks.

@cgwalters
Copy link
Collaborator Author

That sounds pretty good to me!

lukewarmtemp pushed a commit to lukewarmtemp/bootc that referenced this issue Jun 24, 2024
These are things that https://tmt.readthedocs.io/en/stable/
wants, and the goal is to support running under tmt more
directly.

Part of containers#543

With this I can run:

```
provision:
  how: virtual
  image: /home/walters/src/github/containers/bootc/target/testbootc-cloud.qcow2
summary: Basic smoke test
execute:
    how: tmt
    script: bootc status
```

Signed-off-by: Colin Walters <[email protected]>
cgwalters added a commit to cgwalters/bootc that referenced this issue Nov 5, 2024
…-simpler

 Add support for installing static grub configs
cgwalters added a commit to cgwalters/bootc that referenced this issue Nov 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request triaged This looks like a valid issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants