This repository was archived by the owner on Nov 6, 2024. It is now read-only.
Fix mail notifications- update paths constructed from ConfigDir to match the v2.10 changes#336
Open
hfinucane wants to merge 1 commit intoIcinga:masterfrom
Open
Fix mail notifications- update paths constructed from ConfigDir to match the v2.10 changes#336hfinucane wants to merge 1 commit intoIcinga:masterfrom
hfinucane wants to merge 1 commit intoIcinga:masterfrom
Conversation
I managed to really bobble this- see [the documentation](https://github.com/Icinga/icinga2/blob/master/doc/16-upgrading-icinga-2.md#path-constant-changes-) for the warning about the new ConfigDir variable. Anything that uses it needs to be changed to be relative to SysconfDir+"/icinga2/"- ``` object NotificationCommand "mail-service-notification" { - command = [ SysconfDir + "/icinga2/scripts/mail-service-notification.sh" ] + command = [ ConfigDir + "/scripts/mail-service-notification.sh" ] ``` being the example diff. This will finish the transition in this cookbook. Without this, if you use the examples, you will get errors in the logs that look like ``` terminated with exit code 128, output: execvpe(/etc/icinga2/icinga2/scripts/mail-service-notification.sh) failed: No such file or directory ``` and notifications will not work.
dnsmichi
reviewed
Apr 9, 2019
| ca_path 'ConfigDir + "/icinga2/pki/ca.crt"' | ||
| cert_path 'ConfigDir + "/pki/" + NodeName + ".crt"' | ||
| key_path 'ConfigDir + "/pki/" + NodeName + ".key"' | ||
| ca_path 'ConfigDir + "/pki/ca.crt"' |
Contributor
There was a problem hiding this comment.
These paths have been deprecated in 2.8. While at it, think about a possible change here to /var/lib... as well.
Contributor
Author
There was a problem hiding this comment.
https://github.com/Icinga/icinga2/blob/master/doc/16-upgrading-icinga-2.md#upgrading-to-v28- oh shoot.
Icinga 2 automatically migrates the certificates to the new default location if they are configured and detected in /etc/icinga2/pki.
makes it sound like Icinga is papering over that under the covers, at least. I kind of want to address that in a separate PR.
Contributor
There was a problem hiding this comment.
It has been sitting there, but likely not forever. Just wanted to mention it, thanks for taking care :)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I managed to really bobble this in #332- see the documentation for the warning about the new ConfigDir variable. Anything that uses it needs to be changed to be relative to SysconfDir+"/icinga2/"-
being the example diff. This will finish the transition in this cookbook.
Without this, if you use the examples, you will get errors in the logs that look like
and notifications will not work.