Relax the validation of master ipv4 cidr for GKE with private endpoint subnetwork#8338
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
Hello! I am a robot who works on Magic Modules PRs. I've detected that you're a community contributor. @rileykarson, a repository maintainer, has been assigned to assist you and help review your changes. ❓ First time contributing? Click here for more detailsYour assigned reviewer will help review your code by:
You can help make sure that review is quick by running local tests and ensuring they're passing in between each push you make to your PR's branch. Also, try to leave a comment with each push you make, as pushes generally don't generate emails. If your reviewer doesn't get back to you within a week after your most recent change, please feel free to leave a comment on the issue asking them to take a look! In the absence of a dedicated review dashboard most maintainers manage their pending reviews through email, and those will sometimes get lost in their inbox. |
rileykarson
left a comment
There was a problem hiding this comment.
Running tests. I can't determine whether this is an exhaustive change or not as the private cluster page doesn't mention private-endpoint-subnetwork, and the pages that do are about public clusters. The change seems reasonable, though.
|
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. Terraform GA: Diff ( 2 files changed, 80 insertions(+)) |
Tests analyticsTotal tests: 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 testsTestAccContainerCluster_withPrivateClusterConfigPrivateEndpointSubnetwork|TestAccContainerAwsNodePool_BetaBasicHandWritten|TestAccComputeFirewallPolicyRule_multipleRules |
|
|
48003bd to
101fc42
Compare
|
Running generate + test |
|
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. Terraform GA: Diff ( 2 files changed, 74 insertions(+)) |
Tests analyticsTotal tests: 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 testsTestAccContainerAwsNodePool_BetaBasicHandWritten|TestAccContainerCluster_withPrivateClusterConfigPrivateEndpointSubnetwork|TestAccComputeFirewallPolicyRule_multipleRules |
|
|
|
I sent the http log of a successful gcloud request to your Google email account. Please let me know if I can provide additional information. For what it's worth, I was able to create a cluster using Terraform with this configuration by building the terraform provider locally. I have yet to identify what is different between this test and my local build. |
|
Thanks! I've received it. There may be something weird with our test environment- it's added to a bunch of feature flags and allowlists. I'll push on the product team to figure out what's up, given it's working for you. |
|
/gcbrun |
|
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. Terraform GA: Diff ( 2 files changed, 79 insertions(+)) |
Tests analyticsTotal tests: 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 testsTestAccContainerCluster_withPrivateClusterConfigPrivateEndpointSubnetwork |
|
|
|
@ncapps I made & applied a suggestion to save a round trip, since it was a trivial change. Just a note that if we need to iterate further, you'll need to pull the change in from the origin branch yourself! |
|
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. Terraform GA: Diff ( 2 files changed, 79 insertions(+)) |
Tests analyticsTotal tests: 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 testsTestAccContainerCluster_withPrivateClusterConfigPrivateEndpointSubnetwork |
|
|
|
/gcbrun |
|
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. Terraform GA: Diff ( 2 files changed, 79 insertions(+)) |
Tests analyticsTotal tests: Action takenFound 2 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected testsTestAccContainerCluster_withPrivateClusterConfigPrivateEndpointSubnetwork|TestAccBigtableAppProfile_bigtableAppProfileSingleclusterExample |
|
Rerun these tests in REPLAYING mode to catch issues$\textcolor{green}{\textsf{All tests passed!}} |
|
Hi @rileykarson, Looks like the latest test run passed. Thanks so much for following up. Is there anything else I should do to help get this change ready to merge? |
…t subnetwork (GoogleCloudPlatform#8338) Co-authored-by: Riley Karson <rileykarson@google.com>
Master ipv4 cidr is not required when a private endpoint subnetwork is provided. This change relaxes an existing validation.
fixes hashicorp/terraform-provider-google#15064
I was unable to run the relevant acceptance tests because I do not have the permissions to create a network or subnetworks in the project owned by my employer. I built the provider locally and verified manually that I could create a GKE cluster with the expected configuration.
If this PR is for Terraform, I acknowledge that I have:
make testandmake lintin the generated providers to ensure it passes unit and linter tests.Release Note Template for Downstream PRs (will be copied)