Since provider 6.3.0 boot_disk.initialize_params.resource_policies is available and force rebuilt

[mldmld68]

Community Note

We deployed since years a Cloud Function to automatically associate a snapshot schedule each time a GCE is started in our GCP organzation. Whatever the way : console or terraform.
It appears the associated schedule is setup in the resource_policies field of the GCE.

With this new feature, all our GCE are planed to be rebuilt because our existing code do not manage it (normal, it was not availabled before) and the property is not empty on all our GCEs.

Terraform Version & Provider Version(s)

Terraform v1.7.2 on x86

• provider registry.terraform.io/hashicorp/google v6.3.0

Affected Resource(s)

google_compute_instance

Terraform Configuration

Debug Output

    # (8 unchanged attributes hidden)
  ~ boot_disk {
      ~ device_name                = "persistent-disk-0" -> (known after apply)
      + disk_encryption_key_sha256 = (known after apply)
 .....
          - resource_policies           = [ # forces replacement
              - "https://www.googleapis.com/compute/v1/projects/<projectID>/regions/europe-west1/resourcePolicies/<projectID>-europe-west1",
            ] -> null

Expected Behavior

The GCE should not be rebuilt with such parameter change. In particular for a snapshot schedule setting

Actual Behavior

The GCE are rebuilt

Steps to reproduce

1. terraform apply

Important Factoids

We deployed since year a Cloud Function to automatically associate a snapshot schedule each time a GCE is started in our GCP organzation. Whatever the way : console, gcloud or terraform.
It appears the associated schedule is setup in the resource_policies field of the GCE.

With this new feature, all our GCE are planed to be rebuilt because our existing code do not manage it (normal, it was not availabled before) and the property is not empty on all our GCEs.

References

No response

b/372018525

[slevenick]

I don't think there's anything we can do on the provider side here. This is just how Terraform works. As new fields are added to Terraform they are tracked by the resource, so Terraform will start to manage them.

You can add ignore_changes to prevent this for specific fields:
lifecycle {
ignore_changes = [boot_disk.initialize_params.resource_policies]
}

[mldmld68]

Yes, terraform start to manage them, but I would like the resource not being rebuilt for a change of this field

[slevenick]

Adding this as an enhancement rather than a bug then

[SarahFrench]

Closed by GoogleCloudPlatform/magic-modules#11753

[SarahFrench]

Reopening as not wanting permadiff =/= not wanting a force new

[SarahFrench]

Note from triage: Initially we thought that it's valid to have initialize_params for boot_disk force resources to be recreated when they change, as the initialize params arguments control when a resource is first made. Changing those values implies wanting to recreate a resource using new inputs. However if these values are being impacted by using snapshots with the disk we'd like the service team to investigate further.

[karolgorc]

Hi @SarahFrench I'm currently working on closing the feature gap between the API and the provider on disks.

Specifically talking about disks[].initializeParams.onUpdateAction field. Do you think that we could have the on_update_action field to handle all of the disk updates on the instance? This would allow initialize_params to no longer force an instance recreation.

Also bringing this up as there are a few features that are blocked due to the fact that initialize_params is only available under boot_disk. There are some beta features that are under initialize_params but don't work on disks that are boot=true. multiWriter as an example

[SarahFrench]

I've moved off the Google provider project and want to avoid getting into the weeds - @slevenick could you help here?

[slevenick]

Hi @SarahFrench I'm currently working on closing the feature gap between the API and the provider on disks.

Specifically talking about disks[].initializeParams.onUpdateAction field. Do you think that we could have the on_update_action field to handle all of the disk updates on the instance? This would allow initialize_params to no longer force an instance recreation.

Is the idea to add a new field on_update_action that would limit the action taken on update? So if on_update_action is set to REFRESH then we wouldn't trigger the recreation?

That's possible, and we have something similar in most_disruptive_allowed_action already in MIGs, though that's supported through the API rather than in Terraform directly. I'm not sure we would want such behavior in Terraform specifically, as it seems like it would add extra layers of deletion_protection-like behavior but on a specific sub-field.

Also bringing this up as there are a few features that are blocked due to the fact that initialize_params is only available under boot_disk. There are some beta features that are under initialize_params but don't work on disks that are boot=true. multiWriter as an example

Sounds like we need initialize_params under regular disks as well? That may get really complicated, as the initialize_params field is already difficult to manage, but it should be possible

[karolgorc]

Here is the onUpdateAction schema from the compute API discovery doc just to show the values available to be set.

"onUpdateAction": {
          "description": "Specifies which action to take on instance update with this disk. Default is to use the existing disk.",
          "type": "string",
          "enumDescriptions": [
            "Always recreate the disk.",
            "Recreate the disk if source (image, snapshot) of this disk is different from source of existing disk.",
            "Use the existing disk, this is the default behaviour."
          ],
          "enum": [
            "RECREATE_DISK",
            "RECREATE_DISK_IF_SOURCE_CHANGED",
            "USE_EXISTING_DISK"
          ]
        },

I haven't tested this because this would require a lot of logic to support and i'm in the talks with the disks API team if it does work as described (not bugged etc.).

But the basic idea could be that instead of Forcing a new VM on update we could make initialize_params ForceNew: false and then have that field be USE_EXISTING_DISK for every action and RECREATE_DISK when if d.hasChange("boot_disk.0.initialize_params.0") is true.

Sounds like we need initialize_params under regular disks as well? That may get really complicated, as the initialize_params field is already difficult to manage, but it should be possible

This is difficult to manage because of our syntax. boot_disk, attached_disk and scratch_disk is not a thing in the API. It's just disks.[].

I see 2 ways out of this. Either we normalize our fields with the API (confusing to the users) or have an external schema for initialized params that's just passed down to the main schema 3 times. Same with helper functions and so on. This field doesn't have any differences no matter the type. It can just throw different API erros on different disk types.

Still the core of the issue here is that instance gets recreated when initialize_params is updated. This should only recreate the disk in question. This is the ideal scenario but then the user wouldn't be informed in a good way that the data from the disk is being ereased. That's why the ForceNew on initialize_params has it's benefits. Not really sure what's the balance here.

[slevenick]

Can we just expose onUpdateAction to users, default it to USE_EXISTING_DISK (which I assume is the current behavior?) and remove ForceNew: true?

My expectation is that a user trying to update their initialize_params would receive an error unless they specifically set RECREATE_DISK, which seems like what we want.

[karolgorc]

Can we just expose onUpdateAction

Yes this is a field in the instance JSON payload. It is supported within the Go compute packages etc. so no problem implementing it.

default it to USE_EXISTING_DISK (which I assume is the current behavior?)

Yes

My expectation is that a user trying to update their initialize_params would receive an error unless they specifically set RECREATE_DISK

Yes, Initialize params will force a recreation of the disk but on the API's side and only when there is a change on initialize params.

There is one problem with this. I think that the forces replacement makes the user understand that the action he is trying to do will be destructive. And I don't really know what's better. Informing a user about this via plan or throwing an error telling him to directly set RECREATE_DISK.

[karolgorc]

After doing a deep dive into this and playing directly with the API i'd say that this still wouldn't change anything.

1. initializeParams cannot be updated on an instance in the API
   • When you do an instances.Insert call with initializeParams set it will create an instance according to your config
   • When you do an instances.Get call on the created instance the initializeParams field is gone from the config JSON and cannot be changed
   • The onUpdateAction seems to only work on instance updates not the disk itself.

initializeParams can be configured on creation only and isn't stored after so i'd say that the ForceNew configuration is correct and we cannot do anything about it. This does complicate things if we need to add it into all the other disk types.

So first of all i'll focus on solving the issue from the description specifically. We should have initialize_params.resource_policies as an empty array until the user sets it by himself.

The next step in my opinion should be to update the description of initialize_params in the docs to inform about this "configurable on creation only" behaviour as it's not mentioned anywhere

And i don't think that setting the disk with initialize_params should be promoted in our examples as the go to config. I think that using boot_disk.source with "google_compute_disk" should be the standard shown in our docs as it won't force recreations on basic actions like rescaling a disk.