Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: extracted kaniko copyTimeout and copyMaxRetries into config #9267

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 14 additions & 1 deletion docs-v2/content/en/schemas/v4beta10.json
Original file line number Diff line number Diff line change
Expand Up @@ -2567,6 +2567,17 @@
"description": "to specify a sub path within the context.",
"x-intellij-html-description": "to specify a sub path within the context."
},
"copyMaxRetries": {
"type": "integer",
"description": "number of times to retry copy build contexts to a cluster if it fails.",
"x-intellij-html-description": "number of times to retry copy build contexts to a cluster if it fails.",
"default": "3"
},
"copyTimeout": {
"type": "string",
"description": "timeout for copying build contexts to a cluster. Defaults to 5 minutes (`5m`).",
"x-intellij-html-description": "timeout for copying build contexts to a cluster. Defaults to 5 minutes (<code>5m</code>)."
},
"digestFile": {
"type": "string",
"description": "to specify a file in the container. This file will receive the digest of a built image. This can be used to automatically track the exact image built by kaniko.",
Expand Down Expand Up @@ -2803,7 +2814,9 @@
"buildArgs",
"volumeMounts",
"contextSubPath",
"ignorePaths"
"ignorePaths",
"copyMaxRetries",
"copyTimeout"
],
"additionalProperties": false,
"type": "object",
Expand Down
21 changes: 14 additions & 7 deletions pkg/skaffold/build/cluster/kaniko.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,6 @@ import (

const (
initContainer = "kaniko-init-container"
// copyMaxRetries is the number of times to retry copy build contexts to a cluster if it fails.
copyMaxRetries = 3
// copyTimeout is the timeout for copying build contexts to a cluster.
copyTimeout = 5 * time.Minute
)

func (b *Builder) buildWithKaniko(ctx context.Context, out io.Writer, workspace string, artifactName string, artifact *latest.KanikoArtifact, tag string, requiredImages map[string]*string, platforms platform.Matcher) (string, error) {
Expand Down Expand Up @@ -114,6 +110,12 @@ func (b *Builder) buildWithKaniko(ctx context.Context, out io.Writer, workspace
}

func (b *Builder) copyKanikoBuildContext(ctx context.Context, workspace string, artifactName string, artifact *latest.KanikoArtifact, podName string) error {
copyTimeout, err := time.ParseDuration(artifact.CopyTimeout)

if err != nil {
return err
}

ctx, cancel := context.WithTimeout(ctx, copyTimeout)
defer cancel()
errs := make(chan error, 1)
Expand Down Expand Up @@ -153,13 +155,18 @@ func (b *Builder) setupKanikoBuildContext(ctx context.Context, workspace string,
// Retry uploading the build context in case of an error.
// total attempts is `uploadMaxRetries + 1`
attempt := 1
err := wait.Poll(time.Second, copyTimeout*(copyMaxRetries+1), func() (bool, error) {
timeout, err := time.ParseDuration(artifact.CopyTimeout)
if err != nil {
return fmt.Errorf("parsing timeout: %w", err)
}

err = wait.Poll(time.Second, timeout*time.Duration(*artifact.CopyMaxRetries+1), func() (bool, error) {
if err := b.copyKanikoBuildContext(ctx, workspace, artifactName, artifact, podName); err != nil {
if errors.Is(ctx.Err(), context.Canceled) {
return false, err
}
log.Entry(ctx).Warnf("uploading build context failed, retrying (%d/%d): %v", attempt, copyMaxRetries, err)
if attempt == copyMaxRetries {
log.Entry(ctx).Warnf("uploading build context failed, retrying (%d/%d): %v", attempt, *artifact.CopyMaxRetries, err)
if attempt == *artifact.CopyMaxRetries {
return false, err
}
attempt++
Expand Down
6 changes: 5 additions & 1 deletion pkg/skaffold/build/kaniko/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,10 @@ const (
DefaultDockerConfigPath = "/kaniko/.docker"
// DefaultSecretMountPath for kaniko pod
DefaultSecretMountPath = "/secret"
// IgnorePath additional flag
// IgnorePathFlag additional flag
IgnorePathFlag = "--ignore-path"
// DefaultCopyMaxRetries for kaniko pod
DefaultCopyMaxRetries = 3
// DefaultCopyTimeout for kaniko pod
DefaultCopyTimeout = "5m"
)
9 changes: 9 additions & 0 deletions pkg/skaffold/schema/defaults/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,8 @@ func setKanikoArtifactDefaults(a *latest.KanikoArtifact) {
a.DockerfilePath = valueOrDefault(a.DockerfilePath, constants.DefaultDockerfilePath)
a.InitImage = valueOrDefault(a.InitImage, constants.DefaultBusyboxImage)
a.DigestFile = valueOrDefault(a.DigestFile, constants.DefaultKanikoDigestFile)
a.CopyMaxRetries = valueOrDefaultInt(a.CopyMaxRetries, kaniko.DefaultCopyMaxRetries)
a.CopyTimeout = valueOrDefault(a.CopyTimeout, kaniko.DefaultCopyTimeout)
}

func valueOrDefault(v, def string) string {
Expand All @@ -371,6 +373,13 @@ func valueOrDefault(v, def string) string {
return def
}

func valueOrDefaultInt(v *int, def int) *int {
if v != nil {
return v
}
return &def
}

func currentNamespace() (string, error) {
cfg, err := kubectx.CurrentConfig()
if err != nil {
Expand Down
10 changes: 10 additions & 0 deletions pkg/skaffold/schema/defaults/defaults_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,12 @@ func TestSetDefaults(t *testing.T) {
},
Sync: &latest.Sync{Auto: util.Ptr(false)},
},
{
ImageName: "eights",
ArtifactType: latest.ArtifactType{
KanikoArtifact: &latest.KanikoArtifact{},
},
},
},
},
},
Expand Down Expand Up @@ -126,6 +132,10 @@ func TestSetDefaults(t *testing.T) {
testutil.CheckDeepEqual(t, []string(nil), cfg.Build.Artifacts[6].BuildpackArtifact.Dependencies.Ignore)
testutil.CheckDeepEqual(t, "project.toml", cfg.Build.Artifacts[6].BuildpackArtifact.ProjectDescriptor)
testutil.CheckDeepEqual(t, util.Ptr(false), cfg.Build.Artifacts[6].Sync.Auto)

testutil.CheckDeepEqual(t, "eights", cfg.Build.Artifacts[7].ImageName)
testutil.CheckDeepEqual(t, 3, *cfg.Build.Artifacts[7].KanikoArtifact.CopyMaxRetries)
testutil.CheckDeepEqual(t, "5m", cfg.Build.Artifacts[7].KanikoArtifact.CopyTimeout)
}

func TestSetDefaultsOnCluster(t *testing.T) {
Expand Down
8 changes: 8 additions & 0 deletions pkg/skaffold/schema/latest/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -1488,6 +1488,14 @@ type KanikoArtifact struct {

// IgnorePaths is a list of ignored paths when making an image snapshot.
IgnorePaths []string `yaml:"ignorePaths,omitempty"`

// CopyMaxRetries is the number of times to retry copy build contexts to a cluster if it fails.
// Defaults to `3`.
CopyMaxRetries *int `yaml:"copyMaxRetries,omitempty"`

// CopyTimeout is the timeout for copying build contexts to a cluster.
// Defaults to 5 minutes (`5m`).
CopyTimeout string `yaml:"copyTimeout,omitempty"`
}

// DockerArtifact describes an artifact built from a Dockerfile,
Expand Down
3 changes: 3 additions & 0 deletions pkg/skaffold/schema/versions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -620,6 +620,7 @@ func withBazelArtifact() func(*latest.BuildConfig) {

func withKanikoArtifact() func(*latest.BuildConfig) {
return func(cfg *latest.BuildConfig) {
copyMaxRetries := 3
cfg.Artifacts = append(cfg.Artifacts, &latest.Artifact{
ImageName: "image1",
Workspace: "./examples/app1",
Expand All @@ -629,6 +630,8 @@ func withKanikoArtifact() func(*latest.BuildConfig) {
InitImage: constants.DefaultBusyboxImage,
Image: kaniko.DefaultImage,
DigestFile: "/dev/termination-log",
CopyMaxRetries: &copyMaxRetries,
CopyTimeout: "5m",
},
},
})
Expand Down