From ac52976c4a8ca990ea92924d401ed7773e33e521 Mon Sep 17 00:00:00 2001 From: Anna Levenberg Date: Fri, 10 Nov 2023 01:12:20 -0500 Subject: [PATCH 1/5] impl: add a retry with result function (#2837) * impl: add a retry with result function * fix ci errs --- README.md | 7 ++++++- cmd/executor/cmd/root.go | 1 + pkg/config/options.go | 1 + pkg/image/remote/remote.go | 16 ++++++++++++---- 4 files changed, 20 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 5df77e210d..922679e23c 100644 --- a/README.md +++ b/README.md @@ -112,7 +112,8 @@ _If you are interested in contributing to kaniko, see - [Flag `--ignore-var-run`](#flag---ignore-var-run) - [Flag `--ignore-path`](#flag---ignore-path) - [Flag `--image-fs-extract-retry`](#flag---image-fs-extract-retry) - - [Debug Image](#debug-image) + - [Flag `--image-download-retry`](#flag---image-download-retry) + - [Debug Image](#debug-image) - [Security](#security) - [Verifying Signed Kaniko Images](#verifying-signed-kaniko-images) - [Kaniko Builds - Profiling](#kaniko-builds---profiling) @@ -1094,6 +1095,10 @@ snapshot. Set it multiple times for multiple ignore paths. Set this flag to the number of retries that should happen for the extracting an image filesystem. Defaults to `0`. +#### Flag `--image-download-retry` + +Set this flag to the number of retries that should happen when downloading the remote image. Defaults to `0`. + ### Debug Image The kaniko executor image is based on scratch and doesn't contain a shell. We diff --git a/cmd/executor/cmd/root.go b/cmd/executor/cmd/root.go index 90bf63a15b..84310739b8 100644 --- a/cmd/executor/cmd/root.go +++ b/cmd/executor/cmd/root.go @@ -212,6 +212,7 @@ func addKanikoOptionsFlags() { RootCmd.PersistentFlags().BoolVarP(&opts.SkipTLSVerifyPull, "skip-tls-verify-pull", "", false, "Pull from insecure registry ignoring TLS verify") RootCmd.PersistentFlags().IntVar(&opts.PushRetry, "push-retry", 0, "Number of retries for the push operation") RootCmd.PersistentFlags().IntVar(&opts.ImageFSExtractRetry, "image-fs-extract-retry", 0, "Number of retries for image FS extraction") + RootCmd.PersistentFlags().IntVar(&opts.ImageDownloadRetry, "image-download-retry", 0, "Number of retries for downloading the remote image") RootCmd.PersistentFlags().StringVarP(&opts.KanikoDir, "kaniko-dir", "", constants.DefaultKanikoPath, "Path to the kaniko directory, this takes precedence over the KANIKO_DIR environment variable.") RootCmd.PersistentFlags().StringVarP(&opts.TarPath, "tar-path", "", "", "Path to save the image in as a tarball instead of pushing") RootCmd.PersistentFlags().BoolVarP(&opts.SingleSnapshot, "single-snapshot", "", false, "Take a single snapshot at the end of the build.") diff --git a/pkg/config/options.go b/pkg/config/options.go index c82e375e81..df419ff64c 100644 --- a/pkg/config/options.go +++ b/pkg/config/options.go @@ -43,6 +43,7 @@ type RegistryOptions struct { InsecurePull bool SkipTLSVerifyPull bool PushRetry int + ImageDownloadRetry int } // KanikoOptions are options that are set by command line arguments diff --git a/pkg/image/remote/remote.go b/pkg/image/remote/remote.go index b47cbb2f03..2374935c7b 100644 --- a/pkg/image/remote/remote.go +++ b/pkg/image/remote/remote.go @@ -70,8 +70,13 @@ func RetrieveRemoteImage(image string, opts config.RegistryOptions, customPlatfo ref := setNewRegistry(ref, newReg) logrus.Infof("Retrieving image %s from registry mirror %s", ref, registryMirror) - remoteImage, err := remoteImageFunc(ref, remoteOptions(registryMirror, opts, customPlatform)...) - if err != nil { + retryFunc := func() (v1.Image, error) { + return remoteImageFunc(ref, remoteOptions(registryMirror, opts, customPlatform)...) + } + + var remoteImage v1.Image + var err error + if remoteImage, err = util.RetryWithResult(retryFunc, opts.ImageDownloadRetry, 1000); err != nil { logrus.Warnf("Failed to retrieve image %s from registry mirror %s: %s. Will try with the next mirror, or fallback to the default registry.", ref, registryMirror, err) continue } @@ -97,9 +102,12 @@ func RetrieveRemoteImage(image string, opts config.RegistryOptions, customPlatfo logrus.Infof("Retrieving image %s from registry %s", ref, registryName) - remoteImage, err := remoteImageFunc(ref, remoteOptions(registryName, opts, customPlatform)...) + retryFunc := func() (v1.Image, error) { + return remoteImageFunc(ref, remoteOptions(registryName, opts, customPlatform)...) + } - if remoteImage != nil { + var remoteImage v1.Image + if remoteImage, err = util.RetryWithResult(retryFunc, opts.ImageDownloadRetry, 1000); remoteImage != nil { manifestCache[image] = remoteImage } From da533e6ddf156fef370bff03ad56e57ec61d566e Mon Sep 17 00:00:00 2001 From: Anna Levenberg Date: Wed, 15 Nov 2023 10:59:12 -0500 Subject: [PATCH 2/5] test: add unit tests --- pkg/image/remote/remote_test.go | 49 ++++++++++++++++++++++++++++++++- 1 file changed, 48 insertions(+), 1 deletion(-) diff --git a/pkg/image/remote/remote_test.go b/pkg/image/remote/remote_test.go index 0a8103c51e..9981516a9d 100644 --- a/pkg/image/remote/remote_test.go +++ b/pkg/image/remote/remote_test.go @@ -28,7 +28,8 @@ import ( ) // mockImage mocks the v1.Image interface -type mockImage struct{} +type mockImage struct{ +} func (m *mockImage) ConfigFile() (*v1.ConfigFile, error) { return nil, nil @@ -140,3 +141,49 @@ func Test_RetrieveRemoteImage_skipFallback(t *testing.T) { t.Fatal("Expected call to fail because fallback to default registry is skipped") } } + +func Test_RetryRetrieveRemoteImageSucceeds(t *testing.T) { + image := "debian" + + opts := config.RegistryOptions{ + ImageDownloadRetry: 2, + } + attempts := 0 + remoteImageFunc = func(ref name.Reference, options ...remote.Option) (v1.Image, error) { + if attempts < 2{ + attempts ++ + return nil, errors.New("no image found") + } + return &mockImage{}, nil + } + + // Clean cached image + manifestCache = make(map[string]v1.Image) + + if _, err := RetrieveRemoteImage(image, opts, ""); err != nil { + t.Fatal("Expected call to succeed because of retry") + } +} + +func Test_NoRetryRetrieveRemoteImageFails(t *testing.T) { + image := "debian" + + opts := config.RegistryOptions{ + ImageDownloadRetry: 0, + } + attempts := 0 + remoteImageFunc = func(ref name.Reference, options ...remote.Option) (v1.Image, error) { + if attempts < 1 { + attempts ++ + return nil, errors.New("no image found") + } + return &mockImage{}, nil + } + + // Clean cached image + manifestCache = make(map[string]v1.Image) + + if _, err := RetrieveRemoteImage(image, opts, ""); err == nil { + t.Fatal("Expected call to fail because there is no retry") + } +} \ No newline at end of file From 2fd44f2c5739b4afd8aa28cad1c6ee8708bb4b95 Mon Sep 17 00:00:00 2001 From: Anna Levenberg Date: Wed, 15 Nov 2023 11:00:22 -0500 Subject: [PATCH 3/5] gofmt --- pkg/image/remote/remote_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/image/remote/remote_test.go b/pkg/image/remote/remote_test.go index 9981516a9d..e93b1f5a4c 100644 --- a/pkg/image/remote/remote_test.go +++ b/pkg/image/remote/remote_test.go @@ -28,7 +28,7 @@ import ( ) // mockImage mocks the v1.Image interface -type mockImage struct{ +type mockImage struct { } func (m *mockImage) ConfigFile() (*v1.ConfigFile, error) { @@ -150,8 +150,8 @@ func Test_RetryRetrieveRemoteImageSucceeds(t *testing.T) { } attempts := 0 remoteImageFunc = func(ref name.Reference, options ...remote.Option) (v1.Image, error) { - if attempts < 2{ - attempts ++ + if attempts < 2 { + attempts++ return nil, errors.New("no image found") } return &mockImage{}, nil @@ -173,8 +173,8 @@ func Test_NoRetryRetrieveRemoteImageFails(t *testing.T) { } attempts := 0 remoteImageFunc = func(ref name.Reference, options ...remote.Option) (v1.Image, error) { - if attempts < 1 { - attempts ++ + if attempts < 1 { + attempts++ return nil, errors.New("no image found") } return &mockImage{}, nil @@ -186,4 +186,4 @@ func Test_NoRetryRetrieveRemoteImageFails(t *testing.T) { if _, err := RetrieveRemoteImage(image, opts, ""); err == nil { t.Fatal("Expected call to fail because there is no retry") } -} \ No newline at end of file +} From 8196fa104294395ef74aaa4ff15e0cbe883971be Mon Sep 17 00:00:00 2001 From: Anna Levenberg Date: Wed, 15 Nov 2023 11:11:30 -0500 Subject: [PATCH 4/5] make debian a const --- pkg/image/remote/remote_test.go | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/pkg/image/remote/remote_test.go b/pkg/image/remote/remote_test.go index e93b1f5a4c..7fb6089d4a 100644 --- a/pkg/image/remote/remote_test.go +++ b/pkg/image/remote/remote_test.go @@ -27,6 +27,8 @@ import ( "github.com/google/go-containerregistry/pkg/v1/types" ) +const image string = "debian" + // mockImage mocks the v1.Image interface type mockImage struct { } @@ -80,7 +82,6 @@ func (m *mockImage) Size() (int64, error) { } func Test_normalizeReference(t *testing.T) { - image := "debian" expected := "index.docker.io/library/debian:latest" ref, err := name.ParseReference(image) @@ -113,7 +114,6 @@ func Test_RetrieveRemoteImage_manifestCache(t *testing.T) { } func Test_RetrieveRemoteImage_skipFallback(t *testing.T) { - image := "debian" registryMirror := "some-registry" opts := config.RegistryOptions{ @@ -143,8 +143,6 @@ func Test_RetrieveRemoteImage_skipFallback(t *testing.T) { } func Test_RetryRetrieveRemoteImageSucceeds(t *testing.T) { - image := "debian" - opts := config.RegistryOptions{ ImageDownloadRetry: 2, } @@ -166,8 +164,6 @@ func Test_RetryRetrieveRemoteImageSucceeds(t *testing.T) { } func Test_NoRetryRetrieveRemoteImageFails(t *testing.T) { - image := "debian" - opts := config.RegistryOptions{ ImageDownloadRetry: 0, } From defc122118fb36b39b563efe70eed7312f0a4cf4 Mon Sep 17 00:00:00 2001 From: Anna Levenberg Date: Mon, 20 Nov 2023 10:17:21 -0500 Subject: [PATCH 5/5] update param description --- README.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 922679e23c..a26ba6f5bb 100644 --- a/README.md +++ b/README.md @@ -1097,7 +1097,9 @@ image filesystem. Defaults to `0`. #### Flag `--image-download-retry` -Set this flag to the number of retries that should happen when downloading the remote image. Defaults to `0`. +Set this flag to the number of retries that should happen when downloading the +remote image. Consecutive retries occur with exponential backoff and an initial +delay of 1 second. Defaults to 0`. ### Debug Image