Fix aggressive lvRemove and vgRemove

The default storage profile was only using the $VG_NAME, causing lvRemove to remove all logical volumes in the volume group. Additionally, volume groups were being removed without first checking to see if logical volumes exist in the volume group. While both of these issues have no real affect on the removal of the LVs/VGs, this was causing issues with the new PreUnmount commands. The first logical volume to run the lvRemove command was the only fileysystem to run the PreUnmount commands, since it destroys all the other filesystems on the rabbit in that single volume group. With this change, the PreUnmmount command runs on each filesystem before it is destroyed. Signed-off-by: Blake Devcich <[email protected]>
NearNodeFlash · Nov 20, 2024 · 04251c9 · 04251c9
1 parent 17ac743
commit 04251c9
Show file tree

Hide file tree

Showing 5 changed files with 22 additions and 12 deletions.
diff --git a/config/examples/nnf_nnfstorageprofile.yaml b/config/examples/nnf_nnfstorageprofile.yaml
@@ -50,7 +50,7 @@ data:
       lvChange:
         activate: --activate ys $VG_NAME/$LV_NAME
         deactivate: --activate n $VG_NAME/$LV_NAME
-      lvRemove: $VG_NAME
+      lvRemove: $VG_NAME/$LV_NAME
       mkfs: -j2 -p $PROTOCOL -t $CLUSTER_NAME:$LOCK_SPACE $DEVICE
       mountRabbit: $DEVICE $MOUNT_PATH
       mountCompute: $DEVICE $MOUNT_PATH
@@ -70,7 +70,7 @@ data:
       lvChange:
         activate: --activate y $VG_NAME/$LV_NAME
         deactivate: --activate n $VG_NAME/$LV_NAME
-      lvRemove: $VG_NAME
+      lvRemove: $VG_NAME/$LV_NAME
       mkfs: $DEVICE
       mountRabbit: $DEVICE $MOUNT_PATH
       mountCompute: $DEVICE $MOUNT_PATH
@@ -90,4 +90,4 @@ data:
       lvChange:
         activate: --activate y $VG_NAME/$LV_NAME
         deactivate: --activate n $VG_NAME/$LV_NAME
-      lvRemove: $VG_NAME
+      lvRemove: $VG_NAME/$LV_NAME
diff --git a/pkg/blockdevice/lvm.go b/pkg/blockdevice/lvm.go
@@ -140,12 +140,22 @@ func (l *Lvm) Destroy(ctx context.Context) (bool, error) {
 	destroyed, err := l.LogicalVolume.Remove(ctx, l.CommandArgs.LvArgs.Remove)
 	if err != nil {
 		return false, err
-
 	}
 	if destroyed {
 		objectDestroyed = true
 	}
 
+	// Check to ensure the VG has no LVs before removing
+	lvs, err := lvm.LvsListVolumes(ctx, l.VolumeGroup.Log)
+	if err != nil {
+		return false, err
+	}
+	for _, lv := range lvs {
+		if lv.VGName == l.VolumeGroup.Name {
+			return false, nil
+		}
+	}
+
 	destroyed, err = l.VolumeGroup.Remove(ctx, l.CommandArgs.VgArgs.Remove)
 	if err != nil {
 		return false, err

diff --git a/pkg/blockdevice/lvm/logical_volumes.go b/pkg/blockdevice/lvm/logical_volumes.go
@@ -51,7 +51,7 @@ func NewLogicalVolume(ctx context.Context, name string, vg *VolumeGroup, size in
 
 // Exists determines if the LV exists in the OS
 func (lv *LogicalVolume) Exists(ctx context.Context) (bool, error) {
-	existingLVs, err := lvsListVolumes(ctx, lv.Log)
+	existingLVs, err := LvsListVolumes(ctx, lv.Log)
 	if err != nil {
 		return false, err
 	}
@@ -98,7 +98,7 @@ func (lv *LogicalVolume) Create(ctx context.Context, rawArgs string) (bool, erro
 		return false, err
 	}
 
-	existingLVs, err := lvsListVolumes(ctx, lv.Log)
+	existingLVs, err := LvsListVolumes(ctx, lv.Log)
 	if err != nil {
 		return false, err
 	}
@@ -126,7 +126,7 @@ func (lv *LogicalVolume) Remove(ctx context.Context, rawArgs string) (bool, erro
 		return false, err
 	}
 
-	existingLVs, err := lvsListVolumes(ctx, lv.Log)
+	existingLVs, err := LvsListVolumes(ctx, lv.Log)
 	if err != nil {
 		return false, err
 	}
@@ -166,7 +166,7 @@ func (lv *LogicalVolume) Activate(ctx context.Context, rawArgs string) (bool, er
 		return false, nil
 	}
 
-	existingLVs, err := lvsListVolumes(ctx, lv.Log)
+	existingLVs, err := LvsListVolumes(ctx, lv.Log)
 	if err != nil {
 		return false, err
 	}
@@ -189,7 +189,7 @@ func (lv *LogicalVolume) Deactivate(ctx context.Context, rawArgs string) (bool,
 		return false, nil
 	}
 
-	existingLVs, err := lvsListVolumes(ctx, lv.Log)
+	existingLVs, err := LvsListVolumes(ctx, lv.Log)
 	if err != nil {
 		return false, err
 	}

diff --git a/pkg/blockdevice/lvm/lvs.go b/pkg/blockdevice/lvm/lvs.go
@@ -43,7 +43,7 @@ type lvsLogicalVolume struct {
 	Size   string `json:"lv_size"`
 }
 
-func lvsListVolumes(ctx context.Context, log logr.Logger) ([]lvsLogicalVolume, error) {
+func LvsListVolumes(ctx context.Context, log logr.Logger) ([]lvsLogicalVolume, error) {
 	output, err := command.Run("lvs --nolock --reportformat json", log)
 	if err != nil {
 		return nil, fmt.Errorf("could not list logical volumes: %w", err)

diff --git a/pkg/blockdevice/lvm/volume_groups.go b/pkg/blockdevice/lvm/volume_groups.go
@@ -174,11 +174,11 @@ func (vg *VolumeGroup) LockStop(ctx context.Context, rawArgs string) (bool, erro
 		return false, err
 	}
 
-	if exists == false {
+	if !exists {
 		return false, nil
 	}
 
-	lvs, err := lvsListVolumes(ctx, vg.Log)
+	lvs, err := LvsListVolumes(ctx, vg.Log)
 	for _, lv := range lvs {
 		if lv.VGName == vg.Name && lv.Attrs[4] == 'a' {
 			return false, nil