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

Properly handle recurring downtimes definitions #1092

Merged
merged 13 commits into from
Jun 17, 2021
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 4 additions & 8 deletions GNUmakefile
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@ uninstall:
@rm -vf $(DIR)/terraform-provider-datadog

# Run unit tests; these tests don't interact with the API and don't support/need RECORD
test: get-test-deps fmtcheck lint
test: get-test-deps fmtcheck
gotestsum --hide-summary skipped --format testname --debug --packages $(TEST) -- $(TESTARGS) -timeout=30s

# Run acceptance tests (this runs integration CRUD tests through the terraform test framework)
testacc: get-test-deps fmtcheck lint
testacc: get-test-deps fmtcheck
Copy link
Contributor

Choose a reason for hiding this comment

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

I saw there was a slack conversation about this, @therve can you just confirm this is what the recommendation was?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes we'll come back to fix it later.

RECORD=$(RECORD) TF_ACC=1 gotestsum --format testname --debug --rerun-fails --packages ./... -- -v $(TESTARGS) -timeout 120m

# Run both unit and acceptance tests
Expand All @@ -42,9 +42,6 @@ vet:
exit 1; \
fi

lint: get-test-deps
golint -set_exit_status ./...

fmt:
goimports -format-only -local $(LOCAL_PACKAGE) -w $(GOIMPORTS_FILES)
terraform fmt -recursive examples
Expand All @@ -71,7 +68,6 @@ update-go-client:

get-test-deps:
gotestsum --version || (cd `mktemp -d`; GO111MODULE=off GOFLAGS='' go get -u gotest.tools/gotestsum; cd -)
which golint || (cd `mktemp -d`; GO111MODULE=off GOFLAGS='' go get -u golang.org/x/lint/golint; cd -)

license-check:
@sh -c "'$(CURDIR)/scripts/license-check.sh'"
Expand All @@ -84,9 +80,9 @@ docs: tools

check-docs: docs
@if [ "`git status --porcelain docs/`" ]; then \
echo "Uncommitted changes were detected in the autogenerated docs folder. Please run 'make docs' to autogenerate the docs, and commit the changes" && echo `git status --porcelain docs/` && exit 1; \
echo "Uncommitted changes were detected in the autogenerated docs folder. Please run 'make docs' to autogenerate the docs, and commit the changes" && echo `git status --porcelain docs/` && exit 1; \
else \
echo "Success: No generated documentation changes detected"; \
echo "Success: No generated documentation changes detected"; \
fi

.PHONY: build check-docs docs test testall testacc cassettes vet fmt fmtcheck errcheck test-compile tools get-test-deps license-check
172 changes: 155 additions & 17 deletions datadog/resource_datadog_downtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package datadog

import (
"context"
"fmt"
"log"
"reflect"
"strconv"
Expand All @@ -20,6 +21,85 @@ import (
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
)

type downtimeOrDowntimeChild interface {
GetId() int64
GetActive() bool
GetDisabled() bool
GetMessage() string
GetMonitorIdOk() (*int64, bool)
GetTimezone() string
GetRecurrenceOk() (*datadogV1.DowntimeRecurrence, bool)
GetMonitorTags() []string
GetStart() int64
GetEnd() int64
GetScope() []string
GetActiveChild() datadogV1.DowntimeChild
GetActiveChildOk() (*datadogV1.DowntimeChild, bool)
GetCanceledOk() (*int64, bool)
}

// downtimeChild wraps the `datadogV1.DowntimeChild` struct via embedding to implement `downtimeOrDowntimeChild`
// interface missing the `GetActiveChild`, `GetActiveChildOk` methods.
type downtimeChild struct {
child *datadogV1.DowntimeChild
}

func (d *downtimeChild) GetId() int64 {
armcburney marked this conversation as resolved.
Show resolved Hide resolved
return d.child.GetId()
}

func (d *downtimeChild) GetActive() bool {
return d.child.GetActive()
}

func (d *downtimeChild) GetDisabled() bool {
return d.child.GetDisabled()
}

func (d *downtimeChild) GetMessage() string {
return d.child.GetMessage()
}

func (d *downtimeChild) GetMonitorIdOk() (*int64, bool) {
return d.child.GetMonitorIdOk()
}

func (d *downtimeChild) GetTimezone() string {
return d.child.GetTimezone()
}

func (d *downtimeChild) GetRecurrenceOk() (*datadogV1.DowntimeRecurrence, bool) {
return d.child.GetRecurrenceOk()
}

func (d *downtimeChild) GetMonitorTags() []string {
return d.child.GetMonitorTags()
}

func (d *downtimeChild) GetStart() int64 {
return d.child.GetStart()
}

func (d *downtimeChild) GetEnd() int64 {
return d.child.GetEnd()
}

func (d *downtimeChild) GetScope() []string {
return d.child.GetScope()
}

func (d *downtimeChild) GetActiveChild() datadogV1.DowntimeChild {
return datadogV1.DowntimeChild{}
}

func (d *downtimeChild) GetActiveChildOk() (*datadogV1.DowntimeChild, bool) {
return nil, false
}

func (d *downtimeChild) GetCanceledOk() (*int64, bool) {
return d.child.GetCanceledOk()
}

func resourceDatadogDowntime() *schema.Resource {
return &schema.Resource{
Description: "Provides a Datadog downtime resource. This can be used to create and manage Datadog downtimes.",
Expand All @@ -30,7 +110,6 @@ func resourceDatadogDowntime() *schema.Resource {
Importer: &schema.ResourceImporter{
StateContext: schema.ImportStatePassthroughContext,
},

Schema: map[string]*schema.Schema{
"active": {
Type: schema.TypeBool,
Expand All @@ -48,6 +127,7 @@ func resourceDatadogDowntime() *schema.Resource {
DiffSuppressFunc: func(k, oldVal, newVal string, d *schema.ResourceData) bool {
_, startDatePresent := d.GetOk("start_date")
now := time.Now().Unix()

// If "start_date" is set, ignore diff for "start". If "start" isn't set, ignore diff if start is now or in the past
return startDatePresent || (newVal == "0" && oldVal != "0" && int64(d.Get("start").(int)) <= now)
},
Expand Down Expand Up @@ -167,6 +247,12 @@ func resourceDatadogDowntime() *schema.Resource {
ConflictsWith: []string{"monitor_id"},
Elem: &schema.Schema{Type: schema.TypeString},
},
"active_child_id": {
Type: schema.TypeInt,
Computed: true,
Optional: true,
Description: "The id corresponding to the downtime object definition of the active child for the original parent recurring downtime. This field will only exist on recurring downtimes.",
},
},
}
}
Expand Down Expand Up @@ -227,8 +313,9 @@ func buildDowntimeStruct(ctx context.Context, d *schema.ResourceData, client *da
var dt datadogV1.Downtime
var currentStart = *datadogV1.PtrInt64(0)
var currentEnd = *datadogV1.PtrInt64(0)

if updating {
id, err := strconv.ParseInt(d.Id(), 10, 64)
id, err := getID(d)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -320,7 +407,7 @@ func resourceDatadogDowntimeCreate(ctx context.Context, d *schema.ResourceData,

d.SetId(strconv.Itoa(int(dt.GetId())))

return updateDowntimeState(d, &dt)
return updateDowntimeState(d, &dt, true)
}

func resourceDatadogDowntimeRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
Expand All @@ -342,15 +429,28 @@ func resourceDatadogDowntimeRead(ctx context.Context, d *schema.ResourceData, me
return utils.TranslateClientErrorDiag(err, "error getting downtime")
}

// Hack for recurring downtimes, compare the downtime definition in state with the most recent recurring child
// downtime definition returned by the API. Fields which change on each subsequent reschedule will not be compared
// (i.e., start and end), but will be mutated if the terraform resource definition changes (since we update the active
// child downtime we keep a reference to in the terraform state).
if activeChild, ok := dt.GetActiveChildOk(); ok && activeChild != nil {
child := &downtimeChild{activeChild}
if canceled, ok := child.GetCanceledOk(); ok && canceled != nil {
d.SetId("")
return nil
}

return updateDowntimeState(d, child, false)
}

if canceled, ok := dt.GetCanceledOk(); ok && canceled != nil {
d.SetId("")
return nil
}

return updateDowntimeState(d, &dt)
return updateDowntimeState(d, &dt, true)
}

func updateDowntimeState(d *schema.ResourceData, dt *datadogV1.Downtime) diag.Diagnostics {
func updateDowntimeState(d *schema.ResourceData, dt downtimeOrDowntimeChild, updateBounds bool) diag.Diagnostics {
log.Printf("[DEBUG] downtime: %v", dt)

if err := d.Set("active", dt.GetActive()); err != nil {
Expand All @@ -359,9 +459,6 @@ func updateDowntimeState(d *schema.ResourceData, dt *datadogV1.Downtime) diag.Di
if err := d.Set("disabled", dt.GetDisabled()); err != nil {
return diag.FromErr(err)
}
if err := d.Set("end", dt.GetEnd()); err != nil {
return diag.FromErr(err)
}
if err := d.Set("message", dt.GetMessage()); err != nil {
return diag.FromErr(err)
}
Expand Down Expand Up @@ -403,7 +500,7 @@ func updateDowntimeState(d *schema.ResourceData, dt *datadogV1.Downtime) diag.Di
return diag.FromErr(err)
}
}
if err := d.Set("scope", dt.Scope); err != nil {
if err := d.Set("scope", dt.GetScope()); err != nil {
return diag.FromErr(err)
}
// See the comment for monitor_tags in the schema definition above
Expand All @@ -412,8 +509,30 @@ func updateDowntimeState(d *schema.ResourceData, dt *datadogV1.Downtime) diag.Di
return diag.FromErr(err)
}
}
if err := d.Set("start", dt.GetStart()); err != nil {
return diag.FromErr(err)

// Don't set the `start`, `end` stored in terraform unless in specific cases for recurring downtimes.
if updateBounds {
if err := d.Set("start", dt.GetStart()); err != nil {
return diag.FromErr(err)
}
if err := d.Set("end", dt.GetEnd()); err != nil {
return diag.FromErr(err)
}
}

switch dt.(type) {
case *datadogV1.Downtime:
if attr, ok := dt.GetActiveChildOk(); ok {
if err := d.Set("active_child_id", attr.GetId()); err != nil {
return diag.FromErr(err)
}
}
case *downtimeChild:
if err := d.Set("active_child_id", dt.GetId()); err != nil {
phillip-dd marked this conversation as resolved.
Show resolved Hide resolved
return diag.FromErr(err)
}
default:
return diag.FromErr(fmt.Errorf("unsupported interface passed into updateDowntimeState"))
}
return nil
}
Expand All @@ -427,10 +546,12 @@ func resourceDatadogDowntimeUpdate(ctx context.Context, d *schema.ResourceData,
if err != nil {
return diag.Errorf("failed to parse resource configuration: %s", err.Error())
}
id, err := strconv.ParseInt(d.Id(), 10, 64)

id, err := getID(d)
if err != nil {
return diag.FromErr(err)
}

// above downtimeStruct returns nil if downtime is not set. Hence, if we are handling the cases where downtime
// is replaced, the ID of the downtime will be set to 0.
dt.SetId(id)
Expand All @@ -439,18 +560,23 @@ func resourceDatadogDowntimeUpdate(ctx context.Context, d *schema.ResourceData,
if err != nil {
return utils.TranslateClientErrorDiag(err, "error updating downtime")
}
// handle the case when a downtime is replaced
d.SetId(strconv.FormatInt(dt.GetId(), 10))

return updateDowntimeState(d, &updatedDowntime)
// Handle the case when a downtime is replaced. Don't set it if the `active_child_id` is set as we want to maintain
// a reference to the original parent downtime ID.
_, ok := d.GetOk("active_child_id")
if !ok {
d.SetId(strconv.FormatInt(dt.GetId(), 10))
}

return updateDowntimeState(d, &updatedDowntime, !ok)
}

func resourceDatadogDowntimeDelete(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
providerConf := meta.(*ProviderConfiguration)
datadogClientV1 := providerConf.DatadogClientV1
authV1 := providerConf.AuthV1

id, err := strconv.ParseInt(d.Id(), 10, 64)
id, err := getID(d)
if err != nil {
return diag.FromErr(err)
}
Expand All @@ -461,3 +587,15 @@ func resourceDatadogDowntimeDelete(ctx context.Context, d *schema.ResourceData,

return nil
}

func getID(d *schema.ResourceData) (int64, error) {
id, err := strconv.ParseInt(d.Id(), 10, 64)
if err != nil {
return 0, err
}

if activeChildID, ok := d.GetOk("active_child_id"); ok {
id = int64(activeChildID.(int))
}
return id, nil
}
1 change: 1 addition & 0 deletions docs/resources/downtime.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ resource "datadog_downtime" "foo" {

### Optional

- **active_child_id** (Number) The id corresponding to the downtime object definition of the active child for the original parent recurring downtime. This field will only exist on recurring downtimes.
Copy link
Contributor

Choose a reason for hiding this comment

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

if we include this, I think it should be actually read only.

Copy link
Contributor

Choose a reason for hiding this comment

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

Bumping on this, is this part auto generated or a copy/paste?

Copy link
Member

Choose a reason for hiding this comment

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

The doc is auto generated. But to avoid this, we should remove the line below since the attribute is read only:

- **end** (Number) Optionally specify an end date when this downtime should expire
- **end_date** (String) String representing date and time to end the downtime in RFC3339 format.
- **message** (String) An optional message to provide when creating the downtime, can include notification handles
Expand Down