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

Add app-containers/docker-buildx #2204

Merged
merged 7 commits into from
Sep 3, 2024
Merged

Add app-containers/docker-buildx #2204

merged 7 commits into from
Sep 3, 2024

Conversation

krnowak
Copy link
Member

@krnowak krnowak commented Aug 5, 2024

This is to address a deprecation message that shows up when building docker images with docker build:

CI: http://jenkins.infra.kinvolk.io:8080/job/container/job/packages_all_arches/4602/cldsv/

docker.go:370: creating netcat containers
cluster.go:125: DEPRECATED: The legacy builder is deprecated and will be removed in a future release.
cluster.go:125:             Install the buildx component to build images with BuildKit:
cluster.go:125:             https://docs.docker.com/go/buildx/

The PR also drops some obsolete scripts.

  • Changelog entries added in the respective changelog/ directory (user-facing change, bug fix, security fix, update)
  • Inspected CI output for image differences: /boot and /usr size, packages, list files for any missing binaries, kernel modules, config files, kernel modules, etc.

@krnowak
Copy link
Member Author

krnowak commented Aug 8, 2024

CI passed. Another 23mb in /usr taken, because the buildx backend binary is 70mb (crazy!).

Copy link

github-actions bot commented Aug 8, 2024

@ader1990
Copy link
Contributor

ader1990 commented Aug 8, 2024

CI passed. Another 23mb in /usr taken, because the buildx backend binary is 70mb (crazy!).

btrfs compression pays off really well.

Copy link
Contributor

@ader1990 ader1990 left a comment

Choose a reason for hiding this comment

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

LGTM.

@chewi
Copy link
Contributor

chewi commented Aug 8, 2024

Are we supposed to have Docker build tools in the production image?

@krnowak
Copy link
Member Author

krnowak commented Aug 8, 2024

Are we supposed to have Docker build tools in the production image?

I think so. Why? docker build is a part of docker itself, so it was always a part of production images. Or rather - a part of docker base sysext. And some of our kola tests use docker build.

@ader1990
Copy link
Contributor

ader1990 commented Aug 8, 2024

Are we supposed to have Docker build tools in the production image?

Now depends on the use case, but we cannot remove a feature without deprecating it first. And docker build was (and maybe is) a core part of Docker. I think that without the feature, alot of use cases will become impossible.

@chewi
Copy link
Contributor

chewi commented Aug 8, 2024

I just find it odd given that we don't have other build tools, but I guess you don't need other build tools for docker build to be useful.

@chewi
Copy link
Contributor

chewi commented Aug 30, 2024

To expand on this, I feel that you might have removed docker build if you'd been able to before. I don't think adding buildx just because you couldn't remove build before isn't a strong reason. Keep in that mind that the docker-buildx binary is about 85MB.

@krnowak
Copy link
Member Author

krnowak commented Aug 30, 2024

I just find it odd given that we don't have other build tools, but I guess you don't need other build tools for docker build to be useful.

And…

To expand on this, I feel that you might have removed docker build if you'd been able to before. I don't think adding buildx just because you couldn't remove build before isn't a strong reason. Keep in that mind that the docker-buildx binary is about 85MB.

Yeah, the binary size is ridiculous. The saving grace here is that we use compressed btrfs, which shrinks that beast to 20mb.

But honestly, I don't think anyone (certainly not me!) has put any thought of removing having parts of docker that are of some use only in generic images (like docker build). So, thinking about it right now: I'm not sure how good it would sound to user that we have some cut-down version of docker. My uneducated guess would be that most users wouldn't care, because Flatcar is for running docker images, not building them. On the other hand - removing docker build it's another patch in need of maintaining. Tradeoffs, tradeoffs.

Hm. Some of our tests are using docker build, but that's our code, so we can work it around. How about the following plan:

  • We merge this PR, so generic image has a buildx plugin, no pesky deprecation messages are printed.
  • We eventually turn our developer container into a sysext image (I think Thilo wanted to do it), so we could move the plugin there.
  • We patch the docker message telling user to download the developer container sysext for the best experience (or for the actually working solution, because the deprecated functionality is going to be eventually dropped).
  • All those steps would require some deprecation process - possibly a news item for a release announcing the plans would be enough).

@chewi
Copy link
Contributor

chewi commented Sep 2, 2024

Sounds good, but maybe instead of patching Docker, you could just drop in a dummy buildx binary that errors with a message like that.

It's from Gentoo commit 78a80d67558ed5ae0f14ba8ecb8bee5d9aadd329.
The build_docker_aci script only supported docker 12.x, which we don't
have since ages, so it's a clear sign of a script being obsolete.
Removing it results in some other scripts in build_library being
unused, so drop them too.
@krnowak
Copy link
Member Author

krnowak commented Sep 2, 2024

Filed an issue: flatcar/Flatcar#1529

@krnowak
Copy link
Member Author

krnowak commented Sep 3, 2024

CI passed.

@krnowak krnowak merged commit f9d68df into main Sep 3, 2024
1 check failed
@krnowak krnowak deleted the krnowak/add-buildx branch September 3, 2024 13:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

Successfully merging this pull request may close these issues.

3 participants