-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
container: fix node_config.kubelet_config
updates in google_container_cluster
#11697
container: fix node_config.kubelet_config
updates in google_container_cluster
#11697
Conversation
Hello! I am a robot. Tests will require approval from a repository maintainer to run. @trodge, 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. |
mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb
Show resolved
Hide resolved
d5b0198
to
20f7ce8
Compare
mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb
Show resolved
Hide resolved
20f7ce8
to
cf0a010
Compare
node_config.kubelet_config
updatesnode_config.kubelet_config
updates in google_container_cluster
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: 208 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
|
|
@trodge This PR has been waiting for review for 3 weekdays. Please take a look! Use the label |
- Fix updates for fields within `node_config.kubelet_config` where updates didn't function properly (basically all except for `insecure_kubelet_readonly_port_enabled`. - Consolidate test cases for items under `node_config.kubelet_config` with the one for `node_config.kubelet_config.insecure_kubelet_readonly_port_enabled` Part of hashicorp/terraform-provider-google#19225
cf0a010
to
4a61921
Compare
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: 209 Click here to see the affected service packages
View the build log |
@GoogleCloudPlatform/terraform-team @trodge This PR has been waiting for review for 1 week. Please take a look! Use the label |
This is still shy of a fix for updates for everything within
node_config
, and doesn't consolidate where we're updating thegoogle_container_node_pool
resource vs. thedefault-pool
that can be created directly withingoogle_container_cluster
, but it does fix a subset of the fields that don't work, and also simplifies some of the code and consolidates some tests, so maybe still worth considering as another step in the right direction.node_config.kubelet_config
where updates didn't function properly (basically all except forinsecure_kubelet_readonly_port_enabled
.node_config.kubelet_config
with the one fornode_config.kubelet_config.insecure_kubelet_readonly_port_enabled
, including testing updates.Part of hashicorp/terraform-provider-google#19225
Release Note Template for Downstream PRs (will be copied)