From f0c024d44101c3002f762d5cf7fe7b5ed9387993 Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Tue, 29 Sep 2020 13:46:51 +0100 Subject: [PATCH] Fix missing logs due to mixed klog versions This makes klog consistent with the core of the Kubernetes project (as of v1.19) which used klog/v2. While on differing versions we will lose logs (only one version will bind the correct flag value) so we should upgrade to v2 to ensure all logs are being printed correctly. --- cmd/manager/main.go | 4 +- cmd/termination-handler/main.go | 4 +- go.mod | 1 + pkg/actuators/machine/actuator.go | 2 +- pkg/actuators/machine/instances.go | 2 +- pkg/actuators/machine/loadbalancers.go | 4 +- pkg/actuators/machine/machine_scope.go | 2 +- pkg/actuators/machine/reconciler.go | 2 +- pkg/actuators/machine/utils.go | 2 +- pkg/apis/awsprovider/v1beta1/register.go | 2 +- vendor/k8s.io/klog/v2/klogr/README.md | 8 + vendor/k8s.io/klog/v2/klogr/klogr.go | 201 +++++++++++++++++++++++ vendor/modules.txt | 1 + 13 files changed, 223 insertions(+), 12 deletions(-) create mode 100644 vendor/k8s.io/klog/v2/klogr/README.md create mode 100644 vendor/k8s.io/klog/v2/klogr/klogr.go diff --git a/cmd/manager/main.go b/cmd/manager/main.go index dbc16b8a4c..a6f6552e0d 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -24,8 +24,8 @@ import ( "github.com/openshift/machine-api-operator/pkg/controller/machine" "github.com/openshift/machine-api-operator/pkg/metrics" _ "k8s.io/client-go/plugin/pkg/client/auth/gcp" - "k8s.io/klog" - "k8s.io/klog/klogr" + "k8s.io/klog/v2" + "k8s.io/klog/v2/klogr" machineactuator "sigs.k8s.io/cluster-api-provider-aws/pkg/actuators/machine" machinesetcontroller "sigs.k8s.io/cluster-api-provider-aws/pkg/actuators/machineset" awsclient "sigs.k8s.io/cluster-api-provider-aws/pkg/client" diff --git a/cmd/termination-handler/main.go b/cmd/termination-handler/main.go index c4a4a0500c..63a011f5fe 100644 --- a/cmd/termination-handler/main.go +++ b/cmd/termination-handler/main.go @@ -22,8 +22,8 @@ import ( "os" "time" - "k8s.io/klog" - "k8s.io/klog/klogr" + "k8s.io/klog/v2" + "k8s.io/klog/v2/klogr" "sigs.k8s.io/cluster-api-provider-aws/pkg/termination" "sigs.k8s.io/cluster-api-provider-aws/pkg/version" ctrl "sigs.k8s.io/controller-runtime" diff --git a/go.mod b/go.mod index b4391f25e6..b68ffacf60 100644 --- a/go.mod +++ b/go.mod @@ -17,6 +17,7 @@ require ( k8s.io/apimachinery v0.19.0 k8s.io/client-go v0.19.0 k8s.io/klog v1.0.0 + k8s.io/klog/v2 v2.3.0 k8s.io/utils v0.0.0-20200729134348-d5654de09c73 sigs.k8s.io/controller-runtime v0.6.2 sigs.k8s.io/controller-tools v0.3.0 diff --git a/pkg/actuators/machine/actuator.go b/pkg/actuators/machine/actuator.go index 998352efa5..752ea565c2 100644 --- a/pkg/actuators/machine/actuator.go +++ b/pkg/actuators/machine/actuator.go @@ -23,7 +23,7 @@ import ( machinev1 "github.com/openshift/machine-api-operator/pkg/apis/machine/v1beta1" corev1 "k8s.io/api/core/v1" "k8s.io/client-go/tools/record" - "k8s.io/klog" + "k8s.io/klog/v2" awsclient "sigs.k8s.io/cluster-api-provider-aws/pkg/client" runtimeclient "sigs.k8s.io/controller-runtime/pkg/client" ) diff --git a/pkg/actuators/machine/instances.go b/pkg/actuators/machine/instances.go index 022df70498..2218cb5af9 100644 --- a/pkg/actuators/machine/instances.go +++ b/pkg/actuators/machine/instances.go @@ -13,7 +13,7 @@ import ( machinev1 "github.com/openshift/machine-api-operator/pkg/apis/machine/v1beta1" mapierrors "github.com/openshift/machine-api-operator/pkg/controller/machine" "github.com/openshift/machine-api-operator/pkg/metrics" - "k8s.io/klog" + "k8s.io/klog/v2" awsproviderv1 "sigs.k8s.io/cluster-api-provider-aws/pkg/apis/awsprovider/v1beta1" runtimeclient "sigs.k8s.io/controller-runtime/pkg/client" diff --git a/pkg/actuators/machine/loadbalancers.go b/pkg/actuators/machine/loadbalancers.go index 9ee0888159..f74ae8b7b4 100644 --- a/pkg/actuators/machine/loadbalancers.go +++ b/pkg/actuators/machine/loadbalancers.go @@ -5,7 +5,7 @@ import ( "strings" errorutil "k8s.io/apimachinery/pkg/util/errors" - "k8s.io/klog" + "k8s.io/klog/v2" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" @@ -66,7 +66,7 @@ func registerWithNetworkLoadBalancers(client awsclient.Client, names []string, i targetGroups[*targetGroup.TargetGroupArn] = targetGroup } } - if klog.V(4) { + if klog.V(4).Enabled() { targetGroupArns := make([]string, 0, len(targetGroups)) for arn := range targetGroups { targetGroupArns = append(targetGroupArns, fmt.Sprintf("%q", arn)) diff --git a/pkg/actuators/machine/machine_scope.go b/pkg/actuators/machine/machine_scope.go index 2677946459..4a28aee18a 100644 --- a/pkg/actuators/machine/machine_scope.go +++ b/pkg/actuators/machine/machine_scope.go @@ -9,7 +9,7 @@ import ( machinev1 "github.com/openshift/machine-api-operator/pkg/apis/machine/v1beta1" machineapierros "github.com/openshift/machine-api-operator/pkg/controller/machine" corev1 "k8s.io/api/core/v1" - "k8s.io/klog" + "k8s.io/klog/v2" awsproviderv1 "sigs.k8s.io/cluster-api-provider-aws/pkg/apis/awsprovider/v1beta1" awsclient "sigs.k8s.io/cluster-api-provider-aws/pkg/client" runtimeclient "sigs.k8s.io/controller-runtime/pkg/client" diff --git a/pkg/actuators/machine/reconciler.go b/pkg/actuators/machine/reconciler.go index 47db89d81f..1a93ede9d8 100644 --- a/pkg/actuators/machine/reconciler.go +++ b/pkg/actuators/machine/reconciler.go @@ -11,7 +11,7 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" errorutil "k8s.io/apimachinery/pkg/util/errors" - "k8s.io/klog" + "k8s.io/klog/v2" awsproviderv1 "sigs.k8s.io/cluster-api-provider-aws/pkg/apis/awsprovider/v1beta1" ) diff --git a/pkg/actuators/machine/utils.go b/pkg/actuators/machine/utils.go index 76836f756f..6bf13a0d70 100644 --- a/pkg/actuators/machine/utils.go +++ b/pkg/actuators/machine/utils.go @@ -28,7 +28,7 @@ import ( "github.com/aws/aws-sdk-go/service/ec2" machinev1 "github.com/openshift/machine-api-operator/pkg/apis/machine/v1beta1" machinecontroller "github.com/openshift/machine-api-operator/pkg/controller/machine" - "k8s.io/klog" + "k8s.io/klog/v2" awsproviderv1 "sigs.k8s.io/cluster-api-provider-aws/pkg/apis/awsprovider/v1beta1" awsclient "sigs.k8s.io/cluster-api-provider-aws/pkg/client" ) diff --git a/pkg/apis/awsprovider/v1beta1/register.go b/pkg/apis/awsprovider/v1beta1/register.go index fb78966576..67107f8aa4 100644 --- a/pkg/apis/awsprovider/v1beta1/register.go +++ b/pkg/apis/awsprovider/v1beta1/register.go @@ -30,7 +30,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/klog" + "k8s.io/klog/v2" "sigs.k8s.io/controller-runtime/pkg/scheme" "sigs.k8s.io/yaml" ) diff --git a/vendor/k8s.io/klog/v2/klogr/README.md b/vendor/k8s.io/klog/v2/klogr/README.md new file mode 100644 index 0000000000..cfa5de617d --- /dev/null +++ b/vendor/k8s.io/klog/v2/klogr/README.md @@ -0,0 +1,8 @@ +# Minimal Go logging using klog + +This package implements the [logr interface](https://github.com/go-logr/logr) +in terms of Kubernetes' [klog](https://github.com/kubernetes/klog). This +provides a relatively minimalist API to logging in Go, backed by a well-proven +implementation. + +This is a BETA grade implementation. diff --git a/vendor/k8s.io/klog/v2/klogr/klogr.go b/vendor/k8s.io/klog/v2/klogr/klogr.go new file mode 100644 index 0000000000..227d07b2c3 --- /dev/null +++ b/vendor/k8s.io/klog/v2/klogr/klogr.go @@ -0,0 +1,201 @@ +// Package klogr implements github.com/go-logr/logr.Logger in terms of +// k8s.io/klog. +package klogr + +import ( + "bytes" + "encoding/json" + "fmt" + "runtime" + "sort" + "strings" + + "github.com/go-logr/logr" + "k8s.io/klog/v2" +) + +// New returns a logr.Logger which is implemented by klog. +func New() logr.Logger { + return klogger{ + level: 0, + prefix: "", + values: nil, + } +} + +type klogger struct { + level int + prefix string + values []interface{} +} + +func (l klogger) clone() klogger { + return klogger{ + level: l.level, + prefix: l.prefix, + values: copySlice(l.values), + } +} + +func copySlice(in []interface{}) []interface{} { + out := make([]interface{}, len(in)) + copy(out, in) + return out +} + +// Magic string for intermediate frames that we should ignore. +const autogeneratedFrameName = "" + +// Discover how many frames we need to climb to find the caller. This approach +// was suggested by Ian Lance Taylor of the Go team, so it *should* be safe +// enough (famous last words). +func framesToCaller() int { + // 1 is the immediate caller. 3 should be too many. + for i := 1; i < 3; i++ { + _, file, _, _ := runtime.Caller(i + 1) // +1 for this function's frame + if file != autogeneratedFrameName { + return i + } + } + return 1 // something went wrong, this is safe +} + +// trimDuplicates will deduplicates elements provided in multiple KV tuple +// slices, whilst maintaining the distinction between where the items are +// contained. +func trimDuplicates(kvLists ...[]interface{}) [][]interface{} { + // maintain a map of all seen keys + seenKeys := map[interface{}]struct{}{} + // build the same number of output slices as inputs + outs := make([][]interface{}, len(kvLists)) + // iterate over the input slices backwards, as 'later' kv specifications + // of the same key will take precedence over earlier ones + for i := len(kvLists) - 1; i >= 0; i-- { + // initialise this output slice + outs[i] = []interface{}{} + // obtain a reference to the kvList we are processing + kvList := kvLists[i] + + // start iterating at len(kvList) - 2 (i.e. the 2nd last item) for + // slices that have an even number of elements. + // We add (len(kvList) % 2) here to handle the case where there is an + // odd number of elements in a kvList. + // If there is an odd number, then the last element in the slice will + // have the value 'null'. + for i2 := len(kvList) - 2 + (len(kvList) % 2); i2 >= 0; i2 -= 2 { + k := kvList[i2] + // if we have already seen this key, do not include it again + if _, ok := seenKeys[k]; ok { + continue + } + // make a note that we've observed a new key + seenKeys[k] = struct{}{} + // attempt to obtain the value of the key + var v interface{} + // i2+1 should only ever be out of bounds if we handling the first + // iteration over a slice with an odd number of elements + if i2+1 < len(kvList) { + v = kvList[i2+1] + } + // add this KV tuple to the *start* of the output list to maintain + // the original order as we are iterating over the slice backwards + outs[i] = append([]interface{}{k, v}, outs[i]...) + } + } + return outs +} + +func flatten(kvList ...interface{}) string { + keys := make([]string, 0, len(kvList)) + vals := make(map[string]interface{}, len(kvList)) + for i := 0; i < len(kvList); i += 2 { + k, ok := kvList[i].(string) + if !ok { + panic(fmt.Sprintf("key is not a string: %s", pretty(kvList[i]))) + } + var v interface{} + if i+1 < len(kvList) { + v = kvList[i+1] + } + keys = append(keys, k) + vals[k] = v + } + sort.Strings(keys) + buf := bytes.Buffer{} + for i, k := range keys { + v := vals[k] + if i > 0 { + buf.WriteRune(' ') + } + buf.WriteString(pretty(k)) + buf.WriteString("=") + buf.WriteString(pretty(v)) + } + return buf.String() +} + +func pretty(value interface{}) string { + if err, ok := value.(error); ok { + if _, ok := value.(json.Marshaler); !ok { + value = err.Error() + } + } + buffer := &bytes.Buffer{} + encoder := json.NewEncoder(buffer) + encoder.SetEscapeHTML(false) + encoder.Encode(value) + return strings.TrimSpace(string(buffer.Bytes())) +} + +func (l klogger) Info(msg string, kvList ...interface{}) { + if l.Enabled() { + msgStr := flatten("msg", msg) + trimmed := trimDuplicates(l.values, kvList) + fixedStr := flatten(trimmed[0]...) + userStr := flatten(trimmed[1]...) + klog.InfoDepth(framesToCaller(), l.prefix, " ", msgStr, " ", fixedStr, " ", userStr) + } +} + +func (l klogger) Enabled() bool { + return bool(klog.V(klog.Level(l.level)).Enabled()) +} + +func (l klogger) Error(err error, msg string, kvList ...interface{}) { + msgStr := flatten("msg", msg) + var loggableErr interface{} + if err != nil { + loggableErr = err.Error() + } + errStr := flatten("error", loggableErr) + trimmed := trimDuplicates(l.values, kvList) + fixedStr := flatten(trimmed[0]...) + userStr := flatten(trimmed[1]...) + klog.ErrorDepth(framesToCaller(), l.prefix, " ", msgStr, " ", errStr, " ", fixedStr, " ", userStr) +} + +func (l klogger) V(level int) logr.Logger { + new := l.clone() + new.level = level + return new +} + +// WithName returns a new logr.Logger with the specified name appended. klogr +// uses '/' characters to separate name elements. Callers should not pass '/' +// in the provided name string, but this library does not actually enforce that. +func (l klogger) WithName(name string) logr.Logger { + new := l.clone() + if len(l.prefix) > 0 { + new.prefix = l.prefix + "/" + } + new.prefix += name + return new +} + +func (l klogger) WithValues(kvList ...interface{}) logr.Logger { + new := l.clone() + new.values = append(new.values, kvList...) + return new +} + +var _ logr.Logger = klogger{} diff --git a/vendor/modules.txt b/vendor/modules.txt index a69fdf3799..2f9b922d93 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -569,6 +569,7 @@ k8s.io/klog k8s.io/klog/klogr # k8s.io/klog/v2 v2.3.0 k8s.io/klog/v2 +k8s.io/klog/v2/klogr # k8s.io/kube-openapi v0.0.0-20200805222855-6aeccd4b50c6 k8s.io/kube-openapi/pkg/common k8s.io/kube-openapi/pkg/util/proto