-
Notifications
You must be signed in to change notification settings - Fork 2k
use proxy vars if set for build #4275
base: master
Are you sure you want to change the base?
use proxy vars if set for build #4275
Conversation
Signed-off-by: Garth Bushell <garth@garthy.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please make the code easier to read
|
||
if [[ $(docker ps -a | grep $DOCKER_CONTAINER_NAME) != "" ]]; then | ||
docker rm -f $DOCKER_CONTAINER_NAME 2>/dev/null | ||
fi | ||
|
||
docker build -t $DOCKER_IMAGE_NAME . | ||
if [[ ${http_proxy:+x} ]]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why use substitution here and not if [[ -n ${http_proxy} ]]
?
DOCKER_BUILD_ARGS="${DOCKER_BUILD_ARGS} --build-arg http_proxy" | ||
fi | ||
|
||
if [[ ${https_proxy:+x} ]]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
DOCKER_BUILD_ARGS="${DOCKER_BUILD_ARGS} --build-arg https_proxy" | ||
fi | ||
|
||
if [[ ${no_proxy:+x} ]]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here
@@ -4,12 +4,25 @@ set -e | |||
|
|||
DOCKER_IMAGE_NAME="docker-machine-build" | |||
DOCKER_CONTAINER_NAME="docker-machine-build-container" | |||
DOCKER_BUILD_ARGS=${DOCKER_BUILD_ARGS:-""} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why default to empty string it's empty?
Use any http_proxy variables that are set for the image build when building in a container
Signed-off-by: Garth Bushell garth@garthy.com