From 71b911484096b2877a6d55b8f30da35ae62db025 Mon Sep 17 00:00:00 2001 From: Ed Bartosh Date: Wed, 30 Oct 2024 17:11:05 +0200 Subject: [PATCH] kubelet: Migrate pkg/kubelet/sysctl to contextual logging --- hack/golangci-hints.yaml | 1 + hack/golangci-strict.yaml | 1 + hack/golangci.yaml | 1 + hack/logcheck.conf | 1 + pkg/kubelet/kubelet.go | 2 +- pkg/kubelet/sysctl/allowlist_test.go | 7 +++++-- pkg/kubelet/sysctl/safe_sysctls.go | 12 +++++++----- pkg/kubelet/sysctl/safe_sysctls_test.go | 4 +++- 8 files changed, 20 insertions(+), 9 deletions(-) diff --git a/hack/golangci-hints.yaml b/hack/golangci-hints.yaml index d4efbbc18cf..b411e7abeb0 100644 --- a/hack/golangci-hints.yaml +++ b/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 diff --git a/hack/golangci-strict.yaml b/hack/golangci-strict.yaml index b449c1344c6..6c55b51e8ca 100644 --- a/hack/golangci-strict.yaml +++ b/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 diff --git a/hack/golangci.yaml b/hack/golangci.yaml index ad7c036067d..ba824f48363 100644 --- a/hack/golangci.yaml +++ b/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 diff --git a/hack/logcheck.conf b/hack/logcheck.conf index 2c4c9ce01cc..82507bd11ae 100644 --- a/hack/logcheck.conf +++ b/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 diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index 916115dec20..ada3e7517e8 100644 --- a/pkg/kubelet/kubelet.go +++ b/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 diff --git a/pkg/kubelet/sysctl/allowlist_test.go b/pkg/kubelet/sysctl/allowlist_test.go index 6eddb604c6a..29cd8cb0e6d 100644 --- a/pkg/kubelet/sysctl/allowlist_test.go +++ b/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) } diff --git a/pkg/kubelet/sysctl/safe_sysctls.go b/pkg/kubelet/sysctl/safe_sysctls.go index 17b4ef83fab..d5e1146f378 100644 --- a/pkg/kubelet/sysctl/safe_sysctls.go +++ b/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 diff --git a/pkg/kubelet/sysctl/safe_sysctls_test.go b/pkg/kubelet/sysctl/safe_sysctls_test.go index 06d75260279..3c0f930082b 100644 --- a/pkg/kubelet/sysctl/safe_sysctls_test.go +++ b/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) } })