Skip to content

Add Opt for modifying shm size#3655

Merged
estesp merged 1 commit intocontainerd:masterfrom
crosbymichael:shmsize
Sep 18, 2019
Merged

Add Opt for modifying shm size#3655
estesp merged 1 commit intocontainerd:masterfrom
crosbymichael:shmsize

Conversation

@crosbymichael
Copy link
Copy Markdown
Member

@crosbymichael crosbymichael commented Sep 17, 2019

Closes #3654

Signed-off-by: Michael Crosby [email protected]

@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Sep 17, 2019

Build succeeded.

jterry75
jterry75 approved these changes Sep 17, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@jterry75 jterry75 left a comment

Choose a reason for hiding this comment

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

LGTM

@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Sep 17, 2019

Build succeeded.

@dmcgowan dmcgowan added this to the 1.3 milestone Sep 17, 2019
@estesp
Copy link
Copy Markdown
Member

estesp commented Sep 17, 2019

maybe golangci-lint is good for something :) a unix vs. linux issue with the helper?

darwin build:

oci/spec_opts_unix_test.go:89:12: undeclared name: `WithDevShmSize` (typecheck)

@d-axel-b d-axel-b mentioned this pull request Sep 18, 2019
Open
@estesp
Copy link
Copy Markdown
Member

estesp commented Sep 18, 2019

Just as a follow-up, looks like the test would need to go into a new file that is // +build linux as the unix test file is only !windows which allows it to build on darwin, which doesn't have the new Opt for shm available.

@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Sep 18, 2019

Build succeeded.

@codecov-io
Copy link
Copy Markdown

codecov-io commented Sep 18, 2019

Codecov Report

Merging #3655 into master will decrease coverage by 3.49%.
The diff coverage is 75%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #3655     +/-   ##
=========================================
- Coverage    45.9%   42.41%   -3.5%     
=========================================
  Files         114      127     +13     
  Lines       11255    14060   +2805     
=========================================
+ Hits         5167     5964    +797     
- Misses       5189     7196   +2007     
- Partials      899      900      +1
Flag Coverage Δ
#linux 45.93% <72.72%> (+0.02%) ⬆️
#windows 37.31% <75%> (?)
Impacted Files Coverage Δ
oci/spec_opts.go 31.22% <75%> (-0.66%) ⬇️
archive/tar_opts.go 58.82% <0%> (-12.61%) ⬇️
snapshots/native/native.go 43.04% <0%> (-9.99%) ⬇️
metadata/snapshot.go 47.9% <0%> (-8.6%) ⬇️
cio/io.go 46.47% <0%> (-8.53%) ⬇️
archive/tar.go 48.18% <0%> (-8.51%) ⬇️
metadata/containers.go 47.97% <0%> (-6.62%) ⬇️
content/local/writer.go 58.65% <0%> (-5.55%) ⬇️
content/local/store.go 49.52% <0%> (-5.29%) ⬇️
metadata/images.go 57.57% <0%> (-4.99%) ⬇️
... and 73 more

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 c3452ab...20d3fae. Read the comment docs.

@crosbymichael
Copy link
Copy Markdown
Member Author

crosbymichael commented Sep 18, 2019

@estesp updated

estesp
estesp approved these changes Sep 18, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@estesp estesp left a comment

Choose a reason for hiding this comment

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

LGTM

@estesp estesp merged commit a7e67ff into containerd:master Sep 18, 2019
@sfc-gh-aivanou
Copy link
Copy Markdown

sfc-gh-aivanou commented May 25, 2023

Is it possible to start using WithDevShmSize ? Otherwise it does not make sense to have this function defined, without actually using it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mxpv mxpv mxpv approved these changes

@estesp estesp estesp approved these changes

@jterry75 jterry75 jterry75 approved these changes

@AkihiroSuda AkihiroSuda AkihiroSuda approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

1.3

Development

Successfully merging this pull request may close these issues.

/dev/shm default size should be configurable easily

8 participants

@crosbymichael @estesp @codecov-io @sfc-gh-aivanou @mxpv @jterry75 @AkihiroSuda @dmcgowan