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

Adding PagerDuty provider #9022

Merged
merged 26 commits into from
Oct 24, 2016
Merged

Adding PagerDuty provider #9022

merged 26 commits into from
Oct 24, 2016

Conversation

heimweh
Copy link
Contributor

@heimweh heimweh commented Sep 23, 2016

This is my first contribution and my first time writing Go,
and I will most likely need some handholding during this process, but here goes.

Due to the missing PagerDuty support I decided to take a look at writing a provider for it.

It currently supports:

provider "pagerduty" {}

resource "pagerduty_team" "foo" {
  name        = "Engineering"
  description = "All engineering"
}

resource "pagerduty_user" "foo" {
  name  = "Earline Greenholt"
  email = "125.greenholt.earline@graham.name"
  teams = ["${pagerduty_team.foo.id}"]
}

resource "pagerduty_escalation_policy" "foo" {
  name      = "Engineering Escalation Policy"
  num_loops = 2
  teams     = ["${pagerduty_team.foo.id}"]

  rule {
    escalation_delay_in_minutes = 10

    target {
      id = "${pagerduty_user.foo.id}"
    }
  }
}

resource "pagerduty_schedule" "foo" {
  name      = "foo"
  time_zone = "Europe/Berlin"

  layer {
    name                         = "Primary Schedule Layer"
    start                        = "2015-11-06T20:00:00-05:00"
    rotation_virtual_start       = "2015-11-06T20:00:00-05:00"
    rotation_turn_length_seconds = 86400
    users                        = ["${pagerduty_user.foo.id}"]

    restriction {
      type              = "daily_restriction"
      start_time_of_day = "08:00:00"
      duration_seconds  = 32400
    }
  }
}

resource "pagerduty_service" "foo" {
  name                    = "My Web App"
  auto_resolve_timeout    = 14400
  acknowledgement_timeout = 600
  escalation_policy       = "${pagerduty_escalation_policy.foo.id}"
}

data "pagerduty_vendor" "datadog" {
  name_regex = "Datadog"
}

resource "pagerduty_service_integration" "foo" {
  name    = "${data.pagerduty_vendor.datadog.name}"
  type    = "${data.pagerduty_vendor.datadog.type}"
  vendor  = "${data.pagerduty_vendor.datadog.id}"
  service = "${pagerduty_service.foo.id}"
}

I've been testing it a lot in the past couple of days and the basic functionality is there, but as you might notice my knowledge of Go is currently somewhat limited.

$ make testacc TEST=./builtin/providers/pagerduty/
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2016/10/24 14:21:53 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/pagerduty -v  -timeout 120m
=== RUN   TestAccPagerDutyVendor_Basic
--- PASS: TestAccPagerDutyVendor_Basic (9.57s)
=== RUN   TestAccPagerDutyEscalationPolicy_import
--- PASS: TestAccPagerDutyEscalationPolicy_import (7.86s)
=== RUN   TestAccPagerDutySchedule_import
--- PASS: TestAccPagerDutySchedule_import (9.37s)
=== RUN   TestAccPagerDutyService_import
--- PASS: TestAccPagerDutyService_import (9.74s)
=== RUN   TestAccPagerDutyTeam_import
--- PASS: TestAccPagerDutyTeam_import (2.71s)
=== RUN   TestAccPagerDutyUser_import
--- PASS: TestAccPagerDutyUser_import (4.30s)
=== RUN   TestProvider
--- PASS: TestProvider (0.00s)
=== RUN   TestProviderImpl
--- PASS: TestProviderImpl (0.00s)
=== RUN   TestAccPagerDutyEscalationPolicy_Basic
--- PASS: TestAccPagerDutyEscalationPolicy_Basic (9.74s)
=== RUN   TestAccPagerDutyEscalationPolicyWithTeams_Basic
--- PASS: TestAccPagerDutyEscalationPolicyWithTeams_Basic (11.60s)
=== RUN   TestAccPagerDutySchedule_Basic
--- PASS: TestAccPagerDutySchedule_Basic (8.98s)
=== RUN   TestAccPagerDutySchedule_Multi
--- PASS: TestAccPagerDutySchedule_Multi (7.15s)
=== RUN   TestAccPagerDutyServiceIntegration_Basic
--- PASS: TestAccPagerDutyServiceIntegration_Basic (26.88s)
=== RUN   TestAccPagerDutyService_Basic
--- PASS: TestAccPagerDutyService_Basic (11.29s)
=== RUN   TestAccPagerDutyTeam_Basic
--- PASS: TestAccPagerDutyTeam_Basic (3.64s)
=== RUN   TestAccPagerDutyUser_Basic
--- PASS: TestAccPagerDutyUser_Basic (5.73s)
=== RUN   TestAccPagerDutyUserWithTeams_Basic
--- PASS: TestAccPagerDutyUserWithTeams_Basic (12.67s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/pagerduty  141.251s

Any feedback is more than welcome and I would love to see some continuation/improvements of this provider.

@kwilczynski
Copy link
Contributor

kwilczynski commented Sep 23, 2016

@heimweh hi there! This is great, absolutely great.

Once you get the go-pagerduty sorted, consider vendoring it - this will fix the build error. Also, for anything that consumes JSON from users, I would recommend setting up a ValidateFunc plus normalising JSON (compacting/minifying). This can be done using normalizeJsonString helper function.

@heimweh
Copy link
Contributor Author

heimweh commented Sep 24, 2016

Hi! :) thanks for the feedback, sounds good, I'll definitely look into this asap.

@heimweh heimweh force-pushed the master branch 8 times, most recently from 74d8a87 to cc2b025 Compare September 26, 2016 17:07
@kwilczynski
Copy link
Contributor

@heimweh Sir, you are a Gentleman and a Scholar. This is absolutely amazing!

@heimweh heimweh force-pushed the master branch 12 times, most recently from 1273be6 to 1115683 Compare September 27, 2016 22:58
@heimweh
Copy link
Contributor Author

heimweh commented Sep 27, 2016

Thanks @kwilczynski! really appreciate it :)

I've made quite some changes lately and that changed the logic quite a bit, sorry about that.
Hopefully the resources look a bit more clearer now though.

I'll try to refactor my code a bit, I feel like I made a bit of a mess in there when moving things around. I'll also update the documentation and hopefully get around to add more test cases soon.

@heimweh heimweh force-pushed the master branch 4 times, most recently from a8da764 to fd54b16 Compare October 4, 2016 18:43
@stack72
Copy link
Contributor

stack72 commented Oct 24, 2016

Hi @heimweh

Thanks for all the work here - just in preparation to getting this merged, what level of pagerduty plan is needed to run the entire set of tests for this provider?

Paul

@heimweh
Copy link
Contributor Author

heimweh commented Oct 24, 2016

Hi @stack72, to run the entire set of tests a standard plan is required, but the only resource that actually need the standard plan is the teams resource :)

This is the output when running the tests using a basic plan.

$ make testacc TEST=./builtin/providers/pagerduty
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2016/10/24 15:02:30 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/pagerduty -v  -timeout 120m
=== RUN   TestAccPagerDutyVendor_Basic
--- PASS: TestAccPagerDutyVendor_Basic (11.42s)
=== RUN   TestAccPagerDutyEscalationPolicy_import
--- PASS: TestAccPagerDutyEscalationPolicy_import (9.29s)
=== RUN   TestAccPagerDutySchedule_import
--- PASS: TestAccPagerDutySchedule_import (8.84s)
=== RUN   TestAccPagerDutyService_import
--- PASS: TestAccPagerDutyService_import (12.74s)
=== RUN   TestAccPagerDutyTeam_import
--- FAIL: TestAccPagerDutyTeam_import (0.41s)
    testing.go:265: Step 0 error: Error applying: 1 error(s) occurred:

        * pagerduty_team.foo: Failed call API endpoint. HTTP response code: 402. Error: &{2014 Required abilities are unavailable [The teams account ability is required to access teams]}
=== RUN   TestAccPagerDutyUser_import
--- PASS: TestAccPagerDutyUser_import (8.07s)
=== RUN   TestProvider
--- PASS: TestProvider (0.00s)
=== RUN   TestProviderImpl
--- PASS: TestProviderImpl (0.00s)
=== RUN   TestAccPagerDutyEscalationPolicy_Basic
--- PASS: TestAccPagerDutyEscalationPolicy_Basic (11.65s)
=== RUN   TestAccPagerDutyEscalationPolicyWithTeams_Basic
--- FAIL: TestAccPagerDutyEscalationPolicyWithTeams_Basic (5.70s)
    testing.go:265: Step 0 error: Error applying: 1 error(s) occurred:

        * pagerduty_team.foo: Failed call API endpoint. HTTP response code: 402. Error: &{2014 Required abilities are unavailable [The teams account ability is required to access teams]}
=== RUN   TestAccPagerDutySchedule_Basic
--- PASS: TestAccPagerDutySchedule_Basic (12.02s)
=== RUN   TestAccPagerDutySchedule_Multi
--- PASS: TestAccPagerDutySchedule_Multi (9.37s)
=== RUN   TestAccPagerDutyServiceIntegration_Basic
--- PASS: TestAccPagerDutyServiceIntegration_Basic (32.07s)
=== RUN   TestAccPagerDutyService_Basic
--- PASS: TestAccPagerDutyService_Basic (15.92s)
=== RUN   TestAccPagerDutyTeam_Basic
--- FAIL: TestAccPagerDutyTeam_Basic (0.44s)
    testing.go:265: Step 0 error: Error applying: 1 error(s) occurred:

        * pagerduty_team.foo: Failed call API endpoint. HTTP response code: 402. Error: &{2014 Required abilities are unavailable [The teams account ability is required to access teams]}
=== RUN   TestAccPagerDutyUser_Basic
--- PASS: TestAccPagerDutyUser_Basic (7.03s)
=== RUN   TestAccPagerDutyUserWithTeams_Basic
--- FAIL: TestAccPagerDutyUserWithTeams_Basic (0.46s)
    testing.go:265: Step 0 error: Error applying: 1 error(s) occurred:

        * pagerduty_team.foo: Failed call API endpoint. HTTP response code: 402. Error: &{2014 Required abilities are unavailable [The teams account ability is required to access teams]}
FAIL
exit status 1
FAIL    github.com/hashicorp/terraform/builtin/providers/pagerduty  145.436s
make: *** [testacc] Error 1

@stack72
Copy link
Contributor

stack72 commented Oct 24, 2016

Thanks for the info - will get on with a final review now :)

@heimweh
Copy link
Contributor Author

heimweh commented Oct 24, 2016

Awesome, thank you so much :)

@kwilczynski
Copy link
Contributor

@heimweh @stack72 the only things from me would be potentially (just before we merge):

  • Remove surplus &schema.Schema (since Go 1.7+ these are not needed)
  • Introduce errwrap.Wrapf over fmt.Errorf in the provider code (tests are different kettle of fish)
  • Use Go style way of querying maps

But, none of it is really super-essential to get it merged.

Having said that, @heimweh did a truly AMAZING job! I cannot express enough how outstanding this Pull Request really is!

@heimweh
Copy link
Contributor Author

heimweh commented Oct 24, 2016

@kwilczynski thanks for your feedback! :) removed the unnecessary use of &schema.Schema. Will definitely take a look at the rest.

@stack72 stack72 changed the title [WIP] Adding PagerDuty provider Adding PagerDuty provider Oct 24, 2016
@stack72
Copy link
Contributor

stack72 commented Oct 24, 2016

thanks for the work here - this is a really solid looking PR! :)

I agree with the feedback that @kwilczynski has suggested - these can be a follow up PR so we can get on with getting this merged in and having the nightly tests run as expected :)

I am just getting a pagerduty account setup right now so I can run the tests. If all look ok, I will merge this - stay tuned!

@heimweh
Copy link
Contributor Author

heimweh commented Oct 24, 2016

Thanks guys 😊 I really appreciate the feedback :)

This community is just pure awesomeness, so happy to be able to contribute.
That sounds good 👍 would be awesome to do a follow up soon.

Thanks again! :)

@stack72
Copy link
Contributor

stack72 commented Oct 24, 2016

@heimweh this works fantastically :)

% make testacc TEST=./builtin/providers/pagerduty                                                                                          ✭
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2016/10/24 17:03:46 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/pagerduty -v  -timeout 120m
=== RUN   TestAccPagerDutyVendor_Basic
--- PASS: TestAccPagerDutyVendor_Basic (11.17s)
=== RUN   TestAccPagerDutyEscalationPolicy_import
--- PASS: TestAccPagerDutyEscalationPolicy_import (8.22s)
=== RUN   TestAccPagerDutySchedule_import
--- PASS: TestAccPagerDutySchedule_import (8.83s)
=== RUN   TestAccPagerDutyService_import
--- PASS: TestAccPagerDutyService_import (13.31s)
=== RUN   TestAccPagerDutyTeam_import
--- PASS: TestAccPagerDutyTeam_import (2.96s)
=== RUN   TestAccPagerDutyUser_import
--- PASS: TestAccPagerDutyUser_import (4.66s)
=== RUN   TestProvider
--- PASS: TestProvider (0.00s)
=== RUN   TestProviderImpl
--- PASS: TestProviderImpl (0.00s)
=== RUN   TestAccPagerDutyEscalationPolicy_Basic
--- PASS: TestAccPagerDutyEscalationPolicy_Basic (11.44s)
=== RUN   TestAccPagerDutyEscalationPolicyWithTeams_Basic
--- PASS: TestAccPagerDutyEscalationPolicyWithTeams_Basic (14.46s)
=== RUN   TestAccPagerDutySchedule_Basic
--- PASS: TestAccPagerDutySchedule_Basic (9.50s)
=== RUN   TestAccPagerDutySchedule_Multi
--- PASS: TestAccPagerDutySchedule_Multi (8.57s)
=== RUN   TestAccPagerDutyServiceIntegration_Basic
--- PASS: TestAccPagerDutyServiceIntegration_Basic (30.01s)
=== RUN   TestAccPagerDutyService_Basic
--- PASS: TestAccPagerDutyService_Basic (13.53s)
=== RUN   TestAccPagerDutyTeam_Basic
--- PASS: TestAccPagerDutyTeam_Basic (4.82s)
=== RUN   TestAccPagerDutyUser_Basic
--- PASS: TestAccPagerDutyUser_Basic (8.30s)
=== RUN   TestAccPagerDutyUserWithTeams_Basic
--- PASS: TestAccPagerDutyUserWithTeams_Basic (17.40s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/pagerduty  167.225s

@stack72 stack72 merged commit 765dc19 into hashicorp:master Oct 24, 2016
@heimweh
Copy link
Contributor Author

heimweh commented Oct 24, 2016

Awesome! thank you so much @stack72 ! :)

@ghost
Copy link

ghost commented Apr 21, 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 21, 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.

None yet

3 participants