Skip to content
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

provider/datadog: Add datadog_downtime resource #10994

Merged
merged 4 commits into from
Mar 14, 2017

Conversation

bflad
Copy link
Contributor

@bflad bflad commented Jan 2, 2017

Hello! This should hopefully get the ball rolling for adding a datadog_downtime resource to the datadog provider, which is used to manage alerting downtimes. References:

This appears to be pretty close for an initial implementation, however I am having some issues working with the Terraform schema and Go (first time writing in Go, so apologies in advance!). I used the existing datadog_monitor as a reference for writing this resource.

Known Working:

  • CRUD operations for downtimes
  • Verify "deletion" in testing by checking Active attribute (Datadog doesn't actually delete the downtime -- it does disappear from the UI in this state though)
  • Unit and acceptance testing passes

Known Issues:

  • The schema for recurrence.week_days should be a validated List of week days -- currently throws * '[week_days]' expected type 'string', got unconvertible type '[]interface {}' when defined in configuration
  • The recurrence state is occasionally unstable for certain attributes (mostly period), but I'm very unsure why (Go pointers?). Here's an example that testing will not break every run, but almost always in same the way:
--- FAIL: TestAccDatadogDowntime_BasicUntilDateRecurrence (1.04s)
	testing.go:265: Step 0 error: After applying this step and refreshing, the plan was not empty:

		DIFF:

		UPDATE: datadog_downtime.foo
		  recurrence.%:          "1" => "3"
		  recurrence.period:     "" => "1"
		  recurrence.until_date: "" => "1736226000"

		STATE:

		datadog_downtime.foo:
		  ID = 207794591
		  active = false
		  disabled = false
		  end = 1735765200
		  message = Example Datadog downtime message.
		  recurrence.% = 1
		  recurrence.type = days
		  scope.# = 1
		  scope.0 = host:UntilDateRecurrence
		  start = 1735707600

Any help or feedback is appreciated, thanks!

"strings"

"github.com/hashicorp/terraform/helper/schema"
"github.com/zorkian/go-datadog-api"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs to be updated to "gopkg.in/zorkian/go-datadog-api.v2"

Optional: true,
Elem: &schema.Schema{
Type: schema.TypeString,
ValidateFunc: validateWeekDay,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional, but validateWeekDay be defined here, see this example: https://github.com/hashicorp/terraform/blob/master/builtin/providers/aws/resource_aws_kinesis_firehose_delivery_stream.go#L76:

	ValidateFunc: func(v interface{}, k string) (ws []string, errors []error) {
		value := v.(string)
		if value != "s3" && value != "redshift" && value != "elasticsearch" {
			errors = append(errors, fmt.Errorf(
				"%q must be one of 's3', 'redshift', 'elasticsearch'", k))
		}
		return
	},

func validateWeekDay(v interface{}, k string) (ws []string, errors []error) {
value := v.(string)

if value != "Mon" && value != "Tue" && value != "Wed" && value != "Thu" && value != "Fri" && value != "Sat" && value != "Sun" {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would use switch here, see https://gobyexample.com/switch

Something like (untested):

	switch value {
	case "Mon", "Tue", "Wed", "Thu", "Fri", "Sat", "Sun":
		errors = append(errors, fmt.Errorf(
		"%q contains an invalid week day parameter %q. Valid parameters are Mon, Tue, Wed, Thu, Fri, Sat or Sun",
		k))
	}
	return

@ojongerius
Copy link
Contributor

"The schema for recurrence.week_days should be a validated List of week days -- currently throws * '[week_days]' expected type 'string', got unconvertible type '[]interface {}' when defined in configuration"

This is thrown while planning during your test, it is triggered by using a list nested in a map I see it in TestAccDatadogDowntime_WeekDayRecurring:

--- FAIL: TestAccDatadogDowntime_WeekDayRecurring (0.29s)
        testing.go:265: Step 0 error: Error planning: 1 error(s) occurred:
                
                * datadog_downtime.foo: 1 error(s) occurred:
                
                * datadog_downtime.foo: recurrence: 1 error(s) decoding:
                
                * '[week_days]' expected type 'string', got unconvertible type '[]interface {}'
=== RUN   TestAccDatadogMonitor_Basic

Which contains:

  recurrence {
    period    = 1
	type      = "weeks"
	week_days = ["Sat", "Sun"]
  }

The error is not in your code, but the schema, schema.TypeList for "week_days" nested in schema.TypeMap ("recurrence"). I think this should work, but it does not, at the moment.

Terraform does not catch this for you, and the error message is not very informative. AFAICS you'll have to work around this, or fix this in Terraform core. Don't have a good solution for you at the moment, but will let you know if I think of something.

You'll see you'll be able to use week_days if you move it up into the top of your resource, but I understand this not a very elegant solution.

@bflad bflad changed the base branch from master to 0-8-stable February 26, 2017 22:56
@bflad
Copy link
Contributor Author

bflad commented Feb 26, 2017

@ojongerius thanks for the feedback! I spent some time with this today and got it working as expected (hopefully the right way 😄).

PR updates:

  • Switch PR base to 0-8-stable
  • Upgrade datadog_dowtime library to v2
  • Switch recurrence schema from TypeMap to TypeList -- this makes the nested TypeList work interestingly enough
  • Add recurrence type validation function
  • Add testing for validation functions

Here's what I'm seeing now:

make test TEST=./builtin/providers/datadog TESTARGS='-run=TestResourceDatadogDowntime'
==> Checking that code complies with gofmt requirements...
==> Checking AWS provider for unchecked errors...
==> NOTE: at this time we only look for uncheck errors in the AWS package
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/02/26 17:54:55 Generated command/internal_plugin_list.go
TF_ACC= go test ./builtin/providers/datadog -run=TestResourceDatadogDowntime -timeout=30s -parallel=4
ok  	github.com/hashicorp/terraform/builtin/providers/datadog	0.014s

make testacc TEST=./builtin/providers/datadog TESTARGS='-run=TestAccDatadogDowntime'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/02/26 17:55:12 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/datadog -v -run=TestAccDatadogDowntime -timeout 120m
=== RUN   TestAccDatadogDowntime_Basic
--- PASS: TestAccDatadogDowntime_Basic (1.93s)
=== RUN   TestAccDatadogDowntime_BasicMultiScope
--- PASS: TestAccDatadogDowntime_BasicMultiScope (1.76s)
=== RUN   TestAccDatadogDowntime_BasicNoRecurrence
--- PASS: TestAccDatadogDowntime_BasicNoRecurrence (1.50s)
=== RUN   TestAccDatadogDowntime_BasicUntilDateRecurrence
--- PASS: TestAccDatadogDowntime_BasicUntilDateRecurrence (1.39s)
=== RUN   TestAccDatadogDowntime_BasicUntilOccurrencesRecurrence
--- PASS: TestAccDatadogDowntime_BasicUntilOccurrencesRecurrence (1.15s)
=== RUN   TestAccDatadogDowntime_WeekDayRecurring
--- PASS: TestAccDatadogDowntime_WeekDayRecurring (1.26s)
=== RUN   TestAccDatadogDowntime_Updated
--- PASS: TestAccDatadogDowntime_Updated (2.47s)
=== RUN   TestAccDatadogDowntime_TrimWhitespace
--- PASS: TestAccDatadogDowntime_TrimWhitespace (1.18s)
PASS
ok  	github.com/hashicorp/terraform/builtin/providers/datadog	12.661s

@bflad bflad changed the title [WIP] provider/datadog: Add datadog_downtime resource provider/datadog: Add datadog_downtime resource Feb 26, 2017
@ojongerius
Copy link
Contributor

Nice work!

Looks like the last hurdle is getting TestDatadogDowntime_import to pass:

▶ make testacc TEST=./builtin/providers/datadog TESTARGS='-run=TestDatadogDowntime'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/02/27 15:22:52 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/datadog -v -run=TestDatadogDowntime -timeout 120m
=== RUN   TestDatadogDowntime_import
--- FAIL: TestDatadogDowntime_import (2.48s)
        testing.go:265: Step 0 error: Error applying: 1 error(s) occurred:
                
                * datadog_downtime.foo: error updating downtime: API error 400 Bad Request: {"errors":["Scheduled downtime start cannot be in the past"]}
FAIL
exit status 1
FAIL    github.com/hashicorp/terraform/builtin/providers/datadog        2.490s
make: *** [testacc] Error 1

const testAccCheckDatadogDowntimeConfigImported = `
resource "datadog_downtime" "foo" {
scope = ["host:X", "host:Y"]
start = 1483308000
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These need to be in the future, get fancy and generate them dynamically or plan further ahead ;)

@bflad
Copy link
Contributor Author

bflad commented Feb 27, 2017

I've adjusted the import test timestamps to be similar to the other tests in the future and its working locally for me:

make testacc TEST=./builtin/providers/datadog TESTARGS='-run=TestDatadogDowntime'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/02/27 08:08:49 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/datadog -v -run=TestDatadogDowntime -timeout 120m
=== RUN   TestDatadogDowntime_import
--- PASS: TestDatadogDowntime_import (1.67s)
PASS
ok  	github.com/hashicorp/terraform/builtin/providers/datadog	1.684s

Please let me know if there are other adjustments you'd like!

Copy link
Contributor

@ojongerius ojongerius left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thanks!

@ojongerius
Copy link
Contributor

▶ make testacc TEST=./builtin/providers/datadog TESTARGS='-run=TestDatadogDowntime'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/02/28 09:11:00 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/datadog -v -run=TestDatadogDowntime -timeout 120m
=== RUN   TestDatadogDowntime_import
--- PASS: TestDatadogDowntime_import (6.64s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/datadog        6.656s

github.com/hashicorp/terraform  datadog_downtime_resource ✔                                                                                                                                                                                                                8h59m  
▶ make testacc TEST=./builtin/providers/datadog TESTARGS='-run=TestAccDatadogDowntime'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/02/28 09:11:19 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/datadog -v -run=TestAccDatadogDowntime -timeout 120m
=== RUN   TestAccDatadogDowntime_Basic
--- PASS: TestAccDatadogDowntime_Basic (5.87s)
=== RUN   TestAccDatadogDowntime_BasicMultiScope
--- PASS: TestAccDatadogDowntime_BasicMultiScope (4.66s)
=== RUN   TestAccDatadogDowntime_BasicNoRecurrence
--- PASS: TestAccDatadogDowntime_BasicNoRecurrence (4.94s)
=== RUN   TestAccDatadogDowntime_BasicUntilDateRecurrence
--- PASS: TestAccDatadogDowntime_BasicUntilDateRecurrence (5.06s)
=== RUN   TestAccDatadogDowntime_BasicUntilOccurrencesRecurrence
--- PASS: TestAccDatadogDowntime_BasicUntilOccurrencesRecurrence (5.19s)
=== RUN   TestAccDatadogDowntime_WeekDayRecurring
--- PASS: TestAccDatadogDowntime_WeekDayRecurring (4.92s)
=== RUN   TestAccDatadogDowntime_Updated
--- PASS: TestAccDatadogDowntime_Updated (10.06s)
=== RUN   TestAccDatadogDowntime_TrimWhitespace
--- PASS: TestAccDatadogDowntime_TrimWhitespace (4.74s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/datadog        45.465s

@stack72
Copy link
Contributor

stack72 commented Mar 14, 2017

LGTM Thanks for all the work here @bflad and for the review @ojongerius

@stack72 stack72 merged commit c360381 into hashicorp:0-8-stable Mar 14, 2017
stack72 pushed a commit that referenced this pull request Mar 14, 2017
* provider/datadog: Initial datadog_downtime resource

* provider/datadog: Update datadog_downtime resource to v2 library, fix recurrence handling

* provider/datadog: Fix datadog_downtime import test
@ghost
Copy link

ghost commented Apr 15, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants