PDB per Node type (Master, Data, Ingest)

[akrzos-bw]

PodDisruptionBudget per node type (master, data, injest, ...)

If we are performing maintenance on a kubernetes cluster (cordon, drain, updates/upgrades, then reboot) an ECK cluster can end up blocking the upgrade for a lengthy period of time while the ECK cluster goes into yellow status while a data node is removed from service but replaced on a separate node. This disruption often causes re-balancing of shards and now the Elastic PDB prevents other ECK cluster pods from being evicted even though they may not have an impact on data loss such as a master pod. If we had a PDB per node type we could mitigate some of the nodes that would be blocked while waiting for the ECK cluster to return to healthy green state.

It also appears that regardless of the underlying data redundancy the cluster is limited to a single disruption at this point.

[anyasabo]

Just to address your last point, the maxUnavailable setting can be tuned to modify that behavior: https://www.elastic.co/guide/en/cloud-on-k8s/current/k8s-update-strategy.html

[sebgl]

updateStrategy.maxUnavailable only impacts rolling upgrades of the cluster, not disruptions related to the PodDisruptionBudget.

Note that you can also set your own PodDisruptionBudget, or override the default one. For your use case I think you may want something similar to the following:

apiVersion: elasticsearch.k8s.elastic.co/v1
kind: Elasticsearch
metadata:
  name: quickstart
spec:
  version: 7.6.2
  nodeSets:
  - name: default
    count: 3
  podDisruptionBudget: {} # disable the default PDB
---
apiVersion: policy/v1beta1
kind: PodDisruptionBudget
metadata:
  name: data-nodes-pdb
spec:
  minAvailable: 2 # should probably be (number of data nodes - 1), I'm considering you have 3 data nodes here
  selector:
    matchLabels:
      elasticsearch.k8s.elastic.co/cluster-name: your-es-cluster
      elasticsearch.k8s.elastic.co/node-data: true
      elasticsearch.k8s.elastic.co/node-master: false
---
apiVersion: policy/v1beta1
kind: PodDisruptionBudget
metadata:
  name: master-nodes-pdb
spec:
  minAvailable: 2 # should probably be (number of master nodes - 1), I'm considering you have 3 master nodes here
  selector:
    matchLabels:
      elasticsearch.k8s.elastic.co/cluster-name: your-es-cluster
      elasticsearch.k8s.elastic.co/node-data: false
      elasticsearch.k8s.elastic.co/node-master: true

This example (to adapt to your use case) would:

• allow one data node to go down
• also allow one master node to go down
• allow any number of other node roles (eg. ingest) to go down, since no PDB is defined for them

[sebgl]

It's hard for ECK to come up with the best default depending on use cases here. In which case the example above demonstrates more flexibility to achieve one's needs.
Still keeping the issue open though, we may want to consider improving the existing defaults to allow disrupting more than one node under certain conditions.

[sanderma]

I also have some issues with the default pdb.
When people deploy an elastic to test with (single node) we are unable to reboot the Kubernetes nodes (see below text for the reason). This is because the operator by default sets a pdb of 1.

I have changed the operator to set the pdb to 0 when nodecount is 1.
IMHO when someone sets a nodecount of 1, he or she is not interested in being highly available.

If you set maxUnavailable to 0% or 0, or you set minAvailable to 100% or the number of replicas, you are requiring zero voluntary evictions. When you set zero voluntary evictions for a workload object such as ReplicaSet, then you cannot successfully drain a Node running one of those Pods. If you try to drain a Node where an unevictable Pod is running, the drain never completes. This is permitted as per the semantics of PodDisruptionBudget.
source: https://kubernetes.io/docs/tasks/run-application/configure-pdb/#think-about-how-your-application-reacts-to-disruptions

// buildPDBSpec returns a PDBSpec computed from the current StatefulSets,
// considering the cluster health and topology.
func buildPDBSpec(es esv1beta1.Elasticsearch, statefulSets sset.StatefulSetList) v1beta1.PodDisruptionBudgetSpec {
	// compute MinAvailable based on the maximum number of Pods we're supposed to have.
	// Do not set MinAvailable when nodecount is 1. When the PDB is 1, and the amount of pods is 1, Kubernetes will not be able to reboot the node.
	nodeCount := statefulSets.ExpectedNodeCount()
	var minAvailable int32
	if nodeCount == 1 {
		// when only single node, allow the pod to be rebooted, to ensure Kubernetes nodes do not get hung up by the pdb.
		minAvailable = 0
	} else {
		// maybe allow some Pods to be disrupted
		minAvailable = nodeCount - allowedDisruptions(es, statefulSets)
	}

	minAvailableIntStr := intstr.IntOrString{Type: intstr.Int, IntVal: minAvailable}

	return v1beta1.PodDisruptionBudgetSpec{
		// match all pods for this cluster
		Selector: &metav1.LabelSelector{
			MatchLabels: map[string]string{
				label.ClusterNameLabelName: es.Name,
			},
		},
		MinAvailable: &minAvailableIntStr,
		// MaxUnavailable can only be used if the selector matches a builtin controller selector
		// (eg. Deployments, StatefulSets, etc.). We cannot use it with our own cluster-name selector.
		MaxUnavailable: nil,
	}
}

[sanderma]

By setting this default this way, when one does not give a manual pdb, the operator will set the pdb to 0 when there is a single node cluster:

NAME                    MIN AVAILABLE   MAX UNAVAILABLE   ALLOWED DISRUPTIONS   AGE
quickstart-es-default   0               N/A               1                     8s

When you scale the cluster up, the operator will set the pdb as intended, and treat the cluster as highly available:

NAME                    MIN AVAILABLE   MAX UNAVAILABLE   ALLOWED DISRUPTIONS   AGE
quickstart-es-default   1               N/A               1                     38s

[sebgl]

@sanderma this makes sense to me. Thanks for mentioning the idea in the issue and then opening a PR! Will review it shortly.

[erdody]

@sebgl if you configure your own PDBs (and disable the default), you disable the ability of the operator to adjust it dynamically based on the cluster status ("It allows one Elasticsearch Pod to be taken down, as long as the cluster has a green health"). I would like to keep that behavior but restricted to data nodes only.

[sebgl]

@erdody indeed. Can you give more details about what you're trying to achieve? Allowing all master nodes to be taken down would be rather dangerous?

I'm thinking about relaxing the way the default PDB can be configured to allow users to override the Pod selector only. So that you can keep that default logic for data nodes only and come up with your own additional PDBs for other type of nodes.

[erdody]

@sebgl that would work for us. I was in fact thinking of setting the non-data nodes PDBs separately. Besides data nodes, the other PDBs don't need any dynamic behavior anyways.