Browse code

Update `docker stop` and `docker restart` to allow not specifying timeout and use the one specified at container creation time.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>

Yong Tang authored on 2016/06/07 12:29:05
Showing 13 changed files
... ...
@@ -37,10 +37,10 @@ type stateBackend interface {
37 37
 	ContainerPause(name string) error
38 38
 	ContainerRename(oldName, newName string) error
39 39
 	ContainerResize(name string, height, width int) error
40
-	ContainerRestart(name string, seconds int) error
40
+	ContainerRestart(name string, seconds *int) error
41 41
 	ContainerRm(name string, config *types.ContainerRmConfig) error
42 42
 	ContainerStart(name string, hostConfig *container.HostConfig, validateHostname bool, checkpoint string) error
43
-	ContainerStop(name string, seconds int) error
43
+	ContainerStop(name string, seconds *int) error
44 44
 	ContainerUnpause(name string) error
45 45
 	ContainerUpdate(name string, hostConfig *container.HostConfig, validateHostname bool) (types.ContainerUpdateResponse, error)
46 46
 	ContainerWait(name string, timeout time.Duration) (int, error)
... ...
@@ -169,7 +169,14 @@ func (s *containerRouter) postContainersStop(ctx context.Context, w http.Respons
169 169
 		return err
170 170
 	}
171 171
 
172
-	seconds, _ := strconv.Atoi(r.Form.Get("t"))
172
+	var seconds *int
173
+	if tmpSeconds := r.Form.Get("t"); tmpSeconds != "" {
174
+		valSeconds, err := strconv.Atoi(tmpSeconds)
175
+		if err != nil {
176
+			return err
177
+		}
178
+		seconds = &valSeconds
179
+	}
173 180
 
174 181
 	if err := s.backend.ContainerStop(vars["name"], seconds); err != nil {
175 182
 		return err
... ...
@@ -223,9 +230,16 @@ func (s *containerRouter) postContainersRestart(ctx context.Context, w http.Resp
223 223
 		return err
224 224
 	}
225 225
 
226
-	timeout, _ := strconv.Atoi(r.Form.Get("t"))
226
+	var seconds *int
227
+	if tmpSeconds := r.Form.Get("t"); tmpSeconds != "" {
228
+		valSeconds, err := strconv.Atoi(tmpSeconds)
229
+		if err != nil {
230
+			return err
231
+		}
232
+		seconds = &valSeconds
233
+	}
227 234
 
228
-	if err := s.backend.ContainerRestart(vars["name"], timeout); err != nil {
235
+	if err := s.backend.ContainerRestart(vars["name"], seconds); err != nil {
229 236
 		return err
230 237
 	}
231 238
 
... ...
@@ -13,7 +13,8 @@ import (
13 13
 )
14 14
 
15 15
 type restartOptions struct {
16
-	nSeconds int
16
+	nSeconds        int
17
+	nSecondsChanged bool
17 18
 
18 19
 	containers []string
19 20
 }
... ...
@@ -28,6 +29,7 @@ func NewRestartCommand(dockerCli *command.DockerCli) *cobra.Command {
28 28
 		Args:  cli.RequiresMinArgs(1),
29 29
 		RunE: func(cmd *cobra.Command, args []string) error {
30 30
 			opts.containers = args
31
+			opts.nSecondsChanged = cmd.Flags().Changed("time")
31 32
 			return runRestart(dockerCli, &opts)
32 33
 		},
33 34
 	}
... ...
@@ -40,9 +42,14 @@ func NewRestartCommand(dockerCli *command.DockerCli) *cobra.Command {
40 40
 func runRestart(dockerCli *command.DockerCli, opts *restartOptions) error {
41 41
 	ctx := context.Background()
42 42
 	var errs []string
43
+	var timeout *time.Duration
44
+	if opts.nSecondsChanged {
45
+		timeoutValue := time.Duration(opts.nSeconds) * time.Second
46
+		timeout = &timeoutValue
47
+	}
48
+
43 49
 	for _, name := range opts.containers {
44
-		timeout := time.Duration(opts.nSeconds) * time.Second
45
-		if err := dockerCli.Client().ContainerRestart(ctx, name, &timeout); err != nil {
50
+		if err := dockerCli.Client().ContainerRestart(ctx, name, timeout); err != nil {
46 51
 			errs = append(errs, err.Error())
47 52
 		} else {
48 53
 			fmt.Fprintf(dockerCli.Out(), "%s\n", name)
... ...
@@ -13,7 +13,8 @@ import (
13 13
 )
14 14
 
15 15
 type stopOptions struct {
16
-	time int
16
+	time        int
17
+	timeChanged bool
17 18
 
18 19
 	containers []string
19 20
 }
... ...
@@ -28,6 +29,7 @@ func NewStopCommand(dockerCli *command.DockerCli) *cobra.Command {
28 28
 		Args:  cli.RequiresMinArgs(1),
29 29
 		RunE: func(cmd *cobra.Command, args []string) error {
30 30
 			opts.containers = args
31
+			opts.timeChanged = cmd.Flags().Changed("time")
31 32
 			return runStop(dockerCli, &opts)
32 33
 		},
33 34
 	}
... ...
@@ -39,12 +41,17 @@ func NewStopCommand(dockerCli *command.DockerCli) *cobra.Command {
39 39
 
40 40
 func runStop(dockerCli *command.DockerCli, opts *stopOptions) error {
41 41
 	ctx := context.Background()
42
-	timeout := time.Duration(opts.time) * time.Second
42
+
43
+	var timeout *time.Duration
44
+	if opts.timeChanged {
45
+		timeoutValue := time.Duration(opts.time) * time.Second
46
+		timeout = &timeoutValue
47
+	}
43 48
 
44 49
 	var errs []string
45 50
 
46 51
 	errChan := parallelOperation(ctx, opts.containers, func(ctx context.Context, id string) error {
47
-		return dockerCli.Client().ContainerStop(ctx, id, &timeout)
52
+		return dockerCli.Client().ContainerStop(ctx, id, timeout)
48 53
 	})
49 54
 	for _, container := range opts.containers {
50 55
 		if err := <-errChan; err != nil {
... ...
@@ -296,7 +296,7 @@ func (cli *DaemonCli) start(opts daemonOptions) (err error) {
296 296
 	// Wait for serve API to complete
297 297
 	errAPI := <-serveAPIWait
298 298
 	c.Cleanup()
299
-	shutdownDaemon(d, 15)
299
+	shutdownDaemon(d)
300 300
 	containerdRemote.Cleanup()
301 301
 	if errAPI != nil {
302 302
 		return fmt.Errorf("Shutting down due to ServeAPI error: %v", errAPI)
... ...
@@ -342,16 +342,22 @@ func (cli *DaemonCli) stop() {
342 342
 // shutdownDaemon just wraps daemon.Shutdown() to handle a timeout in case
343 343
 // d.Shutdown() is waiting too long to kill container or worst it's
344 344
 // blocked there
345
-func shutdownDaemon(d *daemon.Daemon, timeout time.Duration) {
345
+func shutdownDaemon(d *daemon.Daemon) {
346
+	shutdownTimeout := d.ShutdownTimeout()
346 347
 	ch := make(chan struct{})
347 348
 	go func() {
348 349
 		d.Shutdown()
349 350
 		close(ch)
350 351
 	}()
352
+	if shutdownTimeout < 0 {
353
+		<-ch
354
+		logrus.Debug("Clean shutdown succeeded")
355
+		return
356
+	}
351 357
 	select {
352 358
 	case <-ch:
353 359
 		logrus.Debug("Clean shutdown succeeded")
354
-	case <-time.After(timeout * time.Second):
360
+	case <-time.After(time.Duration(shutdownTimeout) * time.Second):
355 361
 		logrus.Error("Force shutdown daemon")
356 362
 	}
357 363
 }
... ...
@@ -45,8 +45,8 @@ import (
45 45
 const configFileName = "config.v2.json"
46 46
 
47 47
 const (
48
-	// defaultStopTimeout is the timeout (in seconds) for the syscall signal used to stop a container.
49
-	defaultStopTimeout = 10
48
+	// DefaultStopTimeout is the timeout (in seconds) for the syscall signal used to stop a container.
49
+	DefaultStopTimeout = 10
50 50
 )
51 51
 
52 52
 var (
... ...
@@ -588,7 +588,7 @@ func (container *Container) StopTimeout() int {
588 588
 	if container.Config.StopTimeout != nil {
589 589
 		return *container.Config.StopTimeout
590 590
 	}
591
-	return defaultStopTimeout
591
+	return DefaultStopTimeout
592 592
 }
593 593
 
594 594
 // InitDNSHostConfig ensures that the dns fields are never nil.
... ...
@@ -43,8 +43,8 @@ func TestContainerStopTimeout(t *testing.T) {
43 43
 	}
44 44
 
45 45
 	s := c.StopTimeout()
46
-	if s != defaultStopTimeout {
47
-		t.Fatalf("Expected %v, got %v", defaultStopTimeout, s)
46
+	if s != DefaultStopTimeout {
47
+		t.Fatalf("Expected %v, got %v", DefaultStopTimeout, s)
48 48
 	}
49 49
 
50 50
 	stopTimeout := 15
... ...
@@ -25,7 +25,7 @@ type Backend interface {
25 25
 	PullImage(ctx context.Context, image, tag string, metaHeaders map[string][]string, authConfig *types.AuthConfig, outStream io.Writer) error
26 26
 	CreateManagedContainer(config types.ContainerCreateConfig, validateHostname bool) (types.ContainerCreateResponse, error)
27 27
 	ContainerStart(name string, hostConfig *container.HostConfig, validateHostname bool, checkpoint string) error
28
-	ContainerStop(name string, seconds int) error
28
+	ContainerStop(name string, seconds *int) error
29 29
 	ConnectContainerToNetwork(containerName, networkName string, endpointConfig *network.EndpointSettings) error
30 30
 	UpdateContainerServiceConfig(containerName string, serviceConfig *clustertypes.ServiceConfig) error
31 31
 	ContainerInspectCurrent(name string, size bool) (*types.ContainerJSON, error)
... ...
@@ -279,11 +279,12 @@ func (c *containerAdapter) wait(ctx context.Context) error {
279 279
 }
280 280
 
281 281
 func (c *containerAdapter) shutdown(ctx context.Context) error {
282
-	// Default stop grace period to 10s.
283
-	stopgrace := 10
282
+	// Default stop grace period to nil (daemon will use the stopTimeout of the container)
283
+	var stopgrace *int
284 284
 	spec := c.container.spec()
285 285
 	if spec.StopGracePeriod != nil {
286
-		stopgrace = int(spec.StopGracePeriod.Seconds)
286
+		stopgraceValue := int(spec.StopGracePeriod.Seconds)
287
+		stopgrace = &stopgraceValue
287 288
 	}
288 289
 	return c.backend.ContainerStop(c.container.name(), stopgrace)
289 290
 }
... ...
@@ -692,6 +692,8 @@ func NewDaemon(config *Config, registryService registry.Service, containerdRemot
692 692
 }
693 693
 
694 694
 func (daemon *Daemon) shutdownContainer(c *container.Container) error {
695
+	stopTimeout := c.StopTimeout()
696
+
695 697
 	// TODO(windows): Handle docker restart with paused containers
696 698
 	if c.IsPaused() {
697 699
 		// To terminate a process in freezer cgroup, we should send
... ...
@@ -708,8 +710,8 @@ func (daemon *Daemon) shutdownContainer(c *container.Container) error {
708 708
 		if err := daemon.containerUnpause(c); err != nil {
709 709
 			return fmt.Errorf("Failed to unpause container %s with error: %v", c.ID, err)
710 710
 		}
711
-		if _, err := c.WaitStop(time.Duration(c.StopTimeout()) * time.Second); err != nil {
712
-			logrus.Debugf("container %s failed to exit in %d second of SIGTERM, sending SIGKILL to force", c.ID, c.StopTimeout())
711
+		if _, err := c.WaitStop(time.Duration(stopTimeout) * time.Second); err != nil {
712
+			logrus.Debugf("container %s failed to exit in %d second of SIGTERM, sending SIGKILL to force", c.ID, stopTimeout)
713 713
 			sig, ok := signal.SignalMap["KILL"]
714 714
 			if !ok {
715 715
 				return fmt.Errorf("System does not support SIGKILL")
... ...
@@ -721,8 +723,8 @@ func (daemon *Daemon) shutdownContainer(c *container.Container) error {
721 721
 			return err
722 722
 		}
723 723
 	}
724
-	// If container failed to exit in c.StopTimeout() seconds of SIGTERM, then using the force
725
-	if err := daemon.containerStop(c, c.StopTimeout()); err != nil {
724
+	// If container failed to exit in stopTimeout seconds of SIGTERM, then using the force
725
+	if err := daemon.containerStop(c, stopTimeout); err != nil {
726 726
 		return fmt.Errorf("Failed to stop container %s with error: %v", c.ID, err)
727 727
 	}
728 728
 
... ...
@@ -730,6 +732,29 @@ func (daemon *Daemon) shutdownContainer(c *container.Container) error {
730 730
 	return nil
731 731
 }
732 732
 
733
+// ShutdownTimeout returns the shutdown timeout based on the max stopTimeout of the containers
734
+func (daemon *Daemon) ShutdownTimeout() int {
735
+	// By default we use container.DefaultStopTimeout + 5s, which is 15s.
736
+	// TODO (yongtang): Will need to allow shutdown-timeout once #23036 is in place.
737
+	graceTimeout := 5
738
+	shutdownTimeout := container.DefaultStopTimeout + graceTimeout
739
+	if daemon.containers != nil {
740
+		for _, c := range daemon.containers.List() {
741
+			if shutdownTimeout >= 0 {
742
+				stopTimeout := c.StopTimeout()
743
+				if stopTimeout < 0 {
744
+					shutdownTimeout = -1
745
+				} else {
746
+					if stopTimeout+graceTimeout > shutdownTimeout {
747
+						shutdownTimeout = stopTimeout + graceTimeout
748
+					}
749
+				}
750
+			}
751
+		}
752
+	}
753
+	return shutdownTimeout
754
+}
755
+
733 756
 // Shutdown stops the daemon.
734 757
 func (daemon *Daemon) Shutdown() error {
735 758
 	daemon.shutdown = true
... ...
@@ -13,15 +13,20 @@ import (
13 13
 // timeout, ContainerRestart will wait forever until a graceful
14 14
 // stop. Returns an error if the container cannot be found, or if
15 15
 // there is an underlying error at any stage of the restart.
16
-func (daemon *Daemon) ContainerRestart(name string, seconds int) error {
16
+func (daemon *Daemon) ContainerRestart(name string, seconds *int) error {
17 17
 	container, err := daemon.GetContainer(name)
18 18
 	if err != nil {
19 19
 		return err
20 20
 	}
21
-	if err := daemon.containerRestart(container, seconds); err != nil {
21
+	if seconds == nil {
22
+		stopTimeout := container.StopTimeout()
23
+		seconds = &stopTimeout
24
+	}
25
+	if err := daemon.containerRestart(container, *seconds); err != nil {
22 26
 		return fmt.Errorf("Cannot restart container %s: %v", name, err)
23 27
 	}
24 28
 	return nil
29
+
25 30
 }
26 31
 
27 32
 // containerRestart attempts to gracefully stop and then start the
... ...
@@ -16,7 +16,7 @@ import (
16 16
 // will wait for a graceful termination. An error is returned if the
17 17
 // container is not found, is already stopped, or if there is a
18 18
 // problem stopping the container.
19
-func (daemon *Daemon) ContainerStop(name string, seconds int) error {
19
+func (daemon *Daemon) ContainerStop(name string, seconds *int) error {
20 20
 	container, err := daemon.GetContainer(name)
21 21
 	if err != nil {
22 22
 		return err
... ...
@@ -25,7 +25,11 @@ func (daemon *Daemon) ContainerStop(name string, seconds int) error {
25 25
 		err := fmt.Errorf("Container %s is already stopped", name)
26 26
 		return errors.NewErrorWithStatusCode(err, http.StatusNotModified)
27 27
 	}
28
-	if err := daemon.containerStop(container, seconds); err != nil {
28
+	if seconds == nil {
29
+		stopTimeout := container.StopTimeout()
30
+		seconds = &stopTimeout
31
+	}
32
+	if err := daemon.containerStop(container, *seconds); err != nil {
29 33
 		return fmt.Errorf("Cannot stop container %s: %v", name, err)
30 34
 	}
31 35
 	return nil
... ...
@@ -94,6 +94,7 @@ type ContainerOptions struct {
94 94
 	cgroupParent      string
95 95
 	volumeDriver      string
96 96
 	stopSignal        string
97
+	stopTimeout       int
97 98
 	isolation         string
98 99
 	shmSize           string
99 100
 	noHealthcheck     bool
... ...
@@ -106,7 +107,6 @@ type ContainerOptions struct {
106 106
 	init              bool
107 107
 	initPath          string
108 108
 	credentialSpec    string
109
-	stopTimeout       int
110 109
 
111 110
 	Image string
112 111
 	Args  []string
... ...
@@ -146,7 +146,6 @@ func AddFlags(flags *pflag.FlagSet) *ContainerOptions {
146 146
 		ulimits:           NewUlimitOpt(nil),
147 147
 		volumes:           opts.NewListOpts(nil),
148 148
 		volumesFrom:       opts.NewListOpts(nil),
149
-		stopSignal:        flags.String("stop-signal", signal.DefaultStopSignal, fmt.Sprintf("Signal to stop a container, %v by default", signal.DefaultStopSignal)),
150 149
 	}
151 150
 
152 151
 	// General purpose flags
... ...
@@ -163,6 +162,7 @@ func AddFlags(flags *pflag.FlagSet) *ContainerOptions {
163 163
 	flags.BoolVar(&copts.readonlyRootfs, "read-only", false, "Mount the container's root filesystem as read only")
164 164
 	flags.StringVar(&copts.restartPolicy, "restart", "no", "Restart policy to apply when a container exits")
165 165
 	flags.StringVar(&copts.stopSignal, "stop-signal", signal.DefaultStopSignal, fmt.Sprintf("Signal to stop a container, %v by default", signal.DefaultStopSignal))
166
+	flags.IntVar(&copts.stopTimeout, "stop-timeout", 0, "Timeout (in seconds) to stop a container")
166 167
 	flags.Var(copts.sysctls, "sysctl", "Sysctl options")
167 168
 	flags.BoolVarP(&copts.tty, "tty", "t", false, "Allocate a pseudo-TTY")
168 169
 	flags.Var(copts.ulimits, "ulimit", "Ulimit options")
... ...
@@ -561,7 +561,7 @@ func Parse(flags *pflag.FlagSet, copts *ContainerOptions) (*container.Config, *c
561 561
 		config.StopSignal = copts.stopSignal
562 562
 	}
563 563
 	if flags.Changed("stop-timeout") {
564
-		config.StopTimeout = copts.flStopTimeout
564
+		config.StopTimeout = &copts.stopTimeout
565 565
 	}
566 566
 
567 567
 	hostConfig := &container.HostConfig{