Skip to content
This repository has been archived by the owner on Aug 14, 2020. It is now read-only.

handle hard links and whiteouts correctly (v3) #105

Merged
merged 2 commits into from Nov 23, 2015

Conversation

alban
Copy link
Member

@alban alban commented Nov 23, 2015

Replace #100 to trigger the tests in Semaphore.
Compared to the previous version:

  • no changes with --nosquash: no .hidden files anymore
  • include tests

Hard links and whiteout were not handled correctly in this scenario:

  • The first Docker layer contains:
    • file A
    • file B hard link to file A
  • The second Docker layer contains:
    • whiteout file A

Then, the squashed ACI previously generated by docker2aci contained:

  • file B dangling hard link to file A

Hence the bug.

This patch changes the squashing algorithm lib/docker2aci.go:SquashLayers()
to have two passes:

  • Pass one: build an in-memory map of hard links and whiteouts
  • Pass two: write the archive using the map generated in pass one

I tested the following images from rkt/rkt#1653:

  • docker://albanc/busybox-hardlinks
  • docker://zopyx/xmldirector-plone

Fixes #98

Hard links and whiteout were not handled correctly in this scenario:
- The first Docker layer contains:
  - file A
  - file B hard link to file A
- The second Docker layer contains:
  - whiteout file A

Then, the squashed ACI previously generated by docker2aci contained:
  - file B dangling hard link to file A

Hence the bug.

This patch changes the squashing algorithm lib/docker2aci.go:SquashLayers()
to have two passes:
  - Pass one: build an in-memory map of hard links and whiteouts
  - Pass two: write the archive using the map generated in pass one

I tested the following images from rkt/rkt#1653:
- docker://albanc/busybox-hardlinks
- docker://zopyx/xmldirector-plone

Fixes appc#98
sudo $RKT run --insecure-skip-verify ./${PREFIX}-${TESTNAME}-latest.aci

echo "### Test case ${TESTNAME}: test in rkt..."
sudo $RKT run --insecure-skip-verify --set-env=CHECK=rkt-run --set-env=DOCKER_STORAGE_BACKEND=$DOCKER_STORAGE_BACKEND ./${PREFIX}-${TESTNAME}-latest.aci
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should remove the pod we create (maybe with rkt prepare & rkt run-prepared & rkt rm $UUID) and the image from the CAS.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

@alban alban force-pushed the alban/hard-link-whiteout-v3 branch from b665940 to b752965 Compare November 23, 2015 10:42

# # Docker with AUFS or overlay storage backend does not handle this test
# # correctly and Semaphore uses AUFS
if [ "$DOCKER_STORAGE_BACKEND" == devicemapper ] ; then
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fixed this test. This test works fine with Docker/devicemapper but not with Docker/aufs or Docker/overlayfs.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

> rkt-uuid-${TESTNAME}
sudo $RKT run-prepared $(cat rkt-uuid-${TESTNAME})
sudo $RKT status $(cat rkt-uuid-${TESTNAME}) | grep app-${TESTNAME}=0
sudo $RKT rm $(cat rkt-uuid-${TESTNAME})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, we still leave the images behind, that's annoying especially since every run they have a different hash. We should also remove them.

It would also be nice to run these clean commands if any part of the script exits (for example, if the exit status is not 0) but we can do that later.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For deleting images, I filed rkt/rkt#1779. Can we do it after rkt/rkt#1779 gets fixed?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

@iaguis
Copy link
Member

iaguis commented Nov 23, 2015

LGTM

alban added a commit that referenced this pull request Nov 23, 2015
handle hard links and whiteouts correctly (v3)
@alban alban merged commit fce83d1 into appc:master Nov 23, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants