kubelet: Migrate pkg/kubelet/sysctl to contextual logging

Author: bart0sh

hack/golangci-hints.yaml

@@ -178,6 +178,7 @@ linters-settings: # please keep this alphabetized
           contextual k8s.io/kubernetes/pkg/kubelet/token/.*
           contextual k8s.io/kubernetes/pkg/kubelet/cadvisor/.*
           contextual k8s.io/kubernetes/pkg/kubelet/oom/.*
+          contextual k8s.io/kubernetes/pkg/kubelet/sysctl/.*
           
           # 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

hack/golangci-strict.yaml

@@ -224,6 +224,7 @@ linters-settings: # please keep this alphabetized
           contextual k8s.io/kubernetes/pkg/kubelet/token/.*
           contextual k8s.io/kubernetes/pkg/kubelet/cadvisor/.*
           contextual k8s.io/kubernetes/pkg/kubelet/oom/.*
+          contextual k8s.io/kubernetes/pkg/kubelet/sysctl/.*
           
           # 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

hack/golangci.yaml

@@ -226,6 +226,7 @@ linters-settings: # please keep this alphabetized
           contextual k8s.io/kubernetes/pkg/kubelet/token/.*
           contextual k8s.io/kubernetes/pkg/kubelet/cadvisor/.*
           contextual k8s.io/kubernetes/pkg/kubelet/oom/.*
+          contextual k8s.io/kubernetes/pkg/kubelet/sysctl/.*
           
           # 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

hack/logcheck.conf

@@ -55,6 +55,7 @@ contextual k8s.io/kubernetes/pkg/kubelet/clustertrustbundle/.*
 contextual k8s.io/kubernetes/pkg/kubelet/token/.*
 contextual k8s.io/kubernetes/pkg/kubelet/cadvisor/.*
 contextual k8s.io/kubernetes/pkg/kubelet/oom/.*
+contextual k8s.io/kubernetes/pkg/kubelet/sysctl/.*
 
 # 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

pkg/kubelet/kubelet.go

@@ -948,7 +948,7 @@ func NewMainKubelet(kubeCfg *kubeletconfiginternal.KubeletConfiguration,
 
 	// Safe, allowed sysctls can always be used as unsafe sysctls in the spec.
 	// Hence, we concatenate those two lists.
-	safeAndUnsafeSysctls := append(sysctl.SafeSysctlAllowlist(), allowedUnsafeSysctls...)
+	safeAndUnsafeSysctls := append(sysctl.SafeSysctlAllowlist(ctx), allowedUnsafeSysctls...)
 	sysctlsAllowlist, err := sysctl.NewAllowlist(safeAndUnsafeSysctls)
 	if err != nil {
 		return nil, err

pkg/kubelet/sysctl/allowlist_test.go

@@ -24,9 +24,11 @@ import (
 
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/kubernetes/pkg/kubelet/lifecycle"
+	"k8s.io/kubernetes/test/utils/ktesting"
 )
 
 func TestNewAllowlist(t *testing.T) {
+	tCtx := ktesting.Init(t)
 	type Test struct {
 		sysctls []string
 		err     bool
@@ -42,7 +44,7 @@ func TestNewAllowlist(t *testing.T) {
 		{sysctls: []string{"foo"}, err: true},
 		{sysctls: []string{"foo*"}, err: true},
 	} {
-		_, err := NewAllowlist(append(SafeSysctlAllowlist(), test.sysctls...))
+		_, err := NewAllowlist(append(SafeSysctlAllowlist(tCtx), test.sysctls...))
 		if test.err && err == nil {
 			t.Errorf("expected an error creating a allowlist for %v", test.sysctls)
 		} else if !test.err && err != nil {
@@ -52,6 +54,7 @@ func TestNewAllowlist(t *testing.T) {
 }
 
 func TestAllowlist(t *testing.T) {
+	tCtx := ktesting.Init(t)
 	type Test struct {
 		sysctl           string
 		hostNet, hostIPC bool
@@ -78,7 +81,7 @@ func TestAllowlist(t *testing.T) {
 	pod.Spec.SecurityContext = &v1.PodSecurityContext{}
 	attrs := &lifecycle.PodAdmitAttributes{Pod: pod}
 
-	w, err := NewAllowlist(append(SafeSysctlAllowlist(), "kernel.msg*", "kernel.sem", "net.b.*"))
+	w, err := NewAllowlist(append(SafeSysctlAllowlist(tCtx), "kernel.msg*", "kernel.sem", "net.b.*"))
 	if err != nil {
 		t.Fatalf("failed to create allowlist: %v", err)
 	}

pkg/kubelet/sysctl/safe_sysctls.go

@@ -17,6 +17,7 @@ limitations under the License.
 package sysctl
 
 import (
+	"context"
 	goruntime "runtime"
 
 	"k8s.io/apimachinery/pkg/util/version"
@@ -75,18 +76,19 @@ var safeSysctls = []sysctl{
 // A sysctl is called safe iff
 // - it is namespaced in the container or the pod
 // - it is isolated, i.e. has no influence on any other pod on the same node.
-func SafeSysctlAllowlist() []string {
+func SafeSysctlAllowlist(ctx context.Context) []string {
 	if goruntime.GOOS != "linux" {
 		return nil
 	}
 
-	return getSafeSysctlAllowlist(utilkernel.GetVersion)
+	return getSafeSysctlAllowlist(ctx, utilkernel.GetVersion)
 }
 
-func getSafeSysctlAllowlist(getVersion func() (*version.Version, error)) []string {
+func getSafeSysctlAllowlist(ctx context.Context, getVersion func() (*version.Version, error)) []string {
+	logger := klog.FromContext(ctx)
 	kernelVersion, err := getVersion()
 	if err != nil {
-		klog.ErrorS(err, "failed to get kernel version, unable to determine which sysctls are available")
+		logger.Error(err, "failed to get kernel version, unable to determine which sysctls are available")
 	}
 
 	var safeSysctlAllowlist []string
@@ -99,7 +101,7 @@ func getSafeSysctlAllowlist(getVersion func() (*version.Version, error)) []strin
 		if kernelVersion != nil && kernelVersion.AtLeast(version.MustParseGeneric(sc.kernel)) {
 			safeSysctlAllowlist = append(safeSysctlAllowlist, sc.name)
 		} else {
-			klog.InfoS("kernel version is too old, dropping the sysctl from safe sysctl list", "kernelVersion", kernelVersion, "sysctl", sc.name)
+			logger.Info("kernel version is too old, dropping the sysctl from safe sysctl list", "kernelVersion", kernelVersion, "sysctl", sc.name)
 		}
 	}
 	return safeSysctlAllowlist

pkg/kubelet/sysctl/safe_sysctls_test.go

@@ -22,9 +22,11 @@ import (
 	"testing"
 
 	"k8s.io/apimachinery/pkg/util/version"
+	"k8s.io/kubernetes/test/utils/ktesting"
 )
 
 func Test_getSafeSysctlAllowlist(t *testing.T) {
+	tCtx := ktesting.Init(t)
 	tests := []struct {
 		name       string
 		getVersion func() (*version.Version, error)
@@ -82,7 +84,7 @@ func Test_getSafeSysctlAllowlist(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			if got := getSafeSysctlAllowlist(tt.getVersion); !reflect.DeepEqual(got, tt.want) {
+			if got := getSafeSysctlAllowlist(tCtx, tt.getVersion); !reflect.DeepEqual(got, tt.want) {
 				t.Errorf("getSafeSysctlAllowlist() = %v, want %v", got, tt.want)
 			}
 		})