This worked at some level because processes did, indeed, get replaced when their nodes got tainted. It did not work as hoped, though, because (as I understand it!) “replacement” and “exclusion” are two separate ideas, and we wound up replacing storage/log processes before they could replicate their data, leading to data and availability loss. My (new) understanding is that replacement will just clobber the process entirely regardless of whether its data has been fully replicated, but exclusion will wait until replication is complete.
Reading through the operator docs, it looks like we can manually exclude processes, and it looks like the operator will automatically manage exclusions if we try to shrink the cluster. I can’t find any documentation, though, of a way to exclude a process if its node is tainted. Is that the case, or have I missed something?
More broadly:
Is there a way to exclude processes based on node state or annotations or similar Kubernetes entities?
Is there an established set of best practices for working with EKS node group updates?
It did not work as hoped, though, because (as I understand it!) “replacement” and “exclusion” are two separate ideas, and we wound up replacing storage/log processes before they could replicate their data, leading to data and availability loss
This should not happen and that would be a bug in the operator (a surprising and critical one). Every (default) replacement will require that all processes on a pod are fully excluded before tearing down the pod (fdb-kubernetes-operator/controllers/remove_process_groups.go at main · FoundationDB/fdb-kubernetes-operator · GitHub). Besides that the operator has multiple safety checks enabled per default to only take down one zone at a time: fdb-kubernetes-operator/controllers/remove_process_groups.go at main · FoundationDB/fdb-kubernetes-operator · GitHub and before removing pods it does a fault tolerance check. Just to minimise the risk of dataloss. The only exception where an exclusion would be skipped is if you would use processGroupsToRemoveWithoutExclusion, but this feature shouldn’t be used on a production cluster. If anything about “replacements” and “exclusions” is not well documented in the operator docs (as it seems ), please feel free to open a PR with improvements to the documentation.
Is there a way to exclude processes based on node state or annotations or similar Kubernetes entities?
You already used the correct feature.
What I believe happened is that EKS drained the node in between the replacements when the exclusion was not done yet. The drain will remove the pods from the node after the grace period (30 seconds if not configured differently): Safely Drain a Node | Kubernetes and I assume your setup doesn’t use any Pod Disruption Budget, therefore none of the removals was blocked.
Is there an established set of best practices for working with EKS node group updates?
I guess you are not using EBS volumes? Otherwise the pods should be able to come up again? In theory the Pod Disruption Budget could be used to protect against such scenarios, the only problem is that Pod Disruption Budget is more focused on how many pod should stay up and not how many fault domains should stay up.
This should not happen and that would be a bug in the operator (a surprising and critical one).
First off, thanks for this—understanding what should have happened narrows our search space considerably, and we’ll continue to work to understand the precise failure mode.
I assume your setup doesn’t use any Pod Disruption Budget, therefore none of the removals was blocked.
We do have pod disruption budgets (and even contributed some docs in this space!), but my guess is that something in this chain of events happily replaced pods as quickly as new pods were technically “ready” as quickly as the PDB allowed.
What I believe happened is that EKS drained the node in between the replacements when the exclusion was not done yet.
I suspect the same, though I’m a little fuzzy on the mechanism, since that would be at odds with our prior testing. Perhaps we inadvertently changed the grace period, though. Will follow up on this.
please feel free to open a PR with improvements to the documentation.
I’d be happy to, but will do so after we’ve finished this investigation and I’m confident in my/our understanding.
My guess would be that your PDB sets a value that is pretty close to the currently running pods, e.g. lets assume you have 10 storage pods and you set .spec.minAvailable = 9, so 1 Pod can be unavailable at a time. If you want to upgrade a node now and the operator replaces e.g. 4 storage pods, you would end up with 14 storage pods but the .spec.minAvailable = 9 would allow to take down 5 pods, so the 4 pods that should be evicted will be drained directly. So the drain will succeed as soon as the new pods are coming up.
I think there are two reasonable ways to handle this:
Dynamically adjust the PDB in case of upgrades. This could be a bit complex depending on how many pods can run on distinct nodes + there is a limitation in the upgrades process that the drain must succeed in 15 minutes (if I scanned correctly over the EKS docs). Depending on your cluster and data size 15 minutes could be not enough to exclude the process(es).
Assuming that you are using EBS volumes (please correct me): Instead of replacing the pods it would be better to set the maintenance mode on the fault domain and delete the pod, the pod should be scheduled onto a new node (same AZ, because EBS is AZ scoped). This could be implemented with an additional controller that, e.g. uses this “interface” fdb-kubernetes-operator/docs/manual/operations.md at main · FoundationDB/fdb-kubernetes-operator · GitHub to interact with the operator to let the operator reset the maintenance mode once the processes are back. Another option would be to implement a new “nodeEvictor” which basically has the ability to replace or delete (configurable) pods on nodes with a certain set of taints (I believe the cordon/drain sets a specific set of taints). This way the operator could handle everything itself and could set the maintenance mode and reset it again.
That is, indeed, how we have our PDBs configured (and sorry to make you guess!): minAvailable = configuredProcessCount - 1. What you’re describing sounds like a plausible explanation. I need to clear out a couple other things first today, but verifying that this is what happened will be my full focus from tomorrow on out.
One confounding factor is that we have automationOptions.replacements.maxConcurrentReplacements set to 1. Even so, I guess that explanation still makes sense:
We have N configured storage pods
We have a PDB that has minAvailable = N - 1 for storage pods
The operator is configured to do one concurrent replacement at a time
We begin a rolling node group update and one node gets cordoned/drained (you are correct that cordon/drain has a specific set of taints, namely node.kubernetes.io/unschedulable)
The operator launches a new storage process as part of the replacement process
The PDB is satisfied and so the rolling update process clobbers the storage process on the tainted node before it’s had a chance to redistribute its data
The operator notices that we’re down a storage pod and spawns a new one, getting us back to a pod count where the PDB is satisfied but the new storage process doesn’t have the data it needs
GOTO 6
Eventually, cluster loses enough of its replicated data to enter an unrecoverable state
Dynamically adjust the PDB in case of upgrades. This could be a bit complex depending on how many pods can run on distinct nodes…
Interesting. I don’t think this path is viable for us (more on that in a minute!), but to make sure we’re on the same page, I think that would basically mean bumping the PDB up such that minAvailable = configuredProcesses + maxConcurrentReplacements while exclusion was in progress, then dropping it down to allow evictions, then bringing it back up once more exclusions were in progress. Is that what you had in mind?
…there is a limitation in the upgrades process that the drain must succeed in 15 minutes (if I scanned correctly over the EKS docs). Depending on your cluster and data size 15 minutes could be not enough to exclude the process(es).
Right—in hindsight, we should have been immediately suspicious that the rolling update was making any progress at all with the 15-minute deadline. Data redistribution/exclusion takes much longer than that for us, and that means out-of-the-box rolling updates with this specific AWS offering won’t work for us at all.
On a few occasions, we observed that the rolling update would time out and fail if the operator couldn’t start new processes. In one case, that was a configuration mistake on our part. In another, we deliberately halted the operator as a test to make sure that the rolling update would, in fact, time out if the operator wasn’t managing node replacement (which was true—but as discussed above, not the whole picture). In the most recent case, the process timed out because the cluster became unavailable and the operator wouldn’t launch new processes.
In case it’s helpful to anybody else, we did ask AWS if we could extend that deadline, but it’s a hard-coded limit for managed node groups and they can’t change it no matter how nicely somebody asks.
Assuming that you are using EBS volumes…
Alas, we’re not. We’re using instances with attached NVMe drives and making them available as persistent volumes with a local static provisioner, so replacing a pod means starting with a blank disk (though perhaps we should revisit that in light of this whole episode).
As a tangent, we’ve had to jump through some hoops to make local SSDs work with persistent volumes and persistent volume claims, particularly around auto-scaling node groups. As a low-priority and not-yet-fully-considered feature request, I think we might have had an easier time if we could use extended resources (e.g. com.example/nvme-3750) in lieu of PVCs to match stateful pods to nodes with disks available. I don’t think the operator would need to worry about the extended resources at all, but having a “don’t do persistent volume claims” option would do the trick for us! I’d be happy to elaborate on this in a separate thread or as an actual feature request ticket, if you’d like. I’m also happy to take “no” for an answer on this one.
Returning to the main point, though, I think the explanation you’ve offered is exactly what happened in our case, and I’ll work to verify that and revise our procedures over the next couple days. I’ll follow up with what we’ve learned.
Once again, thank you! This is all extremely helpful!
), please feel free to open a PR with improvements to the documentation.