fix race condition

pkg/skaffold/docker/dependencies.go

@@ -22,6 +22,7 @@ import (
 	"os"
 	"path/filepath"
 	"sort"
+	"sync"
 
 	"github.com/docker/docker/builder/dockerignore"
 
@@ -36,7 +37,7 @@ type dependency struct {
 
 // dependencyCache caches the results for `GetDependencies` for individual dockerfile.
 var (
-	dependencyCache = map[string]dependency{}
+	dependencyCache = sync.Map{}
 )
 
 // NormalizeDockerfilePath returns the absolute path to the dockerfile.
@@ -61,14 +62,18 @@ func GetDependencies(ctx context.Context, workspace string, dockerfilePath strin
 		return nil, fmt.Errorf("normalizing dockerfile path: %w", err)
 	}
 
-	if _, ok := dependencyCache[absDockerfilePath]; !ok {
+	if v, ok := dependencyCache.Load(absDockerfilePath); !ok {
 		paths, err := getDependencies(workspace, dockerfilePath, absDockerfilePath, buildArgs, cfg)
-		dependencyCache[absDockerfilePath] = dependency{
+		dependencyCache.Store(absDockerfilePath, dependency{
 			files: paths,
 			err:   err,
-		}
+		})
+		return paths, err
+	} else if cv, ok := v.(dependency); ok {
+		return cv.files, cv.err
 	}
-	return dependencyCache[absDockerfilePath].files, dependencyCache[absDockerfilePath].err
+	// TODO: tejaldesai
+	return nil, fmt.Errorf("unexpected skaffold internal error encountered")
 }
 
 func getDependencies(workspace string, dockerfilePath string, absDockerfilePath string, buildArgs map[string]*string, cfg Config) ([]string, error) {

pkg/skaffold/docker/dependencies_test.go

@@ -21,6 +21,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"sync"
 	"testing"
 
 	v1 "github.com/google/go-containerregistry/pkg/v1"
@@ -683,14 +684,13 @@ func TestGetDependenciesCached(t *testing.T) {
 			tmpDir := t.NewTempDir().Touch("server.go", "random.go")
 			tmpDir.Write("Dockerfile", copyServerGo)
 			// construct cache for abs dockerfile paths.
-			cache := map[string]dependency{}
+			defer func() { dependencyCache = sync.Map{} }()
 			for k, v := range test.dependencyCache {
-				cache[tmpDir.Path(k)] = v
+				dependencyCache.Store(tmpDir.Path(k), v)
 			}
-			t.Override(&dependencyCache, cache)
 			deps, err := GetDependencies(context.Background(), tmpDir.Root(), "Dockerfile", map[string]*string{}, nil)
 			t.CheckErrorAndDeepEqual(test.shouldErr, err, test.expected, deps)
-			if _, ok := dependencyCache[tmpDir.Path("Dockerfile")]; !ok {
+			if _, ok := dependencyCache.Load(tmpDir.Path("Dockerfile")); !ok {
 				t.Fatal("expected a cache entry for Dockerfile, did not found")
 			}
 		})

pkg/skaffold/docker/parse.go

@@ -25,13 +25,13 @@ import (
 	"path/filepath"
 	"strings"
 
-	sErrors "github.com/GoogleContainerTools/skaffold/pkg/skaffold/errors"
 	v1 "github.com/google/go-containerregistry/pkg/v1"
 	"github.com/moby/buildkit/frontend/dockerfile/command"
 	"github.com/moby/buildkit/frontend/dockerfile/parser"
 	"github.com/moby/buildkit/frontend/dockerfile/shell"
 	"github.com/sirupsen/logrus"
 
+	sErrors "github.com/GoogleContainerTools/skaffold/pkg/skaffold/errors"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
 )
 

pkg/skaffold/errors/err_map.go

@@ -104,11 +104,12 @@ var (
 		regexp: re(retrieveFailedOldManifest),
 		description: func(err error) string {
 			matchExp := re(retrieveFailedOldManifest)
-			imageName := "specified image"
-			if match := matchExp.FindStringSubmatch(fmt.Sprintf("%s", err)); len(match) >= 2 {
-				imageName = fmt.Sprintf("image %s", match[1])
+			match := matchExp.FindStringSubmatch(fmt.Sprintf("%s", err))
+			pre := "Could not retrieve image pushed with the deprecated manifest v1"
+			if len(match) >= 3 && match[2] != "" {
+				pre = fmt.Sprintf("Could not retrieve image %s pushed with the deprecated manifest v1", match[2])
 			}
-			return fmt.Sprintf("Could not retrieve %s pushed with the deprecated manifest v1. Ignoring files dependencies for all ONBUILD triggers", imageName)
+			return fmt.Sprintf("%s. Ignoring files dependencies for all ONBUILD triggers", pre)
 		},
 		errCode: proto.StatusCode_DEVINIT_UNSUPPORTED_V1_MANIFEST,
 		suggestion: func(opts config.SkaffoldOptions) []*proto.Suggestion {

pkg/skaffold/errors/errors_test.go

@@ -258,6 +258,72 @@ func TestShowAIError(t *testing.T) {
 	}
 }
 
+func TestIsOldImageManifestProblem(t *testing.T) {
+	tests := []struct {
+		description string
+		command     string
+		err         error
+		expectedMsg string
+		expected    bool
+	}{
+		{
+			description: "dev command older manifest with image name",
+			command:     "dev",
+			err:         fmt.Errorf(`listing files: parsing ONBUILD instructions: retrieving image "library/ruby:2.3.0": unsupported MediaType: "application/vnd.docker.distribution.manifest.v1+prettyjws", see https://github.com/google/go-containerregistry/issues/377`),
+			expectedMsg: "Could not retrieve image library/ruby:2.3.0 pushed with the deprecated manifest v1. Ignoring files dependencies for all ONBUILD triggers. To avoid, hit Cntrl-C and run `docker pull` to fetch the specified image and retry.",
+			expected:    true,
+		},
+		{
+			description: "dev command older manifest without image name",
+			command:     "dev",
+			err:         fmt.Errorf(`unsupported MediaType: "application/vnd.docker.distribution.manifest.v1+prettyjws", see https://github.com/google/go-containerregistry/issues/377`),
+			expectedMsg: "Could not retrieve image pushed with the deprecated manifest v1. Ignoring files dependencies for all ONBUILD triggers. To avoid, hit Cntrl-C and run `docker pull` to fetch the specified image and retry.",
+			expected:    true,
+		},
+		{
+			description: "dev command with random name",
+			command:     "dev",
+			err:         fmt.Errorf(`listing files: parsing ONBUILD instructions: retrieve image "noimage" image does not exits`),
+		},
+		{
+			description: "debug command older manifest",
+			command:     "debug",
+			err:         fmt.Errorf(`unsupported MediaType: "application/vnd.docker.distribution.manifest.v1+prettyjws", see https://github.com/google/go-containerregistry/issues/377`),
+			expectedMsg: "Could not retrieve image pushed with the deprecated manifest v1. Ignoring files dependencies for all ONBUILD triggers. To avoid, hit Cntrl-C and run `docker pull` to fetch the specified image and retry.",
+			expected:    true,
+		},
+		{
+			description: "build command older manifest",
+			command:     "build",
+			err:         fmt.Errorf(`unsupported MediaType: "application/vnd.docker.distribution.manifest.v1+prettyjws", see https://github.com/google/go-containerregistry/issues/377`),
+			expected:    true,
+		},
+		{
+			description: "run command older manifest",
+			command:     "run",
+			err:         fmt.Errorf(`unsupported MediaType: "application/vnd.docker.distribution.manifest.v1+prettyjws", see https://github.com/google/go-containerregistry/issues/377`),
+			expected:    true,
+		},
+		{
+			description: "deploy command older manifest",
+			command:     "deploy",
+			err:         fmt.Errorf(`unsupported MediaType: "application/vnd.docker.distribution.manifest.v1+prettyjws", see https://github.com/google/go-containerregistry/issues/377`),
+			expected:    true,
+		},
+	}
+	for _, test := range tests {
+		testutil.Run(t, test.description, func(t *testutil.T) {
+			skaffoldOpts = config.SkaffoldOptions{
+				Command: test.command,
+			}
+			actualMsg, actual := IsOldImageManifestProblem(test.err)
+			fmt.Println(actualMsg)
+			t.CheckDeepEqual(test.expectedMsg, actualMsg)
+			t.CheckDeepEqual(test.expected, actual)
+		})
+	}
+}
+
 func stringOrUndefined(s string) config.StringOrUndefined {
 	c := &config.StringOrUndefined{}
 	c.Set(s)