-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Fix diff suppress function ignoring additive_vpc_scope_dns_domain in Autopilot clusters #12567
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
Fix diff suppress function ignoring additive_vpc_scope_dns_domain in Autopilot clusters #12567
Conversation
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are.
|
Tests analyticsTotal tests: 221 Click here to see the affected service packages
Action takenFound 17 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
|
🟢 Tests passed during RECORDING mode: 🟢 No issues found for passed tests after REPLAYING rerun. 🔴 Tests failed during RECORDING mode: 🔴 Errors occurred during RECORDING mode. Please fix them to complete your PR. |
df4d706
to
5fc163d
Compare
Tests analyticsTotal tests: 222 Click here to see the affected service packages
Action takenFound 3 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
|
🟢 Tests passed during RECORDING mode: 🟢 No issues found for passed tests after REPLAYING rerun. 🔴 Tests failed during RECORDING mode: 🔴 Errors occurred during RECORDING mode. Please fix them to complete your PR. |
5fc163d
to
d10998a
Compare
Tests analyticsTotal tests: 222 Click here to see the affected service packages
Action takenFound 1 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
|
🔴 Tests failed during RECORDING mode: 🔴 Errors occurred during RECORDING mode. Please fix them to complete your PR. |
d10998a
to
e881351
Compare
Is @modular-magician on vacation? 😅 |
Hello! I am a robot. Tests will require approval from a repository maintainer to run. @ScottSuarez, 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. |
Huh, something is off here. There isn't a build I can approve to trigger generation. Are you up to date with the main branch of this repo? |
…Autopilot clusters
e881351
to
f3d19cf
Compare
Seems like recreating the commit helped? Even though the parent commit hasn't changed |
1 similar comment
Tests analyticsTotal tests: 222 Click here to see the affected service packages
Action takenFound 1 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
|
1 similar comment
Tests analyticsTotal tests: 222 Click here to see the affected service packages
Action takenFound 1 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
|
@slevenick I think it's looking better now, PTAL. I know I put an action item to look for more test gaps in the PR description, but I'll file a followup bug internally for this. I think the dns config mutation test I added should be useful enough for now. |
@slevenick This PR has been waiting for review for 3 weekdays. Please take a look! Use the label |
@GoogleCloudPlatform/terraform-team @slevenick This PR has been waiting for review for 1 week. Please take a look! Use the label |
Rerolling reviewer as I'm out sick |
Thanks, hope you get well soon! |
self-appointed successor to #11744
GKE Autopilot allows setting additive VPC scope domain on creation only. However, current implementation of the diff suppress function for Autopilot clusters removes any values in dns_config of Autopilot clusters, all the time.
This change permits setting additive VPC scope domain.
Remaining action items:
dns_config.#
doesn't cause issues (it seems necessary fordns_config.0.additive_vpc_scope_dns_domain
to existadditive_vpc_scope_dns_domain
#11744 to ForceNew when domain changes is exactly the behavior we want and copy it overRelease Note Template for Downstream PRs (will be copied)
See Write release notes for guidance.