kubelet: only track pod certificates for admitted pods

Project: kubernetes/kubernetes | Language: Go | Concepts: error-handling, testing-strategy, refactoring | Difficulty: mid
Link: https://github.com/kubernetes/kubernetes/pull/140318

The problem

Previously, the kubelet would call TrackPod to start tracking pod certificates for all pods as soon as they were added, even before running the admission checks. This meant that pods which would later be rejected (not admitted) still had their certificates tracked, which is unnecessary and potentially wasteful.

Why it's non-trivial

A naive fix might simply move the tracking logic, but it's easy to miss the correct place in the lifecycle where a pod is guaranteed to be admitted. The code must ensure that only pods which pass all admission checks are tracked, and that test coverage reflects this guarantee.

The approach

1. Move TrackPod after admission checks

Previously, TrackPod was called immediately after adding the pod to the pod manager, before any admission logic:

-       kl.podCertificateManager.TrackPod(ctx, pod)

This line was removed from before the admission check, so pods that are later rejected are not prematurely tracked.

Now, TrackPod is called only after the pod passes admission:

+           kl.podCertificateManager.TrackPod(ctx, pod)

This line is now placed after the admission logic, ensuring only admitted pods are tracked.

2. Add a test to verify the correct behavior

A new test, TestHandlePodAdditionsTracksCertificatesOnlyForAdmittedPods, was added to ensure that only admitted pods have their certificates tracked:

func TestHandlePodAdditionsTracksCertificatesOnlyForAdmittedPods(t *testing.T) {
	// ...setup omitted...
	recorder := &recordingPodCertificateManager{}
	kl.podCertificateManager = recorder

	pods := []*v1.Pod{
		{ObjectMeta: metav1.ObjectMeta{UID: "123456789", Name: "podA", Namespace: "foo"}},
		{ObjectMeta: metav1.ObjectMeta{UID: "987654321", Name: "podB", Namespace: "foo"}},
	}
	podToReject := pods[0]
	podToAdmit := pods[1]
	podsToReject := []*v1.Pod{podToReject}

	kl.allocationManager.AddPodAdmitHandlers(lifecycle.PodAdmitHandlers{&testPodAdmitHandler{podsToReject: podsToReject}})

	kl.HandlePodAdditions(tCtx, pods)

	if len(recorder.TrackedPods) != 1 || recorder.TrackedPods[0] != podToAdmit.UID {
		t.Errorf("expected only admitted pod %q to be tracked, got %v", podToAdmit.UID, recorder.TrackedPods)
	}
}

This test sets up two pods, arranges for one to be rejected by admission, and asserts that only the admitted pod's UID is tracked.

3. Update the test double for certificate manager

The mock certificate manager now records which pods are tracked and forgotten, so tests can assert on this behavior:

type recordingPodCertificateManager struct {
	TrackedPods   []types.UID
	ForgottenPods []types.UID
}
func (f *recordingPodCertificateManager) TrackPod(ctx context.Context, pod *corev1.Pod) {
	f.TrackedPods = append(f.TrackedPods, pod.UID)
}
func (f *recordingPodCertificateManager) ForgetPod(ctx context.Context, pod *corev1.Pod) {
	f.ForgottenPods = append(f.ForgottenPods, pod.UID)
}

This enables tests to observe exactly which pods were tracked.

From the review thread

No notable review discussion on this PR.

Takeaways

• Only perform resource tracking or side effects for entities that have passed all relevant validation and admission checks.
• When fixing a bug, add or update tests to verify the new, correct behavior and to prevent regressions.
• Use test doubles (mocks or fakes) that record their interactions to enable precise assertions in tests.