From e7d9095d8b96888c101b8263356809525d58632e Mon Sep 17 00:00:00 2001 From: Tycho Andersen Date: Thu, 28 Mar 2019 12:21:45 -0600 Subject: [PATCH 1/2] tests: fix typo $BUNDLE is really the path to the top level thing, and we only detect changes to $BUNDLE/rootfs. So when we make changes, let's actually make them to the rootfs. The reason this worked before is that we always insert a layer, even if it's empty. With the next patch, we wouldn't do this any more, causing this test to fail. Signed-off-by: Tycho Andersen --- test/repack.bats | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/repack.bats b/test/repack.bats index bb9b5e7db..79771ff67 100644 --- a/test/repack.bats +++ b/test/repack.bats @@ -231,7 +231,7 @@ function teardown() { bundle-verify "$BUNDLE" # Make some small change. - touch "$BUNDLE/a_small_change" + touch "$ROOTFS/a_small_change" now="$(date --iso-8601=seconds --utc)" # Repack the image, setting history values. From b1df4185f236da08f889060b7441ef4a0dd7dcb6 Mon Sep 17 00:00:00 2001 From: Tycho Andersen Date: Thu, 28 Mar 2019 11:17:30 -0600 Subject: [PATCH 2/2] repack: don't insert a tar gzip layer if there is no diff Let's not insert an empty gzip layer if there is no diff. It might make sense to not even insert a history entry, but it also seems reasonable to want to log that umoci ran at some point, so we do that, but just leave everything unchanged. Signed-off-by: Tycho Andersen --- repack.go | 40 +++++++++++++++++++++++++++++++--------- test/repack.bats | 21 +++++++++++++++++++++ 2 files changed, 52 insertions(+), 9 deletions(-) diff --git a/repack.go b/repack.go index 532542616..78c78bbb6 100644 --- a/repack.go +++ b/repack.go @@ -65,16 +65,38 @@ func Repack(engineExt casext.Engine, tagName string, bundlePath string, meta Met mtreefilter.MaskFilter(maskedPaths), mtreefilter.SimplifyFilter(diffs)) - reader, err := layer.GenerateLayer(fullRootfsPath, diffs, &meta.MapOptions) - if err != nil { - return errors.Wrap(err, "generate diff layer") - } - defer reader.Close() + if len(diffs) == 0 { + config, err := mutator.Config(context.Background()) + if err != nil { + return err + } - // TODO: We should add a flag to allow for a new layer to be made - // non-distributable. - if err := mutator.Add(context.Background(), reader, history); err != nil { - return errors.Wrap(err, "add diff layer") + imageMeta, err := mutator.Meta(context.Background()) + if err != nil { + return err + } + + annotations, err := mutator.Annotations(context.Background()) + if err != nil { + return err + } + + err = mutator.Set(context.Background(), config, imageMeta, annotations, history) + if err != nil { + return err + } + } else { + reader, err := layer.GenerateLayer(fullRootfsPath, diffs, &meta.MapOptions) + if err != nil { + return errors.Wrap(err, "generate diff layer") + } + defer reader.Close() + + // TODO: We should add a flag to allow for a new layer to be made + // non-distributable. + if err := mutator.Add(context.Background(), reader, history); err != nil { + return errors.Wrap(err, "add diff layer") + } } newDescriptorPath, err := mutator.Commit(context.Background()) diff --git a/test/repack.bats b/test/repack.bats index 79771ff67..cfcaf5794 100644 --- a/test/repack.bats +++ b/test/repack.bats @@ -790,3 +790,24 @@ function teardown() { [ "$numLinesB" -gt "$numLinesA" ] [ "$numLinesC" -gt "$numLinesB" ] } + +@test "umoci repack (empty diff)" { + # Unpack the original image + new_bundle_rootfs + umoci unpack --image "${IMAGE}:${TAG}" "$BUNDLE" + [ "$status" -eq 0 ] + bundle-verify "$BUNDLE" + + # Repack the image under a new tag. + umoci repack --image "${IMAGE}:${TAG}-new" "$BUNDLE" + [ "$status" -eq 0 ] + image-verify "${IMAGE}" + + # The two manifests should have the same number of layers. + manifest0=$(cat "${IMAGE}/oci/index.json" | jq -r .manifests[0].digest | cut -f2 -d:) + manifest1=$(cat "${IMAGE}/oci/index.json" | jq -r .manifests[1].digest | cut -f2 -d:) + + layers0=$(cat "${IMAGE}/oci/blobs/sha256/$manifest0" | jq -r .layers) + layers1=$(cat "${IMAGE}/oci/blobs/sha256/$manifest1" | jq -r .layers) + [ "$layers0" == "$layers1" ] +}