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

errdefs: detect certain sycall errors as internal #5182

Merged
merged 2 commits into from
Sep 4, 2024

Conversation

tonistiigi
Copy link
Member

@tonistiigi tonistiigi commented Jul 23, 2024

depends on #5181
depends on #5163


var systemErrors = []syscall.Errno{
syscall.EIO, // I/O error
syscall.ENOMEM, // Out of memory
Copy link
Member Author

Choose a reason for hiding this comment

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

@silvin-lubecki I'm not sure if we should mark ENOMEM and ENOSPC as internal like others or maybe some ResourceExhausted code would make more sense. Technically these are likely caused by too much usage from user, not internal bugs.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with you @tonistiigi. I like what you suggested with the specific error code, it allows some latitude to interpret it as user or internal error 👍

Copy link
Member Author

Choose a reason for hiding this comment

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

Moved ENOMEM and ENOSPC to ResourceExhausted code instead.

@thompson-shaun thompson-shaun added this to the v0.16.0 milestone Jul 25, 2024
@thompson-shaun thompson-shaun modified the milestones: v0.16.0, v0.17.0 Aug 9, 2024
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
@tonistiigi tonistiigi marked this pull request as ready for review August 13, 2024 11:57
tonistiigi added a commit to tonistiigi/buildkit that referenced this pull request Aug 16, 2024
If container exits with error and has invoked OOMKiller
mark the origin error as ENOMEM so that it can be detected
on the client side.

gRPC will set ENOMEM as codes.ResouceExhausted based on moby#5182

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
tonistiigi added a commit to tonistiigi/buildkit that referenced this pull request Aug 16, 2024
If container exits with error and has invoked OOMKiller
mark the origin error as ENOMEM so that it can be detected
on the client side.

gRPC will set ENOMEM as codes.ResouceExhausted based on moby#5182

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
@tonistiigi tonistiigi modified the milestones: v0.17.0, v0.16.0 Aug 30, 2024
@crazy-max
Copy link
Member

PTAL @silvin-lubecki 🙏

Copy link
Contributor

@silvin-lubecki silvin-lubecki left a comment

Choose a reason for hiding this comment

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

LGTM

@crazy-max crazy-max merged commit abe8c2b into moby:master Sep 4, 2024
80 checks passed
daghack pushed a commit to daghack/buildkit that referenced this pull request Sep 19, 2024
If container exits with error and has invoked OOMKiller
mark the origin error as ENOMEM so that it can be detected
on the client side.

gRPC will set ENOMEM as codes.ResouceExhausted based on moby#5182

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants