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

Allow a node pool to be created before it is destroyed #256

Merged

Conversation

asproul
Copy link
Contributor

@asproul asproul commented Sep 4, 2019

This change is inspired by hashicorp/terraform-provider-google#1054 (comment) .

Currently, if a node pool has to be recreated for any number of reasons, the node pool is deleted then, created. This can be a problem if it is the only node pool in the gke cluster and the new node pool cannot be provisioned. In this scenario, pods could not be scheduled.

This change allows a node pool to be created before it is deleted so that any issues with node pool creation and/or provisioning are discovered before the node pool is removed. This feature is controlled by the variable node_pools_create_before_destroy. In order to avoid node pool name collisions, a 4 character alphanumeric is added as a suffix to the name.

Copy link
Contributor

@morgante morgante left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! This is definitely an interesting approach - my main question/concern is in how would we track which fields force a recreation (and hence must be in the keepers block)?

autogen/cluster.tf Outdated Show resolved Hide resolved
autogen/cluster.tf Outdated Show resolved Hide resolved
@aaron-lane aaron-lane added enhancement New feature or request bug Something isn't working and removed enhancement New feature or request labels Sep 6, 2019
@asproul
Copy link
Contributor Author

asproul commented Sep 6, 2019

PTAL @morgante

autogen/cluster.tf Outdated Show resolved Hide resolved
autogen/cluster.tf Outdated Show resolved Hide resolved
autogen/cluster.tf Outdated Show resolved Hide resolved
Copy link
Contributor

@morgante morgante left a comment

Choose a reason for hiding this comment

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

Looking good, do you think you'd be able to add a test fixture which exercises this module/variant?

autogen/cluster.tf Show resolved Hide resolved
@asproul
Copy link
Contributor Author

asproul commented Sep 11, 2019

PTAL @morgante

@asproul
Copy link
Contributor Author

asproul commented Sep 13, 2019

Looking good, do you think you'd be able to add a test fixture which exercises this module/variant?

Added two examples and one test fixture.

@asproul
Copy link
Contributor Author

asproul commented Sep 17, 2019

PTAL @morgante

@Jberlinsky Jberlinsky self-assigned this Oct 7, 2019
@morgante morgante assigned morgante and unassigned Jberlinsky Oct 11, 2019
@morgante morgante merged commit 62d79fa into terraform-google-modules:master Oct 11, 2019
@morgante
Copy link
Contributor

Thanks for the contribution!

@asproul asproul deleted the create-before-destroy-nodepools branch October 11, 2019 19:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants