containers / netavark

Container network stack
Apache License 2.0
534 stars 84 forks source link

replace ubuntu/centos container image build tests #775

Open Luap99 opened 1 year ago

Luap99 commented 1 year ago

Currently we do a centos9 and ubuntu 2004 build test in containers, as @cevich noted cirrus CI will start charging for that because we are over the free limit which liekly mean failing tests soon.

So I think we should we move the test to a different platform (github action or our existing VMs).

However while we do this I think the current container based build are not that great to test different rust versions. First we do not have explicit control over the rust version so we actually do not even know what we are building with. Then the quay.io auto builder for these images keeps failing from time to time so manually intervention is needed to update these images so we can get a newer rust.

Because netavark does not really depend on the OS I think it would make more sense to have an explicitly defined Minimum Support Rust Version (MSRV) and then make sure to test this version. Updates to the MSRV will be explicit so we always know what versions we support. Of course we should keep in mind what rust version RHEL uses and not depend on newer versions to not cause downstream build failures.

@baude @mheon @flouthoc @cevich WDYT? Also the problem same applies to the aardvark-dns repo.

baude commented 1 year ago

yeah agree

cevich commented 1 year ago

Thanks for opening this issue Paul, I appreciate it.

So I think we should we move the test to a different platform (github action or our existing VMs).

Converting to GHA would likely only be feasible for Ubuntu. If you'd like to try, please feel free to go for it. You may find some of the changes and notes in my buildah experiment helpful.

General caveat of note: GHA workflows always source their instructions from either a PR or the main branch. Cirrus always uses the cloned copy of it's YAML. We execute runs on release branches daily via cirrus-cron. This means every release branch will also need the cirrus tasks disabled, otherwise they'll continue to use up minutes.

Then the quay.io auto builder for these images keeps failing

Yeah that thing sucks, and the failure notification sucks too. If it would help, this repo. is already setup to e-mail podman-monitor if any cirrus-cron task fails. So, you could (for example) add a container image build & push cirrus-cron task to run daily. Here's a (way more complex than this would need) example of this in skopeo.

Because netavark does not really depend on the OS I think it would make more sense to have an explicitly defined Minimum Support Rust Version (MSRV) and then make sure to test this version.

I'm supportive of this. The CI VM images build scripts simply grab whatever the stable toolchain is at the time. If it's helpful, that could instead clone the netavark repo, and grab the desired toolchain version from a file. That may also help with...

GHA Migration note (remember, the GHA workflow YAML will run from main): Synchronizing the rust version w/ the cirrus CI VM images may be difficult to do. If the intended toolchain version is in a repo. file, it would be relatively trivial to do a runtime install (yuck, but, ignoring) based on the file contents. You'd need to make sure that file exists in all important release branches as well.

HTH

Luap99 commented 1 year ago

I'm supportive of this. The CI VM images build scripts simply grab whatever the stable toolchain is at the time. If it's helpful, that could instead clone the netavark repo, and grab the desired toolchain version from a file. That may also help with...

This is not really what I want to change, we should continue using the latest stable version for builds/testing for new linters, etc... What I want is another check which tests that we can build with a defined older version X (called MSRV).

For a GHA example see conmon-rs: https://github.com/containers/conmon-rs/blob/5900180d31ba3ec82870042ed18c5a8348350be6/.github/workflows/ci.yml#L10C3-L33

cevich commented 1 year ago

Oh okay, I get what you're saying. I know it's not exactly the same thing but we do use "older" toolchain for CI on the release-branches. In case that matters.

baude commented 1 year ago

@Luap99 at one time, the team was adamant about checking the various distributions because rust versions differed so greatly. your comment, although i can get behind it, does disagree with that initial requirement. lets discuss in scrum.

Luap99 commented 1 year ago

Right I am not saying we should ignore all distro versions. The fact is that the current system is broken as the quay auto builder keeps broken and thus we keep building with even older versions instaed of the ones actually in the repos.

What I want is an explicit minimal support rust version. Then we know what works and if someone wants to use a newer feature (either directly or via dependency bump) we can decide to update our minimal supported version. That would be an explicit decision and in order to decide we can look what is in RHEL/ubuntu/debian, etc... For small things we can certainly hold off updating until distros catch up, but we also need to keep in mind that things like CVE fixes may force our hand.

And generally speaking the reason why I said that was because you can look into the rust version in debian SID right now. It is on 1.68.2 which is 6 months old and 4 minor versions behind the current release 1.72. And debian stable is on 1.63. It is just nor realistic for us to support such old versions IMO. It becomes more and more of a burden when dependency updates pile up.