Re-add monitoring monitored project, without metrics scope resource #5235

nat-henderson · 2021-09-23T00:48:30Z

This is an unclean revert of #5229, which is itself a clean revert of #5218.

The unclean part is commit aa2a676 (edit to add: also aa40f0a) - which, if you like, you can review instead of re-looking at all the files in this, since almost none of them needed to change.

The only difference is that this does not include metrics scope, which was a no-op resource (not even a useful data source), now that we have extracted monitored project as a fine-grained resource.

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_monitoring_monitored_project`

modular-magician · 2021-09-23T00:59:36Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 17 files changed, 525 insertions(+), 25 deletions(-))
Terraform Beta: Diff ( 17 files changed, 517 insertions(+), 20 deletions(-))

modular-magician · 2021-09-23T04:09:23Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 9 files changed, 20 insertions(+), 25 deletions(-))
Terraform Beta: Diff ( 17 files changed, 517 insertions(+), 20 deletions(-))

modular-magician · 2021-09-23T14:53:17Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 9 files changed, 20 insertions(+), 25 deletions(-))
Terraform Beta: Diff ( 17 files changed, 517 insertions(+), 21 deletions(-))

rileykarson

Seems like the Magician ate an import in tpgb, it may need a kick (making some changes'll do it)

tpgtools/overrides/monitoring/metrics_scope.yaml

tpgtools/overrides/monitoring/beta/metrics_scope.yaml

tpgtools/overrides/monitoring/samples/monitoredproject/meta.yaml

tpgtools/serialization.go

rileykarson · 2021-09-23T16:27:52Z

tpgtools/templates/serialization.go.tmpl

@@ -110,14 +110,18 @@ func ConvertSampleJSONToHCL(resourceType string, version string, b []byte) (stri
 		{{- else }}
 	// If not found in sample version, fallthrough to GA
 	switch resourceType {
-	{{- range $res := $resList   }}
+			{{- range $res := $resList   }}


Suggested change

{{- range $res := $resList }}

{{- range $res := $resList }}

tpgtools/templates/sweeper.go.tmpl

google-cla · 2021-09-23T16:48:32Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

trodge · 2021-09-23T16:52:48Z

@googlebot I consent.

modular-magician · 2021-09-23T16:59:14Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 9 files changed, 20 insertions(+), 25 deletions(-))
Terraform Beta: Diff ( 17 files changed, 517 insertions(+), 20 deletions(-))

modular-magician · 2021-09-23T17:09:54Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 9 files changed, 20 insertions(+), 25 deletions(-))
Terraform Beta: Diff ( 17 files changed, 517 insertions(+), 20 deletions(-))

modular-magician · 2021-09-23T17:22:11Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 9 files changed, 20 insertions(+), 19 deletions(-))
Terraform Beta: Diff ( 16 files changed, 517 insertions(+), 13 deletions(-))

modular-magician · 2021-09-23T19:09:23Z

I have triggered VCR tests in RECORDING mode for the following tests that failed during VCR: TestAccMonitoringMonitoredProject_BasicMonitoredProject You can view the result here: https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=207234

rileykarson · 2021-09-23T19:39:42Z

tpgtools/overrides/monitoring/beta/monitored_project.yaml

+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+- type: APPEND_TO_BASE_PATH


I think the need for this override is a sign that the DCL misrepresented the API. This means that to override the endpoint- a DCL feature- users need to create two different monitoring clients and pick the right one depending on the resource, with no external indicator which one should get chosen.

This is non-blocking because this code was already reviewed in an earlier iteration, although I would have blocked on this otherwise.

I disagree - I think this is an implication of Terraform's pre-existing choice about how to handle the monitoring API having 2 GA versions and that it won't come up in other APIs and also wouldn't if TF didn't have monitoring resources already.

But we argued about this with Scott, thanks for letting that decision stand. I'll remember that we should re-discuss this with you personally if it ever matters in the future. Monitoring may be singularly weird on this one.

rileykarson · 2021-09-23T19:40:25Z

tpgtools/templates/resource.go.tmpl

-    return nil
-  }
-{{- end}}
+	skip, err := {{$.SkipDeleteFunction}}(config, obj)


Thanks for the formatting fix!

nat-henderson · 2021-09-23T21:34:49Z

Great - upgraded the DCL in another PR so we do need to re-push those, but that should be it and we can merge after that. I'll push that now.

nat-henderson · 2021-09-23T21:36:29Z

I'll go ahead and let that finish before merging but it should be fine.

modular-magician · 2021-09-23T21:46:57Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 6 files changed, 14 insertions(+), 16 deletions(-))
Terraform Beta: Diff ( 14 files changed, 442 insertions(+), 13 deletions(-))

nat-henderson · 2021-09-24T00:32:26Z

Okay, changed my mind - tired of waiting for test runs to finish and getting merge conflicts on the go.mod again. Going to resolve that merge conflict and submit without waiting.

* Added monitoring metrics scope and monitored project resources. * Ran make upgrade-dcl * Fixed indentation * Copied tpgtools go.sum lines into mmv1 go.sum. * Apply suggestions from code review Co-authored-by: Scott Suarez <[email protected]> * Attempted to fix strange downstream change in iam.go. * Added missing custom serializer to GA version, upgraded DCL again. * Update ID format so import is possible. Co-authored-by: Scott Suarez <[email protected]> Co-authored-by: Nathan Mckinley <[email protected]>

modular-magician · 2021-09-24T00:45:52Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 6 files changed, 14 insertions(+), 16 deletions(-))
Terraform Beta: Diff ( 13 files changed, 441 insertions(+), 12 deletions(-))

…oogleCloudPlatform#5235) * Added monitoring metrics scope and monitored project resources. (GoogleCloudPlatform#5218) * Added monitoring metrics scope and monitored project resources. * Ran make upgrade-dcl * Fixed indentation * Copied tpgtools go.sum lines into mmv1 go.sum. * Apply suggestions from code review Co-authored-by: Scott Suarez <[email protected]> * Attempted to fix strange downstream change in iam.go. * Added missing custom serializer to GA version, upgraded DCL again. * Update ID format so import is possible. Co-authored-by: Scott Suarez <[email protected]> Co-authored-by: Nathan Mckinley <[email protected]> * Revert reversion of metrics scopes * Remove scope from provider. * Remove from GA, remove metrics scope from provider. * Added missing strings import for mmv1 dataproc cluster test. * Ran make serialize. * Removed unneeded overrides. Re-ran make serialize. * Removed undeletable override. Co-authored-by: Thomas Rodgers <[email protected]> Co-authored-by: Scott Suarez <[email protected]>

nat-henderson requested review from rileykarson and trodge September 23, 2021 00:48

google-cla bot added the cla: yes label Sep 23, 2021

nat-henderson requested a review from ScottSuarez September 23, 2021 00:48

rileykarson reviewed Sep 23, 2021

View reviewed changes

google-cla bot added cla: no and removed cla: yes labels Sep 23, 2021

google-cla bot added cla: yes and removed cla: no labels Sep 23, 2021

ScottSuarez removed their request for review September 23, 2021 19:25

rileykarson approved these changes Sep 23, 2021

View reviewed changes

nat-henderson mentioned this pull request Sep 23, 2021

tpgtools should use the data that dcl samples export re: variables hashicorp/terraform-provider-google#10147

Open

trodge and others added 5 commits September 23, 2021 17:33

Revert reversion of metrics scopes

e6cc1d0

Remove scope from provider.

071e5fd

Remove from GA, remove metrics scope from provider.

3bf4f37

Added missing strings import for mmv1 dataproc cluster test.

d20b00a

trodge added 3 commits September 23, 2021 17:33

Ran make serialize.

6229f67

Removed unneeded overrides. Re-ran make serialize.

b77d548

Removed undeletable override.

761a587

nat-henderson force-pushed the metrics-monitored-project-360-noscope branch from fcb771b to 761a587 Compare September 24, 2021 00:34

nat-henderson merged commit 8353df6 into master Sep 24, 2021

nat-henderson deleted the metrics-monitored-project-360-noscope branch September 24, 2021 00:54

This was referenced Sep 24, 2021

Re-add monitoring monitored project, without metrics scope resource hashicorp/terraform-provider-google-beta#3658

Merged

Re-add monitoring monitored project, without metrics scope resource hashicorp/terraform-provider-google#10153

Merged

rileykarson mentioned this pull request Sep 30, 2021

Add support for Stackdriver monitored project hashicorp/terraform-provider-google#2605

Closed

melinath mentioned this pull request Oct 1, 2021

Failing test(s): TestAccMonitoringMonitoredProject_BasicMonitoredProject hashicorp/terraform-provider-google#10218

Closed

marktru mentioned this pull request Nov 9, 2021

Metrics Scopes API GA Terraform module hashicorp/terraform-provider-google#10527

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Re-add monitoring monitored project, without metrics scope resource #5235

Re-add monitoring monitored project, without metrics scope resource #5235

nat-henderson commented Sep 23, 2021 •

edited

Loading

modular-magician commented Sep 23, 2021

modular-magician commented Sep 23, 2021

modular-magician commented Sep 23, 2021

rileykarson left a comment

rileykarson Sep 23, 2021

google-cla bot commented Sep 23, 2021

trodge commented Sep 23, 2021

modular-magician commented Sep 23, 2021

modular-magician commented Sep 23, 2021

modular-magician commented Sep 23, 2021

modular-magician commented Sep 23, 2021

rileykarson Sep 23, 2021

nat-henderson Sep 23, 2021

rileykarson Sep 23, 2021

nat-henderson commented Sep 23, 2021

nat-henderson commented Sep 23, 2021

modular-magician commented Sep 23, 2021

nat-henderson commented Sep 24, 2021

modular-magician commented Sep 24, 2021

Re-add monitoring monitored project, without metrics scope resource #5235

Re-add monitoring monitored project, without metrics scope resource #5235

Conversation

nat-henderson commented Sep 23, 2021 • edited Loading

modular-magician commented Sep 23, 2021

Diff report:

modular-magician commented Sep 23, 2021

Diff report:

modular-magician commented Sep 23, 2021

Diff report:

rileykarson left a comment

Choose a reason for hiding this comment

rileykarson Sep 23, 2021

Choose a reason for hiding this comment

google-cla bot commented Sep 23, 2021

trodge commented Sep 23, 2021

modular-magician commented Sep 23, 2021

Diff report:

modular-magician commented Sep 23, 2021

Diff report:

modular-magician commented Sep 23, 2021

Diff report:

modular-magician commented Sep 23, 2021

rileykarson Sep 23, 2021

Choose a reason for hiding this comment

nat-henderson Sep 23, 2021

Choose a reason for hiding this comment

rileykarson Sep 23, 2021

Choose a reason for hiding this comment

nat-henderson commented Sep 23, 2021

nat-henderson commented Sep 23, 2021

modular-magician commented Sep 23, 2021

Diff report:

nat-henderson commented Sep 24, 2021

modular-magician commented Sep 24, 2021

Diff report:

nat-henderson commented Sep 23, 2021 •

edited

Loading