From 6055f03c1d52938fc4e5f85ee41b65528508f7f2 Mon Sep 17 00:00:00 2001 From: Chuck Ha Date: Thu, 11 Apr 2019 10:05:08 -0400 Subject: [PATCH] Adds logr as dependency Adds context for logs and removes excessive logging Signed-off-by: Chuck Ha --- cmd/manager/main.go | 5 +- pkg/cloud/aws/actuators/BUILD.bazel | 3 +- pkg/cloud/aws/actuators/cluster/BUILD.bazel | 1 - pkg/cloud/aws/actuators/cluster/actuator.go | 15 ++- pkg/cloud/aws/actuators/machine/BUILD.bazel | 3 +- pkg/cloud/aws/actuators/machine/actuator.go | 48 ++++---- pkg/cloud/aws/actuators/machine_scope.go | 39 +++--- pkg/cloud/aws/actuators/scope.go | 14 ++- pkg/cloud/aws/services/certificates/BUILD | 1 - .../aws/services/certificates/certificates.go | 112 +++++++++--------- .../certificates/certificates_test.go | 8 +- pkg/cloud/aws/services/ec2/BUILD | 1 - pkg/cloud/aws/services/ec2/ami.go | 7 +- pkg/cloud/aws/services/ec2/bastion.go | 15 ++- pkg/cloud/aws/services/ec2/eips.go | 3 +- pkg/cloud/aws/services/ec2/gateways.go | 15 ++- pkg/cloud/aws/services/ec2/instances.go | 44 +++---- pkg/cloud/aws/services/ec2/natgateways.go | 21 ++-- pkg/cloud/aws/services/ec2/network.go | 12 +- pkg/cloud/aws/services/ec2/routetables.go | 15 ++- pkg/cloud/aws/services/ec2/securitygroups.go | 13 +- pkg/cloud/aws/services/ec2/subnets.go | 16 +-- pkg/cloud/aws/services/ec2/vpc.go | 15 ++- pkg/cloud/aws/services/elb/BUILD.bazel | 1 - pkg/cloud/aws/services/elb/loadbalancer.go | 15 ++- pkg/cloud/aws/services/kubeadm/BUILD.bazel | 2 +- .../aws/services/kubeadm/aws_defaults.go | 33 ++++-- .../aws/services/kubeadm/aws_defaults_test.go | 18 ++- pkg/cloud/aws/services/kubeadm/scheme.go | 3 +- pkg/cloudtest/BUILD | 5 +- pkg/cloudtest/cloudtest.go | 12 ++ test/e2e/BUILD | 1 + test/e2e/aws_test.go | 7 +- 33 files changed, 289 insertions(+), 234 deletions(-) diff --git a/cmd/manager/main.go b/cmd/manager/main.go index 9380bd3159..56e9cc49b0 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -71,12 +71,13 @@ func main() { // Initialize cluster actuator. clusterActuator := cluster.NewActuator(cluster.ActuatorParams{ Client: cs.ClusterV1alpha1(), - LoggingContext: "[cluster actuator]", + LoggingContext: "[cluster-actuator]", }) // Initialize machine actuator. machineActuator := machine.NewActuator(machine.ActuatorParams{ - Client: cs.ClusterV1alpha1(), + Client: cs.ClusterV1alpha1(), + LoggingContext: "[machine-actuator]", }) // Register our cluster deployer (the interface is in clusterctl and we define the Deployer interface on the actuator) diff --git a/pkg/cloud/aws/actuators/BUILD.bazel b/pkg/cloud/aws/actuators/BUILD.bazel index 270aecaa24..dab67a7b56 100644 --- a/pkg/cloud/aws/actuators/BUILD.bazel +++ b/pkg/cloud/aws/actuators/BUILD.bazel @@ -18,10 +18,11 @@ go_library( "//vendor/github.com/aws/aws-sdk-go/service/ec2/ec2iface:go_default_library", "//vendor/github.com/aws/aws-sdk-go/service/elb:go_default_library", "//vendor/github.com/aws/aws-sdk-go/service/elb/elbiface: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", diff --git a/pkg/cloud/aws/actuators/cluster/BUILD.bazel b/pkg/cloud/aws/actuators/cluster/BUILD.bazel index 78d7763874..413d01d7a4 100644 --- a/pkg/cloud/aws/actuators/cluster/BUILD.bazel +++ b/pkg/cloud/aws/actuators/cluster/BUILD.bazel @@ -13,7 +13,6 @@ go_library( "//pkg/deployer:go_default_library", "//vendor/github.com/go-logr/logr:go_default_library", "//vendor/github.com/pkg/errors: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", diff --git a/pkg/cloud/aws/actuators/cluster/actuator.go b/pkg/cloud/aws/actuators/cluster/actuator.go index 956bf749aa..dde799b8c6 100644 --- a/pkg/cloud/aws/actuators/cluster/actuator.go +++ b/pkg/cloud/aws/actuators/cluster/actuator.go @@ -19,7 +19,6 @@ package cluster import ( "github.com/go-logr/logr" "github.com/pkg/errors" - "k8s.io/klog" "k8s.io/klog/klogr" "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/aws/actuators" "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/aws/services/certificates" @@ -59,9 +58,9 @@ func NewActuator(params ActuatorParams) *Actuator { // Reconcile reconciles a cluster and is invoked by the Cluster Controller func (a *Actuator) Reconcile(cluster *clusterv1.Cluster) error { - klog.Infof("Reconciling cluster %v", cluster.Name) + a.log.Info("Reconciling Cluster", "cluster-name", cluster.Name, "cluster-namespace", cluster.Namespace) - scope, err := actuators.NewScope(actuators.ScopeParams{Cluster: cluster, Client: a.client}) + scope, err := actuators.NewScope(actuators.ScopeParams{Cluster: cluster, Client: a.client, Logger: a.log}) if err != nil { return errors.Errorf("failed to create scope: %+v", err) } @@ -94,9 +93,13 @@ func (a *Actuator) Reconcile(cluster *clusterv1.Cluster) error { // Delete deletes a cluster and is invoked by the Cluster Controller func (a *Actuator) Delete(cluster *clusterv1.Cluster) error { - klog.Infof("Deleting cluster %v.", cluster.Name) + a.log.Info("Deleting cluster", "cluster-name", cluster.Name, "cluster-namespace", cluster.Namespace) - scope, err := actuators.NewScope(actuators.ScopeParams{Cluster: cluster, Client: a.client}) + scope, err := actuators.NewScope(actuators.ScopeParams{ + Cluster: cluster, + Client: a.client, + Logger: a.log, + }) if err != nil { return errors.Errorf("failed to create scope: %+v", err) } @@ -115,7 +118,7 @@ func (a *Actuator) Delete(cluster *clusterv1.Cluster) error { } if err := ec2svc.DeleteNetwork(); err != nil { - klog.Errorf("Error deleting cluster %v: %v.", cluster.Name, err) + a.log.Error(err, "Error deleting cluster", "cluster-name", cluster.Name, "cluster-namespace", cluster.Namespace) return &controllerError.RequeueAfterError{ RequeueAfter: 5 * 1000 * 1000 * 1000, } diff --git a/pkg/cloud/aws/actuators/machine/BUILD.bazel b/pkg/cloud/aws/actuators/machine/BUILD.bazel index ca0f0b4fa2..a6ba74dea1 100644 --- a/pkg/cloud/aws/actuators/machine/BUILD.bazel +++ b/pkg/cloud/aws/actuators/machine/BUILD.bazel @@ -20,13 +20,14 @@ go_library( "//pkg/deployer:go_default_library", "//pkg/tokens:go_default_library", "//vendor/github.com/aws/aws-sdk-go/aws:go_default_library", + "//vendor/github.com/go-logr/logr:go_default_library", "//vendor/github.com/pkg/errors:go_default_library", "//vendor/k8s.io/api/core/v1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//vendor/k8s.io/client-go/kubernetes/typed/core/v1:go_default_library", "//vendor/k8s.io/client-go/tools/clientcmd:go_default_library", "//vendor/k8s.io/client-go/tools/clientcmd/api: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/cluster-api/pkg/controller/error:go_default_library", diff --git a/pkg/cloud/aws/actuators/machine/actuator.go b/pkg/cloud/aws/actuators/machine/actuator.go index 8d634304e0..55ab86c756 100644 --- a/pkg/cloud/aws/actuators/machine/actuator.go +++ b/pkg/cloud/aws/actuators/machine/actuator.go @@ -24,6 +24,7 @@ import ( "time" "github.com/aws/aws-sdk-go/aws" + "github.com/go-logr/logr" "github.com/pkg/errors" apicorev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -31,7 +32,7 @@ import ( corev1 "k8s.io/client-go/kubernetes/typed/core/v1" "k8s.io/client-go/tools/clientcmd" clientcmdapi "k8s.io/client-go/tools/clientcmd/api" - "k8s.io/klog" + "k8s.io/klog/klogr" "sigs.k8s.io/cluster-api-provider-aws/pkg/apis/awsprovider/v1alpha1" "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/aws/actuators" "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/aws/services/awserrors" @@ -58,11 +59,13 @@ type Actuator struct { *deployer.Deployer client client.ClusterV1alpha1Interface + log logr.Logger } // ActuatorParams holds parameter information for Actuator. type ActuatorParams struct { - Client client.ClusterV1alpha1Interface + Client client.ClusterV1alpha1Interface + LoggingContext string } // NewActuator returns an actuator. @@ -70,6 +73,7 @@ func NewActuator(params ActuatorParams) *Actuator { return &Actuator{ Deployer: deployer.New(deployer.Params{ScopeGetter: actuators.DefaultScopeGetter}), client: params.Client, + log: klogr.New().WithName(params.LoggingContext), } } @@ -99,6 +103,7 @@ func (a *Actuator) isNodeJoin(scope *actuators.MachineScope, controlPlaneMachine Machine: cm, Cluster: scope.Cluster, Client: a.client, + Logger: a.log, }) if err != nil { @@ -112,7 +117,7 @@ func (a *Actuator) isNodeJoin(scope *actuators.MachineScope, controlPlaneMachine return false, errors.Wrapf(err, "failed to verify existence of machine %q", m.Name()) } - klog.V(2).Infof("Machine %q should join the controlplane: %t", scope.Machine.Name, ok) + a.log.V(2).Info("Machine joining control plane", "machine-name", scope.Machine.Name, "machine-namespace", scope.Machine.Name, "should-join-control-plane", ok) return ok, nil } @@ -127,9 +132,9 @@ func (a *Actuator) Create(ctx context.Context, cluster *clusterv1.Cluster, machi if cluster == nil { return errors.Errorf("missing cluster for machine %s/%s", machine.Namespace, machine.Name) } - klog.Infof("Creating machine %v for cluster %v", machine.Name, cluster.Name) + a.log.Info("Creating machine in cluster", "machine-name", machine.Name, "machine-namespace", machine.Namespace, "cluster-name", cluster.Name) - scope, err := actuators.NewMachineScope(actuators.MachineScopeParams{Machine: machine, Cluster: cluster, Client: a.client}) + scope, err := actuators.NewMachineScope(actuators.MachineScopeParams{Machine: machine, Cluster: cluster, Client: a.client, Logger: a.log}) if err != nil { return errors.Errorf("failed to create scope: %+v", err) } @@ -171,7 +176,7 @@ func (a *Actuator) Create(ctx context.Context, cluster *clusterv1.Cluster, machi i, err := ec2svc.CreateOrGetMachine(scope, bootstrapToken, kubeConfig) if err != nil { if awserrors.IsFailedDependency(errors.Cause(err)) { - klog.Errorf("network not ready to launch instances yet: %+v", err) + a.log.Error(err, "network not ready to launch instances yet") return &controllerError.RequeueAfterError{ RequeueAfter: time.Minute, } @@ -192,7 +197,7 @@ func (a *Actuator) Create(ctx context.Context, cluster *clusterv1.Cluster, machi if err := a.reconcileLBAttachment(scope, machine, i); err != nil { return errors.Errorf("failed to reconcile LB attachment: %+v", err) } - + a.log.Info("Create completed", "machine-name", machine.Name) return nil } @@ -236,9 +241,9 @@ func (a *Actuator) Delete(ctx context.Context, cluster *clusterv1.Cluster, machi if cluster == nil { return errors.Errorf("missing cluster for machine %s/%s", machine.Namespace, machine.Name) } - klog.Infof("Deleting machine %v for cluster %v.", machine.Name, cluster.Name) + a.log.Info("Deleting machine in cluster", "machine-name", machine.Name, "machine-namespace", machine.Namespace, "cluster-name", cluster.Name) - scope, err := actuators.NewMachineScope(actuators.MachineScopeParams{Machine: machine, Cluster: cluster, Client: a.client}) + scope, err := actuators.NewMachineScope(actuators.MachineScopeParams{Machine: machine, Cluster: cluster, Client: a.client, Logger: a.log}) if err != nil { return errors.Errorf("failed to create scope: %+v", err) } @@ -258,7 +263,7 @@ func (a *Actuator) Delete(ctx context.Context, cluster *clusterv1.Cluster, machi return errors.Errorf("failed to query instance by tags: %+v", err) } else if instance == nil { // The machine hasn't been created yet - klog.V(3).Info("Instance is nil and therefore does not exist") + a.log.V(3).Info("Instance is nil and therefore does not exist") return nil } } @@ -269,7 +274,7 @@ func (a *Actuator) Delete(ctx context.Context, cluster *clusterv1.Cluster, machi // https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/ec2-instance-lifecycle.html switch instance.State { case v1alpha1.InstanceStateShuttingDown, v1alpha1.InstanceStateTerminated: - klog.Infof("instance %q is shutting down or already terminated", machine.Name) + a.log.Info("Machine instance is shutting down or already terminated", "machine-name", machine.Name, "machine-namespace", machine.Namespace) return nil default: if err := ec2svc.TerminateInstance(instance.ID); err != nil { @@ -277,7 +282,7 @@ func (a *Actuator) Delete(ctx context.Context, cluster *clusterv1.Cluster, machi } } - klog.Info("shutdown signal was sent. Shutting down machine.") + a.log.Info("Shutdown signal was sent. Shutting down machine.") return nil } @@ -337,9 +342,9 @@ func (a *Actuator) Update(ctx context.Context, cluster *clusterv1.Cluster, machi return errors.Errorf("missing cluster for machine %s/%s", machine.Namespace, machine.Name) } - klog.Infof("Updating machine %v for cluster %v.", machine.Name, cluster.Name) + a.log.Info("Updating machine in cluster", "machine-name", machine.Name, "machine-namespace", machine.Namespace, "cluster-name", cluster.Name) - scope, err := actuators.NewMachineScope(actuators.MachineScopeParams{Machine: machine, Cluster: cluster, Client: a.client}) + scope, err := actuators.NewMachineScope(actuators.MachineScopeParams{Machine: machine, Cluster: cluster, Client: a.client, Logger: a.log}) if err != nil { return errors.Errorf("failed to create scope: %+v", err) } @@ -388,9 +393,9 @@ func (a *Actuator) Exists(ctx context.Context, cluster *clusterv1.Cluster, machi return false, errors.Errorf("missing cluster for machine %s/%s", machine.Namespace, machine.Name) } - klog.Infof("Checking if machine %v for cluster %v exists", machine.Name, cluster.Name) + a.log.Info("Checking if machine exists in cluster", "machine-name", machine.Name, "machine-namespace", machine.Namespace, "cluster-name", cluster.Name) - scope, err := actuators.NewMachineScope(actuators.MachineScopeParams{Machine: machine, Cluster: cluster, Client: a.client}) + scope, err := actuators.NewMachineScope(actuators.MachineScopeParams{Machine: machine, Cluster: cluster, Client: a.client, Logger: a.log}) if err != nil { return false, errors.Errorf("failed to create scope: %+v", err) } @@ -413,13 +418,13 @@ func (a *Actuator) Exists(ctx context.Context, cluster *clusterv1.Cluster, machi return false, nil } - klog.Infof("Found instance for machine %q: %v", machine.Name, instance) + a.log.Info("Found instance for machine", "machine-name", machine.Name, "machine-namespace", machine.Namespace, "instance", instance) switch instance.State { case v1alpha1.InstanceStateRunning: - klog.Infof("Machine %v is running", *scope.MachineStatus.InstanceID) + a.log.Info("Machine instance is running", "instance-id", *scope.MachineStatus.InstanceID) case v1alpha1.InstanceStatePending: - klog.Infof("Machine %v is pending", *scope.MachineStatus.InstanceID) + a.log.Info("Machine instance is pending", "instance-id", *scope.MachineStatus.InstanceID) default: return false, nil } @@ -442,12 +447,13 @@ func (a *Actuator) Exists(ctx context.Context, cluster *clusterv1.Cluster, machi if machine.Status.NodeRef == nil { nodeRef, err := a.getNodeReference(scope) if err != nil { - klog.Warningf("Failed to set nodeRef: %v", err) + // non critical error + a.log.Info("Failed to set nodeRef", "error", err) return true, nil } scope.Machine.Status.NodeRef = nodeRef - klog.Infof("Setting machine %q nodeRef to %q", scope.Name(), nodeRef.Name) + a.log.Info("Setting machine's nodeRef", "machine-name", scope.Name(), "machine-namespace", scope.Namespace(), "nodeRef", nodeRef.Name) } return true, nil diff --git a/pkg/cloud/aws/actuators/machine_scope.go b/pkg/cloud/aws/actuators/machine_scope.go index cbceac18e5..7ea9f908f6 100644 --- a/pkg/cloud/aws/actuators/machine_scope.go +++ b/pkg/cloud/aws/actuators/machine_scope.go @@ -17,10 +17,12 @@ 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" - "k8s.io/klog" "sigs.k8s.io/cluster-api-provider-aws/pkg/apis/awsprovider/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,17 +35,22 @@ 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{AWSClients: params.AWSClients, Client: params.Client, Cluster: params.Cluster}) + scope, err := NewScope(ScopeParams{ + AWSClients: params.AWSClients, + 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") } @@ -57,7 +64,7 @@ func NewMachineScope(params MachineScopeParams) (*MachineScope, error) { if params.Client != nil { machineClient = params.Client.Machines(params.Machine.Namespace) } - + scope.Logger = scope.Logger.WithName(params.Machine.Name) return &MachineScope{ Scope: scope, Machine: params.Machine, @@ -136,54 +143,54 @@ func (m *MachineScope) Close() { latestMachine, err := m.storeMachineSpec(m.Machine) if err != nil { - klog.Errorf("[machinescope] failed to update machine %q in namespace %q: %v", m.Machine.Name, m.Machine.Namespace, err) + m.Error(err, "failed to update machine") return } _, err = m.storeMachineStatus(latestMachine) if err != nil { - klog.Errorf("[machinescope] failed to store provider status for machine %q in namespace %q: %v", m.Machine.Name, m.Machine.Namespace, err) + m.Error(err, "failed to store machine provider status") } } // 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.AWSMachineProviderSpec, error) { +func MachineConfigFromProviderSpec(clusterClient client.MachineClassesGetter, providerConfig clusterv1.ProviderSpec, log logr.Logger) (*v1alpha1.AWSMachineProviderSpec, error) { var config v1alpha1.AWSMachineProviderSpec 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) + 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.AWSMachineProviderSpec, error) { +func unmarshalProviderSpec(spec *runtime.RawExtension, log logr.Logger) (*v1alpha1.AWSMachineProviderSpec, error) { var config v1alpha1.AWSMachineProviderSpec 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 } diff --git a/pkg/cloud/aws/actuators/scope.go b/pkg/cloud/aws/actuators/scope.go index 2447962117..6c06c0f140 100644 --- a/pkg/cloud/aws/actuators/scope.go +++ b/pkg/cloud/aws/actuators/scope.go @@ -21,8 +21,9 @@ import ( "github.com/aws/aws-sdk-go/aws/session" "github.com/aws/aws-sdk-go/service/ec2" "github.com/aws/aws-sdk-go/service/elb" + "github.com/go-logr/logr" "github.com/pkg/errors" - "k8s.io/klog" + "k8s.io/klog/klogr" "sigs.k8s.io/cluster-api-provider-aws/pkg/apis/awsprovider/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 +34,7 @@ type ScopeParams struct { AWSClients Cluster *clusterv1.Cluster Client client.ClusterV1alpha1Interface + Logger logr.Logger } // NewScope creates a new Scope from the supplied parameters. @@ -70,12 +72,17 @@ 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{ AWSClients: params.AWSClients, Cluster: params.Cluster, ClusterClient: clusterClient, ClusterConfig: clusterConfig, ClusterStatus: clusterStatus, + Logger: params.Logger.WithName(params.Cluster.APIVersion).WithName(params.Cluster.Namespace).WithName(params.Cluster.Name), }, nil } @@ -86,6 +93,7 @@ type Scope struct { ClusterClient client.ClusterInterface ClusterConfig *v1alpha1.AWSClusterProviderSpec ClusterStatus *v1alpha1.AWSClusterProviderStatus + logr.Logger } // Network returns the cluster network object. @@ -151,12 +159,12 @@ func (s *Scope) Close() { latestCluster, err := s.storeClusterConfig(s.Cluster) if err != nil { - klog.Errorf("[scope] failed to store provider config for cluster %q in namespace %q: %v", s.Cluster.Name, s.Cluster.Namespace, err) + s.Error(err, "failed to store provider config") return } _, err = s.storeClusterStatus(latestCluster) if err != nil { - klog.Errorf("[scope] failed to store provider status for cluster %q in namespace %q: %v", s.Cluster.Name, s.Cluster.Namespace, err) + s.Error(err, "failed to store provider status") } } diff --git a/pkg/cloud/aws/services/certificates/BUILD b/pkg/cloud/aws/services/certificates/BUILD index 964be62dd9..6dc36a93a2 100644 --- a/pkg/cloud/aws/services/certificates/BUILD +++ b/pkg/cloud/aws/services/certificates/BUILD @@ -13,7 +13,6 @@ go_library( "//pkg/cloud/aws/actuators:go_default_library", "//vendor/github.com/pkg/errors:go_default_library", "//vendor/k8s.io/client-go/tools/clientcmd/api:go_default_library", - "//vendor/k8s.io/klog:go_default_library", ], ) diff --git a/pkg/cloud/aws/services/certificates/certificates.go b/pkg/cloud/aws/services/certificates/certificates.go index f78918515e..d238270c8b 100644 --- a/pkg/cloud/aws/services/certificates/certificates.go +++ b/pkg/cloud/aws/services/certificates/certificates.go @@ -33,7 +33,6 @@ import ( "github.com/pkg/errors" "k8s.io/client-go/tools/clientcmd/api" - "k8s.io/klog" "sigs.k8s.io/cluster-api-provider-aws/pkg/apis/awsprovider/v1alpha1" ) @@ -69,73 +68,78 @@ type Config struct { // ReconcileCertificates generate certificates if none exists. func (s *Service) ReconcileCertificates() error { - klog.V(2).Infof("Reconciling certificates for cluster %q", s.scope.Cluster.Name) - clusterCAKeyPair, err := getOrGenerateCACert(&s.scope.ClusterConfig.CAKeyPair, clusterCA) - if err != nil { - return errors.Wrapf(err, "Failed to generate certs for %q", clusterCA) - } - s.scope.ClusterConfig.CAKeyPair = clusterCAKeyPair + s.scope.V(2).Info("Reconciling certificates", "cluster-name", s.scope.Cluster.Name, "cluster-namespace", s.scope.Cluster.Namespace) - etcdCAKeyPair, err := getOrGenerateCACert(&s.scope.ClusterConfig.EtcdCAKeyPair, etcdCA) - if err != nil { - return errors.Wrapf(err, "Failed to generate certs for %q", etcdCA) + if !s.scope.ClusterConfig.CAKeyPair.HasCertAndKey() { + s.scope.V(2).Info("Generating keypair for", "user", clusterCA) + clusterCAKeyPair, err := generateCACert(&s.scope.ClusterConfig.CAKeyPair, clusterCA) + if err != nil { + return errors.Wrapf(err, "Failed to generate certs for %q", clusterCA) + } + s.scope.ClusterConfig.CAKeyPair = clusterCAKeyPair } - s.scope.ClusterConfig.EtcdCAKeyPair = etcdCAKeyPair - fpCAKeyPair, err := getOrGenerateCACert(&s.scope.ClusterConfig.FrontProxyCAKeyPair, frontProxyCA) - if err != nil { - return errors.Wrapf(err, "Failed to generate certs for %q", frontProxyCA) + if !s.scope.ClusterConfig.EtcdCAKeyPair.HasCertAndKey() { + s.scope.V(2).Info("Generating keypair", "user", etcdCA) + etcdCAKeyPair, err := generateCACert(&s.scope.ClusterConfig.EtcdCAKeyPair, etcdCA) + if err != nil { + return errors.Wrapf(err, "Failed to generate certs for %q", etcdCA) + } + s.scope.ClusterConfig.EtcdCAKeyPair = etcdCAKeyPair } - s.scope.ClusterConfig.FrontProxyCAKeyPair = fpCAKeyPair - - saKeyPair, err := getOrGenerateServiceAccountKeys(&s.scope.ClusterConfig.SAKeyPair, serviceAccount) - if err != nil { - return errors.Wrapf(err, "Failed to generate keyPair for %q", serviceAccount) + if !s.scope.ClusterConfig.FrontProxyCAKeyPair.HasCertAndKey() { + s.scope.V(2).Info("Generating keypair", "user", frontProxyCA) + fpCAKeyPair, err := generateCACert(&s.scope.ClusterConfig.FrontProxyCAKeyPair, frontProxyCA) + if err != nil { + return errors.Wrapf(err, "Failed to generate certs for %q", frontProxyCA) + } + s.scope.ClusterConfig.FrontProxyCAKeyPair = fpCAKeyPair } - s.scope.ClusterConfig.SAKeyPair = saKeyPair + if !s.scope.ClusterConfig.SAKeyPair.HasCertAndKey() { + s.scope.V(2).Info("Generating service account keys", "user", serviceAccount) + saKeyPair, err := generateServiceAccountKeys(&s.scope.ClusterConfig.SAKeyPair, serviceAccount) + if err != nil { + return errors.Wrapf(err, "Failed to generate keyPair for %q", serviceAccount) + } + s.scope.ClusterConfig.SAKeyPair = saKeyPair + } return nil } -func getOrGenerateCACert(kp *v1alpha1.KeyPair, user string) (v1alpha1.KeyPair, error) { - if kp == nil || !kp.HasCertAndKey() { - klog.V(2).Infof("Generating keypair for %q", user) - x509Cert, privKey, err := NewCertificateAuthority() - if err != nil { - return v1alpha1.KeyPair{}, errors.Wrapf(err, "failed to generate CA cert for %q", user) - } - if kp == nil { - return v1alpha1.KeyPair{ - Cert: EncodeCertPEM(x509Cert), - Key: EncodePrivateKeyPEM(privKey), - }, nil - } - kp.Cert = EncodeCertPEM(x509Cert) - kp.Key = EncodePrivateKeyPEM(privKey) +func generateCACert(kp *v1alpha1.KeyPair, user string) (v1alpha1.KeyPair, error) { + x509Cert, privKey, err := NewCertificateAuthority() + if err != nil { + return v1alpha1.KeyPair{}, errors.Wrapf(err, "failed to generate CA cert for %q", user) } + if kp == nil { + return v1alpha1.KeyPair{ + Cert: EncodeCertPEM(x509Cert), + Key: EncodePrivateKeyPEM(privKey), + }, nil + } + kp.Cert = EncodeCertPEM(x509Cert) + kp.Key = EncodePrivateKeyPEM(privKey) return *kp, nil } -func getOrGenerateServiceAccountKeys(kp *v1alpha1.KeyPair, user string) (v1alpha1.KeyPair, error) { - if kp == nil || !kp.HasCertAndKey() { - klog.V(2).Infof("Generating service account keys for %q", user) - saCreds, err := NewPrivateKey() - if err != nil { - return v1alpha1.KeyPair{}, errors.Wrapf(err, "failed to create service account public and private keys") - } - saPub, err := EncodePublicKeyPEM(&saCreds.PublicKey) - if err != nil { - return v1alpha1.KeyPair{}, errors.Wrapf(err, "failed to encode service account public key to PEM") - } - if kp == nil { - return v1alpha1.KeyPair{ - Cert: saPub, - Key: EncodePrivateKeyPEM(saCreds), - }, nil - } - kp.Cert = saPub - kp.Key = EncodePrivateKeyPEM(saCreds) +func generateServiceAccountKeys(kp *v1alpha1.KeyPair, user string) (v1alpha1.KeyPair, error) { + saCreds, err := NewPrivateKey() + if err != nil { + return v1alpha1.KeyPair{}, errors.Wrapf(err, "failed to create service account public and private keys") + } + saPub, err := EncodePublicKeyPEM(&saCreds.PublicKey) + if err != nil { + return v1alpha1.KeyPair{}, errors.Wrapf(err, "failed to encode service account public key to PEM") + } + if kp == nil { + return v1alpha1.KeyPair{ + Cert: saPub, + Key: EncodePrivateKeyPEM(saCreds), + }, nil } + kp.Cert = saPub + kp.Key = EncodePrivateKeyPEM(saCreds) return *kp, nil } diff --git a/pkg/cloud/aws/services/certificates/certificates_test.go b/pkg/cloud/aws/services/certificates/certificates_test.go index c5ecde3951..b1d2279fe7 100644 --- a/pkg/cloud/aws/services/certificates/certificates_test.go +++ b/pkg/cloud/aws/services/certificates/certificates_test.go @@ -24,7 +24,7 @@ import ( "sigs.k8s.io/cluster-api-provider-aws/pkg/apis/awsprovider/v1alpha1" ) -func TestGetOrGenerateCACert(t *testing.T) { +func TestGenerateCACert(t *testing.T) { testCases := []struct { name string inputKeyPair *v1alpha1.KeyPair @@ -69,7 +69,7 @@ func TestGetOrGenerateCACert(t *testing.T) { }, } for _, tc := range testCases { - actualKeyPair, actualError := getOrGenerateCACert(tc.inputKeyPair, tc.inputUser) + actualKeyPair, actualError := generateCACert(tc.inputKeyPair, tc.inputUser) if tc.expectedError != nil { if tc.expectedError.Error() != actualError.Error() { t.Fatalf("[%s], Unexpected error, Want [%v], Got: [%v]", tc.name, tc.expectedError, actualError) @@ -96,7 +96,7 @@ func TestGetOrGenerateCACert(t *testing.T) { } } -func TestGetOrGenerateServiceAccountKeys(t *testing.T) { +func TestGenerateServiceAccountKeys(t *testing.T) { testCases := []struct { name string inputKeyPair *v1alpha1.KeyPair @@ -141,7 +141,7 @@ func TestGetOrGenerateServiceAccountKeys(t *testing.T) { }, } for _, tc := range testCases { - actualKeyPair, actualError := getOrGenerateServiceAccountKeys(tc.inputKeyPair, tc.inputUser) + actualKeyPair, actualError := generateServiceAccountKeys(tc.inputKeyPair, tc.inputUser) if tc.expectedError != nil { if tc.expectedError.Error() != actualError.Error() { t.Fatalf("[%s], Unexpected error, Want [%v], Got: [%v]", tc.name, tc.expectedError, actualError) diff --git a/pkg/cloud/aws/services/ec2/BUILD b/pkg/cloud/aws/services/ec2/BUILD index 46bde94634..577351b6a2 100644 --- a/pkg/cloud/aws/services/ec2/BUILD +++ b/pkg/cloud/aws/services/ec2/BUILD @@ -35,7 +35,6 @@ go_library( "//vendor/github.com/aws/aws-sdk-go/aws:go_default_library", "//vendor/github.com/aws/aws-sdk-go/service/ec2:go_default_library", "//vendor/github.com/pkg/errors:go_default_library", - "//vendor/k8s.io/klog:go_default_library", ], ) diff --git a/pkg/cloud/aws/services/ec2/ami.go b/pkg/cloud/aws/services/ec2/ami.go index 28f604f2ba..56e356f58e 100644 --- a/pkg/cloud/aws/services/ec2/ami.go +++ b/pkg/cloud/aws/services/ec2/ami.go @@ -22,8 +22,6 @@ import ( "strings" "time" - "k8s.io/klog" - "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" "github.com/pkg/errors" @@ -86,7 +84,7 @@ func (s *Service) defaultAMILookup(baseOS, baseOSVersion, kubernetesVersion stri return "", errors.Errorf("found no AMIs with the name: %q", amiName(baseOS, baseOSVersion, kubernetesVersion)) } latestImage := getLatestImage(out.Images) - klog.V(2).Infof("Using AMI: %q", aws.StringValue(latestImage.ImageId)) + s.scope.V(2).Info("Found and using an existing AMI", "ami-id", aws.StringValue(latestImage.ImageId)) return aws.StringValue(latestImage.ImageId), nil } @@ -99,15 +97,14 @@ func (i images) Len() int { // Less reports whether the element with // index i should sort before the element with index j. +// TODO(chuckha) Ignoring errors until this causes a problem func (i images) Less(k, j int) bool { firstTime, err := time.Parse(createDateTimestampFormat, aws.StringValue(i[k].CreationDate)) if err != nil { - klog.Infof("unable to parse an AMI creation timestamp: %q", aws.StringValue(i[k].CreationDate)) return false } secondTime, err := time.Parse(createDateTimestampFormat, aws.StringValue(i[j].CreationDate)) if err != nil { - klog.Infof("unable to parse an AMI creation timestamp: %q", aws.StringValue(i[j].CreationDate)) return false } return firstTime.Before(secondTime) diff --git a/pkg/cloud/aws/services/ec2/bastion.go b/pkg/cloud/aws/services/ec2/bastion.go index 613ba6a472..7094df181e 100644 --- a/pkg/cloud/aws/services/ec2/bastion.go +++ b/pkg/cloud/aws/services/ec2/bastion.go @@ -23,7 +23,6 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" "github.com/pkg/errors" - "k8s.io/klog" "sigs.k8s.io/cluster-api-provider-aws/pkg/apis/awsprovider/v1alpha1" "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/aws/converters" "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/aws/filter" @@ -39,15 +38,15 @@ const ( // ReconcileBastion ensures a bastion is created for the cluster func (s *Service) ReconcileBastion() error { if s.scope.VPC().IsProvided() { - klog.V(4).Info("Skipping bastion reconcile in unmanaged mode") + s.scope.V(4).Info("Skipping bastion reconcile in unmanaged mode") return nil } - klog.V(2).Info("Reconciling bastion host") + s.scope.V(2).Info("Reconciling bastion host") subnets := s.scope.Subnets() if len(subnets.FilterPrivate()) == 0 { - klog.V(2).Info("No private subnets available, skipping bastion host") + s.scope.V(2).Info("No private subnets available, skipping bastion host") return nil } else if len(subnets.FilterPublic()) == 0 { return errors.New("failed to reconcile bastion host, no public subnets are available") @@ -63,7 +62,7 @@ func (s *Service) ReconcileBastion() error { return err } - klog.V(2).Infof("Created new bastion host: %+v", instance) + s.scope.V(2).Info("Created new bastion host", "instance", instance) } else if err != nil { return err @@ -72,21 +71,21 @@ func (s *Service) ReconcileBastion() error { // TODO(vincepri): check for possible changes between the default spec and the instance. instance.DeepCopyInto(&s.scope.ClusterStatus.Bastion) - klog.V(2).Info("Reconcile bastion completed successfully") + s.scope.V(2).Info("Reconcile bastion completed successfully") return nil } // DeleteBastion deletes the Bastion instance func (s *Service) DeleteBastion() error { if s.scope.VPC().IsProvided() { - klog.V(4).Info("Skipping bastion deletion in unmanaged mode") + s.scope.V(4).Info("Skipping bastion deletion in unmanaged mode") return nil } instance, err := s.describeBastionInstance() if err != nil { if awserrors.IsNotFound(err) { - klog.V(2).Info("bastion instance does not exist") + s.scope.V(2).Info("bastion instance does not exist", "instance-id", instance.ID) return nil } return errors.Wrap(err, "unable to describe bastion instance") diff --git a/pkg/cloud/aws/services/ec2/eips.go b/pkg/cloud/aws/services/ec2/eips.go index 99925b31a4..b1dea1fd9c 100644 --- a/pkg/cloud/aws/services/ec2/eips.go +++ b/pkg/cloud/aws/services/ec2/eips.go @@ -25,7 +25,6 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" "github.com/pkg/errors" - "k8s.io/klog" "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/aws/services/awserrors" "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/aws/services/wait" ) @@ -123,7 +122,7 @@ func (s *Service) releaseAddresses() error { return errors.Wrapf(err, "failed to release ElasticIP %q", *ip.AllocationId) } - klog.Infof("released ElasticIP %q with allocation ID %q", *ip.PublicIp, *ip.AllocationId) + s.scope.Info("released ElasticIP", "eip", *ip.PublicIp, "allocation-id", *ip.AllocationId) } return nil } diff --git a/pkg/cloud/aws/services/ec2/gateways.go b/pkg/cloud/aws/services/ec2/gateways.go index d2e0b770de..a6bdae62c0 100644 --- a/pkg/cloud/aws/services/ec2/gateways.go +++ b/pkg/cloud/aws/services/ec2/gateways.go @@ -22,7 +22,6 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" "github.com/pkg/errors" - "k8s.io/klog" "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/aws/converters" "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/aws/filter" "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/aws/services/awserrors" @@ -32,11 +31,11 @@ import ( func (s *Service) reconcileInternetGateways() error { if s.scope.VPC().IsProvided() { - klog.V(4).Info("Skipping internet gateways reconcile in unmanaged mode") + s.scope.V(4).Info("Skipping internet gateways reconcile in unmanaged mode") return nil } - klog.V(2).Infof("Reconciling internet gateways") + s.scope.V(2).Info("Reconciling internet gateways") igs, err := s.describeVpcInternetGateways() if awserrors.IsNotFound(err) { @@ -71,7 +70,7 @@ func (s *Service) reconcileInternetGateways() error { func (s *Service) deleteInternetGateways() error { if s.scope.VPC().IsProvided() { - klog.V(4).Info("Skipping internet gateway deletion in unmanaged mode") + s.scope.V(4).Info("Skipping internet gateway deletion in unmanaged mode") return nil } @@ -92,7 +91,7 @@ func (s *Service) deleteInternetGateways() error { return errors.Wrapf(err, "failed to detach internet gateway %q", *ig.InternetGatewayId) } - klog.Infof("Detached internet gateway %q from VPC %q", *ig.InternetGatewayId, s.scope.VPC().ID) + s.scope.Info("Detached internet gateway from VPC", "internet-gateway-id", *ig.InternetGatewayId, "vpc-id", s.scope.VPC().ID) deleteReq := &ec2.DeleteInternetGatewayInput{ InternetGatewayId: ig.InternetGatewayId, @@ -102,7 +101,7 @@ func (s *Service) deleteInternetGateways() error { return errors.Wrapf(err, "failed to delete internet gateway %q", *ig.InternetGatewayId) } - klog.Infof("Deleted internet gateway %q in VPC %q", *ig.InternetGatewayId, s.scope.VPC().ID) + s.scope.Info("Deleted internet gateway in VPC", "internet-gateway-id", *ig.InternetGatewayId, "vpc-id", s.scope.VPC().ID) record.Eventf(s.scope.Cluster, "DeletedInternetGateway", "Deleted Internet Gateway %q previously attached to VPC %q", *ig.InternetGatewayId, s.scope.VPC().ID) } @@ -115,7 +114,7 @@ func (s *Service) createInternetGateway() (*ec2.InternetGateway, error) { return nil, errors.Wrap(err, "failed to create internet gateway") } - klog.Infof("Created internet gateway %q", s.scope.VPC().ID) + s.scope.Info("Created internet gateway for VPC", "vpc-id", s.scope.VPC().ID) _, err = s.scope.EC2.AttachInternetGateway(&ec2.AttachInternetGatewayInput{ InternetGatewayId: ig.InternetGateway.InternetGatewayId, VpcId: aws.String(s.scope.VPC().ID), @@ -125,7 +124,7 @@ func (s *Service) createInternetGateway() (*ec2.InternetGateway, error) { return nil, errors.Wrapf(err, "failed to attach internet gateway %q to vpc %q", *ig.InternetGateway.InternetGatewayId, s.scope.VPC().ID) } - klog.Infof("attached internet gateway %q to VPC %q", *ig.InternetGateway.InternetGatewayId, s.scope.VPC().ID) + s.scope.Info("attached internet gateway to VPC", "internet-gateway-id", *ig.InternetGateway.InternetGatewayId, "vpc-id", s.scope.VPC().ID) record.Eventf(s.scope.Cluster, "CreatedInternetGateway", "Created new Internet Gateway %q attached to VPC %q", *ig.InternetGateway.InternetGatewayId, s.scope.VPC().ID) return ig.InternetGateway, nil } diff --git a/pkg/cloud/aws/services/ec2/instances.go b/pkg/cloud/aws/services/ec2/instances.go index 941618920e..d6c2b2eeff 100644 --- a/pkg/cloud/aws/services/ec2/instances.go +++ b/pkg/cloud/aws/services/ec2/instances.go @@ -24,7 +24,6 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" "github.com/pkg/errors" - "k8s.io/klog" "sigs.k8s.io/cluster-api-provider-aws/pkg/apis/awsprovider/v1alpha1" "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/aws/actuators" "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/aws/converters" @@ -39,7 +38,7 @@ import ( // InstanceByTags returns the existing instance or nothing if it doesn't exist. func (s *Service) InstanceByTags(machine *actuators.MachineScope) (*v1alpha1.Instance, error) { - klog.V(2).Infof("Looking for existing instance for machine %q in cluster %q", machine.Name(), s.scope.Name()) + s.scope.V(2).Info("Looking for existing machine instance") input := &ec2.DescribeInstancesInput{ Filters: []*ec2.Filter{ @@ -73,11 +72,11 @@ func (s *Service) InstanceByTags(machine *actuators.MachineScope) (*v1alpha1.Ins // InstanceIfExists returns the existing instance or nothing if it doesn't exist. func (s *Service) InstanceIfExists(id *string) (*v1alpha1.Instance, error) { if id == nil { - klog.Error("Instance does not have an instance id") + s.scope.Info("Instance does not have an instance id") return nil, nil } - klog.V(2).Infof("Looking for instance %q", *id) + s.scope.V(2).Info("Looking for instance", "instance-id", *id) input := &ec2.DescribeInstancesInput{ InstanceIds: []*string{id}, @@ -104,7 +103,7 @@ func (s *Service) InstanceIfExists(id *string) (*v1alpha1.Instance, error) { // createInstance runs an ec2 instance. func (s *Service) createInstance(machine *actuators.MachineScope, bootstrapToken, kubeConfig string) (*v1alpha1.Instance, error) { - klog.V(2).Infof("Creating a new instance for machine %q", machine.Name()) + s.scope.V(2).Info("Creating an instance for a machine") input := &v1alpha1.Instance{ Type: machine.MachineConfig.InstanceType, @@ -154,6 +153,7 @@ func (s *Service) createInstance(machine *actuators.MachineScope, bootstrapToken ) } + s.scope.V(3).Info("Generating CA key pair") caCertHash, err := certificates.GenerateCertificateHash(s.scope.ClusterConfig.CAKeyPair.Cert) if err != nil { return input, err @@ -171,7 +171,7 @@ func (s *Service) createInstance(machine *actuators.MachineScope, bootstrapToken var userData string if bootstrapToken != "" { - klog.V(2).Infof("Allowing machine %q to join control plane for cluster %q", machine.Name(), s.scope.Name()) + s.scope.V(2).Info("Allowing a machine to join the control plane") updatedJoinConfiguration := kubeadm.SetJoinNodeConfigurationOverrides(caCertHash, bootstrapToken, machine, &machine.MachineConfig.KubeadmConfiguration.Join) updatedJoinConfiguration = kubeadm.SetControlPlaneJoinConfigurationOverrides(updatedJoinConfiguration) @@ -195,7 +195,7 @@ func (s *Service) createInstance(machine *actuators.MachineScope, bootstrapToken return input, err } } else { - klog.V(2).Infof("Machine %q is the first controlplane machine for cluster %q", machine.Name(), s.scope.Name()) + s.scope.V(2).Info("Machine is the first control plane machine for the cluster") if !s.scope.ClusterConfig.CAKeyPair.HasCertAndKey() { return nil, awserrors.NewFailedDependency( errors.New("failed to run controlplane, missing CAPrivateKey"), @@ -208,7 +208,7 @@ func (s *Service) createInstance(machine *actuators.MachineScope, bootstrapToken return nil, err } - initConfiguration := kubeadm.SetInitConfigurationOverrides(&machine.MachineConfig.KubeadmConfiguration.Init) + initConfiguration := kubeadm.SetInitConfigurationOverrides(machine, &machine.MachineConfig.KubeadmConfiguration.Init) initConfigYAML, err := kubeadm.ConfigurationToYAML(initConfiguration) if err != nil { return nil, err @@ -235,6 +235,7 @@ func (s *Service) createInstance(machine *actuators.MachineScope, bootstrapToken input.UserData = aws.String(userData) input.SecurityGroupIDs = append(input.SecurityGroupIDs, s.scope.SecurityGroups()[v1alpha1.SecurityGroupControlPlane].ID, s.scope.SecurityGroups()[v1alpha1.SecurityGroupNode].ID) case "node": + s.scope.V(2).Info("Joining a worker node to the cluster") input.SecurityGroupIDs = append(input.SecurityGroupIDs, s.scope.SecurityGroups()[v1alpha1.SecurityGroupNode].ID) joinConfiguration := kubeadm.SetJoinNodeConfigurationOverrides(caCertHash, bootstrapToken, machine, &machine.MachineConfig.KubeadmConfiguration.Join) @@ -264,6 +265,7 @@ func (s *Service) createInstance(machine *actuators.MachineScope, bootstrapToken input.KeyName = aws.String(defaultSSHKeyName) } + s.scope.V(2).Info("Running instance", "machine-role", machine.Role()) out, err := s.runInstance(machine.Role(), input) if err != nil { return nil, err @@ -276,7 +278,7 @@ func (s *Service) createInstance(machine *actuators.MachineScope, bootstrapToken // TerminateInstance terminates an EC2 instance. // Returns nil on success, error in all other cases. func (s *Service) TerminateInstance(instanceID string) error { - klog.V(2).Infof("Attempting to terminate instance with id %q", instanceID) + s.scope.V(2).Info("Attempting to terminate instance", "instance-id", instanceID) input := &ec2.TerminateInstancesInput{ InstanceIds: aws.StringSlice([]string{instanceID}), @@ -286,7 +288,7 @@ func (s *Service) TerminateInstance(instanceID string) error { return errors.Wrapf(err, "failed to terminate instance with id %q", instanceID) } - klog.V(2).Infof("Terminated instance with id %q", instanceID) + s.scope.V(2).Info("Terminated instance", "instance-id", instanceID) record.Eventf(s.scope.Cluster, "DeletedInstance", "Terminated instance %q", instanceID) return nil } @@ -298,7 +300,7 @@ func (s *Service) TerminateInstanceAndWait(instanceID string) error { return err } - klog.V(2).Infof("Waiting for EC2 instance with id %q to terminate", instanceID) + s.scope.V(2).Info("Waiting for EC2 instance to terminate", "instance-id", instanceID) input := &ec2.DescribeInstancesInput{ InstanceIds: aws.StringSlice([]string{instanceID}), @@ -332,11 +334,11 @@ func (s *Service) MachineExists(machine *actuators.MachineScope) (bool, error) { // CreateOrGetMachine will either return an existing instance or create and return an instance. func (s *Service) CreateOrGetMachine(machine *actuators.MachineScope, bootstrapToken, kubeConfig string) (*v1alpha1.Instance, error) { - klog.V(2).Infof("Attempting to create or get machine %q", machine.Name()) + s.scope.V(2).Info("Attempting to create or get machine") // instance id exists, try to get it if machine.MachineStatus.InstanceID != nil { - klog.V(2).Infof("Looking up machine %q by id %q", machine.Name(), *machine.MachineStatus.InstanceID) + s.scope.V(2).Info("Looking up machine by id", "instance-id", *machine.MachineStatus.InstanceID) instance, err := s.InstanceIfExists(machine.MachineStatus.InstanceID) if err != nil && !awserrors.IsNotFound(err) { @@ -346,7 +348,7 @@ func (s *Service) CreateOrGetMachine(machine *actuators.MachineScope, bootstrapT } } - klog.V(2).Infof("Looking up machine %q by tags", machine.Name()) + s.scope.V(2).Info("Looking up machine by tags") instance, err := s.InstanceByTags(machine) if err != nil && !awserrors.IsNotFound(err) { return nil, errors.Wrapf(err, "failed to query machine %q instance by tags", machine.Name()) @@ -414,21 +416,21 @@ func (s *Service) runInstance(role string, i *v1alpha1.Instance) (*v1alpha1.Inst return nil, errors.Errorf("no instance returned for reservation %v", out.GoString()) } - s.scope.EC2.WaitUntilInstanceRunning(&ec2.DescribeInstancesInput{InstanceIds: []*string{out.Instances[0].InstanceId}}) - return converters.SDKToInstance(out.Instances[0]), nil + err = s.scope.EC2.WaitUntilInstanceRunning(&ec2.DescribeInstancesInput{InstanceIds: []*string{out.Instances[0].InstanceId}}) + return converters.SDKToInstance(out.Instances[0]), err } // UpdateInstanceSecurityGroups modifies the security groups of the given // EC2 instance. func (s *Service) UpdateInstanceSecurityGroups(instanceID string, ids []string) error { - klog.V(2).Infof("Attempting to update security groups on instance %q", instanceID) + s.scope.V(2).Info("Attempting to update security groups on instance", "instance-id", instanceID) enis, err := s.getInstanceENIs(instanceID) if err != nil { return errors.Wrapf(err, "failed to get ENIs for instance %q", instanceID) } - klog.V(3).Infof("Found %v ENIs on instance %q", len(enis), instanceID) + s.scope.V(3).Info("Found ENIs on instance", "number-of-enis", len(enis), "instance-id", instanceID) for _, eni := range enis { input := &ec2.ModifyNetworkInterfaceAttributeInput{ @@ -449,11 +451,11 @@ func (s *Service) UpdateInstanceSecurityGroups(instanceID string, ids []string) // We may not always have to perform each action, so we check what we're // receiving to avoid calling AWS if we don't need to. func (s *Service) UpdateResourceTags(resourceID *string, create map[string]string, remove map[string]string) error { - klog.V(2).Infof("Attempting to update tags on resource %q", *resourceID) + s.scope.V(2).Info("Attempting to update tags on resource", "resource-id", *resourceID) // If we have anything to create or update if len(create) > 0 { - klog.V(2).Infof("Attempting to create tags on resource %q", *resourceID) + s.scope.V(2).Info("Attempting to create tags on resource", "resource-id", *resourceID) // Convert our create map into an array of *ec2.Tag createTagsInput := converters.MapToTags(create) @@ -472,7 +474,7 @@ func (s *Service) UpdateResourceTags(resourceID *string, create map[string]strin // If we have anything to remove if len(remove) > 0 { - klog.V(2).Infof("Attempting to delete tags on resource %q", *resourceID) + s.scope.V(2).Info("Attempting to delete tags on resource", "resource-id", *resourceID) // Convert our remove map into an array of *ec2.Tag removeTagsInput := converters.MapToTags(remove) diff --git a/pkg/cloud/aws/services/ec2/natgateways.go b/pkg/cloud/aws/services/ec2/natgateways.go index 701f92107a..a6c8da14b4 100644 --- a/pkg/cloud/aws/services/ec2/natgateways.go +++ b/pkg/cloud/aws/services/ec2/natgateways.go @@ -22,7 +22,6 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" "github.com/pkg/errors" - "k8s.io/klog" "sigs.k8s.io/cluster-api-provider-aws/pkg/apis/awsprovider/v1alpha1" "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/aws/converters" "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/aws/filter" @@ -33,17 +32,17 @@ import ( func (s *Service) reconcileNatGateways() error { if s.scope.VPC().IsProvided() { - klog.V(4).Info("Skipping NAT gateway reconcile in unmanaged mode") + s.scope.V(4).Info("Skipping NAT gateway reconcile in unmanaged mode") return nil } - klog.V(2).Infof("Reconciling NAT gateways") + s.scope.V(2).Info("Reconciling NAT gateways") if len(s.scope.Subnets().FilterPrivate()) == 0 { - klog.V(2).Infof("No private subnets available, skipping NAT gateways") + s.scope.V(2).Info("No private subnets available, skipping NAT gateways") return nil } else if len(s.scope.Subnets().FilterPublic()) == 0 { - klog.V(2).Infof("No public subnets available. Cannot create NAT gateways for private subnets, this might be a configuration error.") + s.scope.V(2).Info("No public subnets available. Cannot create NAT gateways for private subnets, this might be a configuration error.") return nil } @@ -84,15 +83,15 @@ func (s *Service) reconcileNatGateways() error { func (s *Service) deleteNatGateways() error { if s.scope.VPC().IsProvided() { - klog.V(4).Info("Skipping NAT gateway deletion in unmanaged mode") + s.scope.V(4).Info("Skipping NAT gateway deletion in unmanaged mode") return nil } if len(s.scope.Subnets().FilterPrivate()) == 0 { - klog.V(2).Infof("No private subnets available, skipping NAT gateways") + s.scope.V(2).Info("No private subnets available, skipping NAT gateways") return nil } else if len(s.scope.Subnets().FilterPublic()) == 0 { - klog.V(2).Infof("No public subnets available. Cannot create NAT gateways for private subnets, this might be a configuration error.") + s.scope.V(2).Info("No public subnets available. Cannot create NAT gateways for private subnets, this might be a configuration error.") return nil } @@ -178,14 +177,14 @@ func (s *Service) createNatGateway(subnetID string) (*ec2.NatGateway, error) { return nil, errors.Wrapf(err, "failed to tag nat gateway %q", *out.NatGateway.NatGatewayId) } - klog.Infof("Created NAT gateway %q for subnet ID %q, waiting for it to become available...", *out.NatGateway.NatGatewayId, subnetID) + s.scope.Info("Created NAT gateway for subnet, waiting for it to become available...", "nat-gateway-id", *out.NatGateway.NatGatewayId, "subnet-id", subnetID) wReq := &ec2.DescribeNatGatewaysInput{NatGatewayIds: []*string{out.NatGateway.NatGatewayId}} if err := s.scope.EC2.WaitUntilNatGatewayAvailable(wReq); err != nil { return nil, errors.Wrapf(err, "failed to wait for nat gateway %q in subnet %q", *out.NatGateway.NatGatewayId, subnetID) } - klog.Infof("NAT gateway %q for subnet ID %q is now available", *out.NatGateway.NatGatewayId, subnetID) + s.scope.Info("NAT gateway for subnet is now available", "nat-gateway-id", *out.NatGateway.NatGatewayId, "subnet-id", subnetID) record.Eventf(s.scope.Cluster, "CreatedNATGateway", "Created new NAT Gateway %q", *out.NatGateway.NatGatewayId) return out.NatGateway, nil } @@ -232,7 +231,7 @@ func (s *Service) deleteNatGateway(id string) error { return errors.Wrapf(err, "failed to wait for NAT gateway deletion %q", id) } - klog.Infof("Deleted NAT gateway %q", id) + s.scope.Info("Deleted NAT gateway", "nat-gateway-id", id) record.Eventf(s.scope.Cluster, "DeletedNATGateway", "Deleted NAT Gateway %q", id) return nil } diff --git a/pkg/cloud/aws/services/ec2/network.go b/pkg/cloud/aws/services/ec2/network.go index 7d55fc9d22..b91b9b00da 100644 --- a/pkg/cloud/aws/services/ec2/network.go +++ b/pkg/cloud/aws/services/ec2/network.go @@ -16,13 +16,9 @@ limitations under the License. package ec2 -import ( - "k8s.io/klog" -) - // ReconcileNetwork reconciles the network of the given cluster. func (s *Service) ReconcileNetwork() (err error) { - klog.V(2).Infof("Reconciling network for cluster %q", s.scope.Cluster.Name) + s.scope.V(2).Info("Reconciling network for cluster", "cluster-name", s.scope.Cluster.Name, "cluster-namespace", s.scope.Cluster.Namespace) // VPC. if err := s.reconcileVPC(); err != nil { @@ -54,13 +50,13 @@ func (s *Service) ReconcileNetwork() (err error) { return err } - klog.V(2).Info("Reconcile network completed successfully") + s.scope.V(2).Info("Reconcile network completed successfully") return nil } // DeleteNetwork deletes the network of the given cluster. func (s *Service) DeleteNetwork() (err error) { - klog.V(2).Info("Deleting network") + s.scope.V(2).Info("Deleting network") // Security groups. if err := s.deleteSecurityGroups(); err != nil { @@ -97,6 +93,6 @@ func (s *Service) DeleteNetwork() (err error) { return err } - klog.V(2).Info("Delete network completed successfully") + s.scope.V(2).Info("Delete network completed successfully") return nil } diff --git a/pkg/cloud/aws/services/ec2/routetables.go b/pkg/cloud/aws/services/ec2/routetables.go index b2153bff63..1e7795fbb6 100644 --- a/pkg/cloud/aws/services/ec2/routetables.go +++ b/pkg/cloud/aws/services/ec2/routetables.go @@ -22,7 +22,6 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" "github.com/pkg/errors" - "k8s.io/klog" "sigs.k8s.io/cluster-api-provider-aws/pkg/apis/awsprovider/v1alpha1" "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/aws/converters" "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/aws/filter" @@ -35,11 +34,11 @@ const ( func (s *Service) reconcileRouteTables() error { if s.scope.VPC().IsProvided() { - klog.V(4).Info("Skipping routing tables reconcile in unmanaged mode") + s.scope.V(4).Info("Skipping routing tables reconcile in unmanaged mode") return nil } - klog.V(2).Infof("Reconciling routing tables") + s.scope.V(2).Info("Reconciling routing tables") subnetRouteMap, err := s.describeVpcRouteTablesBySubnet() if err != nil { @@ -48,7 +47,7 @@ func (s *Service) reconcileRouteTables() error { for _, sn := range s.scope.Subnets() { if igw, ok := subnetRouteMap[sn.ID]; ok { - klog.V(2).Infof("Subnet %q is already associated with route table %q", sn.ID, *igw.RouteTableId) + s.scope.V(2).Info("Subnet is already associated with route table", "subnet-id", sn.ID, "route-table-id", *igw.RouteTableId) // TODO(vincepri): if the route table ids are both non-empty and they don't match, replace the association. // TODO(vincepri): check that everything is in order, e.g. routes match the subnet type. @@ -92,7 +91,7 @@ func (s *Service) reconcileRouteTables() error { return err } - klog.V(2).Infof("Subnet %q has been associated with route table %q", sn.ID, rt.ID) + s.scope.V(2).Info("Subnet has been associated with route table", "subnet-id", sn.ID, "route-table-id", rt.ID) sn.RouteTableID = aws.String(rt.ID) } @@ -123,7 +122,7 @@ func (s *Service) describeVpcRouteTablesBySubnet() (map[string]*ec2.RouteTable, func (s *Service) deleteRouteTables() error { if s.scope.VPC().IsProvided() { - klog.V(4).Info("Skipping routing tables deletion in unmanaged mode") + s.scope.V(4).Info("Skipping routing tables deletion in unmanaged mode") return nil } @@ -142,14 +141,14 @@ func (s *Service) deleteRouteTables() error { return errors.Wrapf(err, "failed to disassociate route table %q from subnet %q", *rt.RouteTableId, *as.SubnetId) } - klog.Infof("Deleted association between route table %q and subnet %q", *rt.RouteTableId, *as.SubnetId) + s.scope.Info("Deleted association between route table and subnet", "route-table-id", *rt.RouteTableId, "subnet-id", *as.SubnetId) } if _, err := s.scope.EC2.DeleteRouteTable(&ec2.DeleteRouteTableInput{RouteTableId: rt.RouteTableId}); err != nil { return errors.Wrapf(err, "failed to delete route table %q", *rt.RouteTableId) } - klog.Infof("Deleted route table %q", *rt.RouteTableId) + s.scope.Info("Deleted route table", "route-table-id", *rt.RouteTableId) } return nil } diff --git a/pkg/cloud/aws/services/ec2/securitygroups.go b/pkg/cloud/aws/services/ec2/securitygroups.go index 3f1bc7880b..004a92a479 100644 --- a/pkg/cloud/aws/services/ec2/securitygroups.go +++ b/pkg/cloud/aws/services/ec2/securitygroups.go @@ -26,7 +26,6 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" "github.com/pkg/errors" - "k8s.io/klog" "sigs.k8s.io/cluster-api-provider-aws/pkg/apis/awsprovider/v1alpha1" "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/aws/services/awserrors" ) @@ -46,7 +45,7 @@ const ( ) func (s *Service) reconcileSecurityGroups() error { - klog.V(2).Infof("Reconciling security groups") + s.scope.V(2).Info("Reconciling security groups") if s.scope.Network().SecurityGroups == nil { s.scope.Network().SecurityGroups = make(map[v1alpha1.SecurityGroupRole]*v1alpha1.SecurityGroup) @@ -78,7 +77,7 @@ func (s *Service) reconcileSecurityGroups() error { ID: *sg.GroupId, Name: *sg.GroupName, } - klog.V(2).Infof("Security group for role %q: %v", role, s.scope.SecurityGroups()[role]) + s.scope.V(2).Info("Created security group for role", "role", role, "security-group", s.scope.SecurityGroups()[role]) continue } @@ -112,7 +111,7 @@ func (s *Service) reconcileSecurityGroups() error { return errors.Wrapf(err, "failed to revoke security group ingress rules for %q", sg.ID) } - klog.V(2).Infof("Revoked ingress rules %v from security group %q", toRevoke, sg) + s.scope.V(2).Info("Revoked ingress rules from security group", "revoked-ingress-rules", toRevoke, "security-group-id", sg.ID) } toAuthorize := want.Difference(current) @@ -121,7 +120,7 @@ func (s *Service) reconcileSecurityGroups() error { return err } - klog.V(2).Infof("Authorized ingress rules %v in security group %q", toAuthorize, sg) + s.scope.V(2).Info("Authorized ingress rules in security group", "authorized-ingress-rules", toAuthorize, "security-group-id", sg.ID) } } @@ -136,7 +135,7 @@ func (s *Service) deleteSecurityGroups() error { return err } - klog.V(2).Infof("Revoked ingress rules %v from security group %q", current, sg.ID) + s.scope.V(2).Info("Revoked ingress rules from security group", "revoked-ingress-rules", current, "security-group-id", sg.ID) } for _, sg := range s.scope.SecurityGroups() { @@ -148,7 +147,7 @@ func (s *Service) deleteSecurityGroups() error { return errors.Wrapf(err, "failed to delete security group %q", sg.ID) } - klog.V(2).Infof("Deleted security group security group %q", sg.ID) + s.scope.V(2).Info("Deleted security group security group", "security-group-id", sg.ID) } return nil diff --git a/pkg/cloud/aws/services/ec2/subnets.go b/pkg/cloud/aws/services/ec2/subnets.go index 97c3b21d3d..05400b64aa 100644 --- a/pkg/cloud/aws/services/ec2/subnets.go +++ b/pkg/cloud/aws/services/ec2/subnets.go @@ -24,7 +24,6 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" "github.com/pkg/errors" - "k8s.io/klog" "sigs.k8s.io/cluster-api-provider-aws/pkg/apis/awsprovider/v1alpha1" "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/aws/filter" "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/aws/tags" @@ -37,7 +36,7 @@ const ( ) func (s *Service) reconcileSubnets() error { - klog.V(2).Infof("Reconciling subnets") + s.scope.V(2).Info("Reconciling subnets") subnets := s.scope.Subnets() defer func() { @@ -132,13 +131,13 @@ LoopExisting: } } - klog.V(2).Infof("Subnets available: %v", subnets) + s.scope.V(2).Info("Subnets available", "subnets", subnets) return nil } func (s *Service) deleteSubnets() error { if s.scope.VPC().IsProvided() { - klog.V(4).Info("Skipping subnets deletion in unmanaged mode") + s.scope.V(4).Info("Skipping subnets deletion in unmanaged mode") return nil } @@ -260,8 +259,11 @@ func (s *Service) createSubnet(sn *v1alpha1.SubnetSpec) (*v1alpha1.SubnetSpec, e } } - klog.V(2).Infof("Created new subnet %q in VPC %q with cidr %q and availability zone %q", - *out.Subnet.SubnetId, *out.Subnet.VpcId, *out.Subnet.CidrBlock, *out.Subnet.AvailabilityZone) + s.scope.V(2).Info("Created new subnet in VPC with cidr and availability zone ", + "subnet-id", *out.Subnet.SubnetId, + "vpc-id", *out.Subnet.VpcId, + "cidr-block", *out.Subnet.CidrBlock, + "availability-zone", *out.Subnet.AvailabilityZone) record.Eventf(s.scope.Cluster, "CreatedSubnet", "Created new managed Subnet %q", *out.Subnet.SubnetId) @@ -282,7 +284,7 @@ func (s *Service) deleteSubnet(id string) error { return errors.Wrapf(err, "failed to delete subnet %q", id) } - klog.V(2).Infof("Deleted subnet %q in vpc %q", id, s.scope.VPC().ID) + s.scope.V(2).Info("Deleted subnet in vpc", "subnet-id", id, "vpc-id", s.scope.VPC().ID) record.Eventf(s.scope.Cluster, "DeletedSubnet", "Deleted managed Subnet %q", id) return nil } diff --git a/pkg/cloud/aws/services/ec2/vpc.go b/pkg/cloud/aws/services/ec2/vpc.go index 1c4deaa3c1..da3054e4d9 100644 --- a/pkg/cloud/aws/services/ec2/vpc.go +++ b/pkg/cloud/aws/services/ec2/vpc.go @@ -24,7 +24,6 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" "github.com/pkg/errors" - "k8s.io/klog" "sigs.k8s.io/cluster-api-provider-aws/pkg/apis/awsprovider/v1alpha1" "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/aws/converters" "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/aws/filter" @@ -37,7 +36,7 @@ const ( ) func (s *Service) reconcileVPC() error { - klog.V(2).Infof("Reconciling VPC") + s.scope.V(2).Info("Reconciling VPC") vpc, err := s.describeVPC() if awserrors.IsNotFound(err) { @@ -53,7 +52,7 @@ func (s *Service) reconcileVPC() error { if vpc.IsProvided() { vpc.DeepCopyInto(s.scope.VPC()) - klog.V(2).Infof("Working on unmanaged VPC %q", vpc.ID) + s.scope.V(2).Info("Working on unmanaged VPC", "vpc-id", vpc.ID) return nil } @@ -68,7 +67,7 @@ func (s *Service) reconcileVPC() error { } vpc.DeepCopyInto(s.scope.VPC()) - klog.V(2).Infof("Working on managed VPC %q", vpc.ID) + s.scope.V(2).Info("Working on managed VPC", "vpc-id", vpc.ID) return nil } @@ -95,7 +94,7 @@ func (s *Service) createVPC() (*v1alpha1.VPCSpec, error) { return nil, errors.Wrapf(err, "failed to wait for vpc %q", *out.Vpc.VpcId) } - klog.V(2).Infof("Created new VPC %q with cidr %q", *out.Vpc.VpcId, *out.Vpc.CidrBlock) + s.scope.V(2).Info("Created new VPC with cidr", "vpc-id", *out.Vpc.VpcId, "cidr-block", *out.Vpc.CidrBlock) record.Eventf(s.scope.Cluster, "CreatedVPC", "Created new managed VPC %q", *out.Vpc.VpcId) tagParams := s.getVPCTagParams(*out.Vpc.VpcId) @@ -119,7 +118,7 @@ func (s *Service) deleteVPC() error { vpc := s.scope.VPC() if vpc.IsProvided() { - klog.V(4).Info("Skipping VPC deletion in unmanaged mode") + s.scope.V(4).Info("Skipping VPC deletion in unmanaged mode") return nil } @@ -131,13 +130,13 @@ func (s *Service) deleteVPC() error { if err != nil { // Ignore if it's already deleted if code, ok := awserrors.Code(err); code == "InvalidVpcID.NotFound" && ok { - klog.V(4).Info("Skipping VPC deletion, VPC not found") + s.scope.V(4).Info("Skipping VPC deletion, VPC not found") return nil } return errors.Wrapf(err, "failed to delete vpc %q", vpc.ID) } - klog.V(2).Infof("Deleted VPC %q", vpc.ID) + s.scope.V(2).Info("Deleted VPC", "vpc-id", vpc.ID) record.Eventf(s.scope.Cluster, "DeletedVPC", "Deleted managed VPC %q", vpc.ID) return nil } diff --git a/pkg/cloud/aws/services/elb/BUILD.bazel b/pkg/cloud/aws/services/elb/BUILD.bazel index dcf0a3888b..99c232bab4 100644 --- a/pkg/cloud/aws/services/elb/BUILD.bazel +++ b/pkg/cloud/aws/services/elb/BUILD.bazel @@ -20,7 +20,6 @@ go_library( "//vendor/github.com/aws/aws-sdk-go/aws/awserr:go_default_library", "//vendor/github.com/aws/aws-sdk-go/service/elb:go_default_library", "//vendor/github.com/pkg/errors:go_default_library", - "//vendor/k8s.io/klog:go_default_library", ], ) diff --git a/pkg/cloud/aws/services/elb/loadbalancer.go b/pkg/cloud/aws/services/elb/loadbalancer.go index ac051b6b19..a9852083fd 100644 --- a/pkg/cloud/aws/services/elb/loadbalancer.go +++ b/pkg/cloud/aws/services/elb/loadbalancer.go @@ -30,13 +30,12 @@ import ( "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/service/elb" "github.com/pkg/errors" - "k8s.io/klog" "sigs.k8s.io/cluster-api-provider-aws/pkg/apis/awsprovider/v1alpha1" ) // ReconcileLoadbalancers reconciles the load balancers for the given cluster. func (s *Service) ReconcileLoadbalancers() error { - klog.V(2).Info("Reconciling load balancers") + s.scope.V(2).Info("Reconciling load balancers") // Get default api server spec. spec := s.getAPIServerClassicELBSpec() @@ -49,7 +48,7 @@ func (s *Service) ReconcileLoadbalancers() error { return err } - klog.V(2).Infof("Created new classic load balancer for apiserver: %v", apiELB) + s.scope.V(2).Info("Created new classic load balancer for apiserver", "api-server-elb-name", apiELB.Name) } else if err != nil { return err } @@ -63,9 +62,9 @@ func (s *Service) ReconcileLoadbalancers() error { // TODO(vincepri): check if anything has changed and reconcile as necessary. apiELB.DeepCopyInto(&s.scope.Network().APIServerELB) - klog.V(2).Infof("Control plane load balancer: %+v", apiELB) + s.scope.V(4).Info("Control plane load balancer", "api-server-elb", apiELB) - klog.V(2).Info("Reconcile load balancers completed successfully") + s.scope.V(2).Info("Reconcile load balancers completed successfully") return nil } @@ -82,7 +81,7 @@ func (s *Service) GetAPIServerDNSName() (string, error) { // DeleteLoadbalancers deletes the load balancers for the given cluster. func (s *Service) DeleteLoadbalancers() error { - klog.V(2).Info("Deleting load balancers") + s.scope.V(2).Info("Deleting load balancers") // Get default api server spec. spec := s.getAPIServerClassicELBSpec() @@ -100,7 +99,7 @@ func (s *Service) DeleteLoadbalancers() error { return err } - klog.V(2).Info("Deleting load balancers completed successfully") + s.scope.V(2).Info("Deleting load balancers completed successfully") return nil } @@ -218,7 +217,7 @@ func (s *Service) createClassicELB(spec *v1alpha1.ClassicELB) (*v1alpha1.Classic } } - klog.V(2).Infof("Created classic load balancer with dns name: %q", *out.DNSName) + s.scope.V(2).Info("Created classic load balancer", "dns-name", *out.DNSName) res := spec.DeepCopy() res.DNSName = *out.DNSName diff --git a/pkg/cloud/aws/services/kubeadm/BUILD.bazel b/pkg/cloud/aws/services/kubeadm/BUILD.bazel index fdf8a9c106..77fdb0154b 100644 --- a/pkg/cloud/aws/services/kubeadm/BUILD.bazel +++ b/pkg/cloud/aws/services/kubeadm/BUILD.bazel @@ -13,7 +13,6 @@ go_library( "//vendor/github.com/pkg/errors:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime/serializer:go_default_library", - "//vendor/k8s.io/klog:go_default_library", "//vendor/k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1beta1:go_default_library", "//vendor/k8s.io/kubernetes/cmd/kubeadm/app/util:go_default_library", "//vendor/sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha1:go_default_library", @@ -29,6 +28,7 @@ go_test( deps = [ "//pkg/apis/awsprovider/v1alpha1:go_default_library", "//pkg/cloud/aws/actuators:go_default_library", + "//pkg/cloudtest:go_default_library", "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//vendor/k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1beta1:go_default_library", "//vendor/sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha1:go_default_library", diff --git a/pkg/cloud/aws/services/kubeadm/aws_defaults.go b/pkg/cloud/aws/services/kubeadm/aws_defaults.go index 3441c8fbf3..95095b8387 100644 --- a/pkg/cloud/aws/services/kubeadm/aws_defaults.go +++ b/pkg/cloud/aws/services/kubeadm/aws_defaults.go @@ -22,7 +22,6 @@ import ( "sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha1" "sigs.k8s.io/cluster-api/pkg/util" - "k8s.io/klog" kubeadmv1beta1 "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1beta1" "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/aws/actuators" ) @@ -88,7 +87,7 @@ func SetClusterConfigurationOverrides(machine joinMachine, base *kubeadmv1beta1. out.APIServer.ControlPlaneComponent.ExtraArgs = map[string]string{} } if cp, ok := out.APIServer.ControlPlaneComponent.ExtraArgs["cloud-provider"]; ok && cp != CloudProvider { - klog.Infof("Overriding cloud provider %q with required value %q", cp, CloudProvider) + machine.GetScope().Logger.Info("Overriding cloud provider with required value", "provided-cloud-provider", cp, "required-cloud-provider", CloudProvider) } out.APIServer.ControlPlaneComponent.ExtraArgs["cloud-provider"] = CloudProvider @@ -96,13 +95,15 @@ func SetClusterConfigurationOverrides(machine joinMachine, base *kubeadmv1beta1. out.ControllerManager.ExtraArgs = map[string]string{} } if cp, ok := out.ControllerManager.ExtraArgs["cloud-provider"]; ok && cp != CloudProvider { - klog.Infof("Overriding cloud provider %q with required value %q", cp, CloudProvider) + machine.GetScope().Logger.Info("Overriding cloud provider with required value", "provided-cloud-provider", cp, "required-cloud-provider", CloudProvider) } out.ControllerManager.ExtraArgs["cloud-provider"] = CloudProvider // The kubeadm config clustername must match the provided name of the cluster. if out.ClusterName != "" && out.ClusterName != s.Name() { - klog.Infof("Overriding provided cluster name %q with %q. The kubeadm cluster name and cluster-api name must match.", out.ClusterName, s.Name()) + machine.GetScope().Logger.Info("Overriding provided cluster name. The kubeadm cluster name and cluster-api name must match.", + "provided-cluster-name", out.ClusterName, + "required-cluster-name", s.Name()) } out.ClusterName = s.Name() @@ -120,20 +121,24 @@ func SetClusterConfigurationOverrides(machine joinMachine, base *kubeadmv1beta1. // SetInitConfigurationOverrides overrides user input on particular fields for // the kubeadm InitConfiguration. -func SetInitConfigurationOverrides(base *kubeadmv1beta1.InitConfiguration) *kubeadmv1beta1.InitConfiguration { +func SetInitConfigurationOverrides(machine joinMachine, base *kubeadmv1beta1.InitConfiguration) *kubeadmv1beta1.InitConfiguration { if base == nil { base = &kubeadmv1beta1.InitConfiguration{} } out := base.DeepCopy() if out.NodeRegistration.Name != "" && out.NodeRegistration.Name != HostnameLookup { - klog.Infof("Overriding NodeRegistration name from %q to %q. The node registration needs to be dynamically generated in aws.", out.NodeRegistration.Name, HostnameLookup) + machine.GetScope().Info("Overriding NodeRegistration name. The node registration needs to be dynamically generated in aws.", + "provided-node-registration-name", out.NodeRegistration.Name, + "required-node-registration-name", HostnameLookup) } out.NodeRegistration.Name = HostnameLookup // TODO(chuckha): This may become a default instead of an override. if out.NodeRegistration.CRISocket != "" && out.NodeRegistration.CRISocket != ContainerdSocket { - klog.Infof("Overriding CRISocket from %q to %q. Containerd is only supported container runtime.", out.NodeRegistration.CRISocket, ContainerdSocket) + machine.GetScope().Info("Overriding CRISocket. Containerd is only supported container runtime.", + "provided-container-runtime-socket", out.NodeRegistration.CRISocket, + "required-container-runtime-socket", ContainerdSocket) } out.NodeRegistration.CRISocket = ContainerdSocket @@ -141,7 +146,7 @@ func SetInitConfigurationOverrides(base *kubeadmv1beta1.InitConfiguration) *kube out.NodeRegistration.KubeletExtraArgs = map[string]string{} } if cp, ok := out.NodeRegistration.KubeletExtraArgs["cloud-provider"]; ok && cp != CloudProvider { - klog.Infof("Overriding node's cloud-provider to the required value of %q.", CloudProvider) + machine.GetScope().Info("Overriding node's cloud-provider", "provided-cloud-provider", cp, "required-cloud-provider", CloudProvider) } out.NodeRegistration.KubeletExtraArgs["cloud-provider"] = CloudProvider return out @@ -164,13 +169,17 @@ func SetJoinNodeConfigurationOverrides(caCertHash, bootstrapToken string, machin out.Discovery.BootstrapToken.CACertHashes = append(out.Discovery.BootstrapToken.CACertHashes, caCertHash) if out.NodeRegistration.Name != "" && out.NodeRegistration.Name != HostnameLookup { - klog.Infof("Overriding NodeRegistration name from %q to %q. The node registration needs to be dynamically generated in aws.", out.NodeRegistration.Name, HostnameLookup) + machine.GetScope().Logger.Info("Overriding NodeRegistration name . The node registration needs to be dynamically generated in aws.", + "provided-node-registration-name", out.NodeRegistration.Name, + "required-node-registration-name", HostnameLookup) } out.NodeRegistration.Name = HostnameLookup // TODO(chuckha): This may become a default instead of an override. if out.NodeRegistration.CRISocket != "" && out.NodeRegistration.CRISocket != ContainerdSocket { - klog.Infof("Overriding CRISocket from %q to %q. Containerd is only supported container runtime.", out.NodeRegistration.CRISocket, ContainerdSocket) + machine.GetScope().Logger.Info("Overriding CRISocket. Containerd is only supported container runtime.", + "provided-container-runtime-socket", out.NodeRegistration.CRISocket, + "required-container-runtime-socket", ContainerdSocket) } out.NodeRegistration.CRISocket = ContainerdSocket @@ -178,7 +187,9 @@ func SetJoinNodeConfigurationOverrides(caCertHash, bootstrapToken string, machin out.NodeRegistration.KubeletExtraArgs = map[string]string{} } if cp, ok := out.NodeRegistration.KubeletExtraArgs["cloud-provider"]; ok && cp != CloudProvider { - klog.Infof("Overriding node's cloud-provider to the required value of %q.", CloudProvider) + machine.GetScope().Logger.Info("Overriding node's cloud-provider to the required value", + "provided-cloud-provider", cp, + "required-cloud-provider", CloudProvider) } out.NodeRegistration.KubeletExtraArgs["cloud-provider"] = CloudProvider if !util.IsControlPlaneMachine(machine.GetMachine()) { diff --git a/pkg/cloud/aws/services/kubeadm/aws_defaults_test.go b/pkg/cloud/aws/services/kubeadm/aws_defaults_test.go index 3117fa1ed3..18719da208 100644 --- a/pkg/cloud/aws/services/kubeadm/aws_defaults_test.go +++ b/pkg/cloud/aws/services/kubeadm/aws_defaults_test.go @@ -20,6 +20,8 @@ import ( "fmt" "testing" + "sigs.k8s.io/cluster-api-provider-aws/pkg/cloudtest" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" kubeadmv1beta1 "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1beta1" "sigs.k8s.io/cluster-api-provider-aws/pkg/apis/awsprovider/v1alpha1" @@ -108,6 +110,7 @@ func TestSetJoinNodeConfigurationOverrides(t *testing.T) { }, }, }, + Logger: &cloudtest.Log{}, }, Machine: &clusterv1.Machine{}, }, @@ -126,6 +129,7 @@ func TestSetJoinNodeConfigurationOverrides(t *testing.T) { joinMachine: &jm{ Scope: &actuators.Scope{ ClusterStatus: &v1alpha1.AWSClusterProviderStatus{}, + Logger: &cloudtest.Log{}, }, Machine: &clusterv1.Machine{}, }, @@ -253,11 +257,15 @@ func TestSetDefaultClusterConfiguration(t *testing.T) { func TestSetInitConfigurationOverrides(t *testing.T) { testcasts := []struct { name string + joinMachine *jm initConfiguration *kubeadmv1beta1.InitConfiguration }{ // Assumption: Do not use any default values as input values for testcases. { - name: "assertions pass with an empty configuration", + name: "assertions pass with an empty configuration", + joinMachine: &jm{ + Scope: &actuators.Scope{}, + }, initConfiguration: &kubeadmv1beta1.InitConfiguration{}, }, { @@ -265,6 +273,11 @@ func TestSetInitConfigurationOverrides(t *testing.T) { }, { name: "overrides actually work", + joinMachine: &jm{ + Scope: &actuators.Scope{ + Logger: &cloudtest.Log{}, + }, + }, initConfiguration: &kubeadmv1beta1.InitConfiguration{ NodeRegistration: kubeadmv1beta1.NodeRegistrationOptions{ Name: "moonshine", @@ -279,7 +292,7 @@ func TestSetInitConfigurationOverrides(t *testing.T) { for _, tc := range testcasts { t.Run(tc.name, func(t *testing.T) { - out := kubeadm.SetInitConfigurationOverrides(tc.initConfiguration) + out := kubeadm.SetInitConfigurationOverrides(tc.joinMachine, tc.initConfiguration) // Ignore assertions if the initConfiguration is nil if tc.initConfiguration == nil { @@ -332,6 +345,7 @@ func TestSetClusterConfigurationOverrides(t *testing.T) { }, }, }, + Logger: &cloudtest.Log{}, }, Machine: &clusterv1.Machine{}, }, diff --git a/pkg/cloud/aws/services/kubeadm/scheme.go b/pkg/cloud/aws/services/kubeadm/scheme.go index 2c3a0e0e45..fb5e8cfed6 100644 --- a/pkg/cloud/aws/services/kubeadm/scheme.go +++ b/pkg/cloud/aws/services/kubeadm/scheme.go @@ -20,7 +20,6 @@ import ( "github.com/pkg/errors" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/serializer" - "k8s.io/klog" kubeadmv1beta1 "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1beta1" "k8s.io/kubernetes/cmd/kubeadm/app/util" "sigs.k8s.io/controller-runtime/pkg/runtime/scheme" @@ -34,7 +33,7 @@ func GetCodecs() serializer.CodecFactory { sb.Register(&kubeadmv1beta1.JoinConfiguration{}, &kubeadmv1beta1.InitConfiguration{}, &kubeadmv1beta1.ClusterConfiguration{}) kubeadmScheme, err := sb.Build() if err != nil { - klog.Fatal(err) + panic(err) } return serializer.NewCodecFactory(kubeadmScheme) } diff --git a/pkg/cloudtest/BUILD b/pkg/cloudtest/BUILD index 910346114a..ebe0e86be0 100644 --- a/pkg/cloudtest/BUILD +++ b/pkg/cloudtest/BUILD @@ -5,5 +5,8 @@ go_library( srcs = ["cloudtest.go"], importpath = "sigs.k8s.io/cluster-api-provider-aws/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", + ], ) diff --git a/pkg/cloudtest/cloudtest.go b/pkg/cloudtest/cloudtest.go index 2e4bf4678f..ad72a6c1f2 100644 --- a/pkg/cloudtest/cloudtest.go +++ b/pkg/cloudtest/cloudtest.go @@ -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 } diff --git a/test/e2e/BUILD b/test/e2e/BUILD index 1fbcee9fe4..f6db591fc7 100644 --- a/test/e2e/BUILD +++ b/test/e2e/BUILD @@ -60,6 +60,7 @@ go_test( "//pkg/cloud/aws/services/awserrors:go_default_library", "//pkg/cloud/aws/services/cloudformation:go_default_library", "//pkg/cloud/aws/services/sts:go_default_library", + "//pkg/cloudtest:go_default_library", "//test/e2e/util/kind:go_default_library", "//vendor/github.com/aws/aws-sdk-go/aws:go_default_library", "//vendor/github.com/aws/aws-sdk-go/aws/client:go_default_library", diff --git a/test/e2e/aws_test.go b/test/e2e/aws_test.go index 0343a05487..0e1b794c87 100644 --- a/test/e2e/aws_test.go +++ b/test/e2e/aws_test.go @@ -19,6 +19,7 @@ package e2e_test import ( "bytes" "flag" + "fmt" "io/ioutil" "time" @@ -39,15 +40,13 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/serializer" "k8s.io/client-go/kubernetes" - - "fmt" - capa "sigs.k8s.io/cluster-api-provider-aws/pkg/apis/awsprovider/v1alpha1" "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/aws/actuators" "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/aws/actuators/machine" "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/aws/services/awserrors" "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/aws/services/cloudformation" "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/aws/services/sts" + "sigs.k8s.io/cluster-api-provider-aws/pkg/cloudtest" "sigs.k8s.io/cluster-api-provider-aws/test/e2e/util/kind" capi "sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha1" clientset "sigs.k8s.io/cluster-api/pkg/client/clientset_generated/clientset" @@ -181,7 +180,7 @@ func makeMachine() *capi.Machine { machine.ObjectMeta.Name = controlPlaneName machine.ObjectMeta.Labels[capi.MachineClusterLabelName] = clusterName - awsSpec, err := actuators.MachineConfigFromProviderSpec(nil, machine.Spec.ProviderSpec) + awsSpec, err := actuators.MachineConfigFromProviderSpec(nil, machine.Spec.ProviderSpec, &cloudtest.Log{}) Expect(err).To(BeNil()) awsSpec.KeyName = keyPairName machine.Spec.ProviderSpec.Value, err = capa.EncodeMachineSpec(awsSpec)