Skip to content

devmapper: log pool status when mkfs fails#5283

Merged
estesp merged 1 commit intocontainerd:masterfrom
alakesh:check-out-of-space
Apr 13, 2021
Merged

devmapper: log pool status when mkfs fails#5283
estesp merged 1 commit intocontainerd:masterfrom
alakesh:check-out-of-space

Conversation

@alakesh
Copy link
Copy Markdown
Contributor

@alakesh alakesh commented Mar 29, 2021 •
edited
Loading

if mkfs on device mapper thin pool fails, it will show pool status
as returned by dmsetup for enahnced error reporting.

Signed-off-by: Alakesh Haloi [email protected]

@k8s-ci-robot
Copy link
Copy Markdown

k8s-ci-robot commented Mar 29, 2021

Hi @alakesh. Thanks for your PR.

I'm waiting for a containerd member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Mar 29, 2021

Build succeeded.

kzys
kzys requested changes Mar 29, 2021
View reviewed changes
Comment thread snapshots/devmapper/pool_device.go Outdated
Comment thread snapshots/devmapper/pool_device.go Outdated
Comment thread snapshots/devmapper/snapshotter.go Outdated
@alakesh alakesh force-pushed the check-out-of-space branch from f517da0 to e414989 Compare March 29, 2021 21:26
@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Mar 29, 2021

Build succeeded.

@alakesh alakesh force-pushed the check-out-of-space branch from e414989 to d0a462d Compare March 29, 2021 22:16
@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Mar 29, 2021

Build succeeded.

@mxpv
Copy link
Copy Markdown
Member

mxpv commented Mar 30, 2021 •
edited
Loading

Why not to use "official" way, what linux/devmapper already provide to take care of this? (specify water mark in thin-pool and trap the event)? containerd will not reallocate thin pool, so it needs custom handling any way. Also I don't think ppl will be noticing this warning until it's too late.

@alakesh
Copy link
Copy Markdown
Contributor Author

alakesh commented Mar 30, 2021

Why not to use "official" way, what linux/devmapper already provide to take care of this? (specify water mark in thin-pool and trap the event)? containerd will not reallocate thin pool, so it needs custom handling any way. Also I don't think ppl will be noticing this warning until it's too late.

I agree, that is the right thing to do. But in cases where the event is not handled, device mapper can silently run out of space and cause number of other issues which are sometimes difficult to track down. A warning or a debug level message at least would help in this case. We can possibly rate limit such warnings/debug level messages.

@kzys
Copy link
Copy Markdown
Member

kzys commented Mar 30, 2021 •
edited
Loading

If containerd's usage is way above the low water mark and cannot run mkfs anymore, what would the error we will get from either mkfs or dmsetup?

@alakesh alakesh force-pushed the check-out-of-space branch from d0a462d to ce13a76 Compare April 1, 2021 23:37
@alakesh alakesh changed the title devmapper: check if pool data space running low devmapper: do not create snapshot if pool data space running low Apr 1, 2021
@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Apr 1, 2021

Build succeeded.

@alakesh alakesh force-pushed the check-out-of-space branch from ce13a76 to aae6a6f Compare April 2, 2021 20:38
@alakesh alakesh changed the title devmapper: do not create snapshot if pool data space running low devmapper: log pool status when mkfs fails Apr 2, 2021
@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Apr 2, 2021

Build succeeded.

@alakesh alakesh force-pushed the check-out-of-space branch from aae6a6f to e7ff8f0 Compare April 2, 2021 20:46
@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Apr 2, 2021

Build succeeded.

mxpv
mxpv approved these changes Apr 5, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@mxpv mxpv left a comment

Choose a reason for hiding this comment

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

LGTM on green
(CI is having a hard time)

@mxpv
Copy link
Copy Markdown
Member

mxpv commented Apr 12, 2021

Can you rebase changes to get CI green?

@alakesh alakesh force-pushed the check-out-of-space branch 2 times, most recently from 2f9ccf2 to 3deaba0 Compare April 12, 2021 17:42
@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Apr 12, 2021

Build succeeded.

@alakesh
Copy link
Copy Markdown
Contributor Author

alakesh commented Apr 12, 2021

Can you rebase changes to get CI green?

Looks like one test is still having hard time: "CI / Project Checks". I tried twice. Any suggestion?

@alakesh alakesh force-pushed the check-out-of-space branch from 3deaba0 to cb3ddb2 Compare April 12, 2021 18:52
@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Apr 12, 2021

Build succeeded.

@alakesh alakesh force-pushed the check-out-of-space branch 2 times, most recently from f63373b to 59877fc Compare April 12, 2021 19:16
@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Apr 12, 2021

Build succeeded.

@alakesh
devmapper: log pool status when mkfs fails
5ce35ac 
If mkfs on device mapper thin pool fails, it will show pool status
as returned by dmsetup for enahnced error reporting.

Signed-off-by: Alakesh Haloi <[email protected]>
@alakesh alakesh force-pushed the check-out-of-space branch from 59877fc to 5ce35ac Compare April 12, 2021 19:25
@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Apr 12, 2021

Build succeeded.

@alakesh
Copy link
Copy Markdown
Contributor Author

alakesh commented Apr 12, 2021

Can you rebase changes to get CI green?

CI looks green now.

estesp
estesp approved these changes Apr 13, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@estesp estesp left a comment

Choose a reason for hiding this comment

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

LGTM

@estesp estesp merged commit b7c8136 into containerd:master Apr 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kzys kzys kzys approved these changes

@mxpv mxpv mxpv approved these changes

@estesp estesp estesp approved these changes

Assignees

No one assigned

Labels

needs-ok-to-test

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@alakesh @k8s-ci-robot @mxpv @kzys @estesp