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

Handle whiteout files#36

Merged
iaguis merged 1 commit intoappc:masterfrom
endocode:iaguis/handle-whiteouts
Mar 31, 2015
Merged

Handle whiteout files#36
iaguis merged 1 commit intoappc:masterfrom
endocode:iaguis/handle-whiteouts

Conversation

@iaguis
Copy link
Member

@iaguis iaguis commented Mar 26, 2015

We were ignoring Docker's whiteout files so it's time to handle them.

To do that, we keep a white list of files (PathWhitelist). As we go
through the layers, we remove any files maked as whiteouts from the list
so so that, at any time, each layer ACI has the list of its desired
files and the ones of its parent layers.

We change order we process the images to start from the base image so
that we can generate the correct white lists. Then we reverse the image
list because acirenderer expects images starting from the upper layer.

@jonboulle
Copy link
Contributor

@iaguis thanks for tackling this. Is there any particular reason you went with the pathWhitelist approach as opposed to e.g. simply omitting the actual files when constructing the rootfs? I'm a bit ambivalent about this approach since we potentially have to create a giant pathWhiteList just to remove a single file. Thoughts?

@iaguis
Copy link
Member Author

iaguis commented Mar 27, 2015

You need to use pathWhitelist if you're generating the layers separately as ACIs. Now, for a squashed image we could do like you say but by using pathWhitelist I can let acirenderer do the hard work of finding which files should I get from which layer.

What we can do is remove pathWhitelist from the squashed manifest. Once it's squashed, it was already taken into account by acirenderer and it's redundant. Would that be enough?

Just pushed that change.

@iaguis iaguis force-pushed the iaguis/handle-whiteouts branch from a070006 to 3930bbb Compare March 27, 2015 08:58
Copy link
Contributor

Choose a reason for hiding this comment

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

subtract

Copy link
Member Author

Choose a reason for hiding this comment

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

Damn, my eternal mistake in English...

@iaguis iaguis force-pushed the iaguis/handle-whiteouts branch from 3930bbb to 3b6f488 Compare March 30, 2015 07:54
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know the specific legalities of filenames here, but wouldn't 1 be safer than -1? (e.g. .wh.myfile.wh.ich.has.a.silly.name)

Copy link
Member Author

Choose a reason for hiding this comment

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

You're right

@jonboulle
Copy link
Contributor

One question, LGTM aside from that

We were ignoring Docker's whiteout files so it's time to handle them.

To do that, we keep a white list of files (PathWhitelist). As we go
through the layers, we remove any files maked as whiteouts from the list
so so that, at any time, each layer ACI has the list of its desired
files and the ones of its parent layers.

We change order we process the images to start from the base image so
that we can generate the correct white lists. Then we reverse the image
list because acirenderer expects images starting from the upper layer.
@iaguis iaguis force-pushed the iaguis/handle-whiteouts branch from 3b6f488 to d6b818f Compare March 31, 2015 08:13
@iaguis
Copy link
Member Author

iaguis commented Mar 31, 2015

Thanks!

iaguis added a commit that referenced this pull request Mar 31, 2015
@iaguis iaguis merged commit e81bbde into appc:master Mar 31, 2015
@jaybuff
Copy link

jaybuff commented Mar 31, 2015

+1 iaguis, thanks for doing this!

@jonboulle
Copy link
Contributor

@iaguis thank you!

On Tue, Mar 31, 2015 at 5:42 AM, Iago López Galeiras <
[email protected]> wrote:

Thanks!


Reply to this email directly or view it on GitHub
#36 (comment).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants