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

Change the import behaviour of GCE instance metadata startup-script #3765

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changelog/5329.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:breaking-change
compute: changed the import / drift detection behaviours for `metadata_startup_script`, `metadata.startup-script` in `google_compute_instance`. Now, `metadata.startup-script` will be set by default, and `metadata_startup_script` will only be set if present.
```
14 changes: 5 additions & 9 deletions google-beta/resource_compute_instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -1141,19 +1141,15 @@ func resourceComputeInstanceRead(d *schema.ResourceData, meta interface{}) error
}

md := flattenMetadataBeta(instance.Metadata)
existingMetadata := d.Get("metadata").(map[string]interface{})

// If the existing config specifies "metadata.startup-script" instead of "metadata_startup_script",
// we shouldn't move the remote metadata.startup-script to metadata_startup_script. Otherwise,
// we should.
if _, ok := existingMetadata["startup-script"]; !ok {
// If the existing state contains "metadata_startup_script" instead of "metadata.startup-script",
// we should move the remote metadata.startup-script to metadata_startup_script to avoid
// specifying it in two places.
if _, ok := d.GetOk("metadata_startup_script"); ok {
if err := d.Set("metadata_startup_script", md["startup-script"]); err != nil {
return fmt.Errorf("Error setting metadata_startup_script: %s", err)
}
// Note that here we delete startup-script from our metadata list. This is to prevent storing the startup-script
// as a value in the metadata since the config specifically tracks it under 'metadata_startup_script'
delete(md, "startup-script")
} else if _, ok := d.GetOk("metadata_startup_script"); ok {

delete(md, "startup-script")
}

Expand Down
2 changes: 2 additions & 0 deletions google-beta/resource_compute_instance_template.go
Original file line number Diff line number Diff line change
Expand Up @@ -1313,8 +1313,10 @@ func resourceComputeInstanceTemplateRead(d *schema.ResourceData, meta interface{
if err = d.Set("metadata_startup_script", script); err != nil {
return fmt.Errorf("Error setting metadata_startup_script: %s", err)
}

delete(_md, "startup-script")
}

if err = d.Set("metadata", _md); err != nil {
return fmt.Errorf("Error setting metadata: %s", err)
}
Expand Down
4 changes: 2 additions & 2 deletions google-beta/resource_compute_instance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,8 @@ func testSweepComputeInstance(region string) error {

func computeInstanceImportStep(zone, instanceName string, additionalImportIgnores []string) resource.TestStep {
// metadata is only read into state if set in the config
// since importing doesn't know whether metadata.startup_script vs metadata_startup_script is set in the config,
// it guesses metadata_startup_script
// importing doesn't know whether metadata.startup_script vs metadata_startup_script is set in the config,
// it always takes metadata.startup-script
ignores := []string{"metadata.%", "metadata.startup-script", "metadata_startup_script"}

return resource.TestStep{
Expand Down
3 changes: 1 addition & 2 deletions google-beta/resource_dataproc_cluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,8 @@ import (
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
"github.com/hashicorp/terraform-plugin-sdk/v2/terraform"

"google.golang.org/api/googleapi"

dataproc "google.golang.org/api/dataproc/v1beta2"
"google.golang.org/api/googleapi"
)

func TestDataprocExtractInitTimeout(t *testing.T) {
Expand Down
22 changes: 22 additions & 0 deletions website/docs/guides/version_4_upgrade.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,28 @@ The provider will now enforce at plan time that one of these fields be set.

Previously, if all of these fields were left empty, the firewall defaulted to allowing traffic from 0.0.0.0/0, which is a suboptimal default.

## Resource: `google_compute_instance`

### `metadata_startup_script` is no longer set on import

Earlier versions of the provider set the `metadata_startup_script` value on
import, omitting the value of `metadata.startup-script` for historical backwards
compatibility. This was dangerous in practice, as `metadata_startup_script`
would flag an instance for recreation if the values differed rather than for
just an update.

In `4.0.0` the behaviour has been flipped, and `metadata.startup-script` is the
default value that gets written. Users who want `metadata_startup_script` set
on an imported instance will need to modify their state manually. This is more
consistent with our expectations for the field, that a user who manages an
instance **only** through Terraform uses it but that most users should prefer
the `metadata` block.

No action is required for user configs with instances already imported. If you
have a config or module where neither is specified- where `import` will be run,
or an old config that is not reconciled with the API- the value that gets set
will change.

## Resource: `google_compute_instance_group_manager`

### `update_policy.min_ready_sec` is removed from the GA provider
Expand Down
20 changes: 10 additions & 10 deletions website/docs/r/compute_instance.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -132,16 +132,16 @@ The following arguments are supported:
we provide a special attribute, `metadata_startup_script`, which is documented below.

* `metadata_startup_script` - (Optional) An alternative to using the
startup-script metadata key, except this one forces the instance to be
recreated (thus re-running the script) if it is changed. This replaces the
startup-script metadata key on the created instance and thus the two
mechanisms are not allowed to be used simultaneously. Users are free to use
either mechanism - the only distinction is that this separate attribute
will cause a recreate on modification. On import, `metadata_startup_script`
will be set, but `metadata.startup-script` will not - if you choose to use the
other mechanism, you will see a diff immediately after import, which will cause a
destroy/recreate operation. You may want to modify your state file manually
using `terraform state` commands, depending on your use case.
startup-script metadata key, except this one forces the instance to be recreated
(thus re-running the script) if it is changed. This replaces the startup-script
metadata key on the created instance and thus the two mechanisms are not
allowed to be used simultaneously. Users are free to use either mechanism - the
only distinction is that this separate attribute will cause a recreate on
modification. On import, `metadata_startup_script` will not be set - if you
choose to specify it you will see a diff immediately after import causing a
destroy/recreate operation. If importing an instance and specifying this value
is desired, you will need to modify your state file manually using
`terraform state` commands.

* `min_cpu_platform` - (Optional) Specifies a minimum CPU platform for the VM instance. Applicable values are the friendly names of CPU platforms, such as
`Intel Haswell` or `Intel Skylake`. See the complete list [here](https://cloud.google.com/compute/docs/instances/specify-min-cpu-platform).
Expand Down