From d283c7fa2b93e00d4e1b0feaee99028b00dd775d Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Wed, 13 Mar 2019 10:37:35 +1100 Subject: [PATCH 1/3] *: remove interfacer linter from CI It has been declared deprecated by the author, and has a knack for false-positives (as well as giving bad advice when it comes to APIs -- which is quite clear when looking at "nolint: interfacer" comments). Signed-off-by: Aleksa Sarai --- daemon/container_operations.go | 2 +- daemon/metrics.go | 2 +- distribution/pull_v1.go | 5 +---- distribution/push_v2.go | 1 - hack/validate/gometalinter.json | 1 - registry/auth.go | 1 - 6 files changed, 3 insertions(+), 9 deletions(-) diff --git a/daemon/container_operations.go b/daemon/container_operations.go index 909c7ccb27..dc4654f146 100644 --- a/daemon/container_operations.go +++ b/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") } diff --git a/daemon/metrics.go b/daemon/metrics.go index 20030c4270..ec633d6cd7 100644 --- a/daemon/metrics.go +++ b/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 } diff --git a/distribution/pull_v1.go b/distribution/pull_v1.go index c2c897dc1c..b244bb9cdc 100644 --- a/distribution/pull_v1.go +++ b/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) diff --git a/distribution/push_v2.go b/distribution/push_v2.go index 5281677734..8bc0edd4c1 100644 --- a/distribution/push_v2.go +++ b/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++ { diff --git a/hack/validate/gometalinter.json b/hack/validate/gometalinter.json index 81eb1017cb..e36dd73306 100644 --- a/hack/validate/gometalinter.json +++ b/hack/validate/gometalinter.json @@ -18,7 +18,6 @@ "golint", "gosimple", "ineffassign", - "interfacer", "unconvert", "vet" ], diff --git a/registry/auth.go b/registry/auth.go index 1f2043a0d9..3f58fc6cff 100644 --- a/registry/auth.go +++ b/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 From 175b1d783013b7e73f117f512e1179ee3b66a697 Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Tue, 12 Mar 2019 18:37:31 +1100 Subject: [PATCH 2/3] integration-cli: don't build -test images if they already exist There's no need to try to re-build the test images if they already exist. This change makes basically no difference to the upstream integration test-suite running, but for users who want to run the integration-cli suite on a host machine (such as distributions doing tests) this change allows images to be pre-loaded such that compilers aren't needed on the test machine. However, this does remove the accidental re-compilation of nnp-test, as well as handling errors far more cleanly (previously if an error occurred during a test build, further tests won't attempt to rebuild it). Signed-off-by: Aleksa Sarai --- integration-cli/fixtures_linux_daemon_test.go | 21 +++++++++-------- internal/test/environment/environment.go | 23 +++++++++++++++++++ 2 files changed, 35 insertions(+), 9 deletions(-) diff --git a/integration-cli/fixtures_linux_daemon_test.go b/integration-cli/fixtures_linux_daemon_test.go index 2387a9ebee..0a770a76a2 100644 --- a/integration-cli/fixtures_linux_daemon_test.go +++ b/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 diff --git a/internal/test/environment/environment.go b/internal/test/environment/environment.go index 5538d2097e..763c08ba48 100644 --- a/internal/test/environment/environment.go +++ b/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 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 { From ba0afa6ba865c264e0fbcb5d5e4590d8f2748f72 Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Wed, 13 Mar 2019 10:39:00 +1100 Subject: [PATCH 3/3] internal: test/env: switch to assert.TestingT Signed-off-by: Aleksa Sarai --- internal/test/environment/clean.go | 10 ---------- internal/test/environment/environment.go | 2 +- internal/test/environment/protect.go | 22 +++++++++++----------- 3 files changed, 12 insertions(+), 22 deletions(-) diff --git a/internal/test/environment/clean.go b/internal/test/environment/clean.go index 93dee593f2..ad6ec93e4a 100644 --- a/internal/test/environment/clean.go +++ b/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. diff --git a/internal/test/environment/environment.go b/internal/test/environment/environment.go index 763c08ba48..828c94d23f 100644 --- a/internal/test/environment/environment.go +++ b/internal/test/environment/environment.go @@ -157,7 +157,7 @@ func (e *Execution) IsUserNamespace() bool { // 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 testingT, reference string) bool { +func (e *Execution) HasExistingImage(t assert.TestingT, reference string) bool { if ht, ok := t.(test.HelperT); ok { ht.Helper() } diff --git a/internal/test/environment/protect.go b/internal/test/environment/protect.go index b5b27d2dd4..a47ea3c2fd 100644 --- a/internal/test/environment/protect.go +++ b/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() }