From 33615762324cdaaf494b091483ae1e9240f13a24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakob=20M=C3=B6ller?= Date: Fri, 25 Aug 2023 13:47:38 +0200 Subject: [PATCH] chore: remove double error logging in vgmanager_controller --- pkg/vgmanager/status.go | 27 +-- pkg/vgmanager/vgmanager_controller.go | 241 ++++++++++++-------------- 2 files changed, 122 insertions(+), 146 deletions(-) diff --git a/pkg/vgmanager/status.go b/pkg/vgmanager/status.go index 7c2d72802..3edcbf0cf 100644 --- a/pkg/vgmanager/status.go +++ b/pkg/vgmanager/status.go @@ -55,20 +55,18 @@ func (r *VGReconciler) setVolumeGroupReadyStatus(ctx context.Context, vgName str return r.setVolumeGroupStatus(ctx, status) } -func (r *VGReconciler) setVolumeGroupFailedStatus(ctx context.Context, vgName string, reason string) error { +func (r *VGReconciler) setVolumeGroupFailedStatus(ctx context.Context, vgName string, err error) error { status := &lvmv1alpha1.VGStatus{ Name: vgName, Status: lvmv1alpha1.VGStatusFailed, - Reason: reason, + Reason: err.Error(), } // Set devices for the VGStatus. // If there is backing volume group, then set as degraded - devicesExist, err := r.setDevices(status) - if err != nil { - return err - } - if devicesExist { + if devicesExist, err := r.setDevices(status); err != nil { + return fmt.Errorf("could not set devices in VGStatus: %w", err) + } else if devicesExist { status.Status = lvmv1alpha1.VGStatusDegraded } @@ -98,10 +96,12 @@ func (r *VGReconciler) setVolumeGroupStatus(ctx context.Context, status *lvmv1al return nil }) + if err != nil { - r.Log.Error(err, "failed to create or update LVMVolumeGroupNodeStatus", "name", nodeStatus.Name) - return err - } else if result != controllerutil.OperationResultNone { + return fmt.Errorf("LVMVolumeGroupNodeStatus could not be updated: %w", err) + } + + if result != controllerutil.OperationResultNone { r.Log.Info("LVMVolumeGroupNodeStatus modified", "operation", result, "name", nodeStatus.Name) } else { r.Log.Info("LVMVolumeGroupNodeStatus unchanged") @@ -136,9 +136,10 @@ func (r *VGReconciler) removeVolumeGroupStatus(ctx context.Context, vgName strin return nil }) if err != nil { - r.Log.Error(err, "failed to create or update LVMVolumeGroupNodeStatus", "name", nodeStatus.Name) - return err - } else if result != controllerutil.OperationResultNone { + return fmt.Errorf("failed to create or update LVMVolumeGroupNodeStatus %s", nodeStatus.GetName()) + } + + if result != controllerutil.OperationResultNone { r.Log.Info("LVMVolumeGroupNodeStatus modified", "operation", result, "name", nodeStatus.Name) } else { r.Log.Info("LVMVolumeGroupNodeStatus unchanged") diff --git a/pkg/vgmanager/vgmanager_controller.go b/pkg/vgmanager/vgmanager_controller.go index 17c2ef77c..edf3fc11e 100644 --- a/pkg/vgmanager/vgmanager_controller.go +++ b/pkg/vgmanager/vgmanager_controller.go @@ -24,7 +24,6 @@ import ( "strings" "time" - "github.com/go-logr/logr" "github.com/google/go-cmp/cmp" lvmv1alpha1 "github.com/openshift/lvm-operator/api/v1alpha1" "github.com/openshift/lvm-operator/controllers" @@ -33,7 +32,6 @@ import ( lvmdCMD "github.com/topolvm/topolvm/pkg/lvmd/cmd" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" corev1helper "k8s.io/component-helpers/scheduling/corev1" @@ -64,66 +62,55 @@ func (r *VGReconciler) SetupWithManager(mgr ctrl.Manager) error { type VGReconciler struct { client.Client Scheme *runtime.Scheme - Log logr.Logger executor internal.Executor NodeName string Namespace string } func (r *VGReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { - r.Log = log.FromContext(ctx).WithName(ControllerName) - r.Log.Info("reconciling", "LVMVolumeGroup", req) + logger := log.FromContext(ctx).WithName(ControllerName) + logger.Info("reconciling", "LVMVolumeGroup", req) // Check if this LVMVolumeGroup needs to be processed on this node volumeGroup := &lvmv1alpha1.LVMVolumeGroup{} - err := r.Client.Get(ctx, req.NamespacedName, volumeGroup) - if err != nil { - r.Log.Error(err, "failed to get LVMVolumeGroup", "VGName", req.Name) - if errors.IsNotFound(err) { - return ctrl.Result{}, nil - } - return reconcileAgain, err + if err := r.Client.Get(ctx, req.NamespacedName, volumeGroup); err != nil { + return ctrl.Result{}, client.IgnoreNotFound(err) } + // Check if the nodeSelector matches the labels on this node nodeMatches, err := r.matchesThisNode(ctx, volumeGroup.Spec.NodeSelector) if err != nil { - r.Log.Error(err, "failed to match nodeSelector to node labels", "VGName", volumeGroup.Name) - return ctrl.Result{}, err + return ctrl.Result{}, fmt.Errorf("failed to match nodeSelector to node labels: %w", err) } if !nodeMatches { // Nothing to be done on this node for the VG. - r.Log.Info("node labels do not match the selector", "VGName", volumeGroup.Name) + logger.Info("node labels do not match the selector", "VGName", volumeGroup.Name) return ctrl.Result{}, nil } r.executor = &internal.CommandExecutor{} - res, err := r.reconcile(ctx, volumeGroup) - if err != nil { - r.Log.Error(err, "reconcile error", "LVMVolumeGroup", volumeGroup.Name) - } - r.Log.Info("reconcile complete", "result", res) - return res, err + return r.reconcile(ctx, volumeGroup) } func (r *VGReconciler) reconcile(ctx context.Context, volumeGroup *lvmv1alpha1.LVMVolumeGroup) (ctrl.Result, error) { + logger := log.FromContext(ctx).WithName(ControllerName) // Check if the LVMVolumeGroup resource is deleted if !volumeGroup.DeletionTimestamp.IsZero() { - err := r.processDelete(ctx, volumeGroup) - return ctrl.Result{}, err + return ctrl.Result{}, r.processDelete(ctx, volumeGroup) } // Read the lvmd config file lvmdConfig, err := loadLVMDConfig() if err != nil { - r.Log.Error(err, "failed to read the lvmd config file") - if statuserr := r.setVolumeGroupFailedStatus(ctx, volumeGroup.Name, fmt.Sprintf("failed to read the lvmd config file: %v", err.Error())); statuserr != nil { - r.Log.Error(statuserr, "failed to update status", "name", volumeGroup.Name) + err = fmt.Errorf("failed to read the lvmd config file: %w", err) + if err := r.setVolumeGroupFailedStatus(ctx, volumeGroup.Name, err); err != nil { + logger.Error(err, "failed to set status to failed", "VGName", volumeGroup.GetName()) } - return reconcileAgain, err + return ctrl.Result{}, err } if lvmdConfig == nil { // The lvmdconfig file does not exist and will be created. - r.Log.Info("lvmd config file doesn't exist, will create") + logger.Info("lvmd config file doesn't exist, will create") lvmdConfig = &lvmdCMD.Config{ SocketName: controllers.DefaultLVMdSocket, } @@ -132,23 +119,21 @@ func (r *VGReconciler) reconcile(ctx context.Context, volumeGroup *lvmv1alpha1.L vgs, err := ListVolumeGroups(r.executor) if err != nil { - return reconcileAgain, fmt.Errorf("failed to list volume groups. %v", err) + return ctrl.Result{}, fmt.Errorf("failed to list volume groups. %v", err) } blockDevices, err := internal.ListBlockDevices(r.executor) if err != nil { - return reconcileAgain, fmt.Errorf("failed to list block devices: %v", err) + return ctrl.Result{}, fmt.Errorf("failed to list block devices: %v", err) } - //Get the available block devices that can be used for this volume group + // Get the available block devices that can be used for this volume group availableDevices, err := r.getAvailableDevicesForVG(blockDevices, vgs, volumeGroup) if err != nil { - r.Log.Error(err, "failed to get block devices for volumegroup, will retry", "name", volumeGroup.Name) - // Failed to get devices for this volume group. Reconcile again. - return reconcileAgain, err + return ctrl.Result{}, fmt.Errorf("failed to get block devices for volumegroup %s: %w", volumeGroup.GetName(), err) } - r.Log.Info("listing available and delayed devices", "availableDevices", availableDevices) + logger.Info("listing available and delayed devices", "availableDevices", availableDevices) // If there are no available devices, that could mean either // - There is no available devices to attach to the volume group @@ -164,30 +149,25 @@ func (r *VGReconciler) reconcile(ctx context.Context, volumeGroup *lvmv1alpha1.L } if !devicesExist { - err := fmt.Errorf("no available devices found for volume group") - r.Log.Error(err, err.Error(), "VGName", volumeGroup.Name) - if statuserr := r.setVolumeGroupFailedStatus(ctx, volumeGroup.Name, err.Error()); statuserr != nil { - r.Log.Error(statuserr, "failed to update status", "name", volumeGroup.Name) - return reconcileAgain, statuserr + err := fmt.Errorf("no available devices found for volume group %s", volumeGroup.GetName()) + if err := r.setVolumeGroupFailedStatus(ctx, volumeGroup.Name, err); err != nil { + logger.Error(err, "failed to set status to failed", "VGName", volumeGroup.GetName()) } - return reconcileAgain, err + return ctrl.Result{}, err } // since the last reconciliation there could have been corruption on the LVs so we need to verify them again - if err := r.validateLVs(volumeGroup); err != nil { + if err := r.validateLVs(ctx, volumeGroup); err != nil { err := fmt.Errorf("error while validating logical volumes in existing volume group: %w", err) - r.Log.Error(err, err.Error(), "VGName", volumeGroup.Name) - if statuserr := r.setVolumeGroupFailedStatus(ctx, volumeGroup.Name, err.Error()); statuserr != nil { - r.Log.Error(statuserr, "failed to update status", "name", volumeGroup.Name) - return reconcileAgain, statuserr + if err := r.setVolumeGroupFailedStatus(ctx, volumeGroup.Name, err); err != nil { + logger.Error(err, "failed to set status to failed", "VGName", volumeGroup.GetName()) } - return reconcileAgain, err + return ctrl.Result{}, err } - r.Log.Info("all the available devices are attached to the volume group", "VGName", volumeGroup.Name) - if statuserr := r.setVolumeGroupReadyStatus(ctx, volumeGroup.Name); statuserr != nil { - r.Log.Error(statuserr, "failed to update status", "VGName", volumeGroup.Name) - return reconcileAgain, statuserr + logger.Info("all the available devices are attached to the volume group", "VGName", volumeGroup.Name) + if err := r.setVolumeGroupReadyStatus(ctx, volumeGroup.Name); err != nil { + return ctrl.Result{}, fmt.Errorf("failed to set status for volume group %s to ready: %w", volumeGroup.Name, err) } return reconcileAgain, nil @@ -196,33 +176,30 @@ func (r *VGReconciler) reconcile(ctx context.Context, volumeGroup *lvmv1alpha1.L // Create/extend VG err = r.addDevicesToVG(vgs, volumeGroup.Name, availableDevices) if err != nil { - r.Log.Error(err, "failed to create/extend volume group", "VGName", volumeGroup.Name) - if statuserr := r.setVolumeGroupFailedStatus(ctx, volumeGroup.Name, fmt.Sprintf("failed to create/extend volume group %s: %v", volumeGroup.Name, err.Error())); statuserr != nil { - r.Log.Error(statuserr, "failed to update status", "VGName", volumeGroup.Name) + err = fmt.Errorf("failed to create/extend volume group %s: %w", volumeGroup.Name, err) + if err := r.setVolumeGroupFailedStatus(ctx, volumeGroup.Name, err); err != nil { + logger.Error(err, "failed to set status to failed", "VGName", volumeGroup.GetName()) } - return reconcileAgain, err + return ctrl.Result{}, err } // Create thin pool - err = r.addThinPoolToVG(volumeGroup.Name, volumeGroup.Spec.ThinPoolConfig) + err = r.addThinPoolToVG(ctx, volumeGroup.Name, volumeGroup.Spec.ThinPoolConfig) if err != nil { - r.Log.Error(err, "failed to create thin pool", "VGName", "ThinPool", volumeGroup.Name, volumeGroup.Spec.ThinPoolConfig.Name) - if statuserr := r.setVolumeGroupFailedStatus(ctx, volumeGroup.Name, - fmt.Sprintf("failed to create thin pool %s for volume group %s: %v", volumeGroup.Spec.ThinPoolConfig.Name, volumeGroup.Name, err.Error())); statuserr != nil { - r.Log.Error(statuserr, "failed to update status", "VGName", volumeGroup.Name) - return reconcileAgain, statuserr + err := fmt.Errorf("failed to create thin pool %s for volume group %s: %w", volumeGroup.Spec.ThinPoolConfig.Name, volumeGroup.Name, err) + if err := r.setVolumeGroupFailedStatus(ctx, volumeGroup.Name, err); err != nil { + logger.Error(err, "failed to set status to failed", "VGName", volumeGroup.GetName()) } + return ctrl.Result{}, err } // Validate the LVs created from the Thin-Pool to make sure the adding went as planned. - if err := r.validateLVs(volumeGroup); err != nil { + if err := r.validateLVs(ctx, volumeGroup); err != nil { err := fmt.Errorf("error while validating logical volumes in existing volume group: %w", err) - r.Log.Error(err, err.Error(), "VGName", volumeGroup.Name) - if statuserr := r.setVolumeGroupFailedStatus(ctx, volumeGroup.Name, err.Error()); statuserr != nil { - r.Log.Error(statuserr, "failed to update status", "name", volumeGroup.Name) - return reconcileAgain, statuserr + if err := r.setVolumeGroupFailedStatus(ctx, volumeGroup.Name, err); err != nil { + logger.Error(err, "failed to set status to failed", "VGName", volumeGroup.GetName()) } - return reconcileAgain, err + return ctrl.Result{}, err } // Add the volume group to device classes inside lvmd config if not exists @@ -251,41 +228,38 @@ func (r *VGReconciler) reconcile(ctx context.Context, volumeGroup *lvmv1alpha1.L // Apply and save lvmd config if !cmp.Equal(existingLvmdConfig, lvmdConfig) { - err := saveLVMDConfig(lvmdConfig) - if err != nil { - r.Log.Error(err, "failed to update lvmd config file", "VGName", volumeGroup.Name) - if statuserr := r.setVolumeGroupFailedStatus(ctx, volumeGroup.Name, fmt.Sprintf("failed to update lvmd config file: %v", err.Error())); statuserr != nil { - r.Log.Error(statuserr, "failed to update status", "name", volumeGroup.Name) + if err := saveLVMDConfig(lvmdConfig); err != nil { + err := fmt.Errorf("failed to update lvmd config file to update volume group %s: %w", volumeGroup.GetName(), err) + if err := r.setVolumeGroupFailedStatus(ctx, volumeGroup.Name, err); err != nil { + logger.Error(err, "failed to set status to failed", "VGName", volumeGroup.GetName()) } - return reconcileAgain, err + return ctrl.Result{}, err } - r.Log.Info("updated lvmd config", "VGName", volumeGroup.Name) + logger.Info("updated lvmd config", "VGName", volumeGroup.Name) } - if statuserr := r.setVolumeGroupReadyStatus(ctx, volumeGroup.Name); statuserr != nil { - r.Log.Error(statuserr, "failed to update status", "VGName", volumeGroup.Name) - return reconcileAgain, nil + if err := r.setVolumeGroupReadyStatus(ctx, volumeGroup.Name); err != nil { + return ctrl.Result{}, fmt.Errorf("failed to set status for volume group %s to ready: %w", volumeGroup.Name, err) } return reconcileAgain, nil } func (r *VGReconciler) processDelete(ctx context.Context, volumeGroup *lvmv1alpha1.LVMVolumeGroup) error { + logger := log.FromContext(ctx).WithName(ControllerName) // Read the lvmd config file lvmdConfig, err := loadLVMDConfig() if err != nil { // Failed to read lvmdconfig file. Reconcile again - r.Log.Error(err, "failed to read the lvmd config file") - return err + return fmt.Errorf("failed to read the lvmd config file: %w", err) } if lvmdConfig == nil { - r.Log.Info("lvmd config file does not exist") + logger.Info("lvmd config file does not exist") // Remove the VG entry in the LVMVolumeGroupNodeStatus that was added to indicate the failures to the user. // This allows the LVMCluster to get deleted and not stuck/wait forever as LVMCluster looks for the LVMVolumeGroupNodeStatus before deleting. - if statuserr := r.removeVolumeGroupStatus(ctx, volumeGroup.Name); statuserr != nil { - r.Log.Error(statuserr, "failed to update status", "VGName", volumeGroup.Name) - return statuserr + if err := r.removeVolumeGroupStatus(ctx, volumeGroup.Name); err != nil { + return fmt.Errorf("failed to remove status for volume group %s: %w", volumeGroup.Name, err) } return nil } @@ -297,10 +271,9 @@ func (r *VGReconciler) processDelete(ctx context.Context, volumeGroup *lvmv1alph index, found := deviceClassMap[volumeGroup.Name] if !found { // Nothing to do here. - r.Log.Info("failed to find volume group in lvmd deviceclasses list", "VGName", volumeGroup.Name) - if statuserr := r.removeVolumeGroupStatus(ctx, volumeGroup.Name); statuserr != nil { - r.Log.Error(statuserr, "failed to update status", "VGName", volumeGroup.Name) - return statuserr + logger.Info("could not find volume group in lvmd deviceclasses list", "VGName", volumeGroup.Name) + if err := r.removeVolumeGroupStatus(ctx, volumeGroup.Name); err != nil { + return fmt.Errorf("failed to remove status for volume group %s: %w", volumeGroup.Name, err) } return nil } @@ -309,7 +282,7 @@ func (r *VGReconciler) processDelete(ctx context.Context, volumeGroup *lvmv1alph vg, err := GetVolumeGroup(r.executor, volumeGroup.Name) if err != nil { if err != ErrVolumeGroupNotFound { - return fmt.Errorf("failed to get volume group. %q, %v", volumeGroup.Name, err) + return fmt.Errorf("failed to get volume group %s, %w", volumeGroup.GetName(), err) } return nil } @@ -323,48 +296,43 @@ func (r *VGReconciler) processDelete(ctx context.Context, volumeGroup *lvmv1alph } if lvExists { - err := DeleteLV(r.executor, thinPoolName, volumeGroup.Name) - if err != nil { - if statuserr := r.setVolumeGroupFailedStatus(ctx, volumeGroup.Name, fmt.Sprintf("failed to delete thin pool %s in volume group %s: %v", thinPoolName, volumeGroup.Name, err.Error())); statuserr != nil { - r.Log.Error(statuserr, "failed to update status", "name", volumeGroup.Name) + if err := DeleteLV(r.executor, thinPoolName, volumeGroup.Name); err != nil { + err := fmt.Errorf("failed to delete thin pool %s in volume group %s: %w", thinPoolName, volumeGroup.Name, err) + if err := r.setVolumeGroupFailedStatus(ctx, volumeGroup.Name, err); err != nil { + logger.Error(err, "failed to set status to failed", "VGName", volumeGroup.GetName()) } - return fmt.Errorf("failed to delete thin pool %q in volume group %q. %v", thinPoolName, volumeGroup.Name, err) + return err } - r.Log.Info("thin pool deleted in the volume group.", "VGName", volumeGroup.Name, "ThinPool", thinPoolName) + logger.Info("thin pool deleted in the volume group", "VGName", volumeGroup.Name, "ThinPool", thinPoolName) } else { - r.Log.Info("thin pool not found in the volume group.", "VGName", volumeGroup.Name, "ThinPool", thinPoolName) + logger.Info("thin pool not found in the volume group", "VGName", volumeGroup.Name, "ThinPool", thinPoolName) } } - err = vg.Delete(r.executor) - if err != nil { - if statuserr := r.setVolumeGroupFailedStatus(ctx, volumeGroup.Name, fmt.Sprintf("failed to delete volume group %s: %v", volumeGroup.Name, err.Error())); statuserr != nil { - r.Log.Error(statuserr, "failed to update status", "name", volumeGroup.Name) + if err = vg.Delete(r.executor); err != nil { + err := fmt.Errorf("failed to delete volume group %s: %w", volumeGroup.Name, err) + if err := r.setVolumeGroupFailedStatus(ctx, volumeGroup.Name, err); err != nil { + logger.Error(err, "failed to set status to failed", "VGName", volumeGroup.GetName()) } - return fmt.Errorf("failed to delete volume group. %q, %v", volumeGroup.Name, err) + return err } // Remove this vg from the lvmdconf file lvmdConfig.DeviceClasses = append(lvmdConfig.DeviceClasses[:index], lvmdConfig.DeviceClasses[index+1:]...) - r.Log.Info("updating lvmd config") + logger.Info("updating lvmd config") if len(lvmdConfig.DeviceClasses) > 0 { - err = saveLVMDConfig(lvmdConfig) - if err != nil { - r.Log.Error(err, "failed to update lvmd.conf file", "VGName", volumeGroup.Name) - return err + if err = saveLVMDConfig(lvmdConfig); err != nil { + return fmt.Errorf("failed to update lvmd.conf file for volume group %s: %w", volumeGroup.GetName(), err) } } else { - err = deleteLVMDConfig() - if err != nil { - r.Log.Error(err, "failed to delete lvmd.conf file", "VGName", volumeGroup.Name) - return err + if err = deleteLVMDConfig(); err != nil { + return fmt.Errorf("failed to delete lvmd.conf file for volume group %s: %w", volumeGroup.GetName(), err) } } - if statuserr := r.removeVolumeGroupStatus(ctx, volumeGroup.Name); statuserr != nil { - r.Log.Error(statuserr, "failed to update status", "VGName", volumeGroup.Name) - return statuserr + if err := r.removeVolumeGroupStatus(ctx, volumeGroup.Name); err != nil { + return fmt.Errorf("failed to remove status for volume group %s: %w", volumeGroup.Name, err) } return nil @@ -372,7 +340,9 @@ func (r *VGReconciler) processDelete(ctx context.Context, volumeGroup *lvmv1alph // validateLVs verifies that all lvs that should have been created in the volume group are present and // in their correct state -func (r *VGReconciler) validateLVs(volumeGroup *lvmv1alpha1.LVMVolumeGroup) error { +func (r *VGReconciler) validateLVs(ctx context.Context, volumeGroup *lvmv1alpha1.LVMVolumeGroup) error { + logger := log.FromContext(ctx).WithName(ControllerName) + // If we don't have a ThinPool, VG Manager has no authority about the top Level LVs inside the VG, but TopoLVM if volumeGroup.Spec.ThinPoolConfig == nil { return nil @@ -422,7 +392,7 @@ func (r *VGReconciler) validateLVs(volumeGroup *lvmv1alpha1.LVMVolumeGroup) erro "you should manually extend the metadata_partition or you will risk data loss: metadata_percent: %v", metadataPercentage, lv.MetadataPercent) } - r.Log.Info("confirmed created logical volume has correct attributes", "lv_attr", lvAttr.String()) + logger.Info("confirmed created logical volume has correct attributes", "lv_attr", lvAttr.String()) } if !thinPoolExists { return fmt.Errorf("the thin-pool LV is no longer present, but the volume group might still exist") @@ -431,7 +401,9 @@ func (r *VGReconciler) validateLVs(volumeGroup *lvmv1alpha1.LVMVolumeGroup) erro return nil } -func (r *VGReconciler) addThinPoolToVG(vgName string, config *lvmv1alpha1.ThinPoolConfig) error { +func (r *VGReconciler) addThinPoolToVG(ctx context.Context, vgName string, config *lvmv1alpha1.ThinPoolConfig) error { + logger := log.FromContext(ctx).WithName(ControllerName) + resp, err := GetLVSOutput(r.executor, vgName) if err != nil { return fmt.Errorf("failed to list logical volumes in the volume group %q. %v", vgName, err) @@ -441,30 +413,29 @@ func (r *VGReconciler) addThinPoolToVG(vgName string, config *lvmv1alpha1.ThinPo for _, lv := range report.Lv { if lv.Name == config.Name { if strings.Contains(lv.LvAttr, "t") { - r.Log.Info("lvm thinpool already exists", "VGName", vgName, "ThinPool", config.Name) - err = r.extendThinPool(vgName, lv.LvSize, config) - if err != nil { - r.Log.Error(err, "failed to extend the lvm thinpool", "VGName", vgName, "ThinPool", config.Name) + logger.Info("lvm thinpool already exists", "VGName", vgName, "ThinPool", config.Name) + if err := r.extendThinPool(ctx, vgName, lv.LvSize, config); err != nil { + return fmt.Errorf("failed to extend the lvm thinpool %s in volume group %s: %w", config.Name, vgName, err) } - return err + return nil } - return fmt.Errorf("failed to create thin pool %q. Logical volume with same name already exists", config.Name) + return fmt.Errorf("failed to create thin pool %q, logical volume with same name already exists", config.Name) } } } args := []string{"-l", fmt.Sprintf("%d%%FREE", config.SizePercent), "-c", DefaultChunkSize, "-Z", "y", "-T", fmt.Sprintf("%s/%s", vgName, config.Name)} - _, err = r.executor.ExecuteCommandWithOutputAsHost(lvCreateCmd, args...) - if err != nil { + if _, err = r.executor.ExecuteCommandWithOutputAsHost(lvCreateCmd, args...); err != nil { return fmt.Errorf("failed to create thin pool %q in the volume group %q using command '%s': %v", config.Name, vgName, fmt.Sprintf("%s %s", lvCreateCmd, strings.Join(args, " ")), err) } return nil } -func (r *VGReconciler) extendThinPool(vgName string, lvSize string, config *lvmv1alpha1.ThinPoolConfig) error { +func (r *VGReconciler) extendThinPool(ctx context.Context, vgName string, lvSize string, config *lvmv1alpha1.ThinPoolConfig) error { + logger := log.FromContext(ctx).WithName(ControllerName) vg, err := GetVolumeGroup(r.executor, vgName) if err != nil { @@ -489,16 +460,15 @@ func (r *VGReconciler) extendThinPool(vgName string, lvSize string, config *lvmv return nil } - r.Log.Info("extending lvm thinpool ", "VGName", vgName, "ThinPool", config.Name) + logger.Info("extending lvm thinpool ", "VGName", vgName, "ThinPool", config.Name) args := []string{"-l", fmt.Sprintf("%d%%Vg", config.SizePercent), fmt.Sprintf("%s/%s", vgName, config.Name)} - _, err = r.executor.ExecuteCommandWithOutputAsHost(lvExtendCmd, args...) - if err != nil { + if _, err = r.executor.ExecuteCommandWithOutputAsHost(lvExtendCmd, args...); err != nil { return fmt.Errorf("failed to extend thin pool %q in the volume group %q using command '%s': %v", config.Name, vgName, fmt.Sprintf("%s %s", lvExtendCmd, strings.Join(args, " ")), err) } - r.Log.Info("successfully extended the thin pool in the volume group ", "thinpool", config.Name, "vgName", vgName) + logger.Info("successfully extended the thin pool in the volume group ", "thinpool", config.Name, "vgName", vgName) return nil } @@ -531,12 +501,11 @@ func loadLVMDConfig() (*lvmdCMD.Config, error) { // If the file does not exist, return nil for both return nil, nil } else if err != nil { - return nil, err + return nil, fmt.Errorf("failed to load config file %s: %w", controllers.LvmdConfigFile, err) } else { lvmdconfig := &lvmdCMD.Config{} - err = yaml.Unmarshal(cfgBytes, lvmdconfig) - if err != nil { - return nil, err + if err = yaml.Unmarshal(cfgBytes, lvmdconfig); err != nil { + return nil, fmt.Errorf("failed to unmarshal config file %s: %w", controllers.LvmdConfigFile, err) } return lvmdconfig, nil } @@ -547,10 +516,16 @@ func saveLVMDConfig(lvmdConfig *lvmdCMD.Config) error { if err == nil { err = os.WriteFile(controllers.LvmdConfigFile, out, 0600) } - return err + if err != nil { + return fmt.Errorf("failed to save config file %s: %w", controllers.LvmdConfigFile, err) + } + return nil } func deleteLVMDConfig() error { err := os.Remove(controllers.LvmdConfigFile) + if err != nil { + return fmt.Errorf("failed to delete config file %s: %w", controllers.LvmdConfigFile, err) + } return err }