Introduce new docker strategy for build container
What does this MR do?
The way the build script is executing on the build container is by starting the container with the script as ps1, as soon as the script is done, the container exists.
Follow the same flow the kubernetes executor, where the build container is started in detached mode and the build script are executed on top of the running build container.
This is done so interactive web terminal have the same behavior as kubernetes, where even after the main script is done the user stays connected to the web terminal.
e as kubernetes
and shell
executor
Limitations for DinD
When using docker in docker the --link
does not seem to work at all when we run docker exec
on the container. I have opened a question in https://forums.docker.com/t/cant-access-docker-socket-with-docker-exec/59190 but I might open an issue upstream for this. Even though I'm pretty sure it will not be supported since --link
is deprecated and docker in docker is being phased out.
Why was this MR needed?
For context please read the following issue
Are there points in the code the reviewer needs to double check?
-
docker-ssh
shouldn't need any changes right? - I wonder if we should have tests around this.
- Is there any docuemntation that needs to be updated? I can't find it anywhere
Does this MR meet the acceptance criteria?
-
Documentation created/updated -
Added tests for this feature/bug -
In case of conflicts with master
- branch was rebased
What are the relevant issue numbers?
closes #3605
Merge request reports
Activity
mentioned in merge request !1008 (merged)
@tmaczukin you think you have time to take a look at this? This is preliminary work for the web terminal for docker in #3467 (closed)
If anything is not clear tell me please!
- Resolved by Steve Xuereb
@SteveAzz
This in general looks nice. However, I did not dive deep yet. I think that we should finish Terminal Docker first, as the other MR seems way simpler in terms of complexity.
My biggest takeaway of this work:
- We cannot change today's behavior, so we have to introduce a feature flag (environment variable?) to use a new method,
- Once we introduce we enable that on GitLab.com to measure the impact, and then switch a default to always use
docker/exec/attach
approach, instead of today'sdocker/run/attach
.
My proposal is that we finish the !1008 (merged), even though we know limitations. It seems pretty straightforward otherwise. Then we try to get this MR merged for %11.4 in our best effort.
Edited by Kamil Trzcińskimentioned in issue #3605
added Stretch label and removed Deliverable label
added 1 commit
- 92ac642c - Add DOCKER_STRATEGY variable for docker executor