X Tutup
Skip to content

add support for dns nameservers and search domains in stack files#32059

Merged
vdemeester merged 1 commit intomoby:masterfrom
seriousben:support-dns-stack-file
Apr 11, 2017
Merged

add support for dns nameservers and search domains in stack files#32059
vdemeester merged 1 commit intomoby:masterfrom
seriousben:support-dns-stack-file

Conversation

@seriousben
Copy link
Contributor

@seriousben seriousben commented Mar 24, 2017 •
edited
Loading

- What I did

Added support for dns config flags in stack files. (only nameservers and search domain)

version: '3'

services:
  busybox:
    image: busybox:latest
    dns:
      ### 8.8.8.8 is an example
      - 8.8.8.8

- How I did it

docker service create --dns x.x.x.x ... is supported right now but stack files do not support it.
I just simply hooked the ServiceConfig with the ContainerSpec used in ServiceSpec

- How to verify it

  1. docker swarm init

  2. stack deploy behaviour before this PR

$ docker stack deploy -c docker-compose.yml test
Ignoring unsupported options: dns

Creating network testing_default
Creating service testing_busybox
  1. stack inspect
$ docker service inspect testing_busybox
[
    {
        "ID": "lp97yfpg5mnljpaqn67eu8sys",
        "Version": {
            "Index": 1311
        },
        "CreatedAt": "2017-03-23T23:56:10.907936203Z",
        "UpdatedAt": "2017-03-23T23:56:10.921987465Z",
        "Spec": {
            "Name": "testing_busybox",
            "Labels": {
                "com.docker.stack.namespace": "testing"
            },
            "TaskTemplate": {
                "ContainerSpec": {
                    "Image": "busybox:latest@sha256:32f093055929dbc23dec4d03e09dfe971f5973a9ca5cf059cbfb644c206aa83f",
                    "Labels": {
                        "com.docker.stack.namespace": "testing"
                    }
                },
                ...
            },
            ...
        }
    }
]
  1. stack deploy behaviour with this PR
$ docker stack deploy -c docker-compose-test.yml testing
Creating network testing_default
Creating service testing_busybox
  1. stack inspect

Notice the DNSConfig is passed in correctly.

$ docker service inspect testing_busybox
[
    {
        "ID": "93mp786llsintm6i8ua6eix0w",
        "Spec": {
            "Name": "testing_busybox",
            "Labels": {
                "com.docker.stack.namespace": "testing"
            },
            "TaskTemplate": {
                "ContainerSpec": {
                    "Image": "busybox:latest@sha256:32f093055929dbc23dec4d03e09dfe971f5973a9ca5cf059cbfb644c206aa83f",
                    "Labels": {
                        "com.docker.stack.namespace": "testing"
                    },
                    #
                    # DNSConfig are created correctly
                    #
                    "DNSConfig": {
                        "Nameservers": [
                            "8.8.8.8"
                        ]
                    }
                    #
                    #
                },
                ...
              },
           ...
        }
    }
]

- Description for the changelog

add support for dns nameservers and search domains in stack files

- A picture of a cute animal (not mandatory but encouraged)

landseer

Fixes #29685

@GordonTheTurtle GordonTheTurtle added dco/no Automatically set by a bot when one of the commits lacks proper signature status/0-triage labels Mar 24, 2017
@seriousben seriousben changed the title adds support for dns in stack files add support for dns in stack files Mar 24, 2017
@seriousben seriousben force-pushed the support-dns-stack-file branch from b1f94dc to 8d77d27 Compare March 24, 2017 00:06
@GordonTheTurtle GordonTheTurtle removed the dco/no Automatically set by a bot when one of the commits lacks proper signature label Mar 24, 2017
@seriousben seriousben changed the title add support for dns in stack files add support for dns nameservers and search domains in stack files Mar 24, 2017
vdemeester
vdemeester reviewed Mar 26, 2017
View reviewed changes
Copy link
Member

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

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

@dnephin I don't remember if there was a reason for not supporting it (other than not available in the service/create endpoint at that time 👼

Design SGTM otherwise 👍

@vdemeester vdemeester mentioned this pull request Mar 27, 2017
Closed
@seriousben
Copy link
Contributor Author

seriousben commented Mar 27, 2017

Cool. I will go forward with a test and also not sending an empty DNSConfig if no dns options are passed in.

@seriousben seriousben force-pushed the support-dns-stack-file branch 3 times, most recently from 7ddc1d2 to d73eb2b Compare March 27, 2017 16:42
@seriousben seriousben mentioned this pull request Mar 27, 2017
Closed
@dnephin
Copy link
Member

dnephin commented Apr 7, 2017

Design LGTM

@dnephin
Copy link
Member

dnephin commented Apr 7, 2017 •
edited
Loading

I guess they are already in the jsonschema

@seriousben
Copy link
Contributor Author

seriousben commented Apr 7, 2017

I will add a test for the conversion and will ping you when done.

@seriousben
Adding support for dns in stack files
6b60a60 
Signed-off-by: Benjamin Boudreau <boudreau.benjamin@gmail.com>
@seriousben seriousben force-pushed the support-dns-stack-file branch from d73eb2b to 6b60a60 Compare April 11, 2017 00:47
@seriousben
Copy link
Contributor Author

seriousben commented Apr 11, 2017

@dnephin Just added some tests. I believe this PR is ready to go.

vdemeester
vdemeester approved these changes Apr 11, 2017
View reviewed changes
Copy link
Member

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

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

LGTM 🐯

dnephin
dnephin approved these changes Apr 11, 2017
View reviewed changes
Copy link
Member

@dnephin dnephin left a comment

Choose a reason for hiding this comment

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

LGTM

@vdemeester vdemeester merged commit 8a8c196 into moby:master Apr 11, 2017
@seriousben seriousben deleted the support-dns-stack-file branch April 11, 2017 20:26
@thaJeztah thaJeztah added this to the 17.06 milestone Apr 15, 2017
dnephin pushed a commit to dnephin/docker that referenced this pull request Apr 17, 2017
@vdemeester
Merge pull request moby#32059 from seriousben/support-dns-stack-file
c3cf029 
add support for dns nameservers and search domains in stack files
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@vdemeester vdemeester vdemeester approved these changes

@dnephin dnephin dnephin approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@dnephin dnephin

Labels

impact/changelog impact/documentation status/4-merge

Projects

None yet

Milestone

17.06.0

Development

Successfully merging this pull request may close these issues.

DNS property is ignored while creating service via compose file

5 participants

@seriousben @dnephin @vdemeester @thaJeztah @GordonTheTurtle
X Tutup