Skip to content

Commit

Permalink
fix warnings and UTs import cycle
Browse files Browse the repository at this point in the history
  • Loading branch information
reingart committed Jan 14, 2025
1 parent b53ff7b commit ba2c5e7
Show file tree
Hide file tree
Showing 13 changed files with 32 additions and 21 deletions.
2 changes: 1 addition & 1 deletion cmd/skaffold/app/cmd/config/set_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,7 @@ func TestGetConfigStructWithIndex(t *testing.T) {
description: "survey flag set",
cfg: &config.ContextConfig{},
survey: true,
expectedIdx: []int{7},
expectedIdx: []int{9},
},
{
description: "no survey flag set",
Expand Down
13 changes: 13 additions & 0 deletions docs-v2/content/en/schemas/v4beta12.json
Original file line number Diff line number Diff line change
Expand Up @@ -1766,6 +1766,18 @@
"[\"golang:1.10.1-alpine3.7\", \"alpine:3.7\"]"
]
},
"cacheTo": {
"items": {
"type": "string"
},
"type": "array",
"description": "the Docker images used as cache destination. If ommited, cacheFrom is used with max mode to export all layers.",
"x-intellij-html-description": "the Docker images used as cache destination. If ommited, cacheFrom is used with max mode to export all layers.",
"default": "[]",
"examples": [
"[\"type=registry,ref=gcr.io/k8s-skaffold/example:cache,mode=max\"]"
]
},
"cliFlags": {
"items": {
"type": "string"
Expand Down Expand Up @@ -1830,6 +1842,7 @@
"network",
"addHost",
"cacheFrom",
"cacheTo",
"cliFlags",
"pullParent",
"noCache",
Expand Down
2 changes: 1 addition & 1 deletion pkg/skaffold/build/cache/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ func NewCache(ctx context.Context, cfg Config, isLocalImage func(imageName strin
}

// for backward compatibility, extended build capabilities with BuildKit are disabled by default
buildx := cfg.DetectBuildX()
buildx := cfg.DetectBuildX() && docker.IsBuildXDetected()

return &cache{
artifactCache: artifactCache,
Expand Down
6 changes: 3 additions & 3 deletions pkg/skaffold/build/cache/lookup.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ func (c *cache) lookup(ctx context.Context, out io.Writer, a *latest.Artifact, t
}

if isLocal, err := c.isLocalImage(a.ImageName); err != nil {
log.Entry(ctx).Debugf("isLocalImage failed %w", err)
log.Entry(ctx).Debugf("isLocalImage failed %v", err)
return failed{err}
} else if isLocal {
return c.lookupLocal(ctx, hash, tag, entry)
Expand Down Expand Up @@ -128,7 +128,7 @@ func (c *cache) lookupRemote(ctx context.Context, hash, tag string, platforms []
return found{hash: hash}
}
} else {
log.Entry(ctx).Debugf("RemoteDigest error %w", err)
log.Entry(ctx).Debugf("RemoteDigest error %v", err)
}

// Image exists remotely with a different tag
Expand Down Expand Up @@ -162,7 +162,7 @@ func (c *cache) tryImport(ctx context.Context, a *latest.Artifact, tag string, h
_, err := c.client.ServerVersion(ctx)
load = err == nil
if !load {
log.Entry(ctx).Debugf("Docker client error, disabling image load as using buildx: %w", err)
log.Entry(ctx).Debugf("Docker client error, disabling image load as using buildx: %v", err)
}
}
if load {
Expand Down
4 changes: 2 additions & 2 deletions pkg/skaffold/build/docker/docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ func (b *Builder) adjustCache(ctx context.Context, a *latest.Artifact, artifactT
tag, _ := config.GetCacheTag(b.cfg.GlobalConfig())
imgRef, err := docker.ParseReference(artifactTag)
if err != nil {
log.Entry(ctx).Errorf("couldn't parse image tag: %w", err)
log.Entry(ctx).Errorf("couldn't parse image tag: %v", err)
} else if tag != "" {
cacheTag = fmt.Sprintf("%s/%s:%s", imgRef.Repo, cacheRef, tag)
}
Expand Down Expand Up @@ -273,7 +273,7 @@ func getBuildxDigest(ctx context.Context, filename string) (string, error) {
return digest, nil
}
}
log.Entry(ctx).Warnf("No digest found in buildx metadata: %w", err)
log.Entry(ctx).Warnf("No digest found in buildx metadata: %v", err)
// if image is not pushed, it could not contain the digest log for debugging:
log.Entry(ctx).Debugf("Full buildx metadata: %s", data)
return "", err
Expand Down
4 changes: 2 additions & 2 deletions pkg/skaffold/build/docker/docker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ func TestDockerCLIBuild(t *testing.T) {
}
t.Override(&util.OSEnviron, func() []string { return []string{"KEY=VALUE"} })

builder := NewArtifactBuilder(fakeLocalDaemonWithExtraEnv(test.extraEnv), test.cfg, test.localBuild.UseDockerCLI, test.localBuild.UseBuildkit, false, mockArtifactResolver{make(map[string]string)}, nil)
builder := NewArtifactBuilder(fakeLocalDaemonWithExtraEnv(test.extraEnv), test.cfg, test.localBuild.UseDockerCLI, test.localBuild.UseBuildkit, false, false, mockArtifactResolver{make(map[string]string)}, nil)

artifact := &latest.Artifact{
Workspace: ".",
Expand Down Expand Up @@ -268,7 +268,7 @@ func TestDockerCLICheckCacheFromArgs(t *testing.T) {
)
t.Override(&util.DefaultExecCommand, mockCmd)

builder := NewArtifactBuilder(fakeLocalDaemonWithExtraEnv([]string{}), mockConfig{}, true, util.Ptr(false), false, mockArtifactResolver{make(map[string]string)}, nil)
builder := NewArtifactBuilder(fakeLocalDaemonWithExtraEnv([]string{}), mockConfig{}, true, util.Ptr(false), false, false, mockArtifactResolver{make(map[string]string)}, nil)
_, err := builder.Build(context.Background(), io.Discard, &a, test.tag, platform.Matcher{})
t.CheckNoError(err)
})
Expand Down
2 changes: 1 addition & 1 deletion pkg/skaffold/build/docker/errors_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ Refer https://skaffold.dev/docs/references/yaml/#build-artifacts-docker for deta
"docker build . --file "+dockerfilePath+" -t tag",
))
t.Override(&docker.DefaultAuthHelper, stubAuth{})
builder := NewArtifactBuilder(fakeLocalDaemonWithExtraEnv([]string{}), mockConfig{}, true, nil, false, mockArtifactResolver{make(map[string]string)}, nil)
builder := NewArtifactBuilder(fakeLocalDaemonWithExtraEnv([]string{}), mockConfig{}, true, nil, false, false, mockArtifactResolver{make(map[string]string)}, nil)

artifact := &latest.Artifact{
ImageName: "test-image",
Expand Down
2 changes: 1 addition & 1 deletion pkg/skaffold/build/local/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ func NewBuilder(ctx context.Context, bCtx BuilderContext, buildCfg *latest.Local
tryImportMissing := buildCfg.TryImportMissing

// for backward compatibility, extended build capabilities with BuildKit are disabled by default
buildx := bCtx.DetectBuildX()
buildx := bCtx.DetectBuildX() && docker.IsBuildXDetected()

return &Builder{
local: *buildCfg,
Expand Down
6 changes: 3 additions & 3 deletions pkg/skaffold/config/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ func GetDebugHelpersRegistry(configFile string) (string, error) {
func GetCacheTag(configFile string) (string, error) {
cfg, err := GetConfigForCurrentKubectx(configFile)
if err != nil {
log.Entry(context.TODO()).Errorf("Cannot read cache-tag from config: %w", err)
log.Entry(context.TODO()).Errorf("Cannot read cache-tag from config: %v", err)
return "", err
}
if cfg.CacheTag != "" {
Expand All @@ -232,9 +232,9 @@ func GetCacheTag(configFile string) (string, error) {
func GetDetectBuildX(configFile string) bool {
cfg, err := GetConfigForCurrentKubectx(configFile)
if err != nil {
log.Entry(context.TODO()).Errorf("Cannot read detect-buildx option from config: %w", err)
log.Entry(context.TODO()).Errorf("Cannot read detect-buildx option from config: %v", err)
} else if cfg.DetectBuildX != nil {
log.Entry(context.TODO()).Infof("Using detect-buildx=%s from config", *cfg.DetectBuildX)
log.Entry(context.TODO()).Infof("Using detect-buildx=%t from config", *cfg.DetectBuildX)
return *cfg.DetectBuildX
}
return false
Expand Down
2 changes: 1 addition & 1 deletion pkg/skaffold/docker/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ func LoadDockerConfig(access bool) (*configfile.ConfigFile, error) {
defer file.Close()
}
if err != nil {
log.Entry(context.TODO()).Warnf("cannot access docker config file %s: %w", configDir, err)
log.Entry(context.TODO()).Warnf("cannot access docker config file %s: %v", configDir, err)
}
}
return cf, err
Expand Down
2 changes: 1 addition & 1 deletion pkg/skaffold/docker/buildx.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ limitations under the License.

package docker

func DetectBuildX() bool {
func IsBuildXDetected() bool {
// detect if buildx was installed (docker build then is an alias for docker buildx build):
// https://github.com/docker/buildx/blob/master/commands/install.go
cf, err := LoadDockerConfig(true)
Expand Down
6 changes: 2 additions & 4 deletions pkg/skaffold/runner/runcontext/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import (

"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/config"
"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/constants"
"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/docker"
kubectx "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/kubernetes/context"
"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/output/log"
"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/schema/latest"
Expand Down Expand Up @@ -367,9 +366,8 @@ func (rc *RunContext) EnableGKEARMNodeTolerationInRenderedManifests() bool {

func (rc *RunContext) DetectBuildX() bool {
if config.GetDetectBuildX(rc.GlobalConfig()) {
buildx := docker.DetectBuildX()
log.Entry(context.TODO()).Debugf("buildx detection result is %t", buildx)
return buildx
log.Entry(context.TODO()).Debugf("buildx detection is enabled")
return true
} else {
log.Entry(context.TODO()).Debugf("buildx detection is disabled")
return false
Expand Down
2 changes: 1 addition & 1 deletion pkg/skaffold/schema/latest/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -1590,7 +1590,7 @@ type DockerArtifact struct {
CacheFrom []string `yaml:"cacheFrom,omitempty"`

// CacheTo lists the Docker images used as cache destination.
// If ommited, cacheFrom is used with max mode to export all layers
// If ommited, cacheFrom is used with max mode to export all layers.
// For example: `["type=registry,ref=gcr.io/k8s-skaffold/example:cache,mode=max"]`.
CacheTo []string `yaml:"cacheTo,omitempty"`

Expand Down

0 comments on commit ba2c5e7

Please sign in to comment.