From 9e34626cef7f43d9f4ccc614c9350df90f99540a Mon Sep 17 00:00:00 2001 From: Stephen Lewis Date: Mon, 15 May 2023 12:00:18 -0700 Subject: [PATCH 1/7] Added update tests proving that metadata is not updatable --- .../resource_notebooks_instance_test.go.erb | 35 ++++++++++++++----- 1 file changed, 27 insertions(+), 8 deletions(-) diff --git a/mmv1/third_party/terraform/tests/resource_notebooks_instance_test.go.erb b/mmv1/third_party/terraform/tests/resource_notebooks_instance_test.go.erb index cca64062f71a..5ac2897cb6c0 100644 --- a/mmv1/third_party/terraform/tests/resource_notebooks_instance_test.go.erb +++ b/mmv1/third_party/terraform/tests/resource_notebooks_instance_test.go.erb @@ -1,8 +1,6 @@ <% autogen_exception -%> package google -<% unless version == 'ga' %> - import ( "fmt" "testing" @@ -50,7 +48,7 @@ func TestAccNotebooksInstance_update(t *testing.T) { ImportStateVerifyIgnore: []string{"vm_image"}, }, { - Config: testAccNotebooksInstance_update(context), + Config: testAccNotebooksInstance_update(context, false), }, { ResourceName: "google_notebooks_instance.instance", @@ -59,7 +57,7 @@ func TestAccNotebooksInstance_update(t *testing.T) { ImportStateVerifyIgnore: []string{"vm_image"}, }, { - Config: testAccNotebooksInstance_basic(context), + Config: testAccNotebooksInstance_update(context, false), }, { ResourceName: "google_notebooks_instance.instance", @@ -108,11 +106,28 @@ resource "google_notebooks_instance" "instance" { project = "deeplearning-platform-release" image_family = "tf-latest-cpu" } + + metadata = { + proxy-mode = "service_account" + terraform = "true" + } + + lifecycle { + prevent_destroy = true + } } `, context) } -func testAccNotebooksInstance_update(context map[string]interface{}) string { +func testAccNotebooksInstance_update(context map[string]interface{}, preventDestroy bool) string { + context["lifecycle_block"] = "" + if preventDestroy { + context["lifecycle_block"] = ` + lifecycle { + prevent_destroy = true + }` + } + return Nprintf(` resource "google_notebooks_instance" "instance" { name = "tf-test-notebooks-instance%{random_suffix}" @@ -130,12 +145,16 @@ resource "google_notebooks_instance" "instance" { image_family = "tf-latest-cpu" } + metadata = { + proxy-mode = "service_account" + terraform = "true" + notebook-upgrade-schedule = "0 * * * *" + } + labels = { key = "value" } + %{lifecycle_block} } `, context) } - - -<% end -%> From ffa79b809e5a5d770bbfae384dd1f6291ccf75fc Mon Sep 17 00:00:00 2001 From: Stephen Lewis Date: Mon, 15 May 2023 15:33:50 -0700 Subject: [PATCH 2/7] Test fixes --- .../resource_notebooks_instance_test.go.erb | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/mmv1/third_party/terraform/tests/resource_notebooks_instance_test.go.erb b/mmv1/third_party/terraform/tests/resource_notebooks_instance_test.go.erb index 5ac2897cb6c0..6d3f1c9bb557 100644 --- a/mmv1/third_party/terraform/tests/resource_notebooks_instance_test.go.erb +++ b/mmv1/third_party/terraform/tests/resource_notebooks_instance_test.go.erb @@ -3,6 +3,7 @@ package google import ( "fmt" + "strconv" "testing" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" @@ -48,7 +49,7 @@ func TestAccNotebooksInstance_update(t *testing.T) { ImportStateVerifyIgnore: []string{"vm_image"}, }, { - Config: testAccNotebooksInstance_update(context, false), + Config: testAccNotebooksInstance_update(context, true), }, { ResourceName: "google_notebooks_instance.instance", @@ -120,13 +121,7 @@ resource "google_notebooks_instance" "instance" { } func testAccNotebooksInstance_update(context map[string]interface{}, preventDestroy bool) string { - context["lifecycle_block"] = "" - if preventDestroy { - context["lifecycle_block"] = ` - lifecycle { - prevent_destroy = true - }` - } + context["prevent_destroy"] = strconv.FormatBool(preventDestroy) return Nprintf(` resource "google_notebooks_instance" "instance" { @@ -154,7 +149,10 @@ resource "google_notebooks_instance" "instance" { labels = { key = "value" } - %{lifecycle_block} + + lifecycle { + prevent_destroy = %{prevent_destroy} + } } `, context) } From 1ac238a057060d5dfbc10d98521421fd9eec4a2b Mon Sep 17 00:00:00 2001 From: Stephen Lewis Date: Mon, 15 May 2023 15:34:39 -0700 Subject: [PATCH 3/7] Made notebooks instance metadata field updatable --- mmv1/products/notebooks/Instance.yaml | 3 +++ .../terraform/update_encoder/notebooks_instance.go | 7 +++++++ 2 files changed, 10 insertions(+) create mode 100644 mmv1/templates/terraform/update_encoder/notebooks_instance.go diff --git a/mmv1/products/notebooks/Instance.yaml b/mmv1/products/notebooks/Instance.yaml index b7463f1e6976..db2219e3c1e2 100644 --- a/mmv1/products/notebooks/Instance.yaml +++ b/mmv1/products/notebooks/Instance.yaml @@ -81,6 +81,7 @@ examples: service_account: :SERVICE_ACCT custom_code: !ruby/object:Provider::Terraform::CustomCode constants: templates/terraform/constants/notebooks_instance.go + update_encoder: templates/terraform/update_encoder/notebooks_instance.go parameters: - !ruby/object:Api::Type::ResourceRef name: 'location' @@ -363,6 +364,8 @@ properties: Custom metadata to apply to this instance. An object containing a list of "key": value pairs. Example: { "name": "wrench", "mass": "1.3kg", "count": "3" }. ignore_read: true + update_verb: :PATCH + update_url: 'projects/{{project}}/locations/{{location}}/instances/{{name}}:updateMetadataItems' - !ruby/object:Api::Type::Time name: 'createTime' description: 'Instance creation time' diff --git a/mmv1/templates/terraform/update_encoder/notebooks_instance.go b/mmv1/templates/terraform/update_encoder/notebooks_instance.go new file mode 100644 index 000000000000..4171986de2fb --- /dev/null +++ b/mmv1/templates/terraform/update_encoder/notebooks_instance.go @@ -0,0 +1,7 @@ +// Update requests use "items" as the api name instead of "metadata" +// https://cloud.google.com/vertex-ai/docs/workbench/reference/rest/v1/projects.locations.instances/updateMetadataItems +if metadata, ok := obj["metadata"]; ok { + obj["items"] = metadata + delete(obj, "metadata") +} +return obj, nil From 08864abf58139148dafec92eae85a011c00f08e9 Mon Sep 17 00:00:00 2001 From: Stephen Lewis Date: Tue, 16 May 2023 09:00:18 -0700 Subject: [PATCH 4/7] Added metadata to ImportStateVerifyIgnore for impacted tests --- mmv1/products/notebooks/Instance.yaml | 2 ++ .../terraform/tests/resource_notebooks_instance_test.go.erb | 6 +++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/mmv1/products/notebooks/Instance.yaml b/mmv1/products/notebooks/Instance.yaml index db2219e3c1e2..6986b560e77e 100644 --- a/mmv1/products/notebooks/Instance.yaml +++ b/mmv1/products/notebooks/Instance.yaml @@ -363,6 +363,8 @@ properties: description: | Custom metadata to apply to this instance. An object containing a list of "key": value pairs. Example: { "name": "wrench", "mass": "1.3kg", "count": "3" }. + # This was added as part of https://github.com/GoogleCloudPlatform/magic-modules/pull/3761 to + # avoid detecting drift, since this field is partially server-managed. ignore_read: true update_verb: :PATCH update_url: 'projects/{{project}}/locations/{{location}}/instances/{{name}}:updateMetadataItems' diff --git a/mmv1/third_party/terraform/tests/resource_notebooks_instance_test.go.erb b/mmv1/third_party/terraform/tests/resource_notebooks_instance_test.go.erb index 6d3f1c9bb557..731add87cb89 100644 --- a/mmv1/third_party/terraform/tests/resource_notebooks_instance_test.go.erb +++ b/mmv1/third_party/terraform/tests/resource_notebooks_instance_test.go.erb @@ -46,7 +46,7 @@ func TestAccNotebooksInstance_update(t *testing.T) { ResourceName: "google_notebooks_instance.instance", ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"vm_image"}, + ImportStateVerifyIgnore: []string{"vm_image", "metadata"}, }, { Config: testAccNotebooksInstance_update(context, true), @@ -55,7 +55,7 @@ func TestAccNotebooksInstance_update(t *testing.T) { ResourceName: "google_notebooks_instance.instance", ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"vm_image"}, + ImportStateVerifyIgnore: []string{"vm_image", "metadata"}, }, { Config: testAccNotebooksInstance_update(context, false), @@ -64,7 +64,7 @@ func TestAccNotebooksInstance_update(t *testing.T) { ResourceName: "google_notebooks_instance.instance", ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"vm_image"}, + ImportStateVerifyIgnore: []string{"vm_image", "metadata"}, }, }, }) From f380e41f63aed4913e996e65b923104df80d120d Mon Sep 17 00:00:00 2001 From: Stephen Lewis Date: Tue, 16 May 2023 09:07:42 -0700 Subject: [PATCH 5/7] Removed non-updatable fields from update test --- .../terraform/tests/resource_notebooks_instance_test.go.erb | 6 ------ 1 file changed, 6 deletions(-) diff --git a/mmv1/third_party/terraform/tests/resource_notebooks_instance_test.go.erb b/mmv1/third_party/terraform/tests/resource_notebooks_instance_test.go.erb index 731add87cb89..af241c8b5e75 100644 --- a/mmv1/third_party/terraform/tests/resource_notebooks_instance_test.go.erb +++ b/mmv1/third_party/terraform/tests/resource_notebooks_instance_test.go.erb @@ -129,12 +129,6 @@ resource "google_notebooks_instance" "instance" { location = "us-central1-a" machine_type = "e2-medium" - nic_type = "VIRTIO_NET" - - reservation_affinity { - consume_reservation_type = "NO_RESERVATION" - } - vm_image { project = "deeplearning-platform-release" image_family = "tf-latest-cpu" From f22a9e4cd29c3e0d35c1888aca5280a1bef6c22b Mon Sep 17 00:00:00 2001 From: Stephen Lewis Date: Tue, 16 May 2023 09:09:36 -0700 Subject: [PATCH 6/7] Temporarily made metadata field not updatable to prove that only these changes are required to make the tests pass --- mmv1/products/notebooks/Instance.yaml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/mmv1/products/notebooks/Instance.yaml b/mmv1/products/notebooks/Instance.yaml index 6986b560e77e..7922251f8678 100644 --- a/mmv1/products/notebooks/Instance.yaml +++ b/mmv1/products/notebooks/Instance.yaml @@ -81,7 +81,7 @@ examples: service_account: :SERVICE_ACCT custom_code: !ruby/object:Provider::Terraform::CustomCode constants: templates/terraform/constants/notebooks_instance.go - update_encoder: templates/terraform/update_encoder/notebooks_instance.go + # update_encoder: templates/terraform/update_encoder/notebooks_instance.go parameters: - !ruby/object:Api::Type::ResourceRef name: 'location' @@ -366,8 +366,8 @@ properties: # This was added as part of https://github.com/GoogleCloudPlatform/magic-modules/pull/3761 to # avoid detecting drift, since this field is partially server-managed. ignore_read: true - update_verb: :PATCH - update_url: 'projects/{{project}}/locations/{{location}}/instances/{{name}}:updateMetadataItems' + # update_verb: :PATCH + # update_url: 'projects/{{project}}/locations/{{location}}/instances/{{name}}:updateMetadataItems' - !ruby/object:Api::Type::Time name: 'createTime' description: 'Instance creation time' From b7154575e01274a65498b51faf829260c00243c2 Mon Sep 17 00:00:00 2001 From: Stephen Lewis Date: Tue, 16 May 2023 10:36:15 -0700 Subject: [PATCH 7/7] Revert "Temporarily made metadata field not updatable to prove that only these changes are required to make the tests pass" This reverts commit f22a9e4cd29c3e0d35c1888aca5280a1bef6c22b. --- mmv1/products/notebooks/Instance.yaml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/mmv1/products/notebooks/Instance.yaml b/mmv1/products/notebooks/Instance.yaml index 7922251f8678..6986b560e77e 100644 --- a/mmv1/products/notebooks/Instance.yaml +++ b/mmv1/products/notebooks/Instance.yaml @@ -81,7 +81,7 @@ examples: service_account: :SERVICE_ACCT custom_code: !ruby/object:Provider::Terraform::CustomCode constants: templates/terraform/constants/notebooks_instance.go - # update_encoder: templates/terraform/update_encoder/notebooks_instance.go + update_encoder: templates/terraform/update_encoder/notebooks_instance.go parameters: - !ruby/object:Api::Type::ResourceRef name: 'location' @@ -366,8 +366,8 @@ properties: # This was added as part of https://github.com/GoogleCloudPlatform/magic-modules/pull/3761 to # avoid detecting drift, since this field is partially server-managed. ignore_read: true - # update_verb: :PATCH - # update_url: 'projects/{{project}}/locations/{{location}}/instances/{{name}}:updateMetadataItems' + update_verb: :PATCH + update_url: 'projects/{{project}}/locations/{{location}}/instances/{{name}}:updateMetadataItems' - !ruby/object:Api::Type::Time name: 'createTime' description: 'Instance creation time'