Skip to content

Add support for init on services#37183

Merged
thaJeztah merged 1 commit intomoby:masterfrom
vdemeester:service-create-init
Jun 8, 2018
Merged

Add support for init on services#37183
thaJeztah merged 1 commit intomoby:masterfrom
vdemeester:service-create-init

Conversation

@vdemeester
Copy link
Member

@vdemeester vdemeester commented May 31, 2018

It's already supported by swarmkit, and act the same as
HostConfig.Init on container creation.

Depends on moby/swarmkit#2652
Depends on #37059
Required for docker/cli#479

Signed-off-by: Vincent Demeester [email protected]

Copy link
Member

Choose a reason for hiding this comment

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

Ah, yes, ran into this one as well 😄

@thaJeztah
Copy link
Member

Just so that I don't forget (haven't checked yet);

  • This option needs to be on docker service update as well
  • We need a test to verify that running docker service update without passing the --init option preserves the existing value
  • We need a test to verify that the daemon option (which can set --init so that it's added by default) is used if the --init flag is not passed, on docker service create

@vdemeester vdemeester force-pushed the service-create-init branch from 8178e57 to 52d3c47 Compare May 31, 2018 16:35
@codecov
Copy link

codecov bot commented May 31, 2018

Codecov Report

❗ No coverage uploaded for pull request base (master@1fe0e49). Click here to learn what that means.
The diff coverage is 70.58%.

@@            Coverage Diff            @@
##             master   #37183   +/-   ##
=========================================
  Coverage          ?   34.71%           
=========================================
  Files             ?      609           
  Lines             ?    45001           
  Branches          ?        0           
=========================================
  Hits              ?    15622           
  Misses            ?    27296           
  Partials          ?     2083

@vdemeester
Copy link
Member Author

Updated with a test 👼

@vdemeester vdemeester force-pushed the service-create-init branch 4 times, most recently from afa65f8 to c213698 Compare June 6, 2018 09:16
It's already supported by `swarmkit`, and act the same as
`HostConfig.Init` on container creation.

Signed-off-by: Vincent Demeester <[email protected]>
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.

LGTM, thanks!

@thaJeztah
Copy link
Member

Janky failing on a flaky test; #37132

22:35:10 --- FAIL: TestCreateServiceSecretFileMode (32.28s)
22:35:10 	daemon.go:289: [d826463c0a601] waiting for daemon to start
22:35:10 	daemon.go:321: [d826463c0a601] daemon started
22:35:10 	create_test.go:244: timeout hit after 30s: task count at 1 waiting for 0
22:35:10 	daemon.go:279: [d826463c0a601] exiting daemon

- "process"
- "hyperv"
Init:
description: "Run an init inside the container that forwards signals and reaps processes. This field is omitted if empty, and the default (as configured on the daemon) is used."
Copy link
Contributor

Choose a reason for hiding this comment

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

correction: an init *process*, can be done in followup

Copy link
Contributor

@tiborvass tiborvass left a comment

Choose a reason for hiding this comment

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

LGTM

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants