Skip to content
This repository has been archived by the owner on Feb 2, 2022. It is now read-only.

Prevent nesting of volumes with overlay driver from exceeding kernel limit #38

Merged
merged 5 commits into from
Aug 6, 2020

Conversation

vito
Copy link
Member

@vito vito commented Jul 27, 2020

No description provided.

vito and others added 3 commits July 27, 2020 16:09
this just makes it easier to run with `fly execute`; it'll be overridden
with the proper image in the pipeline

Signed-off-by: Alex Suraci <asuraci@vmware.com>
Co-authored-by: Vikram Yadav <vyadav@vmware.com>
Signed-off-by: Alex Suraci <asuraci@vmware.com>
Co-authored-by: Vikram Yadav <vyadav@vmware.com>
when creating a copy of a copy, instead of nesting, copy the parent's
upper dir into the child's upper dir and then set the child's parent to
the grandparent.

this keeps the depth from growing unbounded, which is important
considering the kernel has a hardcoded depth limit of 2. the trade-off
is that we'll copy the upper directory contents in this situation, but
hopefully the upper directory will usually be small enough for this to
not matter much.

(in any case, there doesn't seem to be a better choice. sadly.)

concourse/concourse#5799

Signed-off-by: Alex Suraci <asuraci@vmware.com>
Co-authored-by: Vikram Yadav <vyadav@vmware.com>
@vito vito force-pushed the fix-nested-volumes branch from 62fa252 to 9aeb665 Compare July 27, 2020 21:06
@vito vito marked this pull request as draft July 28, 2020 18:49
vito and others added 2 commits July 28, 2020 15:57
use filesystem types, since the Overlay driver cares a lot more about
the handle and parent relationship

subsequent commit will redo the mount recovery so that we don't need the
layerDirOld/workDirOld methods

Signed-off-by: Alex Suraci <asuraci@vmware.com>
Co-authored-by: Vikram Yadav <vyadav@vmware.com>
now that we can trust that volumes only ever have a parent (and not a
grandparent), recovery can be done in just two passes.

adds a Recover(Filesystem) method to the Driver interface.

Signed-off-by: Alex Suraci <asuraci@vmware.com>
Co-authored-by: Vikram Yadav <vyadav@vmware.com>
@vito vito force-pushed the fix-nested-volumes branch from 9aeb665 to 3e4b940 Compare July 28, 2020 20:00
@vito vito marked this pull request as ready for review July 28, 2020 20:49
@xtreme-sameer-vohra
Copy link
Contributor

Based on the changes to Overlay recover, the workers would need to be recreated to ensure the recover method works

@xtreme-sameer-vohra
Copy link
Contributor

Baggage claim unit test are failing
Screen Shot 2020-08-04 at 7 53 34 PM

@vito
Copy link
Member Author

vito commented Aug 5, 2020

@xtreme-sameer-vohra They seem to be passing in CI? 🤔 That's the error you should see without the fix. How are you running the tests?

@xtreme-sameer-vohra
Copy link
Contributor

xtreme-sameer-vohra commented Aug 5, 2020

@xtreme-sameer-vohra They seem to be passing in CI? 🤔 That's the error you should see without the fix. How are you running the tests?

hmm interesting, I ran it on ci using

~/workspace/baggageclaim on fix-nested-volumes [?]
$ fly -t ci execute -p -c ci/unit-linux.yml -j baggageclaim/baggageclaim -i baggageclaim=.

and the error is from creating the first child volume. It looks like it didn't get far enough to try to created the doubly nested one.

Copy link
Contributor

@xtreme-sameer-vohra xtreme-sameer-vohra left a comment

Choose a reason for hiding this comment

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

lgtm !

@vito
Copy link
Member Author

vito commented Aug 6, 2020

@xtreme-sameer-vohra 👻 spooky 👻

I re-ran the build a few times to be sure, and they passed. Would be nice to know why that happened, but I guess I'll merge anyway. 🤷‍♂️

@vito vito merged commit ea9252e into master Aug 6, 2020
@vito vito changed the title fix nested volumes Prevent nesting of volumes with overlay driver from exceeding kernel limit Aug 6, 2020
@vito vito added the bug label Aug 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants