From 61b1489a0f0868c5b7e124544520bc46badef85c Mon Sep 17 00:00:00 2001 From: Andrey Smirnov Date: Wed, 11 Dec 2024 17:32:35 +0400 Subject: [PATCH] fix: order volume config by the requested size This fixes an issue like that: * the system disk is say 10GiB * STATE is fixed 100 MiB always * EPHEMERAL is configured to be min 6 GiB, max 100 GiB As the EPHEMERAL/STATE provisioning order was not defined, EPHEMERAL might be created first, occupying whole disk and leaving no space left for STATE. Signed-off-by: Andrey Smirnov --- .github/workflows/ci.yaml | 3 +- .../workflows/integration-misc-0-cron.yaml | 3 +- .kres.yaml | 1 + hack/test/patches/ephemeral-min-max.yaml | 8 ++ .../block/internal/volumes/volumes.go | 21 ++- .../block/internal/volumes/volumes_test.go | 132 ++++++++++++++++++ 6 files changed, 165 insertions(+), 3 deletions(-) create mode 100644 hack/test/patches/ephemeral-min-max.yaml create mode 100644 internal/app/machined/pkg/controllers/block/internal/volumes/volumes_test.go diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 742eedc218..9f7d4493b4 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -1,6 +1,6 @@ # THIS FILE WAS AUTOMATICALLY GENERATED, PLEASE DO NOT EDIT. # -# Generated on 2024-12-06T17:21:12Z by kres 1ebe796. +# Generated on 2024-12-11T13:30:10Z by kres 8183c20. name: default concurrency: @@ -2074,6 +2074,7 @@ jobs: GITHUB_STEP_NAME: ${{ github.job}}-e2e-controlplane-port IMAGE_REGISTRY: registry.dev.siderolabs.io SHORT_INTEGRATION_TEST: "yes" + WITH_CONFIG_PATCH: '@hack/test/patches/ephemeral-min-max.yaml' WITH_CONTROL_PLANE_PORT: "443" run: | sudo -E make e2e-qemu diff --git a/.github/workflows/integration-misc-0-cron.yaml b/.github/workflows/integration-misc-0-cron.yaml index 50cf02c984..151e7b3a1d 100644 --- a/.github/workflows/integration-misc-0-cron.yaml +++ b/.github/workflows/integration-misc-0-cron.yaml @@ -1,6 +1,6 @@ # THIS FILE WAS AUTOMATICALLY GENERATED, PLEASE DO NOT EDIT. # -# Generated on 2024-11-28T13:53:18Z by kres 232fe63. +# Generated on 2024-12-11T13:30:10Z by kres 8183c20. name: integration-misc-0-cron concurrency: @@ -99,6 +99,7 @@ jobs: GITHUB_STEP_NAME: ${{ github.job}}-e2e-controlplane-port IMAGE_REGISTRY: registry.dev.siderolabs.io SHORT_INTEGRATION_TEST: "yes" + WITH_CONFIG_PATCH: '@hack/test/patches/ephemeral-min-max.yaml' WITH_CONTROL_PLANE_PORT: "443" run: | sudo -E make e2e-qemu diff --git a/.kres.yaml b/.kres.yaml index d6000eedaf..e29070f7c1 100644 --- a/.kres.yaml +++ b/.kres.yaml @@ -683,6 +683,7 @@ spec: SHORT_INTEGRATION_TEST: yes WITH_CONTROL_PLANE_PORT: 443 IMAGE_REGISTRY: registry.dev.siderolabs.io + WITH_CONFIG_PATCH: "@hack/test/patches/ephemeral-min-max.yaml" - name: save-talos-logs conditions: - always diff --git a/hack/test/patches/ephemeral-min-max.yaml b/hack/test/patches/ephemeral-min-max.yaml new file mode 100644 index 0000000000..90b861cb34 --- /dev/null +++ b/hack/test/patches/ephemeral-min-max.yaml @@ -0,0 +1,8 @@ +apiVersion: v1alpha1 +kind: VolumeConfig +name: EPHEMERAL +provisioning: + diskSelector: + match: system_disk + minSize: 4GB + maxSize: 100GB diff --git a/internal/app/machined/pkg/controllers/block/internal/volumes/volumes.go b/internal/app/machined/pkg/controllers/block/internal/volumes/volumes.go index 9dcca75003..699e388210 100644 --- a/internal/app/machined/pkg/controllers/block/internal/volumes/volumes.go +++ b/internal/app/machined/pkg/controllers/block/internal/volumes/volumes.go @@ -8,6 +8,7 @@ package volumes import ( "cmp" "context" + "math" "github.com/siderolabs/gen/optional" @@ -18,11 +19,29 @@ import ( // CompareVolumeConfigs compares two volume configs in the proposed order of provisioning. func CompareVolumeConfigs(a, b *block.VolumeConfig) int { + // first, sort by wave, smaller wave first if c := cmp.Compare(a.TypedSpec().Provisioning.Wave, b.TypedSpec().Provisioning.Wave); c != 0 { return c } - return cmpBool(a.TypedSpec().Provisioning.PartitionSpec.Grow, b.TypedSpec().Provisioning.PartitionSpec.Grow) + // prefer partitions which do not grow, as growing partitions may consume space needed by other partitions + if c := cmpBool(a.TypedSpec().Provisioning.PartitionSpec.Grow, b.TypedSpec().Provisioning.PartitionSpec.Grow); c != 0 { + return c + } + + // prefer partitions with smaller sizes first + // e.g.: for a disk of size 1GiB, and following config with min-max requested sizes: + // 1. 100MiB - 200MiB + // 2. 300MiB - 2GiB + // + // if the order is 2-1, the second partition will grow to full disk size and will leave no space for the first partition, + // but if the order is 1-2, partition sizes will 200MiB and 800MiB respectively. + // + // we compare only max size, as it affects the resulting size of the partition + desiredSizeA := cmp.Or(a.TypedSpec().Provisioning.PartitionSpec.MaxSize, math.MaxUint64) + desiredSizeB := cmp.Or(b.TypedSpec().Provisioning.PartitionSpec.MaxSize, math.MaxUint64) + + return cmp.Compare(desiredSizeA, desiredSizeB) } func cmpBool(a, b bool) int { diff --git a/internal/app/machined/pkg/controllers/block/internal/volumes/volumes_test.go b/internal/app/machined/pkg/controllers/block/internal/volumes/volumes_test.go new file mode 100644 index 0000000000..8e006131fd --- /dev/null +++ b/internal/app/machined/pkg/controllers/block/internal/volumes/volumes_test.go @@ -0,0 +1,132 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +package volumes_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/siderolabs/talos/internal/app/machined/pkg/controllers/block/internal/volumes" + "github.com/siderolabs/talos/pkg/machinery/resources/block" +) + +func TestCompareVolumeConfigs(t *testing.T) { + t.Parallel() + + for _, test := range []struct { + name string + + a *block.VolumeConfigSpec + b *block.VolumeConfigSpec + + expected int + }{ + { + name: "different wave", + + a: &block.VolumeConfigSpec{ + Provisioning: block.ProvisioningSpec{ + Wave: block.WaveSystemDisk, + }, + }, + b: &block.VolumeConfigSpec{ + Provisioning: block.ProvisioningSpec{ + Wave: block.WaveUserDisks, + }, + }, + + expected: -1, + }, + { + name: "prefer grow", + + a: &block.VolumeConfigSpec{ + Provisioning: block.ProvisioningSpec{ + Wave: block.WaveSystemDisk, + PartitionSpec: block.PartitionSpec{ + Grow: true, + }, + }, + }, + b: &block.VolumeConfigSpec{ + Provisioning: block.ProvisioningSpec{ + Wave: block.WaveSystemDisk, + PartitionSpec: block.PartitionSpec{ + Grow: false, + }, + }, + }, + + expected: 1, + }, + { + name: "prefer smaller size", + + a: &block.VolumeConfigSpec{ + Provisioning: block.ProvisioningSpec{ + Wave: block.WaveSystemDisk, + PartitionSpec: block.PartitionSpec{ + Grow: false, + MinSize: 100, + MaxSize: 200, + }, + }, + }, + b: &block.VolumeConfigSpec{ + Provisioning: block.ProvisioningSpec{ + Wave: block.WaveSystemDisk, + PartitionSpec: block.PartitionSpec{ + Grow: false, + MinSize: 150, + MaxSize: 1000, + }, + }, + }, + + expected: -1, + }, + { + name: "prefer max size", + + a: &block.VolumeConfigSpec{ + Provisioning: block.ProvisioningSpec{ + Wave: block.WaveSystemDisk, + PartitionSpec: block.PartitionSpec{ + Grow: false, + MinSize: 100, + MaxSize: 200, + }, + }, + }, + b: &block.VolumeConfigSpec{ + Provisioning: block.ProvisioningSpec{ + Wave: block.WaveSystemDisk, + PartitionSpec: block.PartitionSpec{ + Grow: false, + MinSize: 50, + MaxSize: 0, // no limit + }, + }, + }, + + expected: -1, + }, + } { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + resA := block.NewVolumeConfig(block.NamespaceName, "A") + *resA.TypedSpec() = *test.a + + resB := block.NewVolumeConfig(block.NamespaceName, "B") + *resB.TypedSpec() = *test.b + + actual := volumes.CompareVolumeConfigs(resA, resB) + + assert.Equal(t, test.expected, actual) + }) + } +}