Deprecate beta fields for (R)IGM.

[paddycarver]

Update (Regional) Instance Group Managers to deprecate the beta fields
(rolling_update_policy, versions, and auto_healing_policy). Also, rename
RESTART to REPLACE for Instance Group Manager's update_strategy, to be a
bit clearer. This required a DiffSuppress to make sure RESTART and
REPLACE are considered equivalent. In 2.0.0, we should remove RESTART
and drop the DiffSuppressFunc. Sadly, we have no way to do a deprecation
message for RESTART. We'll have to put it in the CHANGELOG. I've already
removed it from the website.

Fixes #1971, replaces #1969.

[rileykarson]

I think I'm still a little confused on what part update_strategy plays here. Our docs talk about preventing Terraform from restarting instances, and have lost me every time I read them. My understanding based on scanning through the update code again, though, is:

• "NONE" means that if I update instance_template, my igm will opportunistically replace my instances
• "REPLACE" means that my igm will proactively restart all my instances

So what we're really doing is replicating update_policy.type as described in the updating igm docs.

This means if we keep using update_strategy, we're going to have to deprecate it all over again when igm updating goes ga because it will be redundant. I may have missed some discussion/not caught on to something here so I have 2 questions:

• Why NONE and REPLACE? Wouldn't OPPORTUNISTIC and PROACTIVE describe them more accurately and in line with the future/beta API? This is a bigger change than just replace => restart of course, but imo would make the field easier to understand and would make it clearer how to translate to update_policy.type in the future.

• Building on that, what would the downside be to just using update_policy.type with our current behaviour? (along with using a subset of versions in config early! The idea being that the migration to the updater will be seamless) When the feature goes GA, behaviour will change a little because of fun new features like max surge and max unavailable, but I don't think that would be a negative change for any use case nor a breaking change.

It also turns out we can make less fields Required in update_policy, and the API will accept the block with just type set, choosing reasonable defaults! For example, the following is a valid request body (and same w/ proactive subbed in):

{
  "versions": [
    {
      "instanceTemplate": "projects/482878270665/global/instanceTemplates/terraform-20180917062937282300000001"
    }
  ],
  "updatePolicy": {
    "type": "OPPORTUNISTIC"
  },
  "fingerprint": "ITkm514-hQ0="
}

[paddycarver]

So update_strategy (AFAIK) predates update_policy, and is a Terraform-specific construct. It basically offers a way to have Terraform force the recreation of every instance in a managed instance group when the instance_template changes.

It should not survive once update_policy is available in GA, as it becomes pointless.

Why NONE and REPLACE? Wouldn't OPPORTUNISTIC and PROACTIVE describe them more accurately and in line with the future/beta API? This is a bigger change than just replace => restart of course, but imo would make the field easier to understand and would make it clearer how to translate to update_policy.type in the future.

Honestly, it's to minimize the size of the step people need to take now, given we can't actively deprecate values. I'm not wed to it, but RESTART was actively disingenuous about what it was doing, whereas OPPORTUNISTIC/PROACTIVE feels more a matter of aesthetics than correctness. From what I understand, we're not going to be keeping this alive very long, so I'm hesitant to sink too much effort into moving people onto better values, when the entire field will go away.

Building on that, what would the downside be to just using update_policy.type with our current behaviour? (along with using a subset of versions in config early! The idea being that the migration to the updater will be seamless) When the feature goes GA, behaviour will change a little because of fun new features like max surge and max unavailable, but I don't think that would be a negative change for any use case nor a breaking change.

It'd mean deprecating update_strategy now, and honestly, I'm just wary of having two varied behaviours with the same name. It's easier to keep track of which is which when we have distinct names for them.

I can see the attraction of deprecating the instance_template now and using version, and deprecating update_strategy so we can use update_policy.type, because it would mean we don't have to carry update_strategy and instance_template through 2.0.0 as deprecated fields, but I think the subtle differences in what they do, the fact that they were already smushed into a resource together with confusing (and broken) behaviour, and the confusion of the deprecation will lead to headaches as people try to sort out exactly what each field does and what they can use. In my opinion, it's simpler to separate it into the "old behaviour" that was never labeled beta, and the "new behaviour" that was always beta and is now in the beta provider.

[rileykarson]

Sounds good! I guess ideally I'd like to skip another deprecation/removal period but you're right that having our old behaviour and the new better behaviour under the same name wouldn't be fun.

[rileykarson]

What do you think about prefixing these fields with a message that the field is beta? "This field is beta, use the instance_group_manager resource..."

Reading the message as it stands suggests that the resource itself is beta to me.

[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 👉 hashibot-feedback@hashicorp.com. Thanks!