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

Add "pid" to unsupported options list #768

Merged
merged 1 commit into from Jan 2, 2018

Conversation

Projects
None yet
5 participants
@thaJeztah
Member

thaJeztah commented Dec 27, 2017

Services do not support custom "pid"-modes (e.g. --pid=host, see docker/swarmkit#1605), but this option was ignored silently when deploying a stack.

This patch adds pid to the list of unsupported options so that a warning is printed;

With this patch applied:

$ docker stack deploy -c docker-compose.yml foobar
Ignoring unsupported options: pid

Creating network foobar_default
Creating service foobar_test

- How to verify it

Deploy this docker-compose file, and verify that a warning is printed;

version: '3.4'
services:
    test:
        image: nginx:alpine
        pid: host

- Description for the changelog

Add "pid" to unsupported options list
Services do not support custom "pid"-modes (e.g. `--pid=host`), but this
option was ignored silently when deploying a stack.

This patch adds `pid` to the list of unsupported options so that a warning
is printed;

With this patch applied:

    $ docker stack deploy -c docker-compose.yml foobar
    Ignoring unsupported options: pid

    Creating network foobar_default
    Creating service foobar_test

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@codecov-io

This comment has been minimized.

Show comment
Hide comment
@codecov-io

codecov-io Dec 27, 2017

Codecov Report

Merging #768 into master will decrease coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #768      +/-   ##
==========================================
- Coverage   53.46%   53.46%   -0.01%     
==========================================
  Files         218      218              
  Lines       14642    14642              
==========================================
- Hits         7829     7828       -1     
- Misses       6327     6328       +1     
  Partials      486      486

codecov-io commented Dec 27, 2017

Codecov Report

Merging #768 into master will decrease coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #768      +/-   ##
==========================================
- Coverage   53.46%   53.46%   -0.01%     
==========================================
  Files         218      218              
  Lines       14642    14642              
==========================================
- Hits         7829     7828       -1     
- Misses       6327     6328       +1     
  Partials      486      486
@dnephin

LGTM

@vdemeester

LGTM 🚜

@vdemeester vdemeester merged commit 4a66821 into docker:master Jan 2, 2018

9 checks passed

ci/circleci: cross Your tests passed on CircleCI!
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
ci/circleci: shellcheck Your tests passed on CircleCI!
Details
ci/circleci: test Your tests passed on CircleCI!
Details
ci/circleci: validate Your tests passed on CircleCI!
Details
codecov/patch Coverage not affected when comparing 424fe2b...70a29b4
Details
codecov/project 53.46% (-0.01%) compared to 424fe2b
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
dco-signed All commits are signed

@GordonTheTurtle GordonTheTurtle added this to the 18.01.0 milestone Jan 2, 2018

@thaJeztah thaJeztah deleted the thaJeztah:warn-unsupported-pid branch Jan 2, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment