Add envvar GOOGLE_CHRONICLE_INSTANCE_ID for tests

[roaks3]

Release Note Template for Downstream PRs (will be copied)

See Write release notes for guidance.

[shuyama1]

I wonder if this is backward-compatible. It seems the current change requires all PRs to rebase to include the updates in the cloud build configuration .yml file. Otherwise, the build process might fail due to the missing environment variable GOOGLE_CHRONICLE_INSTANCE_ID.

[roaks3]

I think you could be right, but I don't know enough about how this is set up. Could you elaborate a bit? Is there a set of .ci functionality that runs off of main, and needs to be updated as a "step 1"? (also, is any of this documented for me to get clarification on my own?)

[shuyama1]

Among the files modified, .ci/gcb-pr-downstream-generation-and-test.yml (we usually call it the generate-diff job) is the only one that runs off the PR branch's head. The previously used .ci/gcb-generate-diffs-new.yml file has already been removed and replaced with this new configuration, we can safely remove it from this PR. This behavior occurs because the Cloud Build trigger is configured to respond to pull request events, invoking the configuration file from the PR branch.

In contrast, other YAML files are for triggers working directly with the main branch and can immediately adapt to code changes once merged.

For the generate-diff job, the Magician code should sync at the beginning of execution. Therefore, any updates to the Magician source code take immediate effect for all PRs once the change is merged.

To ensure backward compatibility when introducing a new environment variable, we generally have two options:

1. Split the change in two PRs.

   • The first PR updates the configuration yml file
   • The second PR updates the Magician source code.
   • Allow a soak period after merging the first PR to ensure most PRs already have the updated configuration checked in before merging the second PR.

2. Make the source code backward compatible

   • Currently, the Magician checks for existence of all env variables, as seen in this code: 


   env := make(map[string]string, len(ttvEnvironmentVariables))
   for _, ev := range ttvEnvironmentVariables {
   	val, ok := os.LookupEnv(ev)
   	if !ok {
   		return fmt.Errorf("did not provide %s environment variable", ev)
   	}
   	env[ev] = val
   }
   

   (https://github.com/GoogleCloudPlatform/magic-modules/blob/main/.ci/magician/cmd/test_terraform_vcr.go#L108-L115)

   • To add a new env var without breaking existing functionality, we should make an exception for the new variable, ensuring it does not trigger an error if it is missing. This approach allows the change to take effect immediately for PRs rebased off main after the change is merged.

I hope this helps clarify things.

[roaks3]

Should be updated now with optional env vars, thanks for the help!

[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.

google provider: Diff ( 4 files changed, 32 insertions(+))
google-beta provider: Diff ( 1 file changed, 12 insertions(+))

[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.

google provider: Diff ( 4 files changed, 32 insertions(+))
google-beta provider: Diff ( 1 file changed, 12 insertions(+))

[shuyama1]

It seems the issue is due to permission issues with the secret value. I've granted access to the relevant service accounts and will retry now.

/gcbrun

[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.

google provider: Diff ( 4 files changed, 32 insertions(+))
google-beta provider: Diff ( 1 file changed, 12 insertions(+))

[modular-magician]

Tests analytics

Total tests: 4370
Passed tests: 3951
Skipped tests: 415
Affected tests: 4

Click here to see the affected service packages

All service packages are affected

Action taken

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

• TestAccDataSourceGoogleGkeHubFeature_basic
• TestAccDataSourceGoogleQuotaInfo_basic
• TestAccEphemeralServiceAccountKey_basic
• TestAccSecureSourceManagerInstance_secureSourceManagerInstancePrivatePscBackendExample

Get to know how VCR tests work

[modular-magician]

🟢 Tests passed during RECORDING mode:
TestAccDataSourceGoogleGkeHubFeature_basic [Debug log]

🔴 Tests failed when rerunning REPLAYING mode:
TestAccDataSourceGoogleGkeHubFeature_basic [Error message] [Debug log]

Tests failed due to non-determinism or randomness when the VCR replayed the response after the HTTP request was made.

Please fix these to complete your PR. If you believe these test failures to be incorrect or unrelated to your change, or if you have any questions, please raise the concern with your reviewer.

---

🔴 Tests failed during RECORDING mode:
TestAccDataSourceGoogleQuotaInfo_basic [Error message] [Debug log]
TestAccEphemeralServiceAccountKey_basic [Error message] [Debug log]
TestAccSecureSourceManagerInstance_secureSourceManagerInstancePrivatePscBackendExample [Error message] [Debug log]

🔴 Errors occurred during RECORDING mode. Please fix them to complete your PR.

View the build log or the debug log for each test

[roaks3]

Test failures appear unrelated (same showing in other PRs like #12580)