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

Add pagerduty_business_service_subscriber resource #414

Merged

Conversation

gsreynolds
Copy link
Member

@gsreynolds gsreynolds commented Oct 28, 2021

TF_ACC=1 go test -run "TestAccPagerDutyBusinessServiceSubscriber" ./pagerduty -v -timeout 120m     
=== RUN   TestAccPagerDutyBusinessServiceSubscriber_import
--- PASS: TestAccPagerDutyBusinessServiceSubscriber_import (32.39s)
=== RUN   TestAccPagerDutyBusinessServiceSubscriber_User
--- PASS: TestAccPagerDutyBusinessServiceSubscriber_User (25.49s)
=== RUN   TestAccPagerDutyBusinessServiceSubscriber_Team
--- PASS: TestAccPagerDutyBusinessServiceSubscriber_Team (25.30s)
=== RUN   TestAccPagerDutyBusinessServiceSubscriber_TeamUser
--- PASS: TestAccPagerDutyBusinessServiceSubscriber_TeamUser (27.49s)
PASS
ok      github.com/terraform-providers/terraform-provider-pagerduty/pagerduty   111.033s

Closes #390

@gsreynolds gsreynolds marked this pull request as ready for review November 5, 2021 12:15
Copy link
Contributor

@stmcallister stmcallister left a comment

Choose a reason for hiding this comment

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

Very nice! Thank you!! 🎉 🎉

@stmcallister
Copy link
Contributor

@gsreynolds before we merge this, can you update the reference for the go library?

go get -u github.com/heimweh/go-pagerduty

go mod vendor

And then push those changes here?

@gsreynolds gsreynolds force-pushed the business_service_subscribers branch from 2885670 to 5ef2fd0 Compare November 11, 2021 09:36
@gsreynolds
Copy link
Member Author

Hey @stmcallister, thanks. Rebased against master to resolve merge conflicts. Should be good to go!

Signed-off-by: Gavin Reynolds <greynolds@pagerduty.com>
Signed-off-by: Gavin Reynolds <greynolds@pagerduty.com>
…ID for business service, subscriber type and subscriber id

Signed-off-by: Gavin Reynolds <greynolds@pagerduty.com>
Signed-off-by: Gavin Reynolds <greynolds@pagerduty.com>
@gsreynolds
Copy link
Member Author

gsreynolds commented Nov 19, 2021

@stmcallister As discussed, I've submitted heimweh/go-pagerduty#73 to additionally check the response body for a successful subscription, as the API will return 200 OK if the result of one (or more) subscriptions in the request was not success. The endpoint is designed to accept multiple subscriptions for users and teams in one request, but here it is only being used to send one subscription in a single request.

I've also rebased the branch to catch up with latest changes, but no changes required in this PR other than updating the vendored packages once heimweh/go-pagerduty#73 is merged.

Example of 200 OK response body:

{
 "subscriptions": [
  {
   "account_id": "PQL6MPB",
   "result": "unauthorized",
   "subscribable_id": "P3D3U4T",
   "subscribable_type": "business_service",
   "subscriber_id": "PGW87TJ",
   "subscriber_type": "team"
  }
 ]
}

Fixes occasional errors resulting from non-success responses. If a non-success response is returned, the resource will retry as normal until retries are exceeded or it is successful.

    resource_pagerduty_business_service_subscriber_test.go:39: Step 1/1 error: Error running apply: exit status 1
        
        Error: Provider produced inconsistent result after apply
        
        When applying changes to pagerduty_business_service_subscriber.foo, provider
        "provider[\"registry.terraform.io/hashicorp/pagerduty\"]" produced an
        unexpected new value: Root resource was present, but now absent.
        
        This is a bug in the provider, which should be reported in the provider's own
        issue tracker.

@stmcallister stmcallister merged commit 83f19d5 into PagerDuty:master Nov 20, 2021
@gsreynolds gsreynolds deleted the business_service_subscribers branch November 22, 2021 10:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support Business Service Subscribers
2 participants