Small question.

[iwasaki-kenta]

podset-operator/pkg/controller/podset/podset_controller.go

Line 58 in b30288a

err = c.Watch(&source.Kind{Type: &corev1.Pod{}}, &handler.EnqueueRequestForOwner{

Just out of curiosity, wouldn't watching for all pod events (pending, running, terminating, etc.) cause a race condition in the reconciliation loop where pods might keep getting created (even when a sufficient amount of pods as requested in the spec were created)?

The way I am looking at it is if a pod gets spawned (which concurrently triggers the reconciliation loop again), then the number of running pods would not have updated in time and thus cause another pod to be erroneously spawned.

If not, what exactly is preventing such a race condition from happening?

[xcoulon]

hello @iwasaki-kenta and sorry for the late response. I don't think there's a risk of race condition here: the controller receives one event at a time, and processes it accordingly.

In the Reconcile loop and more specifically in

podset-operator/pkg/controller/podset/podset_controller.go

Lines 109 to 115 in b30288a

	existingPods := &corev1.PodList{}
	err = r.client.List(context.TODO(),
	&client.ListOptions{
	Namespace: request.Namespace,
	LabelSelector: labels.SelectorFromSet(lbls),
	},
	existingPods)

the controller lists the current pods with the expected label(s) and checks their state. If they are in a pending or running state, then they are counted, unless they already have a deletion timestamp set (in which case they will have move to terminating state which will trigger another call to the Reconcile method). (

podset-operator/pkg/controller/podset/podset_controller.go

Lines 122 to 129 in b30288a

	for _, pod := range existingPods.Items {
	if pod.GetObjectMeta().GetDeletionTimestamp() != nil {
	continue
	}
	if pod.Status.Phase == corev1.PodPending || pod.Status.Phase == corev1.PodRunning {
	existingPodNames = append(existingPodNames, pod.GetObjectMeta().GetName())
	}
	}

)

Finally, if the current number of pods does not match the expectation, the controller scales up or down a single pod. Since a pod is created or terminated, the Reconcile loop will be called again, until the desired state is reached.

I hope this clarifies.

[ian-howell]

I know this is a relatively old project at this point, but there is definitely a race condition here (evidenced by this demo of a slightly modified variant of the code).

@xcoulon I've found a fix, but it isn't perfect. Using predicates, we can prevent the Reconcilerfrom waking up on certain events (in this case, updates). This works, but it prevents new pods from spinning up until after other pods have finished terminating.

	err = c.Watch(
		&source.Kind{Type: &corev1.Pod{}},
		&handler.EnqueueRequestForOwner{
			IsController: true,
			OwnerType:    &clonerv1alpha1.Cloner{},
		},
		predicate.Funcs{
			DeleteFunc: func(_ event.DeleteEvent) bool { return true },
			CreateFunc: func(_ event.CreateEvent) bool { return true },
			UpdateFunc: func(_ event.UpdateEvent) bool { return false },
		},
	)