Skip to content
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: refactor node_config node pool update #11826

Conversation

wyardley
Copy link
Contributor

This sets the stage for consolidating the logic for updating nodepools between the container_cluster and container_node_pool resources, by creating a new nodePoolNodeConfigUpdate() function.

Part of hashicorp/terraform-provider-google#19225

Release Note Template for Downstream PRs (will be copied)


@modular-magician modular-magician added the awaiting-approval Pull requests that need reviewer's approval to run presubmit tests label Sep 26, 2024
@wyardley wyardley force-pushed the wyardley/node_config_update/19225_partial branch from 5486b3d to 8e3c116 Compare September 26, 2024 20:37
@wyardley wyardley marked this pull request as ready for review September 26, 2024 20:38
@github-actions github-actions bot requested a review from NickElliot September 26, 2024 20:39
Copy link

Hello! I am a robot. Tests will require approval from a repository maintainer to run.

@NickElliot, 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.

nodePoolInfo.project,
nodePoolInfo.location,
"updating GKE node pool disk_size_gb/disk_type/machine_type/storage_pools", userAgent,
timeout)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A lot of opportunity to DRY up these parts with a function at some point too, but trying to keep it pretty self-contained here for now.

@wyardley wyardley force-pushed the wyardley/node_config_update/19225_partial branch from 8e3c116 to d467fd7 Compare September 26, 2024 22:00
@wyardley wyardley changed the title refactor: move node_config node pool update container: refactor node_config node pool update Sep 27, 2024
@modular-magician modular-magician added service/container and removed awaiting-approval Pull requests that need reviewer's approval to run presubmit tests labels Sep 30, 2024
@modular-magician
Copy link
Collaborator

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 ( 2 files changed, 532 insertions(+), 509 deletions(-))
google-beta provider: Diff ( 2 files changed, 531 insertions(+), 509 deletions(-))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 212
Passed tests: 199
Skipped tests: 13
Affected tests: 0

Click here to see the affected service packages
  • container

$\textcolor{green}{\textsf{All tests passed!}}$

View the build log

Copy link

github-actions bot commented Oct 1, 2024

@NickElliot This PR has been waiting for review for 3 weekdays. Please take a look! Use the label disable-review-reminders to disable these notifications.

Copy link

github-actions bot commented Oct 3, 2024

@GoogleCloudPlatform/terraform-team @NickElliot This PR has been waiting for review for 1 week. Please take a look! Use the label disable-review-reminders to disable these notifications.

Copy link

@GoogleCloudPlatform/terraform-team @NickElliot This PR has been waiting for review for 2 weeks. Please take a look! Use the label disable-review-reminders to disable these notifications.

This sets the stage for consolidating the logic for updating nodepools
between the `container_cluster` and `container_node_pool` resources, by
creating a new `nodePoolNodeConfigUpdate()` function.

Part of hashicorp/terraform-provider-google#19225
@wyardley wyardley force-pushed the wyardley/node_config_update/19225_partial branch from d467fd7 to 2291e12 Compare October 15, 2024 18:14
@modular-magician modular-magician added the awaiting-approval Pull requests that need reviewer's approval to run presubmit tests label Oct 15, 2024
Copy link
Contributor

@NickElliot NickElliot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry for the delays! Didn't realize the scope of this specific change in the larger issue was deceptively small and kept pushing back the review to put more time into it.

LGTM! will wait on this last build

@modular-magician modular-magician removed the awaiting-approval Pull requests that need reviewer's approval to run presubmit tests label Oct 15, 2024
@wyardley
Copy link
Contributor Author

Hah, yes, the actual change is one line plus moving a big block of code. It should set the stage for sharing the code with the default node pool update. Can check out the version before the second commit if you want to see which you prefer.

@github-actions github-actions bot requested a review from NickElliot October 15, 2024 21:31
@modular-magician
Copy link
Collaborator

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 ( 2 files changed, 535 insertions(+), 512 deletions(-))
google-beta provider: Diff ( 2 files changed, 534 insertions(+), 512 deletions(-))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 212
Passed tests: 199
Skipped tests: 13
Affected tests: 0

Click here to see the affected service packages
  • container

🟢 All tests passed!

View the build log

@wyardley wyardley deleted the wyardley/node_config_update/19225_partial branch October 15, 2024 21:49
wyardley added a commit to wyardley/magic-modules that referenced this pull request Oct 16, 2024
This resolves an issue where many subfields of `node_config` in a
cluster (which affects the default node-pool "default-pool" when
`remove_default_node_pool` is not set to `false`) don't support updates
properly.

Fixes #19225
Follow up to GoogleCloudPlatform#11826
wyardley added a commit to wyardley/magic-modules that referenced this pull request Oct 16, 2024
This resolves an issue where many subfields of `node_config` in a
cluster (which affects the default node-pool "default-pool" when
`remove_default_node_pool` is not set to `false`) don't support updates
properly.

Fixes hashicorp/terraform-provider-google#19225

Followup to GoogleCloudPlatform#11826 where the code used by the regular node pool update
code was broken out.
gontech pushed a commit to gontech/magic-modules that referenced this pull request Oct 16, 2024
wyardley added a commit to wyardley/magic-modules that referenced this pull request Oct 19, 2024
This resolves an issue where many subfields of `node_config` in a
cluster (which affects the default node-pool "default-pool" when
`remove_default_node_pool` is not set to `false`) don't support updates
properly.

Fixes hashicorp/terraform-provider-google#19225

Followup to GoogleCloudPlatform#11826 where the code used by the regular node pool update
code was broken out.
varshatumburu pushed a commit to varshatumburu/magic-modules that referenced this pull request Oct 19, 2024
wyardley added a commit to wyardley/magic-modules that referenced this pull request Oct 21, 2024
This resolves an issue where many subfields of `node_config` in a
cluster (which affects the default node-pool "default-pool" when
`remove_default_node_pool` is not set to `false`) don't support updates
properly.

Fixes hashicorp/terraform-provider-google#19225

Followup to GoogleCloudPlatform#11826 where the code used by the regular node pool update
code was broken out.
BBBmau pushed a commit to BBBmau/magic-modules that referenced this pull request Oct 23, 2024
wyardley added a commit to wyardley/magic-modules that referenced this pull request Oct 23, 2024
This resolves an issue where many subfields of `node_config` in a
cluster (which affects the default node-pool "default-pool" when
`remove_default_node_pool` is not set to `false`) don't support updates
properly.

Fixes hashicorp/terraform-provider-google#19225

Followup to GoogleCloudPlatform#11826 where the code used by the regular node pool update
code was broken out.
wyardley added a commit to wyardley/magic-modules that referenced this pull request Oct 24, 2024
This resolves an issue where many subfields of `node_config` in a
cluster (which affects the default node-pool "default-pool" when
`remove_default_node_pool` is not set to `false`) don't support updates
properly.

Fixes hashicorp/terraform-provider-google#19225

Followup to GoogleCloudPlatform#11826 where the code used by the regular node pool update
code was broken out.
BBBmau pushed a commit to BBBmau/magic-modules that referenced this pull request Oct 24, 2024
wyardley added a commit to wyardley/magic-modules that referenced this pull request Oct 25, 2024
This resolves an issue where many subfields of `node_config` in a
cluster (which affects the default node-pool "default-pool" when
`remove_default_node_pool` is not set to `false`) don't support updates
properly.

Fixes hashicorp/terraform-provider-google#19225

Followup to GoogleCloudPlatform#11826 where the code used by the regular node pool update
code was broken out.
BBBmau pushed a commit to BBBmau/magic-modules that referenced this pull request Nov 5, 2024
akshat-jindal-nit pushed a commit to akshat-jindal-nit/magic-modules that referenced this pull request Nov 18, 2024
amanMahendroo pushed a commit to amanMahendroo/magic-modules that referenced this pull request Dec 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants