diff --git a/components/engine/api/server/router/container/container_routes.go b/components/engine/api/server/router/container/container_routes.go index 95bbe0e533..106a7087cd 100644 --- a/components/engine/api/server/router/container/container_routes.go +++ b/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, }) } diff --git a/components/engine/api/swagger.yaml b/components/engine/api/swagger.yaml index c3b9c29244..b0c0575fc0 100644 --- a/components/engine/api/swagger.yaml +++ b/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: diff --git a/components/engine/api/types/container/container_wait.go b/components/engine/api/types/container/container_wait.go index 77ecdbaf7a..47fb17578a 100644 --- a/components/engine/api/types/container/container_wait.go +++ b/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"` diff --git a/components/engine/container/state.go b/components/engine/container/state.go index 1a4c45cbab..3af4015ed9 100644 --- a/components/engine/container/state.go +++ b/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() } diff --git a/components/engine/daemon/delete.go b/components/engine/daemon/delete.go index 6db08f38cd..4d56d14529 100644 --- a/components/engine/daemon/delete.go +++ b/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) diff --git a/components/engine/daemon/graphdriver/devmapper/driver.go b/components/engine/daemon/graphdriver/devmapper/driver.go index f41afa2ae7..b485096bc8 100644 --- a/components/engine/daemon/graphdriver/devmapper/driver.go +++ b/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}) } diff --git a/components/engine/docs/api/version-history.md b/components/engine/docs/api/version-history.md index 9ac31dcaaf..77b8545bcc 100644 --- a/components/engine/docs/api/version-history.md +++ b/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 diff --git a/components/engine/integration-cli/cli/build/fakestorage/storage.go b/components/engine/integration-cli/cli/build/fakestorage/storage.go index 25cd872f50..eb0363628d 100644 --- a/components/engine/integration-cli/cli/build/fakestorage/storage.go +++ b/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) diff --git a/components/engine/integration-cli/docker_cli_build_test.go b/components/engine/integration-cli/docker_cli_build_test.go index f6ab1923b9..89e62c14e0 100644 --- a/components/engine/integration-cli/docker_cli_build_test.go +++ b/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", diff --git a/components/engine/integration-cli/docker_cli_run_test.go b/components/engine/integration-cli/docker_cli_run_test.go index 67bf585d04..6ac10e70e9 100644 --- a/components/engine/integration-cli/docker_cli_run_test.go +++ b/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) diff --git a/components/engine/integration-cli/docker_cli_tag_test.go b/components/engine/integration-cli/docker_cli_tag_test.go index 907977fc06..ee94a9b14e 100644 --- a/components/engine/integration-cli/docker_cli_tag_test.go +++ b/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} diff --git a/components/engine/integration-cli/utils_test.go b/components/engine/integration-cli/utils_test.go index d176c7f062..1146e1b2fc 100644 --- a/components/engine/integration-cli/utils_test.go +++ b/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 \ } diff --git a/components/engine/internal/testutil/stringutils.go b/components/engine/internal/testutil/stringutils.go new file mode 100644 index 0000000000..76cf8d86a9 --- /dev/null +++ b/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) +} diff --git a/components/engine/internal/testutil/stringutils_test.go b/components/engine/internal/testutil/stringutils_test.go new file mode 100644 index 0000000000..2985a06553 --- /dev/null +++ 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) +} diff --git a/components/engine/libcontainerd/client_local_windows.go b/components/engine/libcontainerd/client_local_windows.go index 209b00db67..c33e346a7a 100644 --- a/components/engine/libcontainerd/client_local_windows.go +++ b/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, diff --git a/components/engine/oci/defaults.go b/components/engine/oci/defaults.go index 667dd4147a..0cc07ffa13 100644 --- a/components/engine/oci/defaults.go +++ b/components/engine/oci/defaults.go @@ -65,6 +65,7 @@ func DefaultLinuxSpec() specs.Spec { Effective: defaultCapabilities(), }, }, + Root: &specs.Root{}, } s.Mounts = []specs.Mount{ { diff --git a/components/engine/pkg/namesgenerator/cmd/names-generator/main.go b/components/engine/pkg/namesgenerator/cmd/names-generator/main.go index 18a939b70b..7fd5955beb 100644 --- a/components/engine/pkg/namesgenerator/cmd/names-generator/main.go +++ b/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)) } diff --git a/components/engine/pkg/stringutils/stringutils.go b/components/engine/pkg/stringutils/stringutils.go index 8c4c39875e..b294de2c23 100644 --- a/components/engine/pkg/stringutils/stringutils.go +++ b/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" + diff --git a/components/engine/pkg/stringutils/stringutils_test.go b/components/engine/pkg/stringutils/stringutils_test.go index 8af2bdcc0b..15b3cf8e86 100644 --- a/components/engine/pkg/stringutils/stringutils_test.go +++ b/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) }