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

Changing NodePool configuration should doing a rolling replacement #255

Closed
nesl247 opened this issue Nov 20, 2019 · 5 comments
Closed

Changing NodePool configuration should doing a rolling replacement #255

nesl247 opened this issue Nov 20, 2019 · 5 comments
Assignees
Labels
kind/enhancement Improvements or new features resolution/by-design This issue won't be fixed because the functionality is working as designed

Comments

@nesl247
Copy link

nesl247 commented Nov 20, 2019

Currently when changing a NodePool configuration that requires the NodePool to be recreated, such as changing from a n1-standard-1 to a n1-standard-4 for exmaple, the NodePool is recreated in a dangerous manner.

What currently happens is that Pulumi creates the new NodePool, and then tells Google to remove the old one. However, the way Google appears to handle this is to bypass PDBs. What happens is that all nodes in the pool are cordoned off, and then force drained (bypassing PDBs). Unfortunately this is extremely problematic because it removes the insurance that some pods stay running. For example, when we've made this change, our istio installation has gone down, which is a problem because it means we are entirely down until istio is rescheduled and started.

In our case, we also realized that we had our minNodeCount set to too small of an amount, so when the new NodePool was created, it was done so with nodes that could not handle the number of pods, and relied upon autoscaling for more nodes to be added. So our downtime was even longer due to this.

@pgavlin
Copy link
Member

pgavlin commented Nov 20, 2019

Interesting. Do you know if it is possible to safely recreate the node pool manually? What steps would you take to do so e.g. using the GCP console/CLI?

If this can be fixed, it's going to require changes in the upstream TF provider.

@mnlumi mnlumi added the kind/enhancement Improvements or new features label Jul 26, 2023
@mnlumi
Copy link

mnlumi commented Jul 26, 2023

@lblackstone Do you believe this behavior is still true or can this be closed?

@lblackstone
Copy link
Member

I don't know. Somebody should test to confirm before closing the issue.

@antdking
Copy link

antdking commented Sep 8, 2023

I can't confirm the diagnosis in the description is accurate; but yes.. replacing a node pool doesn't correctly wait for workloads to transfer, be it from Pulumi or via the Console.

@rshade rshade self-assigned this Dec 6, 2024
@rshade
Copy link
Contributor

rshade commented Dec 10, 2024

Thank you for raising this issue. After reviewing Google's official documentation and information provided by the upstream provider, it looks like this is the expected behavior.
When a GKE node pool is deleted, Google Cloud does drain the nodes before they are deleted. According to the official GKE documentation:
"When you delete a node pool, GKE drains all the nodes in the node pool, deleting and rescheduling all Pods."

This behavior ensures that workloads are gracefully rescheduled onto other available nodes before node deletion. This behavior is controlled by GKE and is applied regardless of whether you're managing the infrastructure via Google Cloud Console, gcloud CLI, or Pulumi.

Recommended Next Steps:

  1. Adjust the pod disruption budget and set the minAvailable to a lower number, then run pulumi up.
  2. Set the configuration for the new NodePool, then run pulumi up.
  3. Wait for all the pods to transfer
  4. Adjust the pod disruption budget back to the original settings, and then run pulumi up

We will now close this issue as it appears to be working as intended. If you believe you are experiencing behavior that deviates from Google's expected node draining process, we recommend reviewing PDBs, cluster capacity, and preStop hooks. Feel free to reopen this issue if there is anything more to address.

Thank you for your time and for being part of the Pulumi community!

Best regards,
The Pulumi Team

@rshade rshade closed this as completed Dec 10, 2024
@rshade rshade added the resolution/by-design This issue won't be fixed because the functionality is working as designed label Dec 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Improvements or new features resolution/by-design This issue won't be fixed because the functionality is working as designed
Projects
None yet
Development

No branches or pull requests

7 participants