enabling using another container's network stack on build process #5088

zedfmario · 2020-12-01T12:41:21Z

Description
Modifying docker network mode validation to accept container:<name|id> network identifier, allowing to reuse the network stack from an existing container (defined by name or id).

User facing changes (remove if N/A)

Before

For a given skaffold.yaml:

apiVersion: skaffold/v2beta9
kind: Config
build:
  artifacts:
  - image: skaffold-example
    docker:
      network: "container:alpine"
deploy:
  kubectl:
    manifests:
      - k8s-*

the output was (described in the issue):

invalid skaffold config: artifact skaffold-example has invalid networkMode 'container:alpine'

After

When there is NO running container with such name

Checking that no alpine container runs in minikube:

> minikube ssh
                         _             _            
            _         _ ( )           ( )           
  ___ ___  (_)  ___  (_)| |/')  _   _ | |_      __  
/' _ ` _ `\| |/' _ `\| || , <  ( ) ( )| '_`\  /'__`\
| ( ) ( ) || || ( ) || || |\`\ | (_) || |_) )(  ___/
(_) (_) (_)(_)(_) (_)(_)(_) (_)`\___/'(_,__/'`\____)

$ docker ps | grep alpine
$ exit

Running skaffold with the previous yaml file:

> skaffold -f skaffold.yaml build
Generating tags...
 - skaffold-example -> skaffold-example:v1.17.0-9-gdcae2d749-dirty
Found [minikube] context, using local docker daemon.
Building [skaffold-example]...
Sending build context to Docker daemon  3.072kB
Step 1/8 : FROM golang:1.12.9-alpine3.10 as builder
 ---> e0d646523991
Step 2/8 : COPY main.go .
 ---> 7d8855ef4f5e
Step 3/8 : ARG SKAFFOLD_GO_GCFLAGS
 ---> Running in d12006f12763
Removing intermediate container d12006f12763
 ---> 476ac0f33bfa
Step 4/8 : RUN go build -gcflags="${SKAFFOLD_GO_GCFLAGS}" -o /app main.go
 ---> Running in acffd1f7b525
Removing intermediate container acffd1f7b525
unable to stream build output: No such container: alpine. Please fix the Dockerfile and try again..

When there is a running container with such name

Starting an alpine container within minikube:

> minikube ssh 
                         _             _            
            _         _ ( )           ( )           
  ___ ___  (_)  ___  (_)| |/')  _   _ | |_      __  
/' _ ` _ `\| |/' _ `\| || , <  ( ) ( )| '_`\  /'__`\
| ( ) ( ) || || ( ) || || |\`\ | (_) || |_) )(  ___/
(_) (_) (_)(_)(_) (_)(_)(_) (_)`\___/'(_,__/'`\____)

$ docker run -d --rm --name alpine alpine tail -f /dev/null
47a7c119285fa482f6c30988eb6901407d5d166622cbfb60c5039a7a7b58da60

when running skaffold:

> skaffold -f skaffold.yaml build
Generating tags...
 - skaffold-example -> skaffold-example:v1.17.0-9-gdcae2d749-dirty
Found [minikube] context, using local docker daemon.
Building [skaffold-example]...
Sending build context to Docker daemon  3.072kB
Step 1/8 : FROM golang:1.12.9-alpine3.10 as builder
 ---> e0d646523991
Step 2/8 : COPY main.go .
 ---> fadbbb6feed0
Step 3/8 : ARG SKAFFOLD_GO_GCFLAGS
 ---> Running in 631e4c80c9e4
Removing intermediate container 631e4c80c9e4
 ---> fc227b6dca3f
Step 4/8 : RUN go build -gcflags="${SKAFFOLD_GO_GCFLAGS}" -o /app main.go
 ---> Running in 87b76b851e49
Removing intermediate container 87b76b851e49
 ---> 71de6b1a8bd2
Step 5/8 : FROM alpine:3.10
 ---> be4e4bea2c2e
Step 6/8 : ENV GOTRACEBACK=single
 ---> Running in 64029361d041
Removing intermediate container 64029361d041
 ---> 090953bcc0b9
Step 7/8 : CMD ["./app"]
 ---> Running in 76a68bb0b4b5
Removing intermediate container 76a68bb0b4b5
 ---> ae291dcfc12a
Step 8/8 : COPY --from=builder /app .
 ---> bb19b9188eb0
Successfully built bb19b9188eb0
Successfully tagged skaffold-example:v1.17.0-9-gdcae2d749-dirty

codecov · 2020-12-01T13:30:58Z

Codecov Report

Merging #5088 (5600d38) into master (272f104) will increase coverage by 0.01%.
The diff coverage is 71.21%.

@@            Coverage Diff             @@
##           master    #5088      +/-   ##
==========================================
+ Coverage   72.19%   72.21%   +0.01%     
==========================================
  Files         380      380              
  Lines       13303    13391      +88     
==========================================
+ Hits         9604     9670      +66     
- Misses       3006     3022      +16     
- Partials      693      699       +6

Impacted Files	Coverage Δ
cmd/skaffold/app/cmd/runner.go	`68.29% <0.00%> (-3.51%)`	⬇️
pkg/skaffold/schema/latest/config.go	`100.00% <ø> (ø)`
pkg/skaffold/schema/validation/validation.go	`89.30% <73.43%> (-6.76%)`	⬇️
pkg/skaffold/yamltags/tags.go	`89.47% <0.00%> (-2.20%)`	⬇️
pkg/skaffold/docker/image.go	`79.34% <0.00%> (-0.20%)`	⬇️
pkg/skaffold/util/store.go	`100.00% <0.00%> (ø)`
pkg/skaffold/docker/context.go	`75.00% <0.00%> (ø)`
pkg/skaffold/build/cluster/kaniko.go	`22.97% <0.00%> (+0.60%)`	⬆️
pkg/skaffold/diagnose/diagnose.go	`54.54% <0.00%> (+0.69%)`	⬆️
pkg/skaffold/docker/dependencies.go	`74.71% <0.00%> (+1.37%)`	⬆️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 272f104...5600d38. Read the comment docs.

pkg/skaffold/schema/validation/validation.go

tejal29

LGTM except 1 question

Building docker images in a Jenkins-like environment running on a AWS EKS cluster secured with `kube2iam` requires the running pod to include an annotation. In our particular case, we share the docker daemon from the node with the running pod by mounting the socket as a volume. When `skaffold` builds a new image within this pod, it creates containers at node level due to the shared socket. Therefore, to have a brand new container handled by `kube2iam`, this container should reuse the pod's network stack –ergo the "pause" container's stack–. Enabling the use of `container:<name|id>` network mode from `docker run` command. It allows a build process to reuse another container's stack. > Described in [here](https://docs.docker.com/engine/reference/run/#network-settings)

According to the `docker` CLI, when running a container with a wrong name, the following output is shown: ``` > docker run -it --name '$$' alpine docker: Error response from daemon: Invalid container name ($$), only [a-zA-Z0-9][a-zA-Z0-9_.-] are allowed. ``` Concatenating `container:` to the expected regular expression `[a-zA-Z0-9][a-zA-Z0-9_.-]` helps us rejecting wrong values.

zedfmario · 2020-12-04T22:41:32Z

@tejal29 I just added a validation process using the runtime context.
Please, take a look.

The idea is as follows

**User facing changes **

When a user tries to use a docker:<id|name> network mode, if the container isn't running the validation process shows an error before starting the build process.

Assuming there's no running container in minikube

> minikube ssh
                         _             _            
            _         _ ( )           ( )           
  ___ ___  (_)  ___  (_)| |/')  _   _ | |_      __  
/' _ ` _ `\| |/' _ `\| || , <  ( ) ( )| '_`\  /'__`\
| ( ) ( ) || || ( ) || || |\`\ | (_) || |_) )(  ___/
(_) (_) (_)(_)(_) (_)(_)(_) (_)`\___/'(_,__/'`\____)

$ docker ps | grep alpine
# no running containers
$ exit

when invoking skaffold we get the error

> skaffold build
invalid skaffold config: container 'alpine' not found. Required by image 'skaffold-example' for docker network stack sharing

After starting a container named 'alpine' in minikube

> minikube ssh
                         _             _            
            _         _ ( )           ( )           
  ___ ___  (_)  ___  (_)| |/')  _   _ | |_      __  
/' _ ` _ `\| |/' _ `\| || , <  ( ) ( )| '_`\  /'__`\
| ( ) ( ) || || ( ) || || |\`\ | (_) || |_) )(  ___/
(_) (_) (_)(_)(_) (_)(_)(_) (_)`\___/'(_,__/'`\____)

$ docker run --name alpine -d alpine tail -f /dev/null
c9bfa667b273c1de7a1b160a2a07f83cb0a7a83924150e5d08225971a3a64b0f
$ docker ps | grep alpine
c9bfa667b273        alpine                            "tail -f /dev/null"      5 seconds ago       Up 4 seconds                                                   alpine
$

Then we can run our build with no problems

> skaffold build
Generating tags...
 - skaffold-example -> skaffold-example:v1.17.1-11-g0474c1e92
Checking cache...
 - skaffold-example: Not found. Building
Found [minikube] context, using local docker daemon.
Building [skaffold-example]...
Sending build context to Docker daemon  3.072kB
Step 1/8 : FROM golang:1.12.9-alpine3.10 as builder
 ---> e0d646523991
Step 2/8 : COPY main.go .
 ---> Using cache
 ---> fadbbb6feed0
Step 3/8 : ARG SKAFFOLD_GO_GCFLAGS
 ---> Using cache
 ---> fc227b6dca3f
Step 4/8 : RUN go build -gcflags="${SKAFFOLD_GO_GCFLAGS}" -o /app main.go
 ---> Using cache
 ---> 71de6b1a8bd2
Step 5/8 : FROM alpine:3.10
 ---> be4e4bea2c2e
Step 6/8 : ENV GOTRACEBACK=single
 ---> Using cache
 ---> 090953bcc0b9
Step 7/8 : CMD ["./app"]
 ---> Using cache
 ---> ae291dcfc12a
Step 8/8 : COPY --from=builder /app .
 ---> Using cache
 ---> bb19b9188eb0
Successfully built bb19b9188eb0
Successfully tagged skaffold-example:v1.17.1-11-g0474c1e92
There is a new version (1.17.1) of Skaffold available. Download it from:
  https://github.com/GoogleContainerTools/skaffold/releases/tag/v1.17.1

Let's try to, by using the local docker daemon. After doing unset on the current-context

> kubeclt config unset current-context
> docker ps | grep alpine 
# no running containers
> skaffold build --cache-artifacts=false
invalid skaffold config: container 'alpine' not found. Required by image 'skaffold-example' for docker network stack sharing

Last but not least, testing the id

> docker run --name whatever_you_want --name foo -d alpine tail -f /dev/null
9067633f45d156d26f4fb1f9bf86482ffbcc5efd8c557bfd8660a10d3d6cb4ac

> cat skaffold.yaml
apiVersion: skaffold/v2beta10
kind: Config
build:
  artifacts:
  - image: skaffold-example
    context: .
    docker:
      network: "container:9067633f45d156d26f4fb1f9bf86482ffbcc5efd8c557bfd8660a10d3d6cb4ac"
deploy:
  kubectl:
    manifests:
      - k8s-*

> skaffold build --cache-artifacts=false                          
Generating tags...
 - skaffold-example -> skaffold-example:v1.17.1-12-g57946e9a3-dirty
Building [skaffold-example]...
Sending build context to Docker daemon  3.072kB
Step 1/8 : FROM golang:1.12.9-alpine3.10 as builder
 ---> e0d646523991
Step 2/8 : COPY main.go .
 ---> Using cache
 ---> 51959c80c02f
Step 3/8 : ARG SKAFFOLD_GO_GCFLAGS
 ---> Using cache
 ---> 048acaefe013
Step 4/8 : RUN go build -gcflags="${SKAFFOLD_GO_GCFLAGS}" -o /app main.go
 ---> Using cache
 ---> c6d6281f5217
Step 5/8 : FROM alpine:3.10
 ---> be4e4bea2c2e
Step 6/8 : ENV GOTRACEBACK=single
 ---> Using cache
 ---> 29368b6f33b0
Step 7/8 : CMD ["./app"]
 ---> Using cache
 ---> e4b5dc1c773e
Step 8/8 : COPY --from=builder /app .
 ---> Using cache
 ---> 061f7f7a9de6
Successfully built 061f7f7a9de6
Successfully tagged skaffold-example:v1.17.1-12-g57946e9a3-dirty

better testing allows to unify methods

zedfmario · 2020-12-07T17:55:08Z

@tejal29
I just made the last change to simplify the validation. Would you mind reviewing the PR again? Thx!!!!

pkg/skaffold/schema/validation/validation.go

lgg42 · 2021-01-29T11:15:42Z

Yesssss!!!

zedfmario requested a review from a team as a code owner December 1, 2020 12:41

zedfmario requested a review from ilya-zuyev December 1, 2020 12:41

google-cla bot added the cla: yes label Dec 1, 2020

pull-request-size bot added the size/M label Dec 1, 2020

zedfmario force-pushed the docker-build-network-container-stack branch 2 times, most recently from b2be10f to 70f9b13 Compare December 3, 2020 15:58

tejal29 reviewed Dec 3, 2020

View reviewed changes

pkg/skaffold/schema/validation/validation.go Show resolved Hide resolved

tejal29 reviewed Dec 3, 2020

View reviewed changes

tejal29 added the kokoro:run runs the kokoro jobs on a PR label Dec 3, 2020

kokoro-team removed the kokoro:run runs the kokoro jobs on a PR label Dec 3, 2020

zedfmario requested a review from tejal29 December 4, 2020 11:52

zedfmario force-pushed the docker-build-network-container-stack branch 2 times, most recently from 8980118 to 0474c1e Compare December 4, 2020 21:01

zedfmario force-pushed the docker-build-network-container-stack branch from 0474c1e to df3e6b3 Compare December 4, 2020 21:26

pull-request-size bot added size/L and removed size/M labels Dec 4, 2020

zedfmario force-pushed the docker-build-network-container-stack branch from 3a8ec00 to f7078b3 Compare December 4, 2020 23:18

runtime validation for docker network stack sharing

3802ebc

zedfmario force-pushed the docker-build-network-container-stack branch 2 times, most recently from 39f12ba to 6081358 Compare December 5, 2020 01:05

overriding docker local daemon in tests

27e3614

zedfmario force-pushed the docker-build-network-container-stack branch from 6081358 to 27e3614 Compare December 5, 2020 10:18

validation with run context unified

cf88666

better testing allows to unify methods

zedfmario force-pushed the docker-build-network-container-stack branch from d3ce994 to cf88666 Compare December 7, 2020 17:04

tejal29 reviewed Dec 8, 2020

View reviewed changes

pkg/skaffold/schema/validation/validation.go Show resolved Hide resolved

tejal29 added the kokoro:run runs the kokoro jobs on a PR label Dec 8, 2020

kokoro-team removed the kokoro:run runs the kokoro jobs on a PR label Dec 8, 2020

adding skaffold error handling

f8e1dc1

zedfmario force-pushed the docker-build-network-container-stack branch from 53d1918 to c38af5c Compare December 9, 2020 09:41

removing unnecessary dependency

5600d38

zedfmario force-pushed the docker-build-network-container-stack branch from c38af5c to 5600d38 Compare December 9, 2020 10:21

tejal29 added the kokoro:force-run forces a kokoro re-run on a PR label Dec 9, 2020

tejal29 approved these changes Dec 9, 2020

View reviewed changes

kokoro-team removed the kokoro:force-run forces a kokoro re-run on a PR label Dec 9, 2020

tejal29 merged commit 1c96651 into GoogleContainerTools:master Dec 9, 2020

zedfmario deleted the docker-build-network-container-stack branch December 9, 2020 18:35

tejal29 mentioned this pull request Dec 9, 2020

Support Mutiple errors codes in Error Framework. #5128

Open

zedfmario mentioned this pull request Dec 10, 2020

Deleting unexpected lines #5131

Merged

tejal29 mentioned this pull request Feb 9, 2021

Document steps to use sErrors.ErrDef class to provide actionable error messages #5375

Merged

zedfmario mentioned this pull request Mar 22, 2021

Adapting validation for docker container network mode to include ENV_VARS #5589

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

enabling using another container's network stack on build process #5088

enabling using another container's network stack on build process #5088

zedfmario commented Dec 1, 2020 •

edited

Loading

codecov bot commented Dec 1, 2020 •

edited

Loading

tejal29 left a comment

zedfmario commented Dec 4, 2020 •

edited

Loading

zedfmario commented Dec 7, 2020

lgg42 commented Jan 29, 2021

enabling using another container's network stack on build process #5088

enabling using another container's network stack on build process #5088

Conversation

zedfmario commented Dec 1, 2020 • edited Loading

Before

After

When there is NO running container with such name

When there is a running container with such name

codecov bot commented Dec 1, 2020 • edited Loading

Codecov Report

tejal29 left a comment

Choose a reason for hiding this comment

zedfmario commented Dec 4, 2020 • edited Loading

zedfmario commented Dec 7, 2020

lgg42 commented Jan 29, 2021

zedfmario commented Dec 1, 2020 •

edited

Loading

codecov bot commented Dec 1, 2020 •

edited

Loading

zedfmario commented Dec 4, 2020 •

edited

Loading