Attached clusters data sources #7086

hankfreund · 2023-01-06T15:01:05Z

Added new datasource for retrieving valid attached cluster platform versions google_container_attached_versions
Added new datasource for retrieving a manifest to apply to the user cluster for boot-strapping google_container_attached_install_manifest

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)

google_container_attached_versions

google_container_attached_install_manifest

modular-magician · 2023-01-06T15:02:08Z

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

I've detected that you're a community contributor. @ScottSuarez, 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 · 2023-01-06T15:17:15Z

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 ( 8 files changed, 321 insertions(+), 7 deletions(-))
Terraform Beta: Diff ( 8 files changed, 321 insertions(+), 7 deletions(-))
TF Validator: Diff ( 2 files changed, 3 insertions(+), 3 deletions(-))
TF OiCS: Diff ( 2 files changed, 12 insertions(+), 2 deletions(-))

modular-magician · 2023-01-06T16:03:37Z

Tests analytics

Total tests: 2393
Passed tests 2135
Skipped tests: 248
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

TestAccFirebaserulesRelease_BasicRelease|TestAccDatastreamStream_datastreamStreamFullExample|TestAccDatastreamStream_update|TestAccDatastreamStream_datastreamStreamBasicExample|TestAccContainerCluster_withInvalidGatewayApiConfigChannel|TestAccContainerAttachedCluster_update|TestAccContainerAttachedCluster_containerAttachedClusterFullExample|TestAccContainerAttachedCluster_containerAttachedClusterBasicExample|TestAccComputeForwardingRule_update|TestAccDataSourceGoogleContainerAttachedInstallManifest

modular-magician · 2023-01-06T16:26:48Z

Tests passed during RECORDING mode:
TestAccFirebaserulesRelease_BasicRelease[Debug log]
TestAccDatastreamStream_datastreamStreamFullExample[Debug log]
TestAccDatastreamStream_update[Debug log]
TestAccDatastreamStream_datastreamStreamBasicExample[Debug log]
TestAccContainerCluster_withInvalidGatewayApiConfigChannel[Debug log]
TestAccContainerAttachedCluster_update[Debug log]
TestAccContainerAttachedCluster_containerAttachedClusterFullExample[Debug log]
TestAccContainerAttachedCluster_containerAttachedClusterBasicExample[Debug log]
TestAccComputeForwardingRule_update[Debug log]
TestAccDataSourceGoogleContainerAttachedInstallManifest[Debug log]

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

ScottSuarez · 2023-01-06T23:50:50Z

Hey ! wanted to check in. Is there a corresponding issues for these datasources being added? Looking for more information on the need and reason for why. internal or external

hankfreund · 2023-01-09T14:28:10Z

Hi! Sorry for the confusion. These datasources round out the required implementation for the new attached clusters API. You can see my previous PR for adding the resource here: #6858
There is not a corresponding external issue for this. We can chat offline if there's any other info you need.

ScottSuarez

Seems fairly straightforward.. Here are some initial thoughts.

ScottSuarez · 2023-01-10T01:45:05Z

...third_party/terraform/data_sources/data_source_google_container_attached_install_manifest.go

+	return &schema.Resource{
+		Read: dataSourceGoogleContainerAttachedInstallManifestRead,
+		Schema: map[string]*schema.Schema{
+			"location": {
+				Type:     schema.TypeString,
+				Required: true,
+			},
+			"parent": {
+				Type:     schema.TypeString,
+				Required: true,
+			},
+			"cluster_id": {
+				Type:     schema.TypeString,
+				Required: true,
+			},
+			"platform_version": {
+				Type:     schema.TypeString,
+				Required: true,
+			},
+			"manifest": {
+				Type:     schema.TypeString,
+				Computed: true,
+			},
+		},
+	}
+}


Switching from "parent" to project, region might be a better UX. If region is identical to location this region can be omitted.

ScottSuarez · 2023-01-10T01:45:50Z

mmv1/templates/terraform/examples/container_attached_cluster_basic.tf.erb

@@ -1,6 +1,11 @@
 data "google_project" "project" {


Both data sources could use a datasource markdown file.

Checkout https://github.com/GoogleCloudPlatform/magic-modules/tree/main/mmv1/third_party/terraform/website/docs/d

ScottSuarez · 2023-01-10T01:46:56Z

mmv1/templates/terraform/examples/container_attached_cluster_basic.tf.erb

@@ -1,6 +1,11 @@
 data "google_project" "project" {
 }


I would prefer splitting this down to two PRs.. one for each data source. To keep the context more focused. It doesn't seem like the data sources are dependent on eachother.

modular-magician · 2023-01-10T16:14:54Z

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 ( 7 files changed, 225 insertions(+), 7 deletions(-))
Terraform Beta: Diff ( 7 files changed, 225 insertions(+), 7 deletions(-))
TF Validator: Diff ( 2 files changed, 3 insertions(+), 3 deletions(-))
TF OiCS: Diff ( 2 files changed, 12 insertions(+), 2 deletions(-))

hankfreund · 2023-01-10T16:16:57Z

I've split the PR into two. This one is now only for google_container_attached_versions and test modifications to use it.
Addressed the review comments.

modular-magician · 2023-01-10T17:06:27Z

Tests analytics

Total tests: 2400
Passed tests 2143
Skipped tests: 251
Failed tests: 6

Action taken

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

TestAccFirebaserulesRelease_BasicRelease|TestAccContainerCluster_withInvalidGatewayApiConfigChannel|TestAccDataSourceSqlDatabaseInstances_tierFilter|TestAccDataSourceSqlDatabaseInstances_regionFilter|TestAccDataSourceSqlDatabaseInstances_databaseVersionFilter|TestAccDataSourceSqlDatabaseInstances_basic

modular-magician · 2023-01-10T17:31:07Z

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

Tests failed during RECORDING mode:
TestAccDataSourceSqlDatabaseInstances_tierFilter[Error message] [Debug log]
TestAccDataSourceSqlDatabaseInstances_regionFilter[Error message] [Debug log]
TestAccDataSourceSqlDatabaseInstances_databaseVersionFilter[Error message] [Debug log]
TestAccDataSourceSqlDatabaseInstances_basic[Error message] [Debug log]

Please fix these to complete your PR
View the build log or the debug log for each test

ScottSuarez

Thanks Hank! The failures are unrelated

hankfreund added 4 commits November 29, 2022 11:27

Split attached cluster data sources into a separate PR

32d4fb3

Merge branch 'GoogleCloudPlatform:main' into attached_data_sources

6907a88

Update tests to use attached clusters platform version datasource.

6e9df90

Code formatting.

ea0934f

modular-magician requested a review from ScottSuarez January 6, 2023 15:02

ScottSuarez requested changes Jan 10, 2023

View reviewed changes

hankfreund added 2 commits January 10, 2023 07:05

Move container_attached_install_manifest to a separate PR.

22370fc

Add markdown for container_attached_versions.

7ae9974

modular-magician requested a review from ScottSuarez January 10, 2023 15:59

ScottSuarez approved these changes Jan 10, 2023

View reviewed changes

ScottSuarez merged commit eebb210 into GoogleCloudPlatform:main Jan 10, 2023

hankfreund mentioned this pull request Jan 10, 2023

Add container_attached_install_manifest data source. #7108

Merged

5 tasks

This was referenced Jan 11, 2023

Add container_attached_install_manifest data source. hashicorp/terraform-provider-google-beta#5080

Merged

Add container_attached_install_manifest data source. hashicorp/terraform-provider-google#13455

Merged

melinath added the service/container label Feb 7, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Attached clusters data sources #7086

Attached clusters data sources #7086

hankfreund commented Jan 6, 2023

modular-magician commented Jan 6, 2023

modular-magician commented Jan 6, 2023

modular-magician commented Jan 6, 2023

modular-magician commented Jan 6, 2023

ScottSuarez commented Jan 6, 2023

hankfreund commented Jan 9, 2023

ScottSuarez left a comment

ScottSuarez Jan 10, 2023

ScottSuarez Jan 10, 2023

ScottSuarez Jan 10, 2023

modular-magician commented Jan 10, 2023

hankfreund commented Jan 10, 2023

modular-magician commented Jan 10, 2023

modular-magician commented Jan 10, 2023

ScottSuarez left a comment

Attached clusters data sources #7086

Attached clusters data sources #7086

Conversation

hankfreund commented Jan 6, 2023

modular-magician commented Jan 6, 2023

modular-magician commented Jan 6, 2023

Diff report

modular-magician commented Jan 6, 2023

Tests analytics

Action taken

modular-magician commented Jan 6, 2023

ScottSuarez commented Jan 6, 2023

hankfreund commented Jan 9, 2023

ScottSuarez left a comment

Choose a reason for hiding this comment

ScottSuarez Jan 10, 2023

Choose a reason for hiding this comment

ScottSuarez Jan 10, 2023

Choose a reason for hiding this comment

ScottSuarez Jan 10, 2023

Choose a reason for hiding this comment

modular-magician commented Jan 10, 2023

Diff report

hankfreund commented Jan 10, 2023

modular-magician commented Jan 10, 2023

Tests analytics

Action taken

modular-magician commented Jan 10, 2023

ScottSuarez left a comment

Choose a reason for hiding this comment