Skip to content

Commit

Permalink
Update logs as per capi standards
Browse files Browse the repository at this point in the history
  • Loading branch information
Karthik-K-N committed Sep 29, 2022
1 parent 5b42006 commit d17630a
Show file tree
Hide file tree
Showing 11 changed files with 54 additions and 66 deletions.
4 changes: 2 additions & 2 deletions cloud/scope/powervs_cluster.go
Expand Up @@ -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(
Expand All @@ -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)
Expand Down
8 changes: 4 additions & 4 deletions cloud/scope/powervs_image.go
Expand Up @@ -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")
Expand All @@ -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
}
}
Expand Down Expand Up @@ -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
}

Expand Down
37 changes: 19 additions & 18 deletions cloud/scope/powervs_machine.go
Expand Up @@ -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"

Expand Down Expand Up @@ -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(
Expand All @@ -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)
Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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"]
Expand All @@ -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
}
}
Expand All @@ -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
}
}
Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
}
Expand All @@ -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
}
Expand Down
5 changes: 2 additions & 3 deletions controllers/ibmpowervscluster_controller.go
Expand Up @@ -21,7 +21,6 @@ import (
"strings"
"time"

"github.com/go-logr/logr"
"github.com/pkg/errors"

apierrors "k8s.io/apimachinery/pkg/api/errors"
Expand All @@ -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
Expand All @@ -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{}
Expand All @@ -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{
Expand Down
3 changes: 0 additions & 3 deletions controllers/ibmpowervscluster_controller_test.go
Expand Up @@ -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)))
Expand Down Expand Up @@ -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))
Expand All @@ -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) {
Expand Down
24 changes: 12 additions & 12 deletions controllers/ibmpowervsimage_controller.go
Expand Up @@ -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"
Expand All @@ -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"
Expand All @@ -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
Expand All @@ -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)
Expand All @@ -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{
Expand Down Expand Up @@ -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:
Expand All @@ -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, "", "")
}
}
Expand All @@ -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
Expand Down
3 changes: 0 additions & 3 deletions controllers/ibmpowervsimage_controller_test.go
Expand Up @@ -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)))
Expand Down Expand Up @@ -135,7 +134,6 @@ func TestIBMPowerVSImageReconciler_reconcile(t *testing.T) {
recorder := record.NewFakeRecorder(2)
reconciler = IBMPowerVSImageReconciler{
Client: testEnv.Client,
Log: klogr.New(),
Recorder: recorder,
}
}
Expand Down Expand Up @@ -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{
Expand Down

0 comments on commit d17630a

Please sign in to comment.