CLIP-1701: Wait until pods in Helm release are terminated before destroying nfs module #282
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bianchi2
requested review from
nghazali,
yzha645,
jjeongatl,
nanux,
errcode1202,
tarka and
bconroyA
as code owners
3 tasks
Out of curiosity, where can I find part that waits extra 5 seconds |
|
@jjeongatl oops, my bad. That was the original plan, but just termination_grace_period is enough since it takes some time to delete helm release itself and the pod starts terminating already. |
nghazali
reviewed
Nov 15, 2022
modules/products/bamboo/helm.tf
Outdated
| depends_on = [kubernetes_job.import_dataset] | ||
| depends_on = [ | ||
| kubernetes_job.import_dataset, | ||
| module.nfs, |
There was a problem hiding this comment.
Do we need to have module.nfs here? time_sleep is already depends on nfs, so here we can remove it as is embedded.
There was a problem hiding this comment.
Nice catch. Yes, depending on nfs module is implied here. Leftovers from trying just to get away with dependency on nfs (without sleep). Removed this redundant dependency
nghazali
approved these changes
Nov 15, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes an issue with pods stuck in a Terminating state when destroying infrastructure. The problem was with the deletion of a shared home pvc which is stuck in pending as long as its pod is stuck in Terminating.
Helm release server pods are often stuck in terminating (unless termination grace period is set to 0) because of the following reason:
Destruction of nfs module (that includes nfs helm release and EBS volume with PV and PVCs) and product helm releases happens almost simultaneously, as a result, when, say, confluence pod is in Terminating state (preStop hook can take some time), EBS volume that backs the underlying PV and PVCs gets destroyed too. As a result, confluence container enters a weird state, and kubelet cannot kill the pod because of the following error:
When trying to delete a docker container directly from the node, it turns out that the container is indeed unresponsive:
As a result, confluence pod is stuck in Terminating, and shared home PVC is stuck too and will be in this state until confluence pod exists. As a result, Terraform gives up waiting for the PVC deletion.
Having investigating the issue, and being not able to reproduce manually with
helm deleteit became obvious that helm_release destruction needs to wait for all pods to be wiped out or else there are chances that critical pieces of infra are destroyed when the pod is being terminated.Unfortunately, helm provider cannot wait for pods to be deleted, it just waits for the release to be deleted. See: hashicorp/terraform-provider-helm#593 and helm/helm#2378
The workaround is:
This way the following deletion order is achieved:
We're giving helm release pods time to be terminated and only then EBS, PV and PVCs get deleted.
Checklist