From 4da1f79f991f52f69dbb9d567bdd4f5e1d7b0b24 Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Sat, 17 Dec 2022 05:09:19 +0000 Subject: [PATCH] enhance and fix log calls Some of these changes are cosmetic (repeatedly calling klog.V instead of reusing the result), others address real issues: - Logging a message only above a certain verbosity threshold without recording that verbosity level (if klog.V().Enabled() { klog.Info... }): this matters when using a logging backend which records the verbosity level. - Passing a format string with parameters to a logging function that doesn't do string formatting. All of these locations where found by the enhanced logcheck tool from https://github.com/kubernetes/klog/pull/297. In some cases it reports false positives, but those can be suppressed with source code comments. Partial cherry-pick of edffc700a43e610f641907290a5152ca593bad79 --- cmd/kubeadm/app/util/users/users_linux.go | 4 ++-- pkg/registry/apps/deployment/storage/storage.go | 2 +- pkg/registry/apps/replicaset/storage/storage.go | 2 +- pkg/registry/apps/statefulset/storage/storage.go | 2 +- pkg/registry/core/replicationcontroller/storage/storage.go | 2 +- .../pkg/registry/generic/registry/storage_factory.go | 1 + staging/src/k8s.io/component-base/logs/json/json_test.go | 1 + 7 files changed, 8 insertions(+), 6 deletions(-) diff --git a/cmd/kubeadm/app/util/users/users_linux.go b/cmd/kubeadm/app/util/users/users_linux.go index 8b9310d9243c..63fc544c8bf9 100644 --- a/cmd/kubeadm/app/util/users/users_linux.go +++ b/cmd/kubeadm/app/util/users/users_linux.go @@ -149,11 +149,11 @@ func addUsersAndGroupsImpl(pathLoginDef, pathUsers, pathGroups string) (*UsersAn var loginDef string f, close, err := openFileWithLock(pathLoginDef) if err != nil { - klog.V(1).Info("Could not open %q, using default system limits: %v", pathLoginDef, err) + klog.V(1).Infof("Could not open %q, using default system limits: %v", pathLoginDef, err) } else { loginDef, err = readFile(f) if err != nil { - klog.V(1).Info("Could not read %q, using default system limits: %v", pathLoginDef, err) + klog.V(1).Infof("Could not read %q, using default system limits: %v", pathLoginDef, err) } close() } diff --git a/pkg/registry/apps/deployment/storage/storage.go b/pkg/registry/apps/deployment/storage/storage.go index 773ec94b1847..4d9a4d7ad9f3 100644 --- a/pkg/registry/apps/deployment/storage/storage.go +++ b/pkg/registry/apps/deployment/storage/storage.go @@ -398,7 +398,7 @@ func (i *scaleUpdatedObjectInfo) UpdatedObject(ctx context.Context, oldObj runti if _, ok := replicasPathInDeployment[requestGroupVersion.String()]; ok { groupVersion = requestGroupVersion } else { - klog.Fatal("Unrecognized group/version in request info %q", requestGroupVersion.String()) + klog.Fatalf("Unrecognized group/version in request info %q", requestGroupVersion.String()) } } diff --git a/pkg/registry/apps/replicaset/storage/storage.go b/pkg/registry/apps/replicaset/storage/storage.go index 11e51a300b48..a16c33bde5ab 100644 --- a/pkg/registry/apps/replicaset/storage/storage.go +++ b/pkg/registry/apps/replicaset/storage/storage.go @@ -299,7 +299,7 @@ func (i *scaleUpdatedObjectInfo) UpdatedObject(ctx context.Context, oldObj runti if _, ok := replicasPathInReplicaSet[requestGroupVersion.String()]; ok { groupVersion = requestGroupVersion } else { - klog.Fatal("Unrecognized group/version in request info %q", requestGroupVersion.String()) + klog.Fatalf("Unrecognized group/version in request info %q", requestGroupVersion.String()) } } diff --git a/pkg/registry/apps/statefulset/storage/storage.go b/pkg/registry/apps/statefulset/storage/storage.go index 6035350ac078..052d0265286b 100644 --- a/pkg/registry/apps/statefulset/storage/storage.go +++ b/pkg/registry/apps/statefulset/storage/storage.go @@ -286,7 +286,7 @@ func (i *scaleUpdatedObjectInfo) UpdatedObject(ctx context.Context, oldObj runti if _, ok := replicasPathInStatefulSet[requestGroupVersion.String()]; ok { groupVersion = requestGroupVersion } else { - klog.Fatal("Unrecognized group/version in request info %q", requestGroupVersion.String()) + klog.Fatalf("Unrecognized group/version in request info %q", requestGroupVersion.String()) } } diff --git a/pkg/registry/core/replicationcontroller/storage/storage.go b/pkg/registry/core/replicationcontroller/storage/storage.go index 925a308eec20..7f9639645751 100644 --- a/pkg/registry/core/replicationcontroller/storage/storage.go +++ b/pkg/registry/core/replicationcontroller/storage/storage.go @@ -258,7 +258,7 @@ func (i *scaleUpdatedObjectInfo) UpdatedObject(ctx context.Context, oldObj runti if _, ok := replicasPathInReplicationController[requestGroupVersion.String()]; ok { groupVersion = requestGroupVersion } else { - klog.Fatal("Unrecognized group/version in request info %q", requestGroupVersion.String()) + klog.Fatalf("Unrecognized group/version in request info %q", requestGroupVersion.String()) } } diff --git a/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/storage_factory.go b/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/storage_factory.go index 6a4426ee6091..0d1c00c0086f 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/storage_factory.go +++ b/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/storage_factory.go @@ -50,6 +50,7 @@ func StorageWithCacher() generic.StorageDecorator { return s, d, err } if klog.V(5).Enabled() { + //nolint:logcheck // It complains about the key/value pairs because it cannot check them. klog.InfoS("Storage caching is enabled", objectTypeToArgs(newFunc())...) } diff --git a/staging/src/k8s.io/component-base/logs/json/json_test.go b/staging/src/k8s.io/component-base/logs/json/json_test.go index 75edaad09863..4f2add678077 100644 --- a/staging/src/k8s.io/component-base/logs/json/json_test.go +++ b/staging/src/k8s.io/component-base/logs/json/json_test.go @@ -65,6 +65,7 @@ func TestZapLoggerInfo(t *testing.T) { var buffer bytes.Buffer writer := zapcore.AddSync(&buffer) sampleInfoLogger, _ := NewJSONLogger(writer, nil) + // nolint:logcheck // The linter cannot and doesn't need to check the key/value pairs. sampleInfoLogger.Info(data.msg, data.keysValues...) logStr := buffer.String()