Skip to content

Commit

Permalink
enhance and fix log calls
Browse files Browse the repository at this point in the history
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
kubernetes/klog#297.

In some cases it reports false positives, but those can be suppressed with
source code comments.

Partial cherry-pick of edffc70
  • Loading branch information
pohly authored and liggitt committed Dec 20, 2022
1 parent c95a8a1 commit 4da1f79
Show file tree
Hide file tree
Showing 7 changed files with 8 additions and 6 deletions.
4 changes: 2 additions & 2 deletions cmd/kubeadm/app/util/users/users_linux.go
Expand Up @@ -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()
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/registry/apps/deployment/storage/storage.go
Expand Up @@ -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())
}
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/registry/apps/replicaset/storage/storage.go
Expand Up @@ -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())
}
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/registry/apps/statefulset/storage/storage.go
Expand Up @@ -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())
}
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/registry/core/replicationcontroller/storage/storage.go
Expand Up @@ -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())
}
}

Expand Down
Expand Up @@ -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())...)
}

Expand Down
1 change: 1 addition & 0 deletions staging/src/k8s.io/component-base/logs/json/json_test.go
Expand Up @@ -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()

Expand Down

0 comments on commit 4da1f79

Please sign in to comment.