From 8b01d3b9f7a3385931b7db9890e3a17cd20ab5c2 Mon Sep 17 00:00:00 2001 From: Liran Rotenberg Date: Mon, 11 Dec 2023 11:56:51 +0200 Subject: [PATCH 1/3] Make the block overhead configurable This patch will allow the user to configure the block overhead. Currently it defaults to 0. The overhead is required when migrating to encrypted Ceph RBD (block). The pod may see less space on the disk and therefore the migration will fail when it tries to allocate the last sectors. See more about encrypted Ceph RBD in: https://docs.ceph.com/en/quincy/rbd/rbd-encryption/ The value needs to be set in the ForkliftController spec under the value: `controller_block_overhead`. The addition will be the fixed number provided to the size of the disk. Signed-off-by: Liran Rotenberg --- operator/config/manager/manager.yaml | 2 ++ .../roles/forkliftcontroller/defaults/main.yml | 1 + .../controller/deployment-controller.yml.j2 | 4 ++++ pkg/controller/plan/adapter/openstack/builder.go | 5 +---- pkg/controller/plan/adapter/ovirt/builder.go | 15 +++++---------- pkg/controller/plan/util/utils.go | 10 ++++++++-- pkg/settings/migration.go | 6 ++++++ 7 files changed, 27 insertions(+), 16 deletions(-) diff --git a/operator/config/manager/manager.yaml b/operator/config/manager/manager.yaml index 5313cc2bd..d218e6335 100644 --- a/operator/config/manager/manager.yaml +++ b/operator/config/manager/manager.yaml @@ -66,6 +66,8 @@ spec: value: ${CDI_EXPORT_TOKEN_TTL} - name: FILESYSTEM_OVERHEAD value: ${FILESYSTEM_OVERHEAD} + - name: BLOCK_OVERHEAD + value: ${BLOCK_OVERHEAD} - name: POPULATOR_CONTROLLER_IMAGE value: ${POPULATOR_CONTROLLER_IMAGE} - name: OVIRT_POPULATOR_IMAGE diff --git a/operator/roles/forkliftcontroller/defaults/main.yml b/operator/roles/forkliftcontroller/defaults/main.yml index 3cb13c740..865cb821b 100644 --- a/operator/roles/forkliftcontroller/defaults/main.yml +++ b/operator/roles/forkliftcontroller/defaults/main.yml @@ -35,6 +35,7 @@ controller_vsphere_incremental_backup: true controller_ovirt_warm_migration: true controller_max_vm_inflight: 20 controller_filesystem_overhead: 10 +controller_block_overhead: 0 profiler_volume_path: "/var/cache/profiler" inventory_volume_path: "/var/cache/inventory" diff --git a/operator/roles/forkliftcontroller/templates/controller/deployment-controller.yml.j2 b/operator/roles/forkliftcontroller/templates/controller/deployment-controller.yml.j2 index a1bc486df..64d5709b9 100644 --- a/operator/roles/forkliftcontroller/templates/controller/deployment-controller.yml.j2 +++ b/operator/roles/forkliftcontroller/templates/controller/deployment-controller.yml.j2 @@ -80,6 +80,10 @@ spec: - name: FILESYSTEM_OVERHEAD value: "{{ controller_filesystem_overhead }}" {% endif %} +{% if controller_block_overhead is number %} + - name: BLOCK_OVERHEAD + value: "{{ controller_block_overhead }}" +{% endif %} {% if controller_vsphere_incremental_backup|bool %} - name: FEATURE_VSPHERE_INCREMENTAL_BACKUP value: "true" diff --git a/pkg/controller/plan/adapter/openstack/builder.go b/pkg/controller/plan/adapter/openstack/builder.go index 7afe9d182..71de8b534 100644 --- a/pkg/controller/plan/adapter/openstack/builder.go +++ b/pkg/controller/plan/adapter/openstack/builder.go @@ -1100,10 +1100,7 @@ func (r *Builder) persistentVolumeClaimWithSourceRef(image model.Image, storageC err = liberr.Wrap(err) return } - - if *volumeMode == core.PersistentVolumeFilesystem { - virtualSize = utils.CalculateSpaceWithOverhead(virtualSize) - } + virtualSize = utils.CalculateSpaceWithOverhead(virtualSize, volumeMode) // The image might be a VM Snapshot Image and has no volume associated to it if originalVolumeDiskId, ok := image.Properties["forklift_original_volume_id"]; ok { diff --git a/pkg/controller/plan/adapter/ovirt/builder.go b/pkg/controller/plan/adapter/ovirt/builder.go index 0d9ecf211..497afde9e 100644 --- a/pkg/controller/plan/adapter/ovirt/builder.go +++ b/pkg/controller/plan/adapter/ovirt/builder.go @@ -780,11 +780,7 @@ func (r *Builder) getDefaultVolumeAndAccessMode(storageClassName string) ([]core // Build a PersistentVolumeClaim with DataSourceRef for VolumePopulator func (r *Builder) persistentVolumeClaimWithSourceRef(diskAttachment model.XDiskAttachment, storageClassName string, populatorName string, annotations map[string]string) (pvc *core.PersistentVolumeClaim, err error) { - - // We add 10% overhead because of the fsOverhead in CDI, around 5% to ext4 and 5% for root partition. - // This value is configurable using `FILESYSTEM_OVERHEAD` diskSize := diskAttachment.Disk.ProvisionedSize - var accessModes []core.PersistentVolumeAccessMode var volumeMode *core.PersistentVolumeMode accessModes, volumeMode, err = r.getDefaultVolumeAndAccessMode(storageClassName) @@ -792,12 +788,11 @@ func (r *Builder) persistentVolumeClaimWithSourceRef(diskAttachment model.XDiskA err = liberr.Wrap(err) return } - - // Accounting for fsOverhead is only required for `volumeMode: Filesystem`, as we may not have enough space - // after creating a filesystem on an underlying block device - if *volumeMode == core.PersistentVolumeFilesystem { - diskSize = utils.CalculateSpaceWithOverhead(diskSize) - } + // We add 10% overhead because of the fsOverhead in CDI, around 5% to ext4 and 5% for root partition. + // This value is configurable using `FILESYSTEM_OVERHEAD` + // Encrypted Ceph RBD makes the pod see less space, this possible overhead needs to be taken into account. + // For Block the value is configurable using `BLOCK_OVERHEAD` + diskSize = utils.CalculateSpaceWithOverhead(diskSize, volumeMode) annotations[AnnImportDiskId] = diskAttachment.ID diff --git a/pkg/controller/plan/util/utils.go b/pkg/controller/plan/util/utils.go index 782fe210f..1151dd30d 100644 --- a/pkg/controller/plan/util/utils.go +++ b/pkg/controller/plan/util/utils.go @@ -4,6 +4,7 @@ import ( "math" "github.com/konveyor/forklift-controller/pkg/settings" + core "k8s.io/api/core/v1" ) // Disk alignment size used to align FS overhead, @@ -20,8 +21,13 @@ func roundUp(requestedSpace, multiple int64) int64 { return int64(partitions) * multiple } -func CalculateSpaceWithOverhead(requestedSpace int64) int64 { +func CalculateSpaceWithOverhead(requestedSpace int64, volumeMode *core.PersistentVolumeMode) int64 { alignedSize := roundUp(requestedSpace, DefaultAlignBlockSize) - spaceWithOverhead := int64(math.Ceil(float64(alignedSize) / (1 - float64(settings.Settings.FileSystemOverhead)/100))) + var spaceWithOverhead int64 + if *volumeMode == core.PersistentVolumeFilesystem { + spaceWithOverhead = int64(math.Ceil(float64(alignedSize) / (1 - float64(settings.Settings.FileSystemOverhead)/100))) + } else { + spaceWithOverhead = alignedSize + int64(settings.Settings.BlockOverhead) + } return spaceWithOverhead } diff --git a/pkg/settings/migration.go b/pkg/settings/migration.go index ff9cfdc5c..035122727 100644 --- a/pkg/settings/migration.go +++ b/pkg/settings/migration.go @@ -19,6 +19,7 @@ const ( SnapshotStatusCheckRate = "SNAPSHOT_STATUS_CHECK_RATE" CDIExportTokenTTL = "CDI_EXPORT_TOKEN_TTL" FileSystemOverhead = "FILESYSTEM_OVERHEAD" + BlockOverhead = "BLOCK_OVERHEAD" ) // Default virt-v2v image. @@ -49,6 +50,8 @@ type Migration struct { CDIExportTokenTTL int // FileSystem overhead in percantage FileSystemOverhead int + // Block fixed overhead size + BlockOverhead int } // Load settings. @@ -98,6 +101,9 @@ func (r *Migration) Load() (err error) { if err != nil { return liberr.Wrap(err) } + if r.BlockOverhead, err = getNonNegativeEnvLimit(BlockOverhead, 0); err != nil { + return liberr.Wrap(err) + } return } From a13c200dbea0460d9a28ccc2ae34726efa78462c Mon Sep 17 00:00:00 2001 From: Arik Hadas Date: Tue, 12 Dec 2023 17:45:20 +0200 Subject: [PATCH 2/3] Change controller_block_overhead to quantity When specifying resources like disk sizes, the value is expected to represent a quantity rather than an integer so users can set values like "1Gi". Applying this to controller_block_overhead as well. Signed-off-by: Arik Hadas --- .../templates/controller/deployment-controller.yml.j2 | 2 -- pkg/controller/plan/util/utils.go | 2 +- pkg/settings/BUILD.bazel | 1 + pkg/settings/migration.go | 11 ++++++++--- 4 files changed, 10 insertions(+), 6 deletions(-) diff --git a/operator/roles/forkliftcontroller/templates/controller/deployment-controller.yml.j2 b/operator/roles/forkliftcontroller/templates/controller/deployment-controller.yml.j2 index 64d5709b9..f34e969b3 100644 --- a/operator/roles/forkliftcontroller/templates/controller/deployment-controller.yml.j2 +++ b/operator/roles/forkliftcontroller/templates/controller/deployment-controller.yml.j2 @@ -80,10 +80,8 @@ spec: - name: FILESYSTEM_OVERHEAD value: "{{ controller_filesystem_overhead }}" {% endif %} -{% if controller_block_overhead is number %} - name: BLOCK_OVERHEAD value: "{{ controller_block_overhead }}" -{% endif %} {% if controller_vsphere_incremental_backup|bool %} - name: FEATURE_VSPHERE_INCREMENTAL_BACKUP value: "true" diff --git a/pkg/controller/plan/util/utils.go b/pkg/controller/plan/util/utils.go index 1151dd30d..dbafdf24c 100644 --- a/pkg/controller/plan/util/utils.go +++ b/pkg/controller/plan/util/utils.go @@ -27,7 +27,7 @@ func CalculateSpaceWithOverhead(requestedSpace int64, volumeMode *core.Persisten if *volumeMode == core.PersistentVolumeFilesystem { spaceWithOverhead = int64(math.Ceil(float64(alignedSize) / (1 - float64(settings.Settings.FileSystemOverhead)/100))) } else { - spaceWithOverhead = alignedSize + int64(settings.Settings.BlockOverhead) + spaceWithOverhead = alignedSize + settings.Settings.BlockOverhead } return spaceWithOverhead } diff --git a/pkg/settings/BUILD.bazel b/pkg/settings/BUILD.bazel index e9fa88ca0..3d0779ed1 100644 --- a/pkg/settings/BUILD.bazel +++ b/pkg/settings/BUILD.bazel @@ -18,5 +18,6 @@ go_library( deps = [ "//pkg/lib/error", "//pkg/lib/logging", + "//vendor/k8s.io/apimachinery/pkg/api/resource", ], ) diff --git a/pkg/settings/migration.go b/pkg/settings/migration.go index 035122727..6a1e7dfbd 100644 --- a/pkg/settings/migration.go +++ b/pkg/settings/migration.go @@ -5,6 +5,7 @@ import ( "strings" liberr "github.com/konveyor/forklift-controller/pkg/lib/error" + "k8s.io/apimachinery/pkg/api/resource" ) // Environment variables. @@ -51,7 +52,7 @@ type Migration struct { // FileSystem overhead in percantage FileSystemOverhead int // Block fixed overhead size - BlockOverhead int + BlockOverhead int64 } // Load settings. @@ -101,8 +102,12 @@ func (r *Migration) Load() (err error) { if err != nil { return liberr.Wrap(err) } - if r.BlockOverhead, err = getNonNegativeEnvLimit(BlockOverhead, 0); err != nil { - return liberr.Wrap(err) + if overhead, ok := os.LookupEnv(BlockOverhead); ok { + if quantity, err := resource.ParseQuantity(overhead); err != nil { + return liberr.Wrap(err) + } else if r.BlockOverhead, ok = quantity.AsInt64(); !ok { + return fmt.Errorf("Block overhead is invalid: %s", overhead) + } } return From 06d4d2a01c7044e33ae4cd06f9be8e9345511431 Mon Sep 17 00:00:00 2001 From: Liran Rotenberg Date: Tue, 12 Dec 2023 18:13:44 +0200 Subject: [PATCH 3/3] add fmt import Signed-off-by: Liran Rotenberg --- pkg/settings/migration.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/settings/migration.go b/pkg/settings/migration.go index 6a1e7dfbd..13a3a15c0 100644 --- a/pkg/settings/migration.go +++ b/pkg/settings/migration.go @@ -1,6 +1,7 @@ package settings import ( + "fmt" "os" "strings"