Skip to content
This repository has been archived by the owner on Nov 30, 2021. It is now read-only.

fix(Dockerfile): ensure apt-get install is prefixed by update #1249

Merged
merged 1 commit into from
Jul 15, 2014

Conversation

mboersma
Copy link
Member

@mboersma mboersma commented Jul 3, 2014

Since Dockerfile layers are cached, having apt-get update as a separate
command creates the potential for a subsequent apt-get install to fail.
Best practices (other than "install from source") seem to be to make
both commands into a one-liner.

This PR ensures we don't apt-get install as its own command anywhere,
removes the installation of some packages that were already in deis/base,
and removes a redundant ENV DEBIAN_FRONTEND statement.

See also #1246 for discussion.

@mboersma mboersma added the bug label Jul 3, 2014
@mboersma mboersma added this to the 0.10.0 milestone Jul 3, 2014
@mboersma mboersma self-assigned this Jul 3, 2014
@carmstrong
Copy link
Contributor

For reference, deis/base installs: make ca-certificates net-tools sudo wget vim strace lsof netcat

@carmstrong
Copy link
Contributor

@mboersma It looks like we should combine apt-get installs in deis/base as well.

RUN echo "deb http://get.docker.io/ubuntu docker main" > /etc/apt/sources.list.d/docker.list
RUN apt-key adv --keyserver keyserver.ubuntu.com --recv-keys 36A1D7869245C8950F966E92D8576A8BA88D21E9
RUN apt-get update -qy
RUN apt-get install -yq lxc-docker-1.0.0
RUN apt-get update && apt-get install -yq lxc-docker-1.0.0
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason we can't move this up into the main install block? I'd prefer to avoid two updates if we can help it. We can add the Docker repository, then do an update and install all packages in one fell swoop.

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, good catch.

Copy link
Member

Choose a reason for hiding this comment

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

👍

@mboersma
Copy link
Member Author

mboersma commented Jul 3, 2014

I also updated the Go install in logger to 1.2.2, because 1.2.1 was no longer available at the speedy Docker-Index-friendly URL.

@carmstrong
Copy link
Contributor

Code LGTM.

Since Dockerfile layers are cached, having `apt-get update` as a separate
command creates the potential for a subsequent `apt-get install` to fail.
Best practices (other than "install from source") seem to be to make
both commands into a one-liner.

This PR ensures we don't `apt-get install` as its own command anywhere,
removes the installation of some packages that were already in deis/base,
and removes a redundant ENV DEBIAN_FRONTEND statement.
@bacongobbler
Copy link
Member

Jenkins, test this please.

mboersma added a commit that referenced this pull request Jul 15, 2014
fix(Dockerfile): ensure `apt-get install` is prefixed by `update`
@mboersma mboersma merged commit ded216f into master Jul 15, 2014
@mboersma mboersma deleted the dockerfile-apt-get-fix branch July 15, 2014 16:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants