From 0258fb66ce9254962bbbdc37b956e1dcef1954b6 Mon Sep 17 00:00:00 2001 From: r Date: Sun, 8 Oct 2023 15:50:34 +0300 Subject: [PATCH] update logging --- controllers/ironicconductor_controller.go | 59 +++++++++++--------- controllers/ironicinspector_controller.go | 10 ++-- controllers/ironicneutronagent_controller.go | 47 +++++++++++----- tests/functional/suite_test.go | 4 +- 4 files changed, 72 insertions(+), 48 deletions(-) diff --git a/controllers/ironicconductor_controller.go b/controllers/ironicconductor_controller.go index 424e3a86..e54f3ed3 100644 --- a/controllers/ironicconductor_controller.go +++ b/controllers/ironicconductor_controller.go @@ -24,6 +24,7 @@ import ( k8s_types "k8s.io/apimachinery/pkg/types" + "github.com/go-logr/logr" routev1 "github.com/openshift/api/route/v1" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" @@ -35,6 +36,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/handler" + "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/reconcile" "sigs.k8s.io/controller-runtime/pkg/source" @@ -63,6 +65,11 @@ type IronicConductorReconciler struct { Scheme *runtime.Scheme } +// getlogger returns a logger object with a prefix of "conroller.name" and aditional controller context fields +func (r *IronicConductorReconciler) GetLogger(ctx context.Context) logr.Logger { + return log.FromContext(ctx).WithName("Controllers").WithName("IronicConductor") +} + // +kubebuilder:rbac:groups=ironic.openstack.org,resources=ironicconductors,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=ironic.openstack.org,resources=ironicconductors/status,verbs=get;update;patch // +kubebuilder:rbac:groups=ironic.openstack.org,resources=ironicconductors/finalizers,verbs=update @@ -87,7 +94,7 @@ type IronicConductorReconciler struct { // Reconcile - func (r *IronicConductorReconciler) Reconcile(ctx context.Context, req ctrl.Request) (result ctrl.Result, _err error) { - log := GetLog(ctx, "IronicConductor") + Log := r.GetLogger(ctx) // Fetch the IronicConductor instance instance := &ironicv1.IronicConductor{} @@ -108,7 +115,7 @@ func (r *IronicConductorReconciler) Reconcile(ctx context.Context, req ctrl.Requ r.Client, r.Kclient, r.Scheme, - l, + Log, ) if err != nil { return ctrl.Result{}, err @@ -200,7 +207,7 @@ func (r *IronicConductorReconciler) SetupWithManager(mgr ctrl.Manager, ctx conte // watch for configmap where the CM owner label AND the CR.Spec.ManagingCrName label matches configMapFn := func(o client.Object) []reconcile.Request { result := []reconcile.Request{} - log := GetLog(ctx, "IronicConductor") + Log := r.GetLogger(ctx) // get all API CRs apis := &ironicv1.IronicConductorList{} @@ -208,7 +215,7 @@ func (r *IronicConductorReconciler) SetupWithManager(mgr ctrl.Manager, ctx conte client.InNamespace(o.GetNamespace()), } if err := r.Client.List(context.Background(), apis, listOpts...); err != nil { - l.Error(err, "Unable to retrieve API CRs %v") + Log.Error(err, "Unable to retrieve API CRs %v") return nil } @@ -223,7 +230,7 @@ func (r *IronicConductorReconciler) SetupWithManager(mgr ctrl.Manager, ctx conte Namespace: o.GetNamespace(), Name: cr.Name, } - log.Info(fmt.Sprintf("ConfigMap object %s and CR %s marked with label: %s", o.GetName(), cr.Name, l)) + Log.Info(fmt.Sprintf("ConfigMap object %s and CR %s marked with label: %s", o.GetName(), cr.Name, l)) result = append(result, reconcile.Request{NamespacedName: name}) } } @@ -251,13 +258,13 @@ func (r *IronicConductorReconciler) SetupWithManager(mgr ctrl.Manager, ctx conte } func (r *IronicConductorReconciler) reconcileDelete(ctx context.Context, instance *ironicv1.IronicConductor, helper *helper.Helper) (ctrl.Result, error) { - log := GetLog(ctx, "IronicConductor") + Log := r.GetLogger(ctx) - log.Info("Reconciling Conductor delete") + Log.Info("Reconciling Conductor delete") // Service is deleted so remove the finalizer. controllerutil.RemoveFinalizer(instance, helper.GetFinalizer()) - log.Info("Reconciled Conductor delete successfully") + Log.Info("Reconciled Conductor delete successfully") return ctrl.Result{}, nil } @@ -268,9 +275,9 @@ func (r *IronicConductorReconciler) reconcileServices( helper *helper.Helper, serviceLabels map[string]string, ) (ctrl.Result, error) { - log := GetLog(ctx, "IronicConductor") + Log := r.GetLogger(ctx) - log.Info("Reconciling Conductor Services") + Log.Info("Reconciling Conductor Services") podList, err := ironicconductor.ConductorPods(ctx, instance, helper, serviceLabels) if err != nil { @@ -304,13 +311,13 @@ func (r *IronicConductorReconciler) reconcileServices( conductorService, ) if err != nil && k8s_errors.IsNotFound(err) { - log.Info(fmt.Sprintf("Service port %s does not exist, creating it", conductorService.Name)) + Log.Info(fmt.Sprintf("Service port %s does not exist, creating it", conductorService.Name)) err = r.Create(ctx, conductorService) if err != nil { return ctrl.Result{}, err } } else { - log.Info(fmt.Sprintf("Service port %s exists, updating it", conductorService.Name)) + Log.Info(fmt.Sprintf("Service port %s exists, updating it", conductorService.Name)) err = r.Update(ctx, conductorService) if err != nil { return ctrl.Result{}, err @@ -347,13 +354,13 @@ func (r *IronicConductorReconciler) reconcileServices( conductorRoute, ) if err != nil && k8s_errors.IsNotFound(err) { - log.Info(fmt.Sprintf("Route %s does not exist, creating it", conductorRoute.Name)) + Log.Info(fmt.Sprintf("Route %s does not exist, creating it", conductorRoute.Name)) err = r.Create(ctx, conductorRoute) if err != nil { return ctrl.Result{}, err } } else { - log.Info(fmt.Sprintf("Route %s exists, updating it", conductorRoute.Name)) + Log.Info(fmt.Sprintf("Route %s exists, updating it", conductorRoute.Name)) err = r.Update(ctx, conductorRoute) if err != nil { return ctrl.Result{}, err @@ -367,14 +374,14 @@ func (r *IronicConductorReconciler) reconcileServices( // TODO: rework this // - log.Info("Reconciled Conductor Services successfully") + Log.Info("Reconciled Conductor Services successfully") return ctrl.Result{}, nil } func (r *IronicConductorReconciler) reconcileNormal(ctx context.Context, instance *ironicv1.IronicConductor, helper *helper.Helper) (ctrl.Result, error) { - log := GetLog(ctx, "IronicConductor") + Log := r.GetLogger(ctx) - log.Info("Reconciling Conductor") + Log.Info("Reconciling Conductor") if ironicv1.GetOwningIronicName(instance) == "" { // Service account, role, binding @@ -619,21 +626,21 @@ func (r *IronicConductorReconciler) reconcileNormal(ctx context.Context, instanc instance.Status.Conditions.MarkTrue(condition.DeploymentReadyCondition, condition.DeploymentReadyMessage) } - log.Info("Reconciled Conductor successfully") + Log.Info("Reconciled Conductor successfully") return ctrl.Result{}, nil } func (r *IronicConductorReconciler) reconcileUpdate(ctx context.Context, instance *ironicv1.IronicConductor, helper *helper.Helper) (ctrl.Result, error) { - // log.Info("Reconciling Service update") + // Log.Info("Reconciling Service update") - // log.Info("Reconciled Service update successfully") + // Log.Info("Reconciled Service update successfully") return ctrl.Result{}, nil } func (r *IronicConductorReconciler) reconcileUpgrade(ctx context.Context, instance *ironicv1.IronicConductor, helper *helper.Helper) (ctrl.Result, error) { - // log.Info("Reconciling Service upgrade") + // Log.Info("Reconciling Service upgrade") - // log.Info("Reconciled Service upgrade successfully") + // Log.Info("Reconciled Service upgrade successfully") return ctrl.Result{}, nil } @@ -649,7 +656,7 @@ func (r *IronicConductorReconciler) generateServiceConfigMaps( // create custom Configmap for ironic-conductor-specific config input // - %-config-data configmap holding custom config for the service's ironic.conf // - + Log := r.GetLogger(ctx) cmLabels := labels.GetLabels(instance, labels.GetGroupLabel(ironic.ServiceName), map[string]string{}) // customData hold any customization for the service. @@ -682,7 +689,7 @@ func (r *IronicConductorReconciler) generateServiceConfigMaps( } dhcpRanges, err := ironic.PrefixOrNetmaskFromCIDR(instance.Spec.DHCPRanges) if err != nil { - l.Error(err, "Failed to get Prefix or Netmask from IP network Prefix (CIDR)") + Log.Error(err, "Failed to get Prefix or Netmask from IP network Prefix (CIDR)") } templateParameters["DHCPRanges"] = dhcpRanges templateParameters["Standalone"] = instance.Spec.Standalone @@ -730,7 +737,7 @@ func (r *IronicConductorReconciler) createHashOfInputHashes( instance *ironicv1.IronicConductor, envVars map[string]env.Setter, ) (string, bool, error) { - log := GetLog(ctx, "IronicConductor") + Log := r.GetLogger(ctx) var hashMap map[string]string changed := false @@ -741,7 +748,7 @@ func (r *IronicConductorReconciler) createHashOfInputHashes( } if hashMap, changed = util.SetHash(instance.Status.Hash, common.InputHashName, hash); changed { instance.Status.Hash = hashMap - log.Info(fmt.Sprintf("Input maps hash %s - %s", common.InputHashName, hash)) + Log.Info(fmt.Sprintf("Input maps hash %s - %s", common.InputHashName, hash)) } return hash, changed, nil } diff --git a/controllers/ironicinspector_controller.go b/controllers/ironicinspector_controller.go index 77eed83f..05a1d5e6 100644 --- a/controllers/ironicinspector_controller.go +++ b/controllers/ironicinspector_controller.go @@ -22,7 +22,6 @@ import ( "strings" "time" - "github.com/cloudflare/cfssl/log" "github.com/go-logr/logr" ironic "github.com/openstack-k8s-operators/ironic-operator/pkg/ironic" ironicinspector "github.com/openstack-k8s-operators/ironic-operator/pkg/ironicinspector" @@ -59,6 +58,7 @@ import ( "k8s.io/client-go/kubernetes" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/log" corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" @@ -270,11 +270,11 @@ func (r *IronicInspectorReconciler) SetupWithManager( client.InNamespace(o.GetNamespace()), } if err := r.Client.List( - context.Background(), + ctx, apis, listOpts...); err != nil { - log.Error(err, "Unable to retrieve API CRs %v") + Log.Error(err, "Unable to retrieve API CRs %v") return nil } @@ -1131,7 +1131,7 @@ func (r *IronicInspectorReconciler) generateServiceConfigMaps( instance, labels.GetGroupLabel(ironic.ServiceName), map[string]string{}) - + Log := r.GetLogger(ctx) // customData hold any customization for the service. // custom.conf is going to /etc/ironic-inspector/inspector.conf.d // all other files get placed into /etc/ironic-inspector to allow @@ -1176,7 +1176,7 @@ func (r *IronicInspectorReconciler) generateServiceConfigMaps( } dhcpRanges, err := ironic.PrefixOrNetmaskFromCIDR(instance.Spec.DHCPRanges) if err != nil { - log.Error(err, "unable to get Prefix or Netmask from IP network Prefix (CIDR)") + Log.Error(err, "unable to get Prefix or Netmask from IP network Prefix (CIDR)") } templateParameters["DHCPRanges"] = dhcpRanges templateParameters["Standalone"] = instance.Spec.Standalone diff --git a/controllers/ironicneutronagent_controller.go b/controllers/ironicneutronagent_controller.go index 5751bcae..36e4cc9a 100644 --- a/controllers/ironicneutronagent_controller.go +++ b/controllers/ironicneutronagent_controller.go @@ -31,8 +31,9 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sigs.k8s.io/controller-runtime/pkg/log" - "github.com/cloudflare/cfssl/log" + "github.com/go-logr/logr" rabbitmqv1 "github.com/openstack-k8s-operators/infra-operator/apis/rabbitmq/v1beta1" ironicv1 "github.com/openstack-k8s-operators/ironic-operator/api/v1beta1" ironic "github.com/openstack-k8s-operators/ironic-operator/pkg/ironic" @@ -59,6 +60,11 @@ type IronicNeutronAgentReconciler struct { Scheme *runtime.Scheme } +// getlogger returns a logger object with a prefix of "conroller.name" and aditional controller context fields +func (r *IronicNeutronAgentReconciler) GetLogger(ctx context.Context) logr.Logger { + return log.FromContext(ctx).WithName("Controllers").WithName("IronicNeutronAgent") +} + // +kubebuilder:rbac:groups=ironic.openstack.org,resources=ironicneutronagents,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=ironic.openstack.org,resources=ironicneutronagents/status,verbs=get;update;patch // +kubebuilder:rbac:groups=ironic.openstack.org,resources=ironicneutronagents/finalizers,verbs=update @@ -216,6 +222,8 @@ func (r *IronicNeutronAgentReconciler) reconcileTransportURL( // Create RabbitMQ transport URL CR and get the actual URL from the // associted secret that is created // + Log := r.GetLogger(ctx) + transportURL, op, err := ironic.TransportURLCreateOrUpdate( instance.Name, instance.Namespace, @@ -234,13 +242,13 @@ func (r *IronicNeutronAgentReconciler) reconcileTransportURL( return ctrl.Result{}, err } if op != controllerutil.OperationResultNone { - log.Info(fmt.Sprintf( + Log.Info(fmt.Sprintf( "TransportURL %s successfully reconciled - operation: %s", transportURL.Name, string(op))) } instance.Status.TransportURLSecret = transportURL.Status.SecretName if instance.Status.TransportURLSecret == "" { - log.Info(fmt.Sprintf( + Log.Info(fmt.Sprintf( "Waiting for TransportURL %s secret to be created", transportURL.Name)) instance.Status.Conditions.Set(condition.FalseCondition( @@ -381,7 +389,8 @@ func (r *IronicNeutronAgentReconciler) reconcileNormal( instance *ironicv1.IronicNeutronAgent, helper *helper.Helper, ) (ctrl.Result, error) { - log.Info("Reconciling IronicNeutronAgent") + Log := r.GetLogger(ctx) + Log.Info("Reconciling IronicNeutronAgent") if ironicv1.GetOwningIronicName(instance) == "" { // Service account, role, binding @@ -460,7 +469,7 @@ func (r *IronicNeutronAgentReconciler) reconcileNormal( return ctrlResult, nil } - log.Info("Reconciled IronicNeutronAgent Service successfully") + Log.Info("Reconciled IronicNeutronAgent Service successfully") return ctrl.Result{}, nil } @@ -470,8 +479,10 @@ func (r *IronicNeutronAgentReconciler) reconcileInit( helper *helper.Helper, serviceLabels map[string]string, ) (ctrl.Result, error) { - log.Info("Reconciling IronicNeutronAgent init") - log.Info("Reconciled IronicNeutronAgent init successfully") + Log := r.GetLogger(ctx) + + Log.Info("Reconciling IronicNeutronAgent init") + Log.Info("Reconciled IronicNeutronAgent init successfully") return ctrl.Result{}, nil } @@ -481,10 +492,12 @@ func (r *IronicNeutronAgentReconciler) reconcileDelete( instance *ironicv1.IronicNeutronAgent, helper *helper.Helper, ) (ctrl.Result, error) { - log.Info("Reconciling IronicNeutronAgent delete") + Log := r.GetLogger(ctx) + + Log.Info("Reconciling IronicNeutronAgent delete") // Service is deleted so remove the finalizer. controllerutil.RemoveFinalizer(instance, helper.GetFinalizer()) - log.Info("Reconciled IronicNeutronAgent delete successfully") + Log.Info("Reconciled IronicNeutronAgent delete successfully") return ctrl.Result{}, nil } @@ -494,8 +507,10 @@ func (r *IronicNeutronAgentReconciler) reconcileUpdate( instance *ironicv1.IronicNeutronAgent, helper *helper.Helper, ) (ctrl.Result, error) { - log.Info("Reconciling IronicNeutronAgent update") - log.Info("Reconciled IronicNeutronAgent update successfully") + Log := r.GetLogger(ctx) + + Log.Info("Reconciling IronicNeutronAgent update") + Log.Info("Reconciled IronicNeutronAgent update successfully") return ctrl.Result{}, nil } @@ -505,8 +520,10 @@ func (r *IronicNeutronAgentReconciler) reconcileUpgrade( instance *ironicv1.IronicNeutronAgent, helper *helper.Helper, ) (ctrl.Result, error) { - log.Info("Reconciling IronicNeutronAgent upgrade") - log.Info("Reconciled IronicNeutronAgent upgrade successfully") + Log := r.GetLogger(ctx) + + Log.Info("Reconciling IronicNeutronAgent upgrade") + Log.Info("Reconciled IronicNeutronAgent upgrade successfully") return ctrl.Result{}, nil } @@ -585,6 +602,8 @@ func (r *IronicNeutronAgentReconciler) createHashOfInputHashes( instance *ironicv1.IronicNeutronAgent, envVars map[string]env.Setter, ) (string, bool, error) { + Log := r.GetLogger(ctx) + var hashMap map[string]string changed := false mergedMapVars := env.MergeEnvs([]corev1.EnvVar{}, envVars) @@ -594,7 +613,7 @@ func (r *IronicNeutronAgentReconciler) createHashOfInputHashes( } if hashMap, changed = util.SetHash(instance.Status.Hash, common.InputHashName, hash); changed { instance.Status.Hash = hashMap - log.Info(fmt.Sprintf("Input maps hash %s - %s", common.InputHashName, hash)) + Log.Info(fmt.Sprintf("Input maps hash %s - %s", common.InputHashName, hash)) } return hash, changed, nil } diff --git a/tests/functional/suite_test.go b/tests/functional/suite_test.go index 1892f9aa..b4aff0cb 100644 --- a/tests/functional/suite_test.go +++ b/tests/functional/suite_test.go @@ -163,7 +163,6 @@ var _ = BeforeSuite(func() { Client: k8sManager.GetClient(), Scheme: k8sManager.GetScheme(), Kclient: kclient, - Log: ctrl.Log.WithName("controllers").WithName("IronicNeutronAgent"), }).SetupWithManager(k8sManager) Expect(err).ToNot(HaveOccurred()) @@ -171,8 +170,7 @@ var _ = BeforeSuite(func() { Client: k8sManager.GetClient(), Scheme: k8sManager.GetScheme(), Kclient: kclient, - Log: ctrl.Log.WithName("controllers").WithName("IronicInspector"), - }).SetupWithManager(k8sManager) + }).SetupWithManager(k8sManager, context.Background()) Expect(err).ToNot(HaveOccurred()) err = (&controllers.IronicConductorReconciler{