Update logs as per capi standards (#895)

Author: Karthik-K-N

cloud/scope/powervs_cluster.go

@@ -112,7 +112,7 @@ func NewPowerVSClusterScope(params PowerVSClusterScopeParams) (scope *PowerVSClu
 		if err := rc.SetServiceURL(rcEndpoint); err != nil {
 			return nil, errors.Wrap(err, "failed to set resource controller endpoint")
 		}
-		scope.Logger.V(3).Info("overriding the default resource controller endpoint")
+		scope.Logger.V(3).Info("Overriding the default resource controller endpoint")
 	}
 
 	res, _, err := rc.GetResourceInstance(
@@ -135,7 +135,7 @@ func NewPowerVSClusterScope(params PowerVSClusterScopeParams) (scope *PowerVSClu
 	// Fetch the service endpoint.
 	if svcEndpoint := endpoints.FetchPVSEndpoint(endpoints.CostructRegionFromZone(*res.RegionID), params.ServiceEndpoint); svcEndpoint != "" {
 		options.IBMPIOptions.URL = svcEndpoint
-		scope.Logger.V(3).Info("overriding the default powervs service endpoint")
+		scope.Logger.V(3).Info("Overriding the default powervs service endpoint")
 	}
 
 	c, err := powervs.NewService(options)

cloud/scope/powervs_image.go

@@ -167,7 +167,7 @@ func (i *PowerVSImageScope) CreateImageCOSBucket() (*models.ImageReference, *mod
 
 	imageReply, err := i.ensureImageUnique(m.Name)
 	if err != nil {
-		record.Warnf(i.IBMPowerVSImage, "FailedRetriveImage", "Failed to retrieve image %q", m.Name)
+		record.Warnf(i.IBMPowerVSImage, "FailedRetrieveImage", "Failed to retrieve image %q", m.Name)
 		return nil, nil, err
 	} else if imageReply != nil {
 		i.Info("Image already exists")
@@ -176,7 +176,7 @@ func (i *PowerVSImageScope) CreateImageCOSBucket() (*models.ImageReference, *mod
 
 	if lastJob, _ := i.GetImportJob(); lastJob != nil {
 		if *lastJob.Status.State != "completed" && *lastJob.Status.State != "failed" {
-			i.Info("Previous import job not yet fininshed - " + *lastJob.Status.State)
+			i.Info("Previous import job not yet finished", "state", *lastJob.Status.State)
 			return nil, nil, nil
 		}
 	}
@@ -229,10 +229,10 @@ func (i *PowerVSImageScope) GetImportJob() (*models.Job, error) {
 // DeleteImportJob will delete the image import job.
 func (i *PowerVSImageScope) DeleteImportJob() error {
 	if err := i.IBMPowerVSClient.DeleteJob(i.IBMPowerVSImage.Status.JobID); err != nil {
-		record.Warnf(i.IBMPowerVSImage, "FailedDeleteImageImoprtJob", "Failed image import job deletion - %v", err)
+		record.Warnf(i.IBMPowerVSImage, "FailedDeleteImageImportJob", "Failed image import job deletion - %v", err)
 		return err
 	}
-	record.Eventf(i.IBMPowerVSImage, "SuccessfulDeleteImageImoprtJob", "Deleted image import job %q", i.IBMPowerVSImage.Status.JobID)
+	record.Eventf(i.IBMPowerVSImage, "SuccessfulDeleteImageImportJob", "Deleted image import job %q", i.IBMPowerVSImage.Status.JobID)
 	return nil
 }
 

cloud/scope/powervs_machine.go

@@ -35,6 +35,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/tools/cache"
+	"k8s.io/klog/v2"
 	"k8s.io/klog/v2/klogr"
 	"k8s.io/utils/pointer"
 
@@ -135,7 +136,7 @@ func NewPowerVSMachineScope(params PowerVSMachineScopeParams) (scope *PowerVSMac
 		if err := rc.SetServiceURL(rcEndpoint); err != nil {
 			return nil, errors.Wrap(err, "failed to set resource controller endpoint")
 		}
-		scope.Logger.V(3).Info("overriding the default resource controller endpoint")
+		scope.Logger.V(3).Info("Overriding the default resource controller endpoint")
 	}
 
 	res, _, err := rc.GetResourceInstance(
@@ -162,7 +163,7 @@ func NewPowerVSMachineScope(params PowerVSMachineScopeParams) (scope *PowerVSMac
 	// Fetch the service endpoint.
 	if svcEndpoint := endpoints.FetchPVSEndpoint(region, params.ServiceEndpoint); svcEndpoint != "" {
 		serviceOptions.IBMPIOptions.URL = svcEndpoint
-		scope.Logger.V(3).Info("overriding the default powervs service endpoint")
+		scope.Logger.V(3).Info("Overriding the default powervs service endpoint")
 	}
 
 	c, err := powervs.NewService(serviceOptions)
@@ -226,7 +227,7 @@ func (m *PowerVSMachineScope) CreateMachine() (*models.PVMInstanceReference, err
 
 	networkID, err := getNetworkID(s.Network, m)
 	if err != nil {
-		record.Warnf(m.IBMPowerVSMachine, "FailedRetriveNetwork", "Failed network retrival - %v", err)
+		record.Warnf(m.IBMPowerVSMachine, "FailedRetrieveNetwork", "Failed network retrieval - %v", err)
 		return nil, fmt.Errorf("error getting network ID: %v", err)
 	}
 
@@ -286,7 +287,7 @@ func (m *PowerVSMachineScope) GetBootstrapData() (string, error) {
 	secret := &corev1.Secret{}
 	key := types.NamespacedName{Namespace: m.Machine.Namespace, Name: *m.Machine.Spec.Bootstrap.DataSecretName}
 	if err := m.Client.Get(context.TODO(), key, secret); err != nil {
-		return "", errors.Wrapf(err, "failed to retrieve bootstrap data secret for IBMPowerVSMachine %s/%s", m.Machine.Namespace, m.Machine.Name)
+		return "", errors.Wrapf(err, "failed to retrieve bootstrap data secret for IBMPowerVSMachine %v", klog.KObj(m.Machine))
 	}
 
 	value, ok := secret.Data["value"]
@@ -303,12 +304,12 @@ func getImageID(image *infrav1beta1.IBMPowerVSResourceReference, m *PowerVSMachi
 	} else if image.Name != nil {
 		images, err := m.GetImages()
 		if err != nil {
-			m.Logger.Error(err, "failed to get images")
+			m.Logger.Error(err, "Failed to get images")
 			return nil, err
 		}
 		for _, img := range images.Images {
 			if *image.Name == *img.Name {
-				m.Logger.Info("image found with ID", "Image", *image.Name, "ID", *img.ImageID)
+				m.Logger.Info("Image found with ID", "Image", *image.Name, "ID", *img.ImageID)
 				return img.ImageID, nil
 			}
 		}
@@ -329,12 +330,12 @@ func getNetworkID(network infrav1beta1.IBMPowerVSResourceReference, m *PowerVSMa
 	} else if network.Name != nil {
 		networks, err := m.GetNetworks()
 		if err != nil {
-			m.Logger.Error(err, "failed to get networks")
+			m.Logger.Error(err, "Failed to get networks")
 			return nil, err
 		}
 		for _, nw := range networks.Networks {
 			if *network.Name == *nw.Name {
-				m.Logger.Info("network found with ID", "Network", *network.Name, "ID", *nw.NetworkID)
+				m.Logger.Info("Network found with ID", "Network", *network.Name, "ID", *nw.NetworkID)
 				return nw.NetworkID, nil
 			}
 		}
@@ -429,10 +430,10 @@ func (m *PowerVSMachineScope) SetAddresses(instance *models.PVMInstance) {
 	// Look for DHCP IP from the cache
 	obj, exists, err := m.DHCPIPCacheStore.GetByKey(*instance.ServerName)
 	if err != nil {
-		m.Error(err, "failed to fetch the DHCP IP address from cache store", "VM", *instance.ServerName)
+		m.Error(err, "Failed to fetch the DHCP IP address from cache store", "VM", *instance.ServerName)
 	}
 	if exists {
-		m.Info("found IP for VM from DHCP cache", "IP", obj.(powervs.VMip).IP, "VM", *instance.ServerName)
+		m.Info("Found IP for VM from DHCP cache", "IP", obj.(powervs.VMip).IP, "VM", *instance.ServerName)
 		addresses = append(addresses, corev1.NodeAddress{
 			Type:    corev1.NodeInternalIP,
 			Address: obj.(powervs.VMip).IP,
@@ -443,25 +444,25 @@ func (m *PowerVSMachineScope) SetAddresses(instance *models.PVMInstance) {
 	// Fetch the VM network ID
 	networkID, err := getNetworkID(m.IBMPowerVSMachine.Spec.Network, m)
 	if err != nil {
-		m.Error(err, "failed to fetch network id from network resource", "VM", *instance.ServerName)
+		m.Error(err, "Failed to fetch network id from network resource", "VM", *instance.ServerName)
 		return
 	}
 	// Fetch the details of the network attached to the VM
 	var pvmNetwork *models.PVMInstanceNetwork
 	for _, network := range instance.Networks {
 		if network.NetworkID == *networkID {
 			pvmNetwork = network
-			m.Info("found network attached to VM", "Network ID", network.NetworkID, "VM", *instance.ServerName)
+			m.Info("Found network attached to VM", "Network ID", network.NetworkID, "VM", *instance.ServerName)
 		}
 	}
 	if pvmNetwork == nil {
-		m.Info("failed to get network attached to VM", "VM", *instance.ServerName, "Network ID", *networkID)
+		m.Info("Failed to get network attached to VM", "VM", *instance.ServerName, "Network ID", *networkID)
 		return
 	}
 	// Get all the DHCP servers
 	dhcpServer, err := m.IBMPowerVSClient.GetAllDHCPServers()
 	if err != nil {
-		m.Error(err, "failed to get DHCP server")
+		m.Error(err, "Failed to get DHCP server")
 		return
 	}
 	// Get the Details of DHCP server associated with the network
@@ -471,7 +472,7 @@ func (m *PowerVSMachineScope) SetAddresses(instance *models.PVMInstance) {
 			m.Info("found DHCP server for network", "DHCP server ID", *server.ID, "network ID", *networkID)
 			dhcpServerDetails, err = m.IBMPowerVSClient.GetDHCPServer(*server.ID)
 			if err != nil {
-				m.Error(err, "failed to get DHCP server details", "DHCP server ID", *server.ID)
+				m.Error(err, "Failed to get DHCP server details", "DHCP server ID", *server.ID)
 				return
 			}
 			break
@@ -487,14 +488,14 @@ func (m *PowerVSMachineScope) SetAddresses(instance *models.PVMInstance) {
 	var internalIP *string
 	for _, lease := range dhcpServerDetails.Leases {
 		if *lease.InstanceMacAddress == pvmNetwork.MacAddress {
-			m.Info("found internal ip for VM from DHCP lease", "IP", *lease.InstanceIP, "VM", *instance.ServerName)
+			m.Info("Found internal ip for VM from DHCP lease", "IP", *lease.InstanceIP, "VM", *instance.ServerName)
 			internalIP = lease.InstanceIP
 			break
 		}
 	}
 	if internalIP == nil {
 		errStr := fmt.Errorf("internal IP is nil")
-		m.Error(errStr, "failed to get internal IP, DHCP lease not found for VM with MAC in DHCP network", "VM", *instance.ServerName,
+		m.Error(errStr, "Failed to get internal IP, DHCP lease not found for VM with MAC in DHCP network", "VM", *instance.ServerName,
 			"MAC", pvmNetwork.MacAddress, "DHCP server ID", *dhcpServerDetails.ID)
 		return
 	}
@@ -509,7 +510,7 @@ func (m *PowerVSMachineScope) SetAddresses(instance *models.PVMInstance) {
 		IP:   *internalIP,
 	})
 	if err != nil {
-		m.Error(err, "failed to update the DHCP cache store with the IP", "VM", *instance.ServerName, "IP", *internalIP)
+		m.Error(err, "Failed to update the DHCP cache store with the IP", "VM", *instance.ServerName, "IP", *internalIP)
 	}
 	m.IBMPowerVSMachine.Status.Addresses = addresses
 }

controllers/ibmpowervscluster_controller.go

@@ -21,7 +21,6 @@ import (
 	"strings"
 	"time"
 
-	"github.com/go-logr/logr"
 	"github.com/pkg/errors"
 
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -46,7 +45,6 @@ import (
 // IBMPowerVSClusterReconciler reconciles a IBMPowerVSCluster object.
 type IBMPowerVSClusterReconciler struct {
 	client.Client
-	Log             logr.Logger
 	Recorder        record.EventRecorder
 	ServiceEndpoint []endpoints.ServiceEndpoint
 	Scheme          *runtime.Scheme
@@ -57,7 +55,7 @@ type IBMPowerVSClusterReconciler struct {
 
 // Reconcile implements controller runtime Reconciler interface and handles reconcileation logic for IBMPowerVSCluster.
 func (r *IBMPowerVSClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Result, reterr error) {
-	log := r.Log.WithValues("ibmpowervscluster", req.NamespacedName)
+	log := ctrl.LoggerFrom(ctx)
 
 	// Fetch the IBMPowerVSCluster instance.
 	ibmCluster := &infrav1beta1.IBMPowerVSCluster{}
@@ -78,6 +76,7 @@ func (r *IBMPowerVSClusterReconciler) Reconcile(ctx context.Context, req ctrl.Re
 		log.Info("Cluster Controller has not yet set OwnerRef")
 		return ctrl.Result{}, nil
 	}
+	log = log.WithValues("cluster", cluster.Name)
 
 	// Create the scope.
 	clusterScope, err := scope.NewPowerVSClusterScope(scope.PowerVSClusterScopeParams{

controllers/ibmpowervscluster_controller_test.go

@@ -80,7 +80,6 @@ func TestIBMPowerVSClusterReconciler_Reconcile(t *testing.T) {
 			g := NewWithT(t)
 			reconciler := &IBMPowerVSClusterReconciler{
 				Client: testEnv.Client,
-				Log:    klogr.New(),
 			}
 
 			ns, err := testEnv.CreateNamespace(ctx, fmt.Sprintf("namespace-%s", util.RandomString(5)))
@@ -159,7 +158,6 @@ func TestIBMPowerVSClusterReconciler_reconcile(t *testing.T) {
 			g := NewWithT(t)
 			reconciler := &IBMPowerVSClusterReconciler{
 				Client: testEnv.Client,
-				Log:    klogr.New(),
 			}
 			_ = reconciler.reconcile(tc.powervsClusterScope)
 			g.Expect(tc.powervsClusterScope.IBMPowerVSCluster.Status.Ready).To(Equal(tc.clusterStatus))
@@ -175,7 +173,6 @@ func TestIBMPowerVSClusterReconciler_delete(t *testing.T) {
 	)
 	reconciler = IBMPowerVSClusterReconciler{
 		Client: testEnv.Client,
-		Log:    klogr.New(),
 	}
 	t.Run("Reconciling delete IBMPowerVSCluster", func(t *testing.T) {
 		t.Run("Should reconcile successfully if no descendants are found", func(t *testing.T) {

controllers/ibmpowervsimage_controller.go

@@ -21,7 +21,6 @@ import (
 	"fmt"
 	"time"
 
-	"github.com/go-logr/logr"
 	"github.com/pkg/errors"
 
 	"github.com/IBM-Cloud/power-go-client/power/models"
@@ -30,6 +29,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/tools/record"
+	"k8s.io/klog/v2"
 
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -49,7 +49,6 @@ import (
 // IBMPowerVSImageReconciler reconciles a IBMPowerVSImage object.
 type IBMPowerVSImageReconciler struct {
 	client.Client
-	Log             logr.Logger
 	Recorder        record.EventRecorder
 	ServiceEndpoint []endpoints.ServiceEndpoint
 	Scheme          *runtime.Scheme
@@ -58,9 +57,9 @@ type IBMPowerVSImageReconciler struct {
 //+kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=ibmpowervsimages,verbs=get;list;watch;create;update;patch;delete
 //+kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=ibmpowervsimages/status,verbs=get;update;patch
 
-// Reconcile implements controller runtime Reconciler interface and handles reconcileation logic for IBMPowerVSImage.
+// Reconcile implements controller runtime Reconciler interface and handles reconciliation logic for IBMPowerVSImage.
 func (r *IBMPowerVSImageReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Result, reterr error) {
-	log := r.Log.WithValues("ibmpowervsimage", req.NamespacedName)
+	log := ctrl.LoggerFrom(ctx)
 
 	ibmImage := &infrav1beta1.IBMPowerVSImage{}
 	err := r.Get(ctx, req.NamespacedName, ibmImage)
@@ -75,6 +74,7 @@ func (r *IBMPowerVSImageReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 	if err != nil {
 		return ctrl.Result{}, err
 	}
+	log = log.WithValues("cluster", cluster.Name)
 
 	// Create the scope.
 	imageScope, err := scope.NewPowerVSImageScope(scope.PowerVSImageScopeParams{
@@ -174,9 +174,9 @@ func reconcileImage(img *models.ImageReference, imageScope *scope.PowerVSImageSc
 		}
 
 		imageScope.SetImageID(image.ImageID)
-		imageScope.Info("ImageID - " + imageScope.GetImageID())
+		imageScope.Info("ImageID", imageScope.GetImageID())
 		imageScope.SetImageState(image.State)
-		imageScope.Info("ImageState - " + image.State)
+		imageScope.Info("ImageState", image.State)
 
 		switch imageScope.GetImageState() {
 		case infrav1beta1.PowerVSImageStateQue:
@@ -190,7 +190,7 @@ func reconcileImage(img *models.ImageReference, imageScope *scope.PowerVSImageSc
 			conditions.MarkTrue(imageScope.IBMPowerVSImage, infrav1beta1.ImageReadyCondition)
 		default:
 			imageScope.SetNotReady()
-			imageScope.Info("PowerVS image state is undefined", "state", &image.State, "image-id", imageScope.GetImageID())
+			imageScope.Info("PowerVS image state is undefined", "state", image.State, "image-id", imageScope.GetImageID())
 			conditions.MarkUnknown(imageScope.IBMPowerVSImage, infrav1beta1.ImageReadyCondition, "", "")
 		}
 	}
@@ -215,22 +215,22 @@ func (r *IBMPowerVSImageReconciler) reconcileDelete(scope *scope.PowerVSImageSco
 	}()
 
 	if scope.GetImageID() == "" {
-		scope.Info("ImageID is not yet set, hence not invoking the powervs API to delete the image")
+		scope.Info("ImageID is not yet set, hence not invoking the PowerVS API to delete the image")
 		if scope.GetJobID() == "" {
-			scope.Info("JobID is not yet set, hence not invoking the powervs API to delete the image import job")
+			scope.Info("JobID is not yet set, hence not invoking the PowerVS API to delete the image import job")
 			return ctrl.Result{}, nil
 		}
 		if err := scope.DeleteImportJob(); err != nil {
-			scope.Info("error deleting IBMPowerVSImage Import Job")
+			scope.Error(err, "Error deleting IBMPowerVSImage Import Job")
 			return ctrl.Result{}, errors.Wrapf(err, "error deleting IBMPowerVSImage Import Job")
 		}
 		return ctrl.Result{}, nil
 	}
 
 	if scope.IBMPowerVSImage.Spec.DeletePolicy != string(infrav1beta1.DeletePolicyRetain) {
 		if err := scope.DeleteImage(); err != nil {
-			scope.Info("error deleting IBMPowerVSImage")
-			return ctrl.Result{}, errors.Wrapf(err, "error deleting IBMPowerVSImage %s/%s", scope.IBMPowerVSImage.Namespace, scope.IBMPowerVSImage.Name)
+			scope.Error(err, "Error deleting IBMPowerVSImage")
+			return ctrl.Result{}, errors.Wrapf(err, "error deleting IBMPowerVSImage %v", klog.KObj(scope.IBMPowerVSImage))
 		}
 	}
 	return ctrl.Result{}, nil

controllers/ibmpowervsimage_controller_test.go

@@ -77,7 +77,6 @@ func TestIBMPowerVSImageReconciler_Reconcile(t *testing.T) {
 			g := NewWithT(t)
 			reconciler := &IBMPowerVSImageReconciler{
 				Client: testEnv.Client,
-				Log:    klogr.New(),
 			}
 
 			ns, err := testEnv.CreateNamespace(ctx, fmt.Sprintf("namespace-%s", util.RandomString(5)))
@@ -135,7 +134,6 @@ func TestIBMPowerVSImageReconciler_reconcile(t *testing.T) {
 		recorder := record.NewFakeRecorder(2)
 		reconciler = IBMPowerVSImageReconciler{
 			Client:   testEnv.Client,
-			Log:      klogr.New(),
 			Recorder: recorder,
 		}
 	}
@@ -335,7 +333,6 @@ func TestIBMPowerVSImageReconciler_delete(t *testing.T) {
 		recorder := record.NewFakeRecorder(2)
 		reconciler = IBMPowerVSImageReconciler{
 			Client:   testEnv.Client,
-			Log:      klogr.New(),
 			Recorder: recorder,
 		}
 		imageScope = &scope.PowerVSImageScope{