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

Add optional parameter variable to docker build in Makefile #1657

Merged
merged 2 commits into from
Jun 27, 2017
Merged

Add optional parameter variable to docker build in Makefile #1657

merged 2 commits into from
Jun 27, 2017

Conversation

jwflory
Copy link
Contributor

@jwflory jwflory commented Jun 27, 2017

Summary

Add an optional variable ($ISO_DOCKER_EXTRA_ARGS) that can be passed via the command line to add additional parameters to docker build.

Description

This change is helpful for people who might be trying to build a Minikube ISO in a local environment and need to use additional parameters to get it to build successfully. My use case for this is mounting a volume from the host with custom SSL certificates, so the build is able to complete (otherwise it fails when it tries to pull dumb-init from GitHub).

I tested and verified this to work on RHEL 7.3 and Fedora 26. It also works for me with if I pass arguments or not. Ultimately, I think it's a pretty self-explanatory change, but I'm really new to the Kubernetes / Minikube world, so I'm very open to feedback!

Note about docs

As an additional note, I'm more than willing to update the docs with this PR to explain the variable, but wanted to get feedback on the change before doing so.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jun 27, 2017
@minikube-bot
Copy link
Collaborator

Can one of the admins verify this patch?

@r2d4
Copy link
Contributor

r2d4 commented Jun 27, 2017

@minikube-bot ok to test

Makefile Outdated
@@ -98,7 +98,7 @@ out/minikube.iso: $(shell find deploy/iso/minikube-iso -type f)
ifeq ($(IN_DOCKER),1)
$(MAKE) minikube_iso
else
docker run --rm --workdir /mnt --volume $(CURDIR):/mnt \
docker run --rm --workdir /mnt --volume $(CURDIR):/mnt $(DOCKER_EXTRA_ARGS) \
Copy link
Contributor

Choose a reason for hiding this comment

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

Only comment here would be to make it clear that this option is only for the docker build of the iso. Since we have other docker build rules and they are going to become more prominent soon (see #1656)

Maybe $(ISO_DOCKER_EXTRA_ARGS) or something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@r2d4 Yeah, this makes a lot of sense and this proposed name sounds good to me. I'll change this really quick.

Copy link
Contributor

@r2d4 r2d4 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!

@r2d4 r2d4 merged commit 3950629 into kubernetes:master Jun 27, 2017
@jwflory jwflory deleted the change/makefile-docker-parameter branch June 27, 2017 21:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants