-
Notifications
You must be signed in to change notification settings - Fork 2
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
Fix documentation #94
Conversation
doc/04-Kubeconfig.md
Outdated
|
||
|
||
```shell | ||
export KUBECONFIG=/path/to/your/kubeconfig |
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.
I do appreciate the much needed updates to the documentation.
However, this cannot be the production solution right?
First of all, this needs to mention for which user to set the variable.
If I install icinga-kubernetes from the Linux packages this variable needs to be set for the user the service starts with (e.g. User=icinga-kubernetes
).
In that case it's probably best to set this in the systemd unit like so:
[Service]
Environment="KUBECONFIG=/etc/icinga-kubernetes/kubeconfig.yml"
and then the documentation should tell users to create the file. There are probably other solutions, like using /etc/default/
on Debian systems (not sure if the directory should still be used)
Alternatively, why not have an option in /etc/icinga-kubernetes/config.yml
? Or have a specific file like /etc/icinga-kubernetes/kubeconfig.yml
, then the daemon could even check the file for changes and hot reload the file.
Is there a benefit of having it as an env variable?
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.
Is there a benefit of having it as an env variable?
Since we are using the same libraries as kubectl
, accessing the Kubernetes API should work the same way. I don't think there is any advantage in setting up anything else. Some important CLI options like --cluster
or --context
we do not provide yet. @Jan-Schuppik If I'm not mistaken, we already had code for this in sample projects some time ago. Also for filtering namespaces directly from the CLI, but that's something I wouldn't support at the moment. Could you please take care of the additional CLI options and the adjustments in the documentation? You can take a look at how other projects do this, e.g. https://github.com/vladimirvivien/ktop. Or #58. If @martialblog has time, don't hesitate to ask for feedback.
c0807c5
to
d86f1be
Compare
Fix location of schema.sql and required versions of Databases.
refs #66
fixes #71