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

Use official docker:dind image instead of custom image #2515

Merged
merged 2 commits into from Feb 21, 2020

Conversation

thaJeztah
Copy link
Member

@thaJeztah thaJeztah commented Feb 14, 2020

This replaces the custom dockerswarm/dind image (https://hub.docker.com/r/dockerswarm/dind) with the official dind images, which should provide the same functionality.

The custom image is build from a private repo (https://github.com/docker/dind), using this dockerfile;

# Dockerfile for building Docker
FROM debian:jessie

# compile and runtime deps
# https://github.com/docker/docker/blob/master/project/PACKAGERS.md#runtime-dependencies
RUN apt-get update && apt-get install -y --no-install-recommends \
        ca-certificates \
        curl \
        iptables \
        procps \
        e2fsprogs \
        xz-utils \
        git \
    && apt-get clean && rm -rf /var/lib/apt/lists/*

ARG VERSION
COPY get_docker.sh /get_docker.sh
RUN bash /get_docker.sh

RUN curl -L -o /dind https://raw.githubusercontent.com/docker/docker/master/hack/dind \
    && chmod +x /dind

VOLUME /var/lib/docker

ENTRYPOINT ["/dind"]

At a glance, I don't see anything in there that the official docker:dind images don't provide, so we might as well use those.

If this works successfully, we can disable the builds of that custom image after this is merged

@thaJeztah
Copy link
Member Author

(hope this works, as the official images enable TLS by default, so possibly some changes could still be needed 🤞)

@thaJeztah

This comment has been minimized.

@thaJeztah
Copy link
Member Author

thaJeztah commented Feb 14, 2020

Ok; looks like one test is failing; at a glance, I suspect it's making some assumptions that are specific to how the previous image was set up (the docker top command is known to produce difference output depending on the distro it's running on);


__________________________ ContainerTopTest.test_top ___________________________
tests/integration/api_container_test.py:1115: in test_top
    assert res['Titles'] == [
E   AssertionError: assert ['PID', 'USER...E', 'COMMAND'] == ['UID', 'PID',...', 'TTY', ...]
E     At index 0 diff: u'PID' != 'UID'
E     Right contains more items, first extra item: 'STIME'
E     Full diff:
E     - [u'PID', u'USER', u'TIME', u'COMMAND']
E     + ['UID', 'PID', 'PPID', 'C', 'STIME', 'TTY', 'TIME', 'CMD']
____________________ ContainerTopTest.test_top_with_psargs _____________________
tests/integration/api_container_test.py:1133: in test_top_with_psargs
    assert res['Titles'] == [
E   AssertionError: assert ['PID', 'USER...E', 'COMMAND'] == ['USER', 'PID'...', 'RSS', ...]
E     At index 0 diff: u'PID' != 'USER'
E     Right contains more items, first extra item: 'VSZ'
E     Full diff:
E     - [u'PID', u'USER', u'TIME', u'COMMAND']
E     + ['USER',
E     +  'PID',
E     +  '%CPU',...
E     
E     ...Full output truncated (9 lines hidden), use '-vv' to show

For example, on Ubuntu, docker top outputs:

docker top b92eb3f7099e
UID                 PID                 PPID                C                   STIME               TTY                 TIME                CMD
www-data            3753                12133               0                   2019                ?                   00:00:05            apache2 -DFOREGROUND

Whereas on Alpine, the output is:

PID                 USER                TIME                COMMAND
44862               root                0:03                dockerd -H tcp://0.0.0.0:2375 --experimental

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This replaces the custom dockerswarm/dind image with the official
dind images, which should provide the same functionality.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
Copy link
Member Author

OK, this is all green now.

@ulyssessouza @chris-crone @shin- PTAL

@thaJeztah
Copy link
Member Author

Looks like the dockerswarm/dind image is also used in CI for classic swarm, so we should update that repository as well before we can fully get rid of that image (e.g. https://github.com/docker/swarm/blob/b9091461c559c357e4017b400e24c07517399f4b/test/integration/Dockerfile)

Copy link
Member

@chris-crone chris-crone left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @thaJeztah !

@chris-crone
Copy link
Member

Looks like the dockerswarm/dind image is also used in CI for classic swarm, so we should update that repository as well before we can fully get rid of that image

@thaJeztah please create an issue so that we can track doing that work 🤗

@thaJeztah
Copy link
Member Author

Opened docker-archive/classicswarm#2973

Copy link
Collaborator

@ulyssessouza ulyssessouza left a comment

Choose a reason for hiding this comment

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

LGTM

@ulyssessouza ulyssessouza merged commit 12fe645 into docker:master Feb 21, 2020
@thaJeztah thaJeztah deleted the use_official_dind_images branch February 21, 2020 15:42
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.

None yet

3 participants