-
Notifications
You must be signed in to change notification settings - Fork 3
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
Load config from both env and yaml #162
base: main
Are you sure you want to change the base?
Conversation
6d9ce31
to
d6335e5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, please take another look at the Icinga DB PR where I added the default configuration path in the flag description and added code comments to the flags struct on what it actually implements. That should be included here as well.
cmd/icinga-kubernetes/main.go
Outdated
|
||
if err = config.Load(&cfg, config.LoadOptions{ | ||
Flags: glue, | ||
EnvOptions: config.EnvOptions{Prefix: "IFK_"}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although there is (much) more to type, ICINGA_FOR_KUBERNETES_
simply reads much better than IFK
.
cmd/icinga-kubernetes/main.go
Outdated
pflag.StringVar( | ||
&glue.Config, | ||
"config", | ||
daemon.DefaultConfigPath, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A default shouldn't be set, right? Else f.Config != ""
is never false.
93c8e25
to
a5ec519
Compare
pkg/daemon/config.go
Outdated
@@ -7,12 +7,16 @@ import ( | |||
"github.com/icinga/icinga-kubernetes/pkg/notifications" | |||
) | |||
|
|||
// DefaultConfigPath specifies the default location of Icinga DB's config.yml for package installations. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything about this sentence is wrong 😆
pkg/daemon/config.go
Outdated
@@ -7,12 +7,16 @@ import ( | |||
"github.com/icinga/icinga-kubernetes/pkg/notifications" | |||
) | |||
|
|||
// DefaultConfigPath specifies the default location of Icinga DB's config.yml for package installations. | |||
// const DefaultConfigPath = "/etc/icinga-kubernetes/config.yml" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove.
Default gets returned in ConfigFlagGlue.GetConfigPath() if config is not explicitly set.
a8abe6a
to
e919743
Compare
With this PR Icinga for Kubernetes supports configuration loading in three scenarios:
The configuration is loaded via
config.Load()
Requires Icinga/icinga-go-library#87
Closes #160