Merge pull request kubernetes#128447 from bart0sh/PR164-migrate-cadvisor-to-contextual-logging

k8s-ci-robot · web-flow · commit 0edef5aa918a · 2024-11-06T20:10:10.000Z
kubelet: Migrate CAdvisor to contextual logging
diff --git a/hack/golangci-hints.yaml b/hack/golangci-hints.yaml
@@ -167,6 +167,7 @@ linters-settings: # please keep this alphabetized
           contextual k8s.io/kubernetes/pkg/kubelet/pleg/.*
           contextual k8s.io/kubernetes/pkg/kubelet/clustertrustbundle/.*
           contextual k8s.io/kubernetes/pkg/kubelet/token/.*
+          contextual k8s.io/kubernetes/pkg/kubelet/cadvisor/.*
           
           # 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
@@ -213,6 +213,7 @@ linters-settings: # please keep this alphabetized
           contextual k8s.io/kubernetes/pkg/kubelet/pleg/.*
           contextual k8s.io/kubernetes/pkg/kubelet/clustertrustbundle/.*
           contextual k8s.io/kubernetes/pkg/kubelet/token/.*
+          contextual k8s.io/kubernetes/pkg/kubelet/cadvisor/.*
           
           # 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
@@ -215,6 +215,7 @@ linters-settings: # please keep this alphabetized
           contextual k8s.io/kubernetes/pkg/kubelet/pleg/.*
           contextual k8s.io/kubernetes/pkg/kubelet/clustertrustbundle/.*
           contextual k8s.io/kubernetes/pkg/kubelet/token/.*
+          contextual k8s.io/kubernetes/pkg/kubelet/cadvisor/.*
           
           # 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
@@ -51,6 +51,7 @@ contextual k8s.io/kubernetes/pkg/kubelet/cm/dra/.*
 contextual k8s.io/kubernetes/pkg/kubelet/pleg/.*
 contextual k8s.io/kubernetes/pkg/kubelet/clustertrustbundle/.*
 contextual k8s.io/kubernetes/pkg/kubelet/token/.*
+contextual k8s.io/kubernetes/pkg/kubelet/cadvisor/.*
 
 # 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/cadvisor/cadvisor_linux.go b/pkg/kubelet/cadvisor/cadvisor_linux.go
@@ -20,6 +20,7 @@ limitations under the License.
 package cadvisor
 
 import (
+	"context"
 	"flag"
 	"fmt"
 	"net/http"
@@ -71,7 +72,8 @@ func init() {
 			f.DefValue = defaultValue
 			f.Value.Set(defaultValue)
 		} else {
-			klog.ErrorS(nil, "Expected cAdvisor flag not found", "flag", name)
+			ctx := context.Background()
+			klog.FromContext(ctx).Error(nil, "Expected cAdvisor flag not found", "flag", name)
 		}
 	}
 }
@@ -140,19 +142,19 @@ func (cc *cadvisorClient) MachineInfo() (*cadvisorapi.MachineInfo, error) {
 	return cc.GetMachineInfo()
 }
 
-func (cc *cadvisorClient) ImagesFsInfo() (cadvisorapiv2.FsInfo, error) {
+func (cc *cadvisorClient) ImagesFsInfo(ctx context.Context) (cadvisorapiv2.FsInfo, error) {
 	label, err := cc.imageFsInfoProvider.ImageFsInfoLabel()
 	if err != nil {
 		return cadvisorapiv2.FsInfo{}, err
 	}
-	return cc.getFsInfo(label)
+	return cc.getFsInfo(ctx, label)
 }
 
 func (cc *cadvisorClient) RootFsInfo() (cadvisorapiv2.FsInfo, error) {
 	return cc.GetDirFsInfo(cc.rootPath)
 }
 
-func (cc *cadvisorClient) getFsInfo(label string) (cadvisorapiv2.FsInfo, error) {
+func (cc *cadvisorClient) getFsInfo(ctx context.Context, label string) (cadvisorapiv2.FsInfo, error) {
 	res, err := cc.GetFsInfo(label)
 	if err != nil {
 		return cadvisorapiv2.FsInfo{}, err
@@ -162,16 +164,16 @@ func (cc *cadvisorClient) getFsInfo(label string) (cadvisorapiv2.FsInfo, error)
 	}
 	// TODO(vmarmol): Handle this better when a label has more than one image filesystem.
 	if len(res) > 1 {
-		klog.InfoS("More than one filesystem labeled. Only using the first one", "label", label, "fileSystem", res)
+		klog.FromContext(ctx).Info("More than one filesystem labeled. Only using the first one", "label", label, "fileSystem", res)
 	}
 
 	return res[0], nil
 }
 
-func (cc *cadvisorClient) ContainerFsInfo() (cadvisorapiv2.FsInfo, error) {
+func (cc *cadvisorClient) ContainerFsInfo(ctx context.Context) (cadvisorapiv2.FsInfo, error) {
 	label, err := cc.imageFsInfoProvider.ContainerFsInfoLabel()
 	if err != nil {
 		return cadvisorapiv2.FsInfo{}, err
 	}
-	return cc.getFsInfo(label)
+	return cc.getFsInfo(ctx, label)
 }
diff --git a/pkg/kubelet/cadvisor/cadvisor_unsupported.go b/pkg/kubelet/cadvisor/cadvisor_unsupported.go
@@ -20,6 +20,7 @@ limitations under the License.
 package cadvisor
 
 import (
+	"context"
 	"errors"
 
 	cadvisorapi "github.com/google/cadvisor/info/v1"
@@ -58,15 +59,15 @@ func (cu *cadvisorUnsupported) VersionInfo() (*cadvisorapi.VersionInfo, error) {
 	return nil, errUnsupported
 }
 
-func (cu *cadvisorUnsupported) ImagesFsInfo() (cadvisorapiv2.FsInfo, error) {
+func (cu *cadvisorUnsupported) ImagesFsInfo(context.Context) (cadvisorapiv2.FsInfo, error) {
 	return cadvisorapiv2.FsInfo{}, errUnsupported
 }
 
 func (cu *cadvisorUnsupported) RootFsInfo() (cadvisorapiv2.FsInfo, error) {
 	return cadvisorapiv2.FsInfo{}, errUnsupported
 }
 
-func (cu *cadvisorUnsupported) ContainerFsInfo() (cadvisorapiv2.FsInfo, error) {
+func (cu *cadvisorUnsupported) ContainerFsInfo(context.Context) (cadvisorapiv2.FsInfo, error) {
 	return cadvisorapiv2.FsInfo{}, errUnsupported
 }
 
diff --git a/pkg/kubelet/cadvisor/cadvisor_windows.go b/pkg/kubelet/cadvisor/cadvisor_windows.go
@@ -20,6 +20,8 @@ limitations under the License.
 package cadvisor
 
 import (
+	"context"
+
 	cadvisorapi "github.com/google/cadvisor/info/v1"
 	cadvisorapiv2 "github.com/google/cadvisor/info/v2"
 	"k8s.io/kubernetes/pkg/kubelet/winstats"
@@ -62,11 +64,11 @@ func (cu *cadvisorClient) VersionInfo() (*cadvisorapi.VersionInfo, error) {
 	return cu.winStatsClient.WinVersionInfo()
 }
 
-func (cu *cadvisorClient) ImagesFsInfo() (cadvisorapiv2.FsInfo, error) {
+func (cu *cadvisorClient) ImagesFsInfo(context.Context) (cadvisorapiv2.FsInfo, error) {
 	return cadvisorapiv2.FsInfo{}, nil
 }
 
-func (cu *cadvisorClient) ContainerFsInfo() (cadvisorapiv2.FsInfo, error) {
+func (cu *cadvisorClient) ContainerFsInfo(context.Context) (cadvisorapiv2.FsInfo, error) {
 	return cadvisorapiv2.FsInfo{}, nil
 }
 
diff --git a/pkg/kubelet/cadvisor/testing/cadvisor_fake.go b/pkg/kubelet/cadvisor/testing/cadvisor_fake.go
@@ -17,6 +17,8 @@ limitations under the License.
 package testing
 
 import (
+	"context"
+
 	cadvisorapi "github.com/google/cadvisor/info/v1"
 	cadvisorapiv2 "github.com/google/cadvisor/info/v2"
 	"k8s.io/kubernetes/pkg/kubelet/cadvisor"
@@ -76,7 +78,7 @@ func (c *Fake) VersionInfo() (*cadvisorapi.VersionInfo, error) {
 }
 
 // ImagesFsInfo is a fake implementation of Interface.ImagesFsInfo.
-func (c *Fake) ImagesFsInfo() (cadvisorapiv2.FsInfo, error) {
+func (c *Fake) ImagesFsInfo(context.Context) (cadvisorapiv2.FsInfo, error) {
 	return cadvisorapiv2.FsInfo{}, nil
 }
 
@@ -86,7 +88,7 @@ func (c *Fake) RootFsInfo() (cadvisorapiv2.FsInfo, error) {
 }
 
 // ContainerFsInfo is a fake implementation of Interface.ContainerFsInfo.
-func (c *Fake) ContainerFsInfo() (cadvisorapiv2.FsInfo, error) {
+func (c *Fake) ContainerFsInfo(context.Context) (cadvisorapiv2.FsInfo, error) {
 	return cadvisorapiv2.FsInfo{}, nil
 }
 
diff --git a/pkg/kubelet/cadvisor/testing/cadvisor_mock.go b/pkg/kubelet/cadvisor/testing/cadvisor_mock.go
diff --git a/pkg/kubelet/cadvisor/types.go b/pkg/kubelet/cadvisor/types.go
@@ -18,6 +18,8 @@ limitations under the License.
 package cadvisor
 
 import (
+	"context"
+
 	cadvisorapi "github.com/google/cadvisor/info/v1"
 	cadvisorapiv2 "github.com/google/cadvisor/info/v2"
 )
@@ -32,14 +34,14 @@ type Interface interface {
 	VersionInfo() (*cadvisorapi.VersionInfo, error)
 
 	// Returns usage information about the filesystem holding container images.
-	ImagesFsInfo() (cadvisorapiv2.FsInfo, error)
+	ImagesFsInfo(context.Context) (cadvisorapiv2.FsInfo, error)
 
 	// Returns usage information about the root filesystem.
 	RootFsInfo() (cadvisorapiv2.FsInfo, error)
 
 	// Returns usage information about the writeable layer.
 	// KEP 4191 can separate the image filesystem
-	ContainerFsInfo() (cadvisorapiv2.FsInfo, error)
+	ContainerFsInfo(context.Context) (cadvisorapiv2.FsInfo, error)
 
 	// Get filesystem information for the filesystem that contains the given file.
 	GetDirFsInfo(path string) (cadvisorapiv2.FsInfo, error)
diff --git a/pkg/kubelet/kubelet_test.go b/pkg/kubelet/kubelet_test.go
@@ -60,7 +60,6 @@ import (
 	remote "k8s.io/cri-client/pkg"
 	fakeremote "k8s.io/cri-client/pkg/fake"
 	"k8s.io/klog/v2"
-	"k8s.io/klog/v2/ktesting"
 	"k8s.io/kubernetes/pkg/features"
 	kubeletconfiginternal "k8s.io/kubernetes/pkg/kubelet/apis/config"
 	cadvisortest "k8s.io/kubernetes/pkg/kubelet/cadvisor/testing"
@@ -104,6 +103,7 @@ import (
 	"k8s.io/kubernetes/pkg/volume/util"
 	"k8s.io/kubernetes/pkg/volume/util/hostutil"
 	"k8s.io/kubernetes/pkg/volume/util/subpath"
+	"k8s.io/kubernetes/test/utils/ktesting"
 	"k8s.io/utils/clock"
 	testingclock "k8s.io/utils/clock/testing"
 	"k8s.io/utils/ptr"
@@ -3091,6 +3091,7 @@ func createRemoteRuntimeService(endpoint string, t *testing.T, tp oteltrace.Trac
 }
 
 func TestNewMainKubeletStandAlone(t *testing.T) {
+	tCtx := ktesting.Init(t)
 	tempDir, err := os.MkdirTemp("", "logs")
 	ContainerLogsDir = tempDir
 	assert.NoError(t, err)
@@ -3106,7 +3107,7 @@ func TestNewMainKubeletStandAlone(t *testing.T) {
 	tp := noopoteltrace.NewTracerProvider()
 	cadvisor := cadvisortest.NewMockInterface(t)
 	cadvisor.EXPECT().MachineInfo().Return(&cadvisorapi.MachineInfo{}, nil).Maybe()
-	cadvisor.EXPECT().ImagesFsInfo().Return(cadvisorapiv2.FsInfo{
+	cadvisor.EXPECT().ImagesFsInfo(tCtx).Return(cadvisorapiv2.FsInfo{
 		Usage:     400,
 		Capacity:  1000,
 		Available: 600,
diff --git a/pkg/kubelet/runonce_test.go b/pkg/kubelet/runonce_test.go
@@ -59,7 +59,7 @@ func TestRunOnce(t *testing.T) {
 
 	cadvisor := cadvisortest.NewMockInterface(t)
 	cadvisor.EXPECT().MachineInfo().Return(&cadvisorapi.MachineInfo{}, nil).Maybe()
-	cadvisor.EXPECT().ImagesFsInfo().Return(cadvisorapiv2.FsInfo{
+	cadvisor.EXPECT().ImagesFsInfo(ctx).Return(cadvisorapiv2.FsInfo{
 		Usage:     400,
 		Capacity:  1000,
 		Available: 600,
diff --git a/pkg/kubelet/stats/cadvisor_stats_provider.go b/pkg/kubelet/stats/cadvisor_stats_provider.go
diff --git a/pkg/kubelet/stats/cadvisor_stats_provider_test.go b/pkg/kubelet/stats/cadvisor_stats_provider_test.go
diff --git a/pkg/kubelet/stats/provider.go b/pkg/kubelet/stats/provider.go

Original file line number	Diff line number	Diff line change
`@@ -20,6 +20,7 @@ limitations under the License.`
`20`	`20`	`package cadvisor`
`21`	`21`
`22`	`22`	`import (`
	`23`	`+ "context"`
`23`	`24`	`"flag"`
`24`	`25`	`"fmt"`
`25`	`26`	`"net/http"`
`@@ -71,7 +72,8 @@ func init() {`
`71`	`72`	`f.DefValue = defaultValue`
`72`	`73`	`f.Value.Set(defaultValue)`
`73`	`74`	`} else {`
`74`		`- klog.ErrorS(nil, "Expected cAdvisor flag not found", "flag", name)`
	`75`	`+ ctx := context.Background()`
	`76`	`+ klog.FromContext(ctx).Error(nil, "Expected cAdvisor flag not found", "flag", name)`
`75`	`77`	`}`
`76`	`78`	`}`
`77`	`79`	`}`
`@@ -140,19 +142,19 @@ func (cc cadvisorClient) MachineInfo() (cadvisorapi.MachineInfo, error) {`
`140`	`142`	`return cc.GetMachineInfo()`
`141`	`143`	`}`
`142`	`144`
`143`		`-func (cc *cadvisorClient) ImagesFsInfo() (cadvisorapiv2.FsInfo, error) {`
	`145`	`+func (cc *cadvisorClient) ImagesFsInfo(ctx context.Context) (cadvisorapiv2.FsInfo, error) {`
`144`	`146`	`label, err := cc.imageFsInfoProvider.ImageFsInfoLabel()`
`145`	`147`	`if err != nil {`
`146`	`148`	`return cadvisorapiv2.FsInfo{}, err`
`147`	`149`	`}`
`148`		`- return cc.getFsInfo(label)`
	`150`	`+ return cc.getFsInfo(ctx, label)`
`149`	`151`	`}`
`150`	`152`
`151`	`153`	`func (cc *cadvisorClient) RootFsInfo() (cadvisorapiv2.FsInfo, error) {`
`152`	`154`	`return cc.GetDirFsInfo(cc.rootPath)`
`153`	`155`	`}`
`154`	`156`
`155`		`-func (cc *cadvisorClient) getFsInfo(label string) (cadvisorapiv2.FsInfo, error) {`
	`157`	`+func (cc *cadvisorClient) getFsInfo(ctx context.Context, label string) (cadvisorapiv2.FsInfo, error) {`
`156`	`158`	`res, err := cc.GetFsInfo(label)`
`157`	`159`	`if err != nil {`
`158`	`160`	`return cadvisorapiv2.FsInfo{}, err`
`@@ -162,16 +164,16 @@ func (cc *cadvisorClient) getFsInfo(label string) (cadvisorapiv2.FsInfo, error)`
`162`	`164`	`}`
`163`	`165`	`// TODO(vmarmol): Handle this better when a label has more than one image filesystem.`
`164`	`166`	`if len(res) > 1 {`
`165`		`- klog.InfoS("More than one filesystem labeled. Only using the first one", "label", label, "fileSystem", res)`
	`167`	`+ klog.FromContext(ctx).Info("More than one filesystem labeled. Only using the first one", "label", label, "fileSystem", res)`
`166`	`168`	`}`
`167`	`169`
`168`	`170`	`return res[0], nil`
`169`	`171`	`}`
`170`	`172`
`171`		`-func (cc *cadvisorClient) ContainerFsInfo() (cadvisorapiv2.FsInfo, error) {`
	`173`	`+func (cc *cadvisorClient) ContainerFsInfo(ctx context.Context) (cadvisorapiv2.FsInfo, error) {`
`172`	`174`	`label, err := cc.imageFsInfoProvider.ContainerFsInfoLabel()`
`173`	`175`	`if err != nil {`
`174`	`176`	`return cadvisorapiv2.FsInfo{}, err`
`175`	`177`	`}`
`176`		`- return cc.getFsInfo(label)`
	`178`	`+ return cc.getFsInfo(ctx, label)`
`177`	`179`	`}`
Original file line number	Diff line number	Diff line change
`@@ -20,6 +20,8 @@ limitations under the License.`
`20`	`20`	`package cadvisor`
`21`	`21`
`22`	`22`	`import (`
	`23`	`+ "context"`
	`24`	`+`
`23`	`25`	`cadvisorapi "github.com/google/cadvisor/info/v1"`
`24`	`26`	`cadvisorapiv2 "github.com/google/cadvisor/info/v2"`
`25`	`27`	`"k8s.io/kubernetes/pkg/kubelet/winstats"`
`@@ -62,11 +64,11 @@ func (cu cadvisorClient) VersionInfo() (cadvisorapi.VersionInfo, error) {`
`62`	`64`	`return cu.winStatsClient.WinVersionInfo()`
`63`	`65`	`}`
`64`	`66`
`65`		`-func (cu *cadvisorClient) ImagesFsInfo() (cadvisorapiv2.FsInfo, error) {`
	`67`	`+func (cu *cadvisorClient) ImagesFsInfo(context.Context) (cadvisorapiv2.FsInfo, error) {`
`66`	`68`	`return cadvisorapiv2.FsInfo{}, nil`
`67`	`69`	`}`
`68`	`70`
`69`		`-func (cu *cadvisorClient) ContainerFsInfo() (cadvisorapiv2.FsInfo, error) {`
	`71`	`+func (cu *cadvisorClient) ContainerFsInfo(context.Context) (cadvisorapiv2.FsInfo, error) {`
`70`	`72`	`return cadvisorapiv2.FsInfo{}, nil`
`71`	`73`	`}`
`72`	`74`
Original file line number	Diff line number	Diff line change
`@@ -17,6 +17,8 @@ limitations under the License.`
`17`	`17`	`package testing`
`18`	`18`
`19`	`19`	`import (`
	`20`	`+ "context"`
	`21`	`+`
`20`	`22`	`cadvisorapi "github.com/google/cadvisor/info/v1"`
`21`	`23`	`cadvisorapiv2 "github.com/google/cadvisor/info/v2"`
`22`	`24`	`"k8s.io/kubernetes/pkg/kubelet/cadvisor"`
`@@ -76,7 +78,7 @@ func (c Fake) VersionInfo() (cadvisorapi.VersionInfo, error) {`
`76`	`78`	`}`
`77`	`79`
`78`	`80`	`// ImagesFsInfo is a fake implementation of Interface.ImagesFsInfo.`
`79`		`-func (c *Fake) ImagesFsInfo() (cadvisorapiv2.FsInfo, error) {`
	`81`	`+func (c *Fake) ImagesFsInfo(context.Context) (cadvisorapiv2.FsInfo, error) {`
`80`	`82`	`return cadvisorapiv2.FsInfo{}, nil`
`81`	`83`	`}`
`82`	`84`
`@@ -86,7 +88,7 @@ func (c *Fake) RootFsInfo() (cadvisorapiv2.FsInfo, error) {`
`86`	`88`	`}`
`87`	`89`
`88`	`90`	`// ContainerFsInfo is a fake implementation of Interface.ContainerFsInfo.`
`89`		`-func (c *Fake) ContainerFsInfo() (cadvisorapiv2.FsInfo, error) {`
	`91`	`+func (c *Fake) ContainerFsInfo(context.Context) (cadvisorapiv2.FsInfo, error) {`
`90`	`92`	`return cadvisorapiv2.FsInfo{}, nil`
`91`	`93`	`}`
`92`	`94`