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

Add maintenance_interval field to Instance Template and Instance scheduling. #7365

Merged

Conversation

kubalaguna
Copy link
Contributor

@kubalaguna kubalaguna commented Feb 28, 2023

Add maintenance_interval field to Instance Template scheduling.

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).
  • 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).
  • Generated Terraform providers, and ran make test and make lint in the generated providers to ensure it passes unit and linter tests.
  • Ran relevant acceptance tests using my own Google Cloud project and credentials (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)

compute: added `maintenance_interval` field to `instance-template` and `instance` resource (beta)

@kubalaguna kubalaguna marked this pull request as ready for review February 28, 2023 12:12
@kubalaguna kubalaguna requested a review from a team as a code owner February 28, 2023 12:12
@kubalaguna kubalaguna requested review from zli82016 and removed request for a team February 28, 2023 12:12
@zli82016
Copy link
Member

/gcbrun

1 similar comment
@zli82016
Copy link
Member

/gcbrun

@@ -1441,7 +1441,13 @@ overrides: !ruby/object:Overrides::ResourceOverrides
custom_code: !ruby/object:Provider::Terraform::CustomCode
encoder: 'templates/terraform/encoders/normalize_group.go.erb'
InstanceTemplate: !ruby/object:Overrides::Terraform::ResourceOverride
exclude: true
examples:
Copy link
Member

@zli82016 zli82016 Feb 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For some reason, exclude: true is needed here and the tests should be added to this fie https://github.com/GoogleCloudPlatform/magic-modules/blob/main/mmv1/third_party/terraform/tests/resource_compute_instance_template_test.go.erb

Otherwise, without exclude: true here, resource_compute_instance_template_sweeper_test.go will be generated automatically. However, there is already the handwritten sweeper file resource_compute_instance_template_sweeper_test.go https://github.com/GoogleCloudPlatform/magic-modules/blob/main/mmv1/third_party/terraform/tests/resource_compute_instance_template_sweeper_test.go. That is the reason for the failed generate-diffs check.
https://pantheon.corp.google.com/cloud-build/builds/4faf9424-5347-4c66-8160-cb2ebb01905e?project=graphite-docker-images&jsmode=O&mods=logs_tg_staging

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many thanks Zhenhua. You saved me hours of debugging.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are welcome.

@zli82016
Copy link
Member

zli82016 commented Mar 1, 2023

/gcbrun

@modular-magician
Copy link
Collaborator

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

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 0
Passed tests 0
Skipped tests: 0
Failed tests: 0

Errors occurred during REPLAYING mode. Please fix them to complete your PR
View the build log

@kubalaguna kubalaguna force-pushed the feature/maintenance-interval branch from 0c9c73c to 896cae0 Compare March 1, 2023 18:49
@zli82016
Copy link
Member

zli82016 commented Mar 1, 2023

/gcbrun

@modular-magician
Copy link
Collaborator

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

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 0
Passed tests 0
Skipped tests: 0
Failed tests: 0

Errors occurred during REPLAYING mode. Please fix them to complete your PR
View the build log

@zli82016
Copy link
Member

zli82016 commented Mar 1, 2023

/gcbrun

@modular-magician
Copy link
Collaborator

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

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 2483
Passed tests 2224
Skipped tests: 255
Failed tests: 4

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests
TestAccComputeInstanceTemplate_maintenance_interval|TestAccFirebaserulesRelease_BasicRelease|TestAccWorkstationsWorkstationConfig_workstationConfigEncryptionKeyExample|TestAccSqlDatabaseInstance_Timezone

@zli82016
Copy link
Member

zli82016 commented Mar 1, 2023

  1. The field maintenanceinterval is not in the v1 API, but it is in beta API. I think that is the reason that terraform-provider-google-test failed.

  2. Please check the how to doc to add a beta field
    https://googlecloudplatform.github.io/magic-modules/docs/how-to/add-mmv1-resource/#adding-beta-fields
    https://googlecloudplatform.github.io/magic-modules/docs/how-to/add-handwritten-test/#testing-beta-features

  3. The field needs to be added to the resource documentation file https://github.com/GoogleCloudPlatform/magic-modules/blob/main/mmv1/third_party/terraform/website/docs/r/compute_instance_template.html.markdown?plain=1

    How to doc: https://googlecloudplatform.github.io/magic-modules/docs/how-to/update-handwritten-documentation/

  4. Is there a githug issue to add the field? If yes, please add it to the description.

@modular-magician
Copy link
Collaborator

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

Tests failed during RECORDING mode:
TestAccComputeInstanceTemplate_maintenance_interval[Error message] [Debug log]
TestAccWorkstationsWorkstationConfig_workstationConfigEncryptionKeyExample[Error message] [Debug log]

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

@zli82016
Copy link
Member

zli82016 commented Mar 2, 2023

/gcbrun

@modular-magician
Copy link
Collaborator

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 ( 1 file changed, 4 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 2 files changed, 101 insertions(+), 2 deletions(-))
TF Validator: Diff ( 2 files changed, 3 insertions(+), 3 deletions(-))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 0
Passed tests 0
Skipped tests: 0
Failed tests: 0

Errors occurred during REPLAYING mode. Please fix them to complete your PR
View the build log

@zli82016
Copy link
Member

zli82016 commented Mar 2, 2023

/gcbrun

@modular-magician
Copy link
Collaborator

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 ( 1 file changed, 4 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 2 files changed, 101 insertions(+), 2 deletions(-))
TF Validator: Diff ( 2 files changed, 3 insertions(+), 3 deletions(-))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 2487
Passed tests 2225
Skipped tests: 258
Failed tests: 4

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests
TestAccDataSourceGoogleServiceAccountIdToken_impersonation|TestAccFirebaserulesRelease_BasicRelease|TestAccComputeInstanceTemplate_maintenance_interval|TestAccWorkstationsWorkstationConfig_workstationConfigEncryptionKeyExample

@modular-magician
Copy link
Collaborator

Tests passed during RECORDING mode:
TestAccDataSourceGoogleServiceAccountIdToken_impersonation[Debug log]
TestAccFirebaserulesRelease_BasicRelease[Debug log]
TestAccWorkstationsWorkstationConfig_workstationConfigEncryptionKeyExample[Debug log]

Tests failed during RECORDING mode:
TestAccComputeInstanceTemplate_maintenance_interval[Error message] [Debug log]

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

@zli82016
Copy link
Member

zli82016 commented Mar 3, 2023

/gcbrun

@kubalaguna
Copy link
Contributor Author

kubalaguna commented Mar 6, 2023

This is the doc to run acceptance tests locally https://googlecloudplatform.github.io/magic-modules/docs/getting-started/run-provider-tests/. You can run tests locally, which is faster than running the checks for a PR.

Yeah I tried that. It stopped working for me on cloudtop at some point. I'd be happy to provide some feedback if somebody is interested in making developing magic-modules more user-friendly on cloudtops.

Comment on lines 6132 to 6138
- !ruby/object:Api::Type::Enum
name: 'maintenanceInterval'
description: |
Specifies the frequency of planned maintenance events.
values:
- :PERIODIC
min_version: "beta"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like there is a conflict with the main branch now. You can delete the code block.

@kubalaguna kubalaguna force-pushed the feature/maintenance-interval branch from 407154a to 4cfd351 Compare March 7, 2023 09:39
@zli82016
Copy link
Member

zli82016 commented Mar 7, 2023

/gcbrun

@modular-magician
Copy link
Collaborator

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

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 2492
Passed tests 2232
Skipped tests: 258
Affected tests: 2

Action taken

Found 2 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
TestAccFirebaserulesRelease_BasicRelease|TestAccWorkstationsWorkstationConfig_workstationConfigEncryptionKeyExample

Get to know how VCR tests work

@modular-magician
Copy link
Collaborator

Tests passed during RECORDING mode:
TestAccFirebaserulesRelease_BasicRelease[Debug log]

Tests failed during RECORDING mode:
TestAccWorkstationsWorkstationConfig_workstationConfigEncryptionKeyExample[Error message] [Debug log]

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

Copy link
Member

@zli82016 zli82016 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@zli82016 zli82016 merged commit 3e99c66 into GoogleCloudPlatform:main Mar 7, 2023
dbjnbnrj added a commit to dbjnbnrj/magic-modules that referenced this pull request Mar 15, 2023
Adding AuthorizedOrgsDesc to AccessContextManager (GoogleCloudPlatform#7178)

Allow project field in Firebase apps datasource (GoogleCloudPlatform#7300)

* Allow project field in Firebase apps datasource

* Add tf_test to the display name

* update to capitalized dependencies
Rename compute_(region)_per_instance_config test files to .erb files … (GoogleCloudPlatform#7409)

* Rename compute_(region)_per_instance_config test files to .erb files since the resources have fields in beta.

* Omit stateful_ip tests for GA
Bootstrap network cleanup (GoogleCloudPlatform#7367)

* Update bootstrapped networks for alloydb

* Update bootstrapped networks for redis

* Update bootstrapped networks for vertexai

* Update comment for BootstrapSharedTestNetwork to be more clear
Fix override directory concat (GoogleCloudPlatform#7413)

Bootstrap required permissions for composer environment tests (GoogleCloudPlatform#7391)

* Bootstrap the required permissions

* Make BootstrapAllPSARoles actually work

Also adds some helper functions for debugging what the bootstrap
function does. It will now log the roles that were missing in the
policy.

* Add missing return, fix nits

* Fix typo in service agent name

* Account for newly exported test functions
Add maintenance_interval field to Instance Template and Instance scheduling. (GoogleCloudPlatform#7365)

* Move testing to handwritten

* Move testing to handwritten

* Fix lint

* Fix typo

* Add remove step and update documentation

* Add remove step and update documentation

* Fix comparing string to nil

* Add field to handwritten defs

* rebase on main & resolve conflicts

* Fix doc formatting

* Add maintenance interval to Instance resource

* Add maintenance interval to Instance resource

* Add maintenance interval to Instance resource

* Change function names to valid

* Fix type mismatch

* Fix type mismatch

* Fix function name
Fix naming of resource policies in tests to be sweepable (GoogleCloudPlatform#7412)

Skip TestAccWorkstationsWorkstationConfig_workstationConfigEncryptionKeyExample (GoogleCloudPlatform#7415)

Add support for Apigee Sharedflow (GoogleCloudPlatform#7202)

* added sharedflows mmv1 with error

* fix indentation issue for sharedflow

* add semi auto generated resourceApigeeSharedFlow

* remove sharedflow mmv1 and add sharedflow manual provider

* adding files for debugging review, test have issue

* adding files for debugging review, test have issue

* update test, doc, provider for apigee

* Update api.yaml

remove empty new line

* Update api.yaml

* Rename resource_apigee_shared_flow_generated_test.go to resource_apigee_shared_flow_test.go

* Update resource_apigee_shared_flow.go

update import regex, change debug messages to log.Printf

* Update mmv1/third_party/terraform/utils/provider.go.erb

Co-authored-by: megan07 <[email protected]>

* update PR to address comments

* update test

* fix sharedflow attribute update issue, add sharedflow deployment

* Update apigee_shared_flow_deployment.html.markdown

minor doc fix

* Update mmv1/third_party/terraform/utils/provider.go.erb

Co-authored-by: megan07 <[email protected]>

* Update mmv1/third_party/terraform/website/docs/r/apigee_shared_flow.html.markdown

Co-authored-by: megan07 <[email protected]>

* Update resource_apigee_shared_flow_test.go

remove commented out (unused) line of code

* Delete api.yaml

remove api.yaml as it is not needed for handwritten resource. Current repo structure no longer uses aggregated api.yaml

* Rename apigee_shared_flow.html.markdown to apigee_sharedflow.html.markdown

* Rename apigee_shared_flow_deployment.html.markdown to apigee_sharedflow_deployment.html.markdown

* Rename resource_apigee_shared_flow.go to resource_apigee_sharedflow.go

* Rename resource_apigee_shared_flow_deployment_test.go to resource_apigee_sharedflow_deployment_test.go

* Rename resource_apigee_shared_flow_test.go to resource_apigee_sharedflow_test.go

* Rename resource_apigee_shared_flow_sweeper_test.go to resource_apigee_sharedflow_sweeper_test.go

* Update resource_apigee_sharedflow.go

remove comment lines

* Update mmv1/third_party/terraform/resources/resource_apigee_sharedflow.go

Co-authored-by: megan07 <[email protected]>

* Update resource_apigee_sharedflow_deployment_test.go

remove unused debugging log

* update function name to capitalize

* Update resource_apigee_sharedflow_deployment_test.go

remove fmt.print

* add flowhook

* move flowhook doc

* Update mmv1/third_party/terraform/tests/resource_apigee_flowhook_test.go

Co-authored-by: megan07 <[email protected]>

* Update mmv1/third_party/terraform/website/docs/r/apigee_flowhook.html.markdown

Co-authored-by: megan07 <[email protected]>

* Update mmv1/third_party/terraform/website/docs/r/apigee_flowhook.html.markdown

Co-authored-by: megan07 <[email protected]>

* Update mmv1/third_party/terraform/tests/resource_apigee_flowhook_test.go

Co-authored-by: megan07 <[email protected]>

* Update resource_apigee_flowhook_test.go

remove fmt.print

* update some resource attribute properties

* Update apigee_flowhook.html.markdown

updated `description` and `continue_on_error` as optional

* update referenced function naming case

---------

Co-authored-by: Ray Xu <[email protected]>
Co-authored-by: megan07 <[email protected]>
add tests (GoogleCloudPlatform#7357)

Co-authored-by: Edward Sun <[email protected]>
run gofmt + fix capitalization errors from previous PRs (GoogleCloudPlatform#7420)

switch ci secrets to use secret manager (GoogleCloudPlatform#7408)

Update field descriptions (GoogleCloudPlatform#7402)

HA VPN over Cloud Interconnect has launched in GA
We should still probably add a sample or two showing how to use it.
Document PRODUCT_BASE_PATH override for DCL resource IAM (GoogleCloudPlatform#7418)

yaqs/1723187608399380480
Add support for accelerators to google_datafusion_instance (GoogleCloudPlatform#6851)

Added support to the field for Data Fusion, updated corresponding generated and handwritten tests, and added a custom diffsuppressfunc.

Currently suggests a manual fix to a potential diff issue, will be able to update down line based on feedback from API team.
Fix alloydb_cluster permadiff (GoogleCloudPlatform#7421)

Fix dns_managed_zone tests using unverified domain (GoogleCloudPlatform#7422)

Add SKIP_PROJECT_SWEEPER env variable for skipping the project sweeper (GoogleCloudPlatform#7432)

Remove percent sign when parsing metadata name (GoogleCloudPlatform#7417)

* Remove percent sign when parsing metadata name

* Remove percent sign when parsing metadata name
Merge branch 'GoogleCloudPlatform:main' into feature/iam_policy

Add ignore read on reserved ip ranges (GoogleCloudPlatform#7429)

Fix serviceusage links, clean up service resource stub (GoogleCloudPlatform#7435)

Remove use of `make generate` from TPG/TPGB-specific step in CI (GoogleCloudPlatform#7379)

Updating default accelerator.state field behavior in Data Fusion Instance (GoogleCloudPlatform#7434)

Making suggested default behavior functionality updates in previous accelerators PR.
Modify resource attribute 'input' to 'immutable' (GoogleCloudPlatform#7431)

* Modify resource attribute 'input' to 'immutable'
Remove noisy logging of project sweeper being skipped (GoogleCloudPlatform#7439)

Fix PubSub-to-BigQuery push configuration example (GoogleCloudPlatform#7410)

Rework + reflow provider reference, touch up index (GoogleCloudPlatform#7437)

Fix eventarc tests to use dedicated bootstrapped keys (GoogleCloudPlatform#7440)

Update docs for changing attribute `input` to `immutable` (GoogleCloudPlatform#7442)

Feat name constraints (GoogleCloudPlatform#7400)

Remove game_service_cluster_basic test (GoogleCloudPlatform#7443)

send friendly_name (GoogleCloudPlatform#7433)

* send friendly_name

* fixed validator failures

* update

* fix validator errors

---------

Co-authored-by: Edward Sun <[email protected]>
Fix headline of google_iam_workload_identity_pool (GoogleCloudPlatform#7449)

Move IAM bootstrap to its own file, improve error messages (GoogleCloudPlatform#7424)

* Move IAM bootstrap to its own file, improve errors

Also bootstrap roles/cloudbuild.builds.builder for cloudbuild service
agent.

* Re-add BootstrapConfig (accidentally deleted)

* Fix wrong variable name

* Bootstrap the role previously hardcoded for pubsub

* Move error message back into bootstrap function

This will dedup the code that calls this function. It now returns a
boolean and sends the more useful error through t.Error.

* Bootstrap the permissions for pubsub service agent

* Bootstrap the role in the correct test

* Fix formatting
Changing name field on google_compute_disk for TestAccComputeInstanceTemplate_sourceSnapshotEncryptionKey to include randomly generated string (GoogleCloudPlatform#7392)

* Changed name within google_compute_disk  on line 3162 to inclue and randomly generated string

* Cleaned up arguments being passed in to be more inline with other tests

* Updated based on addtional PR comments

* Fixed swapping of values within new context
If not specified, a default Workstations service agent SA is returned (GoogleCloudPlatform#7428)

* If not specified, a default service agent SA is returned

* Revert "Skip TestAccWorkstationsWorkstationConfig_workstationConfigEncryptionKeyExample"

This reverts commit 097bdbe.

* remove `rotation_period`
update BigqueryDatapolicy to ga (GoogleCloudPlatform#6797)

fix managed.dnsAuthorizations: (GoogleCloudPlatform#7445)

Co-authored-by: Edward Sun <[email protected]>
google_compute_security_policy: force send enforce_on_key so it can be unset on (GoogleCloudPlatform#7454)

Make `hostname` and `matcher` fields immutable (GoogleCloudPlatform#7448)

Add note on requiredness (GoogleCloudPlatform#7453)

Update BootstrapProject to support use across multiple projects/environments (GoogleCloudPlatform#7369)

* Update BootstrapProject to support use across multiple projects/environments

* Fix typo

* Fix arguments for getTestProjectFromEnv

* Fix project naming strategy for BootstrapProject to produce valid names in all envs

* Fix typo

* Fix function name after rebase
Update resource names in acceptance test to use `tf-test-` prefix (GoogleCloudPlatform#7450)

fix datastream_stream dataset id import (GoogleCloudPlatform#7451)

Merge branch 'GoogleCloudPlatform:main' into feature/iam_policy

Removing api.yaml and replacing with per product configs

Merge branch 'feature/iam_policy' of https://github.com/dbjnbnrj/magic-modules into feature/iam_policy
ericayyliu pushed a commit to ericayyliu/magic-modules that referenced this pull request Jul 26, 2023
…duling. (GoogleCloudPlatform#7365)

* Move testing to handwritten

* Move testing to handwritten

* Fix lint

* Fix typo

* Add remove step and update documentation

* Add remove step and update documentation

* Fix comparing string to nil

* Add field to handwritten defs

* rebase on main & resolve conflicts

* Fix doc formatting

* Add maintenance interval to Instance resource

* Add maintenance interval to Instance resource

* Add maintenance interval to Instance resource

* Change function names to valid

* Fix type mismatch

* Fix type mismatch

* Fix function name
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants