modern linuxkit, no more patch
With the last patches into buildkit and linuxkit, plus several fixes that @giggsoff found and added, we finally can upgrade to the latest linuxkit, which gives us the ability to use modern containerd and runc, etc. etc. It also fixes the issues we have had with running on M1.
This PR just upgrades linuxkit and removes the no-longer-necessary patch. We can do containerd and runc next.
I am seeing 2 failures in CI:
- eve image build - this likely is related, and I will track it down
- pillar unit test failure, see here
The pillar failure is not related to this change. I switched to current HEAD on master branch, reran the test, got the same error. Whatever it is, it needs to be fixed, but not part of this PR.
- pillar unit test failure, see here
The fix for that is in https://github.com/lf-edge/eve/pull/2645 which I just pulled to master.
Thanks, rebasing.
Unit tests pass now, thanks @eriknordmark and @giggsoff
It seems that we also should change the logic here: https://github.com/lf-edge/eve/blob/9856a9b4a5c84b44c95f4f054bdc10fe0e8faf72/.github/workflows/publish.yml#L36-L48
I hope that build for riscv64 will work out-of-box with --platforms=linux/riscv64 (or ZARCH=riscv64) and with adding of binfmt (i.e. by docker/setup-qemu-action@v2). But not sure, nice to check.
@giggsoff we need to discuss the --platform suggestion you made above.
Old linuxkit built packages in several stages. On each arch you did the following:
- run
lkt pkg push, which: - built it for the local arch (e.g. amd64)
- tagged it locally in docker
<digest>-<arch>, e.g.abcd1234-amd64 - tagged it locally in docker
<digest>just so you coulddocker runit locally, e.g.abcd1234 - pushed it to the registry with the
<digest>-<arch>tag, e.g.abcd1234-amd64 - created a multi-arch index for
<digest>for all arches it found in the registry, ignoring any errors
So you would run lkt pkg push on amd64, wait for it to be done, then went to arm64 and repeated, then riscv and repeated, etc.
This had several big shortcomings:
- You had to have a builder for each arch (no emulation or cross-building)
- You needed push credentials on each builder
- You could not run multiple in parallel, as the last stage run in two places at once could trounce each other. Best case, you missed an arch; worst, you got corruption.
The modern linuxkit bypasses all of this. You can have builders, but it is not required. You run lkt pkg push in one place, and it:
- Builds for your local arch and stores it locally in linuxkit cache as
<digest>-<arch>, e.g.abcd1234-amd64 - Finds any configured builders for other archs, builds on them remotely, storing the results in the local linuxkit cache as
<digest>-<arch>, e.g.abcd1234-arm64andabcd1234-riscv - Creates a multi-arch index locally for all the archs it built and saves it in local cache as
<digest>e.g.abcd1234 - Pushes out the index, and by extension all of the arch-specific manifests and their content, to the remote registry
lkt pkg build is the same for all steps, except for the last: pushing out the index and manifests and content.
How does this all fit into the eve build process, and do we need to change more?
In terms of local build, we are just building locally and not pushing anywhere, so there is no reason we cannot build just for the local platform.
In terms of release, i.e. we are building locally and pushing, we may need to rethink this. Pushing an image from the default GitHub amd64 builder, and then from our own arm64 builder, is going to create messed-up images.
Here are some possibilities:
- We take your
--platforms=$(ZARCH)suggestion for the CIbuild.yaml, but forpublish.yaml, we remove the whole Actions matrix, and build and push all platforms we want, either the default, or by explicitly listing all platforms via--platforms=arch1,arch2,..,archN. - We remove the Actions matrix entirely, remove the arm64 builder as GitHub Actions builder, and instead make it a linuxkit builder so we don't have to do emulation.
- Middle ground: we remove the Actions matrix for builds, but use it to run tests. This may not be necessary; I did a quick sweep of the 8 actions yaml files, and as far as I can tell, we use it primarily to build, not to run things.
Thoughts? Other ideas? I believe @rvs knows the history of the builder and what we use it for.
@giggsoff wrote here about the "riscv" special case.
I think that the second or third option above makes all of this go away.
@giggsoff wrote here about the "riscv" special case.
I think that the second or third option above makes all of this go away.
One problem here is that we build another image for riscv64 arch. It differ not only in arch, but also in content. We build subset of packages you can find them here. Moreover, EVE-OS image itself uses special manifest. We set HV=mini and use this patch for rootfs.yml. So riscv64 it is not just another arch in our case, but different image. It may be worth leaving the various architectures in the CI matrix for now and assembling packages under the architecture from the matrix.
One problem here is that we build another image for riscv64 arch. It differ not only in arch, but also in content.
I have no issues with that. what we build can be distinct from where we build it. We can have separate tasks, and even use the matrix to run them. The matrix does not have to indicate where it runs, only that it is a variable, and that the actions get run each time with the variables set differently each time.
One of those variables happens to control the runner. (well, it wasn't pure chance; GitHub did design it that way 😄 )
As it stands now, we build arm64 and amd64 natively (using native GH Actions runners), and riscv via emulation. That is a bit messy (and hence the "magic code" you referenced above). We really should clean that up.
Let us get back to the core questions. What do we build, and where do we build it? (I am ignoring the --docker load question for the moment; one problem at a time).
What are the results we want from each stage. Here is what I think I see, but I am not sure.
- PR builds: build packages, build eve-os, run tests. We do not push to anywhere, we do not use any cross-images. This just happens on the runner and stays on the runner.
- Merge-to-master builds: build packages, build eve-os, push to registry.
- Release: same as merge-to-master, except with semver tags, plus artifacts on GitHub releases.
Most importantly, those pushed images all are multi-arch indexes. That means that the push must happen from a single place. This is how linuxkit pkg push is designed.
So we could do --platform=$(ZARCH) on the PR builds, but that isn't going to buy us much on merge-to-master builds and release builds, where we need all of the images in the local cache, so we can then do a single push out.
Thoughts @giggsoff ?
I have no issues with that. what we build can be distinct from where we build it. We can have separate tasks, and even use the matrix to run them. The matrix does not have to indicate where it runs, only that it is a variable, and that the actions get run each time with the variables set differently each time.
So we could do
--platform=$(ZARCH)on the PR builds, but that isn't going to buy us much on merge-to-master builds and release builds, where we need all of the images in the local cache, so we can then do a single push out.
Sorry, I cannot understand how we can merge the first and the last paragraph at the same time. Probably I miss something, but without platforms flag we will build all platforms in all tasks/runners and it looks like a waste of time/space.
Another potential problem of having no platforms flag is that linuxkit will build for linux/arm64, linux/amd64 and linux/s390x (please correct me if I am wrong), that is I think we should define something in platforms, why not to re-implement the current behavior with --platforms=linux/$(ZARCH)?
Most importantly, those pushed images all are multi-arch indexes. That means that the push must happen from a single place. This is how linuxkit pkg push is designed.
I cannot see any technical issue with that. We check hashes of packages and EVE-OS itself and expect that build from one runner will provide the same result as build from several runners. At the same time linuxkit can go through previously published images in docker hub and append new one (with another platform) from different task/runner.
but without platforms flag we will build all platforms in all tasks/runners and it looks like a waste of time/space
You are correct, I wasn't very clear.
We always need to do either --platforms or to include the platforms in each build.yaml (which probably is the better idea, but one step at a time).
What I was focusing on is the following.
PR Builds and Tests
If I am building for CI tests only on one runner, e.g. the GHA default amd64, do I:
--platforms=linux/amd64while trusting that the matrix will trigger another run on arm64 with--platforms=linux/arm64; OR--platforms=linux/amd64,linux/arm64and build for all, using our arm64 runner as a builder
Merge-to-Master and Release
How do we build a multi-arch index that contains all targeted arches. We need to run lkt pkg push from one location. We can use remote builders using docker contexts, but it has to be run in one place.
It does understand how to pull from a registry. So if you do:
lkt pkg push pkg/foo
It will:
- Calculate the git-based tag for
pkg/foo - For each targeted arch:
- Try to pull that tagged image from the registry
- If could not pull, or had
--force, build locally
- Push each arch-specific image to registry (unless it already is there, of course)
- Create a multi-arch index for all targeted arches and push it
So, yes, it can pull from remote if it is trying to build locally and does not exist.
But, no, it will not do plain remote manipulation of registry: "I looked on registry, I found 3 arches, I will create a multi-arch index for them". It used to, but there were regular issues with trouncing each other, and there are dedicated tools for that.
At the same time linuxkit can go through previously published images in docker hub and append new one (with another platform) from different task/runner
This is what it will not do. It is not trying to be a generic manifest-tool (see Phil Estes' manifest tool for that). It limits itself to:
- build packages locally
- pull to cache if the source is local and it is available on registry and
--forceis not set - push arch-specific from local cache after build
- create and push multi-arch index for arches that it has locally
Summary
First, is it correct that when we do PR builds, that we do not push out artifacts, or at least not multi-arch index artifacts? If so, then, yes, we can just do --platform=$(ZARCH) and be done there.
Second, for merge-to-master and release builds, we always have multi-arch indexes, which means we need "one place to run it". We just need to decide if we will:
- run build-and-push-arch-only-per-runner, and then somehow coordinate a later stage that pulls them all locally to cache, creates an index, and pushes the index.
- run a single
lkt pkg pushprocess, in a single runner, that builds and pushes it all, either locally via emulation or using remote docker runners
OK, I stand corrected. linuxkit used to check the registry for all arch-specific tags and then update the multi-arch index.
And it still does. That chunk never was removed. I had thought it was (as you can tell from my comments above), and apparently it still does it.
This is risky. If two parallel workers do it at the same time, one will trounce the other. There is no locking mechanism (which is why I had thought it was removed).
With that in hand, our options:
- PR build: this does not push, if we only build locally, then no reason not to use
pkg push --platforms=$(ZARCH) - merge-to-master/release build: this does push, we still can do it locally
pkg push --platforms=$(ZARCH), and it will update the index after each push. But this must be controlled so it doesn't happen in parallel.
The problem with the above is that we would need that riscv "magic", and it is hard to keep track, and parallel builds, etc. But it would work. So I think we should go for it, as @giggsoff suggested, and then in a future PR we can propose an alternate multi-platform process.
I think this is a good enough approach to provide predictable behavior, and we have the same approach now.
it will update the index after each push. But this must be controlled so it doesn't happen in parallel.
Seems we were lucky to not have such problems, but I think it is because of different time to build for several platforms and it would be nice to have some synchronization in the end to check all arches and made manifest. Perhaps we can do synchronization inside another (new?) workflow that we will run after publishing.
Unfortunately I can still see some problems with separate pkg push --platforms=linux/$(ZARCH) without --force flag (we unset this flag for pkgs as I can see).
With lkt pkg manifest, we can avoid the issue by having that run after everything else is complete.
Our steps then are:
- Wait for the
pkg manifestandpkg pushfixes to merge in - Update this PR to use those
- Add
--platform=$(ZARCH)to this PR
The only thing remaining after that is the --docker question.
@giggsoff with all of that work done on linuxkit, it is about time to get this moving forward. I have one open PR on linuxkit, it will merge in early next week (prob Monday). Then your fix can be rebased and I will approve it and merge it in. Then we can come back here, I will rebase on that latest commit after your PR merges, and we can finally hit this.
Trying to summarize:
- Remaining linuxkit PRs - done by early next week
lkt pkg manifest- if I understood correctly, we will- explicitly build for all desired platforms using
--platforms=...or usingbuild.yml(much better) - In PRs, we do not do
pkg push(or don't really care about the results), so it doesn't matter - In merge-to-master, we will do
pkg pushbut at the end of the whole thing do apkg manifest(dependencies managed via GitHub Actions dependencies), so we are guaranteed a complete manifest
- explicitly build for all desired platforms using
--docker- did we have any resolution on it?
Do we have a list somewhere of which packages to build for which arches? I am happy to open the PR to update all of the build.yaml, but I do not know where the list is?
--docker- did we have any resolution on it?
We can try to go with --docker for all cases as the first approach. We need this option for about a third of the packages: https://github.com/lf-edge/eve/pull/2727#discussion_r928537594
Do we have a list somewhere of which packages to build for which arches? I am happy to open the PR to update all of the
build.yaml, but I do not know where the list is?
Is it what you need (subset of packages for riscv64 as well as for amd64 and arm64)? https://github.com/lf-edge/eve/blob/eae8fbe72e2c7ea289b6d35e0bb98a292b6188cd/Makefile#L283-L285
Other packages compiled only for amd64, arm64, not for riscv64.
But I still think that we need control the target platform via --platforms=..., or more specifically through ZARCH option, because when I am sitting behind my laptop and want to check my new version of pillar, I do not want to build it for all arches with all dependencies.
Is it what you need (subset of packages for riscv64 as well as for amd64 and arm64)? Other packages compiled only for amd64, arm64, not for riscv64.
Yeah, that should work.
But I still think that we need control the target platform via --platforms=..., or more specifically through ZARCH option, because when I am sitting behind my laptop and want to check my new version of pillar, I do not want to build it for all arches with all dependencies.
Agreed, but that would be an override. In other words, anytime we build:
- each package should inherently in its directory declare, "these are the platforms you can build me for, and by default I will be built for those". If you want to build a package directly in its own directory, it is clear about what it is for.
- Makefile should have an option to explicitly control which ones you want, which is what you described above
So I should be able to do any of the following and it works (leaving out some detail):
lkt pkg build pkg/pillar- build pillar for all packages for which it is supported and only thosemake pkgs- build all of the packages for all arch for which it is supported and only thosemake eve-pillar- build pillar for all arch for which it is supported and only thosemake pkgs ZARCH=amd64- build all packages for a specific archmake eve-pillar ZARCH=amd64- build pillar for a specific arch
All of the current ones work except for the first. I was suggesting that:
- We should have the first work
- The process for making the second and fourth work should be simplified ("build x for arches y,z by default" should be declared in the package), but this is under the covers
Let's get your linuxkit PR done, then let's update this.
@deitch any idea why the build is failing?
any idea why the build is failing?
No, but I haven't dug deeply enough yet @eriknordmark . It doesn't matter, though, as we still have some changes to make, based on the above.
This now includes the correct latest linuxkit, including the patches @giggsoff or I made over the last several weeks. Going to add the pieces discussed above with @giggsoff and then will let CI run and address any remaining test failures after that.
any idea why the build is failing?
@eriknordmark yes, I do now. It is the very "build dependency" problem that delayed us from moving to this version of linuxkit for a year.
building pkg/pillar to lfedge/eve-pillar:2a84853f699179fcc91e4519459049d7c3736e37 depends upon lfedge/eve-dom0-ztools:ff71338c79558620a3d6dd770ad0291b786e759d (and 2 others), which only exists locally as part of the build. It only is stored in the linuxkit cache.
Now that we can tell buildkit to source from there, we should be able to get it working.
any idea why the build is failing?
@eriknordmark yes, I do now. It is the very "build dependency" problem that delayed us from moving to this version of linuxkit for a year.
building
pkg/pillartolfedge/eve-pillar:2a84853f699179fcc91e4519459049d7c3736e37depends uponlfedge/eve-dom0-ztools:ff71338c79558620a3d6dd770ad0291b786e759d(and 2 others), which only exists locally as part of the build. It only is stored in the linuxkit cache.Now that we can tell buildkit to source from there, we should be able to get it working.
I hope the problem of building solved here https://github.com/lf-edge/eve/pull/2773. It was the problem in update of packages while waiting for another PR to merge.
No, that is not it, and I am quite confused. Maybe @giggsoff knows?
Here is the beginning of pkg/pillar/Dockerfile:
FROM lfedge/eve-dom0-ztools:ff71338c79558620a3d6dd770ad0291b786e759d as zfs
But that tag ff71338c79558620a3d6dd770ad0291b786e759d does not exist in docker hub, and is not the current for pkg/dom0-ztools. So where is it?
Haha, crossed wires! Thanks @giggsoff . So I will hold until you have that one fixed.
Once that is in, I will rebase and push out. That should trigger the few failing points in CI due to the chained builds, which I will fix.
Rebased now that #2773 is in.
Note that I added the pkg manifest steps to the GitHub actions.
Build is failing, but not where I expected. Didn't you see this error before @giggsoff and @rvs ?
Build is failing, but not where I expected. Didn't you see this error before @giggsoff and @rvs ?
I see that we are failing on build of arm64 EVE, but it is not expected in amd64 workflow. Seems we are still missing platforms flag.
As far as I can tell, it is doing a build on ubuntu_20.04 (i.e. amd64) runner as follows :
linuxkit -v pkg build --hash-path /home/runner/work/eve/eve --hash 0.0.0-pr2727-71d556aa-kvm --force /home/runner/work/eve/eve/dist/amd64/0.0.0-pr2727-71d556aa
which is then building for amd64 here and arm64 here.
The second one (cross-build) is failing here.
That means that qemu or cross-build is not setup or running properly.
But I am not sure if that is what we are supposed to be doing? I thought we used the runners matrix to build each locally? And it looks like the generated eve/Dockerfile (from eve/Dockerfile.in) contains amd64 tags?
FROM docker.io/lfedge/eve-mkimage-raw-efi:f4a6c201c9495d36b8de8728794e3aa24e41cd1d-amd64@sha256:a6baafae974de20e864698ab9a97f27789fcbf913c3eb13142189402a68acfe3
Is our tagging system mistaken? Or are we running a cross-build of eve when this should be running only the local architecture?
If I recall correctly, linuxkit started supporting cross-building packages sometime in the year+ that we fell out of date.
@giggsoff is this related to the --platforms argument you discussed? Should we be adding it?
Or perhaps should parse-pkgs.sh be provided index tags, e.g.
docker.io/lfedge/eve-mkimage-raw-efi:f4a6c201c9495d36b8de8728794e3aa24e41cd1d
and not
docker.io/lfedge/eve-mkimage-raw-efi:f4a6c201c9495d36b8de8728794e3aa24e41cd1d-amd64
Some insights from @giggsoff and @rvs please
Looks like @giggsoff and I came to the same conclusions at the same time.
I still wonder if parse-pkgs.sh should give generic tags, and not -<arch> tags? We can fix this independently, but it is a good question.
I will fix it for now, but I am not at all sure that this is the right thing to do @giggsoff . If I run make eve, why should it default to just the platform I am on (or override with ZARCH=something)? Why should it not default to all available platforms?
make eve- build for all known platformsmake eve ZARCH=amd64- build for amd64- etc