search

Icinga / terraform-provider-icinga2

Terraform Icinga2 provider
https://www.terraform.io/docs/providers/icinga2/
Mozilla Public License 2.0
21 stars 20 forks source link

Notifications implementation #10

Closed micwiel closed 6 years ago

micwiel commented 6 years ago

Hi, I need configurable notifications for icinga2 host and service objects. This PR tries to implement it in terraform.

lrsmith commented 6 years ago

@micwiel I made a few comments, mainly around groups being optional rather than required. https://github.com/lrsmith/go-icinga2-api should support that now as of v0.2.x. The only other thing I would suggest is vendoring go-igicinga2-api to v0.3.x which has the groups support and the notifications. Thanks for the commits/enhancements to both.

micwiel commented 6 years ago

Pushed fixed code. Please review once again.

lrsmith commented 6 years ago

@micwiel I merged in two older commits which has caused a conflict. Would you be mind rebasing your PR to get the updates. The vendor/vendor.json is due to updating the version of go-icinga2-api and it should be fine to use the newer version. icigana2/resource_icinga2_service_test.go is probably due to some new tests. Thanks

lrsmith commented 6 years ago

@micwiel The commit is failing due to 'TestAccCreateGroupHost redeclared' in resources_icinga2_host_test.go The commit to add groups to hosts and make them optional had this function declared.

If you could rebase and remove the duplicate TestAccCreateGroupHost at the end of the file that should take care of the duplicate func.

Could you also add documentation on the two new resources underneath the website/ directory.

lrsmith commented 6 years ago

@micwiel I went branched off your changes and made the above changes in https://github.com/terraform-providers/terraform-provider-icinga2/pull/15 The commit has your changes and mine. I am going to close out this PR and merge in 15. Thank you again for the patch.