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

podman: add --ulimit host #3491

Merged
merged 1 commit into from
Jul 11, 2019

Conversation

giuseppe
Copy link
Member

@giuseppe giuseppe commented Jul 3, 2019

add a simple way to copy ulimit values from the host.

if --ulimit host is used then the current ulimits in place are copied
to the container.

Signed-off-by: Giuseppe Scrivano [email protected]

@openshift-ci-robot openshift-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/M labels Jul 3, 2019
pkg/spec/spec.go Outdated
return errors.New("ulimit can use host only once")
}

type Rlimit struct {
Copy link
Member

Choose a reason for hiding this comment

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

Does this need to be public? It's purely scoped within this loop

Copy link
Member Author

Choose a reason for hiding this comment

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

good point. I've pushed a new version

@mheon
Copy link
Member

mheon commented Jul 3, 2019

One nit otherwise LGTM

@giuseppe giuseppe force-pushed the rlimit-host branch 5 times, most recently from 6062cd1 to 0bd184d Compare July 4, 2019 10:55
@rhatdan
Copy link
Member

rhatdan commented Jul 4, 2019

LGTM
But tests need fixes.

@giuseppe giuseppe force-pushed the rlimit-host branch 9 times, most recently from 7f23de3 to 0f99d88 Compare July 8, 2019 16:34
add a simple way to copy ulimit values from the host.

if --ulimit host is used then the current ulimits in place are copied
to the container.

Signed-off-by: Giuseppe Scrivano <[email protected]>
@giuseppe
Copy link
Member Author

giuseppe commented Jul 9, 2019

tests are passing

@giuseppe
Copy link
Member Author

ping

@rhatdan
Copy link
Member

rhatdan commented Jul 11, 2019

@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: giuseppe, jamescassell

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@mheon
Copy link
Member

mheon commented Jul 11, 2019

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 11, 2019
@openshift-merge-robot openshift-merge-robot merged commit 2b64f88 into containers:master Jul 11, 2019
@@ -553,6 +559,20 @@ func addRlimits(config *CreateConfig, g *generate.Generator) error {
)

for _, u := range config.Resources.Ulimit {
if u == "host" {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there really no way to tell runc "just don't change rlimits"?

Copy link
Member Author

Choose a reason for hiding this comment

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

it seems I've unnecessarily complicated things. Looks like it is enough to not specify any limits block and that won't change the rlimits. I'll do some more tests and open a PR

Copy link
Member Author

Choose a reason for hiding this comment

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

much simpler version: #3583

@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 26, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants