Wire context-based logging in scopes (#214)

* Add logging interface (github.com/go-logr/logr) to cloudtest

Signed-off-by: Stephen Augustus <[email protected]>

* Update scope and machine scope to use logr.Logger

Signed-off-by: Stephen Augustus <[email protected]>

Author: justaugustus

Gopkg.lock

@@ -15,10 +15,12 @@ go_library(
         "//pkg/cloud/azure:go_default_library",
         "//vendor/github.com/Azure/go-autorest/autorest:go_default_library",
         "//vendor/github.com/Azure/go-autorest/autorest/azure/auth:go_default_library",
+        "//vendor/github.com/go-logr/logr:go_default_library",
         "//vendor/github.com/pkg/errors:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",
         "//vendor/k8s.io/klog:go_default_library",
+        "//vendor/k8s.io/klog/klogr:go_default_library",
         "//vendor/sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha1:go_default_library",
         "//vendor/sigs.k8s.io/cluster-api/pkg/client/clientset_generated/clientset/typed/cluster/v1alpha1:go_default_library",
         "//vendor/sigs.k8s.io/yaml:go_default_library",

pkg/cloud/azure/actuators/BUILD.bazel

@@ -17,6 +17,9 @@ limitations under the License.
 package actuators
 
 import (
+	"fmt"
+
+	"github.com/go-logr/logr"
 	"github.com/pkg/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -33,17 +36,23 @@ type MachineScopeParams struct {
 	Cluster *clusterv1.Cluster
 	Machine *clusterv1.Machine
 	Client  client.ClusterV1alpha1Interface
+	Logger  logr.Logger
 }
 
 // NewMachineScope creates a new MachineScope from the supplied parameters.
 // This is meant to be called for each machine actuator operation.
 func NewMachineScope(params MachineScopeParams) (*MachineScope, error) {
-	scope, err := NewScope(ScopeParams{AzureClients: params.AzureClients, Client: params.Client, Cluster: params.Cluster})
+	scope, err := NewScope(ScopeParams{
+		AzureClients: params.AzureClients,
+		Client:       params.Client,
+		Cluster:      params.Cluster,
+		Logger:       params.Logger,
+	})
 	if err != nil {
 		return nil, err
 	}
 
-	machineConfig, err := MachineConfigFromProviderSpec(params.Client, params.Machine.Spec.ProviderSpec)
+	machineConfig, err := MachineConfigFromProviderSpec(params.Client, params.Machine.Spec.ProviderSpec, scope.Logger)
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to get machine config")
 	}
@@ -137,43 +146,43 @@ func (m *MachineScope) Close() {
 }
 
 // MachineConfigFromProviderSpec tries to decode the JSON-encoded spec, falling back on getting a MachineClass if the value is absent.
-func MachineConfigFromProviderSpec(clusterClient client.MachineClassesGetter, providerConfig clusterv1.ProviderSpec) (*v1alpha1.AzureMachineProviderSpec, error) {
+func MachineConfigFromProviderSpec(clusterClient client.MachineClassesGetter, providerConfig clusterv1.ProviderSpec, log logr.Logger) (*v1alpha1.AzureMachineProviderSpec, error) {
 	var config v1alpha1.AzureMachineProviderSpec
 	if providerConfig.Value != nil {
-		klog.V(4).Info("Decoding ProviderConfig from Value")
-		return unmarshalProviderSpec(providerConfig.Value)
+		log.V(4).Info("Decoding ProviderConfig from Value")
+		return unmarshalProviderSpec(providerConfig.Value, log)
 	}
 
 	if providerConfig.ValueFrom != nil && providerConfig.ValueFrom.MachineClass != nil {
 		ref := providerConfig.ValueFrom.MachineClass
-		klog.V(4).Info("Decoding ProviderConfig from MachineClass")
-		klog.V(6).Infof("ref: %v", ref)
-		if ref.Provider != "" && ref.Provider != "azure" {
+		log.V(4).Info("Decoding ProviderConfig from MachineClass")
+		log.V(6).Info("Machine class reference", "ref", fmt.Sprintf("%+v", ref))
+		if ref.Provider != "" && ref.Provider != "aws" {
 			return nil, errors.Errorf("Unsupported provider: %q", ref.Provider)
 		}
 
 		if len(ref.Namespace) > 0 && len(ref.Name) > 0 {
-			klog.V(4).Infof("Getting MachineClass: %s/%s", ref.Namespace, ref.Name)
+			log.V(4).Info("Getting MachineClass", "reference-namespace", ref.Namespace, "reference-name", ref.Name)
 			mc, err := clusterClient.MachineClasses(ref.Namespace).Get(ref.Name, metav1.GetOptions{})
-			klog.V(6).Infof("Retrieved MachineClass: %+v", mc)
+			log.V(6).Info("Retrieved MachineClass", "machine-class", fmt.Sprintf("%+v", mc))
 			if err != nil {
 				return nil, err
 			}
 			providerConfig.Value = &mc.ProviderSpec
-			return unmarshalProviderSpec(&mc.ProviderSpec)
+			return unmarshalProviderSpec(&mc.ProviderSpec, log)
 		}
 	}
 
 	return &config, nil
 }
 
-func unmarshalProviderSpec(spec *runtime.RawExtension) (*v1alpha1.AzureMachineProviderSpec, error) {
+func unmarshalProviderSpec(spec *runtime.RawExtension, log logr.Logger) (*v1alpha1.AzureMachineProviderSpec, error) {
 	var config v1alpha1.AzureMachineProviderSpec
 	if spec != nil {
 		if err := yaml.Unmarshal(spec.Raw, &config); err != nil {
 			return nil, err
 		}
 	}
-	klog.V(6).Infof("Found ProviderSpec: %+v", config)
+	log.V(6).Info("Found ProviderSpec", "provider-spec", fmt.Sprintf("%+v", config))
 	return &config, nil
 }

pkg/cloud/azure/actuators/machine_scope.go

@@ -21,8 +21,10 @@ import (
 	"os"
 
 	"github.com/Azure/go-autorest/autorest/azure/auth"
+	"github.com/go-logr/logr"
 	"github.com/pkg/errors"
 	"k8s.io/klog"
+	"k8s.io/klog/klogr"
 	"sigs.k8s.io/cluster-api-provider-azure/pkg/apis/azureprovider/v1alpha1"
 	clusterv1 "sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha1"
 	client "sigs.k8s.io/cluster-api/pkg/client/clientset_generated/clientset/typed/cluster/v1alpha1"
@@ -33,6 +35,7 @@ type ScopeParams struct {
 	AzureClients
 	Cluster *clusterv1.Cluster
 	Client  client.ClusterV1alpha1Interface
+	Logger  logr.Logger
 }
 
 // NewScope creates a new Scope from the supplied parameters.
@@ -44,12 +47,12 @@ func NewScope(params ScopeParams) (*Scope, error) {
 
 	clusterConfig, err := v1alpha1.ClusterConfigFromProviderSpec(params.Cluster.Spec.ProviderSpec)
 	if err != nil {
-		return nil, errors.Wrap(err, "failed to load cluster provider config")
+		return nil, errors.Errorf("failed to load cluster provider config: %v", err)
 	}
 
 	clusterStatus, err := v1alpha1.ClusterStatusFromProviderStatus(params.Cluster.Status.ProviderStatus)
 	if err != nil {
-		return nil, errors.Wrap(err, "failed to load cluster provider status")
+		return nil, errors.Errorf("failed to load cluster provider status: %v", err)
 	}
 
 	authorizer, err := auth.NewAuthorizerFromEnvironment()
@@ -69,13 +72,18 @@ func NewScope(params ScopeParams) (*Scope, error) {
 		clusterClient = params.Client.Clusters(params.Cluster.Namespace)
 	}
 
+	if params.Logger == nil {
+		params.Logger = klogr.New().WithName("default-logger")
+	}
+
 	return &Scope{
 		AzureClients:  params.AzureClients,
 		Cluster:       params.Cluster,
 		ClusterClient: clusterClient,
 		ClusterConfig: clusterConfig,
 		ClusterStatus: clusterStatus,
 		Context:       context.Background(),
+		Logger:        params.Logger.WithName(params.Cluster.APIVersion).WithName(params.Cluster.Namespace).WithName(params.Cluster.Name),
 	}, nil
 }
 
@@ -87,6 +95,7 @@ type Scope struct {
 	ClusterConfig *v1alpha1.AzureClusterProviderSpec
 	ClusterStatus *v1alpha1.AzureClusterProviderStatus
 	Context       context.Context
+	logr.Logger
 }
 
 // Network returns the cluster network object.

pkg/cloud/azure/actuators/scope.go

@@ -5,5 +5,8 @@ go_library(
     srcs = ["cloudtest.go"],
     importpath = "sigs.k8s.io/cluster-api-provider-azure/pkg/cloudtest",
     visibility = ["//visibility:public"],
-    deps = ["//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library"],
+    deps = [
+        "//vendor/github.com/go-logr/logr:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",
+    ],
 )

pkg/cloudtest/BUILD.bazel

@@ -20,6 +20,7 @@ import (
 	"encoding/json"
 	"testing"
 
+	"github.com/go-logr/logr"
 	"k8s.io/apimachinery/pkg/runtime"
 )
 
@@ -36,3 +37,14 @@ func RuntimeRawExtension(t *testing.T, p interface{}) *runtime.RawExtension {
 		Raw: out,
 	}
 }
+
+// Log implements logr.Logger for testing. Do not use if you actually want to
+// test log messages.
+type Log struct{}
+
+func (l *Log) Error(err error, msg string, keysAndValues ...interface{}) {}
+func (l *Log) V(level int) logr.InfoLogger                               { return l }
+func (l *Log) WithValues(keysAndValues ...interface{}) logr.Logger       { return l }
+func (l *Log) WithName(name string) logr.Logger                          { return l }
+func (l *Log) Info(msg string, keysAndValues ...interface{})             {}
+func (l *Log) Enabled() bool                                             { return false }