Merge component 'engine' from git@github.com:moby/moby master

components/engine/api/server/router/container/container_routes.go

@@ -280,11 +280,12 @@ func (s *containerRouter) postContainersWait(ctx context.Context, w http.Respons
 	// Behavior changed in version 1.30 to handle wait condition and to
 	// return headers immediately.
 	version := httputils.VersionFromContext(ctx)
-	legacyBehavior := versions.LessThan(version, "1.30")
+	legacyBehaviorPre130 := versions.LessThan(version, "1.30")
+	legacyRemovalWaitPre134 := false
 
 	// The wait condition defaults to "not-running".
 	waitCondition := containerpkg.WaitConditionNotRunning
-	if !legacyBehavior {
+	if !legacyBehaviorPre130 {
 		if err := httputils.ParseForm(r); err != nil {
 			return err
 		}
@@ -293,6 +294,7 @@ func (s *containerRouter) postContainersWait(ctx context.Context, w http.Respons
 			waitCondition = containerpkg.WaitConditionNextExit
 		case container.WaitConditionRemoved:
 			waitCondition = containerpkg.WaitConditionRemoved
+			legacyRemovalWaitPre134 = versions.LessThan(version, "1.34")
 		}
 	}
 
@@ -306,7 +308,7 @@ func (s *containerRouter) postContainersWait(ctx context.Context, w http.Respons
 
 	w.Header().Set("Content-Type", "application/json")
 
-	if !legacyBehavior {
+	if !legacyBehaviorPre130 {
 		// Write response header immediately.
 		w.WriteHeader(http.StatusOK)
 		if flusher, ok := w.(http.Flusher); ok {
@@ -317,8 +319,22 @@ func (s *containerRouter) postContainersWait(ctx context.Context, w http.Respons
 	// Block on the result of the wait operation.
 	status := <-waitC
 
+	// With API < 1.34, wait on WaitConditionRemoved did not return
+	// in case container removal failed. The only way to report an
+	// error back to the client is to not write anything (i.e. send
+	// an empty response which will be treated as an error).
+	if legacyRemovalWaitPre134 && status.Err() != nil {
+		return nil
+	}
+
+	var waitError *container.ContainerWaitOKBodyError
+	if status.Err() != nil {
+		waitError = &container.ContainerWaitOKBodyError{Message: status.Err().Error()}
+	}
+
 	return json.NewEncoder(w).Encode(&container.ContainerWaitOKBody{
 		StatusCode: int64(status.ExitCode()),
+		Error:      waitError,
 	})
 }
 

components/engine/api/swagger.yaml

@@ -5723,6 +5723,13 @@ paths:
                 description: "Exit code of the container"
                 type: "integer"
                 x-nullable: false
+              Error:
+                description: "container waiting error, if any"
+                type: "object"
+                properties:
+                  Message:
+                    description: "Details of an error"
+                    type: "string"
         404:
           description: "no such container"
           schema:

components/engine/api/types/container/container_wait.go

@@ -7,10 +7,22 @@ package container
 // See hack/generate-swagger-api.sh
 // ----------------------------------------------------------------------------
 
+// ContainerWaitOKBodyError container waiting error, if any
+// swagger:model ContainerWaitOKBodyError
+type ContainerWaitOKBodyError struct {
+
+	// Details of an error
+	Message string `json:"Message,omitempty"`
+}
+
 // ContainerWaitOKBody container wait o k body
 // swagger:model ContainerWaitOKBody
 type ContainerWaitOKBody struct {
 
+	// error
+	// Required: true
+	Error *ContainerWaitOKBodyError `json:"Error"`
+
 	// Exit code of the container
 	// Required: true
 	StatusCode int64 `json:"StatusCode"`

components/engine/container/state.go

@@ -29,7 +29,7 @@ type State struct {
 	Dead              bool
 	Pid               int
 	ExitCodeValue     int    `json:"ExitCode"`
-	ErrorMsg          string `json:"Error"` // contains last known error when starting the container
+	ErrorMsg          string `json:"Error"` // contains last known error during container start or remove
 	StartedAt         time.Time
 	FinishedAt        time.Time
 	Health            *Health
@@ -326,7 +326,10 @@ func (s *State) SetRestarting(exitStatus *ExitStatus) {
 // know the error that occurred when container transits to another state
 // when inspecting it
 func (s *State) SetError(err error) {
-	s.ErrorMsg = err.Error()
+	s.ErrorMsg = ""
+	if err != nil {
+		s.ErrorMsg = err.Error()
+	}
 }
 
 // IsPaused returns whether the container is paused or not.
@@ -392,8 +395,18 @@ func (s *State) IsDead() bool {
 // closes the internal waitRemove channel to unblock callers waiting for a
 // container to be removed.
 func (s *State) SetRemoved() {
+	s.SetRemovalError(nil)
+}
+
+// SetRemovalError is to be called in case a container remove failed.
+// It sets an error and closes the internal waitRemove channel to unblock
+// callers waiting for the container to be removed.
+func (s *State) SetRemovalError(err error) {
+	s.SetError(err)
 	s.Lock()
 	close(s.waitRemove) // Unblock those waiting on remove.
+	// Recreate the channel so next ContainerWait will work
+	s.waitRemove = make(chan struct{})
 	s.Unlock()
 }
 

components/engine/daemon/delete.go

@@ -120,12 +120,16 @@ func (daemon *Daemon) cleanupContainer(container *container.Container, forceRemo
 		metadata, err := daemon.stores[container.OS].layerStore.ReleaseRWLayer(container.RWLayer)
 		layer.LogReleaseMetadata(metadata)
 		if err != nil && err != layer.ErrMountDoesNotExist && !os.IsNotExist(errors.Cause(err)) {
-			return errors.Wrapf(err, "driver %q failed to remove root filesystem for %s", daemon.GraphDriverName(container.OS), container.ID)
+			e := errors.Wrapf(err, "driver %q failed to remove root filesystem for %s", daemon.GraphDriverName(container.OS), container.ID)
+			container.SetRemovalError(e)
+			return e
 		}
 	}
 
 	if err := system.EnsureRemoveAll(container.Root); err != nil {
-		return errors.Wrapf(err, "unable to remove filesystem for %s", container.ID)
+		e := errors.Wrapf(err, "unable to remove filesystem for %s", container.ID)
+		container.SetRemovalError(e)
+		return e
 	}
 
 	linkNames := daemon.linkIndex.delete(container)

components/engine/daemon/graphdriver/devmapper/driver.go

@@ -73,19 +73,14 @@ func (d *Driver) Status() [][2]string {
 		{"Pool Blocksize", units.HumanSize(float64(s.SectorSize))},
 		{"Base Device Size", units.HumanSize(float64(s.BaseDeviceSize))},
 		{"Backing Filesystem", s.BaseDeviceFS},
-		{"Data file", s.DataFile},
-		{"Metadata file", s.MetadataFile},
-		{"Data Space Used", units.HumanSize(float64(s.Data.Used))},
-		{"Data Space Total", units.HumanSize(float64(s.Data.Total))},
-		{"Data Space Available", units.HumanSize(float64(s.Data.Available))},
-		{"Metadata Space Used", units.HumanSize(float64(s.Metadata.Used))},
-		{"Metadata Space Total", units.HumanSize(float64(s.Metadata.Total))},
-		{"Metadata Space Available", units.HumanSize(float64(s.Metadata.Available))},
-		{"Thin Pool Minimum Free Space", units.HumanSize(float64(s.MinFreeSpace))},
 		{"Udev Sync Supported", fmt.Sprintf("%v", s.UdevSyncSupported)},
-		{"Deferred Removal Enabled", fmt.Sprintf("%v", s.DeferredRemoveEnabled)},
-		{"Deferred Deletion Enabled", fmt.Sprintf("%v", s.DeferredDeleteEnabled)},
-		{"Deferred Deleted Device Count", fmt.Sprintf("%v", s.DeferredDeletedDeviceCount)},
+	}
+
+	if len(s.DataFile) > 0 {
+		status = append(status, [2]string{"Data file", s.DataFile})
+	}
+	if len(s.MetadataFile) > 0 {
+		status = append(status, [2]string{"Metadata file", s.MetadataFile})
 	}
 	if len(s.DataLoopback) > 0 {
 		status = append(status, [2]string{"Data loop file", s.DataLoopback})
@@ -93,6 +88,20 @@ func (d *Driver) Status() [][2]string {
 	if len(s.MetadataLoopback) > 0 {
 		status = append(status, [2]string{"Metadata loop file", s.MetadataLoopback})
 	}
+
+	status = append(status, [][2]string{
+		{"Data Space Used", units.HumanSize(float64(s.Data.Used))},
+		{"Data Space Total", units.HumanSize(float64(s.Data.Total))},
+		{"Data Space Available", units.HumanSize(float64(s.Data.Available))},
+		{"Metadata Space Used", units.HumanSize(float64(s.Metadata.Used))},
+		{"Metadata Space Total", units.HumanSize(float64(s.Metadata.Total))},
+		{"Metadata Space Available", units.HumanSize(float64(s.Metadata.Available))},
+		{"Thin Pool Minimum Free Space", units.HumanSize(float64(s.MinFreeSpace))},
+		{"Deferred Removal Enabled", fmt.Sprintf("%v", s.DeferredRemoveEnabled)},
+		{"Deferred Deletion Enabled", fmt.Sprintf("%v", s.DeferredDeleteEnabled)},
+		{"Deferred Deleted Device Count", fmt.Sprintf("%v", s.DeferredDeletedDeviceCount)},
+	}...)
+
 	if vStr, err := devicemapper.GetLibraryVersion(); err == nil {
 		status = append(status, [2]string{"Library Version", vStr})
 	}

components/engine/docs/api/version-history.md

@@ -17,12 +17,19 @@ keywords: "API, Docker, rcli, REST, documentation"
 
 [Docker Engine API v1.34](https://docs.docker.com/engine/api/v1.34/) documentation
 
+* `POST /containers/(name)/wait?condition=removed` now also also returns
+  in case of container removal failure. A pointer to a structure named
+  `Error` added to the response JSON in order to indicate a failure.
+  If `Error` is `null`, container removal has succeeded, otherwise
+  the test of an error message indicating why container removal has failed
+  is available from `Error.Message` field.
+
 ## v1.33 API changes
 
 [Docker Engine API v1.33](https://docs.docker.com/engine/api/v1.33/) documentation
 
 * `GET /events` now supports filtering 4 more kinds of events: `config`, `node`,
-`secret` and `service`. 
+`secret` and `service`.
 
 ## v1.32 API changes
 

components/engine/integration-cli/cli/build/fakestorage/storage.go

@@ -14,7 +14,7 @@ import (
 	"github.com/docker/docker/integration-cli/cli/build/fakecontext"
 	"github.com/docker/docker/integration-cli/request"
 	"github.com/docker/docker/internal/test/environment"
-	"github.com/docker/docker/pkg/stringutils"
+	"github.com/docker/docker/internal/testutil"
 	"github.com/stretchr/testify/require"
 )
 
@@ -124,8 +124,8 @@ func (f *remoteFileServer) Close() error {
 
 func newRemoteFileServer(t testingT, ctx *fakecontext.Fake) *remoteFileServer {
 	var (
-		image     = fmt.Sprintf("fileserver-img-%s", strings.ToLower(stringutils.GenerateRandomAlphaOnlyString(10)))
-		container = fmt.Sprintf("fileserver-cnt-%s", strings.ToLower(stringutils.GenerateRandomAlphaOnlyString(10)))
+		image     = fmt.Sprintf("fileserver-img-%s", strings.ToLower(testutil.GenerateRandomAlphaOnlyString(10)))
+		container = fmt.Sprintf("fileserver-cnt-%s", strings.ToLower(testutil.GenerateRandomAlphaOnlyString(10)))
 	)
 
 	ensureHTTPServerImage(t)

components/engine/integration-cli/docker_cli_build_test.go

@@ -23,8 +23,8 @@ import (
 	"github.com/docker/docker/integration-cli/cli/build/fakecontext"
 	"github.com/docker/docker/integration-cli/cli/build/fakegit"
 	"github.com/docker/docker/integration-cli/cli/build/fakestorage"
+	"github.com/docker/docker/internal/testutil"
 	"github.com/docker/docker/pkg/archive"
-	"github.com/docker/docker/pkg/stringutils"
 	"github.com/go-check/check"
 	"github.com/gotestyourself/gotestyourself/icmd"
 	digest "github.com/opencontainers/go-digest"
@@ -3185,7 +3185,7 @@ func (s *DockerSuite) TestBuildOnBuildOutput(c *check.C) {
 
 // FIXME(vdemeester) should be a unit test
 func (s *DockerSuite) TestBuildInvalidTag(c *check.C) {
-	name := "abcd:" + stringutils.GenerateRandomAlphaOnlyString(200)
+	name := "abcd:" + testutil.GenerateRandomAlphaOnlyString(200)
 	buildImage(name, build.WithDockerfile("FROM "+minimalBaseImage()+"\nMAINTAINER quux\n")).Assert(c, icmd.Expected{
 		ExitCode: 125,
 		Err:      "invalid reference format",

components/engine/integration-cli/docker_cli_run_test.go

@@ -26,10 +26,10 @@ import (
 	"github.com/docker/docker/integration-cli/cli"
 	"github.com/docker/docker/integration-cli/cli/build"
 	"github.com/docker/docker/integration-cli/cli/build/fakecontext"
+	"github.com/docker/docker/internal/testutil"
 	"github.com/docker/docker/pkg/mount"
 	"github.com/docker/docker/pkg/parsers/kernel"
 	"github.com/docker/docker/pkg/stringid"
-	"github.com/docker/docker/pkg/stringutils"
 	"github.com/docker/docker/runconfig"
 	"github.com/docker/go-connections/nat"
 	"github.com/docker/libnetwork/resolvconf"
@@ -1828,7 +1828,7 @@ func testRunWriteSpecialFilesAndNotCommit(c *check.C, name, path string) {
 }
 
 func eqToBaseDiff(out string, c *check.C) bool {
-	name := "eqToBaseDiff" + stringutils.GenerateRandomAlphaOnlyString(32)
+	name := "eqToBaseDiff" + testutil.GenerateRandomAlphaOnlyString(32)
 	dockerCmd(c, "run", "--name", name, "busybox", "echo", "hello")
 	cID := getIDByName(c, name)
 	baseDiff, _ := dockerCmd(c, "diff", cID)

components/engine/integration-cli/docker_cli_tag_test.go

@@ -6,8 +6,8 @@ import (
 
 	"github.com/docker/docker/integration-cli/checker"
 	"github.com/docker/docker/integration-cli/cli/build"
+	"github.com/docker/docker/internal/testutil"
 	"github.com/docker/docker/pkg/stringid"
-	"github.com/docker/docker/pkg/stringutils"
 	"github.com/go-check/check"
 )
 
@@ -34,7 +34,7 @@ func (s *DockerSuite) TestTagInvalidUnprefixedRepo(c *check.C) {
 
 // ensure we don't allow the use of invalid tags; these tag operations should fail
 func (s *DockerSuite) TestTagInvalidPrefixedRepo(c *check.C) {
-	longTag := stringutils.GenerateRandomAlphaOnlyString(121)
+	longTag := testutil.GenerateRandomAlphaOnlyString(121)
 
 	invalidTags := []string{"repo:fo$z$", "repo:Foo@3cc", "repo:Foo$3", "repo:Foo*3", "repo:Fo^3", "repo:Foo!3", "repo:%goodbye", "repo:#hashtagit", "repo:F)xcz(", "repo:-foo", "repo:..", longTag}
 

components/engine/integration-cli/utils_test.go

@@ -7,7 +7,7 @@ import (
 	"path/filepath"
 	"strings"
 
-	"github.com/docker/docker/pkg/stringutils"
+	"github.com/docker/docker/internal/testutil"
 	"github.com/go-check/check"
 	"github.com/gotestyourself/gotestyourself/icmd"
 	"github.com/pkg/errors"
@@ -60,7 +60,7 @@ func RandomTmpDirPath(s string, platform string) string {
 	if platform == "windows" {
 		tmp = os.Getenv("TEMP")
 	}
-	path := filepath.Join(tmp, fmt.Sprintf("%s.%s", s, stringutils.GenerateRandomAlphaOnlyString(10)))
+	path := filepath.Join(tmp, fmt.Sprintf("%s.%s", s, testutil.GenerateRandomAlphaOnlyString(10)))
 	if platform == "windows" {
 		return filepath.FromSlash(path) // Using \
 	}

components/engine/internal/testutil/stringutils.go

@@ -0,0 +1,14 @@
+package testutil
+
+import "math/rand"
+
+// GenerateRandomAlphaOnlyString generates an alphabetical random string with length n.
+func GenerateRandomAlphaOnlyString(n int) string {
+	// make a really long string
+	letters := []byte("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ")
+	b := make([]byte, n)
+	for i := range b {
+		b[i] = letters[rand.Intn(len(letters))]
+	}
+	return string(b)
+}

components/engine/internal/testutil/stringutils_test.go

@@ -0,0 +1,33 @@
+package testutil
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func testLengthHelper(generator func(int) string, t *testing.T) {
+	expectedLength := 20
+	s := generator(expectedLength)
+	assert.Equal(t, expectedLength, len(s))
+}
+
+func testUniquenessHelper(generator func(int) string, t *testing.T) {
+	repeats := 25
+	set := make(map[string]struct{}, repeats)
+	for i := 0; i < repeats; i = i + 1 {
+		str := generator(64)
+		assert.Equal(t, 64, len(str))
+		_, ok := set[str]
+		assert.False(t, ok, "Random number is repeated")
+		set[str] = struct{}{}
+	}
+}
+
+func TestGenerateRandomAlphaOnlyStringLength(t *testing.T) {
+	testLengthHelper(GenerateRandomAlphaOnlyString, t)
+}
+
+func TestGenerateRandomAlphaOnlyStringUniqueness(t *testing.T) {
+	testUniquenessHelper(GenerateRandomAlphaOnlyString, t)
+}

components/engine/libcontainerd/client_local_windows.go

@@ -513,7 +513,7 @@ func (c *client) createLinux(id string, spec *specs.Spec, runtimeOptions interfa
 	ctr := &container{
 		id:           id,
 		execs:        make(map[string]*process),
-		isWindows:    true,
+		isWindows:    false,
 		ociSpec:      spec,
 		hcsContainer: hcsContainer,
 		status:       StatusCreated,

components/engine/oci/defaults.go

@@ -65,6 +65,7 @@ func DefaultLinuxSpec() specs.Spec {
 				Effective:   defaultCapabilities(),
 			},
 		},
+		Root: &specs.Root{},
 	}
 	s.Mounts = []specs.Mount{
 		{

components/engine/pkg/namesgenerator/cmd/names-generator/main.go

@@ -2,10 +2,13 @@ package main
 
 import (
 	"fmt"
+	"math/rand"
+	"time"
 
 	"github.com/docker/docker/pkg/namesgenerator"
 )
 
 func main() {
+	rand.Seed(time.Now().UnixNano())
 	fmt.Println(namesgenerator.GetRandomName(0))
 }

components/engine/pkg/stringutils/stringutils.go

@@ -7,17 +7,6 @@ import (
 	"strings"
 )
 
-// GenerateRandomAlphaOnlyString generates an alphabetical random string with length n.
-func GenerateRandomAlphaOnlyString(n int) string {
-	// make a really long string
-	letters := []byte("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ")
-	b := make([]byte, n)
-	for i := range b {
-		b[i] = letters[rand.Intn(len(letters))]
-	}
-	return string(b)
-}
-
 // GenerateRandomASCIIString generates an ASCII random string with length n.
 func GenerateRandomASCIIString(n int) string {
 	chars := "abcdefghijklmnopqrstuvwxyz" +

components/engine/pkg/stringutils/stringutils_test.go

@@ -34,14 +34,6 @@ func isASCII(s string) bool {
 	return true
 }
 
-func TestGenerateRandomAlphaOnlyStringLength(t *testing.T) {
-	testLengthHelper(GenerateRandomAlphaOnlyString, t)
-}
-
-func TestGenerateRandomAlphaOnlyStringUniqueness(t *testing.T) {
-	testUniquenessHelper(GenerateRandomAlphaOnlyString, t)
-}
-
 func TestGenerateRandomAsciiStringLength(t *testing.T) {
 	testLengthHelper(GenerateRandomASCIIString, t)
 }