Merge pull request #568 from jakobmoellerdev/OCPBUGS-26768-release-4.…

…15-correction

[release-4.15] OCPBUGS-26768: fix: more sophisticated tagging migration

catalog/lvms-operator/operator.yaml

@@ -86,13 +86,25 @@ properties:
             }
           }
       operatorframework.io/suggested-namespace: openshift-storage
+      operatorframework.io/suggested-namespace-template: |-
+        {
+          "apiVersion": "v1",
+          "kind": "Namespace",
+          "metadata": {
+            "name": "openshift-storage",
+            "annotations": {
+              "workload.openshift.io/allowed": "management"
+            }
+          }
+        }
       operators.openshift.io/infrastructure-features: '["csi", "disconnected"]'
-      operators.operatorframework.io/builder: operator-sdk-v1.25.3
+      operators.openshift.io/valid-subscription: '["OpenShift Container Platform",
+        "OpenShift Platform Plus"]'
+      operators.operatorframework.io/builder: operator-sdk-v1.33.0
       operators.operatorframework.io/internal-objects: '["logicalvolumes.topolvm.io",
         "lvmvolumegroups.lvm.topolvm.io", "lvmvolumegroupnodestatuses.lvm.topolvm.io"]'
       operators.operatorframework.io/project_layout: go.kubebuilder.io/v3
       repository: https://github.com/openshift/lvm-operator
-      target.workload.openshift.io/management: '{"effect": "PreferredDuringScheduling"}'
     apiServiceDefinitions: {}
     crdDescriptions:
       owned:

internal/controllers/vgmanager/controller.go

@@ -187,7 +187,7 @@ func (r *Reconciler) reconcile(
 
 	devices := r.filterDevices(ctx, newDevices, r.Filters(volumeGroup, r.LVM, r.LSBLK))
 
-	vgs, err := r.LVM.ListVGs()
+	vgs, err := r.LVM.ListVGs(true)
 	if err != nil {
 		return ctrl.Result{}, fmt.Errorf("failed to list volume groups: %w", err)
 	}

internal/controllers/vgmanager/controller_test.go

@@ -245,7 +245,7 @@ func testMockedBlockDeviceOnHost(ctx context.Context) {
 	}
 
 	By("triggering the second reconciliation after the initial setup", func() {
-		instances.LVM.EXPECT().ListVGs().Return(nil, nil).Twice()
+		instances.LVM.EXPECT().ListVGs(true).Return(nil, nil).Twice()
 		instances.LSBLK.EXPECT().ListBlockDevices().Return([]lsblk.BlockDevice{blockDevice}, nil).Once()
 		instances.LSBLK.EXPECT().HasBindMounts(blockDevice).Return(false, "", nil).Once()
 		_, err := instances.Reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(vg)})
@@ -262,7 +262,7 @@ func testMockedBlockDeviceOnHost(ctx context.Context) {
 	})
 
 	// Requeue effects
-	instances.LVM.EXPECT().ListVGs().Return(nil, nil).Twice()
+	instances.LVM.EXPECT().ListVGs(true).Return(nil, nil).Twice()
 	instances.LSBLK.EXPECT().ListBlockDevices().Return([]lsblk.BlockDevice{blockDevice}, nil).Once()
 	instances.LSBLK.EXPECT().HasBindMounts(blockDevice).Return(false, "", nil).Once()
 
@@ -303,7 +303,7 @@ func testMockedBlockDeviceOnHost(ctx context.Context) {
 		instances.LVM.EXPECT().ListLVs(vg.GetName()).Return(&lvm.LVReport{Report: []lvm.LVReportItem{{
 			Lv: []lvm.LogicalVolume{thinPool},
 		}}}, nil).Once()
-		instances.LVM.EXPECT().ListVGs().Return([]lvm.VolumeGroup{createdVG}, nil).Twice()
+		instances.LVM.EXPECT().ListVGs(true).Return([]lvm.VolumeGroup{createdVG}, nil).Twice()
 		instances.LVM.EXPECT().ActivateLV(thinPool.Name, vg.GetName()).Return(nil).Once()
 	})
 
@@ -370,7 +370,7 @@ func testMockedBlockDeviceOnHost(ctx context.Context) {
 	})
 
 	By("mocking the now created vg and thin pool", func() {
-		instances.LVM.EXPECT().ListVGs().Return([]lvm.VolumeGroup{createdVG}, nil).Once()
+		instances.LVM.EXPECT().ListVGs(true).Return([]lvm.VolumeGroup{createdVG}, nil).Once()
 		instances.LVM.EXPECT().ListLVs(vg.GetName()).Return(&lvm.LVReport{Report: []lvm.LVReportItem{{
 			Lv: []lvm.LogicalVolume{thinPool},
 		}}}, nil).Once()
@@ -561,13 +561,13 @@ func testLVMD(ctx context.Context) {
 	r.LVM = mockLVM
 
 	mockLVMD.EXPECT().Load().Once().Return(nil, fmt.Errorf("mock load failure"))
-	mockLVM.EXPECT().ListVGs().Once().Return(nil, fmt.Errorf("mock list failure"))
+	mockLVM.EXPECT().ListVGs(true).Once().Return(nil, fmt.Errorf("mock list failure"))
 	err := r.applyLVMDConfig(ctx, vg, devices)
 	Expect(err).To(HaveOccurred(), "should error if lvmd config cannot be loaded and/or status cannot be set")
 
 	mockLVMD.EXPECT().Load().Once().Return(&lvmd.Config{}, nil)
 	mockLVMD.EXPECT().Save(mock.Anything).Once().Return(fmt.Errorf("mock save failure"))
-	mockLVM.EXPECT().ListVGs().Once().Return(nil, fmt.Errorf("mock list failure"))
+	mockLVM.EXPECT().ListVGs(true).Once().Return(nil, fmt.Errorf("mock list failure"))
 	err = r.applyLVMDConfig(ctx, vg, devices)
 	Expect(err).To(HaveOccurred(), "should error if lvmd config cannot be saved and/or status cannot be set")
 }
@@ -768,7 +768,7 @@ func testReconcileFailure(ctx context.Context) {
 			{Name: "/dev/xxx", KName: "/dev/xxx", FSType: "ext4"},
 		}
 		instances.LSBLK.EXPECT().ListBlockDevices().Once().Return(blockDevices, nil)
-		instances.LVM.EXPECT().ListVGs().Once().Return(nil, nil)
+		instances.LVM.EXPECT().ListVGs(true).Once().Return(nil, nil)
 		_, err := instances.Reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(vg)})
 		nodeStatus := &lvmv1alpha1.LVMVolumeGroupNodeStatus{}
 		Expect(instances.client.Get(ctx, client.ObjectKey{
@@ -795,7 +795,7 @@ func testReconcileFailure(ctx context.Context) {
 		instances.LSBLK.EXPECT().ListBlockDevices().Once().Return([]lsblk.BlockDevice{
 			{Name: "/dev/sda", KName: "/dev/sda", FSType: "xfs", PartLabel: "reserved"},
 		}, nil)
-		instances.LVM.EXPECT().ListVGs().Twice().Return(nil, nil)
+		instances.LVM.EXPECT().ListVGs(true).Twice().Return(nil, nil)
 		instances.LSBLK.EXPECT().HasBindMounts(mock.Anything).Once().Return(false, "", nil)
 		_, err := instances.Reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(vg)})
 		Expect(err).To(HaveOccurred())
@@ -813,8 +813,8 @@ func testReconcileFailure(ctx context.Context) {
 		instances.LSBLK.EXPECT().ListBlockDevices().Once().Return([]lsblk.BlockDevice{
 			{Name: "/dev/sda", KName: "/dev/sda", FSType: "xfs", PartLabel: "reserved"},
 		}, nil)
-		instances.LVM.EXPECT().ListVGs().Once().Return(nil, nil)
-		instances.LVM.EXPECT().ListVGs().Once().Return(nil, fmt.Errorf("mocked error"))
+		instances.LVM.EXPECT().ListVGs(true).Once().Return(nil, nil)
+		instances.LVM.EXPECT().ListVGs(true).Once().Return(nil, fmt.Errorf("mocked error"))
 		instances.LSBLK.EXPECT().HasBindMounts(mock.Anything).Once().Return(false, "", nil)
 		_, err := instances.Reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(vg)})
 		Expect(err).To(HaveOccurred())
@@ -834,7 +834,7 @@ func testReconcileFailure(ctx context.Context) {
 		vgs := []lvm.VolumeGroup{
 			{Name: "vg1", VgSize: "1g"},
 		}
-		instances.LVM.EXPECT().ListVGs().Twice().Return(vgs, nil)
+		instances.LVM.EXPECT().ListVGs(true).Twice().Return(vgs, nil)
 		instances.LSBLK.EXPECT().HasBindMounts(mock.Anything).Once().Return(false, "", nil)
 		instances.LVM.EXPECT().ListLVs("vg1").Once().Return(nil, fmt.Errorf("mocked error"))
 		_, err := instances.Reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(vg)})
@@ -854,8 +854,8 @@ func testReconcileFailure(ctx context.Context) {
 		vgs := []lvm.VolumeGroup{
 			{Name: "vg1", VgSize: "1g"},
 		}
-		instances.LVM.EXPECT().ListVGs().Once().Return(vgs, nil)
-		instances.LVM.EXPECT().ListVGs().Once().Return(nil, fmt.Errorf("mocked error"))
+		instances.LVM.EXPECT().ListVGs(true).Once().Return(vgs, nil)
+		instances.LVM.EXPECT().ListVGs(true).Once().Return(nil, fmt.Errorf("mocked error"))
 		instances.LSBLK.EXPECT().HasBindMounts(mock.Anything).Once().Return(false, "", nil)
 		instances.LVM.EXPECT().ListLVs("vg1").Once().Return(nil, fmt.Errorf("mocked error"))
 		_, err := instances.Reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(vg)})

internal/controllers/vgmanager/lvm/lvm.go

@@ -58,6 +58,7 @@ type VGReport struct {
 		Vg []struct {
 			Name   string `json:"vg_name"`
 			VgSize string `json:"vg_size"`
+			Tags   string `json:"vg_tags"`
 		} `json:"vg"`
 	} `json:"report"`
 }
@@ -95,7 +96,7 @@ type LVM interface {
 	GetVG(name string) (VolumeGroup, error)
 
 	ListPVs(vgName string) ([]PhysicalVolume, error)
-	ListVGs() ([]VolumeGroup, error)
+	ListVGs(taggedByLVMS bool) ([]VolumeGroup, error)
 	ListLVsByName(vgName string) ([]string, error)
 	ListLVs(vgName string) (*LVReport, error)
 
@@ -128,6 +129,9 @@ type VolumeGroup struct {
 
 	// PVs is the list of physical volumes associated with the volume group
 	PVs []PhysicalVolume `json:"pvs"`
+
+	// Tags is the list of tags associated with the volume group
+	Tags []string `json:"vg_tags"`
 }
 
 // PhysicalVolume represents a physical volume of linux lvm.
@@ -336,17 +340,29 @@ func (hlvm *HostLVM) ListPVs(vgName string) ([]PhysicalVolume, error) {
 }
 
 // ListVolumeGroups lists all volume groups and the physical volumes associated with them.
-func (hlvm *HostLVM) ListVGs() ([]VolumeGroup, error) {
+func (hlvm *HostLVM) ListVGs(tagged bool) ([]VolumeGroup, error) {
 	res := new(VGReport)
 
-	if err := hlvm.execute(res, "vgs", lvmsTag, "--reportformat", "json"); err != nil {
+	args := []string{
+		"vgs", "-o", "vg_name,vg_size,vg_tags", "--units", "g", "--reportformat", "json",
+	}
+	if tagged {
+		args = append(args, lvmsTag)
+	}
+
+	if err := hlvm.execute(res, args...); err != nil {
 		return nil, fmt.Errorf("failed to list volume groups. %v", err)
 	}
 
 	var vgList []VolumeGroup
 	for _, report := range res.Report {
 		for _, vg := range report.Vg {
-			vgList = append(vgList, VolumeGroup{Name: vg.Name, PVs: []PhysicalVolume{}})
+			vgList = append(vgList, VolumeGroup{
+				Name:   vg.Name,
+				VgSize: vg.VgSize,
+				PVs:    []PhysicalVolume{},
+				Tags:   strings.Split(vg.Tags, ","),
+			})
 		}
 	}
 
@@ -359,6 +375,10 @@ func (hlvm *HostLVM) ListVGs() ([]VolumeGroup, error) {
 		vgList[i].PVs = pvs
 	}
 
+	if !tagged {
+		return untaggedVGs(vgList), nil
+	}
+
 	return vgList, nil
 }
 
@@ -514,3 +534,20 @@ func (hlvm *HostLVM) execute(v interface{}, args ...string) error {
 
 	return nil
 }
+
+func untaggedVGs(vgs []VolumeGroup) []VolumeGroup {
+	var untaggedVGs []VolumeGroup
+	for _, vg := range vgs {
+		tagPresent := false
+		for _, tag := range vg.Tags {
+			if tag == lvmsTag {
+				tagPresent = true
+				break
+			}
+		}
+		if !tagPresent {
+			untaggedVGs = append(untaggedVGs, vg)
+		}
+	}
+	return untaggedVGs
+}

internal/controllers/vgmanager/lvm/lvm_test.go

@@ -260,7 +260,7 @@ func TestHostLVM_ListVGs(t *testing.T) {
 					return "", fmt.Errorf("invalid args %q", args[0])
 				},
 			}
-			vgs, err := NewHostLVM(executor).ListVGs()
+			vgs, err := NewHostLVM(executor).ListVGs(true)
 			if tt.wantErr {
 				assert.Error(t, err)
 			} else {
@@ -603,3 +603,15 @@ func TestHostLVM_execute(t *testing.T) {
 		})
 	}
 }
+
+func Test_untaggedVGs(t *testing.T) {
+	vgs := []VolumeGroup{
+		{Name: "vg1", Tags: []string{"tag1"}},
+		{Name: "vg2", Tags: []string{lvmsTag}},
+	}
+
+	vgs = untaggedVGs(vgs)
+
+	assert.Len(t, vgs, 1)
+	assert.Equal(t, "vg1", vgs[0].Name)
+}

internal/controllers/vgmanager/status.go

@@ -164,7 +164,7 @@ func (r *Reconciler) removeVolumeGroupStatus(ctx context.Context, vg *lvmv1alpha
 }
 
 func (r *Reconciler) setDevices(status *lvmv1alpha1.VGStatus, devices *FilteredBlockDevices) (bool, error) {
-	vgs, err := r.LVM.ListVGs()
+	vgs, err := r.LVM.ListVGs(true)
 	if err != nil {
 		return false, fmt.Errorf("failed to list volume groups. %v", err)
 	}