Rewrite glogx to work with klogv2 (+rename klogx)

Author: MaciekPytel

cluster-autoscaler/cloudprovider/gce/autoscaling_gce_client.go

@@ -27,7 +27,7 @@ import (
 	"time"
 
 	"k8s.io/autoscaler/cluster-autoscaler/cloudprovider"
-	"k8s.io/autoscaler/cluster-autoscaler/utils/glogx"
+	"k8s.io/autoscaler/cluster-autoscaler/utils/klogx"
 
 	gce "google.golang.org/api/compute/v1"
 	klog "k8s.io/klog/v2"
@@ -212,7 +212,7 @@ func (client *autoscalingGceClientV1) FetchMigInstances(migRef GceRef) ([]cloudp
 	}
 	infos := []cloudprovider.Instance{}
 	errorCodeCounts := make(map[string]int)
-	errorLoggingQuota := glogx.NewLoggingQuota(100)
+	errorLoggingQuota := klogx.NewLoggingQuota(100)
 	for _, gceInstance := range gceInstances.ManagedInstances {
 		ref, err := ParseInstanceUrlRef(gceInstance.Instance)
 		if err != nil {
@@ -274,12 +274,12 @@ func (client *autoscalingGceClientV1) FetchMigInstances(migRef GceRef) ([]cloudp
 				} else {
 					gceInstanceJSON = string(gceInstanceJSONBytes)
 				}
-				glogx.V(4).UpTo(errorLoggingQuota).Infof("Got GCE instance which is being created and has lastAttemptErrors; gceInstance=%v; errorInfo=%#v", gceInstanceJSON, errorInfo)
+				klogx.V(4).UpTo(errorLoggingQuota).Infof("Got GCE instance which is being created and has lastAttemptErrors; gceInstance=%v; errorInfo=%#v", gceInstanceJSON, errorInfo)
 			}
 		}
 		infos = append(infos, instance)
 	}
-	glogx.V(4).Over(errorLoggingQuota).Infof("Got %v other GCE instances being created with lastAttemptErrors", -errorLoggingQuota.Left())
+	klogx.V(4).Over(errorLoggingQuota).Infof("Got %v other GCE instances being created with lastAttemptErrors", -errorLoggingQuota.Left())
 	if len(errorCodeCounts) > 0 {
 		klog.V(4).Infof("Spotted following instance creation error codes: %#v", errorCodeCounts)
 	}

cluster-autoscaler/core/scale_up.go

@@ -39,7 +39,7 @@ import (
 	"k8s.io/autoscaler/cluster-autoscaler/processors/nodegroupset"
 	"k8s.io/autoscaler/cluster-autoscaler/processors/status"
 	"k8s.io/autoscaler/cluster-autoscaler/utils/errors"
-	"k8s.io/autoscaler/cluster-autoscaler/utils/glogx"
+	"k8s.io/autoscaler/cluster-autoscaler/utils/klogx"
 	"k8s.io/autoscaler/cluster-autoscaler/utils/gpu"
 	schedulerframework "k8s.io/kubernetes/pkg/scheduler/framework/v1alpha1"
 
@@ -320,12 +320,12 @@ func ScaleUp(context *context.AutoscalingContext, processors *ca_processors.Auto
 
 	now := time.Now()
 
-	loggingQuota := glogx.PodsLoggingQuota()
+	loggingQuota := klogx.PodsLoggingQuota()
 
 	for _, pod := range unschedulablePods {
-		glogx.V(1).UpTo(loggingQuota).Infof("Pod %s/%s is unschedulable", pod.Namespace, pod.Name)
+		klogx.V(1).UpTo(loggingQuota).Infof("Pod %s/%s is unschedulable", pod.Namespace, pod.Name)
 	}
-	glogx.V(1).Over(loggingQuota).Infof("%v other pods are also unschedulable", -loggingQuota.Left())
+	klogx.V(1).Over(loggingQuota).Infof("%v other pods are also unschedulable", -loggingQuota.Left())
 
 	nodesFromNotAutoscaledGroups, err := utils.FilterOutNodesFromNotAutoscaledGroups(nodes, context.CloudProvider)
 	if err != nil {

cluster-autoscaler/simulator/cluster.go

@@ -23,7 +23,7 @@ import (
 
 	"k8s.io/autoscaler/cluster-autoscaler/utils/drain"
 	"k8s.io/autoscaler/cluster-autoscaler/utils/errors"
-	"k8s.io/autoscaler/cluster-autoscaler/utils/glogx"
+	"k8s.io/autoscaler/cluster-autoscaler/utils/klogx"
 	"k8s.io/autoscaler/cluster-autoscaler/utils/gpu"
 	kube_util "k8s.io/autoscaler/cluster-autoscaler/utils/kubernetes"
 	pod_util "k8s.io/autoscaler/cluster-autoscaler/utils/pod"
@@ -295,11 +295,11 @@ func findPlaceFor(removedNode string, pods []*apiv1.Pod, nodes map[string]bool,
 		return fmt.Sprintf("%s/%s", pod.Namespace, pod.Name)
 	}
 
-	loggingQuota := glogx.PodsLoggingQuota()
+	loggingQuota := klogx.PodsLoggingQuota()
 
 	tryNodeForPod := func(nodename string, pod *apiv1.Pod) bool {
 		if err := predicateChecker.CheckPredicates(clusterSnapshot, pod, nodename); err != nil {
-			glogx.V(4).UpTo(loggingQuota).Infof("Evaluation %s for %s/%s -> %v", nodename, pod.Namespace, pod.Name, err.VerboseMessage())
+			klogx.V(4).UpTo(loggingQuota).Infof("Evaluation %s for %s/%s -> %v", nodename, pod.Namespace, pod.Name, err.VerboseMessage())
 			return false
 		}
 
@@ -353,7 +353,7 @@ func findPlaceFor(removedNode string, pods []*apiv1.Pod, nodes map[string]bool,
 				}
 			}
 			if !foundPlace {
-				glogx.V(4).Over(loggingQuota).Infof("%v other nodes evaluated for %s/%s", -loggingQuota.Left(), pod.Namespace, pod.Name)
+				klogx.V(4).Over(loggingQuota).Infof("%v other nodes evaluated for %s/%s", -loggingQuota.Left(), pod.Namespace, pod.Name)
 				return fmt.Errorf("failed to find place for %s", podKey(pod))
 			}
 		}

cluster-autoscaler/utils/glogx/defaults.go renamed to cluster-autoscaler/utils/klogx/defaults.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package glogx
+package klogx
 
 import klog "k8s.io/klog/v2"
 
@@ -29,7 +29,7 @@ const (
 
 // PodsLoggingQuota returns a new quota with default limit for pods at current verbosity.
 func PodsLoggingQuota() *quota {
-	if klog.V(5) {
+	if klog.V(5).Enabled() {
 		return NewLoggingQuota(MaxPodsLoggedV5)
 	}
 	return NewLoggingQuota(MaxPodsLogged)

cluster-autoscaler/utils/glogx/glogx.go renamed to cluster-autoscaler/utils/klogx/klogx.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package glogx
+package klogx
 
 import (
 	klog "k8s.io/klog/v2"
@@ -40,41 +40,57 @@ func (q *quota) Reset() {
 	q.left = q.limit
 }
 
-// UpTo decreases quota for logging and reports whether there was any left.
-// The returned value is a boolean of type glogx.Verbose.
-func UpTo(quota *quota) klog.Verbose {
-	quota.left--
-	return quota.left >= 0
-}
-
-// Over reports whether quota for logging was exceeded.
-// The returned value is a boolean of type glogx.Verbose.
-func Over(quota *quota) klog.Verbose {
-	return quota.left < 0
-}
-
 // V calls V from glog and wraps the result into glogx.Verbose.
 func V(n klog.Level) Verbose {
-	return Verbose(klog.V(n))
+	return Verbose{
+		enabled: true,
+		v:       klog.V(n)}
 }
 
 // Verbose is a wrapper for klog.Verbose that implements UpTo and Over.
-type Verbose klog.Verbose
+// It provides a subset of methods exposed by klog.Verbose.
+type Verbose struct {
+	enabled bool
+	v       klog.Verbose
+}
+
+func (v Verbose) enable(b bool) Verbose {
+	return Verbose{
+		enabled: b,
+		v:       v.v}
+}
 
 // UpTo calls UpTo from this package if called on true object.
-// The returned value is a boolean of type klog.Verbose.
-func (v Verbose) UpTo(quota *quota) klog.Verbose {
-	if v {
-		return UpTo(quota)
+// The returned value is of type Verbose.
+func (v Verbose) UpTo(quota *quota) Verbose {
+	if v.v.Enabled() {
+		quota.left--
+		return v.enable(quota.left >= 0)
 	}
-	return klog.Verbose(false)
+	return v.enable(false)
 }
 
 // Over calls Over from this package if called on true object.
-// The returned value is a boolean of type klog.Verbose.
-func (v Verbose) Over(quota *quota) klog.Verbose {
-	if v {
-		return Over(quota)
+// The returned value is of type Verbose.
+func (v Verbose) Over(quota *quota) Verbose {
+	if v.v.Enabled() {
+		return v.enable(quota.left < 0)
+	}
+	return v.enable(false)
+}
+
+// Infof is a wrapper for klog.Infof that logs if the quota
+// allows for it.
+func (v Verbose) Infof(format string, args ...interface{}) {
+	if v.enabled {
+		v.v.Infof(format, args...)
+	}
+}
+
+// Info is a wrapper for klog.Info that logs if the quota
+// allows for it.
+func (v Verbose) Info(args ...interface{}) {
+	if v.enabled {
+		v.v.Info(args...)
 	}
-	return klog.Verbose(false)
 }

cluster-autoscaler/utils/glogx/glogx_test.go renamed to cluster-autoscaler/utils/klogx/klogx_test.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package glogx
+package klogx
 
 import (
 	"testing"
@@ -29,8 +29,8 @@ func TestLoggingQuota(t *testing.T) {
 
 	for i := 0; i < 5; i++ {
 		assert.Equal(t, 3-i, q.Left())
-		assert.Equal(t, i < 3, bool(UpTo(q)))
-		assert.Equal(t, i >= 3, bool(Over(q)))
+		assert.Equal(t, i < 3, V(0).UpTo(q).enabled)
+		assert.Equal(t, i >= 3, V(0).Over(q).enabled)
 	}
 }
 
@@ -39,28 +39,30 @@ func TestReset(t *testing.T) {
 	q := NewLoggingQuota(3)
 
 	for i := 0; i < 5; i++ {
-		assert.Equal(t, i < 3, bool(UpTo(q)))
+		assert.Equal(t, i < 3, V(0).UpTo(q).enabled)
 	}
 
 	q.Reset()
 
 	assert.Equal(t, 3, q.Left())
-	assert.False(t, bool(Over(q)))
-	assert.True(t, bool(UpTo(q)))
+	assert.False(t, V(0).Over(q).enabled)
+	assert.True(t, V(0).UpTo(q).enabled)
 }
 
 // Tests that quota isn't used up by calls limited by verbosity.
 func TestVFalse(t *testing.T) {
-	v := Verbose(false)
+	// XXX: this is a hack to get a disabled V, since klog v2 no longer
+	// provides an easy way to create it
+	v := V(10000)
 	q := NewLoggingQuota(3)
 
-	assert.False(t, bool(v.UpTo(q)))
+	assert.False(t, v.UpTo(q).v.Enabled())
 	assert.Equal(t, 3, q.Left())
 }
 
 // Tests that V limits calls based on verbosity the same way as klog.V.
 func TestV(t *testing.T) {
 	for i := klog.Level(0); i <= 10; i++ {
-		assert.Equal(t, bool(klog.V(i)), bool(V(i)))
+		assert.Equal(t, klog.V(i).Enabled(), V(i).v.Enabled())
 	}
 }