Add support for Libpod configuration file #430
Conversation
This allows more graceful handling of multiple paths in a config file. Signed-off-by: Matthew Heon <matthew.heon@gmail.com>
Signed-off-by: Matthew Heon <matthew.heon@gmail.com>
Signed-off-by: Matthew Heon <matthew.heon@gmail.com>
Signed-off-by: Matthew Heon <matthew.heon@gmail.com>
Signed-off-by: Matthew Heon <matthew.heon@gmail.com>
|
I would prefer that we did not have to add a configuration file, because the goal of all these tools is to share content. So I don't want to have a libpod.conf, buildah.conf, skopeo.conf and would love to get rid of most of crio.conf. If we decide we really need a libpod.conf, it should be stored in /usr/lib/containers/libpod.conf and should have everything commented out so that it shares storage.conf, policy.conf ... with the other tools. I don't see many users ever editing this file, so it brings little value, and is likely to cause us issues where crio, buildah, podman, and skopeo don't work together. |
|
This does not override c/storage or c/image configuration. It exists solely for libpod-specific configuration (paths for runc, conmon, where to store the DB, etc). |
|
The big driver for this one is going to be different distributions, we can't know where runc and conmon will be, so we need to give the ability to customize those paths. Same for the libpod database. |
|
Ok as long as we stick to those libpod specific paths and don't venture into conflicting with other config, I am fine with it. But it should be installed in /usr/share/containers/libpod.conf with the overide in /etc/containers/libpod.conf |
The standard config has moved to /usr/share/containers/ per discussion. An override configuration file is allowed at the previous /etc/containers/ location. This override will be used in place of the normal config if both are present, and exists to override distro packaged configs without modifying the standard config. Signed-off-by: Matthew Heon <matthew.heon@gmail.com>
|
Default location changed to /usr/share/containers with override path of /etc/containers allowed. |
libpod/runtime.go
Outdated
| // images | ||
| ImageDefaultTransport string `toml:"image_default_transport"` | ||
| // SignaturePolicyPath is the path to a signature policy to use for | ||
| // validationg images |
There was a problem hiding this comment.
nit validtiong to validating
libpod/runtime.go
Outdated
| // Overwrite it with user-given configuration options | ||
| // Check to see if the given configuration file exists | ||
| if _, err := os.Stat(configPath); err != nil { | ||
| return nil, errors.Wrapf(err, "error stating configuration file %s", configPath) |
There was a problem hiding this comment.
might be me, but I'd pref getting rid of 'stating'. Perhaps:
"error performing stat on configuration file"
"error unable to determine status of configuration file"
When I saw stating, my first thought is you had trouble starting the config file.
Signed-off-by: Matthew Heon <matthew.heon@gmail.com>
libpod.conf
Outdated
| cgroup_manager = "cgroupfs" | ||
|
|
||
| # Directory for persistent libpod files (database, etc) | ||
| static_dir = "/var/lib/containers/storage/libpod" |
There was a problem hiding this comment.
Can this be commented out, and we just get it relative to where container storage is configured in /etc/containers/storage.conf, also have a reference to that here.
There was a problem hiding this comment.
Fixed. We already set it relative to containers/storage in the baked-in defaults, so just commenting this out is enough to get a path relative to containers/storage
| @@ -101,7 +101,11 @@ func WithOCIRuntime(runtimePath string) RuntimeOption { | |||
| return ErrRuntimeFinalized | |||
| } | |||
|
|
|||
| rt.config.RuntimePath = runtimePath | |||
| if runtimePath == "" { | |||
There was a problem hiding this comment.
Should check if the path exists also?
There was a problem hiding this comment.
Runtime will do that when it starts up
| if path != "" { | ||
| rt.config.ConmonPath = path | ||
|
|
||
| if path == "" { |
There was a problem hiding this comment.
Ditto, should check if the path exists.
There was a problem hiding this comment.
Runtime will do that when it starts up
| @@ -394,3 +549,15 @@ func (r *Runtime) Info() ([]InfoData, error) { | |||
|
|
|||
| return info, nil | |||
| } | |||
|
|
|||
| // SaveDefaultConfig saves a copy of the default config at the given path | |||
| func SaveDefaultConfig(path string) error { | |||
There was a problem hiding this comment.
What do you plan to do with this?
There was a problem hiding this comment.
Mostly there just in case we ever change the config and need to regenerate a default one. It seems useful to keep around
Signed-off-by: Matthew Heon <matthew.heon@gmail.com>
|
Should be ready now |
|
📌 Commit 8e59371 has been approved by |
Signed-off-by: Matthew Heon <matthew.heon@gmail.com> Closes: #430 Approved by: rhatdan
Signed-off-by: Matthew Heon <matthew.heon@gmail.com> Closes: #430 Approved by: rhatdan
Signed-off-by: Matthew Heon <matthew.heon@gmail.com> Closes: #430 Approved by: rhatdan
Signed-off-by: Matthew Heon <matthew.heon@gmail.com> Closes: #430 Approved by: rhatdan
The standard config has moved to /usr/share/containers/ per discussion. An override configuration file is allowed at the previous /etc/containers/ location. This override will be used in place of the normal config if both are present, and exists to override distro packaged configs without modifying the standard config. Signed-off-by: Matthew Heon <matthew.heon@gmail.com> Closes: #430 Approved by: rhatdan
Signed-off-by: Matthew Heon <matthew.heon@gmail.com> Closes: #430 Approved by: rhatdan
Signed-off-by: Matthew Heon <matthew.heon@gmail.com> Closes: #430 Approved by: rhatdan
|
☀️ Test successful - status-papr |
This allows more graceful handling of multiple paths in a config file. Signed-off-by: Matthew Heon <matthew.heon@gmail.com> Closes: containers#430 Approved by: rhatdan
Signed-off-by: Matthew Heon <matthew.heon@gmail.com> Closes: containers#430 Approved by: rhatdan
Signed-off-by: Matthew Heon <matthew.heon@gmail.com> Closes: containers#430 Approved by: rhatdan
Signed-off-by: Matthew Heon <matthew.heon@gmail.com> Closes: containers#430 Approved by: rhatdan
Signed-off-by: Matthew Heon <matthew.heon@gmail.com> Closes: containers#430 Approved by: rhatdan
The standard config has moved to /usr/share/containers/ per discussion. An override configuration file is allowed at the previous /etc/containers/ location. This override will be used in place of the normal config if both are present, and exists to override distro packaged configs without modifying the standard config. Signed-off-by: Matthew Heon <matthew.heon@gmail.com> Closes: containers#430 Approved by: rhatdan
Signed-off-by: Matthew Heon <matthew.heon@gmail.com> Closes: containers#430 Approved by: rhatdan
Signed-off-by: Matthew Heon <matthew.heon@gmail.com> Closes: containers#430 Approved by: rhatdan
Wondering if this is still a proposal?
github-actions
bot
added
the
locked - please file new issue/PR
Add a configuration file (
/etc/containerd/libpod.conf) that we can read to override our default configuration. This file is optional, and if not present we'll use our baked-in defaults as before.