Ensure good defaults on blank c/storage configuration #3954
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mheon The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
| @@ -281,10 +281,16 @@ func defaultRuntimeConfig() (RuntimeConfig, error) { | |||
| if err != nil { | |||
| return RuntimeConfig{}, err | |||
| } | |||
| graphRoot := storeOpts.GraphRoot | |||
| if graphRoot == "" { | |||
| graphRoot = "/var/lib/containers/storage" | |||
There was a problem hiding this comment.
You should at least warn on this.
There was a problem hiding this comment.
Good point, added warning logs
There was a problem hiding this comment.
Concur, but could be talked into a debug instead. Chocolate chip cookies would help sway me! ;^)
| // Relative paths can cause nasty bugs, because core paths we use could | ||
| // shift between runs (or even parts of the program - the OCI runtime | ||
| // uses a different working directory than we do, for example. | ||
| if !filepath.IsAbs(runtime.config.StaticDir) { |
There was a problem hiding this comment.
Should these checks should go into containers/storage?
There was a problem hiding this comment.
These paths are all ours - libpod static and temporary files directories and libpod named volumes
If c/storage paths are explicitly set to "" (the empty string) it will use compiled-in defaults. However, it won't tell us this via `storage.GetDefaultStoreOptions()` - we just get the empty string (which can put our defaults, some of which are relative to c/storage, in a bad spot). Hardcode a sane default for cases like this. Furthermore, add some sanity checks to paths, to ensure we don't use relative paths for core parts of libpod. Fixes containers#3952 Signed-off-by: Matthew Heon <matthew.heon@pm.me>
06561c1 to
3a09956
Compare
|
LGTM assuming happy tests. |
|
/lgtm |
If c/storage paths are explicitly set to "" (the empty string) it will use compiled-in defaults. However, it won't tell us this via
storage.GetDefaultStoreOptions()- we just get the empty string (which can put our defaults, some of which are relative to c/storage, in a bad spot).Hardcode a sane default for cases like this. Furthermore, add some sanity checks to paths, to ensure we don't use relative paths for core parts of libpod.
Fixes #3952