Blob Blame History Raw
From fdd5cce00433e36feb123a60f2fb447b19b40e59 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Robert-Andr=C3=A9=20Mauchin?= <zebob.m@gmail.com>
Date: Sun, 9 Oct 2022 14:50:08 +0200
Subject: [PATCH 1/3] Revert "backend: add StopOptions to ContainerRestart and
 ContainerStop"

This reverts commit 90de570cfa5f6de0b02bdf2e794627fbf5b0dfc8.
---
 api/server/router/container/backend.go        |   4 +-
 .../router/container/container_routes.go      |  24 +-
 api/types/container/config.go                 |  18 -
 daemon/cluster/executor/backend.go            |   4 +-
 daemon/cluster/executor/container/adapter.go  |  11 +-
 daemon/daemon.go                              |   2 +-
 daemon/delete.go                              |   4 +-
 daemon/restart.go                             |   9 +-
 daemon/stop.go                                |  20 +-
 10 files changed, 29 insertions(+), 67 deletions(-)

diff --git a/api/server/router/container/backend.go b/api/server/router/container/backend.go
index 6145c51de1..395a70f22a 100644
--- a/api/server/router/container/backend.go
+++ b/api/server/router/container/backend.go
@@ -37,10 +37,10 @@ type stateBackend interface {
 	ContainerPause(name string) error
 	ContainerRename(oldName, newName string) error
 	ContainerResize(name string, height, width int) error
-	ContainerRestart(ctx context.Context, name string, options container.StopOptions) error
+	ContainerRestart(name string, seconds *int) error
 	ContainerRm(name string, config *types.ContainerRmConfig) error
 	ContainerStart(name string, hostConfig *container.HostConfig, checkpoint string, checkpointDir string) error
-	ContainerStop(ctx context.Context, name string, options container.StopOptions) error
+	ContainerStop(name string, seconds *int) error
 	ContainerUnpause(name string) error
 	ContainerUpdate(name string, hostConfig *container.HostConfig) (container.ContainerUpdateOKBody, error)
 	ContainerWait(ctx context.Context, name string, condition containerpkg.WaitCondition) (<-chan containerpkg.StateStatus, error)
diff --git a/api/server/router/container/container_routes.go b/api/server/router/container/container_routes.go
index 057a7b0b52..6e4d422d54 100644
--- a/api/server/router/container/container_routes.go
+++ b/api/server/router/container/container_routes.go
@@ -227,22 +227,16 @@ func (s *containerRouter) postContainersStop(ctx context.Context, w http.Respons
 		return err
 	}
 
-	var (
-		options container.StopOptions
-		version = httputils.VersionFromContext(ctx)
-	)
-	if versions.GreaterThanOrEqualTo(version, "1.42") {
-		options.Signal = r.Form.Get("signal")
-	}
+	var seconds *int
 	if tmpSeconds := r.Form.Get("t"); tmpSeconds != "" {
 		valSeconds, err := strconv.Atoi(tmpSeconds)
 		if err != nil {
 			return err
 		}
-		options.Timeout = &valSeconds
+		seconds = &valSeconds
 	}
 
-	if err := s.backend.ContainerStop(ctx, vars["name"], options); err != nil {
+	if err := s.backend.ContainerStop(vars["name"], seconds); err != nil {
 		return err
 	}
 
@@ -280,22 +274,16 @@ func (s *containerRouter) postContainersRestart(ctx context.Context, w http.Resp
 		return err
 	}
 
-	var (
-		options container.StopOptions
-		version = httputils.VersionFromContext(ctx)
-	)
-	if versions.GreaterThanOrEqualTo(version, "1.42") {
-		options.Signal = r.Form.Get("signal")
-	}
+	var seconds *int
 	if tmpSeconds := r.Form.Get("t"); tmpSeconds != "" {
 		valSeconds, err := strconv.Atoi(tmpSeconds)
 		if err != nil {
 			return err
 		}
-		options.Timeout = &valSeconds
+		seconds = &valSeconds
 	}
 
-	if err := s.backend.ContainerRestart(ctx, vars["name"], options); err != nil {
+	if err := s.backend.ContainerRestart(vars["name"], seconds); err != nil {
 		return err
 	}
 
diff --git a/api/types/container/config.go b/api/types/container/config.go
index b073e5dd36..f767195b94 100644
--- a/api/types/container/config.go
+++ b/api/types/container/config.go
@@ -13,24 +13,6 @@ import (
 // Docker interprets it as 3 nanoseconds.
 const MinimumDuration = 1 * time.Millisecond
 
-// StopOptions holds the options to stop or restart a container.
-type StopOptions struct {
-	// Signal (optional) is the signal to send to the container to (gracefully)
-	// stop it before forcibly terminating the container with SIGKILL after the
-	// timeout expires. If not value is set, the default (SIGTERM) is used.
-	Signal string `json:",omitempty"`
-
-	// Timeout (optional) is the timeout (in seconds) to wait for the container
-	// to stop gracefully before forcibly terminating it with SIGKILL.
-	//
-	// - Use nil to use the default timeout (10 seconds).
-	// - Use '-1' to wait indefinitely.
-	// - Use '0' to not wait for the container to exit gracefully, and
-	//   immediately proceeds to forcibly terminating the container.
-	// - Other positive values are used as timeout (in seconds).
-	Timeout *int `json:",omitempty"`
-}
-
 // HealthConfig holds configuration settings for the HEALTHCHECK feature.
 type HealthConfig struct {
 	// Test is the test to perform to check that the container is healthy.
diff --git a/daemon/cluster/executor/backend.go b/daemon/cluster/executor/backend.go
index 89b883ea2f..2187024af7 100644
--- a/daemon/cluster/executor/backend.go
+++ b/daemon/cluster/executor/backend.go
@@ -36,8 +36,8 @@ type Backend interface {
 	ReleaseIngress() (<-chan struct{}, error)
 	CreateManagedContainer(config types.ContainerCreateConfig) (container.CreateResponse, error)
 	ContainerStart(name string, hostConfig *container.HostConfig, checkpoint string, checkpointDir string) error
-	ContainerStop(ctx context.Context, name string, config container.StopOptions) error
-	ContainerLogs(ctx context.Context, name string, config *types.ContainerLogsOptions) (msgs <-chan *backend.LogMessage, tty bool, err error)
+	ContainerStop(name string, seconds *int) error
+	ContainerLogs(context.Context, string, *types.ContainerLogsOptions) (msgs <-chan *backend.LogMessage, tty bool, err error)
 	ConnectContainerToNetwork(containerName, networkName string, endpointConfig *network.EndpointSettings) error
 	ActivateContainerServiceBinding(containerName string) error
 	DeactivateContainerServiceBinding(containerName string) error
diff --git a/daemon/cluster/executor/container/adapter.go b/daemon/cluster/executor/container/adapter.go
index 4ed45526d2..82da8441d3 100644
--- a/daemon/cluster/executor/container/adapter.go
+++ b/daemon/cluster/executor/container/adapter.go
@@ -407,13 +407,14 @@ func (c *containerAdapter) wait(ctx context.Context) (<-chan containerpkg.StateS
 }
 
 func (c *containerAdapter) shutdown(ctx context.Context) error {
-	var options = containertypes.StopOptions{}
 	// Default stop grace period to nil (daemon will use the stopTimeout of the container)
-	if spec := c.container.spec(); spec.StopGracePeriod != nil {
-		timeout := int(spec.StopGracePeriod.Seconds)
-		options.Timeout = &timeout
+	var stopgrace *int
+	spec := c.container.spec()
+	if spec.StopGracePeriod != nil {
+		stopgraceValue := int(spec.StopGracePeriod.Seconds)
+		stopgrace = &stopgraceValue
 	}
-	return c.backend.ContainerStop(ctx, c.container.name(), options)
+	return c.backend.ContainerStop(c.container.name(), stopgrace)
 }
 
 func (c *containerAdapter) terminate(ctx context.Context) error {
diff --git a/daemon/daemon.go b/daemon/daemon.go
index bb09038b63..d6949b1f00 100644
--- a/daemon/daemon.go
+++ b/daemon/daemon.go
@@ -1136,7 +1136,7 @@ func (daemon *Daemon) waitForStartupDone() {
 
 func (daemon *Daemon) shutdownContainer(c *container.Container) error {
 	// If container failed to exit in stopTimeout seconds of SIGTERM, then using the force
-	if err := daemon.containerStop(context.TODO(), c, containertypes.StopOptions{}); err != nil {
+	if err := daemon.containerStop(c, nil); err != nil {
 		return fmt.Errorf("Failed to stop container %s with error: %v", c.ID, err)
 	}
 
diff --git a/daemon/delete.go b/daemon/delete.go
index c2bbbd4196..05536a67d2 100644
--- a/daemon/delete.go
+++ b/daemon/delete.go
@@ -1,7 +1,6 @@
 package daemon // import "github.com/docker/docker/daemon"
 
 import (
-	"context"
 	"fmt"
 	"os"
 	"path"
@@ -9,7 +8,6 @@ import (
 	"time"
 
 	"github.com/docker/docker/api/types"
-	containertypes "github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/container"
 	"github.com/docker/docker/errdefs"
 	"github.com/docker/docker/pkg/containerfs"
@@ -111,7 +109,7 @@ func (daemon *Daemon) cleanupContainer(container *container.Container, config ty
 	// If you arrived here and know the answer, you earned yourself a picture
 	// of a cute animal of your own choosing.
 	var stopTimeout = 3
-	if err := daemon.containerStop(context.TODO(), container, containertypes.StopOptions{Timeout: &stopTimeout}); err != nil {
+	if err := daemon.containerStop(container, &stopTimeout); err != nil {
 		return err
 	}
 
diff --git a/daemon/restart.go b/daemon/restart.go
index c20e8cbb10..5ac271c5e0 100644
--- a/daemon/restart.go
+++ b/daemon/restart.go
@@ -1,7 +1,6 @@
 package daemon // import "github.com/docker/docker/daemon"
 
 import (
-	"context"
 	"fmt"
 
 	containertypes "github.com/docker/docker/api/types/container"
@@ -15,12 +14,12 @@ import (
 // timeout, ContainerRestart will wait forever until a graceful
 // stop. Returns an error if the container cannot be found, or if
 // there is an underlying error at any stage of the restart.
-func (daemon *Daemon) ContainerRestart(ctx context.Context, name string, options containertypes.StopOptions) error {
+func (daemon *Daemon) ContainerRestart(name string, seconds *int) error {
 	ctr, err := daemon.GetContainer(name)
 	if err != nil {
 		return err
 	}
-	err = daemon.containerRestart(ctx, ctr, options)
+	err = daemon.containerRestart(ctr, seconds)
 	if err != nil {
 		return fmt.Errorf("Cannot restart container %s: %v", name, err)
 	}
@@ -32,7 +31,7 @@ func (daemon *Daemon) ContainerRestart(ctx context.Context, name string, options
 // container. When stopping, wait for the given duration in seconds to
 // gracefully stop, before forcefully terminating the container. If
 // given a negative duration, wait forever for a graceful stop.
-func (daemon *Daemon) containerRestart(ctx context.Context, container *container.Container, options containertypes.StopOptions) error {
+func (daemon *Daemon) containerRestart(container *container.Container, seconds *int) error {
 	// Determine isolation. If not specified in the hostconfig, use daemon default.
 	actualIsolation := container.HostConfig.Isolation
 	if containertypes.Isolation.IsDefault(actualIsolation) {
@@ -57,7 +56,7 @@ func (daemon *Daemon) containerRestart(ctx context.Context, container *container
 		autoRemove := container.HostConfig.AutoRemove
 
 		container.HostConfig.AutoRemove = false
-		err := daemon.containerStop(ctx, container, options)
+		err := daemon.containerStop(container, seconds)
 		// restore AutoRemove irrespective of whether the stop worked or not
 		container.HostConfig.AutoRemove = autoRemove
 		// containerStop will write HostConfig to disk, we shall restore AutoRemove
diff --git a/daemon/stop.go b/daemon/stop.go
index 5a659c1cc8..9caf42cc95 100644
--- a/daemon/stop.go
+++ b/daemon/stop.go
@@ -4,10 +4,8 @@ import (
 	"context"
 	"time"
 
-	containertypes "github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/container"
 	"github.com/docker/docker/errdefs"
-	"github.com/moby/sys/signal"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 )
@@ -20,7 +19,7 @@ import (
 // If the timeout is nil, the container's StopTimeout value is used, if set,
 // otherwise the engine default. A negative timeout value can be specified,
 // meaning no timeout, i.e. no forceful termination is performed.
-func (daemon *Daemon) ContainerStop(ctx context.Context, name string, options containertypes.StopOptions) error {
+func (daemon *Daemon) ContainerStop(name string, timeout *int) error {
 	ctr, err := daemon.GetContainer(name)
 	if err != nil {
 		return err
@@ -28,7 +27,7 @@ func (daemon *Daemon) ContainerStop(ctx context.Context, name string, options co
 	if !ctr.IsRunning() {
 		return containerNotModifiedError{}
 	}
-	err = daemon.containerStop(ctx, ctr, options)
+	err = daemon.containerStop(ctr, timeout)
 	if err != nil {
 		return errdefs.System(errors.Wrapf(err, "cannot stop container: %s", name))
 	}
@@ -36,7 +35,9 @@ func (daemon *Daemon) ContainerStop(ctx context.Context, name string, options co
 }
 
 // containerStop sends a stop signal, waits, sends a kill signal.
-func (daemon *Daemon) containerStop(ctx context.Context, ctr *container.Container, options containertypes.StopOptions) (retErr error) {
+func (daemon *Daemon) containerStop(ctr *container.Container, seconds *int) (retErr error) {
+	// TODO propagate a context down to this function
+	ctx := context.TODO()
 	if !ctr.IsRunning() {
 		return nil
 	}
@@ -45,15 +46,8 @@ func (daemon *Daemon) containerStop(ctx context.Context, ctr *container.Containe
 		stopSignal  = ctr.StopSignal()
 		stopTimeout = ctr.StopTimeout()
 	)
-	if options.Signal != "" {
-		sig, err := signal.ParseSignal(options.Signal)
-		if err != nil {
-			return errdefs.InvalidParameter(err)
-		}
-		stopSignal = sig
-	}
-	if options.Timeout != nil {
-		stopTimeout = *options.Timeout
+	if seconds != nil {
+		stopTimeout = *seconds
 	}
 
 	var wait time.Duration
-- 
2.37.3