Add readiness_checks to google_workstations_workstation_config

[bschaatsbergen]

Adds the readiness_checks argument block to the google_workstations_workstation_config resource.

Release Note Template for Downstream PRs (will be copied)

workstations: added `readiness_checks` argument block to `google_workstations_workstation_config` resource (beta)

Relates hashicorp/terraform-provider-google#16997

[modular-magician]

Hello! I am a robot. It looks like you are a: Community Contributor Googler Core Contributor. Tests will require approval to run.

@shuyama1, a repository maintainer, has been assigned to review your changes. If you have not received review feedback within 2 business days, please leave a comment on this PR asking them to take a look.

You can help make sure that review is quick by doing a self-review and by running impacted tests locally.

[modular-magician]

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, 15 insertions(+))
Terraform Beta: Diff ( 3 files changed, 193 insertions(+))
TF Conversion: Diff ( 1 file changed, 43 insertions(+))

Missing test report

Your PR includes resource fields which are not covered by any test.

Resource: google_workstations_workstation_config (37 total tests)
Please add an acceptance test which includes these fields. The test should include the following:

resource "google_workstations_workstation_config" "primary" {
  readiness_checks {
    path = # value needed
  }
}

[modular-magician]

Tests analytics

Total tests: 30
Passed tests 29
Skipped tests: 0
Affected tests: 1

Click here to see the affected service packages

• workstations

Action taken

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

TestAccWorkstationsWorkstationConfig_readinessChecks

Get to know how VCR tests work

[modular-magician]

$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$
TestAccWorkstationsWorkstationConfig_readinessChecks[Error message] [Debug log]

$\textcolor{red}{\textsf{Please fix these to complete your PR.}}$
View the build log or the debug log for each test

[shuyama1]

Test failed with error:

Error: Error creating WorkstationConfig: googleapi: Error 400: readiness_check.path cannot be empty

[bschaatsbergen]

Test failed with error:

Error: Error creating WorkstationConfig: googleapi: Error 400: readiness_check.path cannot be empty

Thanks for the quick feedback, I'll try '/' and port 80 ad that's apparently the default path and port.

[bschaatsbergen]

@shuyama1 addressed in 423987b

[modular-magician]

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, 15 insertions(+))
Terraform Beta: Diff ( 3 files changed, 194 insertions(+))
TF Conversion: Diff ( 1 file changed, 43 insertions(+))

[modular-magician]

Tests analytics

Total tests: 30
Passed tests 29
Skipped tests: 0
Affected tests: 1

Click here to see the affected service packages

• workstations

Action taken

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

TestAccWorkstationsWorkstationConfig_readinessChecks

Get to know how VCR tests work

[modular-magician]

$\textcolor{red}{\textsf{The provider crashed while running the VCR tests in RECORDING mode}}$
$\textcolor{red}{\textsf{Please fix it to complete your PR}}$
View the build log

[modular-magician]

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, 15 insertions(+))
Terraform Beta: Diff ( 3 files changed, 207 insertions(+))
TF Conversion: Diff ( 1 file changed, 43 insertions(+))

[modular-magician]

Tests analytics

Total tests: 30
Passed tests 29
Skipped tests: 0
Affected tests: 1

Click here to see the affected service packages

• workstations

Action taken

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

TestAccWorkstationsWorkstationConfig_readinessChecks

Get to know how VCR tests work

[modular-magician]

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$
TestAccWorkstationsWorkstationConfig_readinessChecks[Debug log]

Rerun these tests in REPLAYING mode to catch issues

$\textcolor{green}{\textsf{No issues found for passed tests after REPLAYING rerun.}}$

---

$\textcolor{green}{\textsf{All tests passed!}}$
View the build log or the debug log for each test

[bschaatsbergen]

Tests passed during RECORDING mode: TestAccWorkstationsWorkstationConfig_readinessChecks[Debug log]

Rerun these tests in REPLAYING mode to catch issues

No issues found for passed tests after REPLAYING rerun.

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

@shuyama1 🚀 !

[shuyama1]

Mostly good. Some small comments

[shuyama1]

I wonder if we want to mark these two fields required based on the previous error we ran into, even if they seem to be optional based on the API doc. Changing a field from required to optional is non-changing while the other way around is breaking.

Plus, are these updatable? If so, can you add an update test following https://googlecloudplatform.github.io/magic-modules/develop/test/#add-an-update-test. Otherwise, we need to mark them immutable. Also, if you think the fields can be tested in any of the existing tests, feel free to add them to an existing one instead of creating a new test for this feature. Thanks

[bschaatsbergen]

Great suggestion Shuya! I'll make them required, based on the previous error we faced. I would assume they are updatable, I'll introduce a test.

[bschaatsbergen]

Added readiness_checks to the basic example through e8d3ec3, do you think this would be enough? The basic example is part of the existing update test. And, I've set the readiness_checks arguments to be required via 5fba9ba.

[shuyama1]

Would it be possible if we choose another update test? We try to keep only minimum required fields in the basic example config. Sorry about the extra work

[bschaatsbergen]

I don't have an easy way to expose a different port, it's at least not possible via the existing arguments (also looking at https://workstations.googleapis.com/$discovery/rest?version=v1beta). The default exposed port is port 80. Could we perhaps omit the update test? I have a feeling that updating port 80 to any other arbitrary port is going to result into a timeout error.

[bschaatsbergen]

@shuyama1 any chance we can ship this :)?

[modular-magician]

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, 15 insertions(+))
Terraform Beta: Diff ( 3 files changed, 207 insertions(+))
TF Conversion: Diff ( 1 file changed, 43 insertions(+))

[modular-magician]

Tests analytics

Total tests: 30
Passed tests 30
Skipped tests: 0
Affected tests: 0

Click here to see the affected service packages

• workstations

$\textcolor{green}{\textsf{All tests passed in REPLAYING mode.}}$
View the build log

[shuyama1]

Thank you!