Skip to content

Commit 8c2408a

Browse files
qJkeeqJkee
committed
add metadata size field
1 parent fc195c5 commit 8c2408a

11 files changed

+254
-34
lines changed

‎api/v1alpha1/lvmcluster_types.go

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -69,11 +69,37 @@ type ThinPoolConfig struct {
6969
// +optional
7070
ChunkSize *resource.Quantity `json:"chunkSize,omitempty"`
7171

72-
// MetadataSize specifies metadata size for thin pool.
72+
// MetadataSize specifies metadata size for thin pool. It used only when MetadataSizeCalculationPolicy
73+
// is set to Static. No MetadataSize with a MetadataSizeCalculationPolicy set to Static will result in
74+
// default metadata size of 1Gi. It can be between 2Mi and 16Gi due to the underlying limitations of lvm2.
7375
// +optional
7476
MetadataSize *resource.Quantity `json:"metadataSize,omitempty"`
77+
78+
// MetadataSizeCalculationPolicy specifies the policy to calculate metadata size for the underlying volume.
79+
// When set to Host, the metadata size is calculated based on lvm2 default settings
80+
// When set to Static, the metadata size is calculated based on the static size attribute provided within MetadataSize
81+
// +kubebuilder:default=Host
82+
// +kubebuilder:validation:Enum=Host;Static
83+
// +required
84+
MetadataSizeCalculationPolicy MetadataSizePolicy `json:"metadataSizeCalculationPolicy,omitempty"`
7585
}
7686

87+
// MetadataSizePolicy specifies the policy to calculate the metadata size for the underlying volume.
88+
type MetadataSizePolicy string
89+
90+
const (
91+
// MetadataSizePolicyHost calculates the metadata size based on the lvm2 default settings.
92+
MetadataSizePolicyHost MetadataSizePolicy = "Host"
93+
// MetadataSizePolicyStatic calculates the metadata size based on a static size attribute.
94+
MetadataSizePolicyStatic MetadataSizePolicy = "Static"
95+
)
96+
97+
var (
98+
ThinPoolMetadataSizeMinimum = resource.MustParse("2Mi")
99+
ThinPoolMetadataSizeMaximum = resource.MustParse("16Gi")
100+
ThinPoolMetadataSizeDefault = resource.MustParse("1Gi")
101+
)
102+
77103
// ChunkSizeCalculationPolicy specifies the policy to calculate the chunk size for the underlying volume.
78104
// for more information, see man lvm.
79105
type ChunkSizeCalculationPolicy string
@@ -91,10 +117,6 @@ var (
91117
ChunkSizeMaximum = resource.MustParse("1Gi")
92118
)
93119

94-
var (
95-
ThinPoolMetadataSizeDefault = resource.MustParse("1Gi")
96-
)
97-
98120
type DeviceFilesystemType string
99121

100122
const (

‎api/v1alpha1/lvmcluster_webhook.go

Lines changed: 42 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ var (
5656
ErrEmptyPathsWithMultipleDeviceClasses = errors.New("path list should not be empty when there are multiple deviceClasses")
5757
ErrDuplicateLVMCluster = errors.New("duplicate LVMClusters are not allowed, remove the old LVMCluster or work with the existing instance")
5858
ErrThinPoolConfigCannotBeChanged = errors.New("ThinPoolConfig can not be changed")
59+
ErrThinPoolMetadataSizeCanBeOnlyIncreased = errors.New("thin pool metadata size can be only increased")
5960
ErrNodeSelectorCannotBeChanged = errors.New("NodeSelector can not be changed")
6061
ErrDevicePathsCannotBeAddedInUpdate = errors.New("device paths can not be added after a device class has been initialized")
6162
ErrForceWipeOptionCannotBeChanged = errors.New("ForceWipeDevicesAndDestroyAllData can not be changed")
@@ -126,6 +127,11 @@ func (v *lvmClusterValidator) ValidateCreate(ctx context.Context, obj runtime.Ob
126127
return warnings, err
127128
}
128129

130+
metadataWarnings, err := v.verifyMetadataSize(l)
131+
if err != nil {
132+
return warnings, err
133+
}
134+
warnings = append(warnings, metadataWarnings...)
129135
return warnings, nil
130136
}
131137

@@ -198,12 +204,18 @@ func (v *lvmClusterValidator) ValidateUpdate(_ context.Context, old, new runtime
198204
return warnings, fmt.Errorf("ThinPoolConfig.ChunkSize is invalid: %w", ErrThinPoolConfigCannotBeChanged)
199205
}
200206

201-
if newThinPoolConfig.MetadataSize != nil {
202-
if oldThinPoolConfig.MetadataSize == nil {
203-
oldThinPoolConfig.MetadataSize = &ThinPoolMetadataSizeDefault
207+
if newThinPoolConfig.MetadataSizeCalculationPolicy == MetadataSizePolicyStatic {
208+
if newThinPoolConfig.MetadataSize == nil {
209+
warnings = append(warnings, "thin pool metadata size is unset. LVMS operator will automatically set it to 1Gb and grow metadata size if needed")
210+
newThinPoolConfig.MetadataSize = &ThinPoolMetadataSizeDefault
204211
}
205-
if newThinPoolConfig.MetadataSize.Value() < oldThinPoolConfig.MetadataSize.Value() {
206-
return warnings, fmt.Errorf("ThinPoolConfig.MetadataSize is invalid: %w", ErrThinPoolConfigCannotBeChanged)
212+
if oldThinPoolConfig.MetadataSizeCalculationPolicy == MetadataSizePolicyStatic {
213+
if oldThinPoolConfig.MetadataSize == nil {
214+
oldThinPoolConfig.MetadataSize = &ThinPoolMetadataSizeDefault
215+
}
216+
if newThinPoolConfig.MetadataSize.Value() < oldThinPoolConfig.MetadataSize.Value() {
217+
return warnings, fmt.Errorf("ThinPoolConfig.MetadataSize is invalid: %w", ErrThinPoolMetadataSizeCanBeOnlyIncreased)
218+
}
207219
}
208220
}
209221
}
@@ -527,3 +539,28 @@ func (v *lvmClusterValidator) verifyChunkSize(l *LVMCluster) error {
527539

528540
return nil
529541
}
542+
543+
func (v *lvmClusterValidator) verifyMetadataSize(l *LVMCluster) ([]string, error) {
544+
warnings := make([]string, 0)
545+
for _, dc := range l.Spec.Storage.DeviceClasses {
546+
if dc.ThinPoolConfig == nil {
547+
continue
548+
}
549+
if dc.ThinPoolConfig.MetadataSizeCalculationPolicy == MetadataSizePolicyHost && dc.ThinPoolConfig.MetadataSize != nil {
550+
return warnings, fmt.Errorf("metadata size can not be set when metadata size calculation policy is set to Host")
551+
}
552+
if dc.ThinPoolConfig.MetadataSizeCalculationPolicy == MetadataSizePolicyStatic && dc.ThinPoolConfig.MetadataSize == nil {
553+
warnings = append(warnings, "metadata size in unset. LVMS will set it to 1Gi by default")
554+
dc.ThinPoolConfig.MetadataSize = &ThinPoolMetadataSizeDefault
555+
}
556+
if dc.ThinPoolConfig.MetadataSize != nil {
557+
if dc.ThinPoolConfig.ChunkSize.Cmp(ThinPoolMetadataSizeMinimum) < 0 {
558+
return warnings, fmt.Errorf("metadata size must be greater than or equal to %s", ThinPoolMetadataSizeMinimum.String())
559+
}
560+
if dc.ThinPoolConfig.MetadataSize.Cmp(ThinPoolMetadataSizeMaximum) > 0 {
561+
return warnings, fmt.Errorf("metadata size must be less than or equal to %s", ThinPoolMetadataSizeMaximum.String())
562+
}
563+
}
564+
}
565+
return warnings, nil
566+
}

‎bundle/manifests/lvm.topolvm.io_lvmclusters.yaml

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,22 @@ spec:
213213
- Host
214214
- Static
215215
type: string
216+
metadataSize:
217+
anyOf:
218+
- type: integer
219+
- type: string
220+
description: MetadataSize specifies metadata size for
221+
thin pool.
222+
pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
223+
x-kubernetes-int-or-string: true
224+
metadataSizeCalculationPolicy:
225+
default: Host
226+
description: MetadataSizeCalculationPolicy used to set
227+
policy for metadata
228+
enum:
229+
- Host
230+
- Static
231+
type: string
216232
name:
217233
description: Name specifies a name for the thin pool.
218234
type: string

‎bundle/manifests/lvm.topolvm.io_lvmvolumegroups.yaml

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,21 @@ spec:
176176
- Host
177177
- Static
178178
type: string
179+
metadataSize:
180+
anyOf:
181+
- type: integer
182+
- type: string
183+
description: MetadataSize specifies metadata size for thin pool.
184+
pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
185+
x-kubernetes-int-or-string: true
186+
metadataSizeCalculationPolicy:
187+
default: Host
188+
description: MetadataSizeCalculationPolicy used to set policy
189+
for metadata
190+
enum:
191+
- Host
192+
- Static
193+
type: string
179194
name:
180195
description: Name specifies a name for the thin pool.
181196
type: string

‎config/crd/bases/lvm.topolvm.io_lvmclusters.yaml

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -213,10 +213,22 @@ spec:
213213
anyOf:
214214
- type: integer
215215
- type: string
216-
description: MetadataSize specifies metadata size for
217-
thin pool.
216+
description: |-
217+
MetadataSize specifies metadata size for thin pool. It used only when MetadataSizeCalculationPolicy
218+
is set to Static. No MetadataSize with a MetadataSizeCalculationPolicy set to Static will result in
219+
default metadata size of 1Gi. It can be between 2Mi and 16Gi due to the underlying limitations of lvm2.
218220
pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
219221
x-kubernetes-int-or-string: true
222+
metadataSizeCalculationPolicy:
223+
default: Host
224+
description: |-
225+
MetadataSizeCalculationPolicy specifies the policy to calculate metadata size for the underlying volume.
226+
When set to Host, the metadata size is calculated based on lvm2 default settings
227+
When set to Static, the metadata size is calculated based on the static size attribute provided within MetadataSize
228+
enum:
229+
- Host
230+
- Static
231+
type: string
220232
name:
221233
description: Name specifies a name for the thin pool.
222234
type: string

‎config/crd/bases/lvm.topolvm.io_lvmvolumegroups.yaml

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,9 +180,22 @@ spec:
180180
anyOf:
181181
- type: integer
182182
- type: string
183-
description: MetadataSize specifies metadata size for thin pool.
183+
description: |-
184+
MetadataSize specifies metadata size for thin pool. It used only when MetadataSizeCalculationPolicy
185+
is set to Static. No MetadataSize with a MetadataSizeCalculationPolicy set to Static will result in
186+
default metadata size of 1Gi. It can be between 2Mi and 16Gi due to the underlying limitations of lvm2.
184187
pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
185188
x-kubernetes-int-or-string: true
189+
metadataSizeCalculationPolicy:
190+
default: Host
191+
description: |-
192+
MetadataSizeCalculationPolicy specifies the policy to calculate metadata size for the underlying volume.
193+
When set to Host, the metadata size is calculated based on lvm2 default settings
194+
When set to Static, the metadata size is calculated based on the static size attribute provided within MetadataSize
195+
enum:
196+
- Host
197+
- Static
198+
type: string
186199
name:
187200
description: Name specifies a name for the thin pool.
188201
type: string

‎config/manager/kustomization.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ apiVersion: kustomize.config.k8s.io/v1beta1
77
kind: Kustomization
88
images:
99
- name: controller
10-
newName: quay.io/bzamalut/lvms
10+
newName: quay.io/lvms_dev/lvms-operator
1111
newTag: latest
1212
namePrefix: lvms-
1313
namespace: openshift-storage

‎internal/controllers/vgmanager/controller.go

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -575,6 +575,10 @@ func (r *Reconciler) validateLVs(ctx context.Context, volumeGroup *lvmv1alpha1.L
575575
return err
576576
}
577577

578+
if err := r.verifyMetadataSize(ctx, volumeGroup.Name, lv.Name, lv.MetadataSize, convertMetadataSize(volumeGroup.Spec.ThinPoolConfig)); err != nil {
579+
return fmt.Errorf("failed to verify metadata size for thinpool %s in volume group %s: %w", volumeGroup.Spec.ThinPoolConfig.Name, volumeGroup.Name, err)
580+
}
581+
578582
logger.V(1).Info("confirmed created logical volume has correct attributes", "lv_attr", lvAttr.String())
579583
}
580584
if !thinPoolExists {
@@ -617,12 +621,7 @@ func (r *Reconciler) addThinPoolToVG(ctx context.Context, vgName string, config
617621

618622
logger.Info("creating lvm thinpool")
619623

620-
metadataSize := lvmv1alpha1.ThinPoolMetadataSizeDefault.Value()
621-
if config.MetadataSize != nil {
622-
metadataSize = config.MetadataSize.Value()
623-
}
624-
625-
if err := r.LVM.CreateLV(ctx, config.Name, vgName, config.SizePercent, convertChunkSize(config), metadataSize); err != nil {
624+
if err := r.LVM.CreateLV(ctx, config.Name, vgName, config.SizePercent, convertChunkSize(config), convertMetadataSize(config)); err != nil {
626625
return fmt.Errorf("failed to create thinpool: %w", err)
627626
}
628627
logger.Info("successfully created thinpool")
@@ -642,6 +641,16 @@ func convertChunkSize(config *lvmv1alpha1.ThinPoolConfig) int64 {
642641
return config.ChunkSize.Value()
643642
}
644643

644+
func convertMetadataSize(config *lvmv1alpha1.ThinPoolConfig) int64 {
645+
if config.MetadataSizeCalculationPolicy == lvmv1alpha1.MetadataSizePolicyHost {
646+
return -1
647+
}
648+
if config.MetadataSize == nil {
649+
return lvmv1alpha1.ThinPoolMetadataSizeDefault.Value()
650+
}
651+
return config.MetadataSize.Value()
652+
}
653+
645654
func (r *Reconciler) extendThinPool(ctx context.Context, vgName string, lvSize string, config *lvmv1alpha1.ThinPoolConfig) error {
646655
logger := log.FromContext(ctx).WithValues("VGName", vgName)
647656
logger = logger.WithValues("ThinPool", config.Name)
@@ -695,6 +704,26 @@ func (r *Reconciler) extendThinPool(ctx context.Context, vgName string, lvSize s
695704
return nil
696705
}
697706

707+
func (r *Reconciler) verifyMetadataSize(ctx context.Context, vgName, lvName string, lvMetadataSize string, newMetadataSize int64) error {
708+
if newMetadataSize == -1 {
709+
return nil
710+
}
711+
712+
currentMetadataSize, err := strconv.ParseInt(lvMetadataSize[:len(lvMetadataSize)-1], 10, 64)
713+
if err != nil {
714+
return fmt.Errorf("failed to parse metadata size. %w", err)
715+
}
716+
if currentMetadataSize >= newMetadataSize {
717+
return nil
718+
}
719+
720+
err = r.LVM.ExtendThinPoolMetadata(ctx, lvName, vgName, newMetadataSize)
721+
if err != nil {
722+
return fmt.Errorf("failed to extend thinpool metadata: %w", err)
723+
}
724+
return nil
725+
}
726+
698727
func (r *Reconciler) matchesThisNode(ctx context.Context, selector *corev1.NodeSelector) (bool, error) {
699728
node := &corev1.Node{}
700729
err := r.Client.Get(ctx, types.NamespacedName{Name: r.NodeName}, node)

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

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ var (
6060
"lv_size",
6161
"metadata_percent",
6262
"chunk_size",
63+
"lv_metadata_size",
6364
}
6465
)
6566

@@ -98,6 +99,7 @@ type LogicalVolume struct {
9899
LvSize string `json:"lv_size"`
99100
MetadataPercent string `json:"metadata_percent"`
100101
ChunkSize string `json:"chunk_size"`
102+
MetadataSize string `json:"lv_metadata_size"`
101103
}
102104

103105
type LVM interface {
@@ -115,6 +117,7 @@ type LVM interface {
115117
LVExists(ctx context.Context, lvName, vgName string) (bool, error)
116118
CreateLV(ctx context.Context, lvName, vgName string, sizePercent int, chunkSizeBytes, metadataSizeBytes int64) error
117119
ExtendLV(ctx context.Context, lvName, vgName string, sizePercent int) error
120+
ExtendThinPoolMetadata(ctx context.Context, lvName, vgName string, size int64) error
118121
ActivateLV(ctx context.Context, lvName, vgName string) error
119122
DeleteLV(ctx context.Context, lvName, vgName string) error
120123
}
@@ -489,7 +492,10 @@ func (hlvm *HostLVM) CreateLV(ctx context.Context, lvName, vgName string, sizePe
489492
args = append(args, "-c", fmt.Sprintf("%vb", chunkSizeBytes))
490493
}
491494

492-
args = append(args, "--poolmetadatasize", fmt.Sprintf("%vb", metadataSizeBytes))
495+
if metadataSizeBytes > 0 {
496+
args = append(args, "--poolmetadatasize", fmt.Sprintf("%vb", metadataSizeBytes))
497+
}
498+
493499
args = append(args, fmt.Sprintf("%s/%s", vgName, lvName))
494500

495501
if err := hlvm.RunCommandAsHost(ctx, lvCreateCmd, args...); err != nil {
@@ -522,6 +528,26 @@ func (hlvm *HostLVM) ExtendLV(ctx context.Context, lvName, vgName string, sizePe
522528
return nil
523529
}
524530

531+
func (hlvm *HostLVM) ExtendThinPoolMetadata(ctx context.Context, lvName, vgName string, size int64) error {
532+
if vgName == "" {
533+
return fmt.Errorf("failed to extend logical volume metadata size in volume group: volume group name is empty")
534+
}
535+
if lvName == "" {
536+
return fmt.Errorf("failed to extend logical volume metadata size in volume group: logical volume name is empty")
537+
}
538+
if size <= 0 {
539+
return fmt.Errorf("failed to extend logical volume metadata size in volume group: size percent should be greater than 0")
540+
}
541+
542+
args := []string{"--poolmetadatasize", fmt.Sprintf("%vb", size), fmt.Sprintf("%s/%s", vgName, lvName)}
543+
err := hlvm.RunCommandAsHost(ctx, lvExtendCmd, args...)
544+
if err != nil {
545+
return fmt.Errorf("failed to extend logical volume metadata size %q in the volume group %q using command '%s': %w",
546+
lvName, vgName, fmt.Sprintf("%s %s", lvExtendCmd, strings.Join(args, " ")), err)
547+
}
548+
return nil
549+
}
550+
525551
// ActivateLV activates the logical volume
526552
func (hlvm *HostLVM) ActivateLV(ctx context.Context, lvName, vgName string) error {
527553
if vgName == "" {

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

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -426,19 +426,20 @@ func TestHostLVM_ListLVsByName(t *testing.T) {
426426

427427
func TestHostLVM_CreateLV(t *testing.T) {
428428
tests := []struct {
429-
name string
430-
lvName string
431-
vgName string
432-
sizePercent int
433-
chunkSizeBytes int64
434-
wantErr bool
435-
execErr bool
429+
name string
430+
lvName string
431+
vgName string
432+
sizePercent int
433+
chunkSizeBytes int64
434+
metadataSizeBytes int64
435+
wantErr bool
436+
execErr bool
436437
}{
437-
{"Empty Volume Group Name", "lv1", "", 10, lvmv1alpha1.ChunkSizeDefault.Value(), true, false},
438-
{"Empty Logical Volume Name", "", "vg1", 10, lvmv1alpha1.ChunkSizeDefault.Value(), true, false},
439-
{"Invalid SizePercent", "lv1", "vg1", -10, lvmv1alpha1.ChunkSizeDefault.Value(), true, false},
440-
{"Error on Exec", "lv1", "vg1", 10, lvmv1alpha1.ChunkSizeDefault.Value(), true, true},
441-
{"LV created successfully", "lv1", "vg1", 10, lvmv1alpha1.ChunkSizeDefault.Value(), false, false},
438+
{"Empty Volume Group Name", "lv1", "", 10, lvmv1alpha1.ChunkSizeDefault.Value(), lvmv1alpha1.ThinPoolMetadataSizeDefault.Value(), true, false},
439+
{"Empty Logical Volume Name", "", "vg1", 10, lvmv1alpha1.ChunkSizeDefault.Value(), lvmv1alpha1.ThinPoolMetadataSizeDefault.Value(), true, false},
440+
{"Invalid SizePercent", "lv1", "vg1", -10, lvmv1alpha1.ChunkSizeDefault.Value(), lvmv1alpha1.ThinPoolMetadataSizeDefault.Value(), true, false},
441+
{"Error on Exec", "lv1", "vg1", 10, lvmv1alpha1.ChunkSizeDefault.Value(), lvmv1alpha1.ThinPoolMetadataSizeDefault.Value(), true, true},
442+
{"LV created successfully", "lv1", "vg1", 10, lvmv1alpha1.ChunkSizeDefault.Value(), lvmv1alpha1.ThinPoolMetadataSizeDefault.Value(), false, false},
442443
}
443444

444445
for _, tt := range tests {
@@ -452,7 +453,7 @@ func TestHostLVM_CreateLV(t *testing.T) {
452453
return nil
453454
}}
454455

455-
err := NewHostLVM(executor).CreateLV(ctx, tt.lvName, tt.vgName, tt.sizePercent, tt.chunkSizeBytes)
456+
err := NewHostLVM(executor).CreateLV(ctx, tt.lvName, tt.vgName, tt.sizePercent, tt.chunkSizeBytes, tt.metadataSizeBytes)
456457
if tt.wantErr {
457458
assert.Error(t, err)
458459
} else {

0 commit comments

Comments
 (0)