Merge pull request #386 from jakobmoellerdev/OCPVE-616-VG-Creation-Fi…

…x-Empty-Reports OCPVE-616: fix: add conditions to error on missing lv in case VG is present but thin-pool is not
openshift · Aug 21, 2023 · 04a7434 · 04a7434
2 parents f3b7531 + e107820
commit 04a7434
Show file tree

Hide file tree

Showing 2 changed files with 80 additions and 0 deletions.
diff --git a/pkg/vgmanager/vgmanager_controller.go b/pkg/vgmanager/vgmanager_controller.go
@@ -382,12 +382,22 @@ func (r *VGReconciler) validateLVs(volumeGroup *lvmv1alpha1.LVMVolumeGroup) erro
 	if err != nil {
 		return fmt.Errorf("could not get logical volumes found inside volume group, volume group content is degraded or corrupt: %w", err)
 	}
+	if len(resp.Report) < 1 {
+		return fmt.Errorf("LV report was empty, meaning that the thin-pool LV is no longer found, " +
+			"but the volume group might still exist")
+	}
 
 	for _, report := range resp.Report {
+		if len(report.Lv) < 1 {
+			return fmt.Errorf("no LV was found in the report, meaning that the thin-pool LV is no longer found, " +
+				"but the volume group might still exist")
+		}
+		thinPoolExists := false
 		for _, lv := range report.Lv {
 			if lv.Name != volumeGroup.Spec.ThinPoolConfig.Name {
 				continue
 			}
+			thinPoolExists = true
 			lvAttr, err := ParsedLvAttr(lv.LvAttr)
 			if err != nil {
 				return fmt.Errorf("could not parse lv_attr from logical volume %s: %w", lv.Name, err)
@@ -414,6 +424,9 @@ func (r *VGReconciler) validateLVs(volumeGroup *lvmv1alpha1.LVMVolumeGroup) erro
 
 			r.Log.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")
+		}
 	}
 	return nil
 }

diff --git a/pkg/vgmanager/vgmanager_controller_test.go b/pkg/vgmanager/vgmanager_controller_test.go
@@ -13,6 +13,34 @@ import (
 	"k8s.io/utils/strings/slices"
 )
 
+var mockLvsOutputNoReportContent = `
+{
+      "report": []
+}
+`
+
+var mockLvsOutputNoLVsInReport = `
+{
+      "report": [
+          {
+              "lv": []
+          }
+      ]
+  }
+`
+
+var mockLvsOutputWrongLVsInReport = `
+{
+      "report": [
+          {
+              "lv": [
+                  {"lv_name":"thin-pool-BLUB", "vg_name":"vg1", "lv_attr":"twi-a-tz--", "lv_size":"26.96g", "pool_lv":"", "origin":"", "data_percent":"0.00", "metadata_percent":"10.52", "move_pv":"", "mirror_log":"", "copy_percent":"", "convert_lv":""}
+              ]
+          }
+      ]
+  }
+`
+
 var mockLvsOutputThinPoolValid = `
 {
       "report": [
@@ -147,6 +175,45 @@ func TestVGReconciler_validateLVs(t *testing.T) {
 			}},
 			wantErr: assert.Error,
 		},
+		{
+			name: "Invalid LV due to empty report",
+			fields: fields{
+				executor: mockExecutorForLVSOutput(mockLvsOutputNoReportContent),
+			},
+			args: args{volumeGroup: &lvmv1alpha1.LVMVolumeGroup{
+				ObjectMeta: metav1.ObjectMeta{Name: "vg1", Namespace: "default"},
+				Spec: lvmv1alpha1.LVMVolumeGroupSpec{ThinPoolConfig: &lvmv1alpha1.ThinPoolConfig{
+					Name: "thin-pool-1",
+				}},
+			}},
+			wantErr: assert.Error,
+		},
+		{
+			name: "Invalid LV due to no LVs in report",
+			fields: fields{
+				executor: mockExecutorForLVSOutput(mockLvsOutputNoLVsInReport),
+			},
+			args: args{volumeGroup: &lvmv1alpha1.LVMVolumeGroup{
+				ObjectMeta: metav1.ObjectMeta{Name: "vg1", Namespace: "default"},
+				Spec: lvmv1alpha1.LVMVolumeGroupSpec{ThinPoolConfig: &lvmv1alpha1.ThinPoolConfig{
+					Name: "thin-pool-1",
+				}},
+			}},
+			wantErr: assert.Error,
+		},
+		{
+			name: "Invalid LV due to wrong LVs in report",
+			fields: fields{
+				executor: mockExecutorForLVSOutput(mockLvsOutputWrongLVsInReport),
+			},
+			args: args{volumeGroup: &lvmv1alpha1.LVMVolumeGroup{
+				ObjectMeta: metav1.ObjectMeta{Name: "vg1", Namespace: "default"},
+				Spec: lvmv1alpha1.LVMVolumeGroupSpec{ThinPoolConfig: &lvmv1alpha1.ThinPoolConfig{
+					Name: "thin-pool-1",
+				}},
+			}},
+			wantErr: assert.Error,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {