Skip to content

v2.1 Add ability to version installations of docker-ce and docker-ce-cli #5

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.

Sign up for GitHub

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

Merged
Fizzadar merged 4 commits into from
Apr 4, 2023
Merged

v2.1 Add ability to version installations of docker-ce and docker-ce-cli #5

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
80b565e
v2.1 Add ability to version installations of docker-ce and docker-ce-cli
Feb 6, 2023
ca64934
Update README and move versioning to global inventory data
Feb 7, 2023
a355b19
Specify version operation for apt/yum
Mar 13, 2023
276073c
Update README.md
ryan109 Mar 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 25 additions & 6 deletions pyinfra_docker/docker.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,23 @@
from pyinfra.operations import apt, dnf, files, yum


def _apt_install():
def get_pkgs_to_install(version):
docker_packages = [
"docker-ce",
"docker-ce-cli"
Copy link
Contributor

@gchazot gchazot Feb 6, 2023

Choose a reason for hiding this comment

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

Probably worth adding a flag to choose whether to install the CLI package, defaulting to False since this was the behaviour before this change.

Copy link
Contributor Author

@ryan109 ryan109 Feb 7, 2023

Choose a reason for hiding this comment

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

I did think about this but that's not true, docker-ce-cli is a dependancy of docker-ce but it looks like docker isn't dependancy managing correctly as docker-ce breaks with docker-ce-cli=>23.0 (from the testing I've done). But just pinning docker-ce installs 23.0 for the cli.

There are other dependancies which didn't give any problems but follow a different versioning so shouldn't be tied to this. docker-ce-rootless-extras seems to follow the same versioning and is part of the docker-ce ecosystem so it might make sense to pin that one as well...

$ sudo apt-get install docker-ce=5:20.10.23~3-0~ubuntu-focal
.....
The following additional packages will be installed:
  containerd.io docker-buildx-plugin docker-ce-cli docker-ce-rootless-extras docker-compose-plugin docker-scan-plugin pigz slirp4netns
Suggested packages:
  aufs-tools cgroupfs-mount | cgroup-lite
The following NEW packages will be installed:
  containerd.io docker-buildx-plugin docker-ce docker-ce-cli docker-ce-rootless-extras docker-compose-plugin docker-scan-plugin pigz slirp4netns
.....
Do you want to continue? [Y/n] y
Get:1 https://mirror.hetzner.com/ubuntu/packages focal/universe amd64 pigz amd64 2.4-1 [57.4 kB]
Get:2 https://mirror.hetzner.com/ubuntu/packages focal/universe amd64 slirp4netns amd64 0.4.3-1 [74.3 kB]
Get:3 https://download.docker.com/linux/ubuntu focal/stable amd64 containerd.io amd64 1.6.16-1 [27.7 MB]   
Get:4 https://download.docker.com/linux/ubuntu focal/stable amd64 docker-buildx-plugin amd64 0.10.2-1~ubuntu.20.04~focal [25.9 MB]
Get:5 https://download.docker.com/linux/ubuntu focal/stable amd64 docker-ce-cli amd64 5:23.0.0-1~ubuntu.20.04~focal [13.2 MB]
Get:6 https://download.docker.com/linux/ubuntu focal/stable amd64 docker-ce amd64 5:20.10.23~3-0~ubuntu-focal [20.5 MB]
Get:7 https://download.docker.com/linux/ubuntu focal/stable amd64 docker-ce-rootless-extras amd64 5:23.0.0-1~ubuntu.20.04~focal [8,765 kB]
Get:8 https://download.docker.com/linux/ubuntu focal/stable amd64 docker-compose-plugin amd64 2.15.1-1~ubuntu.20.04~focal [9,572 kB]
Get:9 https://download.docker.com/linux/ubuntu focal/stable amd64 docker-scan-plugin amd64 0.23.0~ubuntu-focal [3,622 kB]

]
if not version:
return docker_packages

versioned_packages = []
for pkg in docker_packages:
versioned_packages.append(f"{pkg}={version}")

return versioned_packages
Copy link
Contributor

@gchazot gchazot Feb 6, 2023

Choose a reason for hiding this comment

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

Suggested change
versioned_packages = []
for pkg in docker_packages:
versioned_packages.append(f"{pkg}={version}")
return versioned_packages
return [f"{pkg}={version}" for pkg in docker_packages]

Copy link
Contributor Author

@ryan109 ryan109 Feb 7, 2023

Choose a reason for hiding this comment

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

ca64934




def _apt_install(packages):
apt.packages(
name="Install apt requirements to use HTTPS",
packages=["apt-transport-https", "ca-certificates"],
Expand Down Expand Up @@ -40,12 +56,13 @@ def _apt_install():

apt.packages(
name="Install Docker via apt",
packages="docker-ce",
packages=packages,
update=add_apt_repo.changed, # update if we added the repo
allow_downgrades=True,
)


def _yum_or_dnf_install(yum_or_dnf):
def _yum_or_dnf_install(yum_or_dnf, packages):
yum_or_dnf.repo(
name="Add the Docker yum repo",
src="https://download.docker.com/linux/centos/docker-ce.repo",
Expand All @@ -60,25 +77,27 @@ def _yum_or_dnf_install(yum_or_dnf):

yum_or_dnf.packages(
name="Install Docker via yum",
packages=["docker-ce"],
packages=packages,
extra_install_args=extra_install_args,
)


@deploy("Deploy Docker")
def deploy_docker(config=None):
def deploy_docker(config=None, version=None):
"""
Install Docker on the target machine.

Args:
config: filename or dict of JSON data
"""

packages = get_pkgs_to_install(version)
Copy link
Contributor

@gchazot gchazot Feb 6, 2023

Choose a reason for hiding this comment

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

What about making version a host data, maybe with default, instead?

Copy link
Contributor Author

@ryan109 ryan109 Feb 7, 2023

Choose a reason for hiding this comment

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

I don't think we want to force the default to be an older package version for now, while it's having issues for us it might be very swiftly patched and anyone using this will be getting an older package 🙃

Copy link
Contributor

@gchazot gchazot Feb 7, 2023

Choose a reason for hiding this comment

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

The default could be None and the same logic then applies. It would remove the need to pass the version through the functions.
I'm not sure this is the best coding practice (prefer dependency injection over global variables) but it seems to me it would be more in line with the spirit of pyinfra.

Copy link
Contributor Author

@ryan109 ryan109 Feb 7, 2023

Choose a reason for hiding this comment

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

Yeh I see your point!
ca64934

Copy link
Member

@Fizzadar Fizzadar Apr 4, 2023

Choose a reason for hiding this comment

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

I'm not sure this is the best coding practice (prefer dependency injection over global variables) but it seems to me it would be more in line with the spirit of pyinfra.

heh, totally agree with this, the deploy data defaults thing is... not great. But absolutely right thing to do here now.

if host.get_fact(DebPackages):
_apt_install()
_apt_install(packages)
elif host.get_fact(RpmPackages):
_yum_or_dnf_install(
dnf if host.get_fact(Which, command="dnf") else yum,
packages,
)
else:
raise DeployError(
Expand Down
2 changes: 1 addition & 1 deletion setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

if __name__ == "__main__":
setup(
version="2.0",
version="2.1",
name="pyinfra-docker",
description="Install & configure Docker with `pyinfra`.",
long_description=readme,
Expand Down