adding thresholds for alert contacts

[drubin]

@louy I have tried working on this but I got very stuck with adding proper tests :( I don't know how which method to use to validate a "hash"

I am pretty sure this actually sets and updates the recurrence and threshold correctly but I can't validate without fixing the tests.

    testing.go:518: Step 1 error: ImportStateVerify attributes not equivalent. Difference is shown below. Top is actual, bottom is expected.

        (map[string]string) {
        }


        (map[string]string) (len=4) {
         (string) (len=15) "alert_contact.#": (string) (len=1) "1",
         (string) (len=18) "alert_contact.0.id": (string) (len=7) "1234",
         (string) (len=26) "alert_contact.0.recurrence": (string) (len=1) "1",
         (string) (len=25) "alert_contact.0.threshold": (string) (len=1) "8"
        }

I also think my biggest issue is I somehow need to modify the GetMonitor method by adjusting the type Monitor struct { to include the AlertContacts

Any advice?

[louy]

I think the reason it's breaking for you is that the alert contact ID doesn't exist.

can you try adding an alert contact resource to your test?

i.e.

				resource "uptimerobot_alert_contact" "test" {
					...
				}
				resource "uptimerobot_monitor" "test" {
					...
					alert_contact {
						id         = "${uptimerobot_alert_contact.test.id}"
						threshold  = 8
						recurrence = 1
					}
				}

[drubin]

@louy 1234 not the real number was a hard-coded alert contact I have in my account (I was just trying to simplify things)

[louy]

I see. Let me try this locally. However, the error message you mentioned looks like UR rejecting your alert contact id rather than an issue in threshold/recurrence

[drubin]

But this test will only work if you have a paid for account that you are running the tests with on CI (are you?)

[louy]

Sadly no, so I think for the purpose of making the test work for non-paid users (including myself) we can set them to 0

[louy]

Ok I see the problem now. Yeah you're right GetMonitor is missing the return value alert_contact

[louy]

Looking at their API, I think I remember what the issue was now.

The getMonitors api doesn't return alert_contacts, which is why I didn't create a test for this in the past

I think for this one you can skip this step in the test and it should work fine:

			resource.TestStep{
				ResourceName:      "uptimerobot_monitor.test",
				ImportState:       true,
				ImportStateVerify: true,
			},

Please do this in a separate test case (rather than updating the existing http monitor test case) and I'll merge :)

[drubin]

@louy I updated it, not sure why the tests are failing to check out the code?

Also does that I did make sense?

Will terraform try to change the alert_contacts every single time we push a change since it can't read the value from uptimerobot or will it assume it hasn't changed?

[louy]

I'm not sure why the build is breaking either, but I suspect it's an issue with circleci's permission to access github. Maybe try to re-authorise it or something? :)

I don't think TF will try to update this every time, but the issue is rather that TF can't import this field (which is why the ImportVerify test step fails). If we can add a few resource.TestCheckResourceAttr for the alert contact we can confirm this. I'll leave a line comment on this one but otherwise this should be good to go

[louy]

Sorry it's taken me a while btw. Just busy at work as you can imagine

[drubin]

@louy I figured out why it was failing. It was failing because of I setup automatic builds for my fork for #9

I think you have set it not to build fork's because they would have access to your API key.

I have added your suggested changes, tests pass locally.

Let me know if there is anything else to get this merged so we can try it out?

[louy]

Makes sense. Yeah I guess I don't want anyone to be able to get hold of my api key ;)

I've merged this now and it should be released in v0.3.0