Skip to content

errdefs: detect certain sycall errors as internal#5182

Merged
crazy-max merged 2 commits intomoby:masterfrom
tonistiigi:internal-syscall-err
Sep 4, 2024
Merged

errdefs: detect certain sycall errors as internal#5182
crazy-max merged 2 commits intomoby:masterfrom
tonistiigi:internal-syscall-err

Conversation

@tonistiigi
Copy link
Copy Markdown
Member

@tonistiigi tonistiigi commented Jul 23, 2024

depends on #5181
depends on #5163

Comment thread errdefs/internal.go Outdated

var systemErrors = []syscall.Errno{
syscall.EIO, // I/O error
syscall.ENOMEM, // Out of memory
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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.

@tonistiigi tonistiigi force-pushed the internal-syscall-err branch from 977cd82 to d90b759 Compare July 23, 2024 01:42
@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
@tonistiigi tonistiigi force-pushed the internal-syscall-err branch from d90b759 to d9a9a4d Compare August 13, 2024 11:57
@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 <[email protected]>
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 <[email protected]>
@tonistiigi tonistiigi modified the milestones: v0.17.0, v0.16.0 Aug 30, 2024
@crazy-max
Copy link
Copy Markdown
Member

PTAL @silvin-lubecki 🙏

Copy link
Copy Markdown
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
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 <[email protected]>
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