Skip to content

Allow parallel branch builds#492

Merged
rosbo merged 2 commits intomasterfrom
allow-parallel-branch-build-ci
Feb 26, 2019
Merged

Allow parallel branch builds#492
rosbo merged 2 commits intomasterfrom
allow-parallel-branch-build-ci

Conversation

@rosbo
Copy link
Contributor

@rosbo rosbo commented Feb 26, 2019

The Jenkins pipeline allows building branches with the -ci suffix.

However, running multiple builds in parallel had a race.

Part of the pipeline is executed on a CPU-only agent and another part is executed on a GPU agent. Docker Images are transferred between these agents using the Registry but all the parallel builds were writing to the same image tag (race).

This ensures that a different Docker image tag is used when building different branches.

@rosbo rosbo requested a review from sebbov February 26, 2019 02:38
@rosbo rosbo force-pushed the allow-parallel-branch-build-ci branch from 3a17bd9 to d6ca2b0 Compare February 26, 2019 17:09
printf 'ERROR: No TAG specified after the %s flag.\n' "$1" >&2
exit
fi
BUILD_ARGS="--build-arg BASE_TAG=$2"
Copy link
Contributor

Choose a reason for hiding this comment

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

You could make this an array of args and use "${BUILD_ARGS[@]}" below?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

CACHE_FLAG=''
;;
-b|--base-image-tag)
if [[ -z $2 ]]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

Using "$2" would allow spaces in $2, however unlikely.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Docker image tags can't have spaces. I can add the "" but it would fail at docker build.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would pick the option that makes the Docker limitation the most clear. E.g. if tag "with space" results in an error saying "Docker image tags can't have spaces", I think that would be preferable. But, leaving it to you if it's worth the additional investigation & testing.


FROM gcr.io/kaggle-images/python-tensorflow-whl:1.12.0-py36 as tensorflow_whl
FROM continuumio/anaconda3:5.2.0
FROM continuumio/anaconda3:${BASE_TAG}
Copy link
Contributor

Choose a reason for hiding this comment

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

Not clear how this works when BASE_TAG is set to ${GIT_BRANCH}-staging (which is the case when we build an image in Jenkins from a non-master branch. How would a continuumio/anaconda3 tag with our branch's commit exist? Can you explain that bit to me pls?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't override the BASE_TAG when building the CPU image. Only the GPU.

I added the option to have the same behavior for both the CPU and GPU build but we don't use it.

Another option is to fail the build script if the -b flag is set w/o the -g but I figured it would be simpler that way and may be useful when I try bumping up the anaconda3 image in the future (I can easily run multiple build without having to update the Dockerfile and easy to track which command built it with which version).

Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM. The value follows the same semantic in both cases (the tag of the base image, as the variable name is clear about).


FROM gcr.io/kaggle-images/python-tensorflow-whl:1.12.0-py36 as tensorflow_whl
FROM continuumio/anaconda3:5.2.0
FROM continuumio/anaconda3:${BASE_TAG}
Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM. The value follows the same semantic in both cases (the tag of the base image, as the variable name is clear about).

CACHE_FLAG=''
;;
-b|--base-image-tag)
if [[ -z $2 ]]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

I would pick the option that makes the Docker limitation the most clear. E.g. if tag "with space" results in an error saying "Docker image tags can't have spaces", I think that would be preferable. But, leaving it to you if it's worth the additional investigation & testing.

@rosbo rosbo merged commit ed0dea4 into master Feb 26, 2019
@rosbo rosbo deleted the allow-parallel-branch-build-ci branch February 26, 2019 18:36
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.

2 participants