Skip to content

Move package versions to ARGs in Dockerfile#40094

Closed
thaJeztah wants to merge 5 commits intomoby:masterfrom
thaJeztah:dockerfile_versions
Closed

Move package versions to ARGs in Dockerfile#40094
thaJeztah wants to merge 5 commits intomoby:masterfrom
thaJeztah:dockerfile_versions

Conversation

@thaJeztah
Copy link
Member

@thaJeztah thaJeztah commented Oct 15, 2019

built on top of #40362

This makes the Dockerfile the cannonical location to specify which version of the dependencies to build.

@thaJeztah thaJeztah force-pushed the dockerfile_versions branch 3 times, most recently from 5aef008 to ae10676 Compare October 16, 2019 01:36
Copy link
Member Author

Choose a reason for hiding this comment

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

Note; we likely need a make target, or alternative script, because I just discovered that the Docker packaging scripts rely on these; https://github.com/docker/docker-ce-packaging/blob/09b3ac8/deb/common/rules#L12-L15

override_dh_auto_build:
	# Build the daemon and dependencies
	cd engine && PRODUCT=docker ./hack/make.sh dynbinary
	cd engine && TMP_GOPATH="/go" hack/dockerfile/install/install.sh tini
	cd engine && TMP_GOPATH="/go" hack/dockerfile/install/install.sh proxy dynamic
	# Build the CLI

Copy link
Contributor

Choose a reason for hiding this comment

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

There's no need to have this || return, since we do set -e in hack/dockerfile/install/install.sh.

Same in other places.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, right; so my linter highlights it, because (obviously) it doesn't know it's being sourced by that file. Perhaps better to repeat the set -e in this file, for situations where it's not called through install.sh but directly

@kolyshkin
Copy link
Contributor

kolyshkin commented Nov 10, 2019

Left a few comments.

The last commit is marked as WIP, so marking the whole PR as WIP for now. Ah, just noticed it's a draft

@tao12345666333
Copy link
Contributor

This change is great! Any update on this?

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah thaJeztah force-pushed the dockerfile_versions branch from cb3dc2b to c2884a7 Compare April 16, 2021 10:30
This makes the Dockerfile the cannonical location
to specify which version of the dependencies to build.

Also fix some linting issues in the install scripts

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Look for environment variables to override versions
in the Dockerfile. This allows, for example:

```
DOCKERCLI_VERSION=19.03.5 \
CONTAINERD_COMMIT=0a1f2b40642e54ed06cd0a22cdf6025cbe70853c \
RUNC_COMMIT=2b52db75279ca687e18156de86d845876e9ef35d \
make shell
```

or

```
CONTAINERD_COMMIT=HEAD RUNC_COMMIT=HEAD make shell
```

or

```
CONTAINERD_COMMIT=master RUNC_COMMIT=master make shell
```

Which produces (inside the container)

```
containerd --version
containerd github.com/containerd/containerd v1.3.0-251-g0a1f2b40 0a1f2b40642e54ed06cd0a22cdf6025cbe70853c

runc --version
runc version 1.0.0-rc9+dev
commit: 2b52db75279ca687e18156de86d845876e9ef35d
spec: 1.0.1-dev

docker --version
Docker version 19.03.5, build 633a0ea
```

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants