From ecf545d0952c8283d15f0406248a2d7116fad240 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 29 Mar 2019 00:00:06 +0100 Subject: [PATCH 01/24] update containerd/cgroups 4994991857f9b0ae8dc439551e8bebdbb4bf66c1 full diff: https://github.com/containerd/cgroups/compare/dbea6f2bd41658b84b00417ceefa416b979cbf10...4994991857f9b0ae8dc439551e8bebdbb4bf66c1 brings in https://github.com/containerd/cgroups/pull/79 Return ErrCgroupDeleted when no subsystems relates to https://github.com/containerd/containerd/issues/3133 Custom cgroup path does not work in containerd 1.2.5 Signed-off-by: Sebastiaan van Stijn (cherry picked from commit 7392abda03305b35b48c974752d89be2cd513c19) Signed-off-by: Sebastiaan van Stijn Upstream-commit: ad222b36c04ab80e1c40485108c59a112b3771b2 Component: engine --- components/engine/vendor.conf | 2 +- .../engine/vendor/github.com/containerd/cgroups/cgroup.go | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/components/engine/vendor.conf b/components/engine/vendor.conf index 6d3c458496..9da5d28445 100644 --- a/components/engine/vendor.conf +++ b/components/engine/vendor.conf @@ -121,7 +121,7 @@ google.golang.org/genproto 694d95ba50e67b2e363f3483057db5d4910c18f9 github.com/containerd/containerd 9754871865f7fe2f4e74d43e2fc7ccd237edcbce # v1.2.2 github.com/containerd/fifo 3d5202aec260678c48179c56f40e6f38a095738c github.com/containerd/continuity 004b46473808b3e7a4a3049c20e4376c91eb966d -github.com/containerd/cgroups dbea6f2bd41658b84b00417ceefa416b979cbf10 +github.com/containerd/cgroups 4994991857f9b0ae8dc439551e8bebdbb4bf66c1 github.com/containerd/console c12b1e7919c14469339a5d38f2f8ed9b64a9de23 github.com/containerd/cri 0d5cabd006cb5319dc965046067b8432d9fa5ef8 # release/1.2 branch github.com/containerd/go-runc 5a6d9f37cfa36b15efba46dc7ea349fa9b7143c3 diff --git a/components/engine/vendor/github.com/containerd/cgroups/cgroup.go b/components/engine/vendor/github.com/containerd/cgroups/cgroup.go index 9fbea82499..e3ef076519 100644 --- a/components/engine/vendor/github.com/containerd/cgroups/cgroup.go +++ b/components/engine/vendor/github.com/containerd/cgroups/cgroup.go @@ -105,6 +105,10 @@ func Load(hierarchy Hierarchy, path Path, opts ...InitOpts) (Cgroup, error) { } activeSubsystems = append(activeSubsystems, s) } + // if we do not have any active systems then the cgroup is deleted + if len(activeSubsystems) == 0 { + return nil, ErrCgroupDeleted + } return &cgroup{ path: path, subsystems: activeSubsystems, From 41f6c45376ac761e66ccc6b06c8dd94b96bb609d Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Tue, 2 Apr 2019 11:37:44 -0700 Subject: [PATCH 02/24] daemon: fix mirrors validation Signed-off-by: Tonis Tiigi (cherry picked from commit 1a0f04e08e2f23a12b8dd2deb7c10c7f4f7d8804) Signed-off-by: Sebastiaan van Stijn Upstream-commit: 5dab6096fb7fcfe94acd9145b24eda8640427bb5 Component: engine --- components/engine/daemon/daemon.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/components/engine/daemon/daemon.go b/components/engine/daemon/daemon.go index a307863017..b2c02a9712 100644 --- a/components/engine/daemon/daemon.go +++ b/components/engine/daemon/daemon.go @@ -11,6 +11,7 @@ import ( "io/ioutil" "math/rand" "net" + "net/url" "os" "path" "path/filepath" @@ -157,15 +158,18 @@ func (daemon *Daemon) NewResolveOptionsFunc() resolver.ResolveOptionsFunc { ) // must trim "https://" or "http://" prefix for i, v := range daemon.configStore.Mirrors { - v = strings.TrimPrefix(v, "https://") - v = strings.TrimPrefix(v, "http://") + if uri, err := url.Parse(v); err == nil { + v = uri.Host + } mirrors[i] = v } // set "registry-mirrors" m[registryKey] = resolver.RegistryConf{Mirrors: mirrors} // set "insecure-registries" for _, v := range daemon.configStore.InsecureRegistries { - v = strings.TrimPrefix(v, "http://") + if uri, err := url.Parse(v); err == nil { + v = uri.Host + } m[v] = resolver.RegistryConf{ PlainHTTP: true, } From 2d86442852798bc410769a22bf13d22e299b169a Mon Sep 17 00:00:00 2001 From: Alexei Margasov Date: Tue, 9 Apr 2019 16:11:06 +0500 Subject: [PATCH 03/24] Adds PartialLogMetadata to encode protobuf for logger plugins Signed-off-by: Alexei Margasov (cherry picked from commit 4a9836a20b35968eb931dd53a6e00b81990d8b3f) Signed-off-by: Sebastiaan van Stijn Upstream-commit: 722d77e4b76af2b61d8040b3defd52b4d611e032 Component: engine --- components/engine/daemon/logger/adapter.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/components/engine/daemon/logger/adapter.go b/components/engine/daemon/logger/adapter.go index d9370352c5..97d59be5e0 100644 --- a/components/engine/daemon/logger/adapter.go +++ b/components/engine/daemon/logger/adapter.go @@ -39,6 +39,13 @@ func (a *pluginAdapter) Log(msg *Message) error { a.buf.TimeNano = msg.Timestamp.UnixNano() a.buf.Partial = msg.PLogMetaData != nil a.buf.Source = msg.Source + if msg.PLogMetaData != nil { + a.buf.PartialLogMetadata = &logdriver.PartialLogEntryMetadata{ + Id: msg.PLogMetaData.ID, + Last: msg.PLogMetaData.Last, + Ordinal: int32(msg.PLogMetaData.Ordinal), + } + } err := a.enc.Encode(&a.buf) a.buf.Reset() From c9f96feac54b8959a6d30b4728ea540a5574b094 Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Wed, 21 Nov 2018 12:04:58 -0800 Subject: [PATCH 04/24] builder: add workaround for gcr auth issue GCR does not currently support 401 response from blob endpoints. This detects the case where no manifest requests have been performed for the current resolver and does a dummy request to enable authorization. Signed-off-by: Tonis Tiigi (cherry picked from commit bcd8298c35f53f79b42c6e089e8da114ddb5c57e) Signed-off-by: Sebastiaan van Stijn Upstream-commit: 21cce932b05a4b979359624668f7f09e83dc90cd Component: engine --- .../adapters/containerimage/pull.go | 103 +++++++++++++++++- 1 file changed, 101 insertions(+), 2 deletions(-) diff --git a/components/engine/builder/builder-next/adapters/containerimage/pull.go b/components/engine/builder/builder-next/adapters/containerimage/pull.go index b1e44d95e9..2240407de0 100644 --- a/components/engine/builder/builder-next/adapters/containerimage/pull.go +++ b/components/engine/builder/builder-next/adapters/containerimage/pull.go @@ -8,6 +8,7 @@ import ( "io/ioutil" "runtime" "sync" + "sync/atomic" "time" "github.com/containerd/containerd/content" @@ -57,13 +58,15 @@ type SourceOpt struct { type imageSource struct { SourceOpt - g flightcontrol.Group + g flightcontrol.Group + resolverCache *resolverCache } // NewSource creates a new image source func NewSource(opt SourceOpt) (source.Source, error) { is := &imageSource{ - SourceOpt: opt, + SourceOpt: opt, + resolverCache: newResolverCache(), } return is, nil @@ -74,6 +77,9 @@ func (is *imageSource) ID() string { } func (is *imageSource) getResolver(ctx context.Context, rfn resolver.ResolveOptionsFunc, ref string) remotes.Resolver { + if res := is.resolverCache.Get(ctx, ref); res != nil { + return res + } opt := docker.ResolverOptions{ Client: tracing.DefaultClient, } @@ -82,6 +88,7 @@ func (is *imageSource) getResolver(ctx context.Context, rfn resolver.ResolveOpti } opt.Credentials = is.getCredentialsFromSession(ctx) r := docker.NewResolver(opt) + r = is.resolverCache.Add(ctx, ref, r) return r } @@ -380,6 +387,11 @@ func (p *puller) Snapshot(ctx context.Context) (cache.ImmutableRef, error) { return nil, err } + // workaround for GCR bug that requires a request to manifest endpoint for authentication to work. + // if current resolver has not used manifests do a dummy request. + // in most cases resolver should be cached and extra request is not needed. + ensureManifestRequested(ctx, p.resolver, p.ref) + var ( schema1Converter *schema1.Converter handlers []images.Handler @@ -791,3 +803,90 @@ func resolveModeToString(rm source.ResolveMode) string { } return "" } + +type resolverCache struct { + mu sync.Mutex + m map[string]cachedResolver +} + +type cachedResolver struct { + timeout time.Time + remotes.Resolver + counter int64 +} + +func (cr *cachedResolver) Resolve(ctx context.Context, ref string) (name string, desc ocispec.Descriptor, err error) { + atomic.AddInt64(&cr.counter, 1) + return cr.Resolver.Resolve(ctx, ref) +} + +func (r *resolverCache) Add(ctx context.Context, ref string, resolver remotes.Resolver) remotes.Resolver { + r.mu.Lock() + defer r.mu.Unlock() + + ref = r.domain(ref) + "-" + session.FromContext(ctx) + + cr, ok := r.m[ref] + cr.timeout = time.Now().Add(time.Minute) + if ok { + return &cr + } + + cr.Resolver = resolver + r.m[ref] = cr + return &cr +} + +func (r *resolverCache) domain(refStr string) string { + ref, err := distreference.ParseNormalizedNamed(refStr) + if err != nil { + return refStr + } + return distreference.Domain(ref) +} + +func (r *resolverCache) Get(ctx context.Context, ref string) remotes.Resolver { + r.mu.Lock() + defer r.mu.Unlock() + + ref = r.domain(ref) + "-" + session.FromContext(ctx) + + cr, ok := r.m[ref] + if !ok { + return nil + } + return &cr +} + +func (r *resolverCache) clean(now time.Time) { + r.mu.Lock() + for k, cr := range r.m { + if now.After(cr.timeout) { + delete(r.m, k) + } + } + r.mu.Unlock() +} + +func newResolverCache() *resolverCache { + rc := &resolverCache{ + m: map[string]cachedResolver{}, + } + t := time.NewTicker(time.Minute) + go func() { + for { + rc.clean(<-t.C) + } + }() + return rc +} + +func ensureManifestRequested(ctx context.Context, res remotes.Resolver, ref string) { + cr, ok := res.(*cachedResolver) + if !ok { + return + } + if atomic.LoadInt64(&cr.counter) == 0 { + res.Resolve(ctx, ref) + } +} From b82179cd369ff451bf992124f9f8b2b16eeeb56f Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Tue, 7 May 2019 10:33:04 -0700 Subject: [PATCH 05/24] builder-next: fix gcr workaround token cache Signed-off-by: Tonis Tiigi (cherry picked from commit cfce0acd332d7536f85356f99b99a920be6cda87) Signed-off-by: Sebastiaan van Stijn Upstream-commit: 7e4c2474c7858879bcdfa089ee82f311b0d71fe8 Component: engine --- .../builder/builder-next/adapters/containerimage/pull.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/components/engine/builder/builder-next/adapters/containerimage/pull.go b/components/engine/builder/builder-next/adapters/containerimage/pull.go index 2240407de0..c3fa916df3 100644 --- a/components/engine/builder/builder-next/adapters/containerimage/pull.go +++ b/components/engine/builder/builder-next/adapters/containerimage/pull.go @@ -824,7 +824,7 @@ func (r *resolverCache) Add(ctx context.Context, ref string, resolver remotes.Re r.mu.Lock() defer r.mu.Unlock() - ref = r.domain(ref) + "-" + session.FromContext(ctx) + ref = r.repo(ref) + "-" + session.FromContext(ctx) cr, ok := r.m[ref] cr.timeout = time.Now().Add(time.Minute) @@ -837,19 +837,19 @@ func (r *resolverCache) Add(ctx context.Context, ref string, resolver remotes.Re return &cr } -func (r *resolverCache) domain(refStr string) string { +func (r *resolverCache) repo(refStr string) string { ref, err := distreference.ParseNormalizedNamed(refStr) if err != nil { return refStr } - return distreference.Domain(ref) + return ref.Name() } func (r *resolverCache) Get(ctx context.Context, ref string) remotes.Resolver { r.mu.Lock() defer r.mu.Unlock() - ref = r.domain(ref) + "-" + session.FromContext(ctx) + ref = r.repo(ref) + "-" + session.FromContext(ctx) cr, ok := r.m[ref] if !ok { From 64a3cdad968a8c5d2431a21ea148d13849dc3a98 Mon Sep 17 00:00:00 2001 From: Olly Pomeroy Date: Wed, 8 May 2019 09:30:06 +0000 Subject: [PATCH 06/24] Switch swarmmode services to NanoCpu Today `$ docker service create --limit-cpu` configures a containers `CpuPeriod` and `CpuQuota` variables, this commit switches this to configure a containers `NanoCpu` variable instead. Signed-off-by: Olly Pomeroy (cherry picked from commit 8a60a1e14a5e98b7ed0fbea57369615a766a1ae9) Signed-off-by: Sebastiaan van Stijn Upstream-commit: 675f4256ae2b85556b4243da44ff627900660463 Component: engine --- .../daemon/cluster/executor/container/container.go | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/components/engine/daemon/cluster/executor/container/container.go b/components/engine/daemon/cluster/executor/container/container.go index 77d21d2c1f..6311fc86fc 100644 --- a/components/engine/daemon/cluster/executor/container/container.go +++ b/components/engine/daemon/cluster/executor/container/container.go @@ -6,7 +6,6 @@ import ( "net" "strconv" "strings" - "time" "github.com/sirupsen/logrus" @@ -31,10 +30,6 @@ import ( ) const ( - // Explicitly use the kernel's default setting for CPU quota of 100ms. - // https://www.kernel.org/doc/Documentation/scheduler/sched-bwc.txt - cpuQuotaPeriod = 100 * time.Millisecond - // systemLabelPrefix represents the reserved namespace for system labels. systemLabelPrefix = "com.docker.swarm" ) @@ -448,9 +443,7 @@ func (c *containerConfig) resources() enginecontainer.Resources { } if r.Limits.NanoCPUs > 0 { - // CPU Period must be set in microseconds. - resources.CPUPeriod = int64(cpuQuotaPeriod / time.Microsecond) - resources.CPUQuota = r.Limits.NanoCPUs * resources.CPUPeriod / 1e9 + resources.NanoCPUs = r.Limits.NanoCPUs } return resources From 2cb70024b7c3160b19a6ede318b72290ac21387d Mon Sep 17 00:00:00 2001 From: Deep Debroy Date: Tue, 14 May 2019 13:27:18 -0700 Subject: [PATCH 07/24] Consider WINDOWS_BASE_IMAGE_TAG override when setting Windows base image for tests Signed-off-by: Deep Debroy (cherry picked from commit 15419d7ba09bb48dec59c73af51a0fd005eeb460) Signed-off-by: Sebastiaan van Stijn Upstream-commit: fed2792d8570d50bb36572286dd84b23dd7fc08d Component: engine --- .../engine/internal/test/environment/environment.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/components/engine/internal/test/environment/environment.go b/components/engine/internal/test/environment/environment.go index 5538d2097e..76f94a559a 100644 --- a/components/engine/internal/test/environment/environment.go +++ b/components/engine/internal/test/environment/environment.go @@ -75,10 +75,13 @@ func getPlatformDefaults(info types.Info, osType string) PlatformDefaults { } case "windows": baseImage := "microsoft/windowsservercore" - if override := os.Getenv("WINDOWS_BASE_IMAGE"); override != "" { - baseImage = override - fmt.Println("INFO: Windows Base image is ", baseImage) + if overrideBaseImage := os.Getenv("WINDOWS_BASE_IMAGE"); overrideBaseImage != "" { + baseImage = overrideBaseImage + if overrideBaseImageTag := os.Getenv("WINDOWS_BASE_IMAGE_TAG"); overrideBaseImageTag != "" { + baseImage = baseImage + ":" + overrideBaseImageTag + } } + fmt.Println("INFO: Windows Base image is ", baseImage) return PlatformDefaults{ BaseImage: baseImage, VolumesConfigPath: filepath.FromSlash(volumesPath), From b6938b488f2391f428aa9377c1a84d1cee914047 Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Tue, 21 May 2019 15:34:57 -0700 Subject: [PATCH 08/24] Fix error handling for bind mount spec parser. Errors were being ignored and always telling the user that the path doesn't exist even if it was some other problem, such as a permission error. Signed-off-by: Brian Goff (cherry picked from commit ebcef288343698dd86ff307f5b9c58aa52ce9fdd) Signed-off-by: Sebastiaan van Stijn Upstream-commit: 0ac09de395d61d8cecdd6496aa3fb9ceb174f295 Component: engine --- .../engine/volume/mounts/linux_parser.go | 5 +- .../engine/volume/mounts/parser_test.go | 50 +++++++++++++++++++ 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/components/engine/volume/mounts/linux_parser.go b/components/engine/volume/mounts/linux_parser.go index 8e436aec0e..e276a39cee 100644 --- a/components/engine/volume/mounts/linux_parser.go +++ b/components/engine/volume/mounts/linux_parser.go @@ -82,7 +82,10 @@ func (p *linuxParser) validateMountConfigImpl(mnt *mount.Mount, validateBindSour } if validateBindSourceExists { - exists, _, _ := currentFileInfoProvider.fileInfo(mnt.Source) + exists, _, err := currentFileInfoProvider.fileInfo(mnt.Source) + if err != nil { + return &errMountConfig{mnt, err} + } if !exists { return &errMountConfig{mnt, errBindSourceDoesNotExist(mnt.Source)} } diff --git a/components/engine/volume/mounts/parser_test.go b/components/engine/volume/mounts/parser_test.go index 27257d62bd..f9b32e5ccf 100644 --- a/components/engine/volume/mounts/parser_test.go +++ b/components/engine/volume/mounts/parser_test.go @@ -1,6 +1,7 @@ package mounts // import "github.com/docker/docker/volume/mounts" import ( + "errors" "io/ioutil" "os" "runtime" @@ -8,6 +9,8 @@ import ( "testing" "github.com/docker/docker/api/types/mount" + "gotest.tools/assert" + "gotest.tools/assert/cmp" ) type parseMountRawTestSet struct { @@ -477,4 +480,51 @@ func TestParseMountSpec(t *testing.T) { t.Errorf("Expected mount copy data to match. Expected: '%v', Actual: '%v'", c.expected.CopyData, mp.CopyData) } } + +} + +// always returns the configured error +// this is used to test error handling +type mockFiProviderWithError struct{ err error } + +func (m mockFiProviderWithError) fileInfo(path string) (bool, bool, error) { + return false, false, m.err +} + +// TestParseMountSpecBindWithFileinfoError makes sure that the parser returns +// the error produced by the fileinfo provider. +// +// Some extra context for the future in case of changes and possible wtf are we +// testing this for: +// +// Currently this "fileInfoProvider" returns (bool, bool, error) +// The 1st bool is "does this path exist" +// The 2nd bool is "is this path a dir" +// Then of course the error is an error. +// +// The issue is the parser was ignoring the error and only looking at the +// "does this path exist" boolean, which is always false if there is an error. +// Then the error returned to the caller was a (slightly, maybe) friendlier +// error string than what comes from `os.Stat` +// So ...the caller was always getting an error saying the path doesn't exist +// even if it does exist but got some other error (like a permission error). +// This is confusing to users. +func TestParseMountSpecBindWithFileinfoError(t *testing.T) { + previousProvider := currentFileInfoProvider + defer func() { currentFileInfoProvider = previousProvider }() + + testErr := errors.New("some crazy error") + currentFileInfoProvider = &mockFiProviderWithError{err: testErr} + + p := "/bananas" + if runtime.GOOS == "windows" { + p = `c:\bananas` + } + m := mount.Mount{Type: mount.TypeBind, Source: p, Target: p} + + parser := NewParser(runtime.GOOS) + + _, err := parser.ParseMountSpec(m) + assert.Assert(t, err != nil) + assert.Assert(t, cmp.Contains(err.Error(), "some crazy error")) } From a76550469d88c3bd45e18264d2fed4b2105578b7 Mon Sep 17 00:00:00 2001 From: Jonas Dohse Date: Thu, 30 May 2019 16:11:22 +0200 Subject: [PATCH 09/24] Stop sorting uid and gid ranges in id maps Moby currently sorts uid and gid ranges in id maps. This causes subuid and subgid files to be interpreted wrongly. The subuid file ``` > cat /etc/subuid jonas:100000:1000 jonas:1000:1 ``` configures that the container uids 0-999 are mapped to the host uids 100000-100999 and uid 1000 in the container is mapped to uid 1000 on the host. The expected uid_map is: ``` > docker run ubuntu cat /proc/self/uid_map 0 100000 1000 1000 1000 1 ``` Moby currently sorts the ranges by the first id in the range. Therefore with the subuid file above the uid 0 in the container is mapped to uid 100000 on host and the uids 1-1000 in container are mapped to the uids 1-1000 on the host. The resulting uid_map is: ``` > docker run ubuntu cat /proc/self/uid_map 0 1000 1 1 100000 1000 ``` The ordering was implemented to work around a limitation in Linux 3.8. This is fixed since Linux 3.9 as stated on the user namespaces manpage [1]: > In the initial implementation (Linux 3.8), this requirement was > satisfied by a simplistic implementation that imposed the further > requirement that the values in both field 1 and field 2 of successive > lines must be in ascending numerical order, which prevented some > otherwise valid maps from being created. Linux 3.9 and later fix this > limitation, allowing any valid set of nonoverlapping maps. This fix changes the interpretation of subuid and subgid files which do not have the ids of in the numerical order for each individual user. This breaks users that rely on the current behaviour. The desired mapping above - map low user ids in the container to high user ids on the host and some higher user ids in the container to lower user on host - can unfortunately not archived with the current behaviour. [1] http://man7.org/linux/man-pages/man7/user_namespaces.7.html Signed-off-by: Jonas Dohse (cherry picked from commit c4628d79d26c47bfbac9a3b22d684ee5fd78973c) Signed-off-by: Sebastiaan van Stijn Upstream-commit: 601f11300ed9995ae1d9117a52fd38b3e86563d8 Component: engine --- components/engine/pkg/idtools/idtools.go | 3 -- components/engine/pkg/idtools/idtools_test.go | 28 +++++++++++++++++++ 2 files changed, 28 insertions(+), 3 deletions(-) create mode 100644 components/engine/pkg/idtools/idtools_test.go diff --git a/components/engine/pkg/idtools/idtools.go b/components/engine/pkg/idtools/idtools.go index 230422eac8..b3af7a4226 100644 --- a/components/engine/pkg/idtools/idtools.go +++ b/components/engine/pkg/idtools/idtools.go @@ -4,7 +4,6 @@ import ( "bufio" "fmt" "os" - "sort" "strconv" "strings" ) @@ -203,8 +202,6 @@ func (i *IdentityMapping) GIDs() []IDMap { func createIDMap(subidRanges ranges) []IDMap { idMap := []IDMap{} - // sort the ranges by lowest ID first - sort.Sort(subidRanges) containerID := 0 for _, idrange := range subidRanges { idMap = append(idMap, IDMap{ diff --git a/components/engine/pkg/idtools/idtools_test.go b/components/engine/pkg/idtools/idtools_test.go new file mode 100644 index 0000000000..7627d1998c --- /dev/null +++ b/components/engine/pkg/idtools/idtools_test.go @@ -0,0 +1,28 @@ +package idtools // import "github.com/docker/docker/pkg/idtools" + +import ( + "testing" + + "gotest.tools/assert" +) + +func TestCreateIDMapOrder(t *testing.T) { + subidRanges := ranges{ + {100000, 1000}, + {1000, 1}, + } + + idMap := createIDMap(subidRanges) + assert.DeepEqual(t, idMap, []IDMap{ + { + ContainerID: 0, + HostID: 100000, + Size: 1000, + }, + { + ContainerID: 1000, + HostID: 1000, + Size: 1, + }, + }) +} From 927d42432eff5206ba24184b5a98d06333875f6c Mon Sep 17 00:00:00 2001 From: Jintao Zhang Date: Sat, 6 Apr 2019 16:51:56 +0000 Subject: [PATCH 10/24] Update containerd v1.2.6 Signed-off-by: Jintao Zhang (cherry picked from commit 8092cfb6e7396e0c5fc319b86ce9dca2246210f2) Signed-off-by: Sebastiaan van Stijn Upstream-commit: 6ae2ec3fbe75886d8d73f93c46ced33258d86929 Component: engine --- components/engine/hack/dockerfile/install/containerd.installer | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/engine/hack/dockerfile/install/containerd.installer b/components/engine/hack/dockerfile/install/containerd.installer index 3b369259b6..8b15eb8ff6 100755 --- a/components/engine/hack/dockerfile/install/containerd.installer +++ b/components/engine/hack/dockerfile/install/containerd.installer @@ -4,7 +4,7 @@ # containerd is also pinned in vendor.conf. When updating the binary # version you may also need to update the vendor version to pick up bug # fixes or new APIs. -CONTAINERD_COMMIT=bb71b10fd8f58240ca47fbb579b9d1028eea7c84 # v1.2.5 +CONTAINERD_COMMIT=894b81a4b802e4eb2a91d1ce216b8817763c29fb # v1.2.6 install_containerd() { echo "Install containerd version $CONTAINERD_COMMIT" From 0c07852c4253885e1e50c48e57d299d6b9aeec68 Mon Sep 17 00:00:00 2001 From: Jintao Zhang Date: Sat, 6 Apr 2019 16:53:33 +0000 Subject: [PATCH 11/24] Update runc 029124da7af7360afa781a0234d1b083550f797c Signed-off-by: Jintao Zhang (cherry picked from commit d43a41d7af7c4858e4b2ed41775c34b92cd35179) Signed-off-by: Sebastiaan van Stijn Upstream-commit: ce875b746bdcc37ff1c92dc181c78ba619d02d24 Component: engine --- components/engine/hack/dockerfile/install/runc.installer | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/engine/hack/dockerfile/install/runc.installer b/components/engine/hack/dockerfile/install/runc.installer index 532a7f72ee..670cac5878 100755 --- a/components/engine/hack/dockerfile/install/runc.installer +++ b/components/engine/hack/dockerfile/install/runc.installer @@ -4,7 +4,7 @@ # The version of runc should match the version that is used by the containerd # version that is used. If you need to update runc, open a pull request in # the containerd project first, and update both after that is merged. -RUNC_COMMIT=2b18fe1d885ee5083ef9f0838fee39b62d653e30 +RUNC_COMMIT=029124da7af7360afa781a0234d1b083550f797c # v1.0.0-rc7-6-g029124da install_runc() { # If using RHEL7 kernels (3.10.0 el7), disable kmem accounting/limiting From 2db111a869ab059552254530cd65e6d2c6393103 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 25 Apr 2019 18:56:17 -0700 Subject: [PATCH 12/24] bump runc binary v1.0.0-rc8 full diff: https://github.com/opencontainers/runc/compare/029124da7af7360afa781a0234d1b083550f797c...425e105d5a03fabd737a126ad93d62a9eeede87f - opencontainers/runc#2043 Vendor in latest selinux code for keycreate errors Signed-off-by: Sebastiaan van Stijn (cherry picked from commit 4bc310c11babd2aa635eb08a5bbc198f96bc19b3) Signed-off-by: Sebastiaan van Stijn Upstream-commit: 1a1bf23d17e44d22ebec2b1d0e38329bd35b0127 Component: engine --- components/engine/hack/dockerfile/install/runc.installer | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/engine/hack/dockerfile/install/runc.installer b/components/engine/hack/dockerfile/install/runc.installer index 670cac5878..a8156db97f 100755 --- a/components/engine/hack/dockerfile/install/runc.installer +++ b/components/engine/hack/dockerfile/install/runc.installer @@ -4,7 +4,7 @@ # The version of runc should match the version that is used by the containerd # version that is used. If you need to update runc, open a pull request in # the containerd project first, and update both after that is merged. -RUNC_COMMIT=029124da7af7360afa781a0234d1b083550f797c # v1.0.0-rc7-6-g029124da +RUNC_COMMIT=425e105d5a03fabd737a126ad93d62a9eeede87f # v1.0.0-rc8 install_runc() { # If using RHEL7 kernels (3.10.0 el7), disable kmem accounting/limiting From 932cc247c502fbddde170da6edaa2495dd497e70 Mon Sep 17 00:00:00 2001 From: Justin Cormack Date: Fri, 7 Jun 2019 11:21:18 +0100 Subject: [PATCH 13/24] Entropy cannot be saved Remove non cryptographic randomness. Signed-off-by: Justin Cormack (cherry picked from commit 2df693e533e904f432c59279c07b2b8cbeece4f0) Signed-off-by: Sebastiaan van Stijn Upstream-commit: 292b43b15b68cd4b64bfc7b89452dc19ddf2cf48 Component: engine --- components/engine/container/container_unix.go | 2 +- components/engine/daemon/create_unix.go | 2 +- components/engine/daemon/create_windows.go | 2 +- components/engine/daemon/exec/exec.go | 2 +- .../daemon/graphdriver/aufs/aufs_test.go | 2 +- components/engine/daemon/logger/plugin.go | 2 +- components/engine/daemon/names.go | 2 +- ...er_cli_external_volume_driver_unix_test.go | 2 +- .../integration/container/rename_test.go | 4 +- components/engine/pkg/stringid/stringid.go | 44 ++----------------- .../engine/pkg/stringid/stringid_test.go | 8 ---- .../engine/pkg/truncindex/truncindex_test.go | 30 ++++++------- .../engine/plugin/manager_linux_test.go | 2 +- .../engine/volume/mounts/linux_parser.go | 2 +- components/engine/volume/mounts/mounts.go | 2 +- .../engine/volume/mounts/windows_parser.go | 2 +- components/engine/volume/service/service.go | 2 +- 17 files changed, 34 insertions(+), 78 deletions(-) diff --git a/components/engine/container/container_unix.go b/components/engine/container/container_unix.go index 6d402be3a0..8419bd5761 100644 --- a/components/engine/container/container_unix.go +++ b/components/engine/container/container_unix.go @@ -136,7 +136,7 @@ func (container *Container) CopyImagePathContent(v volume.Volume, destination st return err } - id := stringid.GenerateNonCryptoID() + id := stringid.GenerateRandomID() path, err := v.Mount(id) if err != nil { return err diff --git a/components/engine/daemon/create_unix.go b/components/engine/daemon/create_unix.go index 13857bab06..e78415aaef 100644 --- a/components/engine/daemon/create_unix.go +++ b/components/engine/daemon/create_unix.go @@ -41,7 +41,7 @@ func (daemon *Daemon) createContainerOSSpecificSettings(container *container.Con } for spec := range config.Volumes { - name := stringid.GenerateNonCryptoID() + name := stringid.GenerateRandomID() destination := filepath.Clean(spec) // Skip volumes for which we already have something mounted on that diff --git a/components/engine/daemon/create_windows.go b/components/engine/daemon/create_windows.go index 37e425a014..3bce278773 100644 --- a/components/engine/daemon/create_windows.go +++ b/components/engine/daemon/create_windows.go @@ -38,7 +38,7 @@ func (daemon *Daemon) createContainerOSSpecificSettings(container *container.Con // If the mountpoint doesn't have a name, generate one. if len(mp.Name) == 0 { - mp.Name = stringid.GenerateNonCryptoID() + mp.Name = stringid.GenerateRandomID() } // Skip volumes for which we already have something mounted on that diff --git a/components/engine/daemon/exec/exec.go b/components/engine/daemon/exec/exec.go index c036c46a0c..cf2a9556c1 100644 --- a/components/engine/daemon/exec/exec.go +++ b/components/engine/daemon/exec/exec.go @@ -39,7 +39,7 @@ type Config struct { // NewConfig initializes the a new exec configuration func NewConfig() *Config { return &Config{ - ID: stringid.GenerateNonCryptoID(), + ID: stringid.GenerateRandomID(), StreamConfig: stream.NewConfig(), Started: make(chan struct{}), } diff --git a/components/engine/daemon/graphdriver/aufs/aufs_test.go b/components/engine/daemon/graphdriver/aufs/aufs_test.go index fdc502ba65..0752c842b4 100644 --- a/components/engine/daemon/graphdriver/aufs/aufs_test.go +++ b/components/engine/daemon/graphdriver/aufs/aufs_test.go @@ -731,7 +731,7 @@ func BenchmarkConcurrentAccess(b *testing.B) { // create a bunch of ids var ids []string for i := 0; i < numConcurrent; i++ { - ids = append(ids, stringid.GenerateNonCryptoID()) + ids = append(ids, stringid.GenerateRandomID()) } if err := d.Create(ids[0], "", nil); err != nil { diff --git a/components/engine/daemon/logger/plugin.go b/components/engine/daemon/logger/plugin.go index c66540ce52..8c155b0ddb 100644 --- a/components/engine/daemon/logger/plugin.go +++ b/components/engine/daemon/logger/plugin.go @@ -81,7 +81,7 @@ func makePluginCreator(name string, l logPlugin, scopePath func(s string) string return nil, err } - id := stringid.GenerateNonCryptoID() + id := stringid.GenerateRandomID() a := &pluginAdapter{ driverName: name, id: id, diff --git a/components/engine/daemon/names.go b/components/engine/daemon/names.go index 6c31949777..4fa39af2ee 100644 --- a/components/engine/daemon/names.go +++ b/components/engine/daemon/names.go @@ -38,7 +38,7 @@ func (daemon *Daemon) registerName(container *container.Container) error { func (daemon *Daemon) generateIDAndName(name string) (string, string, error) { var ( err error - id = stringid.GenerateNonCryptoID() + id = stringid.GenerateRandomID() ) if name == "" { diff --git a/components/engine/integration-cli/docker_cli_external_volume_driver_unix_test.go b/components/engine/integration-cli/docker_cli_external_volume_driver_unix_test.go index b876d9f6fe..e9e92d4372 100644 --- a/components/engine/integration-cli/docker_cli_external_volume_driver_unix_test.go +++ b/components/engine/integration-cli/docker_cli_external_volume_driver_unix_test.go @@ -558,7 +558,7 @@ func (s *DockerExternalVolumeSuite) TestExternalVolumeDriverCapabilities(c *chec } func (s *DockerExternalVolumeSuite) TestExternalVolumeDriverOutOfBandDelete(c *check.C) { - driverName := stringid.GenerateNonCryptoID() + driverName := stringid.GenerateRandomID() p := newVolumePlugin(c, driverName) defer p.Close() diff --git a/components/engine/integration/container/rename_test.go b/components/engine/integration/container/rename_test.go index 25474a7cb9..24bbe98d53 100644 --- a/components/engine/integration/container/rename_test.go +++ b/components/engine/integration/container/rename_test.go @@ -61,7 +61,7 @@ func TestRenameStoppedContainer(t *testing.T) { assert.NilError(t, err) assert.Check(t, is.Equal("/"+oldName, inspect.Name)) - newName := "new_name" + stringid.GenerateNonCryptoID() + newName := "new_name" + stringid.GenerateRandomID() err = client.ContainerRename(ctx, oldName, newName) assert.NilError(t, err) @@ -79,7 +79,7 @@ func TestRenameRunningContainerAndReuse(t *testing.T) { cID := container.Run(t, ctx, client, container.WithName(oldName)) poll.WaitOn(t, container.IsInState(ctx, client, cID, "running"), poll.WithDelay(100*time.Millisecond)) - newName := "new_name" + stringid.GenerateNonCryptoID() + newName := "new_name" + stringid.GenerateRandomID() err := client.ContainerRename(ctx, oldName, newName) assert.NilError(t, err) diff --git a/components/engine/pkg/stringid/stringid.go b/components/engine/pkg/stringid/stringid.go index fa7d9166eb..5fe071d628 100644 --- a/components/engine/pkg/stringid/stringid.go +++ b/components/engine/pkg/stringid/stringid.go @@ -2,17 +2,12 @@ package stringid // import "github.com/docker/docker/pkg/stringid" import ( - cryptorand "crypto/rand" + "crypto/rand" "encoding/hex" "fmt" - "io" - "math" - "math/big" - "math/rand" "regexp" "strconv" "strings" - "time" ) const shortLen = 12 @@ -41,10 +36,11 @@ func TruncateID(id string) string { return id } -func generateID(r io.Reader) string { +// GenerateRandomID returns a unique id. +func GenerateRandomID() string { b := make([]byte, 32) for { - if _, err := io.ReadFull(r, b); err != nil { + if _, err := rand.Read(b); err != nil { panic(err) // This shouldn't happen } id := hex.EncodeToString(b) @@ -58,18 +54,6 @@ func generateID(r io.Reader) string { } } -// GenerateRandomID returns a unique id. -func GenerateRandomID() string { - return generateID(cryptorand.Reader) -} - -// GenerateNonCryptoID generates unique id without using cryptographically -// secure sources of random. -// It helps you to save entropy. -func GenerateNonCryptoID() string { - return generateID(readerFunc(rand.Read)) -} - // ValidateID checks whether an ID string is a valid image ID. func ValidateID(id string) error { if ok := validHex.MatchString(id); !ok { @@ -77,23 +61,3 @@ func ValidateID(id string) error { } return nil } - -func init() { - // safely set the seed globally so we generate random ids. Tries to use a - // crypto seed before falling back to time. - var seed int64 - if cryptoseed, err := cryptorand.Int(cryptorand.Reader, big.NewInt(math.MaxInt64)); err != nil { - // This should not happen, but worst-case fallback to time-based seed. - seed = time.Now().UnixNano() - } else { - seed = cryptoseed.Int64() - } - - rand.Seed(seed) -} - -type readerFunc func(p []byte) (int, error) - -func (fn readerFunc) Read(p []byte) (int, error) { - return fn(p) -} diff --git a/components/engine/pkg/stringid/stringid_test.go b/components/engine/pkg/stringid/stringid_test.go index a7ccd5faae..2660d2e65f 100644 --- a/components/engine/pkg/stringid/stringid_test.go +++ b/components/engine/pkg/stringid/stringid_test.go @@ -13,14 +13,6 @@ func TestGenerateRandomID(t *testing.T) { } } -func TestGenerateNonCryptoID(t *testing.T) { - id := GenerateNonCryptoID() - - if len(id) != 64 { - t.Fatalf("Id returned is incorrect: %s", id) - } -} - func TestShortenId(t *testing.T) { id := "90435eec5c4e124e741ef731e118be2fc799a68aba0466ec17717f24ce2ae6a2" truncID := TruncateID(id) diff --git a/components/engine/pkg/truncindex/truncindex_test.go b/components/engine/pkg/truncindex/truncindex_test.go index e259017982..6d00a245aa 100644 --- a/components/engine/pkg/truncindex/truncindex_test.go +++ b/components/engine/pkg/truncindex/truncindex_test.go @@ -158,7 +158,7 @@ func assertIndexGet(t *testing.T, index *TruncIndex, input, expectedResult strin func BenchmarkTruncIndexAdd100(b *testing.B) { var testSet []string for i := 0; i < 100; i++ { - testSet = append(testSet, stringid.GenerateNonCryptoID()) + testSet = append(testSet, stringid.GenerateRandomID()) } b.ResetTimer() for i := 0; i < b.N; i++ { @@ -174,7 +174,7 @@ func BenchmarkTruncIndexAdd100(b *testing.B) { func BenchmarkTruncIndexAdd250(b *testing.B) { var testSet []string for i := 0; i < 250; i++ { - testSet = append(testSet, stringid.GenerateNonCryptoID()) + testSet = append(testSet, stringid.GenerateRandomID()) } b.ResetTimer() for i := 0; i < b.N; i++ { @@ -190,7 +190,7 @@ func BenchmarkTruncIndexAdd250(b *testing.B) { func BenchmarkTruncIndexAdd500(b *testing.B) { var testSet []string for i := 0; i < 500; i++ { - testSet = append(testSet, stringid.GenerateNonCryptoID()) + testSet = append(testSet, stringid.GenerateRandomID()) } b.ResetTimer() for i := 0; i < b.N; i++ { @@ -207,7 +207,7 @@ func BenchmarkTruncIndexGet100(b *testing.B) { var testSet []string var testKeys []string for i := 0; i < 100; i++ { - testSet = append(testSet, stringid.GenerateNonCryptoID()) + testSet = append(testSet, stringid.GenerateRandomID()) } index := NewTruncIndex([]string{}) for _, id := range testSet { @@ -231,7 +231,7 @@ func BenchmarkTruncIndexGet250(b *testing.B) { var testSet []string var testKeys []string for i := 0; i < 250; i++ { - testSet = append(testSet, stringid.GenerateNonCryptoID()) + testSet = append(testSet, stringid.GenerateRandomID()) } index := NewTruncIndex([]string{}) for _, id := range testSet { @@ -255,7 +255,7 @@ func BenchmarkTruncIndexGet500(b *testing.B) { var testSet []string var testKeys []string for i := 0; i < 500; i++ { - testSet = append(testSet, stringid.GenerateNonCryptoID()) + testSet = append(testSet, stringid.GenerateRandomID()) } index := NewTruncIndex([]string{}) for _, id := range testSet { @@ -278,7 +278,7 @@ func BenchmarkTruncIndexGet500(b *testing.B) { func BenchmarkTruncIndexDelete100(b *testing.B) { var testSet []string for i := 0; i < 100; i++ { - testSet = append(testSet, stringid.GenerateNonCryptoID()) + testSet = append(testSet, stringid.GenerateRandomID()) } b.ResetTimer() for i := 0; i < b.N; i++ { @@ -301,7 +301,7 @@ func BenchmarkTruncIndexDelete100(b *testing.B) { func BenchmarkTruncIndexDelete250(b *testing.B) { var testSet []string for i := 0; i < 250; i++ { - testSet = append(testSet, stringid.GenerateNonCryptoID()) + testSet = append(testSet, stringid.GenerateRandomID()) } b.ResetTimer() for i := 0; i < b.N; i++ { @@ -324,7 +324,7 @@ func BenchmarkTruncIndexDelete250(b *testing.B) { func BenchmarkTruncIndexDelete500(b *testing.B) { var testSet []string for i := 0; i < 500; i++ { - testSet = append(testSet, stringid.GenerateNonCryptoID()) + testSet = append(testSet, stringid.GenerateRandomID()) } b.ResetTimer() for i := 0; i < b.N; i++ { @@ -347,7 +347,7 @@ func BenchmarkTruncIndexDelete500(b *testing.B) { func BenchmarkTruncIndexNew100(b *testing.B) { var testSet []string for i := 0; i < 100; i++ { - testSet = append(testSet, stringid.GenerateNonCryptoID()) + testSet = append(testSet, stringid.GenerateRandomID()) } b.ResetTimer() for i := 0; i < b.N; i++ { @@ -358,7 +358,7 @@ func BenchmarkTruncIndexNew100(b *testing.B) { func BenchmarkTruncIndexNew250(b *testing.B) { var testSet []string for i := 0; i < 250; i++ { - testSet = append(testSet, stringid.GenerateNonCryptoID()) + testSet = append(testSet, stringid.GenerateRandomID()) } b.ResetTimer() for i := 0; i < b.N; i++ { @@ -369,7 +369,7 @@ func BenchmarkTruncIndexNew250(b *testing.B) { func BenchmarkTruncIndexNew500(b *testing.B) { var testSet []string for i := 0; i < 500; i++ { - testSet = append(testSet, stringid.GenerateNonCryptoID()) + testSet = append(testSet, stringid.GenerateRandomID()) } b.ResetTimer() for i := 0; i < b.N; i++ { @@ -381,7 +381,7 @@ func BenchmarkTruncIndexAddGet100(b *testing.B) { var testSet []string var testKeys []string for i := 0; i < 500; i++ { - id := stringid.GenerateNonCryptoID() + id := stringid.GenerateRandomID() testSet = append(testSet, id) l := rand.Intn(12) + 12 testKeys = append(testKeys, id[:l]) @@ -406,7 +406,7 @@ func BenchmarkTruncIndexAddGet250(b *testing.B) { var testSet []string var testKeys []string for i := 0; i < 500; i++ { - id := stringid.GenerateNonCryptoID() + id := stringid.GenerateRandomID() testSet = append(testSet, id) l := rand.Intn(12) + 12 testKeys = append(testKeys, id[:l]) @@ -431,7 +431,7 @@ func BenchmarkTruncIndexAddGet500(b *testing.B) { var testSet []string var testKeys []string for i := 0; i < 500; i++ { - id := stringid.GenerateNonCryptoID() + id := stringid.GenerateRandomID() testSet = append(testSet, id) l := rand.Intn(12) + 12 testKeys = append(testKeys, id[:l]) diff --git a/components/engine/plugin/manager_linux_test.go b/components/engine/plugin/manager_linux_test.go index fd8fa8523c..1b6a3bf773 100644 --- a/components/engine/plugin/manager_linux_test.go +++ b/components/engine/plugin/manager_linux_test.go @@ -70,7 +70,7 @@ func TestManagerWithPluginMounts(t *testing.T) { } func newTestPlugin(t *testing.T, name, cap, root string) *v2.Plugin { - id := stringid.GenerateNonCryptoID() + id := stringid.GenerateRandomID() rootfs := filepath.Join(root, id) if err := os.MkdirAll(rootfs, 0755); err != nil { t.Fatal(err) diff --git a/components/engine/volume/mounts/linux_parser.go b/components/engine/volume/mounts/linux_parser.go index 8e436aec0e..b727e63ee5 100644 --- a/components/engine/volume/mounts/linux_parser.go +++ b/components/engine/volume/mounts/linux_parser.go @@ -292,7 +292,7 @@ func (p *linuxParser) parseMountSpec(cfg mount.Mount, validateBindSourceExists b switch cfg.Type { case mount.TypeVolume: if cfg.Source == "" { - mp.Name = stringid.GenerateNonCryptoID() + mp.Name = stringid.GenerateRandomID() } else { mp.Name = cfg.Source } diff --git a/components/engine/volume/mounts/mounts.go b/components/engine/volume/mounts/mounts.go index 63a1406814..5bf169f6e0 100644 --- a/components/engine/volume/mounts/mounts.go +++ b/components/engine/volume/mounts/mounts.go @@ -125,7 +125,7 @@ func (m *MountPoint) Setup(mountLabel string, rootIDs idtools.Identity, checkFun if m.Volume != nil { id := m.ID if id == "" { - id = stringid.GenerateNonCryptoID() + id = stringid.GenerateRandomID() } path, err := m.Volume.Mount(id) if err != nil { diff --git a/components/engine/volume/mounts/windows_parser.go b/components/engine/volume/mounts/windows_parser.go index ac61044043..8f427d8c50 100644 --- a/components/engine/volume/mounts/windows_parser.go +++ b/components/engine/volume/mounts/windows_parser.go @@ -385,7 +385,7 @@ func (p *windowsParser) parseMountSpec(cfg mount.Mount, destRegex string, conver switch cfg.Type { case mount.TypeVolume: if cfg.Source == "" { - mp.Name = stringid.GenerateNonCryptoID() + mp.Name = stringid.GenerateRandomID() } else { mp.Name = cfg.Source } diff --git a/components/engine/volume/service/service.go b/components/engine/volume/service/service.go index ebb5e205e9..f1fe5e724d 100644 --- a/components/engine/volume/service/service.go +++ b/components/engine/volume/service/service.go @@ -56,7 +56,7 @@ func (s *VolumesService) GetDriverList() []string { // Create creates a volume func (s *VolumesService) Create(ctx context.Context, name, driverName string, opts ...opts.CreateOption) (*types.Volume, error) { if name == "" { - name = stringid.GenerateNonCryptoID() + name = stringid.GenerateRandomID() } v, err := s.vs.Create(ctx, name, driverName, opts...) if err != nil { From 0addc7f6c542b0efced6454467c0c6bd2cd73e95 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 9 Jan 2019 22:47:33 +0100 Subject: [PATCH 14/24] Update docker-py to 3.7.0 Signed-off-by: Sebastiaan van Stijn (cherry picked from commit c0c05affc79b199248b457af16fff61c305b7623) Signed-off-by: Sebastiaan van Stijn Upstream-commit: b92e9e9da9519efa38d8a5927af668413cf81cd7 Component: engine --- components/engine/Dockerfile | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/components/engine/Dockerfile b/components/engine/Dockerfile index b76ae60909..15ccf0f829 100644 --- a/components/engine/Dockerfile +++ b/components/engine/Dockerfile @@ -77,7 +77,7 @@ RUN set -x \ FROM base AS docker-py # Get the "docker-py" source so we can run their integration tests -ENV DOCKER_PY_COMMIT 8b246db271a85d6541dc458838627e89c683e42f +ENV DOCKER_PY_COMMIT ac922192959870774ad8428344d9faa0555f7ba6 RUN git clone https://github.com/docker/docker-py.git /build \ && cd /build \ && git checkout -q $DOCKER_PY_COMMIT @@ -187,6 +187,9 @@ RUN apt-get update && apt-get install -y \ jq \ libcap2-bin \ libdevmapper-dev \ +# libffi-dev and libssl-dev appear to be required for compiling paramiko on s390x/ppc64le + libffi-dev \ + libssl-dev \ libudev-dev \ libsystemd-dev \ binutils-mingw-w64 \ @@ -195,6 +198,8 @@ RUN apt-get update && apt-get install -y \ pigz \ python-backports.ssl-match-hostname \ python-dev \ +# python-cffi appears to be required for compiling paramiko on s390x/ppc64le + python-cffi \ python-mock \ python-pip \ python-requests \ @@ -227,7 +232,8 @@ COPY --from=docker-py /build/ /docker-py # split out into a separate image, including all the `python-*` deps installed # above. RUN cd /docker-py \ - && pip install docker-pycreds==0.2.1 \ + && pip install docker-pycreds==0.4.0 \ + && pip install paramiko==2.4.2 \ && pip install yamllint==1.5.0 \ && pip install -r test-requirements.txt From cbbe578ba2a0b5659dda8f2329c0b275a93a70b1 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 20 Apr 2019 12:58:58 +0200 Subject: [PATCH 15/24] bump buildkit 05766c5c21a1e528eeb1c3522b2f05493fe9ac47 (docker-18.09 branch) - moby/buildkit#952 [18.09 backport] Have parser error on dockerfiles without instructions - backport of moby/buildkit#771 Have parser error on dockerfiles without instructions Signed-off-by: Sebastiaan van Stijn Upstream-commit: a1d76e0585c498f5167b28b9909fd8c88fd3370d Component: engine --- components/engine/vendor.conf | 2 +- .../moby/buildkit/frontend/dockerfile/parser/parser.go | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/components/engine/vendor.conf b/components/engine/vendor.conf index fac0ede8fb..95def4ba94 100644 --- a/components/engine/vendor.conf +++ b/components/engine/vendor.conf @@ -26,7 +26,7 @@ github.com/imdario/mergo v0.3.6 golang.org/x/sync 1d60e4601c6fd243af51cc01ddf169918a5407ca # buildkit -github.com/moby/buildkit ed4da8b4a9661f278ae8433056ca37d0727c408b # docker-18.09 branch +github.com/moby/buildkit 05766c5c21a1e528eeb1c3522b2f05493fe9ac47 # docker-18.09 branch github.com/tonistiigi/fsutil 2862f6bc5ac9b97124e552a5c108230b38a1b0ca github.com/grpc-ecosystem/grpc-opentracing 8e809c8a86450a29b90dcc9efbf062d0fe6d9746 github.com/opentracing/opentracing-go 1361b9cd60be79c4c3a7fa9841b3c132e40066a7 diff --git a/components/engine/vendor/github.com/moby/buildkit/frontend/dockerfile/parser/parser.go b/components/engine/vendor/github.com/moby/buildkit/frontend/dockerfile/parser/parser.go index 0453f3a9a2..262a768133 100644 --- a/components/engine/vendor/github.com/moby/buildkit/frontend/dockerfile/parser/parser.go +++ b/components/engine/vendor/github.com/moby/buildkit/frontend/dockerfile/parser/parser.go @@ -275,6 +275,11 @@ func Parse(rwc io.Reader) (*Result, error) { if len(warnings) > 0 { warnings = append(warnings, "[WARNING]: Empty continuation lines will become errors in a future release.") } + + if root.StartLine < 0 { + return nil, errors.New("file with no instructions.") + } + return &Result{ AST: root, Warnings: warnings, From a439056531c53363c1f308acfa2e59adf27c6962 Mon Sep 17 00:00:00 2001 From: linuxmercedes Date: Thu, 3 Jan 2019 15:49:48 -0600 Subject: [PATCH 16/24] Convert parse errors to more informative format - Wrap parse errors in errdefs.InvalidParameters - Include dockerfile in error names Signed-off-by: Natasha Jarus (cherry picked from commit 64466b0cd9ff36dc3f123a3a68eae438ac9e8e60) Signed-off-by: Sebastiaan van Stijn Upstream-commit: 7eb0e6a095ac71bfe3e3419cef7d5867fef55f20 Component: engine --- .../engine/builder/remotecontext/detect.go | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/components/engine/builder/remotecontext/detect.go b/components/engine/builder/remotecontext/detect.go index 144eb570ab..1becd0fd59 100644 --- a/components/engine/builder/remotecontext/detect.go +++ b/components/engine/builder/remotecontext/detect.go @@ -12,6 +12,7 @@ import ( "github.com/docker/docker/api/types/backend" "github.com/docker/docker/builder" "github.com/docker/docker/builder/dockerignore" + "github.com/docker/docker/errdefs" "github.com/docker/docker/pkg/fileutils" "github.com/docker/docker/pkg/urlutil" "github.com/moby/buildkit/frontend/dockerfile/parser" @@ -34,8 +35,9 @@ func Detect(config backend.BuildConfig) (remote builder.Source, dockerfile *pars case remoteURL == ClientSessionRemote: res, err := parser.Parse(config.Source) if err != nil { - return nil, nil, err + return nil, nil, errdefs.InvalidParameter(err) } + return nil, res, nil case urlutil.IsGitURL(remoteURL): remote, dockerfile, err = newGitRemote(remoteURL, dockerfilePath) @@ -106,7 +108,7 @@ func newURLRemote(url string, dockerfilePath string, progressReader func(in io.R switch contentType { case mimeTypes.TextPlain: res, err := parser.Parse(progressReader(content)) - return nil, res, err + return nil, res, errdefs.InvalidParameter(err) default: source, err := FromArchive(progressReader(content)) if err != nil { @@ -146,11 +148,17 @@ func readAndParseDockerfile(name string, rc io.Reader) (*parser.Result, error) { br := bufio.NewReader(rc) if _, err := br.Peek(1); err != nil { if err == io.EOF { - return nil, errors.Errorf("the Dockerfile (%s) cannot be empty", name) + return nil, errdefs.InvalidParameter(errors.Errorf("the Dockerfile (%s) cannot be empty", name)) } return nil, errors.Wrap(err, "unexpected error reading Dockerfile") } - return parser.Parse(br) + + dockerfile, err := parser.Parse(br) + if err != nil { + return nil, errdefs.InvalidParameter(errors.Wrapf(err, "failed to parse %s", name)) + } + + return dockerfile, nil } func openAt(remote builder.Source, path string) (driver.File, error) { From a3041ba7dd49395f782adf3b4178ec5c5b265d23 Mon Sep 17 00:00:00 2001 From: linuxmercedes Date: Thu, 3 Jan 2019 15:50:20 -0600 Subject: [PATCH 17/24] Test: dockerfiles with no instructions are detected Signed-off-by: Natasha Jarus (cherry picked from commit 18c7e8b927928654a67ce5d6637e42932fc3e7df) Signed-off-by: Sebastiaan van Stijn Upstream-commit: cd084b23f7c3ca38c5d9c15ec16e119d25e5d7af Component: engine --- .../engine/integration/build/build_test.go | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/components/engine/integration/build/build_test.go b/components/engine/integration/build/build_test.go index 6a2b1fda9f..6fe18fc5b1 100644 --- a/components/engine/integration/build/build_test.go +++ b/components/engine/integration/build/build_test.go @@ -474,6 +474,61 @@ RUN for g in $(seq 0 8); do dd if=/dev/urandom of=rnd bs=1K count=1 seek=$((1024 assert.Check(t, is.Contains(out.String(), "Successfully built")) } +func TestBuildWithEmptyDockerfile(t *testing.T) { + ctx := context.TODO() + defer setupTest(t)() + + tests := []struct { + name string + dockerfile string + expectedErr string + }{ + { + name: "empty-dockerfile", + dockerfile: "", + expectedErr: "cannot be empty", + }, + { + name: "empty-lines-dockerfile", + dockerfile: ` + + + + `, + expectedErr: "file with no instructions", + }, + { + name: "comment-only-dockerfile", + dockerfile: `# this is a comment`, + expectedErr: "file with no instructions", + }, + } + + apiclient := testEnv.APIClient() + + for _, tc := range tests { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + buf := bytes.NewBuffer(nil) + w := tar.NewWriter(buf) + writeTarRecord(t, w, "Dockerfile", tc.dockerfile) + err := w.Close() + assert.NilError(t, err) + + _, err = apiclient.ImageBuild(ctx, + buf, + types.ImageBuildOptions{ + Remove: true, + ForceRemove: true, + }) + + assert.Check(t, is.Contains(err.Error(), tc.expectedErr)) + }) + } +} + func writeTarRecord(t *testing.T, w *tar.Writer, fn, contents string) { err := w.WriteHeader(&tar.Header{ Name: fn, From 36954599b402e444022bad7c638c74366badc458 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 18 Sep 2018 13:28:49 -0700 Subject: [PATCH 18/24] go {build,test}: rm -i option, add go cache volume Looks like -i (together with DOCKER_INCREMENTAL_BINARY etc) were used to get faster incremental builds. Nowdays (since Go 1.10) this is no longer the case, as go build cache is used [1]. Here's a quote: > You do not have to use "go test -i" or "go build -i" or > "go install" just to get fast incremental builds. We will > not have to teach new users those workarounds anymore. > Everything will just be fast. To enable go cache between builds, add a volume for /root/.cache. [1] https://groups.google.com/forum/#!msg/golang-dev/qfa3mHN4ZPA/X2UzjNV1BAAJ Signed-off-by: Kir Kolyshkin (cherry picked from commit bdcd81d3301a053eefc320de16ac842ec47ed459) Signed-off-by: Sebastiaan van Stijn Upstream-commit: 0f190f798f7b9f90bd008fe8fb0fc087ec965642 Component: engine --- components/engine/Makefile | 31 ++++++------------- .../docs/contributing/set-up-dev-env.md | 2 +- .../hack/integration-cli-on-swarm/README.md | 1 - components/engine/hack/make.sh | 8 ----- components/engine/hack/test/unit | 4 --- 5 files changed, 11 insertions(+), 35 deletions(-) diff --git a/components/engine/Makefile b/components/engine/Makefile index 7767409a6f..2c64b809d7 100644 --- a/components/engine/Makefile +++ b/components/engine/Makefile @@ -1,10 +1,8 @@ -.PHONY: all binary dynbinary build cross help init-go-pkg-cache install manpages run shell test test-docker-py test-integration test-unit validate win +.PHONY: all binary dynbinary build cross help install manpages run shell test test-docker-py test-integration test-unit validate win # set the graph driver as the current graphdriver if not set DOCKER_GRAPHDRIVER := $(if $(DOCKER_GRAPHDRIVER),$(DOCKER_GRAPHDRIVER),$(shell docker info 2>&1 | grep "Storage Driver" | sed 's/.*: //')) export DOCKER_GRAPHDRIVER -DOCKER_INCREMENTAL_BINARY := $(if $(DOCKER_INCREMENTAL_BINARY),$(DOCKER_INCREMENTAL_BINARY),1) -export DOCKER_INCREMENTAL_BINARY # get OS/Arch of docker engine DOCKER_OSARCH := $(shell bash -c 'source hack/make/.detect-daemon-osarch && echo $${DOCKER_ENGINE_OSARCH}') @@ -44,7 +42,6 @@ DOCKER_ENVS := \ -e DOCKER_EXPERIMENTAL \ -e DOCKER_GITCOMMIT \ -e DOCKER_GRAPHDRIVER \ - -e DOCKER_INCREMENTAL_BINARY \ -e DOCKER_LDFLAGS \ -e DOCKER_PORT \ -e DOCKER_REMAP_ROOT \ @@ -84,14 +81,10 @@ DOCKER_MOUNT := $(if $(DOCKER_MOUNT),$(DOCKER_MOUNT),-v /go/src/github.com/docke # This allows to set the docker-dev container name DOCKER_CONTAINER_NAME := $(if $(CONTAINER_NAME),--name $(CONTAINER_NAME),) -# enable package cache if DOCKER_INCREMENTAL_BINARY and DOCKER_MOUNT (i.e.DOCKER_HOST) are set -PKGCACHE_MAP := gopath:/go/pkg goroot-linux_amd64:/usr/local/go/pkg/linux_amd64 goroot-linux_amd64_netgo:/usr/local/go/pkg/linux_amd64_netgo -PKGCACHE_VOLROOT := dockerdev-go-pkg-cache -PKGCACHE_VOL := $(if $(PKGCACHE_DIR),$(CURDIR)/$(PKGCACHE_DIR)/,$(PKGCACHE_VOLROOT)-) -DOCKER_MOUNT_PKGCACHE := $(if $(DOCKER_INCREMENTAL_BINARY),$(shell echo $(PKGCACHE_MAP) | sed -E 's@([^ ]*)@-v "$(PKGCACHE_VOL)\1"@g'),) +DOCKER_MOUNT_CACHE := -v docker-dev-cache:/root/.cache DOCKER_MOUNT_CLI := $(if $(DOCKER_CLI_PATH),-v $(shell dirname $(DOCKER_CLI_PATH)):/usr/local/cli,) DOCKER_MOUNT_BASH_COMPLETION := $(if $(DOCKER_BASH_COMPLETION_PATH),-v $(shell dirname $(DOCKER_BASH_COMPLETION_PATH)):/usr/local/completion/bash,) -DOCKER_MOUNT := $(DOCKER_MOUNT) $(DOCKER_MOUNT_PKGCACHE) $(DOCKER_MOUNT_CLI) $(DOCKER_MOUNT_BASH_COMPLETION) +DOCKER_MOUNT := $(DOCKER_MOUNT) $(DOCKER_MOUNT_CACHE) $(DOCKER_MOUNT_CLI) $(DOCKER_MOUNT_BASH_COMPLETION) GIT_BRANCH := $(shell git rev-parse --abbrev-ref HEAD 2>/dev/null) GIT_BRANCH_CLEAN := $(shell echo $(GIT_BRANCH) | sed -e "s/[^[:alnum:]]/-/g") @@ -133,19 +126,19 @@ binary: build ## build the linux binaries dynbinary: build ## build the linux dynbinaries $(DOCKER_RUN_DOCKER) hack/make.sh dynbinary -build: bundles init-go-pkg-cache +build: bundles $(warning The docker client CLI has moved to github.com/docker/cli. For a dev-test cycle involving the CLI, run:${\n} DOCKER_CLI_PATH=/host/path/to/cli/binary make shell ${\n} then change the cli and compile into a binary at the same location.${\n}) docker build ${BUILD_APT_MIRROR} ${DOCKER_BUILD_ARGS} -t "$(DOCKER_IMAGE)" -f "$(DOCKERFILE)" . bundles: mkdir bundles -clean: clean-pkg-cache-vol ## clean up cached resources +.PHONY: clean +clean: clean-cache -clean-pkg-cache-vol: - @- $(foreach mapping,$(PKGCACHE_MAP), \ - $(shell docker volume rm $(PKGCACHE_VOLROOT)-$(shell echo $(mapping) | awk -F':/' '{ print $$1 }') > /dev/null 2>&1) \ - ) +.PHONY: clean-cache +clean-cache: + docker volume rm -f docker-dev-cache cross: build ## cross build the binaries for darwin, freebsd and\nwindows $(DOCKER_RUN_DOCKER) hack/make.sh dynbinary binary cross @@ -153,9 +146,6 @@ cross: build ## cross build the binaries for darwin, freebsd and\nwindows help: ## this help @awk 'BEGIN {FS = ":.*?## "} /^[a-zA-Z_-]+:.*?## / {sub("\\\\n",sprintf("\n%22c"," "), $$2);printf "\033[36m%-20s\033[0m %s\n", $$1, $$2}' $(MAKEFILE_LIST) -init-go-pkg-cache: - $(if $(PKGCACHE_DIR), mkdir -p $(shell echo $(PKGCACHE_MAP) | sed -E 's@([^: ]*):[^ ]*@$(PKGCACHE_DIR)/\1@g')) - install: ## install the linux binaries KEEPBUNDLE=1 hack/make.sh install-binary @@ -206,12 +196,11 @@ build-integration-cli-on-swarm: build ## build images and binary for running int go build -buildmode=pie -o ./hack/integration-cli-on-swarm/integration-cli-on-swarm ./hack/integration-cli-on-swarm/host @echo "Building $(INTEGRATION_CLI_MASTER_IMAGE)" docker build -t $(INTEGRATION_CLI_MASTER_IMAGE) hack/integration-cli-on-swarm/agent -# For worker, we don't use `docker build` so as to enable DOCKER_INCREMENTAL_BINARY and so on @echo "Building $(INTEGRATION_CLI_WORKER_IMAGE) from $(DOCKER_IMAGE)" $(eval tmp := integration-cli-worker-tmp) # We mount pkgcache, but not bundle (bundle needs to be baked into the image) # For avoiding bakings DOCKER_GRAPHDRIVER and so on to image, we cannot use $(DOCKER_ENVS) here - docker run -t -d --name $(tmp) -e DOCKER_GITCOMMIT -e BUILDFLAGS -e DOCKER_INCREMENTAL_BINARY --privileged $(DOCKER_MOUNT_PKGCACHE) $(DOCKER_IMAGE) top + docker run -t -d --name $(tmp) -e DOCKER_GITCOMMIT -e BUILDFLAGS --privileged $(DOCKER_IMAGE) top docker exec $(tmp) hack/make.sh build-integration-test-binary dynbinary docker exec $(tmp) go build -buildmode=pie -o /worker github.com/docker/docker/hack/integration-cli-on-swarm/agent/worker docker commit -c 'ENTRYPOINT ["/worker"]' $(tmp) $(INTEGRATION_CLI_WORKER_IMAGE) diff --git a/components/engine/docs/contributing/set-up-dev-env.md b/components/engine/docs/contributing/set-up-dev-env.md index 3d56c0b8c7..f0619cb464 100644 --- a/components/engine/docs/contributing/set-up-dev-env.md +++ b/components/engine/docs/contributing/set-up-dev-env.md @@ -130,7 +130,7 @@ can take over 15 minutes to complete. ```none Successfully built 3d872560918e Successfully tagged docker-dev:dry-run-test - docker run --rm -i --privileged -e BUILDFLAGS -e KEEPBUNDLE -e DOCKER_BUILD_GOGC -e DOCKER_BUILD_PKGS -e DOCKER_CLIENTONLY -e DOCKER_DEBUG -e DOCKER_EXPERIMENTAL -e DOCKER_GITCOMMIT -e DOCKER_GRAPHDRIVER=devicemapper -e DOCKER_INCREMENTAL_BINARY -e DOCKER_REMAP_ROOT -e DOCKER_STORAGE_OPTS -e DOCKER_USERLANDPROXY -e TESTDIRS -e TESTFLAGS -e TIMEOUT -v "home/ubuntu/repos/docker/bundles:/go/src/github.com/docker/docker/bundles" -t "docker-dev:dry-run-test" bash + docker run --rm -i --privileged -e BUILDFLAGS -e KEEPBUNDLE -e DOCKER_BUILD_GOGC -e DOCKER_BUILD_PKGS -e DOCKER_CLIENTONLY -e DOCKER_DEBUG -e DOCKER_EXPERIMENTAL -e DOCKER_GITCOMMIT -e DOCKER_GRAPHDRIVER=devicemapper -e DOCKER_REMAP_ROOT -e DOCKER_STORAGE_OPTS -e DOCKER_USERLANDPROXY -e TESTDIRS -e TESTFLAGS -e TIMEOUT -v "home/ubuntu/repos/docker/bundles:/go/src/github.com/docker/docker/bundles" -t "docker-dev:dry-run-test" bash # ``` diff --git a/components/engine/hack/integration-cli-on-swarm/README.md b/components/engine/hack/integration-cli-on-swarm/README.md index 4f4f67d4f4..852b36cea1 100644 --- a/components/engine/hack/integration-cli-on-swarm/README.md +++ b/components/engine/hack/integration-cli-on-swarm/README.md @@ -36,7 +36,6 @@ while the client is supposed to be running on a laptop, e.g. Docker for Mac/Wind Following environment variables are known to work in this step: - `BUILDFLAGS` - - `DOCKER_INCREMENTAL_BINARY` Note: during the transition into Moby Project, you might need to create a symbolic link `$GOPATH/src/github.com/docker/docker` to `$GOPATH/src/github.com/moby/moby`. diff --git a/components/engine/hack/make.sh b/components/engine/hack/make.sh index 2f4ece3cdb..62c72a09e2 100755 --- a/components/engine/hack/make.sh +++ b/components/engine/hack/make.sh @@ -148,14 +148,6 @@ EXTLDFLAGS_STATIC='-static' ORIG_BUILDFLAGS=( -tags "autogen netgo osusergo static_build $DOCKER_BUILDTAGS" -installsuffix netgo ) # see https://github.com/golang/go/issues/9369#issuecomment-69864440 for why -installsuffix is necessary here -# When $DOCKER_INCREMENTAL_BINARY is set in the environment, enable incremental -# builds by installing dependent packages to the GOPATH. -REBUILD_FLAG="-a" -if [ "$DOCKER_INCREMENTAL_BINARY" == "1" ] || [ "$DOCKER_INCREMENTAL_BINARY" == "true" ]; then - REBUILD_FLAG="-i" -fi -ORIG_BUILDFLAGS+=( $REBUILD_FLAG ) - BUILDFLAGS=( $BUILDFLAGS "${ORIG_BUILDFLAGS[@]}" ) # Test timeout. diff --git a/components/engine/hack/test/unit b/components/engine/hack/test/unit index d0e85f1ad1..ac27f68c30 100755 --- a/components/engine/hack/test/unit +++ b/components/engine/hack/test/unit @@ -19,10 +19,6 @@ TESTDIRS="${TESTDIRS:-"./..."}" exclude_paths="/vendor/|/integration" pkg_list=$(go list $TESTDIRS | grep -vE "($exclude_paths)") -# install test dependencies once before running tests for each package. This -# significantly reduces the runtime. -go test -i "${BUILDFLAGS[@]}" $pkg_list - for pkg in $pkg_list; do go test "${BUILDFLAGS[@]}" \ -cover \ From 7e0220b6d6949682f5fee9b6f981aa2379bd8e04 Mon Sep 17 00:00:00 2001 From: Jean Rouge Date: Sat, 6 Oct 2018 11:06:45 -0700 Subject: [PATCH 19/24] Allow to override the Makefile's `DOCKER_MOUNT` variable Through the env variable of the same name. The idea here is pretty simple: I/O perf on native mounted disks on non-Linux (notably Mac OS) is just terrible, thus making it a real pain to develop: one has to choose between re-building the image after every single change (eg to run a test) or just work directly inside the same container (eg with vim, but even then one would have to re-configure their dev container every time it gets destroyed - containers, after all, are not supposed to be long-lived). Allowing to override `DOCKER_MOUNT` makes it easy for everyone to decide what their volume/syncing strategy is; for example one can choose to use [docker-sync](https://github.com/EugenMayer/docker-sync) This patch won't change anything for anyone who doesn't set the `DOCKER_MOUNT` env variable in their environment. Signed-off-by: Jean Rouge (cherry picked from commit aea6fdf3d340835a1b0af208839ce42ace3a5b89) Signed-off-by: Sebastiaan van Stijn Upstream-commit: ecc423df7fde7f0edc963f03d70da133f03bb63c Component: engine --- components/engine/Makefile | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/components/engine/Makefile b/components/engine/Makefile index 2c64b809d7..741b05c4ab 100644 --- a/components/engine/Makefile +++ b/components/engine/Makefile @@ -71,6 +71,9 @@ DOCKER_ENVS := \ # (default to no bind mount if DOCKER_HOST is set) # note: BINDDIR is supported for backwards-compatibility here BIND_DIR := $(if $(BINDDIR),$(BINDDIR),$(if $(DOCKER_HOST),,bundles)) + +# DOCKER_MOUNT can be overriden, but use at your own risk! +ifndef DOCKER_MOUNT DOCKER_MOUNT := $(if $(BIND_DIR),-v "$(CURDIR)/$(BIND_DIR):/go/src/github.com/docker/docker/$(BIND_DIR)") # This allows the test suite to be able to run without worrying about the underlying fs used by the container running the daemon (e.g. aufs-on-aufs), so long as the host running the container is running a supported fs. @@ -78,13 +81,14 @@ DOCKER_MOUNT := $(if $(BIND_DIR),-v "$(CURDIR)/$(BIND_DIR):/go/src/github.com/do # Note that `BIND_DIR` will already be set to `bundles` if `DOCKER_HOST` is not set (see above BIND_DIR line), in such case this will do nothing since `DOCKER_MOUNT` will already be set. DOCKER_MOUNT := $(if $(DOCKER_MOUNT),$(DOCKER_MOUNT),-v /go/src/github.com/docker/docker/bundles) -v "$(CURDIR)/.git:/go/src/github.com/docker/docker/.git" -# This allows to set the docker-dev container name -DOCKER_CONTAINER_NAME := $(if $(CONTAINER_NAME),--name $(CONTAINER_NAME),) - DOCKER_MOUNT_CACHE := -v docker-dev-cache:/root/.cache DOCKER_MOUNT_CLI := $(if $(DOCKER_CLI_PATH),-v $(shell dirname $(DOCKER_CLI_PATH)):/usr/local/cli,) DOCKER_MOUNT_BASH_COMPLETION := $(if $(DOCKER_BASH_COMPLETION_PATH),-v $(shell dirname $(DOCKER_BASH_COMPLETION_PATH)):/usr/local/completion/bash,) DOCKER_MOUNT := $(DOCKER_MOUNT) $(DOCKER_MOUNT_CACHE) $(DOCKER_MOUNT_CLI) $(DOCKER_MOUNT_BASH_COMPLETION) +endif # ifndef DOCKER_MOUNT + +# This allows to set the docker-dev container name +DOCKER_CONTAINER_NAME := $(if $(CONTAINER_NAME),--name $(CONTAINER_NAME),) GIT_BRANCH := $(shell git rev-parse --abbrev-ref HEAD 2>/dev/null) GIT_BRANCH_CLEAN := $(shell echo $(GIT_BRANCH) | sed -e "s/[^[:alnum:]]/-/g") From e6d06f71607ac96b402a64ed56e07abba219b3bd Mon Sep 17 00:00:00 2001 From: Mohammad Nasirifar Date: Thu, 13 Dec 2018 20:26:10 -0500 Subject: [PATCH 20/24] Use BuildKit to skip source code COPY if BIND_DIR set build the final stage of the Dockerfile (including COPY ...) if no BIND_DIR is used. if BIND_DIR is used, build the dev stage, thus skipping the COPY. Original author: @thaJeztah Signed-off-by: Mohammad Nasirifar (cherry picked from commit e6d7df2e5d313800414b955e10a26d6687e7a1bf) Signed-off-by: Sebastiaan van Stijn Upstream-commit: e5a039169f389120be3fbc2a9db45ef8bc99eae5 Component: engine --- components/engine/Dockerfile | 2 ++ components/engine/Makefile | 6 +++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/components/engine/Dockerfile b/components/engine/Dockerfile index 15ccf0f829..a2dcbb8624 100644 --- a/components/engine/Dockerfile +++ b/components/engine/Dockerfile @@ -245,5 +245,7 @@ WORKDIR /go/src/github.com/docker/docker VOLUME /var/lib/docker # Wrap all commands in the "docker-in-docker" script to allow nested containers ENTRYPOINT ["hack/dind"] + +FROM dev AS final # Upload docker source COPY . /go/src/github.com/docker/docker diff --git a/components/engine/Makefile b/components/engine/Makefile index 741b05c4ab..8c2c66030e 100644 --- a/components/engine/Makefile +++ b/components/engine/Makefile @@ -34,6 +34,7 @@ DOCKER_ENVS := \ -e KEEPBUNDLE \ -e DOCKER_BUILD_ARGS \ -e DOCKER_BUILD_GOGC \ + -e DOCKER_BUILD_OPTS \ -e DOCKER_BUILD_PKGS \ -e DOCKER_BUILDKIT \ -e DOCKER_BASH_COMPLETION_PATH \ @@ -116,6 +117,9 @@ INTERACTIVE := $(shell [ -t 0 ] && echo 1 || echo 0) ifeq ($(INTERACTIVE), 1) DOCKER_FLAGS += -t endif +ifeq ($(BIND_DIR), .) + DOCKER_BUILD_OPTS += --target=dev +endif DOCKER_RUN_DOCKER := $(DOCKER_FLAGS) "$(DOCKER_IMAGE)" @@ -132,7 +136,7 @@ dynbinary: build ## build the linux dynbinaries build: bundles $(warning The docker client CLI has moved to github.com/docker/cli. For a dev-test cycle involving the CLI, run:${\n} DOCKER_CLI_PATH=/host/path/to/cli/binary make shell ${\n} then change the cli and compile into a binary at the same location.${\n}) - docker build ${BUILD_APT_MIRROR} ${DOCKER_BUILD_ARGS} -t "$(DOCKER_IMAGE)" -f "$(DOCKERFILE)" . + docker build ${BUILD_APT_MIRROR} ${DOCKER_BUILD_ARGS} ${DOCKER_BUILD_OPTS} -t "$(DOCKER_IMAGE)" -f "$(DOCKERFILE)" . bundles: mkdir bundles From fbaec0bb9e83038e730cf7fa7fa29d7ce498536b Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 10 Jan 2019 12:56:26 +0100 Subject: [PATCH 21/24] Makefile: make help: fix newline wrapping, and missing targets This patch; - adds support for multiple newlines - removes the 1-space indentation of wrapped lines - allows numerical characters in targets (0-9) Given these targets: ```Makefile .PHONY: foobar foobar: ## runs the foobar lorum ipsum.\nand so pn\nand so on echo foobar .PHONY: e2e-tests e2e-tests: ## runs the end-to-end tests echo e2e-tests ``` Before this change, the output of `make help` was ``` foobar runs the foobar lorum ipsum. and so pn\nand so on ``` After this change, the output is: ``` foobar runs the foobar lorum ipsum. and so pn and so on e2e-tests runs the end-to-end tests ``` Signed-off-by: Sebastiaan van Stijn (cherry picked from commit 202c9d8c98614e7cce2017f5c99d3d783fe8b509) Signed-off-by: Sebastiaan van Stijn Upstream-commit: 733b86683f347f922affc714d305d9f03fe9b47e Component: engine --- components/engine/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/engine/Makefile b/components/engine/Makefile index 8c2c66030e..686b62ffcf 100644 --- a/components/engine/Makefile +++ b/components/engine/Makefile @@ -152,7 +152,7 @@ cross: build ## cross build the binaries for darwin, freebsd and\nwindows $(DOCKER_RUN_DOCKER) hack/make.sh dynbinary binary cross help: ## this help - @awk 'BEGIN {FS = ":.*?## "} /^[a-zA-Z_-]+:.*?## / {sub("\\\\n",sprintf("\n%22c"," "), $$2);printf "\033[36m%-20s\033[0m %s\n", $$1, $$2}' $(MAKEFILE_LIST) + @awk 'BEGIN {FS = ":.*?## "} /^[a-zA-Z0-9_-]+:.*?## / {gsub("\\\\n",sprintf("\n%22c",""), $$2);printf "\033[36m%-20s\033[0m %s\n", $$1, $$2}' $(MAKEFILE_LIST) install: ## install the linux binaries KEEPBUNDLE=1 hack/make.sh install-binary From 00d14a4862b7276534fbaf2405fc62be36781226 Mon Sep 17 00:00:00 2001 From: Olli Janatuinen Date: Mon, 7 Jan 2019 19:05:54 +0200 Subject: [PATCH 22/24] CI: Introduce flaky test finder comparing PR commit(s) to HEAD of moby/moby master branch and if founds new (or renamed) integration tests will run stress tests for them. Signed-off-by: Olli Janatuinen (cherry picked from commit 8a8fd37f6fb53716cb4b3a7e93e1e3cf385927e2) Signed-off-by: Sebastiaan van Stijn Upstream-commit: e5c0923b277055c977994ebfdc689346c3104a16 Component: engine --- components/engine/Makefile | 3 +++ components/engine/hack/ci/janky | 1 + .../engine/hack/make/test-integration-flaky | 26 +++++++++++++++++++ 3 files changed, 30 insertions(+) create mode 100644 components/engine/hack/make/test-integration-flaky diff --git a/components/engine/Makefile b/components/engine/Makefile index 686b62ffcf..67f6c3dab1 100644 --- a/components/engine/Makefile +++ b/components/engine/Makefile @@ -174,6 +174,9 @@ test-integration-cli: test-integration ## (DEPRECATED) use test-integration test-integration: build ## run the integration tests $(DOCKER_RUN_DOCKER) hack/make.sh dynbinary test-integration +test-integration-flaky: build ## run the stress test for all new integration tests + $(DOCKER_RUN_DOCKER) hack/make.sh dynbinary test-integration-flaky + test-unit: build ## run the unit tests $(DOCKER_RUN_DOCKER) hack/test/unit diff --git a/components/engine/hack/ci/janky b/components/engine/hack/ci/janky index f2bdfbf326..88cb9d9c61 100755 --- a/components/engine/hack/ci/janky +++ b/components/engine/hack/ci/janky @@ -13,5 +13,6 @@ hack/make.sh \ binary-daemon \ dynbinary \ test-docker-py \ + test-integration-flaky \ test-integration \ cross diff --git a/components/engine/hack/make/test-integration-flaky b/components/engine/hack/make/test-integration-flaky new file mode 100644 index 0000000000..00723a75bb --- /dev/null +++ b/components/engine/hack/make/test-integration-flaky @@ -0,0 +1,26 @@ +#!/usr/bin/env bash +set -e -o pipefail + +source hack/validate/.validate +new_tests=$( + validate_diff --diff-filter=ACMR --unified=0 -- 'integration/*_test.go' | + grep -E '^(\+func )(.*)(\*testing)' || true +) + +if [ -z "$new_tests" ]; then + echo 'No new tests added to integration.' + return +fi + +echo +echo "Found new integrations tests:" +echo "$new_tests" +echo "Running stress test for them." + +( + TESTARRAY=$(echo "$new_tests" | sed 's/+func //' | awk -F'\\(' '{print $1}' | tr '\n' '|') + export TESTFLAGS="-test.count 5 -test.run ${TESTARRAY%?}" + export TEST_REPEAT=5 + echo "Using test flags: $TESTFLAGS" + source hack/make/test-integration +) From 1247206ab1c9623151a82b6c223ba9948a8c522f Mon Sep 17 00:00:00 2001 From: Tibor Vass Date: Fri, 8 Feb 2019 00:37:41 +0000 Subject: [PATCH 23/24] hack: Have TIMEOUT take -test.count into account when testing for flakiness Signed-off-by: Tibor Vass (cherry picked from commit 42dcfc894a64e9b4c1751b21db1537b3b68a36d8) Signed-off-by: Sebastiaan van Stijn Upstream-commit: a9c1bfc1b1cd94968b399ffdddef4b908a722bb3 Component: engine --- components/engine/hack/make/test-integration-flaky | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/components/engine/hack/make/test-integration-flaky b/components/engine/hack/make/test-integration-flaky index 00723a75bb..14fb034b75 100644 --- a/components/engine/hack/make/test-integration-flaky +++ b/components/engine/hack/make/test-integration-flaky @@ -19,8 +19,19 @@ echo "Running stress test for them." ( TESTARRAY=$(echo "$new_tests" | sed 's/+func //' | awk -F'\\(' '{print $1}' | tr '\n' '|') - export TESTFLAGS="-test.count 5 -test.run ${TESTARRAY%?}" + # Note: TEST_REPEAT will make the test suite run 5 times, restarting the daemon + # whereas testcount will make each test run 5 times in a row under the same daemon. + # This will make a total of 25 runs for each test in TESTARRAY. export TEST_REPEAT=5 + local testcount=5 + # However, TIMEOUT needs to take testcount into account, or a premature time out may happen. + # The following ugliness will: + # - remove last character (usually 'm' from '10m') + # - multiply by testcount + # - add last character back + export TIMEOUT=$((${TIMEOUT::-1} * $testcount))${TIMEOUT:$((${#TIMEOUT}-1)):1} + + export TESTFLAGS="-test.count $testcount -test.run ${TESTARRAY%?}" echo "Using test flags: $TESTFLAGS" source hack/make/test-integration ) From 138981a2071a62d3ef069d48f9643c7b9de373e8 Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Sat, 9 Mar 2019 18:27:26 -0800 Subject: [PATCH 24/24] Enable buildkit for Makefile build target This is set only if it is not already set. This should give a little speedup to CI builds. Signed-off-by: Brian Goff (cherry picked from commit 1275a001a68722494d090d5beca6749a83710cc2) Signed-off-by: Sebastiaan van Stijn Upstream-commit: e64cd6abed308a26d9048d4d0e4c52207c2ad5df Component: engine --- components/engine/Makefile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/components/engine/Makefile b/components/engine/Makefile index 67f6c3dab1..e2539683e1 100644 --- a/components/engine/Makefile +++ b/components/engine/Makefile @@ -134,9 +134,10 @@ binary: build ## build the linux binaries dynbinary: build ## build the linux dynbinaries $(DOCKER_RUN_DOCKER) hack/make.sh dynbinary +build: DOCKER_BUILDKIT ?= 1 build: bundles $(warning The docker client CLI has moved to github.com/docker/cli. For a dev-test cycle involving the CLI, run:${\n} DOCKER_CLI_PATH=/host/path/to/cli/binary make shell ${\n} then change the cli and compile into a binary at the same location.${\n}) - docker build ${BUILD_APT_MIRROR} ${DOCKER_BUILD_ARGS} ${DOCKER_BUILD_OPTS} -t "$(DOCKER_IMAGE)" -f "$(DOCKERFILE)" . + DOCKER_BUILDKIT="${DOCKER_BUILDKIT}" docker build ${BUILD_APT_MIRROR} ${DOCKER_BUILD_ARGS} ${DOCKER_BUILD_OPTS} -t "$(DOCKER_IMAGE)" -f "$(DOCKERFILE)" . bundles: mkdir bundles