KCP: Consider reconciling on Pod events

[sbueringer]

We recently changed KCP to use a cached client to retrieve the Pods we care about.

This means we already have informers for Pods of workload clusters.

I'm wondering if we should also trigger reconciles if these Pods change (similar to how we watch on Node events in the Machine controller).

Pro

• we can instantly react to Pod changes instead of using requeue after

Con

• we get probably more reconciles
  • This can be mitigated by only letting the update events through that we care about or by rate-limiting. This might even lead to less reconciles when we get rid of the requeues we have because we don't watch on Pod events (

    cluster-api/controlplane/kubeadm/internal/controllers/controller.go

    Line 253 in d934125

    res = ctrl.Result{RequeueAfter: 20 * time.Second}

    )

[chrischdi]

/triage accepted

[chrischdi]

/help

This requires some research about how to avoid too many reconciles + scale tests.

[k8s-ci-robot]

@chrischdi:
This request has been marked as needing help from a contributor.

Guidelines

Please ensure that the issue body includes answers to the following questions:

• Why are we solving this issue?
• To address this issue, are there any code changes? If there are code changes, what needs to be done in the code and what places can the assignee treat as reference points?
• Does this issue have zero to low barrier of entry?
• How can the assignee reach out to you for help?

For more details on the requirements of such an issue, please see here and ensure that they are met.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-help command.

In response to this:

/help

This requires some research about how to avoid too many reconciles + scale tests.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[arshadd-b]

/assign

[arshadd-b]

Hi @sbueringer
I have found where we are watching on node events in machine controller here

But this change I could not find in KCP We recently changed KCP to use a cached client to retrieve the Pods we care about.
Could you please help here?
Thanks

[chrischdi]

The change in KCP was this here:

6e059a2

Please note also the following:

This requires some research about how to avoid too many reconciles + scale tests.

So its not just about start watching and emitting events for this pods.

[arshadd-b]

Thank @chrischdi sharing details

This requires some research about how to avoid too many reconciles + scale tests.

sure will check this

[arshadd-b]

Hi @chrischdi I am trying to add watcher on pod I have added it.
But now for reconciling the KCP controller I need controlPlane object,
but I am not getting that inside function testPod.
From cluster we can get controlPane object but pod, ok := o.(*corev1.Pod) won't have cluster information

func (r *KubeadmControlPlaneReconciler) watchClusterPods(ctx context.Context, cluster *clusterv1.Cluster, controlPlane *internal.ControlPlane) error {
	log := ctrl.LoggerFrom(ctx)
	return r.ClusterCache.Watch(ctx, util.ObjectKey(cluster), clustercache.NewWatcher(clustercache.WatcherOptions{
		Name:         "kcp-watchPods",
		Watcher:      r.controller,
		Kind:         &corev1.Pod{},
		EventHandler: handler.EnqueueRequestsFromMapFunc(r.testPods),
	}))
}

func (r *KubeadmControlPlaneReconciler) testPods(ctx context.Context, o client.Object) []reconcile.Request {
	log := ctrl.LoggerFrom(ctx)
	pod, ok := o.(*corev1.Pod)
	if !ok {
		panic(fmt.Sprintf("Expected a Pod but got a %T", o))
	}
	log.Info("inside testPods::")
	
	labels := pod.GetLabels()
	for k, v := range labels {
		if k == "component" {
			if v == "etcd" || v == "kube-apiserver" || v == "kube-controller-manager" || v == "kube-scheduler" {
				return []reconcile.Request{{NamespacedName: client.ObjectKeyFromObject()}}
			}
		}
	}
}

[chrischdi]

Sorry @arshadd-b for pushing back a little bit on this, but: I think code-wise there is a way to do this today.

What we need instead first is:

This requires some research about how to avoid too many reconciles + scale tests.

We can't just implement and trigger a reconcile for all pod events.
We have to carefully check that we only get events when we want, but also not missing important ones and also see if we can drop requeue's instead.

So its a very case-by-case work which needs lots of research and should be done before jumping to code. Besides that it also has not the highest priority currently.

[arshadd-b]

Hi @chrischdi I actually was thinking I will add this code and build understanding what all pods details getting then seeing the reconciliation frequency and try to do rate limiting reconciliation .
I think I will check this part

This requires some research about how to avoid too many reconciles + scale tests.

and will get back. Thanks