New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
--cni-plugin-dir flag acts strangely #3862
Comments
I also suspect other slice config values behave similarly, probably should test them all in integration/unit tests... |
@rhafer maybe wanna take this one? :) |
@saschagrunert Yeah. I'll take a look. |
It looks as if I don't think it is really needed to call it twice. The subcommands could just get the parsed config via app.Metadata I guess, that would also avoid parsing all config files twice. Additionally I guess we should return a copy of the slice in ... working on a fix. |
Sounds all good to me, thank you @rhafer 🙏 |
Otherwise the underlying flags in the ctx might be overriden, which can cause issue when the config is reloaded (or GetConfigFromContext() is called more than once). Closes cri-o#3862 Signed-off-by: Ralf Haferkamp <rhafer@suse.com>
Up to now the `config` and `wipe` subcommands called GetConfigFromContext() which is also already called during app startup. Apart from being inefficient this also caused issues with StringSlice values being handled wrong. Closes cri-o#3862 Signed-off-by: Ralf Haferkamp <rhafer@suse.com>
Otherwise the underlying flags in the ctx might be overriden, which can cause issue when the config is reloaded (or GetConfigFromContext() is called more than once). Closes cri-o#3862 Signed-off-by: Ralf Haferkamp <rhafer@suse.com>
This splits out the config file loading and commandline argument processing from GetConfigFromContext() so that subcommand like `crio config' and `crio wipe` are able to fetch the configuration from the context without reloading the config files. Apart from being inefficient this also caused issues with StringSlice values being handled wrong. Closes cri-o#3862 Signed-off-by: Ralf Haferkamp <rhafer@suse.com>
Otherwise the underlying flags in the ctx might be overriden, which can cause issue when the config is reloaded (or GetConfigFromContext() is called more than once). Closes cri-o#3862 Signed-off-by: Ralf Haferkamp <rhafer@suse.com> (cherry picked from commit 35a8caf)
This splits out the config file loading and commandline argument processing from GetConfigFromContext() so that subcommand like `crio config' and `crio wipe` are able to fetch the configuration from the context without reloading the config files. Apart from being inefficient this also caused issues with StringSlice values being handled wrong. Closes cri-o#3862 Signed-off-by: Ralf Haferkamp <rhafer@suse.com> (cherry picked from commit 6b9dfc6)
#3880 backports this to 1.18. Does this issue qualify for backports to even older releases? |
Description
in researching a fix for #3823, I found bug where we always overwrite the first plugin_dir to the values in a config file when using
--cni-plugin-dir
, instead of taking the cli as precedenceThis is all with
plugin_dirs
defined in both /etc/crio/crio.conf and /etc/crio/crio.conf.d/00-default.conf:Describe the results you received:
Describe the results you expected:
Note: we get the correct output if we do:
Output of
crio --version
:though I think I observed it as far back as 1.16
The text was updated successfully, but these errors were encountered: