Skip to content

Commit b2145cd

Browse files
openshift-merge-bot[bot]openshift-merge-bot[bot]
authored
Merge pull request #809 from openshift-cherrypick-robot/cherry-pick-808-to-release-4.18
[release-4.18] OCPBUGS-46359: Use bytes as unit in lvm commands
2 parents 165ed25 + 3ab4b0f commit b2145cd

File tree

5 files changed

+26
-58
lines changed

5 files changed

+26
-58
lines changed

‎internal/controllers/vgmanager/controller.go‎

Lines changed: 3 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -657,11 +657,8 @@ func (r *Reconciler) extendThinPool(ctx context.Context, vgName string, lvSize s
657657
if lvSize == "" {
658658
return fmt.Errorf("lvSize is empty and cannot be used for extension")
659659
}
660-
if len(lvSize) < 2 {
661-
return fmt.Errorf("lvSize is too short (maybe missing unit) and cannot be used for extension")
662-
}
663660

664-
thinPoolSize, err := strconv.ParseFloat(lvSize[:len(lvSize)-1], 64)
661+
thinPoolSize, err := strconv.ParseFloat(lvSize, 64)
665662
if err != nil {
666663
return fmt.Errorf("failed to parse lvSize. %v", err)
667664
}
@@ -673,19 +670,8 @@ func (r *Reconciler) extendThinPool(ctx context.Context, vgName string, lvSize s
673670
if vg.VgSize == "" {
674671
return fmt.Errorf("VgSize is empty and cannot be used for extension")
675672
}
676-
if len(vg.VgSize) < 2 {
677-
return fmt.Errorf("VgSize is too short (maybe missing unit) and cannot be used for extension")
678-
}
679-
680-
if vgUnit, lvUnit := vg.VgSize[len(vg.VgSize)-1], lvSize[len(lvSize)-1]; vgUnit != lvUnit {
681-
return fmt.Errorf("VgSize (%s) and lvSize (%s), units do not match and cannot be used for extension",
682-
string(vgUnit), string(lvUnit))
683-
} else if string(vgUnit) != "g" {
684-
return fmt.Errorf("VgSize (%s) and lvSize (%s), units are not in floating point based gibibytes and cannot be used for extension",
685-
string(vgUnit), string(lvUnit))
686-
}
687673

688-
vgSize, err := strconv.ParseFloat(vg.VgSize[:len(vg.VgSize)-1], 64)
674+
vgSize, err := strconv.ParseFloat(vg.VgSize, 64)
689675
if err != nil {
690676
return fmt.Errorf("failed to parse vgSize. %v", err)
691677
}
@@ -709,7 +695,7 @@ func (r *Reconciler) verifyMetadataSize(ctx context.Context, vgName, lvName stri
709695
return nil
710696
}
711697

712-
currentMetadataSize, err := strconv.ParseInt(lvMetadataSize[:len(lvMetadataSize)-1], 10, 64)
698+
currentMetadataSize, err := strconv.ParseInt(lvMetadataSize, 10, 64)
713699
if err != nil {
714700
return fmt.Errorf("failed to parse metadata size. %w", err)
715701
}

‎internal/controllers/vgmanager/controller_test.go‎

Lines changed: 14 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -717,20 +717,20 @@ func testMetadataSizeExtension(ctx context.Context) {
717717
}
718718

719719
By("skip metadata extension when policy set to Host")
720-
err := r.verifyMetadataSize(ctx, "vg1", cfg.Name, "1024b", convertMetadataSize(&cfg))
720+
err := r.verifyMetadataSize(ctx, "vg1", cfg.Name, "1024", convertMetadataSize(&cfg))
721721
Expect(err).NotTo(HaveOccurred(), "should not error if metadata size calculation policy is Host")
722722

723723
By("skip metadata extension when metadata size is same")
724724
cfg.MetadataSizeCalculationPolicy = lvmv1alpha1.MetadataSizePolicyStatic
725725
cfg.MetadataSize = ptr.To(resource.MustParse("1Mi"))
726-
err = r.verifyMetadataSize(ctx, "vg1", cfg.Name, fmt.Sprintf("%vb", cfg.MetadataSize.Value()), convertMetadataSize(&cfg))
726+
err = r.verifyMetadataSize(ctx, "vg1", cfg.Name, fmt.Sprintf("%v", cfg.MetadataSize.Value()), convertMetadataSize(&cfg))
727727
Expect(err).NotTo(HaveOccurred(), "should not error because metadata size is the same")
728728

729729
By("extend metadata size when provided is bigger than actual")
730730
oldSize := cfg.MetadataSize
731731
cfg.MetadataSize = ptr.To(resource.MustParse("1Gi"))
732732
mockLVM.EXPECT().ExtendThinPoolMetadata(ctx, cfg.Name, "vg1", cfg.MetadataSize.Value()).Return(nil).Once()
733-
err = r.verifyMetadataSize(ctx, "vg1", cfg.Name, fmt.Sprintf("%vb", oldSize.Value()), convertMetadataSize(&cfg))
733+
err = r.verifyMetadataSize(ctx, "vg1", cfg.Name, fmt.Sprintf("%v", oldSize.Value()), convertMetadataSize(&cfg))
734734
Expect(err).NotTo(HaveOccurred(), "should not error when metadata extended")
735735
}
736736

@@ -744,58 +744,40 @@ func testThinPoolExtension(ctx context.Context) {
744744
err := r.extendThinPool(ctx, "vg1", "", &lvmv1alpha1.ThinPoolConfig{})
745745
Expect(err).To(HaveOccurred(), "should error if lvSize is empty")
746746

747-
err = r.extendThinPool(ctx, "vg1", "1", &lvmv1alpha1.ThinPoolConfig{})
748-
Expect(err).To(HaveOccurred(), "should error if lvSize has no unit")
749-
750747
mockLVM.EXPECT().GetVG(ctx, "vg1").Once().Return(lvm.VolumeGroup{}, fmt.Errorf("mocked error"))
751-
err = r.extendThinPool(ctx, "vg1", "26.96g", &lvmv1alpha1.ThinPoolConfig{})
748+
err = r.extendThinPool(ctx, "vg1", "2147483648", &lvmv1alpha1.ThinPoolConfig{})
752749
Expect(err).To(HaveOccurred(), "should error if GetVG fails")
753750

754751
err = r.extendThinPool(ctx, "vg1", "26.96gxxx", &lvmv1alpha1.ThinPoolConfig{})
755752
Expect(err).To(HaveOccurred(), "should error if lvSize is malformatted")
756753

757754
lvmVG := lvm.VolumeGroup{Name: "vg1"}
758755
mockLVM.EXPECT().GetVG(ctx, "vg1").Return(lvmVG, nil).Once()
759-
err = r.extendThinPool(ctx, "vg1", "2g", &lvmv1alpha1.ThinPoolConfig{})
756+
err = r.extendThinPool(ctx, "vg1", "2147483648", &lvmv1alpha1.ThinPoolConfig{})
760757
Expect(err).To(HaveOccurred(), "should error if vgSize is empty")
761758

762-
lvmVG.VgSize = "1"
763-
mockLVM.EXPECT().GetVG(ctx, "vg1").Return(lvmVG, nil).Once()
764-
err = r.extendThinPool(ctx, "vg1", "2g", &lvmv1alpha1.ThinPoolConfig{})
765-
Expect(err).To(HaveOccurred(), "should error if vgSize has no unit")
766-
767-
lvmVG.VgSize = "1m"
768-
mockLVM.EXPECT().GetVG(ctx, "vg1").Return(lvmVG, nil).Once()
769-
err = r.extendThinPool(ctx, "vg1", "2g", &lvmv1alpha1.ThinPoolConfig{})
770-
Expect(err).To(HaveOccurred(), "should error if vg unit does not match lv unit")
771-
772-
lvmVG.VgSize = "1m"
773-
mockLVM.EXPECT().GetVG(ctx, "vg1").Return(lvmVG, nil).Once()
774-
err = r.extendThinPool(ctx, "vg1", "2m", &lvmv1alpha1.ThinPoolConfig{})
775-
Expect(err).To(HaveOccurred(), "should error if unit is not gibibytes")
776-
777759
lvmVG.VgSize = "1123xxg"
778760
mockLVM.EXPECT().GetVG(ctx, "vg1").Return(lvmVG, nil).Once()
779-
err = r.extendThinPool(ctx, "vg1", "2g", &lvmv1alpha1.ThinPoolConfig{})
761+
err = r.extendThinPool(ctx, "vg1", "2147483648", &lvmv1alpha1.ThinPoolConfig{})
780762
Expect(err).To(HaveOccurred(), "should error if vgSize is malformatted")
781763

782-
lvmVG.VgSize = "3g"
764+
lvmVG.VgSize = "3221225472"
783765
mockLVM.EXPECT().GetVG(ctx, "vg1").Return(lvmVG, nil).Once()
784-
err = r.extendThinPool(ctx, "vg1", "3g", &lvmv1alpha1.ThinPoolConfig{})
766+
err = r.extendThinPool(ctx, "vg1", "3221225472", &lvmv1alpha1.ThinPoolConfig{})
785767
Expect(err).ToNot(HaveOccurred(), "should fast skip if no expansion is needed")
786768

787-
lvmVG.VgSize = "5g"
769+
lvmVG.VgSize = "5368709120"
788770
thinPool := &lvmv1alpha1.ThinPoolConfig{Name: "thin-pool-1", SizePercent: 90}
789771
mockLVM.EXPECT().GetVG(ctx, "vg1").Return(lvmVG, nil).Once()
790772
mockLVM.EXPECT().ExtendLV(ctx, thinPool.Name, "vg1", thinPool.SizePercent).
791773
Once().Return(fmt.Errorf("failed to extend lv"))
792-
err = r.extendThinPool(ctx, "vg1", "3g", thinPool)
774+
err = r.extendThinPool(ctx, "vg1", "3221225472", thinPool)
793775
Expect(err).To(HaveOccurred(), "should fail if lvm extension fails")
794776

795777
mockLVM.EXPECT().GetVG(ctx, "vg1").Return(lvmVG, nil).Once()
796778
mockLVM.EXPECT().ExtendLV(ctx, thinPool.Name, "vg1", thinPool.SizePercent).
797779
Once().Return(nil)
798-
err = r.extendThinPool(ctx, "vg1", "3g", thinPool)
780+
err = r.extendThinPool(ctx, "vg1", "3221225472", thinPool)
799781
Expect(err).ToNot(HaveOccurred(), "succeed if lvm extension succeeds")
800782
}
801783

@@ -841,9 +823,9 @@ func testThinPoolCreation(ctx context.Context) {
841823
err = r.addThinPoolToVG(ctx, "vg1", thinPool)
842824
Expect(err).ToNot(HaveOccurred(), "should create thin pool if it does not exist")
843825

844-
lvmVG := lvm.VolumeGroup{Name: "vg1", VgSize: "5g"}
826+
lvmVG := lvm.VolumeGroup{Name: "vg1", VgSize: "5368709120"}
845827
mockLVM.EXPECT().ListLVs(ctx, "vg1").Once().Return(&lvm.LVReport{Report: []lvm.LVReportItem{{
846-
Lv: []lvm.LogicalVolume{{Name: "thin-pool-1", VgName: "vg1", LvAttr: "twi---tz--", LvSize: "3g"}},
828+
Lv: []lvm.LogicalVolume{{Name: "thin-pool-1", VgName: "vg1", LvAttr: "twi---tz--", LvSize: "3221225472"}},
847829
}}}, nil)
848830
mockLVM.EXPECT().GetVG(ctx, "vg1").Once().Return(lvmVG, nil)
849831
mockLVM.EXPECT().ExtendLV(ctx, thinPool.Name, "vg1", thinPool.SizePercent).
@@ -962,7 +944,7 @@ func testReconcileFailure(ctx context.Context) {
962944
{Name: "/dev/sda", KName: "/dev/sda", FSType: "xfs", PartLabel: "reserved"},
963945
}, nil)
964946
vgs := []lvm.VolumeGroup{
965-
{Name: "vg1", VgSize: "1g"},
947+
{Name: "vg1", VgSize: "1073741824"},
966948
}
967949
instances.LVM.EXPECT().ListVGs(ctx, true).Once().Return(vgs, nil)
968950
instances.LVM.EXPECT().ListPVs(ctx, "").Once().Return(nil, nil)

‎internal/controllers/vgmanager/lvm/lvm.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -280,7 +280,7 @@ func (hlvm *HostLVM) GetVG(ctx context.Context, name string) (VolumeGroup, error
280280
res := new(VGReport)
281281

282282
args := []string{
283-
DefaultTag, "--units", "g", "--reportformat", "json",
283+
DefaultTag, "--units", "b", "--nosuffix", "--reportformat", "json",
284284
}
285285
if err := hlvm.RunCommandAsHostInto(ctx, res, vgsCmd, args...); err != nil {
286286
return VolumeGroup{}, fmt.Errorf("failed to list volume groups. %v", err)
@@ -317,7 +317,7 @@ func (hlvm *HostLVM) GetVG(ctx context.Context, name string) (VolumeGroup, error
317317
func (hlvm *HostLVM) ListPVs(ctx context.Context, vgName string) ([]PhysicalVolume, error) {
318318
res := new(PVReport)
319319
args := []string{
320-
"--units", "g", "-v", "--reportformat", "json",
320+
"--units", "b", "--nosuffix", "-v", "--reportformat", "json",
321321
}
322322
if vgName != "" {
323323
args = append(args, "-S", fmt.Sprintf("vgname=%s", vgName))
@@ -349,7 +349,7 @@ func (hlvm *HostLVM) ListVGs(ctx context.Context, tagged bool) ([]VolumeGroup, e
349349
res := new(VGReport)
350350

351351
args := []string{
352-
"-o", "vg_name,vg_size,vg_tags", "--units", "g", "--reportformat", "json",
352+
"-o", "vg_name,vg_size,vg_tags", "--units", "b", "--nosuffix", "--reportformat", "json",
353353
}
354354
if tagged {
355355
args = append(args, DefaultTag)

‎internal/controllers/vgmanager/lvm/lvm_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ func TestHostLVM_GetVG(t *testing.T) {
100100
return fmt.Errorf("mocked error")
101101
}
102102
argsConcat := strings.Join(args, " ")
103-
out := "--units g -v --reportformat json -S vgname=%s"
103+
out := "--units b --nosuffix -v --reportformat json -S vgname=%s"
104104
if argsConcat == fmt.Sprintf(out, "vg1") {
105105
return json.Unmarshal([]byte(mockPvsOutputForVG1), &into)
106106
} else if argsConcat == fmt.Sprintf(out, "vg2") {

‎internal/controllers/vgmanager/validatelvs_test.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ var mockLvsOutputWrongLVsInReport = `
4141
"report": [
4242
{
4343
"lv": [
44-
{"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":"", "metadata_size":"1073741824b"}
44+
{"lv_name":"thin-pool-BLUB", "vg_name":"vg1", "lv_attr":"twi-a-tz--", "lv_size":"28948079575", "pool_lv":"", "origin":"", "data_percent":"0.00", "metadata_percent":"10.52", "move_pv":"", "mirror_log":"", "copy_percent":"", "convert_lv":"", "metadata_size":"1073741824"}
4545
]
4646
}
4747
]
@@ -53,7 +53,7 @@ var mockLvsOutputThinPoolValid = `
5353
"report": [
5454
{
5555
"lv": [
56-
{"lv_name":"thin-pool-1", "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":"", "chunk_size": "524288", "metadata_size":"1073741824b"}
56+
{"lv_name":"thin-pool-1", "vg_name":"vg1", "lv_attr":"twi-a-tz--", "lv_size":"28948079575", "pool_lv":"", "origin":"", "data_percent":"0.00", "metadata_percent":"10.52", "move_pv":"", "mirror_log":"", "copy_percent":"", "convert_lv":"", "chunk_size": "524288", "metadata_size":"1073741824"}
5757
]
5858
}
5959
]
@@ -65,7 +65,7 @@ var mockLvsOutputThinPoolHighMetadataUse = `
6565
"report": [
6666
{
6767
"lv": [
68-
{"lv_name":"thin-pool-1", "vg_name":"vg1", "lv_attr":"twi-a-tz--", "lv_size":"26.96g", "pool_lv":"", "origin":"", "data_percent":"0.00", "metadata_percent":"98.52", "move_pv":"", "mirror_log":"", "copy_percent":"", "convert_lv":"", "metadata_size":"1073741824b"}
68+
{"lv_name":"thin-pool-1", "vg_name":"vg1", "lv_attr":"twi-a-tz--", "lv_size":"28948079575", "pool_lv":"", "origin":"", "data_percent":"0.00", "metadata_percent":"98.52", "move_pv":"", "mirror_log":"", "copy_percent":"", "convert_lv":"", "metadata_size":"1073741824"}
6969
]
7070
}
7171
]
@@ -76,7 +76,7 @@ var mockLvsOutputThinPoolSuspended = `
7676
"report": [
7777
{
7878
"lv": [
79-
{"lv_name":"thin-pool-1", "vg_name":"vg1", "lv_attr":"twi-s-tz--", "lv_size":"26.96g", "pool_lv":"", "origin":"", "data_percent":"0.00", "metadata_percent":"10.52", "move_pv":"", "mirror_log":"", "copy_percent":"", "convert_lv":"", "metadata_size":"1073741824b"}
79+
{"lv_name":"thin-pool-1", "vg_name":"vg1", "lv_attr":"twi-s-tz--", "lv_size":"28948079575", "pool_lv":"", "origin":"", "data_percent":"0.00", "metadata_percent":"10.52", "move_pv":"", "mirror_log":"", "copy_percent":"", "convert_lv":"", "metadata_size":"1073741824"}
8080
]
8181
}
8282
]
@@ -88,7 +88,7 @@ var mockLvsOutputRAID = `
8888
"report": [
8989
{
9090
"lv": [
91-
{"lv_name":"thin-pool-1", "vg_name":"vg1", "lv_attr":"rwi-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":"", "metadata_size":"1073741824b"}
91+
{"lv_name":"thin-pool-1", "vg_name":"vg1", "lv_attr":"rwi-a-tz--", "lv_size":"28948079575", "pool_lv":"", "origin":"", "data_percent":"0.00", "metadata_percent":"10.52", "move_pv":"", "mirror_log":"", "copy_percent":"", "convert_lv":"", "metadata_size":"1073741824"}
9292
]
9393
}
9494
]

0 commit comments

Comments
 (0)