Add resource for monitor alerts #679
Conversation
@andrewsomething @danaelhe My first goal is to be able to create a monitor alert with the compiled module, and then work more on the acceptance/unit testing.
This is looking great! Offered some pointers inline including for the expandAlerts.
atombrella
force-pushed
the
droplet_monitoring_gh_479
branch
from
d6be4e7
to
1b376db
Compare
|
I enabled the new resource in provider.go, and you can now run the acceptance tests (I'll add more). This one fails. I doubt I'll have much time to work on this during next week. |
|
It actually creates an alert for a droplet, with one of the acceptance tests. There is a stack trace that's somewhat tricky for me to debug. Help would be appreciated. |
@atombrella When I ran the tests, I received the following panic:
panic: alerts.0.email: '': source data must be an array or slice, got struct
It looks like your last commit tries to address what I initially thought was the problem. Unfortunately, I still receive the above error after fixing that.
The problem is with how the flatten functions handle the email and slack data. I've added some suggestions inline to address this and the test cases.
I also added a comment regarding the alerts property set as a schema.TypeList for some discussion.
|
@scotchneat @danaelhe @andrewsomething Thanks for the assistance. I do have one testing regarding tests: I would have liked to include some tests for validation, e.g., window not in [5m, 10m, 30m, 1h]. However, it doesn't seem like the provider has anything else but acceptance tests (which all require network connections, and a token), so I suppose this is not essential. I think it's approaching a stable state :) |
|
My branch is open for edits/commits from maintainers. |
Co-authored-by: Cesar Garza <[email protected]>
atombrella
force-pushed
the
droplet_monitoring_gh_479
branch
from
d53a1eb
to
a96ed26
Compare
| @@ -31,7 +31,8 @@ The following arguments are supported: | |||
| * `backups` - (Optional) Boolean controlling if backups are made. Defaults to | |||
| false. | |||
| * `monitoring` - (Optional) Boolean controlling whether monitoring agent is installed. | |||
| Defaults to false. | |||
| Defaults to false. If set to `true`, you can configure monitor alert policies | |||
| [monitor alert resource](/providers/digitalocean/digitalocean/latest/docs/resources/monitor_alert) | |||
I think we're probably good to go here, but wouldn't mind a second look
Thanks for another great contribution @atombrella!
I'm glad you saw the comment in the code about those constants :)
Great. I appreciate the help and coaching. A colleague's first reaction to me highlighting the alert monitor feature for droplets was "Can I do it in Terraform?". Now they will be. |
:LGTM:
Thanks for the contribution @atombrella, and the collaboration!
I just had one question about a couple of tests but it's not a blocker.
|
@scotchneat I deleted the duplicated test, and thought it'd be nice to test having two slack channels connected. Please see 65e9d77 |
|
@atombrella Good call. |
* [skip ci] Alert policy resource WIP * [skip ci] More work * [skip ci] read and create functions * [skip ci] flatten/expand functions * More work * flatten/expand work * Add template for testing. WIP * Feedback from @andrewsomething * Temlate for testing * flatten function, and documentation * enable support for monitor_alert in provider.go * add Enabled to create request * flatten/expand functions * Rename resourceDigitalMonitorAlert to resourceDigitalOceanMonitorAlert * creating alerts :) * import acceptance tests * template for update test * added CheckDestroy to acceptance tests * Update digitalocean/resource_digitalocean_monitor_alert.go Co-authored-by: Cesar Garza <[email protected]> * Update digitalocean/resource_digitalocean_monitor_alert_test.go Co-authored-by: Cesar Garza <[email protected]> * Update digitalocean/resource_digitalocean_monitor_alert_test.go Co-authored-by: Cesar Garza <[email protected]> * Update digitalocean/resource_digitalocean_monitor_alert.go Co-authored-by: Cesar Garza <[email protected]> * Update digitalocean/resource_digitalocean_monitor_alert.go Co-authored-by: Cesar Garza <[email protected]> * Update digitalocean/resource_digitalocean_monitor_alert.go Co-authored-by: Cesar Garza <[email protected]> * Update digitalocean/resource_digitalocean_monitor_alert_test.go Co-authored-by: Cesar Garza <[email protected]> * Update digitalocean/resource_digitalocean_monitor_alert_test.go Co-authored-by: Cesar Garza <[email protected]> * Update digitalocean/resource_digitalocean_monitor_alert_test.go Co-authored-by: Cesar Garza <[email protected]> * Make destroy method for tests * Support alerts using only tags. * Fix import test. * Update godo to pick up new consts. * Use consts for all alert types. * Support full updates. * Add test with two Slack channels Co-authored-by: Cesar Garza <[email protected]> Co-authored-by: Andrew Starr-Bochicchio <[email protected]> Co-authored-by: Andrew Starr-Bochicchio <[email protected]>
It still needs a lot of work, but at least it's a start :) Hoping for a bit of help to get it pushed over the finish line. I'll start also on unit/acceptance tests.
#479