Consolidate down to a single Dockerfile #366
Conversation
This change eliminates the duplication between the two Dockerfiles (one for CPU-only instances and one for GPU-enabled instances) by eliminating the second file. The two files were meant to differ in two ways: 1. The base image used (ubuntu vs. nvidia/cuda) 2. The tensorflow package installed (tf-nightly vs. tf-nightly-gpu) It turns out that we can just make both of those arguments (defined by an `ARG` statement with a default value), and then the two files can be merged into one. The default values chosen for the args are the ones for the CPU image, so running `docker build .` without any `--build-arg` flags will produce the CPU image. To build a GPU image, pass in the corresponding values for the `BASE_IMAGE` and `TF_PACKAGE` arguments using `--build-arg` flags. This fixes kubeflow#321
|
/cc inc0 |
|
... Actually, hold off on this for now; the TF_PACKAGE isn't being resolved correctly for some reason. I'll update this PR when it is ready for review. |
| FROM ubuntu:latest | ||
|
|
||
| ARG BASE_IMAGE=ubuntu:latest | ||
| ARG TF_PACKAGE=tf-nightly |
There was a problem hiding this comment.
This ARG should be moved down to right before it is used.
|
I got the TF_PACKAGE issue resolved; this PR should be ready for review now. |
|
This is great. Our release process actually uses this which is invoked by argo here Could you update these so that the release process will still work? |
|
/lgtm |
|
This is great thank you so much. /lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: inc0, Jimexist, jlewi The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This change eliminates the duplication between the two Dockerfiles
(one for CPU-only instances and one for GPU-enabled instances) by
eliminating the second file.
The two files were meant to differ in two ways:
It turns out that we can just make both of those arguments (defined
by an
ARGstatement with a default value), and then the twofiles can be merged into one.
The default values chosen for the args are the ones for the CPU
image, so running
docker build .without any--build-argflagswill produce the CPU image.
To build a GPU image, pass in the corresponding values for the
BASE_IMAGEandTF_PACKAGEarguments using--build-argflags.This fixes #321
This change is