Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enabling using another container's network stack on build process #5087

Closed
zedfmario opened this issue Dec 1, 2020 · 3 comments · Fixed by #5088 or #5131
Closed

Enabling using another container's network stack on build process #5087

zedfmario opened this issue Dec 1, 2020 · 3 comments · Fixed by #5088 or #5131
Labels

Comments

@zedfmario
Copy link
Contributor

No logs provided since there's no error to be fixed.

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–.

Expected behavior

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

Actual behavior

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

Information

  • Skaffold version: v1.16.0
  • Operating system: Darwin Kernel Version 19.6.0
  • Contents of skaffold.yaml:
apiVersion: skaffold/v2beta9
kind: Config
build:
  artifacts:
  - image: skaffold-example
    docker:
      network: "container:alpine"
deploy:
  kubectl:
    manifests:
      - k8s-*

Steps to reproduce the behavior

  1. skaffold -f skaffold.yaml build
@tjwebb
Copy link

tjwebb commented Dec 10, 2020

I ran into this issue when trying to run the getting-started example here: https://skaffold.dev/docs/quickstart/.

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

If I remove the lines

    docker:
      network: "container:alpine"

from the included skaffold.yaml then skaffold dev works as expected.

tjwebb referenced this issue Dec 10, 2020
)

* enabling using another container's network stack on build process

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)

* Adapting REGEXP to docker requirements

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.

* runtime validation for docker network stack sharing

* overriding docker local daemon in tests

* validation with run context unified

better testing allows to unify methods

* adding skaffold error handling

* removing unnecessary dependency
@zedfmario
Copy link
Contributor Author

Hi @tjwebb

Sorry for the noise. I committed those changes by mistake. It was a local test.

Those lines shouldn't be there. Fixing it in #5131

@briandealwis
Copy link
Member

@zedfmario's fix has been applied.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants