Skip to content

Add shim cgroup support for v2 runtimes#3292

Merged
dmcgowan merged 2 commits intocontainerd:masterfrom
crosbymichael:shim-cgroup
May 22, 2019
Merged

Add shim cgroup support for v2 runtimes#3292
dmcgowan merged 2 commits intocontainerd:masterfrom
crosbymichael:shim-cgroup

Conversation

@crosbymichael
Copy link
Copy Markdown
Member

@crosbymichael crosbymichael commented May 20, 2019

This also has a change where task opts are passed to shims via STDIN for early inspection.

@crosbymichael crosbymichael requested a review from Random-Liu May 20, 2019 15:11
@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented May 20, 2019

Build succeeded.

@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented May 20, 2019

Build succeeded.

@codecov-io
Copy link
Copy Markdown

codecov-io commented May 20, 2019

Codecov Report

Merging #3292 into master will decrease coverage by 0.02%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3292      +/-   ##
==========================================
- Coverage    44.4%   44.38%   -0.03%     
==========================================
  Files         113      113              
  Lines       12231    12237       +6     
==========================================
  Hits         5431     5431              
- Misses       5966     5972       +6     
  Partials      834      834
Flag Coverage Δ
#linux 48.3% <0%> (-0.03%) ⬇️
#windows 39.64% <0%> (-0.03%) ⬇️
Impacted Files Coverage Δ
runtime/v2/shim/util.go 0% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cafda1c...fe6a2b0. Read the comment docs.

@mxpv
Copy link
Copy Markdown
Member

mxpv commented May 20, 2019

LGTM

dmcgowan
dmcgowan approved these changes May 21, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@dmcgowan dmcgowan left a comment

Choose a reason for hiding this comment

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

LGTM

@Random-Liu
Copy link
Copy Markdown
Member

Random-Liu commented May 21, 2019

LGTM

The behavior that TaskOptions completely overwrites RuntimeOptions makes it a bit hard to understand and use though.

@dmcgowan
Copy link
Copy Markdown
Member

dmcgowan commented May 21, 2019 •
edited
Loading

The behavior that TaskOptions completely overwrites RuntimeOptions makes it a bit hard to understand and use though.

Is the interaction between containerd and the shim binary documented anywhere?

Maybe we can create a separate issue for that then get this merged.

@crosbymichael
Copy link
Copy Markdown
Member Author

crosbymichael commented May 22, 2019

It has always overwritten these options. Task specific runtime opts take precedence over runtime configured options.

@dmcgowan dmcgowan merged commit ec0b722 into containerd:master May 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dmcgowan dmcgowan dmcgowan approved these changes

@Random-Liu Random-Liu Awaiting requested review from Random-Liu

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@crosbymichael @codecov-io @mxpv @Random-Liu @dmcgowan