Add DOCKER_PULL option #283
Conversation
If image in DOCKER_IMAGE has been created locally, docker pull would fail.
|
@miguelprada: Do you have time for a review? |
|
I can get to review this in a few hours hopefully |
| * **DOCKER_BUILD_OPTS** (default: not set): Used do specify additional build options for Docker. | ||
| * **DOCKER_FILE** (default: not set): Instead of pulling an images from the Docker hub, build it from the given path or URL. Please note, this disables the handling of `ROS_REPOSITORY_PATH` and `ROS_DISTRO`, they have to be set in the build file instead. | ||
| * **DOCKER_IMAGE** (default: not set): Selects a Docker images different from default one. Please note, this disables the handling of `ROS_REPOSITORY_PATH` and `ROS_DISTRO` as ROS needs already to be installed in the image. | ||
| * **DOCKER_PULL** (default: true): set to false if custom docker image should not be pulled, e.g. if it was created locally |
There was a problem hiding this comment.
If false, shouldn't some other DOCKER* variable need to be passed? If that is correct, let's add that as a note (if I'm wrong, sorry in advance).
There was a problem hiding this comment.
These have to be passed anyway. It just adds the option to not pull the image before the run.
This can be used to create the image in another Gitlab stage, which then can be reused in other jobs.
There was a problem hiding this comment.
Yeah, but what's the necessary variable set might not be obvious for esp. users new to this feature. Maybe adding a small sample section is a way to go (I think the tests you're adding in this PR can be used there).
There was a problem hiding this comment.
Actually, I have added this just for #279. Most users don' t have to deal with this.
There was a problem hiding this comment.
I can open another PR for doc then.
| @@ -135,6 +135,9 @@ function docker_cp { | |||
| ####################################### | |||
| function ici_docker_build() { | |||
| local opts=($DOCKER_BUILD_OPTS) | |||
| if [ "$DOCKER_PULL" != false ]; then | |||
There was a problem hiding this comment.
By mistake users can pass DOCKER_PULL with invalid values (similar to #296). So it'd be nice for this if logic to be strict.
There was a problem hiding this comment.
For now I don't want to check these cases, because we don't have the proper infrastructure (something like #285)
There was a problem hiding this comment.
Agreed. That is better than repeating to embed same/similar if-else logic.
This PR adds a flag to disable automatic pull of images.
Setting it to false assumes that the (base) image was already pulled or provided otherwise.
Cherry-picked from #279 and extended to include
DOCKER_BASE_IMAGE.I will update #279 and #280 afterwards.
The test jobs are run on gitlab only: https://gitlab.com/ipa-mdl/industrial_ci/pipelines/23079479