Merge pull request #38853 from cyphar/integration-cli-ensureImage

integration-cli: don't build -test images if they already exist

daemon/container_operations.go

@@ -801,7 +801,7 @@ func (daemon *Daemon) connectToNetwork(container *container.Container, idOrName
 	return nil
 }
 
-func updateJoinInfo(networkSettings *network.Settings, n libnetwork.Network, ep libnetwork.Endpoint) error { // nolint: interfacer
+func updateJoinInfo(networkSettings *network.Settings, n libnetwork.Network, ep libnetwork.Endpoint) error {
 	if ep == nil {
 		return errors.New("invalid enppoint whhile building portmap info")
 	}

daemon/metrics.go

@@ -143,7 +143,7 @@ type metricsPlugin interface {
 	StopMetrics() error
 }
 
-func makePluginAdapter(p plugingetter.CompatPlugin) (metricsPlugin, error) { // nolint: interfacer
+func makePluginAdapter(p plugingetter.CompatPlugin) (metricsPlugin, error) {
 	if pc, ok := p.(plugingetter.PluginWithV1Client); ok {
 		return &metricsPluginAdapter{pc.Client(), p.Name()}, nil
 	}

distribution/pull_v1.go

@@ -13,7 +13,6 @@ import (
 	"time"
 
 	"github.com/docker/distribution/reference"
-	"github.com/docker/distribution/registry/client/auth"
 	"github.com/docker/distribution/registry/client/transport"
 	"github.com/docker/docker/distribution/metadata"
 	"github.com/docker/docker/distribution/xfer"
@@ -70,9 +69,7 @@ func (p *v1Puller) Pull(ctx context.Context, ref reference.Named, _ *specs.Platf
 	return nil
 }
 
-// Note use auth.Scope rather than reference.Named due to this warning causing Jenkins CI to fail:
-// warning: ref can be github.com/docker/docker/vendor/github.com/docker/distribution/registry/client/auth.Scope (interfacer)
-func (p *v1Puller) pullRepository(ctx context.Context, ref auth.Scope) error {
+func (p *v1Puller) pullRepository(ctx context.Context, ref reference.Named) error {
 	progress.Message(p.config.ProgressOutput, "", "Pulling repository "+p.repoInfo.Name.Name())
 
 	tagged, isTagged := ref.(reference.NamedTagged)

distribution/push_v2.go

@@ -649,7 +649,6 @@ func (bla byLikeness) Swap(i, j int) {
 }
 func (bla byLikeness) Len() int { return len(bla.arr) }
 
-// nolint: interfacer
 func sortV2MetadataByLikenessAndAge(repoInfo reference.Named, hmacKey []byte, marr []metadata.V2Metadata) {
 	// reverse the metadata array to shift the newest entries to the beginning
 	for i := 0; i < len(marr)/2; i++ {

hack/validate/gometalinter.json

@@ -18,7 +18,6 @@
     "golint",
     "gosimple",
     "ineffassign",
-    "interfacer",
     "unconvert",
     "vet"
   ],

integration-cli/fixtures_linux_daemon_test.go

@@ -8,7 +8,6 @@ import (
 	"path/filepath"
 	"runtime"
 	"strings"
-	"sync"
 
 	"github.com/docker/docker/integration-cli/checker"
 	"github.com/docker/docker/internal/test/fixtures/load"
@@ -24,17 +23,13 @@ type logT interface {
 	Logf(string, ...interface{})
 }
 
-var ensureSyscallTestOnce sync.Once
-
 func ensureSyscallTest(c *check.C) {
-	var doIt bool
-	ensureSyscallTestOnce.Do(func() {
-		doIt = true
-	})
-	if !doIt {
+	defer testEnv.ProtectImage(c, "syscall-test:latest")
+
+	// If the image already exists, there's nothing left to do.
+	if testEnv.HasExistingImage(c, "syscall-test:latest") {
 		return
 	}
-	defer testEnv.ProtectImage(c, "syscall-test:latest")
 
 	// if no match, must build in docker, which is significantly slower
 	// (slower mostly because of the vfs graphdriver)
@@ -93,6 +88,14 @@ func ensureSyscallTestBuild(c *check.C) {
 
 func ensureNNPTest(c *check.C) {
 	defer testEnv.ProtectImage(c, "nnp-test:latest")
+
+	// If the image already exists, there's nothing left to do.
+	if testEnv.HasExistingImage(c, "nnp-test:latest") {
+		return
+	}
+
+	// if no match, must build in docker, which is significantly slower
+	// (slower mostly because of the vfs graphdriver)
 	if testEnv.OSType != runtime.GOOS {
 		ensureNNPTestBuild(c)
 		return

internal/test/environment/clean.go

@@ -12,16 +12,6 @@ import (
 	"gotest.tools/assert"
 )
 
-type testingT interface {
-	assert.TestingT
-	logT
-	Fatalf(string, ...interface{})
-}
-
-type logT interface {
-	Logf(string, ...interface{})
-}
-
 // Clean the environment, preserving protected objects (images, containers, ...)
 // and removing everything else. It's meant to run after any tests so that they don't
 // depend on each others.

internal/test/environment/environment.go

@@ -8,9 +8,12 @@ import (
 	"strings"
 
 	"github.com/docker/docker/api/types"
+	"github.com/docker/docker/api/types/filters"
 	"github.com/docker/docker/client"
+	"github.com/docker/docker/internal/test"
 	"github.com/docker/docker/internal/test/fixtures/load"
 	"github.com/pkg/errors"
+	"gotest.tools/assert"
 )
 
 // Execution contains information about the current test execution and daemon
@@ -151,6 +154,26 @@ func (e *Execution) IsUserNamespace() bool {
 	return root != ""
 }
 
+// HasExistingImage checks whether there is an image with the given reference.
+// Note that this is done by filtering and then checking whether there were any
+// results -- so ambiguous references might result in false-positives.
+func (e *Execution) HasExistingImage(t assert.TestingT, reference string) bool {
+	if ht, ok := t.(test.HelperT); ok {
+		ht.Helper()
+	}
+	client := e.APIClient()
+	filter := filters.NewArgs()
+	filter.Add("dangling", "false")
+	filter.Add("reference", reference)
+	imageList, err := client.ImageList(context.Background(), types.ImageListOptions{
+		All:     true,
+		Filters: filter,
+	})
+	assert.NilError(t, err, "failed to list images")
+
+	return len(imageList) > 0
+}
+
 // EnsureFrozenImagesLinux loads frozen test images into the daemon
 // if they aren't already loaded
 func EnsureFrozenImagesLinux(testEnv *Execution) error {

internal/test/environment/protect.go

@@ -33,7 +33,7 @@ func newProtectedElements() protectedElements {
 // ProtectAll protects the existing environment (containers, images, networks,
 // volumes, and, on Linux, plugins) from being cleaned up at the end of test
 // runs
-func ProtectAll(t testingT, testEnv *Execution) {
+func ProtectAll(t assert.TestingT, testEnv *Execution) {
 	if ht, ok := t.(test.HelperT); ok {
 		ht.Helper()
 	}
@@ -48,7 +48,7 @@ func ProtectAll(t testingT, testEnv *Execution) {
 
 // ProtectContainer adds the specified container(s) to be protected in case of
 // clean
-func (e *Execution) ProtectContainer(t testingT, containers ...string) {
+func (e *Execution) ProtectContainer(t assert.TestingT, containers ...string) {
 	if ht, ok := t.(test.HelperT); ok {
 		ht.Helper()
 	}
@@ -59,7 +59,7 @@ func (e *Execution) ProtectContainer(t testingT, containers ...string) {
 
 // ProtectContainers protects existing containers from being cleaned up at the
 // end of test runs
-func ProtectContainers(t testingT, testEnv *Execution) {
+func ProtectContainers(t assert.TestingT, testEnv *Execution) {
 	if ht, ok := t.(test.HelperT); ok {
 		ht.Helper()
 	}
@@ -85,7 +85,7 @@ func getExistingContainers(t assert.TestingT, testEnv *Execution) []string {
 }
 
 // ProtectImage adds the specified image(s) to be protected in case of clean
-func (e *Execution) ProtectImage(t testingT, images ...string) {
+func (e *Execution) ProtectImage(t assert.TestingT, images ...string) {
 	if ht, ok := t.(test.HelperT); ok {
 		ht.Helper()
 	}
@@ -96,7 +96,7 @@ func (e *Execution) ProtectImage(t testingT, images ...string) {
 
 // ProtectImages protects existing images and on linux frozen images from being
 // cleaned up at the end of test runs
-func ProtectImages(t testingT, testEnv *Execution) {
+func ProtectImages(t assert.TestingT, testEnv *Execution) {
 	if ht, ok := t.(test.HelperT); ok {
 		ht.Helper()
 	}
@@ -145,7 +145,7 @@ func tagsFromImageSummary(image types.ImageSummary) []string {
 
 // ProtectNetwork adds the specified network(s) to be protected in case of
 // clean
-func (e *Execution) ProtectNetwork(t testingT, networks ...string) {
+func (e *Execution) ProtectNetwork(t assert.TestingT, networks ...string) {
 	if ht, ok := t.(test.HelperT); ok {
 		ht.Helper()
 	}
@@ -156,7 +156,7 @@ func (e *Execution) ProtectNetwork(t testingT, networks ...string) {
 
 // ProtectNetworks protects existing networks from being cleaned up at the end
 // of test runs
-func ProtectNetworks(t testingT, testEnv *Execution) {
+func ProtectNetworks(t assert.TestingT, testEnv *Execution) {
 	if ht, ok := t.(test.HelperT); ok {
 		ht.Helper()
 	}
@@ -180,7 +180,7 @@ func getExistingNetworks(t assert.TestingT, testEnv *Execution) []string {
 }
 
 // ProtectPlugin adds the specified plugin(s) to be protected in case of clean
-func (e *Execution) ProtectPlugin(t testingT, plugins ...string) {
+func (e *Execution) ProtectPlugin(t assert.TestingT, plugins ...string) {
 	if ht, ok := t.(test.HelperT); ok {
 		ht.Helper()
 	}
@@ -191,7 +191,7 @@ func (e *Execution) ProtectPlugin(t testingT, plugins ...string) {
 
 // ProtectPlugins protects existing plugins from being cleaned up at the end of
 // test runs
-func ProtectPlugins(t testingT, testEnv *Execution) {
+func ProtectPlugins(t assert.TestingT, testEnv *Execution) {
 	if ht, ok := t.(test.HelperT); ok {
 		ht.Helper()
 	}
@@ -219,7 +219,7 @@ func getExistingPlugins(t assert.TestingT, testEnv *Execution) []string {
 }
 
 // ProtectVolume adds the specified volume(s) to be protected in case of clean
-func (e *Execution) ProtectVolume(t testingT, volumes ...string) {
+func (e *Execution) ProtectVolume(t assert.TestingT, volumes ...string) {
 	if ht, ok := t.(test.HelperT); ok {
 		ht.Helper()
 	}
@@ -230,7 +230,7 @@ func (e *Execution) ProtectVolume(t testingT, volumes ...string) {
 
 // ProtectVolumes protects existing volumes from being cleaned up at the end of
 // test runs
-func ProtectVolumes(t testingT, testEnv *Execution) {
+func ProtectVolumes(t assert.TestingT, testEnv *Execution) {
 	if ht, ok := t.(test.HelperT); ok {
 		ht.Helper()
 	}

registry/auth.go

@@ -248,7 +248,6 @@ func (err PingResponseError) Error() string {
 // challenge manager for the supported authentication types and
 // whether v2 was confirmed by the response. If a response is received but
 // cannot be interpreted a PingResponseError will be returned.
-// nolint: interfacer
 func PingV2Registry(endpoint *url.URL, transport http.RoundTripper) (challenge.Manager, bool, error) {
 	var (
 		foundV2   = false