Add resource for monitor alerts #679

atombrella · 2021-09-01T09:04:56Z

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.

Have Terraform create the alerts
Documentation. There is an example, attribute reference.
import acceptance test
More acceptance and uni test

#479

atombrella

@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.

digitalocean/resource_digitalocean_monitor_alert.go

andrewsomething

This is looking great! Offered some pointers inline including for the expandAlerts.

digitalocean/resource_digitalocean_monitor_alert.go

atombrella · 2021-09-05T21:15:05Z

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.

make testacc TESTARGS='-run=TestAccDigitalOceanMonitorAlertNoAlerts'

atombrella · 2021-09-09T14:54:44Z

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.

scotchneat

@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.

digitalocean/resource_digitalocean_monitor_alert.go

digitalocean/resource_digitalocean_monitor_alert_test.go

digitalocean/resource_digitalocean_monitor_alert.go

docs/resources/monitor_alert.md

atombrella · 2021-09-14T13:53:14Z

@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 :)

atombrella · 2021-09-16T09:00:43Z

My branch is open for edits/commits from maintainers.

Co-authored-by: Cesar Garza <[email protected]>

digitalocean/import_digitalocean_monitor_alert_test.go

andrewsomething · 2021-09-21T15:36:37Z

@atombrella In addition to fixing the import test (f69c6d6), I pushed a few additional commits:

Support alerts using only tags (48ca9ce).
Update godo to pick up new consts (9eb4610) and use them for all alert types (67f3812).
Support updating all attributes (96cc08c).

andrewsomething · 2021-09-21T15:39:30Z

docs/resources/droplet.md

@@ -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)


👍 Good call out!

andrewsomething

I think we're probably good to go here, but wouldn't mind a second look 👀 @scotchneat

Thanks for another great contribution @atombrella!

atombrella · 2021-09-22T07:40:45Z

@atombrella In addition to fixing the import test (f69c6d6), I pushed a few additional commits:

Support alerts using only tags (48ca9ce).

Update godo to pick up new consts (9eb4610) and use them for all alert types (67f3812).

I'm glad you saw the comment in the code about those constants :)

Support updating all attributes (96cc08c).

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.

scotchneat

: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.

digitalocean/resource_digitalocean_monitor_alert_test.go

atombrella · 2021-09-22T18:57:15Z

@scotchneat I deleted the duplicated test, and thought it'd be nice to test having two slack channels connected. Please see 65e9d77

andrewsomething · 2021-09-22T19:56:34Z

@atombrella Good call.

andrewsomething

@andrewsomething

* [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]>

atombrella changed the title ~~Droplet monitoring gh 479~~ Add resource for monitor alerts Sep 1, 2021

atombrella marked this pull request as draft September 1, 2021 09:11

atombrella commented Sep 2, 2021

View reviewed changes

digitalocean/resource_digitalocean_monitor_alert.go Outdated Show resolved Hide resolved

danaelhe reviewed Sep 2, 2021

View reviewed changes

digitalocean/resource_digitalocean_monitor_alert.go Outdated Show resolved Hide resolved

danaelhe reviewed Sep 2, 2021

View reviewed changes

digitalocean/resource_digitalocean_monitor_alert.go Outdated Show resolved Hide resolved

andrewsomething requested changes Sep 2, 2021

View reviewed changes

atombrella force-pushed the droplet_monitoring_gh_479 branch from d6be4e7 to 1b376db Compare September 5, 2021 21:07

danaelhe mentioned this pull request Sep 8, 2021

Add alerts to digitalocean_app #683

Open

scotchneat suggested changes Sep 13, 2021

View reviewed changes

atombrella marked this pull request as ready for review September 15, 2021 11:09

atombrella added 16 commits September 20, 2021 11:05

[skip ci] Alert policy resource WIP

9be8771

[skip ci] More work

ff46942

[skip ci] read and create functions

360a73c

[skip ci] flatten/expand functions

ad33345

More work

c908152

flatten/expand work

f53591d

Add template for testing. WIP

f3fafbb

Feedback from @andrewsomething

7094ce5

Temlate for testing

79e4181

flatten function, and documentation

7498f43

enable support for monitor_alert in provider.go

28a16f3

add Enabled to create request

958dbc8

flatten/expand functions

78e9792

Rename resourceDigitalMonitorAlert to resourceDigitalOceanMonitorAlert

23e92bc

creating alerts :)

a08f535

import acceptance tests

cc83950

atombrella and others added 2 commits September 20, 2021 11:05

Update digitalocean/resource_digitalocean_monitor_alert_test.go

945aa26

Co-authored-by: Cesar Garza <[email protected]>

Make destroy method for tests

a96ed26

atombrella force-pushed the droplet_monitoring_gh_479 branch from d53a1eb to a96ed26 Compare September 20, 2021 09:05

scotchneat added 2 commits September 20, 2021 16:23

Merge branch 'main' into droplet_monitoring_gh_479

d430a7a

Merge branch 'main' into droplet_monitoring_gh_479

86b14d0

atombrella commented Sep 21, 2021

View reviewed changes

digitalocean/import_digitalocean_monitor_alert_test.go Show resolved Hide resolved

andrewsomething mentioned this pull request Sep 21, 2021

Prep v1.66.0 release. digitalocean/godo#476

Merged

andrewsomething added 4 commits September 21, 2021 10:52

Support alerts using only tags.

48ca9ce

Fix import test.

f69c6d6

Update godo to pick up new consts.

9eb4610

Use consts for all alert types.

67f3812

andrewsomething requested review from scotchneat and andrewsomething September 21, 2021 15:07

andrewsomething and others added 2 commits September 21, 2021 11:33

Support full updates.

96cc08c

Merge branch 'main' into droplet_monitoring_gh_479

ca02eaa

andrewsomething reviewed Sep 21, 2021

View reviewed changes

andrewsomething previously approved these changes Sep 21, 2021

View reviewed changes

scotchneat previously approved these changes Sep 22, 2021

View reviewed changes

digitalocean/resource_digitalocean_monitor_alert_test.go Outdated Show resolved Hide resolved

Add test with two Slack channels

65e9d77

atombrella dismissed stale reviews from scotchneat and andrewsomething via 65e9d77 September 22, 2021 18:55

andrewsomething approved these changes Sep 22, 2021

View reviewed changes

andrewsomething merged commit a9c5474 into digitalocean:main Sep 22, 2021

atombrella deleted the droplet_monitoring_gh_479 branch September 22, 2021 20:15

jessedobbelaere mentioned this pull request Sep 28, 2021

docs: Fix typo in digitalocean monitoring #710

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add resource for monitor alerts #679

Add resource for monitor alerts #679

atombrella commented Sep 1, 2021 •

edited

Loading

atombrella left a comment

andrewsomething left a comment

atombrella commented Sep 5, 2021

atombrella commented Sep 9, 2021

scotchneat left a comment

atombrella commented Sep 14, 2021

atombrella commented Sep 16, 2021

andrewsomething commented Sep 21, 2021

andrewsomething Sep 21, 2021

andrewsomething left a comment

atombrella commented Sep 22, 2021

scotchneat left a comment •

edited

Loading

atombrella commented Sep 22, 2021

andrewsomething commented Sep 22, 2021

andrewsomething left a comment

Add resource for monitor alerts #679

Add resource for monitor alerts #679

Conversation

atombrella commented Sep 1, 2021 • edited Loading

atombrella left a comment

Choose a reason for hiding this comment

andrewsomething left a comment

Choose a reason for hiding this comment

atombrella commented Sep 5, 2021

atombrella commented Sep 9, 2021

scotchneat left a comment

Choose a reason for hiding this comment

atombrella commented Sep 14, 2021

atombrella commented Sep 16, 2021

andrewsomething commented Sep 21, 2021

andrewsomething Sep 21, 2021

Choose a reason for hiding this comment

andrewsomething left a comment

Choose a reason for hiding this comment

atombrella commented Sep 22, 2021

scotchneat left a comment • edited Loading

Choose a reason for hiding this comment

atombrella commented Sep 22, 2021

andrewsomething commented Sep 22, 2021

andrewsomething left a comment

Choose a reason for hiding this comment

atombrella commented Sep 1, 2021 •

edited

Loading

scotchneat left a comment •

edited

Loading