Skip to content

Commit 8dde305

Browse files
committed
Add disticnt error codes for docker no space error and better suggestions
1 parent aa9b115 commit 8dde305

File tree

12 files changed

+380
-243
lines changed

12 files changed

+380
-243
lines changed

docs/content/en/api/skaffold.swagger.json

+31-15
Large diffs are not rendered by default.

docs/content/en/docs/references/api/grpc.md

+3
Original file line numberDiff line numberDiff line change
@@ -897,6 +897,7 @@ For Cancelled Error code, use range 800 to 850.<br>
897897
| BUILD_DOCKERFILE_NOT_FOUND | 115 | Docker build failed due to dockerfile not found |
898898
| BUILD_DOCKER_CACHE_FROM_PULL_ERR | 116 | Docker build failed due `cacheFrom` user config error |
899899
| BUILD_DOCKER_GET_DIGEST_ERR | 117 | Build error due to digest for built artifact could not be retrieved from docker daemon. |
900+
| BUILD_DOCKER_NO_SPACE_ERR | 127 | Build error due no space left in docker. |
900901
| BUILD_REGISTRY_GET_DIGEST_ERR | 118 | Build error due to digest for built artifact could not be retrieved from registry. |
901902
| BUILD_UNKNOWN_JIB_PLUGIN_TYPE | 119 | Build error indicating unknown Jib plugin type. Should be one of [maven, gradle] |
902903
| BUILD_JIB_GRADLE_DEP_ERR | 120 | Build error determining dependency for jib gradle project. |
@@ -1037,6 +1038,8 @@ Enum for Suggestion codes
10371038
| FIX_DOCKER_NETWORK_CONTAINER_NAME | 112 | Docker build network invalid docker container name (or id). |
10381039
| CHECK_DOCKER_NETWORK_CONTAINER_RUNNING | 113 | Docker build network container not existing in the current context. |
10391040
| FIX_DOCKER_NETWORK_MODE_WHEN_EXTRACTING_CONTAINER_NAME | 114 | Executing extractContainerNameFromNetworkMode with a non valid mode (only container mode allowed) |
1041+
| RUN_DOCKER_PRUNE | 115 | Prune Docker image |
1042+
| SET_CLEANUP_FLAG | 116 | Set Cleanup flag for skaffold command. |
10401043
| CHECK_CLUSTER_CONNECTION | 201 | Check cluster connection |
10411044
| CHECK_MINIKUBE_STATUS | 202 | Check minikube status |
10421045
| INSTALL_HELM | 203 | Install helm tool |

pkg/skaffold/build/docker/docker.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ func (b *Builder) Build(ctx context.Context, out io.Writer, a *latestV1.Artifact
5050
if err := b.pullCacheFromImages(ctx, out, a.ArtifactType.DockerArtifact); err != nil {
5151
return "", cacheFromPullErr(err, a.ImageName)
5252
}
53-
opts := docker.BuildOptions{Tag: tag, Mode: b.mode, ExtraBuildArgs: docker.ResolveDependencyImages(a.Dependencies, b.artifacts, true)}
53+
opts := docker.BuildOptions{Tag: tag, Mode: b.cfg.Mode(), ExtraBuildArgs: docker.ResolveDependencyImages(a.Dependencies, b.artifacts, true)}
5454

5555
var imageID string
5656

@@ -61,7 +61,7 @@ func (b *Builder) Build(ctx context.Context, out io.Writer, a *latestV1.Artifact
6161
}
6262

6363
if err != nil {
64-
return "", newBuildError(err)
64+
return "", newBuildError(err, b.cfg)
6565
}
6666

6767
if b.pushImages {
@@ -75,7 +75,7 @@ func (b *Builder) Build(ctx context.Context, out io.Writer, a *latestV1.Artifact
7575

7676
func (b *Builder) dockerCLIBuild(ctx context.Context, out io.Writer, workspace string, dockerfilePath string, a *latestV1.DockerArtifact, opts docker.BuildOptions) (string, error) {
7777
args := []string{"build", workspace, "--file", dockerfilePath, "-t", opts.Tag}
78-
ba, err := docker.EvalBuildArgs(b.mode, workspace, a.DockerfilePath, a.BuildArgs, opts.ExtraBuildArgs)
78+
ba, err := docker.EvalBuildArgs(b.cfg.Mode(), workspace, a.DockerfilePath, a.BuildArgs, opts.ExtraBuildArgs)
7979
if err != nil {
8080
return "", fmt.Errorf("unable to evaluate build args: %w", err)
8181
}
@@ -85,7 +85,7 @@ func (b *Builder) dockerCLIBuild(ctx context.Context, out io.Writer, workspace s
8585
}
8686
args = append(args, cliArgs...)
8787

88-
if b.prune {
88+
if b.cfg.Prune() {
8989
args = append(args, "--force-rm")
9090
}
9191

pkg/skaffold/build/docker/docker_test.go

+82-11
Original file line numberDiff line numberDiff line change
@@ -28,25 +28,28 @@ import (
2828

2929
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
3030
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker"
31+
sErrors "github.com/GoogleContainerTools/skaffold/pkg/skaffold/errors"
3132
latestV1 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest/v1"
3233
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
34+
"github.com/GoogleContainerTools/skaffold/proto/v1"
3335
"github.com/GoogleContainerTools/skaffold/testutil"
3436
)
3537

3638
func TestDockerCLIBuild(t *testing.T) {
3739
tests := []struct {
38-
description string
39-
localBuild latestV1.LocalBuild
40-
mode config.RunMode
41-
extraEnv []string
42-
expectedEnv []string
43-
err error
44-
expectedErr error
40+
description string
41+
localBuild latestV1.LocalBuild
42+
cfg mockConfig
43+
extraEnv []string
44+
expectedEnv []string
45+
err error
46+
expectedErr error
47+
expectedErrCode proto.StatusCode
4548
}{
4649
{
4750
description: "docker build",
4851
localBuild: latestV1.LocalBuild{},
49-
mode: config.RunModes.Dev,
52+
cfg: mockConfig{runMode: config.RunModes.Dev},
5053
expectedEnv: []string{"KEY=VALUE"},
5154
},
5255
{
@@ -77,7 +80,38 @@ func TestDockerCLIBuild(t *testing.T) {
7780
description: "docker build internal error",
7881
localBuild: latestV1.LocalBuild{UseDockerCLI: true},
7982
err: errdefs.Cancelled(fmt.Errorf("cancelled")),
80-
expectedErr: newBuildError(errdefs.Cancelled(fmt.Errorf("cancelled"))),
83+
expectedErr: newBuildError(errdefs.Cancelled(fmt.Errorf("cancelled")), mockConfig{runMode: config.RunModes.Dev}),
84+
},
85+
{
86+
description: "docker build no space left error with prune for dev",
87+
localBuild: latestV1.LocalBuild{UseDockerCLI: true},
88+
cfg: mockConfig{runMode: config.RunModes.Dev, prune: false},
89+
err: errdefs.System(fmt.Errorf("no space left")),
90+
expectedErr: fmt.Errorf("Docker ran out of memory. Please run 'docker system prune' to removed unused docker data or Run skaffold dev with --cleanup=true to clean up images built by skaffold"),
91+
expectedErrCode: proto.StatusCode_BUILD_DOCKER_NO_SPACE_ERR,
92+
},
93+
{
94+
description: "docker build no space left error with prune for build",
95+
localBuild: latestV1.LocalBuild{UseDockerCLI: true},
96+
cfg: mockConfig{runMode: config.RunModes.Build, prune: false},
97+
err: errdefs.System(fmt.Errorf("no space left")),
98+
expectedErr: fmt.Errorf("no space left: Docker ran out of memory. Please run 'docker system prune' to removed unused docker data"),
99+
expectedErrCode: proto.StatusCode_BUILD_DOCKER_NO_SPACE_ERR,
100+
},
101+
{
102+
description: "docker build no space left error with prune true",
103+
localBuild: latestV1.LocalBuild{UseDockerCLI: true},
104+
cfg: mockConfig{prune: true},
105+
err: errdefs.System(fmt.Errorf("no space left")),
106+
expectedErr: fmt.Errorf("no space left: Docker ran out of memory. Please run 'docker system prune' to removed unused docker data"),
107+
expectedErrCode: proto.StatusCode_BUILD_DOCKER_NO_SPACE_ERR,
108+
},
109+
{
110+
description: "docker build system error",
111+
localBuild: latestV1.LocalBuild{UseDockerCLI: true},
112+
err: errdefs.System(fmt.Errorf("something else")),
113+
expectedErr: fmt.Errorf("something else"),
114+
expectedErrCode: proto.StatusCode_BUILD_DOCKER_SYSTEM_ERR,
81115
},
82116
}
83117

@@ -92,8 +126,12 @@ func TestDockerCLIBuild(t *testing.T) {
92126
var mockCmd *testutil.FakeCmd
93127

94128
if test.err != nil {
129+
var pruneFlag string
130+
if test.cfg.Prune() {
131+
pruneFlag = " --force-rm"
132+
}
95133
mockCmd = testutil.CmdRunErr(
96-
"docker build . --file "+dockerfilePath+" -t tag",
134+
"docker build . --file "+dockerfilePath+" -t tag"+pruneFlag,
97135
test.err,
98136
)
99137
t.Override(&util.DefaultExecCommand, mockCmd)
@@ -106,7 +144,7 @@ func TestDockerCLIBuild(t *testing.T) {
106144
}
107145
t.Override(&util.OSEnviron, func() []string { return []string{"KEY=VALUE"} })
108146

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

111149
artifact := &latestV1.Artifact{
112150
Workspace: ".",
@@ -122,6 +160,14 @@ func TestDockerCLIBuild(t *testing.T) {
122160
if mockCmd != nil {
123161
t.CheckDeepEqual(1, mockCmd.TimesCalled())
124162
}
163+
if test.err != nil && test.expectedErrCode != 0 {
164+
if ae, ok := err.(sErrors.ErrDef); ok {
165+
t.CheckDeepEqual(test.expectedErrCode, ae.StatusCode())
166+
t.CheckErrorContains(test.expectedErr.Error(), ae)
167+
} else {
168+
t.Fatalf("expected to find an actionable error. not found")
169+
}
170+
}
125171
})
126172
}
127173
}
@@ -150,3 +196,28 @@ func (t stubAuth) GetAuthConfig(string) (types.AuthConfig, error) {
150196
func (t stubAuth) GetAllAuthConfigs(context.Context) (map[string]types.AuthConfig, error) {
151197
return nil, nil
152198
}
199+
200+
type mockConfig struct {
201+
runMode config.RunMode
202+
prune bool
203+
}
204+
205+
func (m mockConfig) GetKubeContext() string {
206+
return ""
207+
}
208+
209+
func (m mockConfig) MinikubeProfile() string {
210+
return ""
211+
}
212+
213+
func (m mockConfig) GetInsecureRegistries() map[string]bool {
214+
return map[string]bool{}
215+
}
216+
217+
func (m mockConfig) Mode() config.RunMode {
218+
return m.runMode
219+
}
220+
221+
func (m mockConfig) Prune() bool {
222+
return m.prune
223+
}

pkg/skaffold/build/docker/errors.go

+51-26
Original file line numberDiff line numberDiff line change
@@ -19,17 +19,24 @@ package docker
1919
import (
2020
"errors"
2121
"fmt"
22+
"regexp"
2223

2324
"github.com/docker/docker/errdefs"
2425
"github.com/docker/docker/pkg/jsonmessage"
2526

27+
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
28+
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker"
2629
sErrors "github.com/GoogleContainerTools/skaffold/pkg/skaffold/errors"
2730
"github.com/GoogleContainerTools/skaffold/proto/v1"
2831
)
2932

33+
var (
34+
noSpaceLeft = regexp.MustCompile(".*no space left.*")
35+
)
36+
3037
// newBuildError turns Docker-specific errors into actionable errors.
3138
// The input errors are assumed to be from the Skaffold docker invocation.
32-
func newBuildError(err error) error {
39+
func newBuildError(err error, cfg docker.Config) error {
3340
errU := errors.Unwrap(err)
3441
if errU == nil {
3542
return err
@@ -49,48 +56,66 @@ func newBuildError(err error) error {
4956
},
5057
})
5158
default:
52-
return sErrors.NewError(err,
53-
proto.ActionableErr{
54-
Message: errU.Error(),
55-
ErrCode: getErrorCode(errU),
56-
Suggestions: []*proto.Suggestion{
57-
{
58-
SuggestionCode: proto.SuggestionCode_DOCKER_BUILD_RETRY,
59-
Action: "Docker build ran into internal error. Please retry.\nIf this keeps happening, please open an issue.",
60-
},
61-
},
62-
})
59+
return sErrors.NewError(err, getActionableErr(errU, cfg))
6360
}
6461
}
6562

66-
func getErrorCode(err error) proto.StatusCode {
63+
func getActionableErr(err error, cfg docker.Config) proto.ActionableErr {
64+
var errCode proto.StatusCode
65+
suggestions := []*proto.Suggestion{
66+
{
67+
SuggestionCode: proto.SuggestionCode_DOCKER_BUILD_RETRY,
68+
Action: "Docker build ran into internal error. Please retry.\nIf this keeps happening, please open an issue.",
69+
},
70+
}
6771
switch err.(type) {
6872
case errdefs.ErrNotFound:
69-
return proto.StatusCode_BUILD_DOCKER_ERROR_NOT_FOUND
73+
errCode = proto.StatusCode_BUILD_DOCKER_ERROR_NOT_FOUND
7074
case errdefs.ErrInvalidParameter:
71-
return proto.StatusCode_BUILD_DOCKER_INVALID_PARAM_ERR
75+
errCode = proto.StatusCode_BUILD_DOCKER_INVALID_PARAM_ERR
7276
case errdefs.ErrConflict:
73-
return proto.StatusCode_BUILD_DOCKER_CONFLICT_ERR
77+
errCode = proto.StatusCode_BUILD_DOCKER_CONFLICT_ERR
7478
case errdefs.ErrCancelled:
75-
return proto.StatusCode_BUILD_DOCKER_CANCELLED
79+
errCode = proto.StatusCode_BUILD_DOCKER_CANCELLED
7680
case errdefs.ErrForbidden:
77-
return proto.StatusCode_BUILD_DOCKER_FORBIDDEN_ERR
81+
errCode = proto.StatusCode_BUILD_DOCKER_FORBIDDEN_ERR
7882
case errdefs.ErrDataLoss:
79-
return proto.StatusCode_BUILD_DOCKER_DATA_LOSS_ERR
83+
errCode = proto.StatusCode_BUILD_DOCKER_DATA_LOSS_ERR
8084
case errdefs.ErrDeadline:
81-
return proto.StatusCode_BUILD_DOCKER_DEADLINE
85+
errCode = proto.StatusCode_BUILD_DOCKER_DEADLINE
8286
case errdefs.ErrNotImplemented:
83-
return proto.StatusCode_BUILD_DOCKER_NOT_IMPLEMENTED_ERR
87+
errCode = proto.StatusCode_BUILD_DOCKER_NOT_IMPLEMENTED_ERR
8488
case errdefs.ErrNotModified:
85-
return proto.StatusCode_BUILD_DOCKER_NOT_MODIFIED_ERR
89+
errCode = proto.StatusCode_BUILD_DOCKER_NOT_MODIFIED_ERR
8690
case errdefs.ErrSystem:
87-
return proto.StatusCode_BUILD_DOCKER_SYSTEM_ERR
91+
errCode = proto.StatusCode_BUILD_DOCKER_SYSTEM_ERR
92+
if noSpaceLeft.MatchString(err.Error()) {
93+
errCode = proto.StatusCode_BUILD_DOCKER_NO_SPACE_ERR
94+
suggestions = []*proto.Suggestion{
95+
{
96+
SuggestionCode: proto.SuggestionCode_RUN_DOCKER_PRUNE,
97+
Action: "Docker ran out of memory. Please run 'docker system prune' to removed unused docker data",
98+
},
99+
}
100+
if !cfg.Prune() && (cfg.Mode() == config.RunModes.Dev || cfg.Mode() == config.RunModes.Debug) {
101+
suggestions = append(suggestions, &proto.Suggestion{
102+
SuggestionCode: proto.SuggestionCode_SET_CLEANUP_FLAG,
103+
Action: fmt.Sprintf("Run skaffold %s with --cleanup=true to clean up images built by skaffold", cfg.Mode()),
104+
})
105+
}
106+
}
88107
case errdefs.ErrUnauthorized:
89-
return proto.StatusCode_BUILD_DOCKER_UNAUTHORIZED
108+
errCode = proto.StatusCode_BUILD_DOCKER_UNAUTHORIZED
90109
case errdefs.ErrUnavailable:
91-
return proto.StatusCode_BUILD_DOCKER_UNAVAILABLE
110+
errCode = proto.StatusCode_BUILD_DOCKER_UNAVAILABLE
92111
default:
93-
return proto.StatusCode_BUILD_DOCKER_UNKNOWN
112+
errCode = proto.StatusCode_BUILD_DOCKER_UNKNOWN
113+
}
114+
115+
return proto.ActionableErr{
116+
Message: err.Error(),
117+
ErrCode: errCode,
118+
Suggestions: suggestions,
94119
}
95120
}
96121

pkg/skaffold/build/docker/errors_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ Refer https://skaffold.dev/docs/references/yaml/#build-artifacts-docker for deta
5959
"docker build . --file "+dockerfilePath+" -t tag",
6060
))
6161
t.Override(&docker.DefaultAuthHelper, stubAuth{})
62-
builder := NewArtifactBuilder(fakeLocalDaemonWithExtraEnv([]string{}), false, true, false, false, config.RunModes.Build, nil, mockArtifactResolver{make(map[string]string)}, nil)
62+
builder := NewArtifactBuilder(fakeLocalDaemonWithExtraEnv([]string{}), mockConfig{}, true, false, false, mockArtifactResolver{make(map[string]string)}, nil)
6363

6464
artifact := &latestV1.Artifact{
6565
ImageName: "test-image",

pkg/skaffold/build/docker/types.go

+3-8
Original file line numberDiff line numberDiff line change
@@ -19,20 +19,17 @@ package docker
1919
import (
2020
"context"
2121

22-
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
2322
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker"
2423
latestV1 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest/v1"
2524
)
2625

2726
// Builder is an artifact builder that uses docker
2827
type Builder struct {
2928
localDocker docker.LocalDaemon
29+
cfg docker.Config
3030
pushImages bool
31-
prune bool
3231
useCLI bool
3332
useBuildKit bool
34-
mode config.RunMode
35-
insecureRegistries map[string]bool
3633
artifacts ArtifactResolver
3734
sourceDependencies TransitiveSourceDependenciesResolver
3835
}
@@ -48,15 +45,13 @@ type TransitiveSourceDependenciesResolver interface {
4845
}
4946

5047
// NewBuilder returns an new instance of a docker builder
51-
func NewArtifactBuilder(localDocker docker.LocalDaemon, useCLI, useBuildKit, pushImages, prune bool, mode config.RunMode, insecureRegistries map[string]bool, ar ArtifactResolver, dr TransitiveSourceDependenciesResolver) *Builder {
48+
func NewArtifactBuilder(localDocker docker.LocalDaemon, cfg docker.Config, useCLI, useBuildKit, pushImages bool, ar ArtifactResolver, dr TransitiveSourceDependenciesResolver) *Builder {
5249
return &Builder{
5350
localDocker: localDocker,
5451
pushImages: pushImages,
55-
prune: prune,
52+
cfg: cfg,
5653
useCLI: useCLI,
5754
useBuildKit: useBuildKit,
58-
mode: mode,
59-
insecureRegistries: insecureRegistries,
6055
artifacts: ar,
6156
sourceDependencies: dr,
6257
}

pkg/skaffold/build/local/types.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ type artifactBuilder interface {
140140
func newPerArtifactBuilder(b *Builder, a *latestV1.Artifact) (artifactBuilder, error) {
141141
switch {
142142
case a.DockerArtifact != nil:
143-
return dockerbuilder.NewArtifactBuilder(b.localDocker, b.local.UseDockerCLI, b.local.UseBuildkit, b.pushImages, b.prune, b.cfg.Mode(), b.cfg.GetInsecureRegistries(), b.artifactStore, b.sourceDependencies), nil
143+
return dockerbuilder.NewArtifactBuilder(b.localDocker, b.cfg, b.local.UseDockerCLI, b.local.UseBuildkit, b.pushImages, b.artifactStore, b.sourceDependencies), nil
144144

145145
case a.BazelArtifact != nil:
146146
return bazel.NewArtifactBuilder(b.localDocker, b.cfg, b.pushImages), nil

0 commit comments

Comments
 (0)