This repository has been archived by the owner on May 12, 2021. It is now read-only.
rootless: implement rootless fs and support --rootless option #2429
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
/test |
devimc
force-pushed
the
topic/virtcontainers/rootlessStore
branch
from
17ac5ce to
7a1b00f
Compare
|
/test |
Codecov Report
@@ Coverage Diff @@
## master #2429 +/- ##
==========================================
+ Coverage 50.62% 50.65% +0.02%
==========================================
Files 112 114 +2
Lines 16274 16315 +41
==========================================
+ Hits 8239 8264 +25
- Misses 7020 7029 +9
- Partials 1015 1022 +7 |
devimc
force-pushed
the
topic/virtcontainers/rootlessStore
branch
from
7a1b00f to
4151daf
Compare
|
/test |
devimc
force-pushed
the
topic/virtcontainers/rootlessStore
branch
from
4151daf to
1a3c39d
Compare
|
/test |
devimc
force-pushed
the
topic/virtcontainers/rootlessStore
branch
from
1a3c39d to
8324fb2
Compare
|
/test |
|
review please |
jodh-intel
approved these changes
Feb 6, 2020
There was a problem hiding this comment.
Thanks @devimc - that's a pretty big PR but thanks for doing this and updating the tests too!
lgtm
virtcontainers/clh_test.go
Outdated
| clh := &cloudHypervisor{} | ||
| store, err := persist.GetDriver() | ||
| if err != nil { | ||
| t.Errorf("persist.GetDriver() unexpected error != %v", err) |
There was a problem hiding this comment.
Nit: The assert syntax save you two lines of code 😄
assert.NoError(err)
devimc
force-pushed
the
topic/virtcontainers/rootlessStore
branch
from
8324fb2 to
e95bfec
Compare
amshinde
reviewed
Feb 10, 2020
| @@ -208,7 +213,12 @@ func TestAcrnGetSandboxConsole(t *testing.T) { | |||
| func TestAcrnCreateSandbox(t *testing.T) { | |||
| assert := assert.New(t) | |||
| acrnConfig := newAcrnConfig() | |||
| a := &Acrn{} | |||
| store, err := persist.GetDriver() | |||
There was a problem hiding this comment.
Shoudlnt these tests be run with a mockdriver? I dont see if the the variable mockTesting has been set to true anywhere?
There was a problem hiding this comment.
it's set in TestMain (entrypoint) https://github.com/kata-containers/runtime/pull/2429/files#diff-e8e7e93733a02e75346a2f9d43ad6149R113
cli/main.go
Outdated
| if err != nil { | ||
| return err | ||
| } | ||
| // If nil, not set a value, let vc auto-detect rootless |
There was a problem hiding this comment.
This comment is slightly confusing, as we are checking non-nil value below. Perhaps, this could be worded as
If flag is true/false, assign the rootless flag. vc will not perform any auto-detection in that case. In case flag is nil or auto, vc detects if the runtime is running as rootless.
|
|
||
| // from runC | ||
| // parseBoolOrAuto returns (nil, nil) if s is empty or "auto" | ||
| func parseBoolOrAuto(s string) (*bool, error) { |
There was a problem hiding this comment.
I know this is a small function, but I think its better to include the runc license in the file if not included since we have taken this code varbatim. cc @grahamwhaley wdyt?
devimc
force-pushed
the
topic/virtcontainers/rootlessStore
branch
2 times, most recently
from
5b3ebd7 to
1e0042f
Compare
|
@amshinde changes applied, thanks |
amshinde
approved these changes
Feb 10, 2020
devimc
force-pushed
the
topic/virtcontainers/rootlessStore
branch
from
1e0042f to
569bbbb
Compare
added 9 commits
February 12, 2020 19:09
rootless is used in katautils, cli and virtcontainers. It makes more sense if it's part of virtcontainer, this way virtcontainers won't depend on other runtime subpackages Signed-off-by: Julio Montes <[email protected]>
`agent.getVMPath()` is an almost useless method that can be easily replaced with `filepath.Join()` Signed-off-by: Julio Montes <[email protected]>
Update persist FS API and interface to support rootless and mock filesystem implementations. `RunStoragePath` and `RunVMStoragePath` are part of FS object and may change their path depending on the driver (rootless/mock/fs) Signed-off-by: Julio Montes <[email protected]>
Rootless fs driver inherits from FS and may overwrite its methods. All files and directories created by this driver are under a path accessible for the current user, typically this path is defined by the environment variable `XDG_RUNTIME_DIR`, if this variable is not defined, the default path `/run/user/$UID` is used instead, where $UID is the current user ID. fixes kata-containers#2416 Signed-off-by: Julio Montes <[email protected]>
Mock FS driver can be used in unit testing to allow Mock fs driver inherits from FS and may overwrite its methods. All files and directories created by this driver are under a path accessible for all users, this path is created under the system temporal directory. Signed-off-by: Julio Montes <[email protected]>
GetDriver returns new PersistDriver according to current needs, a mock fs driver is returned when mockTesting is enabled, a rootless fs is returned when rootless is detected, otherwise a fs driver is used. Signed-off-by: Julio Montes <[email protected]>
Fix factory implementation and unit tests to support the new persist API Signed-off-by: Julio Montes <[email protected]>
Fix sandbox implementation and unit tests to support the new persist API Signed-off-by: Julio Montes <[email protected]>
Fix hypervisor implementations and unit tests to support the new persist API Signed-off-by: Julio Montes <[email protected]>
added 3 commits
February 12, 2020 19:09
Fix API, container and kata implementations and unit tests to support the new persist API Signed-off-by: Julio Montes <[email protected]>
By default virtcontainer auto-detects if the current process is running rootless or not, but this behavior can change from commandline with the --rootless option fixes kata-containers#2417 Signed-off-by: Julio Montes <[email protected]>
Fix comment on exported var `IsRootless` should be of the form `IsRootless ...` (golint) Signed-off-by: Julio Montes <[email protected]>
devimc
force-pushed
the
topic/virtcontainers/rootlessStore
branch
from
569bbbb to
a45cf62
Compare
|
/test |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.