azurerm_traffic_manager_profile - support for new field max_return and support for traffic_routing_method to be MultiValue

[brandon-dd]

👋 Hi all!

This PR adds a new optional field to the azurerm_traffic_manager_profile resource, which is max_return, which allows profiles with the routing methods MultiValue to be set. Without this field, resources like this that attempt to be created are always rejected by the Azure API because the MaxReturn value isn't set, so it's impossible to create them with Terraform.

Not coincidentally, this type of routing method was also omitted from the acceptance tests (probably because they wouldn't have passed if MultiValue was included). Thus I've done my best to add the missing tests there too, but did run into some trouble running the acceptance tests, so please let me know if there's anything I need to improve there!

Manual tests run by me:

• Creating a MultiValue Traffic Manager without max_return set, verified we see our new error message complaining about how we need to set it (nicer than getting it from Azure itself)
• Creating a MultiValue Traffic Manager with max_return set, but with a bad health check config, verified that we see a different error message about that issue instead
• Creating a MultiValue Traffic Manager with max_return set and everything else correct, verified it can create the Traffic Manger without issue
• Updated a MultiValue Traffic Manager that already had max_return set with a new max_return, verified it updated correctly on Azure's end

Hope this change is good with you guys, feedback welcome 🙂

[katbyte]

Thanks for the pr @brandon-dd - overall this looks good except could we update the docs with the new property? Should be good to merge once thats done 🙂

[brandon-dd]

Yeah, sure will add! Also just figured out my issue with the acceptance tests, ran these two:

TestAccAzureRMTrafficManagerProfile_cycleMethod
TestAccAzureRMTrafficManagerProfile_fastMaxReturnSettingError

which are the ones I changed / added, and they both pass :)

[brandon-dd]

Ok, added docs and fixed up merge conflicts, should be good to go now!

[brandon-dd]

Okay, just fixed conflicts with master again, and acceptance tests still pass -- @katbyte do you think you could have another look again soon? Spending more time than I'd like to on merge conflicts 🙂

One thing to note is that, since tests were moved out of the tests directory, the make acctests was broken for me and I had to edit the makefile in order to run them

[brandon-dd]

Bump. If @katbyte is on vacation or something, is it possible for someone else to have a look at this PR instead?

[magodo]

Hi @brandon-dd Thanks for this PR 👍
I have left some minor comments, one thing is that in the document you said max_return defaults to 8, but I didn't see it is set anywhere in code.
Besides, would you mind to fix the CI problems also?

[brandon-dd]

Hey @magodo, thanks for the review! Your suggestions were good, I've implemented them (I'm not too familiar with Go yet so I appreciated the tips).

As for the default value, sorry I phrased that badly -- I meant that it seems like Azure's default value for this number is 8, since that's what they suggest in the UI when you try to make a Traffic Manager in the portal. It's still required for the API though, so I figured we should make it required here too (when using MultiValue).

And, the tests actually only failed because of the GH Actions outage last week, they're all passing now that I've pushed a new commit 🙂

[katbyte]

Thanks @brandon-dd - this LGTM 👍

[magodo]

[brandon-dd]

Thanks very much, guys!

[ghost]

This has been released in version 2.41.0 of the provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. As an example:

provider "azurerm" {
    version = "~> 2.41.0"
}
# ... other configuration ...

[ghost]

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 feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks!