-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Fix a bug where --pids-limit was parsed incorrectly #6910
Conversation
32c7634
to
10a3fc7
Compare
Oops |
@rhatdan You want me to PR this into your branch for pidslimit? |
@mheon I fixed the main branch PR for pids-limit. |
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
This is waiting on #6906 |
The --pids-limit flag was using strconv.ParseInt with bad arguments, resulting in it being unable to parse standard integers (1024, for example, would produce an 'out of range' error). Change the arguments to make sense (base 10, max 32-bit) and add a test to ensure we don't regress again. Fixes containers#6908 Signed-off-by: Matthew Heon <[email protected]>
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mheon The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Rebased atop latest v2.0, this should go green |
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
/hold
LGTM |
/hold cancel |
The --pids-limit flag was using strconv.ParseInt with bad arguments, resulting in it being unable to parse standard integers (1024, for example, would produce an 'out of range' error).
Change the arguments to make sense (base 10, max 32-bit) and add a test to ensure we don't regress again.
Fixes #6908
Currently only against v2.0 branch, as it looks like changes that caused this have not landed in master yet.