From 7b056b6c44e5002587ea3c25e934d58a900c6f4e Mon Sep 17 00:00:00 2001 From: Abe Winter Date: Wed, 19 Jun 2024 14:05:48 -0400 Subject: [PATCH 1/3] provide a docker.ImageReference in dependency resolution step --- pkg/skaffold/graph/dependencies.go | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/pkg/skaffold/graph/dependencies.go b/pkg/skaffold/graph/dependencies.go index d3a2a9adcd9..b4cc68d5c2b 100644 --- a/pkg/skaffold/graph/dependencies.go +++ b/pkg/skaffold/graph/dependencies.go @@ -43,7 +43,7 @@ type SourceDependenciesCache interface { TransitiveArtifactDependencies(ctx context.Context, a *latest.Artifact) ([]string, error) // SingleArtifactDependencies returns the source dependencies for only the target artifact. // The result (even if an error) is cached so that the function is evaluated only once for every artifact. The cache is reset before the start of the next devloop. - SingleArtifactDependencies(ctx context.Context, a *latest.Artifact) ([]string, error) + SingleArtifactDependencies(ctx context.Context, a *latest.Artifact, tags map[string]string) ([]string, error) // Reset removes the cached source dependencies for all artifacts Reset() } @@ -65,7 +65,7 @@ func (r *dependencyResolverImpl) TransitiveArtifactDependencies(ctx context.Cont }) defer endTrace() - deps, err := r.SingleArtifactDependencies(ctx, a) + deps, err := r.SingleArtifactDependencies(ctx, a, nil) if err != nil { endTrace(instrumentation.TraceEndError(err)) return nil, err @@ -81,14 +81,14 @@ func (r *dependencyResolverImpl) TransitiveArtifactDependencies(ctx context.Cont return deps, nil } -func (r *dependencyResolverImpl) SingleArtifactDependencies(ctx context.Context, a *latest.Artifact) ([]string, error) { +func (r *dependencyResolverImpl) SingleArtifactDependencies(ctx context.Context, a *latest.Artifact, tags map[string]string) ([]string, error) { ctx, endTrace := instrumentation.StartTrace(ctx, "SingleArtifactDependencies", map[string]string{ "ArtifactName": instrumentation.PII(a.ImageName), }) defer endTrace() res, err := r.cache.Exec(a.ImageName, func() ([]string, error) { - return getDependenciesFunc(ctx, a, r.cfg, r.artifactResolver) + return getDependenciesFunc(ctx, a, r.cfg, r.artifactResolver, tags) }) if err != nil { endTrace(instrumentation.TraceEndError(err)) @@ -104,8 +104,20 @@ func (r *dependencyResolverImpl) Reset() { r.cache = util.NewSyncStore[[]string]() } +func quickMakeEnvTags(tag string) (map[string]string, error) { + imgRef, err := docker.ParseReference(tag) + if err != nil { + return nil, fmt.Errorf("couldn't parse image tag %s %w", tag, err) + } + return map[string]string{ + "IMAGE_REPO": imgRef.Repo, + "IMAGE_NAME": imgRef.Name, + "IMAGE_TAG": imgRef.Tag, + }, nil +} + // sourceDependenciesForArtifact returns the build dependencies for the current artifact. -func sourceDependenciesForArtifact(ctx context.Context, a *latest.Artifact, cfg docker.Config, r docker.ArtifactResolver) ([]string, error) { +func sourceDependenciesForArtifact(ctx context.Context, a *latest.Artifact, cfg docker.Config, r docker.ArtifactResolver, tags map[string]string) ([]string, error) { var ( paths []string err error @@ -118,7 +130,12 @@ func sourceDependenciesForArtifact(ctx context.Context, a *latest.Artifact, cfg // For single build scenarios like `build` and `run`, it is called for the cache hash calculations which are already handled in `artifactHasher`. // For `dev` it will succeed on the first dev loop and list any additional dependencies found from the base artifact's ONBUILD instructions as a file added instead of modified (see `filemon.Events`) deps := docker.ResolveDependencyImages(a.Dependencies, r, false) - args, evalErr := docker.EvalBuildArgs(cfg.Mode(), a.Workspace, a.DockerArtifact.DockerfilePath, a.DockerArtifact.BuildArgs, deps) + var envTags map[string]string + envTags, err = quickMakeEnvTags(tags[a.ImageName]) + if err != nil { + return nil, err + } + args, evalErr := docker.EvalBuildArgsWithEnv(cfg.Mode(), a.Workspace, a.DockerArtifact.DockerfilePath, a.DockerArtifact.BuildArgs, deps, envTags) if evalErr != nil { return nil, fmt.Errorf("unable to evaluate build args: %w", evalErr) } From 6067290562dea7622ad7b140791bb0a8ef64db56 Mon Sep 17 00:00:00 2001 From: Abe Winter Date: Wed, 19 Jun 2024 14:08:58 -0400 Subject: [PATCH 2/3] pass nils --- pkg/skaffold/build/gcb/cloud_build.go | 2 +- pkg/skaffold/diagnose/diagnose.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/skaffold/build/gcb/cloud_build.go b/pkg/skaffold/build/gcb/cloud_build.go index d12ee302f96..14a26915ee9 100644 --- a/pkg/skaffold/build/gcb/cloud_build.go +++ b/pkg/skaffold/build/gcb/cloud_build.go @@ -122,7 +122,7 @@ func (b *Builder) buildArtifactWithCloudBuild(ctx context.Context, out io.Writer }) } - dependencies, err := b.sourceDependencies.SingleArtifactDependencies(ctx, artifact) + dependencies, err := b.sourceDependencies.SingleArtifactDependencies(ctx, artifact, nil) if err != nil { return "", sErrors.NewErrorWithStatusCode(&proto.ActionableErr{ ErrCode: proto.StatusCode_BUILD_GCB_GET_DEPENDENCY_ERR, diff --git a/pkg/skaffold/diagnose/diagnose.go b/pkg/skaffold/diagnose/diagnose.go index 5db4f2df9fe..f8b1d58f5fa 100644 --- a/pkg/skaffold/diagnose/diagnose.go +++ b/pkg/skaffold/diagnose/diagnose.go @@ -123,7 +123,7 @@ func timeToListDependencies(ctx context.Context, a *latest.Artifact, cfg Config) start := time.Now() g := graph.ToArtifactGraph(cfg.Artifacts()) sourceDependencies := graph.NewSourceDependenciesCache(cfg, nil, g) - paths, err := sourceDependencies.SingleArtifactDependencies(ctx, a) + paths, err := sourceDependencies.SingleArtifactDependencies(ctx, a, nil) return timeutil.Humanize(time.Since(start)), paths, err } From 0d65a379c461b461a81399d8ef6c7c55eeef1944 Mon Sep 17 00:00:00 2001 From: Abe Winter Date: Wed, 19 Jun 2024 14:10:53 -0400 Subject: [PATCH 3/3] other plumbing --- pkg/skaffold/build/cache/cache.go | 2 +- pkg/skaffold/build/cache/hash.go | 16 ++++++++-------- pkg/skaffold/build/cache/lookup.go | 7 ++++--- pkg/skaffold/runner/new.go | 4 ++-- 4 files changed, 15 insertions(+), 14 deletions(-) diff --git a/pkg/skaffold/build/cache/cache.go b/pkg/skaffold/build/cache/cache.go index 371ffb104f2..d721d1f93bb 100644 --- a/pkg/skaffold/build/cache/cache.go +++ b/pkg/skaffold/build/cache/cache.go @@ -61,7 +61,7 @@ type cache struct { } // DependencyLister fetches a list of dependencies for an artifact -type DependencyLister func(ctx context.Context, artifact *latest.Artifact) ([]string, error) +type DependencyLister func(ctx context.Context, artifact *latest.Artifact, tags map[string]string) ([]string, error) type Config interface { docker.Config diff --git a/pkg/skaffold/build/cache/hash.go b/pkg/skaffold/build/cache/hash.go index 077b83b19fb..1a552ce1a45 100644 --- a/pkg/skaffold/build/cache/hash.go +++ b/pkg/skaffold/build/cache/hash.go @@ -47,7 +47,7 @@ var ( ) type artifactHasher interface { - hash(context.Context, *latest.Artifact, platform.Resolver) (string, error) + hash(context.Context, *latest.Artifact, platform.Resolver, map[string]string) (string, error) } type artifactHasherImpl struct { @@ -67,20 +67,20 @@ func newArtifactHasher(artifacts graph.ArtifactGraph, lister DependencyLister, m } } -func (h *artifactHasherImpl) hash(ctx context.Context, a *latest.Artifact, platforms platform.Resolver) (string, error) { +func (h *artifactHasherImpl) hash(ctx context.Context, a *latest.Artifact, platforms platform.Resolver, tags map[string]string) (string, error) { ctx, endTrace := instrumentation.StartTrace(ctx, "hash_GenerateHashOneArtifact", map[string]string{ "ImageName": instrumentation.PII(a.ImageName), }) defer endTrace() - hash, err := h.safeHash(ctx, a, platforms.GetPlatforms(a.ImageName)) + hash, err := h.safeHash(ctx, a, platforms.GetPlatforms(a.ImageName), tags) if err != nil { endTrace(instrumentation.TraceEndError(err)) return "", err } hashes := []string{hash} for _, dep := range sortedDependencies(a, h.artifacts) { - depHash, err := h.hash(ctx, dep, platforms) + depHash, err := h.hash(ctx, dep, platforms, tags) if err != nil { endTrace(instrumentation.TraceEndError(err)) return "", err @@ -94,15 +94,15 @@ func (h *artifactHasherImpl) hash(ctx context.Context, a *latest.Artifact, platf return encode(hashes) } -func (h *artifactHasherImpl) safeHash(ctx context.Context, a *latest.Artifact, platforms platform.Matcher) (string, error) { +func (h *artifactHasherImpl) safeHash(ctx context.Context, a *latest.Artifact, platforms platform.Matcher, tags map[string]string) (string, error) { return h.syncStore.Exec(a.ImageName, func() (string, error) { - return singleArtifactHash(ctx, h.lister, a, h.mode, platforms) + return singleArtifactHash(ctx, h.lister, a, h.mode, platforms, tags) }) } // singleArtifactHash calculates the hash for a single artifact, and ignores its required artifacts. -func singleArtifactHash(ctx context.Context, depLister DependencyLister, a *latest.Artifact, mode config.RunMode, m platform.Matcher) (string, error) { +func singleArtifactHash(ctx context.Context, depLister DependencyLister, a *latest.Artifact, mode config.RunMode, m platform.Matcher, tags map[string]string) (string, error) { var inputs []string // Append the artifact's configuration @@ -113,7 +113,7 @@ func singleArtifactHash(ctx context.Context, depLister DependencyLister, a *late inputs = append(inputs, config) // Append the digest of each input file - deps, err := depLister(ctx, a) + deps, err := depLister(ctx, a, tags) if err != nil { return "", fmt.Errorf("getting dependencies for %q: %w", a.ImageName, err) } diff --git a/pkg/skaffold/build/cache/lookup.go b/pkg/skaffold/build/cache/lookup.go index c7544853841..00c35792c96 100644 --- a/pkg/skaffold/build/cache/lookup.go +++ b/pkg/skaffold/build/cache/lookup.go @@ -48,7 +48,7 @@ func (c *cache) lookupArtifacts(ctx context.Context, tags tag.ImageTags, platfor i := i go func() { - details[i] = c.lookup(ctx, artifacts[i], tags[artifacts[i].ImageName], platforms, h) + details[i] = c.lookup(ctx, artifacts[i], tags, platforms, h) wg.Done() }() } @@ -57,13 +57,14 @@ func (c *cache) lookupArtifacts(ctx context.Context, tags tag.ImageTags, platfor return details } -func (c *cache) lookup(ctx context.Context, a *latest.Artifact, tag string, platforms platform.Resolver, h artifactHasher) cacheDetails { +func (c *cache) lookup(ctx context.Context, a *latest.Artifact, tags map[string]string, platforms platform.Resolver, h artifactHasher) cacheDetails { + tag := tags[a.ImageName] ctx, endTrace := instrumentation.StartTrace(ctx, "lookup_CacheLookupOneArtifact", map[string]string{ "ImageName": instrumentation.PII(a.ImageName), }) defer endTrace() - hash, err := h.hash(ctx, a, platforms) + hash, err := h.hash(ctx, a, platforms, tags) if err != nil { return failed{err: fmt.Errorf("getting hash for artifact %q: %s", a.ImageName, err)} } diff --git a/pkg/skaffold/runner/new.go b/pkg/skaffold/runner/new.go index 4adc8518f97..e2a8081b16d 100644 --- a/pkg/skaffold/runner/new.go +++ b/pkg/skaffold/runner/new.go @@ -117,11 +117,11 @@ func NewForConfig(ctx context.Context, runCtx *runcontext.RunContext) (*Skaffold return nil, fmt.Errorf("creating actiosn runner: %w", err) } - depLister := func(ctx context.Context, artifact *latest.Artifact) ([]string, error) { + depLister := func(ctx context.Context, artifact *latest.Artifact, tags map[string]string) ([]string, error) { ctx, endTrace := instrumentation.StartTrace(ctx, "NewForConfig_depLister") defer endTrace() - buildDependencies, err := sourceDependencies.SingleArtifactDependencies(ctx, artifact) + buildDependencies, err := sourceDependencies.SingleArtifactDependencies(ctx, artifact, tags) if err != nil { endTrace(instrumentation.TraceEndError(err)) return nil, err