From dd03cc8be51764f1e5a05c9dae76a8c47148fe3b Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 4 Oct 2018 23:17:13 +0200 Subject: [PATCH 1/6] Remove version-checks for containerd and runc With containerd reaching 1.0, the runtime now has a stable API, so there's no need to do a check if the installed version matches the expected version. Current versions of Docker now also package containerd and runc separately, and can be _updated_ separately. Signed-off-by: Sebastiaan van Stijn (cherry picked from commit c65f0bd13c85d29087419fa555281311091825e7) Signed-off-by: Sebastiaan van Stijn Upstream-commit: 054c3c2931cec5dca8bb84af97f1457c343ec02f Component: engine --- components/engine/daemon/info_unix.go | 10 ++++++++-- components/engine/dockerversion/version_lib.go | 2 -- components/engine/hack/make/.go-autogen | 2 -- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/components/engine/daemon/info_unix.go b/components/engine/daemon/info_unix.go index 98935cca70..55b6c6e79b 100644 --- a/components/engine/daemon/info_unix.go +++ b/components/engine/daemon/info_unix.go @@ -29,7 +29,6 @@ func (daemon *Daemon) fillPlatformInfo(v *types.Info, sysInfo *sysinfo.SysInfo) v.DefaultRuntime = daemon.configStore.GetDefaultRuntimeName() v.InitBinary = daemon.configStore.GetInitPath() - v.RuncCommit.Expected = dockerversion.RuncCommitID defaultRuntimeBinary := daemon.configStore.GetRuntime(v.DefaultRuntime).Path if rv, err := exec.Command(defaultRuntimeBinary, "--version").Output(); err == nil { parts := strings.Split(strings.TrimSpace(string(rv)), "\n") @@ -49,7 +48,10 @@ func (daemon *Daemon) fillPlatformInfo(v *types.Info, sysInfo *sysinfo.SysInfo) v.RuncCommit.ID = "N/A" } - v.ContainerdCommit.Expected = dockerversion.ContainerdCommitID + // runc is now shipped as a separate package. Set "expected" to same value + // as "ID" to prevent clients from reporting a version-mismatch + v.RuncCommit.Expected = v.RuncCommit.ID + if rv, err := daemon.containerd.Version(context.Background()); err == nil { v.ContainerdCommit.ID = rv.Revision } else { @@ -57,6 +59,10 @@ func (daemon *Daemon) fillPlatformInfo(v *types.Info, sysInfo *sysinfo.SysInfo) v.ContainerdCommit.ID = "N/A" } + // containerd is now shipped as a separate package. Set "expected" to same + // value as "ID" to prevent clients from reporting a version-mismatch + v.ContainerdCommit.Expected = v.ContainerdCommit.ID + defaultInitBinary := daemon.configStore.GetInitPath() if rv, err := exec.Command(defaultInitBinary, "--version").Output(); err == nil { ver, err := parseInitVersion(string(rv)) diff --git a/components/engine/dockerversion/version_lib.go b/components/engine/dockerversion/version_lib.go index 77b87891be..5d9b3fdd2b 100644 --- a/components/engine/dockerversion/version_lib.go +++ b/components/engine/dockerversion/version_lib.go @@ -10,8 +10,6 @@ const ( Version = "library-import" BuildTime = "library-import" IAmStatic = "library-import" - ContainerdCommitID = "library-import" - RuncCommitID = "library-import" InitCommitID = "library-import" PlatformName = "" ProductName = "" diff --git a/components/engine/hack/make/.go-autogen b/components/engine/hack/make/.go-autogen index 5d1aab930b..ea8a32ff5d 100644 --- a/components/engine/hack/make/.go-autogen +++ b/components/engine/hack/make/.go-autogen @@ -19,7 +19,6 @@ const ( Version string = "$VERSION" BuildTime string = "$BUILDTIME" IAmStatic string = "${IAMSTATIC:-true}" - ContainerdCommitID string = "${CONTAINERD_COMMIT}" PlatformName string = "${PLATFORM}" ProductName string = "${PRODUCT}" DefaultProductLicense string = "${DEFAULT_PRODUCT_LICENSE}" @@ -37,7 +36,6 @@ package dockerversion // Default build-time variable for library-import. // This file is overridden on build with build-time informations. const ( - RuncCommitID string = "${RUNC_COMMIT}" InitCommitID string = "${TINI_COMMIT}" ) From ddba35eade822d6234b0c95569c6a56b1795785d Mon Sep 17 00:00:00 2001 From: Wei Fu Date: Wed, 10 Oct 2018 14:54:00 +0800 Subject: [PATCH 2/6] bugfix: wait for stdin creation before CloseIO The stdin fifo of exec process is created in containerd side after client calls Start. If the client calls CloseIO before Start call, the stdin of exec process is still opened and wait for close. For this case, client closes stdinCloseSync channel after Start. Signed-off-by: Wei Fu (cherry picked from commit c7890f25a9eaae8d07614bd85b2b3231b03e54ec) Signed-off-by: Sebastiaan van Stijn Upstream-commit: 6679a5faeb724f1ad060f2fdf6d189f1005924b9 Component: engine --- components/engine/libcontainerd/client_daemon.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/components/engine/libcontainerd/client_daemon.go b/components/engine/libcontainerd/client_daemon.go index 60fb3353c1..cb9cb43a73 100644 --- a/components/engine/libcontainerd/client_daemon.go +++ b/components/engine/libcontainerd/client_daemon.go @@ -328,6 +328,13 @@ func (c *client) Start(ctx context.Context, id, checkpointDir string, withStdin return int(t.Pid()), nil } +// Exec creates exec process. +// +// The containerd client calls Exec to register the exec config in the shim side. +// When the client calls Start, the shim will create stdin fifo if needs. But +// for the container main process, the stdin fifo will be created in Create not +// the Start call. stdinCloseSync channel should be closed after Start exec +// process. func (c *client) Exec(ctx context.Context, containerID, processID string, spec *specs.Process, withStdin bool, attachStdio StdioCallback) (int, error) { ctr := c.getContainer(containerID) if ctr == nil { @@ -372,7 +379,9 @@ func (c *client) Exec(ctx context.Context, containerID, processID string, spec * ctr.addProcess(processID, p) // Signal c.createIO that it can call CloseIO - close(stdinCloseSync) + // + // the stdin of exec process will be created after p.Start in containerd + defer close(stdinCloseSync) if err = p.Start(ctx); err != nil { p.Delete(context.Background()) From b8f9cfd066d5075cd463d997cf1ac38f3495b798 Mon Sep 17 00:00:00 2001 From: Wei Fu Date: Thu, 11 Oct 2018 19:03:02 +0800 Subject: [PATCH 3/6] testing: add case for exec closeStdin add regression case for the issue#37870 Signed-off-by: Wei Fu (cherry picked from commit 8e25f4ff6d89888a1bcd578f3f8f7aab89dce24d) Signed-off-by: Sebastiaan van Stijn Upstream-commit: ae6284a623bac86ac6ab718fa4a369dd8c0a3cfc Component: engine --- .../engine/integration/container/exec_test.go | 69 +++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/components/engine/integration/container/exec_test.go b/components/engine/integration/container/exec_test.go index 85f9e05915..d33301b8e3 100644 --- a/components/engine/integration/container/exec_test.go +++ b/components/engine/integration/container/exec_test.go @@ -4,6 +4,7 @@ import ( "context" "io/ioutil" "testing" + "time" "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/strslice" @@ -15,6 +16,74 @@ import ( "gotest.tools/skip" ) +// TestExecWithCloseStdin adds case for moby#37870 issue. +func TestExecWithCloseStdin(t *testing.T) { + skip.If(t, versions.LessThan(testEnv.DaemonAPIVersion(), "1.39"), "broken in earlier versions") + defer setupTest(t)() + + ctx := context.Background() + client := request.NewAPIClient(t) + + // run top with detached mode + cID := container.Run(t, ctx, client) + + expected := "closeIO" + execResp, err := client.ContainerExecCreate(ctx, cID, + types.ExecConfig{ + AttachStdin: true, + AttachStdout: true, + Cmd: strslice.StrSlice([]string{"sh", "-c", "cat && echo " + expected}), + }, + ) + assert.NilError(t, err) + + resp, err := client.ContainerExecAttach(ctx, execResp.ID, + types.ExecStartCheck{ + Detach: false, + Tty: false, + }, + ) + assert.NilError(t, err) + defer resp.Close() + + // close stdin to send EOF to cat + assert.NilError(t, resp.CloseWrite()) + + var ( + waitCh = make(chan struct{}) + resCh = make(chan struct { + content string + err error + }) + ) + + go func() { + close(waitCh) + defer close(resCh) + r, err := ioutil.ReadAll(resp.Reader) + + resCh <- struct { + content string + err error + }{ + content: string(r), + err: err, + } + }() + + <-waitCh + select { + case <-time.After(3 * time.Second): + t.Fatal("failed to read the content in time") + case got := <-resCh: + assert.NilError(t, got.err) + + // NOTE: using Contains because no-tty's stream contains UX information + // like size, stream type. + assert.Assert(t, is.Contains(got.content, expected)) + } +} + func TestExec(t *testing.T) { skip.If(t, versions.LessThan(testEnv.DaemonAPIVersion(), "1.35"), "broken in earlier versions") defer setupTest(t)() From ea7e1c4aaaaf7d2c187f4c4dd4b1139cf00aec94 Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Mon, 22 Oct 2018 17:24:20 -0700 Subject: [PATCH 4/6] builder: fix duplicate mount release Signed-off-by: Tonis Tiigi (cherry picked from commit 2732fe527f9258561c7310c128914b4b456c8404) Signed-off-by: Sebastiaan van Stijn Upstream-commit: 5853cd510c3272755ca5d6605ca8039d54a5ba15 Component: engine --- .../builder-next/adapters/snapshot/snapshot.go | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/components/engine/builder/builder-next/adapters/snapshot/snapshot.go b/components/engine/builder/builder-next/adapters/snapshot/snapshot.go index e5e6942653..c1388da7e9 100644 --- a/components/engine/builder/builder-next/adapters/snapshot/snapshot.go +++ b/components/engine/builder/builder-next/adapters/snapshot/snapshot.go @@ -426,10 +426,11 @@ func (s *snapshotter) Close() error { } type mountable struct { - mu sync.Mutex - mounts []mount.Mount - acquire func() ([]mount.Mount, error) - release func() error + mu sync.Mutex + mounts []mount.Mount + acquire func() ([]mount.Mount, error) + release func() error + refCount int } func (m *mountable) Mount() ([]mount.Mount, error) { @@ -437,6 +438,7 @@ func (m *mountable) Mount() ([]mount.Mount, error) { defer m.mu.Unlock() if m.mounts != nil { + m.refCount++ return m.mounts, nil } @@ -445,6 +447,7 @@ func (m *mountable) Mount() ([]mount.Mount, error) { return nil, err } m.mounts = mounts + m.refCount = 1 return m.mounts, nil } @@ -452,6 +455,13 @@ func (m *mountable) Mount() ([]mount.Mount, error) { func (m *mountable) Release() error { m.mu.Lock() defer m.mu.Unlock() + + if m.refCount > 1 { + m.refCount-- + return nil + } + + m.refCount = 0 if m.release == nil { return nil } From 0c72992e431eb57e432cfd8bd4ea0d3b8f2df374 Mon Sep 17 00:00:00 2001 From: Tibor Vass Date: Tue, 16 Oct 2018 18:51:25 +0000 Subject: [PATCH 5/6] builder: fix bugs when pruning buildkit cache with filters Only the filters the user specified should be added as cache filters to buildkit. Make an AND operation of the provided filters. ID filter now does prefix-matching. Signed-off-by: Tibor Vass (cherry picked from commit b6137bebb83e886aef906b7ff277778b69616991) Signed-off-by: Tibor Vass Upstream-commit: 52a3c39506b883f713694ce39d1a4fd9f5638800 Component: engine --- .../engine/builder/builder-next/builder.go | 24 ++++++++++++------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/components/engine/builder/builder-next/builder.go b/components/engine/builder/builder-next/builder.go index 12d7a28b10..261803fee5 100644 --- a/components/engine/builder/builder-next/builder.go +++ b/components/engine/builder/builder-next/builder.go @@ -532,20 +532,26 @@ func toBuildkitPruneInfo(opts types.BuildCachePruneOptions) (client.PruneInfo, e bkFilter := make([]string, 0, opts.Filters.Len()) for cacheField := range cacheFields { - values := opts.Filters.Get(cacheField) - switch len(values) { - case 0: - bkFilter = append(bkFilter, cacheField) - case 1: - bkFilter = append(bkFilter, cacheField+"=="+values[0]) - default: - return client.PruneInfo{}, errMultipleFilterValues + if opts.Filters.Include(cacheField) { + values := opts.Filters.Get(cacheField) + switch len(values) { + case 0: + bkFilter = append(bkFilter, cacheField) + case 1: + if cacheField == "id" { + bkFilter = append(bkFilter, cacheField+"~="+values[0]) + } else { + bkFilter = append(bkFilter, cacheField+"=="+values[0]) + } + default: + return client.PruneInfo{}, errMultipleFilterValues + } } } return client.PruneInfo{ All: opts.All, KeepDuration: unusedFor, KeepBytes: opts.KeepStorage, - Filter: bkFilter, + Filter: []string{strings.Join(bkFilter, ",")}, }, nil } From a78fdada0129deafe639725f7b8f753d538e8f74 Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Tue, 6 Nov 2018 10:45:02 -0800 Subject: [PATCH 6/6] builder: update copy to 0.1.9 Signed-off-by: Tonis Tiigi Upstream-commit: 45654ed0126aadaf6c3293b0a32ca8cf15021626 Component: engine --- components/engine/builder/builder-next/builder.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/engine/builder/builder-next/builder.go b/components/engine/builder/builder-next/builder.go index 261803fee5..1e3c7bda2f 100644 --- a/components/engine/builder/builder-next/builder.go +++ b/components/engine/builder/builder-next/builder.go @@ -222,7 +222,7 @@ func (b *Builder) Build(ctx context.Context, opt backend.BuildConfig) (*builder. id := identity.NewID() frontendAttrs := map[string]string{ - "override-copy-image": "docker.io/docker/dockerfile-copy:v0.1.7@sha256:4211460d4df58cca572825b93e437c09dac6387d88910fe07ac8f7c78d52e0ce", + "override-copy-image": "docker.io/docker/dockerfile-copy:v0.1.9@sha256:e8f159d3f00786604b93c675ee2783f8dc194bb565e61ca5788f6a6e9d304061", } if opt.Options.Target != "" {