-
Notifications
You must be signed in to change notification settings - Fork 870
Remove device mapper support #4832
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
Conversation
|
Maybe I need more tea this morning, but this looks like the test should have passed. @edsantiago can you tell me where I'm mis-reading? |
|
@TomSweeneyRedHat I'm not sure what your question is? The test is poorly written, it's ambiguous in its failure message, but basically it expects line 2 to be "Using cache" and it is not. I don't know enough about this test or about layers to know what the problem is. sorry. |
|
So, to simplify the life of whoever debugs this, here is a good/bad diff: Left is good (expected output), right is bad (failure in this CI run). Basically, the failed run does "Trying to pull alpine". Again, I can't explain why, and leave it to smarter people to investigate. |
|
Thanks @edsantiago your answer is what I was looking for. I saw the "Using cache" in the output, and given that, thought the test |
|
Rebased to include #4837 |
Signed-off-by: Kir Kolyshkin <[email protected]>
|
Rebased |
|
/approve |
|
@giuseppe @vrothberg PTAL |
vrothberg
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: kolyshkin, rhatdan, vrothberg The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/lgtm |
|
Now we need to debate whether we bump the major version of Buildah. |
Assuming devicemapper has never been supported, we may win in court :^) |
What type of PR is this?
What this PR does / why we need it:
Remove device mapper support.
It seems that no one is using devicemapper storage driver nowadays, since overlayfs is so much better.
How to verify it
Which issue(s) this PR fixes:
Related to containers/storage#1619
Special notes for your reviewer:
Does this PR introduce a user-facing change?