Skip to content

Comments

Embedded runc fixes#232

Merged
AkihiroSuda merged 9 commits intogenuinetools:masterfrom
kekoav:embedded-runc-fixes
Jul 2, 2019
Merged

Embedded runc fixes#232
AkihiroSuda merged 9 commits intogenuinetools:masterfrom
kekoav:embedded-runc-fixes

Conversation

@kekoav
Copy link
Contributor

@kekoav kekoav commented Apr 24, 2019

Closes #231 . Depends on #230 being merged.

There was some flakiness with the embedded runc. A system installed runc would previously be used inadvertently. This is an attempt to fix bugs, but also harden some of the logic and error handling around embedding to make sure we are using the runc that the user expected.

What I did

  • Moved embedded runc from temporary directory to the state directory. (eg. .local/share/img/bin)
  • Require runc to be installed for all commands. This improves UX because users don't have to guess which commands require runc, and they will get quick feedback whether their img installation is fully working.
  • If the embedded runc is requested, ensure the runc found on the PATH is indeed the embedded runc.
  • Add debugging to the version endpoint to show which runc is being used.
  • Added debugging log statements to help debug runc issues.
  • Remove requirement of installed runc from builds.

img version Improvements

$ ./img version
img:
 version     : v0.5.6
 git hash    : 5d5df1b-dirty
 go version  : go1.11.9
 go compiler : gc
 platform    : linux/amd64
runc:
 version     : 1.0.0-rc6+dev
 commit      : 7cb3cde1f49eae53fb8fff5012c0750a64eb928b
 spec        : 1.0.1-dev

@codecov-io
Copy link

codecov-io commented Apr 24, 2019

Codecov Report

Merging #232 into master will not change coverage.
The diff coverage is 0%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #232    +/-   ##
======================================
  Coverage       0%     0%            
======================================
  Files          14     16     +2     
  Lines         782    957   +175     
======================================
- Misses        782    957   +175
Impacted Files Coverage Δ
push.go 0% <0%> (ø) ⬆️
unpack.go 0% <0%> (ø) ⬆️
pull.go 0% <0%> (ø) ⬆️
login.go 0% <0%> (ø) ⬆️
tag.go 0% <0%> (ø) ⬆️
main.go 0% <0%> (ø) ⬆️
diskusage.go 0% <0%> (ø) ⬆️
prune.go 0% <0%> (ø) ⬆️
build.go 0% <0%> (ø) ⬆️
list.go 0% <0%> (ø) ⬆️
... and 6 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 158ee8b...52e6930. Read the comment docs.

Copy link
Collaborator

@AkihiroSuda AkihiroSuda left a comment

Choose a reason for hiding this comment

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

LGTM

@jessfraz PTAL

@jessfraz
Copy link
Collaborator

waaah so sorry for the delay seems like it needs a rebase, @AkihiroSuda i think you have merge rights please feel free to merge in the future I feel badly being the hold up :)

@kekoav kekoav force-pushed the embedded-runc-fixes branch from 46f1db9 to 52e6930 Compare June 24, 2019 04:34
@kekoav
Copy link
Contributor Author

kekoav commented Jun 24, 2019

@jessfraz no worries, we are all busy 😄

@AkihiroSuda @jessfraz I have rebased. Reminder, this PR is on top of #230 .

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Embedded runc issues

4 participants