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

Use update time to detect if kmem limits have been set #935

Merged
merged 2 commits into from
Jul 11, 2016

Conversation

vishh
Copy link
Contributor

@vishh vishh commented Jul 6, 2016

This PR should also fix #841

xref: kubernetes/kubernetes#27204
#841 needs to be fixed to make it possible to update libcontainer version in kubernetes.

cc @cyphar @crosbymichael @dubstack

@vishh
Copy link
Contributor Author

vishh commented Jul 7, 2016

cc @mrunalp

_, err = d.join("memory")
if err != nil && !cgroups.IsNotFound(err) {
if _, jerr := d.join("memory"); jerr != nil && !cgroups.IsNotFound(jerr) {
err = jerr
return err
Copy link
Contributor

Choose a reason for hiding this comment

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

could just return jerr

Copy link
Contributor Author

Choose a reason for hiding this comment

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

err needs to be set for the defer function to execute.

@mrunalp
Copy link
Contributor

mrunalp commented Jul 8, 2016

Looks fine except for one nit.

@vishh
Copy link
Contributor Author

vishh commented Jul 8, 2016

ping @LK4D4 @cyphar @crosbymichael @dqminh

@mrunalp
Copy link
Contributor

mrunalp commented Jul 8, 2016

LGTM

Approved with PullApprove

1 similar comment
@hqhq
Copy link
Contributor

hqhq commented Jul 11, 2016

LGTM

Approved with PullApprove

@hqhq hqhq merged commit 4eb8c2f into opencontainers:master Jul 11, 2016
hqhq added a commit to hqhq/runc that referenced this pull request Jul 21, 2016
Revert: opencontainers#935
Fixes: opencontainers#946

I can reproduce opencontainers#946 on some machines, the problem is on
some machines, it could be very fast that modify time
of `memory.kmem.limit_in_bytes` could be the same as
before it's modified.

And now we'll call `SetKernelMemory` twice on container
creation which cause the second time failure.

Revert this before we find a better solution.

Signed-off-by: Qiang Huang <[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.

sysconfig_notcgo does not implement GetLongBit
4 participants