Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

vendor: introduce libcontainer/cgroups package #405

Closed
wants to merge 1 commit into from
Closed

vendor: introduce libcontainer/cgroups package #405

wants to merge 1 commit into from

Conversation

jingxiaolu
Copy link
Contributor

Introduce libcontainer/cgroups package for further cgroups handling refactor

Fixes: #344

Signed-off-by: Jingxiao Lu lujingxiao@huawei.com

Introduce libcontainer/cgroups package for further cgroups handling refactor

Fixes: #344

Signed-off-by: Jingxiao Lu <lujingxiao@huawei.com>
@katacontainersbot
Copy link
Contributor

PSS Measurement:
Qemu: 146593 KB
Proxy: 4690 KB
Shim: 8855 KB

Memory inside container:
Total Memory: 2045972 KB
Free Memory: 2007580 KB

@jingxiaolu
Copy link
Contributor Author

This is the first PR for #344, introducing libcontainer/cgroups package for refactoring cgroups handling, as recommended by @jshachm.

Please share your comments on this issue 😄

Other PRs will come on including:

  1. Refactoring with this libcontainer/cgroups package;
  2. Adding containers to the same sandbox cgroup;
  3. Adding proxy, qemu... to sandbox cgroup

@jshachm
Copy link
Member

jshachm commented Jun 18, 2018

@jodh-intel
Copy link
Contributor

jodh-intel commented Jun 18, 2018

Hi @jingxiaolu - thanks for raising.

lgtm.

Approved with PullApprove

@devimc
Copy link

devimc commented Jun 18, 2018

lgtm

Approved with PullApprove

@jshachm
Copy link
Member

jshachm commented Jun 18, 2018

@jodh-intel CI failure looks same as kata-containers/tests#427 (see #385 (comment)) .

Copy link
Member

@amshinde amshinde left a comment

Choose a reason for hiding this comment

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

@jingxiaolu I dont see any changes to the Gopkg.toml file here.

@jodh-intel
Copy link
Contributor

@amshinde - I noticed that too. I believe that's because no change is necessary to Gopkg.toml since it already contains an entry for runc - the .toml file doesn't seem to record the (sub-)packages used - that only gets added to the .lock file.

@amshinde
Copy link
Member

@jodh-intel You are right. Missed out on the previous opencontainers package.

@amshinde
Copy link
Member

@jingxiaolu Can you rebase your changes on top of the latest master? CI failures have been fixed on master.

Copy link

@sboeuf sboeuf left a comment

Choose a reason for hiding this comment

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

@jingxiaolu sorry but we cannot accept this PR as is. The vendoring diff that you're introducing here depends on the fact that you've added an import into the code, meaning that a dep update gave you such a diff.
So basically, you will have to submit this change through the PR introducing the change in the code where you import the new package.

@jingxiaolu
Copy link
Contributor Author

@sboeuf that's reasonable, I'll resubmit with more implementation~

@jshachm @jodh-intel @devimc @amshinde thanks for your time 👍

# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants