Assorted CLI nitpicks #3374
Merged
Assorted CLI nitpicks #3374
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
|
I have |
The value of root is already an absolute path since commit ede8a86, so it does not make sense to call filepath.Abs() again. Signed-off-by: Kir Kolyshkin <[email protected]>
There is a mix of styles when handling CLI commands. In most cases we return an error, which is handled from app.Run in main.go (it calls fatal if there is an error). In a few cases, though, we call fatal(err) from random places. Let's be consistent and always return an error. The only exception is runc exec, which needs to exit with a particular exit code. Signed-off-by: Kir Kolyshkin <[email protected]>
1. Variable xdgRuntimeDir is only checked to be non-empty. Change it to a boolean. 2. Refactor so that os.Getenv is not called twice. Signed-off-by: Kir Kolyshkin <[email protected]>
1. In case --root option is not provided, do nothing. 2. Instead of checking if root value is empty string, check it after filepath.Abs, and reject "/". Improve docstring while at it. Signed-off-by: Kir Kolyshkin <[email protected]>
It is questionable whether runc list should return an empty list of containers when non-existent --root is specified or not. The current behavior is the directory is always created and then the empty list of container is shown. To my mind, specifying a non-existent root is an error and should be reported as such. This is what this patch does. For backward compatibility, if --root is not set (i.e. a default is used), ENOENT is not reported. Signed-off-by: Kir Kolyshkin <[email protected]>
Signed-off-by: Kir Kolyshkin <[email protected]>
|
centos 8 CI failure is a flake; working on a fix in #3392 |
mrunalp
approved these changes
Mar 1, 2022
|
I have |
AkihiroSuda
approved these changes
Mar 9, 2022
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This is mostly some minor refactoring, making things cleaner.
The only significant change is
runc --root non-existent-dir listnow reports an error for non-existent root directory. The error is not reported in case a default root is used.This is a preparation for #3373.