-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Use CONTAINERS_CONF cgroups flag for remote API. #12860
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rhatdan 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 |
Fixed: #12550 |
I got a bit worried about the diffstat, but CGroup -> Cgroup makes sense to me |
Yes I went both ways and this seems least breaking. |
Signed-off-by: Daniel J Walsh <[email protected]>
Also change code to globably be consistent when refering to capatilized Cgroup. Fixed: containers#12550 Signed-off-by: Daniel J Walsh <[email protected]>
@containers/podman-maintainers PTAL |
LGTM |
inspect.WaitWithDefaultTimeout() | ||
Expect(inspect.OutputToString()).To(Not(Equal("disabled"))) | ||
|
||
os.Setenv("CONTAINERS_CONF", conffile) |
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.
Can you unset it in a defer
?
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.
Not needed in containers_conf_test. The Before/After are taking care of this.
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.
Regarding CGroup
vs Cgroup
. I'd prefer to call it cgroup
in error messages since that's what man pages etc. use. The camel cases in the code LGTM.
No description provided.