Make it possible to configure individual PD service objects independently

[bkabrda]

This PR uses newly added Datadog API features (undocumented ATM, unfortunately) to make it possible to configure individual PD integration service objects independently.

The idea is to be able to use the current datadog_integration_pagerduty resource and allow adding number of independent service objects via datadog_integration_pagerduty_service_object resources.

Some notes:

• I wanted to make sure that we preserve backwards compatibility for the current datadog_integration_pagerduty and the way it works with services. To that end, I added individual_services attribute to it, which is mutually exclusive with services. If services is present, everything will work as it used to up until now. If individual_services is present, then this resource will not be storing list of service object into its state, meaning that creating datadog_integration_pagerduty_service_object resources won't cause TF to produce non-empty plans for it - it will pretty much remain as a configuration object for the whole integration.
• All the individual services defined by datadog_integration_pagerduty_service_object have to have depends_on = ["datadog_integration_pagerduty.foo"], since they can't be created if the integration is deactivated.
• datadog_integration_pagerduty.services is now marked as deprecated because of this new functionality.

TODO:

• Document all of the above
• Write acceptance tests and make sure that it all actually works as expected
• Figure out (for docs) how to force-update a service object; since the API isn't returning the actual service keys, we have no way of detecting a drift; we should find a way for users to explicitly update a service object even if drift is not detected
  • This can be done using terraform taint, which I documented.
• Make sure importing the new service object resource works
  • This doesn't really work, as the API never returns service_key, so import wouldn't work well.

This supersedes https://github.com/terraform-providers/terraform-provider-datadog/pull/164, implements https://github.com/terraform-providers/terraform-provider-datadog/issues/75 and also fixes https://github.com/terraform-providers/terraform-provider-datadog/issues/160 thanks to updating the go-datadog-api dependency to latest master, which contains the fix for it: zorkian/go-datadog-api@ddd27c7

[gzussa]

Could we have some more documentation around these fields and methods?

[bkabrda]

So the code and docs are finished and reviewable from my PoV. While writing the tests, I've discovered a race condition on the API side, so I'll need to wait for it to get fixed before this PR can be finalized.

[bkabrda]

So I was advised to workaround the race condition by serializing all operations that change individual service objects - I did that by using a mutex. I also added tests, so this PR is now complete from my side and ready to be fully reviewed.

[gzussa]

LGTM. 👏 Great Job!!

[nmuesch]

Overall looks great 🚀 Left a couple of small comments.

Just want to confirm the terraform plan that will happen as people migrate over. If you currently use this pagerduty resource, you'll have the service attributes in the state. The plan is to switch to this new mechanism, and on the next apply, the READ call will see nil services and report the diff. BUT the Update call will send nil to the API.

Would this do nothing, or temporarily delete the services until the new service_object attribute calls its Create mechanism?

[nmuesch]

Should we link to the documentation here?

[nmuesch]

And say that you need to install the tile first? (And/or show the TF page about enabling the integration globally?)

[bkabrda]

The error returned from API says Pagerduty Integration not found (which is printed with the error message) is IMO sufficient. I think this is self-explanatory enough. Do you need it needs to be improved? Because if we wanted to provide the docs just in this case, we'd need to scan precisely for the Pagerduty Integration not found string and that can be error-prone if that string changes a bit in the future...

[nmuesch]

Yeah thats totally valid. Lets keep as is then 🙂

[bkabrda]

Thanks for the review @nmuesch. I admit I haven't fully investigated the migration path, so that's something I need to look into and it will likely take some time to make sure it's done right. I'll update this issue when I figure out the best approach here.

[bkabrda]

All right, I figured out a migration path, documented it and wrote an integration test for it. Ready for re-review.

[nmuesch]

🚀 LGTM Thanks especially for figuring out and documenting this migration path!
💜 for the update bug fix as well!

[jvitkauskas]

Any plans to release this? Since this fixes https://github.com/terraform-providers/terraform-provider-datadog/issues/160 it enables datadog pagerduty integration to work unlike current stable version.

[bkabrda]

@jvitkauskas hey, this will be part of the next release. We're hoping to get it released either this Thursday/Friday or in the first half of next week (not guaranteed at this point, we may hold the release if we find some issues during pre-release testing).