Added upgrade_settings with validation for SURGE strategy with promoting min_cpu_platform to GA #6815

mitj04 · 2022-11-11T17:08:31Z

part of hashicorp/terraform-provider-google#9180

If this PR is for Terraform, I acknowledge that I have:

Searched through the issue tracker for an open issue that this either resolves or contributes to, commented on it to claim it, and written "fixes {url}" or "part of {url}" in this PR description. If there were no relevant open issues, I opened one and commented that I would like to work on it (not necessary for very small changes).
Generated Terraform, and ran make test and make lint to ensure it passes unit and linter tests.
Ensured that all new fields I added that can be set by a user appear in at least one example (for generated resources) or third_party test (for handwritten resources or update tests).
Ran relevant acceptance tests (If the acceptance tests do not yet pass or you are unable to run them, please let your reviewer know).
Read the Release Notes Guide before writing my release note below.

Release Note Template for Downstream PRs (will be copied)

container: added `auto_provisioning_defaults.cluster_autoscaling.upgrade_settings` in `google_container_cluster`

container: promoted `min_cpu_platform` in `google_container_cluster` to GA

…ing min_cpu_platform

modular-magician · 2022-11-11T17:08:47Z

Oops! It looks like you're using an unknown release-note type in your changelog entries:

REPLACEME

Please only use the types listed in https://github.com/GoogleCloudPlatform/magic-modules/blob/master/.ci/RELEASE_NOTES_GUIDE.md.

modular-magician · 2022-11-11T17:09:35Z

Hello! I am a robot who works on Magic Modules PRs.

I've detected that you're a community contributor. @slevenick, a repository maintainer, has been assigned to assist you and help review your changes.

❓ First time contributing? Click here for more details

Your assigned reviewer will help review your code by:

Ensuring it's backwards compatible, covers common error cases, etc.
Summarizing the change into a user-facing changelog note.
Passes tests, either our "VCR" suite, a set of presubmit tests, or with manual test runs.

You can help make sure that review is quick by running local tests and ensuring they're passing in between each push you make to your PR's branch. Also, try to leave a comment with each push you make, as pushes generally don't generate emails.

If your reviewer doesn't get back to you within a week after your most recent change, please feel free to leave a comment on the issue asking them to take a look! In the absence of a dedicated review dashboard most maintainers manage their pending reviews through email, and those will sometimes get lost in their inbox.

modular-magician · 2022-11-11T17:27:24Z

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

Terraform GA: Diff ( 3 files changed, 365 insertions(+), 7 deletions(-))
Terraform Beta: Diff ( 3 files changed, 315 insertions(+), 39 deletions(-))
TF Validator: Diff ( 2 files changed, 3 insertions(+), 3 deletions(-))

modular-magician · 2022-11-11T18:15:11Z

Tests analytics

Total tests: 2243
Passed tests 1992
Skipped tests: 241
Failed tests: 10

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests

TestAccBillingSubaccount_renameOnDestroy|TestAccFirebaserulesRelease_BasicRelease|TestAccContainerCluster_autoprovisioningDefaultsManagement|TestAccContainerCluster_nodeAutoprovisioningDefaultsShieldedInstance|TestAccContainerCluster_nodeAutoprovisioningDefaultsImageType|TestAccContainerCluster_nodeAutoprovisioningDefaultsDiskType|TestAccContainerCluster_nodeAutoprovisioningDefaultsBootDiskKmsKey|TestAccContainerCluster_nodeAutoprovisioningDefaultsDiskSizeGb|TestAccContainerCluster_nodeAutoprovisioningDefaults|TestAccContainerCluster_nodeAutoprovisioning

modular-magician · 2022-11-11T18:31:53Z

Tests passed during RECORDING mode:
TestAccBillingSubaccount_renameOnDestroy[Debug log]
TestAccFirebaserulesRelease_BasicRelease[Debug log]
TestAccContainerCluster_autoprovisioningDefaultsManagement[Debug log]
TestAccContainerCluster_nodeAutoprovisioningDefaultsShieldedInstance[Debug log]
TestAccContainerCluster_nodeAutoprovisioningDefaultsImageType[Debug log]
TestAccContainerCluster_nodeAutoprovisioningDefaultsDiskType[Debug log]
TestAccContainerCluster_nodeAutoprovisioningDefaultsBootDiskKmsKey[Debug log]
TestAccContainerCluster_nodeAutoprovisioningDefaultsDiskSizeGb[Debug log]
TestAccContainerCluster_nodeAutoprovisioningDefaults[Debug log]
TestAccContainerCluster_nodeAutoprovisioning[Debug log]

All tests passed
View the build log or the debug log for each test

shuyama1 · 2022-11-15T17:37:20Z

mmv1/third_party/terraform/tests/resource_container_cluster_test.go.erb

+	})
+}
+
+func TestAccContainerCluster_autoprovisioningDefaultsUpgradeSettings(t *testing.T) {


Can we also add another acceptance test that verifies the creation/update of blue_green upgrade? Current tests only test the Surge upgrade.

…odules into gke_b_189224778

modular-magician · 2022-11-18T09:58:03Z

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

Terraform GA: Diff ( 3 files changed, 408 insertions(+), 7 deletions(-))
Terraform Beta: Diff ( 3 files changed, 358 insertions(+), 39 deletions(-))
TF Validator: Diff ( 2 files changed, 3 insertions(+), 3 deletions(-))

modular-magician · 2022-11-18T10:46:51Z

Tests analytics

Total tests: 2272
Passed tests 2025
Skipped tests: 244
Failed tests: 3

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests

TestAccFirebaserulesRelease_BasicRelease|TestAccContainerClusterCustomServiceAccount_withAutopilot|TestAccContainerCluster_autoprovisioningDefaultsUpgradeSettings

modular-magician · 2022-11-18T11:01:27Z

Tests passed during RECORDING mode:
TestAccFirebaserulesRelease_BasicRelease[Debug log]
TestAccContainerClusterCustomServiceAccount_withAutopilot[Debug log]
TestAccContainerCluster_autoprovisioningDefaultsUpgradeSettings[Debug log]

All tests passed
View the build log or the debug log for each test

shuyama1 · 2022-11-21T17:21:30Z

mmv1/third_party/terraform/resources/resource_container_cluster.go.erb

+												"max_surge": {
+													Type:        schema.TypeInt,
+													Optional:    true,
+													Description: `The maximum number of nodes that can be created beyond the current size of the node pool during the upgrade process.`,
+												},
+												"max_unavailable": {
+													Type:        schema.TypeInt,
+													Optional:    true,
+													Description: `The maximum number of nodes that can be simultaneously unavailable during the upgrade process.`,
+												},


Looks like we remove the computed from these two field. I wonder if the API will return values for these two fields if we don't explicitly specify them for surge upgrade?

The max_surge and max_unavailable values in this case have been verified to be only necessary for the SURGE. The strategy from SURGE to BLUE GREEN cannot be updated if we leave this attrs in its computed state since d.GetOk will utilise its last stetted value if its values are not present in the current tf file. As a result, even though we didn't include it in the tf file, it will assume that we have provided its (max_surge/max_unavailable) value and initiate the validation.

The ideal scenario would be to enable us to switch from an SURGE strategy to a BLUE GREEN strategy by removing the SURGE attrs (max surge and max unavailable) and adding the BLUE GREEN settings.

In a different instance, API returned zero values and set them into a state file for the blue_green_settings(which not having computed:True) and resulted in an indication that changes needed to be updated in the subsequent apply even though there were no changes in the TF file. I have added computed:True for it in the next commit.

max_surge and max_unavailable, however, are of the Int type. Due to this, it will consider computed to be zero and set its value to 0 if we don't supply computed as True.

…odules into gke_b_189224778

modular-magician · 2022-11-22T12:13:32Z

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

Terraform GA: Diff ( 3 files changed, 413 insertions(+), 7 deletions(-))
Terraform Beta: Diff ( 3 files changed, 363 insertions(+), 39 deletions(-))
TF Validator: Diff ( 2 files changed, 3 insertions(+), 3 deletions(-))

modular-magician · 2022-11-22T13:03:15Z

Tests analytics

Total tests: 2278
Passed tests 2032
Skipped tests: 244
Failed tests: 2

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests

TestAccLoggingBucketConfigProject_cmekSettings|TestAccFirebaserulesRelease_BasicRelease

modular-magician · 2022-11-22T13:04:23Z

Tests passed during RECORDING mode:
TestAccLoggingBucketConfigProject_cmekSettings[Debug log]
TestAccFirebaserulesRelease_BasicRelease[Debug log]

All tests passed
View the build log or the debug log for each test

shuyama1

LGTM in general!
GA test running: https://ci-oss.hashicorp.engineering/buildConfiguration/GoogleCloud_ProviderGoogleCloudMmUpstream/356190

Will approve it after the tests pass

shuyama1 · 2022-11-28T16:23:01Z

mmv1/third_party/terraform/resources/resource_container_cluster.go.erb

+		OauthScopes:    	convertStringArr(config["oauth_scopes"].([]interface{})),
+		ServiceAccount: 	config["service_account"].(string),
+		DiskSizeGb:     	int64(config["disk_size"].(int)),
+		DiskType:       	config["disk_type"].(string),
+		ImageType:      	config["image_type"].(string),
+		BootDiskKmsKey: 	config["boot_disk_kms_key"].(string),
 		Management:		expandManagement(config["management"]),
+		UpgradeSettings:	expandUpgradeSettings(config["upgrade_settings"]),


Only nit: spacing is a bit off here.

shuyama1 · 2022-11-28T16:24:34Z

mmv1/third_party/terraform/tests/resource_container_cluster_test.go.erb

+	}
+
+	return fmt.Sprintf(`
+resource "google_container_cluster" "with_autoprovisioning_upgrade_settings" {


nit: spacing: we use spaces instead of tabs for resource config.

shuyama1 · 2022-11-28T16:24:54Z

mmv1/third_party/terraform/tests/resource_container_cluster_test.go.erb

+
+func testAccContainerCluster_autoprovisioningDefaultsUpgradeSettingsWithBlueGreenStrategy(clusterName string, duration, strategy string) string {
+	return fmt.Sprintf(`
+resource "google_container_cluster" "with_autoprovisioning_upgrade_settings" {


nit: spacing

…odules into gke_b_189224778

modular-magician · 2022-11-29T06:42:07Z

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

Terraform GA: Diff ( 3 files changed, 413 insertions(+), 7 deletions(-))
Terraform Beta: Diff ( 3 files changed, 363 insertions(+), 39 deletions(-))
TF Validator: Diff ( 2 files changed, 3 insertions(+), 3 deletions(-))

modular-magician · 2022-11-29T07:30:03Z

Tests analytics

Total tests: 2302
Passed tests 2048
Skipped tests: 244
Failed tests: 10

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests

TestAccLoggingBucketConfigProject_cmekSettings|TestAccDataFusionInstance_dataFusionInstanceZoneExample|TestAccDataFusionInstance_dataFusionInstanceEventExample|TestAccDataFusionInstance_dataFusionInstanceFullExample|TestAccVertexAIIndex_updated|TestAccVertexAIIndex_vertexAiIndexStreamingExample|TestAccVertexAIIndex_vertexAiIndexExample|TestAccDataFusionInstanceIamPolicyGenerated|TestAccDataFusionInstanceIamMemberGenerated|TestAccDataFusionInstanceIamBindingGenerated

modular-magician · 2022-11-29T08:19:36Z

Tests passed during RECORDING mode:
TestAccLoggingBucketConfigProject_cmekSettings[Debug log]
TestAccDataFusionInstance_dataFusionInstanceZoneExample[Debug log]
TestAccDataFusionInstance_dataFusionInstanceEventExample[Debug log]
TestAccDataFusionInstance_dataFusionInstanceFullExample[Debug log]
TestAccVertexAIIndex_updated[Debug log]
TestAccVertexAIIndex_vertexAiIndexStreamingExample[Debug log]
TestAccVertexAIIndex_vertexAiIndexExample[Debug log]
TestAccDataFusionInstanceIamPolicyGenerated[Debug log]
TestAccDataFusionInstanceIamMemberGenerated[Debug log]
TestAccDataFusionInstanceIamBindingGenerated[Debug log]

All tests passed
View the build log or the debug log for each test

shuyama1

Apologize for the delay on reviewing this. LGTM! Would you mind rebasing it? Should be good to go after.

…odules into gke_b_189224778

modular-magician · 2022-12-06T06:15:30Z

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

Terraform GA: Diff ( 3 files changed, 413 insertions(+), 7 deletions(-))
Terraform Beta: Diff ( 3 files changed, 363 insertions(+), 39 deletions(-))
TF Validator: Diff ( 2 files changed, 3 insertions(+), 3 deletions(-))

modular-magician · 2022-12-06T06:59:39Z

Tests analytics

Total tests: 2325
Passed tests 2078
Skipped tests: 244
Failed tests: 3

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests

TestAccLoggingBucketConfigProject_cmekSettings|TestAccFirebaserulesRelease_BasicRelease|TestAccContainerNodePool_withUpgradeSettings

modular-magician · 2022-12-06T07:15:15Z

Tests passed during RECORDING mode:
TestAccLoggingBucketConfigProject_cmekSettings[Debug log]
TestAccFirebaserulesRelease_BasicRelease[Debug log]
TestAccContainerNodePool_withUpgradeSettings[Debug log]

All tests passed
View the build log or the debug log for each test

shuyama1 · 2022-12-07T23:13:31Z

/gcbrun

shuyama1

LGTM! Thanks! GA tests passed: https://ci-oss.hashicorp.engineering/buildConfiguration/GoogleCloud_ProviderGoogleCloudMmUpstream/357900
Will merge after the rerun succeed!

modular-magician · 2022-12-07T23:30:55Z

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

Terraform GA: Diff ( 3 files changed, 413 insertions(+), 7 deletions(-))
Terraform Beta: Diff ( 3 files changed, 363 insertions(+), 39 deletions(-))
TF Validator: Diff ( 2 files changed, 3 insertions(+), 3 deletions(-))

modular-magician · 2022-12-08T00:19:41Z

Tests analytics

Total tests: 2329
Passed tests 2083
Skipped tests: 244
Failed tests: 2

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests

TestAccFirebaserulesRelease_BasicRelease|TestAccLoggingBucketConfigProject_cmekSettings

modular-magician · 2022-12-08T00:20:52Z

Tests passed during RECORDING mode:
TestAccFirebaserulesRelease_BasicRelease[Debug log]
TestAccLoggingBucketConfigProject_cmekSettings[Debug log]

All tests passed
View the build log or the debug log for each test

Added upgrade_settings with validation for SURGE strategy with promot…

902b36a

…ing min_cpu_platform

modular-magician requested a review from slevenick November 11, 2022 17:09

mitj04 added 2 commits November 11, 2022 22:41

Fixing indentation

3c0adcc

Fixing indentation

efe1ede

mitj04 marked this pull request as ready for review November 12, 2022 04:48

shuyama1 requested review from shuyama1 and removed request for slevenick November 12, 2022 19:42

shuyama1 reviewed Nov 15, 2022

View reviewed changes

mitj04 added 3 commits November 17, 2022 05:47

Merge branch 'main' of https://github.com/GoogleCloudPlatform/magic-m…

583fc5a

…odules into gke_b_189224778

Adding TC for Blue Green Strategy

fe6e981

Merge branch 'main' of https://github.com/GoogleCloudPlatform/magic-m…

a3fa346

…odules into gke_b_189224778

modular-magician requested a review from shuyama1 November 18, 2022 09:44

shuyama1 reviewed Nov 21, 2022

View reviewed changes

mitj04 added 2 commits November 22, 2022 05:14

Merge branch 'main' of https://github.com/GoogleCloudPlatform/magic-m…

77e2766

…odules into gke_b_189224778

Adding computed:True for the blue_green_settings

f1126e4

modular-magician requested a review from shuyama1 November 22, 2022 11:57

shuyama1 reviewed Nov 28, 2022

View reviewed changes

mitj04 added 2 commits November 29, 2022 06:10

Merge branch 'main' of https://github.com/GoogleCloudPlatform/magic-m…

3f06bc2

…odules into gke_b_189224778

Fixed the spacing

aeabb1b

modular-magician requested a review from shuyama1 November 29, 2022 06:27

shuyama1 reviewed Dec 5, 2022

View reviewed changes

Merge branch 'main' of https://github.com/GoogleCloudPlatform/magic-m…

6f46ea3

…odules into gke_b_189224778

modular-magician requested a review from shuyama1 December 6, 2022 05:59

shuyama1 approved these changes Dec 7, 2022

View reviewed changes

shuyama1 merged commit 2c1b204 into GoogleCloudPlatform:main Dec 8, 2022

mitj04 mentioned this pull request Dec 8, 2022

Add support for the rest of the AutoprovisioningNodePoolDefaults settings in google_container_cluster hashicorp/terraform-provider-google#9180

Closed

modular-magician requested a review from shuyama1 January 25, 2023 00:47

clebio mentioned this pull request Apr 13, 2023

Add support for upgrade_settings in google_container_cluster NAP hashicorp/terraform-provider-google#5580

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added upgrade_settings with validation for SURGE strategy with promoting min_cpu_platform to GA #6815

Added upgrade_settings with validation for SURGE strategy with promoting min_cpu_platform to GA #6815

mitj04 commented Nov 11, 2022 •

edited by shuyama1

modular-magician commented Nov 11, 2022

modular-magician commented Nov 11, 2022

modular-magician commented Nov 11, 2022

modular-magician commented Nov 11, 2022

modular-magician commented Nov 11, 2022

shuyama1 Nov 15, 2022

mitj04 Nov 18, 2022

mitj04 Nov 21, 2022

modular-magician commented Nov 18, 2022

modular-magician commented Nov 18, 2022

modular-magician commented Nov 18, 2022

shuyama1 Nov 21, 2022

mitj04 Nov 22, 2022

modular-magician commented Nov 22, 2022

modular-magician commented Nov 22, 2022

modular-magician commented Nov 22, 2022

shuyama1 left a comment

shuyama1 Nov 28, 2022

shuyama1 Nov 28, 2022

shuyama1 Nov 28, 2022

modular-magician commented Nov 29, 2022

modular-magician commented Nov 29, 2022

modular-magician commented Nov 29, 2022

shuyama1 left a comment

modular-magician commented Dec 6, 2022

modular-magician commented Dec 6, 2022

modular-magician commented Dec 6, 2022

shuyama1 commented Dec 7, 2022

shuyama1 left a comment

modular-magician commented Dec 7, 2022

modular-magician commented Dec 8, 2022

modular-magician commented Dec 8, 2022

Added upgrade_settings with validation for SURGE strategy with promoting min_cpu_platform to GA #6815

Added upgrade_settings with validation for SURGE strategy with promoting min_cpu_platform to GA #6815

Conversation

mitj04 commented Nov 11, 2022 • edited by shuyama1

modular-magician commented Nov 11, 2022

modular-magician commented Nov 11, 2022

modular-magician commented Nov 11, 2022

Diff report

modular-magician commented Nov 11, 2022

Tests analytics

Action taken

modular-magician commented Nov 11, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

modular-magician commented Nov 18, 2022

Diff report

modular-magician commented Nov 18, 2022

Tests analytics

Action taken

modular-magician commented Nov 18, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

modular-magician commented Nov 22, 2022

Diff report

modular-magician commented Nov 22, 2022

Tests analytics

Action taken

modular-magician commented Nov 22, 2022

shuyama1 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

modular-magician commented Nov 29, 2022

Diff report

modular-magician commented Nov 29, 2022

Tests analytics

Action taken

modular-magician commented Nov 29, 2022

shuyama1 left a comment

Choose a reason for hiding this comment

modular-magician commented Dec 6, 2022

Diff report

modular-magician commented Dec 6, 2022

Tests analytics

Action taken

modular-magician commented Dec 6, 2022

shuyama1 commented Dec 7, 2022

shuyama1 left a comment

Choose a reason for hiding this comment

modular-magician commented Dec 7, 2022

Diff report

modular-magician commented Dec 8, 2022

Tests analytics

Action taken

modular-magician commented Dec 8, 2022

mitj04 commented Nov 11, 2022 •

edited by shuyama1