Refactor how the Elasticsearch controller retrieves resources (rely on ResourcesState?)

[sebgl]

There is a bit of confusion coming from the way we check Pods and StatefulSets in the Elasticsearch controller.

In some places we rely on a ResourcesState struct:

cloud-on-k8s/pkg/controller/elasticsearch/reconcile/resources_state.go

Lines 18 to 30 in 459d005

	type ResourcesState struct {
	// AllPods are all the Elasticsearch pods related to the Elasticsearch cluster, including ones with a
	// DeletionTimestamp tombstone set.
	AllPods []corev1.Pod
	// CurrentPods are all non-deleted Elasticsearch pods.
	CurrentPods []corev1.Pod
	// CurrentPodsByPhase are all non-deleted Elasticsearch indexed by their PodPhase
	CurrentPodsByPhase map[corev1.PodPhase][]corev1.Pod
	// DeletingPods are all deleted Elasticsearch pods.
	DeletingPods []corev1.Pod
	// ExternalService is the user-facing service related to the Elasticsearch cluster.
	ExternalService corev1.Service
	}

This struct contains the actual Pods (and StatefulSets), retrieved only once.

In other places we retrieve Pods and StatefulSets from the client in the middle of the reconciliation:

cloud-on-k8s/pkg/controller/elasticsearch/driver/nodes.go

Lines 37 to 49 in 459d005

	// check if actual StatefulSets and corresponding pods match our expectations before applying any change
	ok, err := d.expectationsSatisfied()
	if err != nil {
	return results.WithError(err)
	}
	if !ok {
	return results.WithResult(defaultRequeue)
	}
	actualStatefulSets, err := sset.RetrieveActualStatefulSets(d.Client, k8s.ExtractNamespacedName(&d.ES))
	if err != nil {
	return results.WithError(err)
	}

We should pick a single way of doing things. I tend to think working with the Pods and StatefulSets retrieved through ResourcesState, only once at the beginning of the reconciliation, is cleaner and produces more deterministic actions.

We should also make sure the entire thing plays well with the expectations mechanism.