Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove use of defer in printing logs #87

Merged
merged 3 commits into from
Mar 29, 2023
Merged

Remove use of defer in printing logs #87

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7d247b5
remove use of defer in printing logs
rishabh-11 Mar 27, 2023
d5d27b3
address review comments
rishabh-11 Mar 28, 2023
d463096
revert log to higher level
rishabh-11 Mar 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 10 additions & 11 deletions pkg/azure/core.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,9 @@ func NewAzureDriver(spi spi.SessionProviderInterface) *MachinePlugin {
// It is optionally expected by the safety controller to use an identification mechanisms to map the VM Created by a providerSpec.
// These could be done using tag(s)/resource-groups etc.
// This logic is used by safety controller to delete orphan VMs which are not backed by any machine CRD
//
func (d *MachinePlugin) CreateMachine(ctx context.Context, req *driver.CreateMachineRequest) (*driver.CreateMachineResponse, error) {
// Log messages to track request
klog.V(2).Infof("Machine creation request has been recieved for %q", req.Machine.Name)
klog.V(2).Infof("Machine creation request has been received for %q", req.Machine.Name)
defer klog.V(2).Infof("Machine creation request has been processed for %q", req.Machine.Name)

// Check if provider in the MachineClass is the provider we support
Expand Down Expand Up @@ -114,7 +113,7 @@ func (d *MachinePlugin) CreateMachine(ctx context.Context, req *driver.CreateMac
//
func (d *MachinePlugin) DeleteMachine(ctx context.Context, req *driver.DeleteMachineRequest) (*driver.DeleteMachineResponse, error) {
// Log messages to track delete request
klog.V(2).Infof("Machine deletion request has been recieved for %q", req.Machine.Name)
klog.V(2).Infof("Machine deletion request has been received for %q", req.Machine.Name)
defer klog.V(2).Infof("Machine deletion request has been processed for %q", req.Machine.Name)

// Check if provider in the MachineClass is the provider we support
Expand Down Expand Up @@ -179,8 +178,8 @@ func (d *MachinePlugin) DeleteMachine(ctx context.Context, req *driver.DeleteMac
// The request should return a NOT_FOUND (5) status error code if the machine is not existing
func (d *MachinePlugin) GetMachineStatus(ctx context.Context, req *driver.GetMachineStatusRequest) (*driver.GetMachineStatusResponse, error) {
// Log messages to track start and end of request
klog.V(4).Infof("Get request has been recieved for %q", req.Machine.Name)
defer klog.V(2).Infof("Machine get request has been processed successfully for %q", req.Machine.Name)
klog.V(4).Infof("Machine get request has been received for %q", req.Machine.Name)
defer klog.V(2).Infof("Machine get request has been processed for %q", req.Machine.Name)

// Check if provider in the MachineClass is the provider we support
if req.MachineClass.Provider != ProviderAzure {
Expand All @@ -204,6 +203,7 @@ func (d *MachinePlugin) GetMachineStatus(ctx context.Context, req *driver.GetMac
}
}
err = fmt.Errorf("machine '%s' not found", req.Machine.Name)

return nil, status.Error(codes.NotFound, err.Error())
}

Expand All @@ -222,8 +222,8 @@ func (d *MachinePlugin) GetMachineStatus(ctx context.Context, req *driver.GetMac
//
func (d *MachinePlugin) ListMachines(ctx context.Context, req *driver.ListMachinesRequest) (*driver.ListMachinesResponse, error) {
// Log messages to track start and end of request
klog.V(2).Infof("List machines request has been recieved for %q", req.MachineClass.Name)
defer klog.V(2).Infof("List machines request has been recieved for %q", req.MachineClass.Name)
klog.V(2).Infof("List machines request has been received for %q", req.MachineClass.Name)
himanshu-kun marked this conversation as resolved.
Show resolved Hide resolved
defer klog.V(2).Infof("List machines request has been processed for %q", req.MachineClass.Name)

// Check if provider in the MachineClass is the provider we support
if req.MachineClass.Provider != ProviderAzure {
Expand Down Expand Up @@ -281,6 +281,7 @@ func (d *MachinePlugin) ListMachines(ctx context.Context, req *driver.ListMachin
mergeIntoResult(listOfVMsByDisk)

OnARMAPISuccess(prometheusServiceVM, "VM.List")

return &driver.ListMachinesResponse{MachineList: listOfVMs}, nil
}

Expand All @@ -291,10 +292,9 @@ func (d *MachinePlugin) ListMachines(ctx context.Context, req *driver.ListMachin
//
// RESPONSE PARAMETERS (driver.GetVolumeIDsResponse)
// VolumeIDs []string VolumeIDs is a repeated list of VolumeIDs.
//
func (d *MachinePlugin) GetVolumeIDs(ctx context.Context, req *driver.GetVolumeIDsRequest) (*driver.GetVolumeIDsResponse, error) {
// Log messages to track start and end of request
klog.V(2).Infof("GetVolumeIDs request recieved for %q", req.PVSpecs)
klog.V(2).Infof("GetVolumeIDs request received for %q", req.PVSpecs)
defer klog.V(2).Infof("GetVolumeIDs request processed successfully for %q", req.PVSpecs)

names := []string{}
Expand Down Expand Up @@ -332,11 +332,10 @@ func (d *MachinePlugin) GetVolumeIDs(ctx context.Context, req *driver.GetVolumeI
//
// RESPONSE PARAMETERS (driver.GenerateMachineClassForMigration)
// NONE
//
func (d *MachinePlugin) GenerateMachineClassForMigration(ctx context.Context, req *driver.GenerateMachineClassForMigrationRequest) (*driver.GenerateMachineClassForMigrationResponse, error) {
// Log messages to track start and end of request
klog.V(2).Infof("MigrateMachineClass request has been recieved for %q", req.ClassSpec)
defer klog.V(2).Infof("MigrateMachineClass request has been processed successfully for %q", req.ClassSpec)
defer klog.V(2).Infof("MigrateMachineClass request has been processed for %q", req.ClassSpec)

azureMachineClass := req.ProviderSpecificMachineClass.(*v1alpha1.AzureMachineClass)

Expand Down
15 changes: 9 additions & 6 deletions pkg/azure/utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -650,10 +650,8 @@ func fillUpMachineClass(azureMachineClass *v1alpha1.AzureMachineClass, machineCl

// WaitForDataDiskDetachment is function that ensures all the data disks are detached from the VM
func waitForDataDiskDetachment(ctx context.Context, clients spi.AzureDriverClientsInterface, resourceGroupName string, vm compute.VirtualMachine) error {
klog.V(2).Infof("Data disk detachment began for %q", *vm.Name)
defer klog.V(2).Infof("Data disk detached for %q", *vm.Name)

if len(*vm.StorageProfile.DataDisks) > 0 {
klog.V(2).Infof("Data disk detachment began for %q", *vm.Name)
// There are disks attached hence need to detach them
vm.StorageProfile.DataDisks = &[]compute.DataDisk{}

Expand All @@ -666,6 +664,9 @@ func waitForDataDiskDetachment(ctx context.Context, clients spi.AzureDriverClien
return OnARMAPIErrorFail(prometheusServiceVM, err, "Failed to CreateOrUpdate. Error Message - %s", err)
}
OnARMAPISuccess(prometheusServiceVM, "VM CreateOrUpdate was successful for %s", *vm.Name)
klog.V(2).Infof("Data disk detached for %q", *vm.Name)
} else {
klog.V(2).Infof("No data disk to detach from %q", *vm.Name)
}

return nil
Expand All @@ -687,7 +688,6 @@ func FetchAttachedVMfromNIC(ctx context.Context, clients spi.AzureDriverClientsI
func DeleteNIC(ctx context.Context, clients spi.AzureDriverClientsInterface, resourceGroupName string, nicName string) error {

klog.V(2).Infof("NIC delete started for %q", nicName)
defer klog.V(2).Infof("NIC deleted for %q", nicName)

nicDeletionCtx, cancel := context.WithTimeout(ctx, nicDeletionTimeout)
defer cancel()
Expand Down Expand Up @@ -715,13 +715,14 @@ func DeleteNIC(ctx context.Context, clients spi.AzureDriverClientsInterface, res
}

OnARMAPISuccess(prometheusServiceNIC, "NIC deletion was successful for %s", nicName)
klog.V(2).Infof("NIC deleted for %q", nicName)

return nil
}

// DeleteVM is the helper function to acknowledge the VM deletion
func DeleteVM(ctx context.Context, clients spi.AzureDriverClientsInterface, resourceGroupName string, vmName string) error {
klog.V(2).Infof("VM deletion has began for %q", vmName)
defer klog.V(2).Infof("VM deleted for %q", vmName)

forceDeletion := false
future, err := clients.GetVM().Delete(ctx, resourceGroupName, vmName, &forceDeletion)
Expand All @@ -733,6 +734,8 @@ func DeleteVM(ctx context.Context, clients spi.AzureDriverClientsInterface, reso
return OnARMAPIErrorFail(prometheusServiceVM, err, "vm.Delete")
}
OnARMAPISuccess(prometheusServiceVM, "VM deletion was successful for %s", vmName)
klog.V(2).Infof("VM deleted for %q", vmName)

return nil
}

Expand All @@ -749,7 +752,6 @@ func fetchAttachedVMfromDisk(ctx context.Context, clients spi.AzureDriverClients

func deleteDisk(ctx context.Context, clients spi.AzureDriverClientsInterface, resourceGroupName string, diskName string) error {
klog.V(2).Infof("Disk delete started for %q", diskName)
defer klog.V(2).Infof("Disk deleted for %q", diskName)

future, err := clients.GetDisk().Delete(ctx, resourceGroupName, diskName)
if err != nil {
Expand All @@ -759,6 +761,7 @@ func deleteDisk(ctx context.Context, clients spi.AzureDriverClientsInterface, re
return OnARMAPIErrorFail(prometheusServiceDisk, err, "disk.Delete")
}
OnARMAPISuccess(prometheusServiceDisk, "Disk deletion was successful for %s", diskName)
klog.V(2).Infof("Disk deleted for %q", diskName)
return nil
}

Expand Down