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

[master] add docker-buildx-plugin package (deb, rpm, static) #654

Merged
merged 3 commits into from
Mar 27, 2022

Conversation

crazy-max
Copy link
Member

@crazy-max crazy-max commented Mar 25, 2022

fixes #618
closes #415

for the upcoming major docker release we want to make sure buildx is part of its own package like scan and compose. currently, buildx is part of the docker-ce-cli package.

this also creates the static tarball.

last commit removes the plugin installers logic as it was only used for buildx and should not be used in the future.

in a follow-up we want to fetch plugins packages from upstream repositories instead of rebuilding them everytime. see docker/buildx#840

$ VERSION=22.04.0 make debian-bullseye
$ VERSION=22.04.0 make centos-7
$ VERSION=22.04.0 make DOCKER_BUILD_PKGS='static-linux' static

@crazy-max crazy-max force-pushed the buildx-package branch 2 times, most recently from 365ada1 to 0538f3d Compare March 25, 2022 11:26
@crazy-max crazy-max marked this pull request as ready for review March 25, 2022 11:49
@crazy-max crazy-max marked this pull request as draft March 25, 2022 11:49
@crazy-max
Copy link
Member Author

I'm not able to build locally I'm not sure why:

$ make REF=master debian-bullseye
...
# docker-ce-rootless-extras install
install -D -m 0755 /usr/local/bin/rootlesskit debian/docker-ce-rootless-extras/usr/bin/rootlesskit
install -D -m 0755 /usr/local/bin/rootlesskit-docker-proxy debian/docker-ce-rootless-extras/usr/bin/rootlesskit-docker-proxy
install -D -m 0755 engine/contrib/dockerd-rootless.sh debian/docker-ce-rootless-extras/usr/bin/dockerd-rootless.sh
install -D -m 0755 engine/contrib/dockerd-rootless-setuptool.sh debian/docker-ce-rootless-extras/usr/bin/dockerd-rootless-setuptool.sh
# TODO: how can we install vpnkit?
make[1]: Leaving directory '/root/build-deb'
   debian/rules override_dh_install
make[1]: Entering directory '/root/build-deb'
dh_install
# TODO Can we do this from within our container?
dh_apparmor --profile-name=docker-ce -pdocker-ce
make[1]: Leaving directory '/root/build-deb'
   dh_installdocs
cli/README.md: 3: Syntax error: word unexpected (expecting ")")
dh_installdocs: warning: debian/docs is marked executable but does not appear to an executable config.
dh_installdocs: warning:
dh_installdocs: warning: If debian/docs is intended to be an executable config file, please ensure it can
dh_installdocs: warning: be run as a stand-alone script/program (e.g. "./debian/docs")
dh_installdocs: warning: Otherwise, please remove the executable bit from the file (e.g. chmod -x "debian/docs")
dh_installdocs: warning:
dh_installdocs: warning: Please see "Executable debhelper config files" in debhelper(7) for more information.
dh_installdocs: warning:
dh_installdocs: error: debian/docs (executable config) returned exit code 2
make: *** [debian/rules:127: binary] Error 25
dpkg-buildpackage: error: debian/rules binary subprocess returned exit status 2
make[1]: *** [Makefile:78: debian-bullseye] Error 2

Seems fine on CI: https://ci-next.docker.com/public/blue/organizations/jenkins/docker-ce-packaging/detail/PR-654/3/pipeline

Any idea @thaJeztah?

@crazy-max
Copy link
Member Author

As discussed about #654 (comment), it's an issue with my local env.

@crazy-max crazy-max marked this pull request as ready for review March 25, 2022 12:10
@crazy-max
Copy link
Member Author

crazy-max commented Mar 25, 2022

Comparing pkg generated on docker/buildx#840 and with this PR:

# with https://github.com/docker/buildx/pull/840
$ dpkg-deb -I docker-buildx-plugin_0.7.0~89-g4a49a60f.m-debian11_amd64.deb
 new Debian package, version 2.0.
 size 12365532 bytes: control archive=424 bytes.
       1 bytes,     0 lines      conffiles
     308 bytes,     9 lines      control
      81 bytes,     1 lines      md5sums
 Package: docker-buildx-plugin
 Version: 0:0.7.0-debian11~89-g4a49a60f.m
 Section: default
 Priority: optional
 Architecture: amd64
 Maintainer: Docker <[email protected]>
 Installed-Size: 46516
 Homepage: https://github.com/docker/buildx
 Description: Docker Buildx plugin extends build capabilities with BuildKit.
# with docker-ce-packaging
$ dpkg-deb -I docker-buildx-plugin_0.8.1~debian-bullseye_amd64.deb
 new Debian package, version 2.0.
 size 23107212 bytes: control archive=568 bytes.
     352 bytes,    11 lines      control
     168 bytes,     2 lines      md5sums
 Package: docker-buildx-plugin
 Source: docker-ce (5:0.0.0-20220325091330-87b8e57-0~debian-bullseye)
 Version: 0.8.1~debian-bullseye
 Architecture: amd64
 Maintainer: Docker <[email protected]>
 Installed-Size: 67146
 Enhances: docker-ce-cli
 Section: admin
 Priority: optional
 Homepage: https://github.com/docker/buildx
 Description: Docker Buildx cli plugin.

@crazy-max
Copy link
Member Author

crazy-max commented Mar 25, 2022

Looks like docker-ce-cli pkg also installs docker-app, not sure where it comes from:

$ dpkg -c docker-ce-cli_20.10.14~3-0~debian-bullseye_amd64.deb
drwxr-xr-x root/root         0 2022-03-24 01:46 ./
drwxr-xr-x root/root         0 2022-03-24 01:46 ./usr/
drwxr-xr-x root/root         0 2022-03-24 01:46 ./usr/bin/
-rwxr-xr-x root/root  61885504 2022-03-24 01:46 ./usr/bin/docker
drwxr-xr-x root/root         0 2022-03-24 01:46 ./usr/libexec/
drwxr-xr-x root/root         0 2022-03-24 01:46 ./usr/libexec/docker/
drwxr-xr-x root/root         0 2022-03-24 01:46 ./usr/libexec/docker/cli-plugins/
-rwxr-xr-x root/root  33637568 2022-03-24 01:46 ./usr/libexec/docker/cli-plugins/docker-app
-rwxr-xr-x root/root  68787979 2022-03-24 01:46 ./usr/libexec/docker/cli-plugins/docker-buildx
...

Edit: removed on master: #548 (review)

@thaJeztah
Copy link
Member

yes 20.10.14 still had the docker-app plugin included; it's removed in the master branch

@crazy-max crazy-max changed the title add docker-buildx-plugin package (deb, rpm) add docker-buildx-plugin package (deb, rpm, static) Mar 26, 2022
@crazy-max
Copy link
Member Author

crazy-max commented Mar 26, 2022

@thaJeztah This also adds the static package for buildx:

$ VERSION=22.04.0 make DOCKER_BUILD_PKGS='static-linux' static
...
$ ls static/build/linux/
docker  docker-22.04.0.tgz  docker-buildx-plugin-0.8.1.tgz  docker-rootless-extras  docker-rootless-extras-22.04.0.tgz

I made some tests to check everything is ok with rpm and deb packages because we are now splitting packages (docker-ce-cli with docker-buildx-plugin) so there is an implicit conflict with /usr/libexec/docker/cli-plugins/docker-buildx:

$ VERSION=22.04.0 make centos-7
$ docker run --rm -it -v $(pwd)/rpm/rpmbuild/centos-7/RPMS/x86_64:/pkgs centos:7 bash
[root@cb4fce82d0ec /]# yum install -y yum-utils
[root@cb4fce82d0ec /]# yum-config-manager --add-repo https://download.docker.com/linux/centos/docker-ce.repo
[root@cb4fce82d0ec /]# yum install -y docker-ce-cli
...
Dependencies Resolved

========================================================================================================================================================================================================== Package                                             Arch                                    Version                                              Repository                                         Size 
==========================================================================================================================================================================================================Installing:
 docker-ce-cli                                       x86_64                                  1:20.10.14-3.el7                                     docker-ce-stable                                   30 M 
Installing for dependencies:
 docker-scan-plugin                                  x86_64                                  0.17.0-3.el7                                         docker-ce-stable                                  3.7 M 

Transaction Summary
==========================================================================================================================================================================================================Install  1 Package (+1 Dependent package)

Total download size: 34 M
Installed size: 158 M
Downloading packages:
warning: /var/cache/yum/x86_64/7/docker-ce-stable/packages/docker-scan-plugin-0.17.0-3.el7.x86_64.rpm: Header V4 RSA/SHA512 Signature, key ID 621e9f35: NOKEY
Public key for docker-scan-plugin-0.17.0-3.el7.x86_64.rpm is not installed
(1/2): docker-scan-plugin-0.17.0-3.el7.x86_64.rpm                                                                                                                                  | 3.7 MB  00:00:00     
(2/2): docker-ce-cli-20.10.14-3.el7.x86_64.rpm                                                                                                                                     |  30 MB  00:00:00     
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------Total                                                                                                                                                                      50 MB/s |  34 MB  00:00:00     
Retrieving key from https://download.docker.com/linux/centos/gpg
Importing GPG key 0x621E9F35:
 Userid     : "Docker Release (CE rpm) <[email protected]>"
 Fingerprint: 060a 61c5 1b55 8a7f 742b 77aa c52f eb6b 621e 9f35
 From       : https://download.docker.com/linux/centos/gpg
Running transaction check
Running transaction test
Transaction test succeeded
Running transaction
  Installing : 1:docker-ce-cli-20.10.14-3.el7.x86_64                                                                                                                                                  1/2 
  Installing : docker-scan-plugin-0.17.0-3.el7.x86_64                                                                                                                                                 2/2 
  Verifying  : docker-scan-plugin-0.17.0-3.el7.x86_64                                                                                                                                                 1/2 
  Verifying  : 1:docker-ce-cli-20.10.14-3.el7.x86_64                                                                                                                                                  2/2 

Installed:
  docker-ce-cli.x86_64 1:20.10.14-3.el7

Dependency Installed:
  docker-scan-plugin.x86_64 0:0.17.0-3.el7

Complete!
[root@cb4fce82d0ec /]# yum install -y createrepo
[root@cb4fce82d0ec /]# createrepo /pkgs
[root@cb4fce82d0ec /]# cat > /etc/yum.repos.d/docker-test.repo <<EOL
[docker-test]
name=DockerTest
baseurl=file:///pkgs
enabled=1
gpgcheck=0
EOL
[root@cb4fce82d0ec /]# yum update -y docker-ce-cli
...
Dependencies Resolved

========================================================================================================================================================================================================== Package                                                Arch                                     Version                                              Repository                                     Size 
==========================================================================================================================================================================================================Updating:
 docker-ce-cli                                          x86_64                                   1:22.04.0-3.el7                                      docker-test                                   7.4 M 
Installing for dependencies:
 docker-buildx-plugin                                   x86_64                                   0.8.1-3.el7                                          docker-test                                    11 M 

Transaction Summary
==========================================================================================================================================================================================================Install             ( 1 Dependent package)
Upgrade  1 Package

Total download size: 19 M
Downloading packages:
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------Total                                                                                                                                                                     1.2 GB/s |  19 MB  00:00:00     
Running transaction check
Running transaction test
Transaction test succeeded
Running transaction
  Installing : docker-buildx-plugin-0.8.1-3.el7.x86_64                                                                                                                                                1/3 
  Updating   : 1:docker-ce-cli-22.04.0-3.el7.x86_64                                                                                                                                                   2/3 
  Cleanup    : 1:docker-ce-cli-20.10.14-3.el7.x86_64                                                                                                                                                  3/3 
  Verifying  : 1:docker-ce-cli-22.04.0-3.el7.x86_64                                                                                                                                                   1/3 
  Verifying  : docker-buildx-plugin-0.8.1-3.el7.x86_64                                                                                                                                                2/3 
  Verifying  : 1:docker-ce-cli-20.10.14-3.el7.x86_64                                                                                                                                                  3/3 

Dependency Installed:
  docker-buildx-plugin.x86_64 0:0.8.1-3.el7

Updated:
  docker-ce-cli.x86_64 1:22.04.0-3.el7

Complete!

@crazy-max crazy-max force-pushed the buildx-package branch 2 times, most recently from b740458 to ea44d23 Compare March 26, 2022 20:09
@thaJeztah
Copy link
Member

hmm interesting; looks like something is using the wrong go version perhaps?? (or do we still have a go version hard-coded? perhaps we have in this repo let me check)

./scripts/vendor update
+ go mod tidy -compat=1.17 -modfile=vendor.mod
flag provided but not defined: -compat
usage: go mod tidy [-e] [-v]

@thaJeztah
Copy link
Member

opened #655

@thaJeztah thaJeztah changed the title add docker-buildx-plugin package (deb, rpm, static) [master] add docker-buildx-plugin package (deb, rpm, static) Mar 26, 2022
static/Makefile Outdated
Comment on lines 51 to 52
mkdir -p build/linux/docker/cli-plugins
cp $(BUILDX_DIR)/bin/docker-buildx build/linux/docker/cli-plugins/
Copy link
Member Author

@crazy-max crazy-max Mar 26, 2022

Choose a reason for hiding this comment

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

As discussed, not sure if we should include buildx in the docker-ce-cli static pkg even if it's a recommended component now.

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed for now

@crazy-max crazy-max force-pushed the buildx-package branch 2 times, most recently from 56a7d57 to f46b71d Compare March 27, 2022 09:29
@@ -102,6 +103,14 @@ Description: Rootless support for Docker.
Either VPNKit or slirp4netns (>= 0.4.0) needs to be installed separately.
Homepage: https://docs.docker.com/engine/security/rootless/

Package: docker-buildx-plugin
Priority: optional
Replaces: docker-ce-cli
Copy link
Member

Choose a reason for hiding this comment

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

I initially was confused by the Replaces: here, as I thought it would mark this package to replace the whole docker-ce-cli package, but @crazy-max explained this also can be used to replace some files (given, it's a bit "fuzzy", as it doesn't require speciyfing which files), from the The Debian Administrator's Handbook:

5.2.1.5. Replacing Files: The Replaces Field

The Replaces field indicates that the package contains files that are also present in another package, but that the package is legitimately entitled to replace them. Without this specification, dpkg fails, stating that it can not overwrite the files of another package (technically, it is possible to force it to do so with the --force-overwrite option, but that is not considered standard operation). This allows identification of potential problems and requires the maintainer to study the matter prior to choosing whether to add such a field.

The use of this field is justified when package names change or when a package is included in another. This also happens when the maintainer decides to distribute files differently among various binary packages produced from the same source package: a replaced file no longer belongs to the old package, but only to the new one.

If all of the files in an installed package have been replaced, the package is considered to be removed. Finally, this field also encourages dpkg to remove the replaced package where there is a conflict.

The last paragraph outlines our situation

What we probably could consider doing is (if supported) to include the version here, e.g.

Suggested change
Replaces: docker-ce-cli
Replaces: docker-ce-cli (<< 22.04.0)

But we need to check if it likes CalVer; instead, probably this would work;

Suggested change
Replaces: docker-ce-cli
Replaces: docker-ce-cli (<= 21.10~)

Copy link
Member Author

@crazy-max crazy-max Mar 27, 2022

Choose a reason for hiding this comment

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

Unfortunately I tried and it doesn't work with CalVer. I think it's fine to keep docker-ce-cli as long as the docker-buildx-plugin package only replaces one file from it.

Copy link
Member Author

@crazy-max crazy-max Mar 27, 2022

Choose a reason for hiding this comment

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

About relationships see also https://www.debian.org/doc/debian-policy/ch-relationships.html#overwriting-files-and-replacing-packages-replaces

It is usually an error for a package to contain files which are on the system in another package. However, if the overwriting package declares that it Replaces the one containing the file being overwritten, then dpkg will replace the file from the old package with that from the new. The file will no longer be listed as “owned” by the old package and will be taken over by the new package. Normally, Breaks should be used in conjunction with Replaces.

deb/common/control Outdated Show resolved Hide resolved
@@ -110,6 +107,10 @@ override_dh_install:
dh_apparmor --profile-name=docker-ce -pdocker-ce

override_dh_gencontrol:
# Use separate version for the buildx-plugin package, then generate the other control files as usual
# TODO override "Source" field in control as well (to point to buildx, as it doesn't match the package name)
Copy link
Member

Choose a reason for hiding this comment

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

We should probably (in a follow-up) duplicate the current deb/common directory for each of the packages we build, and split the control file accordingly. That will be a bit of work though (also splitting make targets etc etc)

Ideally, have separate stages in the Dockerfile to collect the source-files (and dependencies) for each, also to get out of the current situation where we bind-mount things and build packages in a docker run (which is horrible, because not only do we lose (e.g.) modules cache, but also currently requires some hacks chown / chmod to be done to adjust file permissions for cleaning up 😞

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah I already have a draft branch for that 👀

@@ -38,6 +38,7 @@ RUN \
yum-builddep --define '_without_btrfs 1' -y /root/rpmbuild/SPECS/*.spec; \
else \
yum-builddep --define '_without_btrfs 1' -y /root/rpmbuild/SPECS/docker-c*.spec; \
yum-builddep --define '_without_btrfs 1' -y /root/rpmbuild/SPECS/docker-b*.spec; \
Copy link
Member

Choose a reason for hiding this comment

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

We probably don't need this line; I think this was specifically using docker-c so that we only defined this for the docker-ce- packages (the engine uses the btrfs build-tag); probably the existing one should even be changed to docker-ce.spec, as docker-ce-cli.spec likely doesn't need it either, but will need to check what we need there.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes let's do that in a follow-up.

thaJeztah
thaJeztah previously approved these changes Mar 27, 2022
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

@thaJeztah
Copy link
Member

oh! you need to update the fedora-36 dockerfile now as well

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

@thaJeztah thaJeztah merged commit f0188d6 into docker:master Mar 27, 2022
@crazy-max crazy-max deleted the buildx-package branch March 27, 2022 15:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Combining static tarball and default cli-plugins
2 participants