From 23ac909b2d2765791f8f201c9c8ca6f5900662d0 Mon Sep 17 00:00:00 2001 From: Tejal Desai Date: Wed, 6 May 2020 12:08:44 -0700 Subject: [PATCH 1/2] Revert "small perf optimizing. Only remove whiteout path if it needs to be included in the tar" This reverts commit 67db51810bca1b10281c4ada0e7340805bd4abd6. --- pkg/util/fs_util.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/pkg/util/fs_util.go b/pkg/util/fs_util.go index 230d87669..af553f8c9 100644 --- a/pkg/util/fs_util.go +++ b/pkg/util/fs_util.go @@ -161,15 +161,18 @@ func GetFSFromLayers(root string, layers []v1.Layer, opts ...FSOpt) ([]string, e dir := filepath.Dir(path) if strings.HasPrefix(base, ".wh.") { - if !cfg.includeWhiteout { - logrus.Debug("not including whiteout files") - continue - } logrus.Debugf("Whiting out %s", path) + name := strings.TrimPrefix(base, ".wh.") if err := os.RemoveAll(filepath.Join(dir, name)); err != nil { return nil, errors.Wrapf(err, "removing whiteout %s", hdr.Name) } + + if !cfg.includeWhiteout { + logrus.Debug("not including whiteout files") + continue + } + } if err := cfg.extractFunc(root, hdr, tr); err != nil { From d9a6bbe9561dca73505217bf6833179e9f390363 Mon Sep 17 00:00:00 2001 From: Tejal Desai Date: Wed, 6 May 2020 12:11:03 -0700 Subject: [PATCH 2/2] add tests --- pkg/util/fs_util_test.go | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/pkg/util/fs_util_test.go b/pkg/util/fs_util_test.go index 5ba37fef9..2e0dcc8c5 100644 --- a/pkg/util/fs_util_test.go +++ b/pkg/util/fs_util_test.go @@ -1024,6 +1024,12 @@ func Test_GetFSFromLayers_with_whiteouts_include_whiteout_enabled(t *testing.T) if err != nil { t.Fatal(err) } + // Write a whiteout path + d1 := []byte("Hello World\n") + if err := ioutil.WriteFile(filepath.Join(root, "foobar"), d1, 0644); err != nil { + t.Fatal(err) + } + defer os.Remove(root) opts := []FSOpt{ @@ -1105,6 +1111,11 @@ func Test_GetFSFromLayers_with_whiteouts_include_whiteout_enabled(t *testing.T) err, expectErr, ) + // Make sure whiteout files are removed form the root. + _, err = os.Lstat(filepath.Join(root, "foobar")) + if err == nil || !os.IsNotExist(err) { + t.Errorf("expected whiteout foobar file to be deleted. However found it.") + } } func Test_GetFSFromLayers_with_whiteouts_include_whiteout_disabled(t *testing.T) { @@ -1114,6 +1125,11 @@ func Test_GetFSFromLayers_with_whiteouts_include_whiteout_disabled(t *testing.T) if err != nil { t.Fatal(err) } + // Write a whiteout path + d1 := []byte("Hello World\n") + if err := ioutil.WriteFile(filepath.Join(root, "foobar"), d1, 0644); err != nil { + t.Fatal(err) + } defer os.Remove(root) opts := []FSOpt{ @@ -1159,6 +1175,13 @@ func Test_GetFSFromLayers_with_whiteouts_include_whiteout_disabled(t *testing.T) mockLayer := mockv1.NewMockLayer(ctrl) mockLayer.EXPECT().MediaType().Return(types.OCILayer, nil) + layerFiles := []string{ + filepath.Join(root, "foobar"), + } + buf = new(bytes.Buffer) + tw = tar.NewWriter(buf) + + f(layerFiles, tw) rc := ioutil.NopCloser(buf) mockLayer.EXPECT().Uncompressed().Return(rc, nil) @@ -1192,6 +1215,11 @@ func Test_GetFSFromLayers_with_whiteouts_include_whiteout_disabled(t *testing.T) err, expectErr, ) + // Make sure whiteout files are removed form the root. + _, err = os.Lstat(filepath.Join(root, "foobar")) + if err == nil || !os.IsNotExist(err) { + t.Errorf("expected whiteout foobar file to be deleted. However found it.") + } } func Test_GetFSFromLayers(t *testing.T) {