targets/linux/deb/distro: add minimal images target for DEB distros#957
targets/linux/deb/distro: add minimal images target for DEB distros#957invidian wants to merge 5 commits into
Conversation
invidian
force-pushed
the
invidian/deb-containers-minimal
branch
from
44e9c93 to
db334e3
Compare
invidian
force-pushed
the
invidian/deb-containers-minimal
branch
from
db334e3 to
5b5cd00
Compare
invidian
force-pushed
the
invidian/deb-containers-minimal
branch
from
5b5cd00 to
8052858
Compare
invidian
force-pushed
the
invidian/deb-containers-minimal
branch
from
8052858 to
04f8665
Compare
invidian
force-pushed
the
invidian/deb-containers-minimal
branch
from
04f8665 to
ab07711
Compare
invidian
force-pushed
the
invidian/deb-containers-minimal
branch
from
ab07711 to
fba847f
Compare
invidian
force-pushed
the
invidian/deb-containers-minimal
branch
from
4decea1 to
fd3536d
Compare
invidian
force-pushed
the
invidian/deb-containers-minimal
branch
from
de959da to
ed4fcb5
Compare
invidian
force-pushed
the
invidian/deb-containers-minimal
branch
from
ed4fcb5 to
6936c77
Compare
invidian
force-pushed
the
invidian/deb-containers-minimal
branch
from
f8e74b6 to
0cd445e
Compare
invidian
force-pushed
the
invidian/deb-containers-minimal
branch
from
4bcdeb7 to
89b720f
Compare
invidian
changed the title
invidian
force-pushed
the
invidian/deb-containers-minimal
branch
from
89b720f to
8700599
Compare
invidian
force-pushed
the
invidian/deb-containers-minimal
branch
from
8700599 to
2bf1f4d
Compare
invidian
force-pushed
the
invidian/deb-containers-minimal
branch
from
2bf1f4d to
51938fb
Compare
There was a problem hiding this comment.
Pull request overview
This pull request adds support for building minimal container images for DEB-based distributions (Debian and Ubuntu) without requiring a pre-existing base image. The implementation downloads and extracts all necessary DEB packages into a volume using a worker image, then installs them using dpkg in the proper environment, addressing issue #448.
Changes:
- Introduces a new
containertarget for DEB distros that creates minimal images from scratch without requiring a base image - Refactors container-related tests into a shared
testContainerTargetfunction to test both the traditional (with base image) and new minimal (bootstrap) implementations - Adds essential base packages (
passwd,apt) to support user management and package operations in minimal images
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| website/docs/examples/targets.md | Documents new /container targets for all DEB-based distros (bionic, bookworm, bullseye, focal, jammy, noble, trixie) |
| test/target_ubuntu_test.go | Adds MinimalContainer field to test configuration to enable testing of the new minimal container target |
| test/linux_target_test.go | Refactors container tests into testContainerTarget function to support testing both traditional and minimal container implementations; adds slices import for container test updates |
| targets/linux/deb/distro/distro.go | Adds new container target handler that sets DefaultOutputImage to empty string to trigger bootstrap path |
| targets/linux/deb/distro/container_test.go | Adds comprehensive unit tests for bootstrap functionality including directory structure creation, dependency downloads, extra repo mounting, and apt cache configuration |
| targets/linux/deb/distro/container.go | Implements bootstrapContainer function with complex shell script that downloads essential packages, extracts them, handles merged-usr directories, and installs them in the target image |
| targets/linux/deb/ubuntu/common.go | Adds passwd and apt to basePackages list for minimal image support |
| targets/linux/deb/debian/common.go | Adds passwd and apt to basePackages list for minimal image support |
|
As far as I understand private feedback I got from @cpuguy83, the images are not really smaller from the original ones, so we should try to further minimize them before merging. I'll have a look into that. |
|
This looks like good progress. [
"base-files",
"ca-certificates",
"dalec-deb-base-packages",
"dash",
"debconf",
"debianutils",
"gcc-12-base",
"go-md2man",
"libacl1",
"libaudit-common",
"libaudit1",
"libbz2-1.0",
"libc-bin",
"libc6",
"libcap-ng0",
"libcom-err2",
"libcrypt1",
"libdb5.3",
"libgcc-s1",
"libgssapi-krb5-2",
"libk5crypto3",
"libkeyutils1",
"libkrb5-3",
"libkrb5support0",
"liblzma5",
"libnsl2",
"libpam-modules",
"libpam-modules-bin",
"libpam0g",
"libpcre2-8-0",
"libselinux1",
"libsemanage-common",
"libsemanage2",
"libsepol2",
"libssl3",
"libtirpc-common",
"libtirpc3",
"libzstd1",
"openssl",
"passwd",
"perl-base",
"tar",
"zlib1g"
]I think Can we clean up dash after everything? zstd, perl, etc seem to be remnants of the package manager. |
|
Thanks for feedback, let me see if more can be removed and I'll document why not. Will also graph the dependencies, so we know why we have these packages. Perhaps we can drop ca-certificates from deb base images then. |
I think we could, but we might need to do some magic and use worker image with dpkg to remove it, since dpkg itself depends on dash. I'm not sure it's worth the effort since it has just one dependency. This approach could simplify the cleanup process though, so I might have a look. On the other hand, we could try to remove
Which image you built? For trixie, this is the list of packages we end up with, if we remove graph LR
base-files
base-passwd
base-passwd --> libc6
base-passwd --> libdebconfclient0
base-passwd --> libselinux1
dalec-deb-base-packages
dalec-deb-base-packages --> passwd
dash
dash --> debianutils
dash --> libc6
debconf
debianutils
debianutils --> libc6
gcc-14-base
go-md2man
go-md2man-dbgsym
go-md2man-dbgsym --> go-md2man
libacl1
libacl1 --> libc6
libattr1
libattr1 --> libc6
libaudit-common
libaudit1
libaudit1 --> libaudit-common
libaudit1 --> libc6
libaudit1 --> libcap-ng0
libbsd0
libbsd0 --> libc6
libbsd0 --> libmd0
libbz2-1.0
libbz2-1.0 --> libc6
libc-bin
libc-bin --> libc6
libc6
libc6 --> libgcc-s1
libcap-ng0
libcap-ng0 --> libc6
libcap2
libcap2 --> libc6
libcrypt1
libcrypt1 --> libc6
libdb5.3t64
libdb5.3t64 --> libc6
libdebconfclient0
libdebconfclient0 --> libc6
libgcc-s1
libgcc-s1 --> gcc-14-base
libgcc-s1 --> libc6
libmd0
libmd0 --> libc6
libpam-modules
libpam-modules --> debconf
libpam-modules --> libaudit1
libpam-modules --> libc6
libpam-modules --> libcrypt1
libpam-modules --> libdb5.3t64
libpam-modules --> libpam-modules-bin
libpam-modules --> libpam0g
libpam-modules --> libselinux1
libpam-modules --> libsystemd0
libpam-modules-bin
libpam-modules-bin --> libaudit1
libpam-modules-bin --> libc6
libpam-modules-bin --> libcrypt1
libpam-modules-bin --> libpam0g
libpam-modules-bin --> libselinux1
libpam-modules-bin --> libsystemd0
libpam0g
libpam0g --> debconf
libpam0g --> libaudit1
libpam0g --> libc6
libpcre2-8-0
libpcre2-8-0 --> libc6
libselinux1
libselinux1 --> libc6
libselinux1 --> libpcre2-8-0
libsemanage-common
libsemanage2
libsemanage2 --> libaudit1
libsemanage2 --> libbz2-1.0
libsemanage2 --> libc6
libsemanage2 --> libselinux1
libsemanage2 --> libsemanage-common
libsemanage2 --> libsepol2
libsepol2
libsepol2 --> libc6
libsystemd0
libsystemd0 --> libc6
libsystemd0 --> libcap2
login.defs
passwd
passwd --> base-passwd
passwd --> libacl1
passwd --> libattr1
passwd --> libaudit1
passwd --> libbsd0
passwd --> libc6
passwd --> libcrypt1
passwd --> libpam-modules
passwd --> libpam0g
passwd --> libselinux1
passwd --> libsemanage2
passwd --> login.defs
|
wat, that's Ubuntu noble... |
|
@cpuguy83 please have a look again. I've updated the PR so that minimal containers do not pull BasePackages anymore, so we do not include Then, spec package can depend on things like graph LR
curl[curl]
curl --> libc6
curl --> libcurl4t64
curl --> zlib1g
gcc-14-base[gcc-14-base]
go-md2man[go-md2man]
go-md2man --> curl
libbrotli1[libbrotli1]
libbrotli1 --> libc6
libc6[libc6]
libc6 --> libgcc-s1
libcom-err2[libcom-err2]
libcom-err2 --> libc6
libcurl4t64[libcurl4t64]
libcurl4t64 --> libbrotli1
libcurl4t64 --> libc6
libcurl4t64 --> libgssapi-krb5-2
libcurl4t64 --> libidn2-0
libcurl4t64 --> libldap2
libcurl4t64 --> libnghttp2-14
libcurl4t64 --> libpsl5t64
libcurl4t64 --> librtmp1
libcurl4t64 --> libssh-4
libcurl4t64 --> libssl3t64
libcurl4t64 --> libzstd1
libcurl4t64 --> zlib1g
libdb5_3t64[libdb5.3t64]
libdb5_3t64 --> libc6
libffi8[libffi8]
libffi8 --> libc6
libgcc-s1[libgcc-s1]
libgcc-s1 --> gcc-14-base
libgcc-s1 --> libc6
libgmp10[libgmp10]
libgmp10 --> libc6
libgnutls30t64[libgnutls30t64]
libgnutls30t64 --> libc6
libgnutls30t64 --> libgmp10
libgnutls30t64 --> libhogweed6t64
libgnutls30t64 --> libidn2-0
libgnutls30t64 --> libnettle8t64
libgnutls30t64 --> libp11-kit0
libgnutls30t64 --> libtasn1-6
libgnutls30t64 --> libunistring5
libgssapi-krb5-2[libgssapi-krb5-2]
libgssapi-krb5-2 --> libc6
libgssapi-krb5-2 --> libcom-err2
libgssapi-krb5-2 --> libk5crypto3
libgssapi-krb5-2 --> libkrb5-3
libgssapi-krb5-2 --> libkrb5support0
libhogweed6t64[libhogweed6t64]
libhogweed6t64 --> libc6
libhogweed6t64 --> libgmp10
libhogweed6t64 --> libnettle8t64
libidn2-0[libidn2-0]
libidn2-0 --> libc6
libidn2-0 --> libunistring5
libk5crypto3[libk5crypto3]
libk5crypto3 --> libc6
libk5crypto3 --> libkrb5support0
libkeyutils1[libkeyutils1]
libkeyutils1 --> libc6
libkrb5-3[libkrb5-3]
libkrb5-3 --> libc6
libkrb5-3 --> libcom-err2
libkrb5-3 --> libk5crypto3
libkrb5-3 --> libkeyutils1
libkrb5-3 --> libkrb5support0
libkrb5-3 --> libssl3t64
libkrb5support0[libkrb5support0]
libkrb5support0 --> libc6
libldap2[libldap2]
libldap2 --> libc6
libldap2 --> libgnutls30t64
libldap2 --> libsasl2-2
libnettle8t64[libnettle8t64]
libnettle8t64 --> libc6
libnghttp2-14[libnghttp2-14]
libnghttp2-14 --> libc6
libp11-kit0[libp11-kit0]
libp11-kit0 --> libc6
libp11-kit0 --> libffi8
libpsl5t64[libpsl5t64]
libpsl5t64 --> libc6
libpsl5t64 --> libidn2-0
libpsl5t64 --> libunistring5
librtmp1[librtmp1]
librtmp1 --> libc6
librtmp1 --> libgmp10
librtmp1 --> libgnutls30
librtmp1 --> libhogweed6
librtmp1 --> libnettle8
librtmp1 --> zlib1g
libsasl2-2[libsasl2-2]
libsasl2-2 --> libc6
libsasl2-2 --> libsasl2-modules-db
libsasl2-2 --> libssl3t64
libsasl2-modules[libsasl2-modules]
libsasl2-modules --> libc6
libsasl2-modules --> libssl3t64
libsasl2-modules-db[libsasl2-modules-db]
libsasl2-modules-db --> libc6
libsasl2-modules-db --> libdb5_3t64
libssh-4[libssh-4]
libssh-4 --> libc6
libssh-4 --> libgssapi-krb5-2
libssh-4 --> libssl3t64
libssh-4 --> zlib1g
libssl3t64[libssl3t64]
libssl3t64 --> libc6
libtasn1-6[libtasn1-6]
libtasn1-6 --> libc6
libunistring5[libunistring5]
libunistring5 --> libc6
libzstd1[libzstd1]
libzstd1 --> libc6
zlib1g[zlib1g]
zlib1g --> libc6
graph LR
dpkg[dpkg]
dpkg --> libc6
dpkg --> liblzma5
dpkg --> libmd0
dpkg --> libselinux1
dpkg --> libzstd1
dpkg --> tarlibbz2-1_0
dpkg --> zlib1g
gcc-14-base[gcc-14-base]
go-md2man[go-md2man]
go-md2man --> dpkg
libacl1[libacl1]
libacl1 --> libc6
libbz2-1_0[libbz2-1.0]
libbz2-1_0 --> libc6
libc6[libc6]
libc6 --> libgcc-s1
libgcc-s1[libgcc-s1]
libgcc-s1 --> gcc-14-base
libgcc-s1 --> libc6
liblzma5[liblzma5]
liblzma5 --> libc6
libmd0[libmd0]
libmd0 --> libc6
libpcre2-8-0[libpcre2-8-0]
libpcre2-8-0 --> libc6
libselinux1[libselinux1]
libselinux1 --> libc6
libselinux1 --> libpcre2-8-0
libzstd1[libzstd1]
libzstd1 --> libc6
tar[tar]
tar --> libacl1
tar --> libc6
tar --> libselinux1
zlib1g[zlib1g]
zlib1g --> libc6
|
cpuguy83
left a comment
cpuguy83
left a comment
There was a problem hiding this comment.
Review by GPT-5.5.
The target split and shared container test refactor look good. Main remaining concern is making cleanup policy explicit and safe: prune bootstrap/package-manager detritus, but do not remove files that belong to the final intended image. In particular, systemd dirs should depend on whether systemd is installed, and docs preservation should include licenses.
| # | ||
| # Spec packages may depend on base packages, so we need to filter to only download remaining packages, since downloading local packages | ||
| # would fail. | ||
| dependencies_to_download=$(for f in ${local_package_files}; do dpkg-deb -f "${f}" Depends 2>/dev/null; done | tr ',' '\n' | sed 's/([^)]*)//g; s/|.*//; s/ //g' | grep -v '^$' | sort -u | grep -vxF "${local_package_names}" || true) |
There was a problem hiding this comment.
This dependency extraction is too lossy for Debian semantics. It only reads Depends, ignores Pre-Depends, strips version constraints, and chooses the first alternative before |, so the bootstrap package set can differ from what apt/dpkg would resolve. Please either move more of this to apt-native resolution or add coverage for Pre-Depends, alternatives/virtual packages, arch restrictions, and versioned deps.
| done | ||
|
|
||
| if [ -n "${purge_first}" ]; then | ||
| dpkg --purge --force-depends --force-remove-essential ${purge_first} || true |
There was a problem hiding this comment.
Since cleanup purges with --force-depends, swallowing purge failures can leave a partially cleaned image or inconsistent dpkg database while still succeeding the build. Can we fail on unexpected purge errors, or at least validate the final dpkg status after cleanup?
| # Remove leftover directories (after dpkg purge so maintainer scripts still work). | ||
| cleanup_dirs=" | ||
| /etc/apt | ||
| /etc/systemd |
There was a problem hiding this comment.
/etc/systemd and /var/lib/systemd should not be removed unconditionally. Suggested policy: if the final image has the systemd package installed, preserve these dirs; otherwise prune them. Checking the installed systemd package in dpkg status is more precise than libsystemd0; checking for systemctl would also be a pragmatic proxy.
| /var/lib/apt | ||
| /var/lib/pam | ||
| /var/lib/systemd | ||
| /var/log |
There was a problem hiding this comment.
Consider preserving /var/log as a directory while removing its contents. Removing the directory itself can surprise packages or processes that expect it to exist.
| /var/log | ||
| " | ||
|
|
||
| if [ "${DALEC_HAS_DOCS}" != "true" ]; then |
There was a problem hiding this comment.
This docs preservation condition should also account for license artifacts. Dalec DEB licenses install under /usr/share/doc, but HasDocs() only covers Docs and Manpages; a spec with only Licenses could have its license files removed during cleanup.
| if [ "${DALEC_HAS_DOCS}" != "true" ]; then | ||
| cleanup_dirs="${cleanup_dirs} | ||
| /usr/share/doc | ||
| /usr/share/man |
There was a problem hiding this comment.
Preserving spec-provided manpages is correct. Pruning dependency-owned manpages by default is fine for a minimal image, but that policy should be explicit/documented so users understand that only requested docs/manpages are preserved.
| dalec.WithConstraints(cleanupOpts...), | ||
| llb.AddMount("/tmp/dalec-cleanup.sh", scriptSt, llb.SourcePath("cleanup.sh"), llb.Readonly), | ||
| llb.AddMount("/tmp/dalec-spec-packages", input.SpecPackages, llb.Readonly), | ||
| llb.AddMount("/tmp/diff", stubSt, llb.SourcePath("stub"), llb.Readonly), |
There was a problem hiding this comment.
The /tmp/diff and /tmp/tar stubs only seem effective for the final purge where PATH="/tmp:${PATH}" is set. Maintainer scripts during the first purge pass will not find these unless they call absolute paths. Either prepend /tmp for both purge phases or adjust the comment/behavior.
| ref, err := res.SingleRef() | ||
| assert.NilError(t, err) | ||
|
|
||
| _, err = ref.StatFile(ctx, gwclient.StatRequest{Path: "/usr/share/doc"}) |
There was a problem hiding this comment.
This should verify the actual doc artifact path/content survives cleanup, not just that /usr/share/doc exists. That would catch cleanup accidentally preserving an empty directory while deleting user-requested docs.
| Tests: []*dalec.TestSpec{ | ||
| { | ||
| Name: "runtime dep binary works after cleanup", | ||
| Files: map[string]dalec.FileCheckOutput{ |
There was a problem hiding this comment.
This test only stats /usr/bin/curl and /usr/bin/dpkg. Since the test name says runtime dependencies are functional after cleanup, please execute them too, e.g. curl --version and dpkg --version, to catch missing interpreters/shared libraries.
This commit adds a new target for building minimal images for DEB-based distros. In new implementation, base image for DEB-based distros is not required, although still supported. New implementation in a nutshell downloads all required DEB packages into a volume using work image, extracts them into a target image then runs dpkg --install to run post-install scripts in the proper environment. It also moves container-related tests to a separate function so they can be executed to test both implementations while we keep the old, experimental implementation around. Closes project-dalec#448 Signed-off-by: Mateusz Gozdek <[email protected]>
So resulting images are smaller and have fewer packages. Signed-off-by: Mateusz Gozdek <[email protected]>
Signed-off-by: Mateusz Gozdek <[email protected]>
Signed-off-by: Mateusz Gozdek <[email protected]>
Signed-off-by: Mateusz Gozdek <[email protected]>
What this PR does / why we need it:
Which issue(s) this PR fixes (optional, using
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when the PR gets merged):Fixes #448
Special notes for your reviewer: