migrate pkg/kubelet/pod to contextual logging

This commit is contained in:
Ed Bartosh
2025-02-01 09:52:02 +02:00
parent 0cb31bc407
commit b96e3cac74
8 changed files with 27 additions and 21 deletions

View File

@@ -214,6 +214,7 @@ linters:
contextual k8s.io/kubernetes/pkg/kubelet/apis/.*
contextual k8s.io/kubernetes/pkg/kubelet/kubeletconfig/.*
contextual k8s.io/kubernetes/pkg/kubelet/nodeshutdown/.*
contextual k8s.io/kubernetes/pkg/kubelet/pod/.*
# As long as contextual logging is alpha or beta, all WithName, WithValues,
# NewContext calls have to go through klog. Once it is GA, we can lift

View File

@@ -228,6 +228,7 @@ linters:
contextual k8s.io/kubernetes/pkg/kubelet/apis/.*
contextual k8s.io/kubernetes/pkg/kubelet/kubeletconfig/.*
contextual k8s.io/kubernetes/pkg/kubelet/nodeshutdown/.*
contextual k8s.io/kubernetes/pkg/kubelet/pod/.*
# As long as contextual logging is alpha or beta, all WithName, WithValues,
# NewContext calls have to go through klog. Once it is GA, we can lift

View File

@@ -60,6 +60,7 @@ contextual k8s.io/kubernetes/pkg/kubelet/sysctl/.*
contextual k8s.io/kubernetes/pkg/kubelet/apis/.*
contextual k8s.io/kubernetes/pkg/kubelet/kubeletconfig/.*
contextual k8s.io/kubernetes/pkg/kubelet/nodeshutdown/.*
contextual k8s.io/kubernetes/pkg/kubelet/pod/.*
# As long as contextual logging is alpha or beta, all WithName, WithValues,
# NewContext calls have to go through klog. Once it is GA, we can lift

View File

@@ -1993,7 +1993,7 @@ func (kl *Kubelet) SyncPod(ctx context.Context, updateType kubetypes.SyncPodType
}
// Create Mirror Pod for Static Pod if it doesn't already exist
kl.tryReconcileMirrorPods(pod, mirrorPod)
kl.tryReconcileMirrorPods(ctx, pod, mirrorPod)
// Make data directories for the pod
if err := kl.makePodDataDirs(pod); err != nil {
@@ -3025,7 +3025,7 @@ func (kl *Kubelet) UnprepareDynamicResources(ctx context.Context, pod *v1.Pod) e
// Ensure Mirror Pod for Static Pod exists and matches the current pod definition.
// The function logs and ignores any errors.
func (kl *Kubelet) tryReconcileMirrorPods(staticPod, mirrorPod *v1.Pod) {
func (kl *Kubelet) tryReconcileMirrorPods(ctx context.Context, staticPod, mirrorPod *v1.Pod) {
if !kubetypes.IsStaticPod(staticPod) {
return
}
@@ -3034,9 +3034,9 @@ func (kl *Kubelet) tryReconcileMirrorPods(staticPod, mirrorPod *v1.Pod) {
if mirrorPod.DeletionTimestamp != nil || !kubepod.IsMirrorPodOf(mirrorPod, staticPod) {
// The mirror pod is semantically different from the static pod. Remove
// it. The mirror pod will get recreated later.
klog.InfoS("Trying to delete pod", "pod", klog.KObj(mirrorPod), "podUID", mirrorPod.ObjectMeta.UID)
klog.InfoS("Trying to delete pod", "pod", klog.KObj(mirrorPod), "podUID", mirrorPod.UID)
podFullName := kubecontainer.GetPodFullName(staticPod)
if ok, err := kl.mirrorPodClient.DeleteMirrorPod(podFullName, &mirrorPod.ObjectMeta.UID); err != nil {
if ok, err := kl.mirrorPodClient.DeleteMirrorPod(ctx, podFullName, &mirrorPod.UID); err != nil {
klog.ErrorS(err, "Failed deleting mirror pod", "pod", klog.KObj(mirrorPod))
} else if ok {
deleted = ok
@@ -3052,7 +3052,7 @@ func (kl *Kubelet) tryReconcileMirrorPods(staticPod, mirrorPod *v1.Pod) {
klog.InfoS("No need to create a mirror pod, since node has been removed from the cluster", "node", klog.KRef("", string(kl.nodeName)))
} else {
klog.InfoS("Creating a mirror pod for static pod", "pod", klog.KObj(staticPod))
if err := kl.mirrorPodClient.CreateMirrorPod(staticPod); err != nil {
if err := kl.mirrorPodClient.CreateMirrorPod(ctx, staticPod); err != nil {
klog.ErrorS(err, "Failed creating a mirror pod", "pod", klog.KObj(staticPod))
}
}
@@ -3075,7 +3075,7 @@ func (kl *Kubelet) fastStaticPodsRegistration(ctx context.Context) {
staticPodToMirrorPodMap := kl.podManager.GetStaticPodToMirrorPodMap()
for staticPod, mirrorPod := range staticPodToMirrorPodMap {
kl.tryReconcileMirrorPods(staticPod, mirrorPod)
kl.tryReconcileMirrorPods(ctx, staticPod, mirrorPod)
}
}

View File

@@ -1245,7 +1245,7 @@ func (kl *Kubelet) HandlePodCleanups(ctx context.Context) error {
klog.V(3).InfoS("Clean up orphaned mirror pods")
for _, podFullname := range orphanedMirrorPodFullnames {
if !kl.podWorkers.IsPodForMirrorPodTerminatingByFullName(podFullname) {
_, err := kl.mirrorPodClient.DeleteMirrorPod(podFullname, nil)
_, err := kl.mirrorPodClient.DeleteMirrorPod(ctx, podFullname, nil)
if err != nil {
klog.ErrorS(err, "Encountered error when deleting mirror pod", "podName", podFullname)
} else {

View File

@@ -36,10 +36,10 @@ type MirrorClient interface {
// pod or returns an error. The mirror pod will have the same annotations
// as the given pod as well as an extra annotation containing the hash of
// the static pod.
CreateMirrorPod(pod *v1.Pod) error
CreateMirrorPod(ctx context.Context, pod *v1.Pod) error
// DeleteMirrorPod deletes the mirror pod with the given full name from
// the API server or returns an error.
DeleteMirrorPod(podFullName string, uid *types.UID) (bool, error)
DeleteMirrorPod(ctx context.Context, podFullName string, uid *types.UID) (bool, error)
}
// nodeGetter is a subset of NodeLister, simplified for testing.
@@ -66,7 +66,7 @@ func NewBasicMirrorClient(apiserverClient clientset.Interface, nodeName string,
}
}
func (mc *basicMirrorClient) CreateMirrorPod(pod *v1.Pod) error {
func (mc *basicMirrorClient) CreateMirrorPod(ctx context.Context, pod *v1.Pod) error {
if mc.apiserverClient == nil {
return nil
}
@@ -96,7 +96,7 @@ func (mc *basicMirrorClient) CreateMirrorPod(pod *v1.Pod) error {
Controller: &controller,
}}
apiPod, err := mc.apiserverClient.CoreV1().Pods(copyPod.Namespace).Create(context.TODO(), &copyPod, metav1.CreateOptions{})
apiPod, err := mc.apiserverClient.CoreV1().Pods(copyPod.Namespace).Create(ctx, &copyPod, metav1.CreateOptions{})
if err != nil && apierrors.IsAlreadyExists(err) {
// Check if the existing pod is the same as the pod we want to create.
if h, ok := apiPod.Annotations[kubetypes.ConfigMirrorAnnotationKey]; ok && h == hash {
@@ -113,13 +113,14 @@ func (mc *basicMirrorClient) CreateMirrorPod(pod *v1.Pod) error {
// while parsing the name of the pod.
// Non-existence of the pod or UID mismatch is not treated as an error; the
// routine simply returns false in that case.
func (mc *basicMirrorClient) DeleteMirrorPod(podFullName string, uid *types.UID) (bool, error) {
func (mc *basicMirrorClient) DeleteMirrorPod(ctx context.Context, podFullName string, uid *types.UID) (bool, error) {
if mc.apiserverClient == nil {
return false, nil
}
logger := klog.FromContext(ctx)
name, namespace, err := kubecontainer.ParsePodFullName(podFullName)
if err != nil {
klog.ErrorS(err, "Failed to parse a pod full name", "podFullName", podFullName)
logger.Error(err, "Failed to parse a pod full name", "podFullName", podFullName)
return false, err
}
@@ -127,15 +128,15 @@ func (mc *basicMirrorClient) DeleteMirrorPod(podFullName string, uid *types.UID)
if uid != nil {
uidValue = *uid
}
klog.V(2).InfoS("Deleting a mirror pod", "pod", klog.KRef(namespace, name), "podUID", uidValue)
logger.V(2).Info("Deleting a mirror pod", "pod", klog.KRef(namespace, name), "podUID", uidValue)
var GracePeriodSeconds int64
if err := mc.apiserverClient.CoreV1().Pods(namespace).Delete(context.TODO(), name, metav1.DeleteOptions{GracePeriodSeconds: &GracePeriodSeconds, Preconditions: &metav1.Preconditions{UID: uid}}); err != nil {
if err := mc.apiserverClient.CoreV1().Pods(namespace).Delete(ctx, name, metav1.DeleteOptions{GracePeriodSeconds: &GracePeriodSeconds, Preconditions: &metav1.Preconditions{UID: uid}}); err != nil {
// Unfortunately, there's no generic error for failing a precondition
if !(apierrors.IsNotFound(err) || apierrors.IsConflict(err)) {
// We should return the error here, but historically this routine does
// not return an error unless it can't parse the pod name
klog.ErrorS(err, "Failed deleting a mirror pod", "pod", klog.KRef(namespace, name))
logger.Error(err, "Failed deleting a mirror pod", "pod", klog.KRef(namespace, name))
}
return false, nil
}

View File

@@ -17,7 +17,6 @@ limitations under the License.
package pod
import (
"context"
"errors"
"testing"
@@ -29,6 +28,7 @@ import (
"k8s.io/client-go/kubernetes/fake"
kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
kubetypes "k8s.io/kubernetes/pkg/kubelet/types"
"k8s.io/kubernetes/test/utils/ktesting"
"k8s.io/utils/ptr"
)
@@ -64,6 +64,7 @@ func TestParsePodFullName(t *testing.T) {
}
func TestCreateMirrorPod(t *testing.T) {
tCtx := ktesting.Init(t)
const (
testNodeName = "test-node-name"
testNodeUID = types.UID("test-node-uid-1234")
@@ -120,13 +121,13 @@ func TestCreateMirrorPod(t *testing.T) {
},
}
err := mc.CreateMirrorPod(pod)
err := mc.CreateMirrorPod(tCtx, pod)
if !test.expectSuccess {
assert.Error(t, err)
return
}
createdPod, err := clientset.CoreV1().Pods(testPodNS).Get(context.TODO(), testPodName, metav1.GetOptions{})
createdPod, err := clientset.CoreV1().Pods(testPodNS).Get(tCtx, testPodName, metav1.GetOptions{})
require.NoError(t, err)
// Validate created pod

View File

@@ -17,6 +17,7 @@ limitations under the License.
package testing
import (
"context"
"sync"
v1 "k8s.io/api/core/v1"
@@ -42,7 +43,7 @@ func NewFakeMirrorClient() *FakeMirrorClient {
return &m
}
func (fmc *FakeMirrorClient) CreateMirrorPod(pod *v1.Pod) error {
func (fmc *FakeMirrorClient) CreateMirrorPod(_ context.Context, pod *v1.Pod) error {
fmc.mirrorPodLock.Lock()
defer fmc.mirrorPodLock.Unlock()
podFullName := kubecontainer.GetPodFullName(pod)
@@ -52,7 +53,7 @@ func (fmc *FakeMirrorClient) CreateMirrorPod(pod *v1.Pod) error {
}
// TODO (Robert Krawitz): Implement UID checking
func (fmc *FakeMirrorClient) DeleteMirrorPod(podFullName string, _ *types.UID) (bool, error) {
func (fmc *FakeMirrorClient) DeleteMirrorPod(_ context.Context, podFullName string, _ *types.UID) (bool, error) {
fmc.mirrorPodLock.Lock()
defer fmc.mirrorPodLock.Unlock()
fmc.mirrorPods.Delete(podFullName)