Per container runtime binary#1454
Conversation
Signed-off-by: Kenfe-Mickael Laventure <[email protected]>
There was a problem hiding this comment.
can we put these in a struct together for the shim and runtime opts?
There was a problem hiding this comment.
What do you mean?
There was a problem hiding this comment.
Just so we don't have all these fields on the Runtime struct, just for grouping, not anything functional
There was a problem hiding this comment.
Oh, I see.
Sure, will update
mlaventure
force-pushed
the
per-container-runtime-binary
branch
from
02967ea to
2e2f10a
Compare
This also fix the type used for RuncOptions.SystemCgroup, hence introducing an API break. Signed-off-by: Kenfe-Mickael Laventure <[email protected]>
This allow specifying wher the OCI runtime should store its state data. Signed-off-by: Kenfe-Mickael Laventure <[email protected]>
mlaventure
force-pushed
the
per-container-runtime-binary
branch
from
2e2f10a to
1b79170
Compare
Codecov Report
@@ Coverage Diff @@
## master #1454 +/- ##
==========================================
+ Coverage 40.8% 40.82% +0.02%
==========================================
Files 23 23
Lines 2924 2920 -4
==========================================
- Hits 1193 1192 -1
+ Misses 1453 1451 -2
+ Partials 278 277 -1
Continue to review full report at Codecov.
|
|
LGTM I'm not a fan of more flags on the shim but I cannot think of a better way to pass this data ;) |
|
@estesp do you mind having a quick look? 😇 |
estesp
left a comment
estesp
left a comment
There was a problem hiding this comment.
LGTM
One random question about criu path flag that I think I answered myself via code inspection.
| }, | ||
| cli.StringFlag{ | ||
| Name: "criu,c", | ||
| Usage: "path to criu", |
There was a problem hiding this comment.
I guess it's ok to not have a default as, if I followed the code through, an empty/unset value will just not set the flag on runc leaving it up to runc to use a default in the case of checkpoint/restore actions. Is that correct? Any value in having the default here versus there? I'm thinking it doesn't matter as we weren't setting the criu binary path before.
There was a problem hiding this comment.
go-runc ignores the flag if it's empty meaning that runc will use its default. But you gave me a doubt for a few seconds :)
So, yes, I'd rather defer to the default built in runc
…ion-for-make use project dco test locally
4 participants
@crosbymichael I ended up adding an option to the shim for each on in
RuncOptions, may get out of hand, but didn't want to add a config file for it.We could always serialize the whole struct in one cli option otherwise 😇