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.

Sign up for GitHub

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

Jump to bottom

Add custom DNS settings to service definition #27567

Merged
merged 2 commits into from
Nov 9, 2016
Merged

Add custom DNS settings to service definition #27567

merged 2 commits into from
Nov 9, 2016

Conversation

yongtang
Copy link
Member

@yongtang yongtang commented Oct 20, 2016 •
edited
Loading

- What I did

This fix tries to fix the issue raised in #24391 about allowing custom DNS settings to service definition.

- How I did it

This fix adds DNS, DNSOptions, DNSSearch to service definition, as well as --dns, --dns-opt, and --dns-search to service create.

- How to verify it

An integration test has been added to cover the changes in this fix.

- Description for the changelog

Add --dns, --dns-opt, and dns-search to service create.

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

This fix fixes #24391.

NOTE: A PR in swarmkit has been created separately:
moby/swarmkit#1681

@yongtang yongtang mentioned this pull request Oct 20, 2016
Closed
@yongtang yongtang mentioned this pull request Oct 22, 2016
Merged
@thaJeztah
Copy link
Member

ping @aluzzardi @mrjana @dnephin PTAL

@thaJeztah thaJeztah mentioned this pull request Oct 25, 2016
Open
yongtang added a commit to yongtang/swarmkit that referenced this pull request Oct 26, 2016
@yongtang
This fix tries to fix the issue raised in docker:
7816b7c 
moby/moby#24391
about allowing custom DNS settings to service definition.

This fix adds `DNSConfig`  to service definition.

This fix is related to the pull request:
moby/moby#27567

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
yongtang added a commit to yongtang/swarmkit that referenced this pull request Oct 26, 2016
@yongtang
This fix tries to fix the issue raised in docker:
b3be8a0 
moby/moby#24391
about allowing custom DNS settings to service definition.

This fix adds `DNSConfig`  to service definition.

This fix is related to the pull request:
moby/moby#27567

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
yongtang added a commit to yongtang/docker that referenced this pull request Oct 26, 2016
@yongtang
Revendor swarmkit to 0ec7c6e
bb568f8 
This fix revendor swarmkit to 0ec7c6e.

Related docker PR and issues:
(moby#27567)
(moby#25437)
(moby#26988)
(moby#25644)

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
@yongtang yongtang mentioned this pull request Oct 26, 2016
Merged
@aaronlehmann
Copy link
Contributor

Design LGTM

@thaJeztah thaJeztah added this to the 1.13.0 milestone Oct 26, 2016
@cpuguy83
Copy link
Member

Moving this back to design review because the UX for the flags for create and update need to be decided on.

thaJeztah
thaJeztah reviewed Oct 26, 2016
View reviewed changes
api/types/swarm/container.go
// DNSConfig specifies DNS related configurations in resolver configuration file (resolv.conf)
// Detailed documentation is available in:
// http://man7.org/linux/man-pages/man5/resolv.conf.5.html
// TODO: domain is not supported yet
Copy link
Member

Choose a reason for hiding this comment

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

Can you explain what's not supported yet?

Copy link
Member Author

Choose a reason for hiding this comment

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

@thaJeztah resolv.conf allows to specify domain option:

http://man7.org/linux/man-pages/man5/resolv.conf.5.html

domain Local domain name.

I will update the PR to make domain (not supported) explicit.

@thaJeztah
Copy link
Member

Discussing this; we probably want this on docker service update as well, can you add that to this PR? (will be 6 flags; --dns-add , --dns-rm, etc.)

@yongtang yongtang force-pushed the 24391-dns-setting branch 2 times, most recently from 8df0912 to 8142785 Compare October 27, 2016 16:42
@yongtang
Copy link
Member Author

Thanks all for the review. The PR has been updated with DNS flags for service update. The docs has not been updated yet but will update the PR if we are OK with the implementation.

@thaJeztah
Copy link
Member

oh, needs a rebase already 😞

@yongtang
Copy link
Member Author

yongtang commented Nov 8, 2016

Thanks @aluzzardi @stevvooe @thaJeztah. The PR has been updated with --dns-options.

thaJeztah
thaJeztah approved these changes Nov 9, 2016
View reviewed changes
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

docs LGTM

@vieux
Copy link
Contributor

vieux commented Nov 9, 2016

docs LGTM

@vieux vieux merged commit 462e57f into moby:master Nov 9, 2016
@yongtang yongtang deleted the 24391-dns-setting branch November 9, 2016 01:46
@yongtang yongtang mentioned this pull request Nov 9, 2016
Merged
@thaJeztah
Copy link
Member

Thanks so much @yongtang!

yongtang added a commit to yongtang/docker that referenced this pull request Nov 9, 2016
@yongtang
Add --dns-option to docker run and hide --dns-opt
9b6b3c2 
This fix is a follow up to moby#27567 based on:
moby#27567 (comment)

In moby#27567, `--dns-options` has been added to `service create/update`,
together with `--dns` and `--dns-search`. The `--dns-opt` was used
in `docker run`.

This fix add `--dns-option` (not `--dns-options`) to `docker run/create`, and hide
`--dns-opt`. It is still possible to use `--dns-opt` with
`docker run/create`, though it will not show up in help output.

This fix change `--dns-options`to --dns-option` for `docker service create`
and `docker service update`.

This fix also updates the docs and bash/zsh completion scripts.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
@albers albers mentioned this pull request Nov 13, 2016
Merged
@sdurrheimer sdurrheimer mentioned this pull request Nov 14, 2016
Merged
thaJeztah added a commit to thaJeztah/docker that referenced this pull request Jan 16, 2017
@thaJeztah
Add version annotation to various flags added in 1.13
bbbddee 
Pull request moby#27745 added support for the
client to talk to older versions of the daemon. Various flags were added to
docker 1.13 that are not compatible with older daemons.

This PR adds annotations to those flags, so that they are automatically hidden
if the daemon is older than docker 1.13 (API 1.25).

Not all new flags affect the API (some are client-side only). The following
PR's added new flags to docker 1.13 that affect the API;

- moby#23430 added `--cpu-rt-period`and `--cpu-rt-runtime`
- moby#27800 / moby#25317 added `--group` / `--group-add` / `--group-rm`
- moby#27702 added `--network` to `docker build`
- moby#25962 added `--attachable` to `docker network create`
- moby#27998 added `--compose-file` to `docker stack deploy`
- moby#22566 added `--stop-timeout` to `docker run` and `docker create`
- moby#26061 added `--init` to `docker run` and `docker create`
- moby#26941 added `--init-path` to `docker run` and `docker create`
- moby#27958 added `--cpus` on `docker run` / `docker create`
- moby#27567 added `--dns`, `--dns-opt`, and `--dns-search` to `docker service create`
- moby#27596 added `--force` to `docker service update`
- moby#27857 added `--hostname` to `docker service create`
- moby#28031 added `--hosts`, `--host-add` / `--host-rm` to `docker service create` and `docker service update`
- moby#28076 added `--tty` on `docker service create` / `docker service update`
- moby#26421 added `--update-max-failure-ratio`, `--update-monitor` and `--rollback` on `docker service update`
- moby#27369 added `--health-cmd`, `--health-interval`, `--health-retries`, `--health-timeout` and `--no-healthcheck` options to `docker service create` and `docker service update`

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah thaJeztah mentioned this pull request Jan 16, 2017
Merged
@stevenao
Copy link

using 1.13 for windows, for some reason the container doesn't try to send query to my host's dns now. It won't resolve my private network dns.

dnephin pushed a commit to dnephin/docker that referenced this pull request Apr 17, 2017
@vieux
Merge pull request moby#27567 from yongtang/24391-dns-setting
f0c8571 
Add custom DNS settings to service definition
tiborvass pushed a commit to tiborvass/cli that referenced this pull request May 11, 2017
@yongtang
Add --dns-option to docker run and hide --dns-opt
ac2014c 
This fix is a follow up to #27567 based on:
moby/moby#27567 (comment)

In #27567, `--dns-options` has been added to `service create/update`,
together with `--dns` and `--dns-search`. The `--dns-opt` was used
in `docker run`.

This fix add `--dns-option` (not `--dns-options`) to `docker run/create`, and hide
`--dns-opt`. It is still possible to use `--dns-opt` with
`docker run/create`, though it will not show up in help output.

This fix change `--dns-options`to --dns-option` for `docker service create`
and `docker service update`.

This fix also updates the docs and bash/zsh completion scripts.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
andrewhsu pushed a commit to docker-archive/docker-ce that referenced this pull request May 19, 2017
@yongtang
Add --dns-option to docker run and hide --dns-opt
724a046 
This fix is a follow up to #27567 based on:
moby/moby#27567 (comment)

In #27567, `--dns-options` has been added to `service create/update`,
together with `--dns` and `--dns-search`. The `--dns-opt` was used
in `docker run`.

This fix add `--dns-option` (not `--dns-options`) to `docker run/create`, and hide
`--dns-opt`. It is still possible to use `--dns-opt` with
`docker run/create`, though it will not show up in help output.

This fix change `--dns-options`to --dns-option` for `docker service create`
and `docker service update`.

This fix also updates the docs and bash/zsh completion scripts.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
Upstream-commit: b38ca0f4c3024fb78e86c7a36bc6e624f4c60e18
Component: cli
andrewhsu pushed a commit to docker-archive/docker-ce that referenced this pull request May 19, 2017
@thaJeztah
Add version annotation to various flags added in 1.13
1daa5da 
Pull request moby/moby#27745 added support for the
client to talk to older versions of the daemon. Various flags were added to
docker 1.13 that are not compatible with older daemons.

This PR adds annotations to those flags, so that they are automatically hidden
if the daemon is older than docker 1.13 (API 1.25).

Not all new flags affect the API (some are client-side only). The following
PR's added new flags to docker 1.13 that affect the API;

- moby/moby#23430 added `--cpu-rt-period`and `--cpu-rt-runtime`
- moby/moby#27800 / moby/moby#25317 added `--group` / `--group-add` / `--group-rm`
- moby/moby#27702 added `--network` to `docker build`
- moby/moby#25962 added `--attachable` to `docker network create`
- moby/moby#27998 added `--compose-file` to `docker stack deploy`
- moby/moby#22566 added `--stop-timeout` to `docker run` and `docker create`
- moby/moby#26061 added `--init` to `docker run` and `docker create`
- moby/moby#26941 added `--init-path` to `docker run` and `docker create`
- moby/moby#27958 added `--cpus` on `docker run` / `docker create`
- moby/moby#27567 added `--dns`, `--dns-opt`, and `--dns-search` to `docker service create`
- moby/moby#27596 added `--force` to `docker service update`
- moby/moby#27857 added `--hostname` to `docker service create`
- moby/moby#28031 added `--hosts`, `--host-add` / `--host-rm` to `docker service create` and `docker service update`
- moby/moby#28076 added `--tty` on `docker service create` / `docker service update`
- moby/moby#26421 added `--update-max-failure-ratio`, `--update-monitor` and `--rollback` on `docker service update`
- moby/moby#27369 added `--health-cmd`, `--health-interval`, `--health-retries`, `--health-timeout` and `--no-healthcheck` options to `docker service create` and `docker service update`

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Upstream-commit: 5d2722f83db9e301c6dcbe1c562c2051a52905db
Component: cli
andrewhsu pushed a commit to docker-archive/docker-ce that referenced this pull request Jun 5, 2017
@yongtang
Change to plural forms for help output of docker service update
6336325 
This fix is based on the comment in
moby/moby#27567 (comment)

Basically, in the help output of `docker service update`, the `--xxx-add`
flags typically have plural forms while `--xxx-rm` flags have singular
forms.

This fix updates the help output for consistency.

This fix also updates the related docs in `service_update.md`.
The help output in `service_update.md` has been quite out-of-sync
with the actual output so this fix replaces the output with the
most up-to-date output.

This fix is related to #27567.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
Upstream-commit: e2f650360918aa0573dac59598a92aa2e1cd5887
Component: cli
andrewhsu pushed a commit to docker-archive/docker-ce that referenced this pull request Jun 5, 2017
@yongtang
Add --dns-option to docker run and hide --dns-opt
890a070 
This fix is a follow up to #27567 based on:
moby/moby#27567 (comment)

In #27567, `--dns-options` has been added to `service create/update`,
together with `--dns` and `--dns-search`. The `--dns-opt` was used
in `docker run`.

This fix add `--dns-option` (not `--dns-options`) to `docker run/create`, and hide
`--dns-opt`. It is still possible to use `--dns-opt` with
`docker run/create`, though it will not show up in help output.

This fix change `--dns-options`to --dns-option` for `docker service create`
and `docker service update`.

This fix also updates the docs and bash/zsh completion scripts.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
Upstream-commit: ebacbb50aec7281467daf59980ccb6d81724c050
Component: cli
dnephin pushed a commit to dnephin/cli that referenced this pull request Jun 14, 2017
@yongtang
Change to plural forms for help output of docker service update
e2f6503 
This fix is based on the comment in
moby/moby#27567 (comment)

Basically, in the help output of `docker service update`, the `--xxx-add`
flags typically have plural forms while `--xxx-rm` flags have singular
forms.

This fix updates the help output for consistency.

This fix also updates the related docs in `service_update.md`.
The help output in `service_update.md` has been quite out-of-sync
with the actual output so this fix replaces the output with the
most up-to-date output.

This fix is related to #27567.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
dnephin pushed a commit to dnephin/cli that referenced this pull request Jun 14, 2017
@yongtang
Add --dns-option to docker run and hide --dns-opt
ebacbb5 
This fix is a follow up to #27567 based on:
moby/moby#27567 (comment)

In #27567, `--dns-options` has been added to `service create/update`,
together with `--dns` and `--dns-search`. The `--dns-opt` was used
in `docker run`.

This fix add `--dns-option` (not `--dns-options`) to `docker run/create`, and hide
`--dns-opt`. It is still possible to use `--dns-opt` with
`docker run/create`, though it will not show up in help output.

This fix change `--dns-options`to --dns-option` for `docker service create`
and `docker service update`.

This fix also updates the docs and bash/zsh completion scripts.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dongluochen dongluochen dongluochen left review comments

@aaronlehmann aaronlehmann aaronlehmann left review comments

@sanimej sanimej sanimej left review comments

@dnephin dnephin dnephin left review comments

@aluzzardi aluzzardi aluzzardi left review comments

@stevvooe stevvooe stevvooe left review comments

@thaJeztah thaJeztah thaJeztah approved these changes

Assignees

@sanimej sanimej

Labels
area/networking area/swarm impact/api impact/changelog impact/cli status/4-merge
Projects
None yet
Milestone
1.13.0
Development

Successfully merging this pull request may close these issues.

Swarm mode: service definition lacks custom DNS settings