From ec0b13295abb718dd37578f0c103e6df9197673e Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Thu, 30 May 2024 13:57:19 +0000 Subject: [PATCH 1/2] Revert "Add extraction progress output to GetFSFromImage/GetFSFromLayers (#11)" This reverts commit 9d0d55902c34880e5a56719f00a32bcaf6f8701e. --- pkg/util/fs_util.go | 80 +++------------------------------------- pkg/util/fs_util_test.go | 8 ---- 2 files changed, 5 insertions(+), 83 deletions(-) diff --git a/pkg/util/fs_util.go b/pkg/util/fs_util.go index e968d62e8e..c8d5a613aa 100644 --- a/pkg/util/fs_util.go +++ b/pkg/util/fs_util.go @@ -96,9 +96,8 @@ type FileContext struct { type ExtractFunction func(string, *tar.Header, string, io.Reader) error type FSConfig struct { - includeWhiteout bool - printExtractionProgress bool - extractFunc ExtractFunction + includeWhiteout bool + extractFunc ExtractFunction } type FSOpt func(*FSConfig) @@ -127,12 +126,6 @@ func IncludeWhiteout() FSOpt { } } -func PrintExtractionProgress() FSOpt { - return func(opts *FSConfig) { - opts.printExtractionProgress = true - } -} - func ExtractFunc(extractFunc ExtractFunction) FSOpt { return func(opts *FSConfig) { opts.extractFunc = extractFunc @@ -151,7 +144,7 @@ func GetFSFromImage(root string, img v1.Image, extract ExtractFunction) ([]strin return nil, err } - return GetFSFromLayers(root, layers, ExtractFunc(extract), PrintExtractionProgress()) + return GetFSFromLayers(root, layers, ExtractFunc(extract)) } func GetFSFromLayers(root string, layers []v1.Layer, opts ...FSOpt) ([]string, error) { @@ -170,37 +163,12 @@ func GetFSFromLayers(root string, layers []v1.Layer, opts ...FSOpt) ([]string, e return nil, errors.New("must supply an extract function") } - var totalSize int64 - layerSizes := make([]int64, 0, len(layers)) - for i, l := range layers { - layerSize, err := l.Size() - if err != nil { - return nil, errors.Wrap(err, fmt.Sprintf("error checking layer size %d", i)) - } - layerSizes = append(layerSizes, layerSize) - totalSize += layerSize - } - printExtractionProgress := cfg.printExtractionProgress - if totalSize == 0 { - printExtractionProgress = false - } - - if printExtractionProgress { - logrus.Infof("Extracting image layers to %s", root) - } - extractedFiles := []string{} - var extractedBytes int64 for i, l := range layers { if mediaType, err := l.MediaType(); err == nil { - logrus.Tracef("Extracting layer %d/%d of media type %s", i+1, len(layers), mediaType) + logrus.Tracef("Extracting layer %d of media type %s", i, mediaType) } else { - logrus.Tracef("Extracting layer %d/%d", i+1, len(layers)) - } - - progressPerc := float64(extractedBytes) / float64(totalSize) * 100 - if printExtractionProgress { - logrus.Infof("Extracting layer %d/%d (%.1f%%)", i+1, len(layers), progressPerc) + logrus.Tracef("Extracting layer %d", i) } r, err := l.Uncompressed() @@ -209,16 +177,6 @@ func GetFSFromLayers(root string, layers []v1.Layer, opts ...FSOpt) ([]string, e } defer r.Close() - if printExtractionProgress { - r = &printAfterReader{ - ReadCloser: r, - after: time.Second, - print: func(n int) { - logrus.Infof("Extracting layer %d/%d (%.1f%%) %s", i+1, len(layers), progressPerc, strings.Repeat(".", n)) - }, - } - } - tr := tar.NewReader(r) for { hdr, err := tr.Next() @@ -267,38 +225,10 @@ func GetFSFromLayers(root string, layers []v1.Layer, opts ...FSOpt) ([]string, e extractedFiles = append(extractedFiles, filepath.Join(root, cleanedName)) } - - extractedBytes += layerSizes[i] } - - if printExtractionProgress { - logrus.Infof("Extraction complete") - } - return extractedFiles, nil } -type printAfterReader struct { - io.ReadCloser - t time.Time - after time.Duration - count int - print func(int) -} - -func (r *printAfterReader) Read(p []byte) (n int, err error) { - n, err = r.ReadCloser.Read(p) - if r.t.IsZero() { - r.t = time.Now() - } - if time.Since(r.t) >= r.after { - r.count++ - r.print(r.count) - r.t = time.Now() - } - return -} - // DeleteFilesystem deletes the extracted image file system func DeleteFilesystem() error { logrus.Info("Deleting filesystem...") diff --git a/pkg/util/fs_util_test.go b/pkg/util/fs_util_test.go index c21c9bfa0f..9bd44c835c 100644 --- a/pkg/util/fs_util_test.go +++ b/pkg/util/fs_util_test.go @@ -1067,7 +1067,6 @@ func Test_GetFSFromLayers_with_whiteouts_include_whiteout_enabled(t *testing.T) f(expectedFiles, tw) mockLayer := mockv1.NewMockLayer(ctrl) - mockLayer.EXPECT().Size().Return(int64(buf.Len()), nil) mockLayer.EXPECT().MediaType().Return(types.OCILayer, nil) rc := io.NopCloser(buf) @@ -1083,7 +1082,6 @@ func Test_GetFSFromLayers_with_whiteouts_include_whiteout_enabled(t *testing.T) f(secondLayerFiles, tw) mockLayer2 := mockv1.NewMockLayer(ctrl) - mockLayer2.EXPECT().Size().Return(int64(buf.Len()), nil) mockLayer2.EXPECT().MediaType().Return(types.OCILayer, nil) rc = io.NopCloser(buf) @@ -1177,7 +1175,6 @@ func Test_GetFSFromLayers_with_whiteouts_include_whiteout_disabled(t *testing.T) f(expectedFiles, tw) mockLayer := mockv1.NewMockLayer(ctrl) - mockLayer.EXPECT().Size().Return(int64(buf.Len()), nil) mockLayer.EXPECT().MediaType().Return(types.OCILayer, nil) layerFiles := []string{ filepath.Join(root, "foobar"), @@ -1200,7 +1197,6 @@ func Test_GetFSFromLayers_with_whiteouts_include_whiteout_disabled(t *testing.T) f(secondLayerFiles, tw) mockLayer2 := mockv1.NewMockLayer(ctrl) - mockLayer2.EXPECT().Size().Return(int64(buf.Len()), nil) mockLayer2.EXPECT().MediaType().Return(types.OCILayer, nil) rc = io.NopCloser(buf) @@ -1284,7 +1280,6 @@ func Test_GetFSFromLayers_ignorelist(t *testing.T) { f(expectedFiles, tw) mockLayer := mockv1.NewMockLayer(ctrl) - mockLayer.EXPECT().Size().Return(int64(buf.Len()), nil) mockLayer.EXPECT().MediaType().Return(types.OCILayer, nil) layerFiles := []string{ filepath.Join(root, ".wh.testdir"), @@ -1350,8 +1345,6 @@ func Test_GetFSFromLayers_ignorelist(t *testing.T) { f(layerFiles, tw) - mockLayer.EXPECT().Size().Return(int64(buf.Len()), nil) - rc = io.NopCloser(buf) mockLayer.EXPECT().Uncompressed().Return(rc, nil) @@ -1417,7 +1410,6 @@ func Test_GetFSFromLayers(t *testing.T) { } mockLayer := mockv1.NewMockLayer(ctrl) - mockLayer.EXPECT().Size().Return(int64(buf.Len()), nil) mockLayer.EXPECT().MediaType().Return(types.OCILayer, nil) rc := io.NopCloser(buf) From eeb1bb480d8feefdb7329469d9cbcfcc976cf8dc Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 24 May 2024 12:41:28 +0000 Subject: [PATCH 2/2] Add support for showing image FS extraction progress --- README.md | 5 +++ cmd/executor/cmd/root.go | 1 + pkg/config/options.go | 1 + pkg/executor/build.go | 8 ++-- pkg/executor/build_test.go | 4 +- pkg/util/fs_util.go | 92 ++++++++++++++++++++++++++++++++++---- pkg/util/fs_util_test.go | 8 ++++ 7 files changed, 105 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index 7f21604698..a08bd1c456 100644 --- a/README.md +++ b/README.md @@ -1143,6 +1143,11 @@ Ignore /var/run when taking image snapshot. Set it to false to preserve Set this flag as `--ignore-path=` to ignore path when taking an image snapshot. Set it multiple times for multiple ignore paths. +#### Flag `--image-fs-extract-progres` + +Set this flag to show the progress of extracting an image filesystem. Defaults +to `false`. + #### Flag `--image-fs-extract-retry` Set this flag to the number of retries that should happen for the extracting an diff --git a/cmd/executor/cmd/root.go b/cmd/executor/cmd/root.go index 7f0339c5df..89743d2ea0 100644 --- a/cmd/executor/cmd/root.go +++ b/cmd/executor/cmd/root.go @@ -239,6 +239,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().BoolVar(&opts.PushIgnoreImmutableTagErrors, "push-ignore-immutable-tag-errors", false, "If true, known tag immutability errors are ignored and the push finishes with success.") + RootCmd.PersistentFlags().BoolVar(&opts.ImageFSExtractProgress, "image-fs-extract-progress", false, "Show progress for image FS extraction") 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.") diff --git a/pkg/config/options.go b/pkg/config/options.go index 6f3b67a197..9afe39457e 100644 --- a/pkg/config/options.go +++ b/pkg/config/options.go @@ -77,6 +77,7 @@ type KanikoOptions struct { OCILayoutPath string Compression Compression CompressionLevel int + ImageFSExtractProgress bool ImageFSExtractRetry int SingleSnapshot bool Reproducible bool diff --git a/pkg/executor/build.go b/pkg/executor/build.go index d163af6831..dabd5b7f42 100644 --- a/pkg/executor/build.go +++ b/pkg/executor/build.go @@ -332,7 +332,7 @@ func (s *stageBuilder) build() error { t := timing.Start("FS Unpacking") retryFunc := func() error { - _, err := getFSFromImage(config.RootDir, s.image, util.ExtractFile) + _, err := getFSFromImage(config.RootDir, s.image, util.ExtractFile, s.opts.ImageFSExtractProgress) return err } @@ -939,7 +939,7 @@ func fetchExtraStages(stages []config.KanikoStage, opts *config.KanikoOptions) e if err := saveStageAsTarball(c.From, sourceImage); err != nil { return err } - if err := extractImageToDependencyDir(c.From, sourceImage); err != nil { + if err := extractImageToDependencyDir(c.From, sourceImage, opts.ImageFSExtractProgress); err != nil { return err } } @@ -960,7 +960,7 @@ func fromPreviousStage(copyCommand *instructions.CopyCommand, previousStageNames return false } -func extractImageToDependencyDir(name string, image v1.Image) error { +func extractImageToDependencyDir(name string, image v1.Image, extractionProgress bool) error { t := timing.Start("Extracting Image to Dependency Dir") defer timing.DefaultRun.Stop(t) dependencyDir := filepath.Join(config.KanikoDir, name) @@ -968,7 +968,7 @@ func extractImageToDependencyDir(name string, image v1.Image) error { return err } logrus.Debugf("Trying to extract to %s", dependencyDir) - _, err := util.GetFSFromImage(dependencyDir, image, util.ExtractFile) + _, err := util.GetFSFromImage(dependencyDir, image, util.ExtractFile, extractionProgress) return err } diff --git a/pkg/executor/build_test.go b/pkg/executor/build_test.go index 4191b13f5d..7586a2c06a 100644 --- a/pkg/executor/build_test.go +++ b/pkg/executor/build_test.go @@ -923,7 +923,7 @@ func Test_stageBuilder_build(t *testing.T) { config *v1.ConfigFile stage config.KanikoStage crossStageDeps map[int][]string - mockGetFSFromImage func(root string, img v1.Image, extract util.ExtractFunction) ([]string, error) + mockGetFSFromImage func(root string, img v1.Image, extract util.ExtractFunction, extractionProgress bool) ([]string, error) shouldInitSnapshot bool } @@ -1441,7 +1441,7 @@ RUN foobar opts: &config.KanikoOptions{InitialFSUnpacked: true}, stage: config.KanikoStage{Index: 0}, crossStageDeps: map[int][]string{0: {"some-dep"}}, - mockGetFSFromImage: func(root string, img v1.Image, extract util.ExtractFunction) ([]string, error) { + mockGetFSFromImage: func(root string, img v1.Image, extract util.ExtractFunction, extractionPogress bool) ([]string, error) { return nil, fmt.Errorf("getFSFromImage shouldn't be called if fs is already unpacked") }, }, diff --git a/pkg/util/fs_util.go b/pkg/util/fs_util.go index c8d5a613aa..968783822c 100644 --- a/pkg/util/fs_util.go +++ b/pkg/util/fs_util.go @@ -96,8 +96,9 @@ type FileContext struct { type ExtractFunction func(string, *tar.Header, string, io.Reader) error type FSConfig struct { - includeWhiteout bool - extractFunc ExtractFunction + includeWhiteout bool + extractFunc ExtractFunction + extractionProgress bool } type FSOpt func(*FSConfig) @@ -132,9 +133,15 @@ func ExtractFunc(extractFunc ExtractFunction) FSOpt { } } +func ExtractionProgress() FSOpt { + return func(opts *FSConfig) { + opts.extractionProgress = true + } +} + // GetFSFromImage extracts the layers of img to root // It returns a list of all files extracted -func GetFSFromImage(root string, img v1.Image, extract ExtractFunction) ([]string, error) { +func GetFSFromImage(root string, img v1.Image, extract ExtractFunction, extractionProgress bool) ([]string, error) { if img == nil { return nil, errors.New("image cannot be nil") } @@ -144,7 +151,11 @@ func GetFSFromImage(root string, img v1.Image, extract ExtractFunction) ([]strin return nil, err } - return GetFSFromLayers(root, layers, ExtractFunc(extract)) + opts := []FSOpt{ExtractFunc(extract)} + if extractionProgress { + opts = append(opts, ExtractionProgress()) + } + return GetFSFromLayers(root, layers, opts...) } func GetFSFromLayers(root string, layers []v1.Layer, opts ...FSOpt) ([]string, error) { @@ -163,19 +174,48 @@ func GetFSFromLayers(root string, layers []v1.Layer, opts ...FSOpt) ([]string, e return nil, errors.New("must supply an extract function") } + var totalSize int64 + layerSizes := make([]int64, 0, len(layers)) + for i, l := range layers { + layerSize, err := l.Size() + if err != nil { + return nil, errors.Wrap(err, fmt.Sprintf("error checking layer size %d", i)) + } + layerSizes = append(layerSizes, layerSize) + totalSize += layerSize + } + + showProgress := totalSize > 0 && cfg.extractionProgress + if showProgress { + logrus.Infof("Extracting image layers to %s", root) + } + extractedFiles := []string{} + var extractedBytes int64 for i, l := range layers { if mediaType, err := l.MediaType(); err == nil { - logrus.Tracef("Extracting layer %d of media type %s", i, mediaType) + logrus.Tracef("Extracting layer %d (%d/%d) of media type %s", i, i+1, len(layers), mediaType) } else { - logrus.Tracef("Extracting layer %d", i) + logrus.Tracef("Extracting layer %d (%d/%d)", i, i+1, len(layers)) } - r, err := l.Uncompressed() + progressPerc := float64(extractedBytes) / float64(totalSize) * 100 + if showProgress { + logrus.Infof("Extracting layer %d/%d (%.1f%%)", i+1, len(layers), progressPerc) + } + + cr, err := l.Uncompressed() if err != nil { return nil, err } - defer r.Close() + defer cr.Close() + + var r io.Reader = cr + if showProgress { + r = newDoAfterReader(r, func(count int) { + logrus.Infof("Extracting layer %d/%d (%.1f%%) %s", i+1, len(layers), progressPerc, strings.Repeat(".", count)) + }, time.Second) + } tr := tar.NewReader(r) for { @@ -225,10 +265,46 @@ func GetFSFromLayers(root string, layers []v1.Layer, opts ...FSOpt) ([]string, e extractedFiles = append(extractedFiles, filepath.Join(root, cleanedName)) } + + extractedBytes += layerSizes[i] + } + + if showProgress { + logrus.Infof("Extraction complete") } + return extractedFiles, nil } +type doAfterReader struct { + r io.Reader + t time.Time + after time.Duration + count int + do func(int) +} + +func newDoAfterReader(r io.Reader, do func(int), after time.Duration) *doAfterReader { + return &doAfterReader{ + r: r, + do: do, + after: after, + } +} + +func (r *doAfterReader) Read(p []byte) (n int, err error) { + n, err = r.r.Read(p) + if r.t.IsZero() { + r.t = time.Now() + } + if time.Since(r.t) >= r.after { + r.count++ + r.do(r.count) + r.t = time.Now() + } + return n, err +} + // DeleteFilesystem deletes the extracted image file system func DeleteFilesystem() error { logrus.Info("Deleting filesystem...") diff --git a/pkg/util/fs_util_test.go b/pkg/util/fs_util_test.go index 9bd44c835c..c21c9bfa0f 100644 --- a/pkg/util/fs_util_test.go +++ b/pkg/util/fs_util_test.go @@ -1067,6 +1067,7 @@ func Test_GetFSFromLayers_with_whiteouts_include_whiteout_enabled(t *testing.T) f(expectedFiles, tw) mockLayer := mockv1.NewMockLayer(ctrl) + mockLayer.EXPECT().Size().Return(int64(buf.Len()), nil) mockLayer.EXPECT().MediaType().Return(types.OCILayer, nil) rc := io.NopCloser(buf) @@ -1082,6 +1083,7 @@ func Test_GetFSFromLayers_with_whiteouts_include_whiteout_enabled(t *testing.T) f(secondLayerFiles, tw) mockLayer2 := mockv1.NewMockLayer(ctrl) + mockLayer2.EXPECT().Size().Return(int64(buf.Len()), nil) mockLayer2.EXPECT().MediaType().Return(types.OCILayer, nil) rc = io.NopCloser(buf) @@ -1175,6 +1177,7 @@ func Test_GetFSFromLayers_with_whiteouts_include_whiteout_disabled(t *testing.T) f(expectedFiles, tw) mockLayer := mockv1.NewMockLayer(ctrl) + mockLayer.EXPECT().Size().Return(int64(buf.Len()), nil) mockLayer.EXPECT().MediaType().Return(types.OCILayer, nil) layerFiles := []string{ filepath.Join(root, "foobar"), @@ -1197,6 +1200,7 @@ func Test_GetFSFromLayers_with_whiteouts_include_whiteout_disabled(t *testing.T) f(secondLayerFiles, tw) mockLayer2 := mockv1.NewMockLayer(ctrl) + mockLayer2.EXPECT().Size().Return(int64(buf.Len()), nil) mockLayer2.EXPECT().MediaType().Return(types.OCILayer, nil) rc = io.NopCloser(buf) @@ -1280,6 +1284,7 @@ func Test_GetFSFromLayers_ignorelist(t *testing.T) { f(expectedFiles, tw) mockLayer := mockv1.NewMockLayer(ctrl) + mockLayer.EXPECT().Size().Return(int64(buf.Len()), nil) mockLayer.EXPECT().MediaType().Return(types.OCILayer, nil) layerFiles := []string{ filepath.Join(root, ".wh.testdir"), @@ -1345,6 +1350,8 @@ func Test_GetFSFromLayers_ignorelist(t *testing.T) { f(layerFiles, tw) + mockLayer.EXPECT().Size().Return(int64(buf.Len()), nil) + rc = io.NopCloser(buf) mockLayer.EXPECT().Uncompressed().Return(rc, nil) @@ -1410,6 +1417,7 @@ func Test_GetFSFromLayers(t *testing.T) { } mockLayer := mockv1.NewMockLayer(ctrl) + mockLayer.EXPECT().Size().Return(int64(buf.Len()), nil) mockLayer.EXPECT().MediaType().Return(types.OCILayer, nil) rc := io.NopCloser(buf)