Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

diffApply: set dir opaque when overwriting whiteout #2614

Merged
merged 1 commit into from
Feb 10, 2022

Conversation

sipsma
Copy link
Collaborator

@sipsma sipsma commented Feb 10, 2022

Before this, there was a bug triggered under the following conditions:

  1. An overlay snapshotter was being used, which caused the optimization
    of preparing a new snapshot off of the base layers to be triggered
  2. The base layers contained a directory that had contents
  3. One subsequent layer deleted that directory w/out recreating it
  4. A later layer recreated the directory

In this case, what happened was a whiteout device would be created as
part of 3 above but then in step 4 the whiteout device would be removed
and replaced with a plain directory. The problem is that such a
directory doesn't block out the files from step 2 and it doesn't know
about them because they are in a lowerdir (not the upperdir being
applied to).

The simplest fix, which this commit implements, is to just set the
directory created in step 4 as opaque, which enables the correct
behavior of blocking out files below it.

This was missed in test coverage before because tests for opaque
handling always combined 3+4 into one layer, whereas the bug requires
they be separate layers. A new integration test has been added to cover
this case.

Signed-off-by: Erik Sipsma [email protected]

Before this, there was a bug triggered under the following conditions:
1. An overlay snapshotter was being used, which caused the optimization
   of preparing a new snapshot off of the base layers to be triggered
2. The base layers contained a directory that had contents
3. One subsequent layer deleted that directory w/out recreating it
4. A later layer recreated the directory

In this case, what happened was a whiteout device would be created as
part of 3 above but then in step 4 the whiteout device would be removed
and replaced with a plain directory. The problem is that such a
directory doesn't block out the files from step 2 and it doesn't know
about them because they are in a lowerdir (not the upperdir being
applied to).

The simplest fix, which this commit implements, is to just set the
directory created in step 4 as opaque, which enables the correct
behavior of blocking out files below it.

This was missed in test coverage before because tests for opaque
handling always combined 3+4 into one layer, whereas the bug requires
they be separate layers. A new integration test has been added to cover
this case.

Signed-off-by: Erik Sipsma <[email protected]>
@sipsma sipsma requested a review from tonistiigi February 10, 2022 04:26
@sipsma sipsma added this to the v0.10.0 milestone Feb 10, 2022
@tonistiigi tonistiigi requested a review from ktock February 10, 2022 05:22
Copy link
Collaborator

@ktock ktock left a comment

Choose a reason for hiding this comment

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

LGTM

@sipsma sipsma merged commit a6fc85f into moby:master Feb 10, 2022
@sipsma sipsma deleted the fix-opaque-diffapply branch February 10, 2022 17:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants