Skip to content

Commit

Permalink
fix: order volume config by the requested size
Browse files Browse the repository at this point in the history
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 <andrey.smirnov@siderolabs.com>
  • Loading branch information
smira committed Dec 11, 2024
1 parent bc3039a commit 61b1489
Show file tree
Hide file tree
Showing 6 changed files with 165 additions and 3 deletions.
3 changes: 2 additions & 1 deletion .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
@@ -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:
Expand Down Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion .github/workflows/integration-misc-0-cron.yaml
Original file line number Diff line number Diff line change
@@ -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:
Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions .kres.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 8 additions & 0 deletions hack/test/patches/ephemeral-min-max.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
apiVersion: v1alpha1
kind: VolumeConfig
name: EPHEMERAL
provisioning:
diskSelector:
match: system_disk
minSize: 4GB
maxSize: 100GB
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ package volumes
import (
"cmp"
"context"
"math"

"github.com/siderolabs/gen/optional"

Expand All @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
@@ -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)
})
}
}

0 comments on commit 61b1489

Please sign in to comment.