fix: disks flag parsing and handling in create qemu command

The disks flag Set method was appending new disk requests to the existing ones,
which caused duplicate disk entries when custom values for the disks flag were set.

Signed-off-by: Orzelius <33936483+Orzelius@users.noreply.github.com>

Author: Orzelius

cmd/talosctl/cmd/mgmt/cluster/create/clusterops/configmaker/internal/makers/qemu_test.go

@@ -7,11 +7,14 @@ package makers_test
 import (
 	"testing"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
 	"github.com/siderolabs/talos/cmd/talosctl/cmd/mgmt/cluster/create/clusterops"
 	"github.com/siderolabs/talos/cmd/talosctl/cmd/mgmt/cluster/create/clusterops/configmaker/internal/makers"
+	"github.com/siderolabs/talos/cmd/talosctl/cmd/mgmt/cluster/create/flags"
 	"github.com/siderolabs/talos/pkg/machinery/config/generate"
+	"github.com/siderolabs/talos/pkg/provision"
 )
 
 func TestQemuMaker_MachineConfig(t *testing.T) {
@@ -29,3 +32,66 @@ func TestQemuMaker_MachineConfig(t *testing.T) {
 
 	assertConfigDefaultness(t, cOps, *m.Maker, desiredExtraGenOps...)
 }
+
+func TestQemuMaker_Disks(t *testing.T) {
+	cOps := clusterops.GetCommon()
+	qOps := clusterops.GetQemu()
+
+	disks := flags.Disks{}
+	err := disks.Set("virtio:10GiB,nvme:20GiB,virtio:30GiB")
+	require.NoError(t, err)
+
+	qOps.Disks = disks
+	cOps.Controlplanes = 1
+	cOps.Workers = 1
+
+	m, err := makers.NewQemu(makers.MakerOptions[clusterops.Qemu]{
+		ExtraOps:    qOps,
+		CommonOps:   cOps,
+		Provisioner: testProvisioner{}, // use test provisioner to simplify the test case.
+	})
+	require.NoError(t, err)
+
+	req, err := m.GetClusterConfigs()
+	require.NoError(t, err)
+
+	controlplaneDisks := req.ClusterRequest.Nodes[0].Disks
+	workerDisks := req.ClusterRequest.Nodes[1].Disks
+
+	assert.Equal(t, 1, len(controlplaneDisks))
+	assert.Equal(t, 3, len(workerDisks))
+
+	assert.Equal(t, []*provision.Disk{
+		{
+			Size:            disks.Requests()[0].Size.Bytes(),
+			SkipPreallocate: !qOps.PreallocateDisks,
+			Driver:          "virtio",
+			BlockSize:       qOps.DiskBlockSize,
+			Serial:          "",
+		},
+	}, controlplaneDisks)
+
+	assert.Equal(t, []*provision.Disk{
+		{
+			Size:            disks.Requests()[0].Size.Bytes(),
+			SkipPreallocate: !qOps.PreallocateDisks,
+			Driver:          "virtio",
+			BlockSize:       qOps.DiskBlockSize,
+			Serial:          "",
+		},
+		{
+			Size:            disks.Requests()[1].Size.Bytes(),
+			SkipPreallocate: !qOps.PreallocateDisks,
+			Driver:          "nvme",
+			BlockSize:       qOps.DiskBlockSize,
+			Serial:          "",
+		},
+		{
+			Size:            disks.Requests()[2].Size.Bytes(),
+			SkipPreallocate: !qOps.PreallocateDisks,
+			Driver:          "virtio",
+			BlockSize:       qOps.DiskBlockSize,
+			Serial:          "",
+		},
+	}, workerDisks)
+}

cmd/talosctl/cmd/mgmt/cluster/create/flags/disks.go

@@ -144,7 +144,7 @@ func (f *Disks) Set(value string) error {
 		return err
 	}
 
-	f.requests = append(f.requests, reqs...)
+	f.requests = reqs
 
 	return nil
 }

cmd/talosctl/cmd/mgmt/cluster/create/flags/disks_test.go

@@ -23,8 +23,7 @@ func TestDisksFlag_ExtraOpts(t *testing.T) {
 	fs.Var(&d, "disks", "")
 
 	args := []string{
-		"--disks", "virtio:1GiB:serial=test-1",
-		"--disks", "virtiofs:1GiB:tag=foo,virtiofs:1GiB:tag=bar",
+		"--disks", "virtio:1GiB:serial=test-1,virtiofs:1GiB:tag=foo,virtiofs:1GiB:tag=bar",
 	}
 
 	err := fs.Parse(args)
@@ -70,8 +69,7 @@ func TestDisksFlag_AccumulatesAndRequests(t *testing.T) {
 	fs.Var(&d, "disks", "")
 
 	args := []string{
-		"--disks", "virtio:1GiB",
-		"--disks", "nvme:10GiB,sata:512MiB",
+		"--disks", "virtio:1GiB,nvme:10GiB,sata:512MiB",
 	}
 
 	err := fs.Parse(args)
@@ -102,11 +100,21 @@ func TestDisksFlag_AccumulatesAndRequests(t *testing.T) {
 	assert.Equal(t, "virtio:1GiB,nvme:10GiB,sata:512MiB", d.String())
 }
 
-func TestDisksFlag_SetInvalid(t *testing.T) {
+func TestDisksFlag_Set(t *testing.T) {
 	t.Parallel()
 
 	var d flags.Disks
 
-	err := d.Set("invalid-no-colon")
+	err := d.Set("virtio:6gb")
+	assert.NoError(t, err)
+
+	assert.Equal(t, "virtio:6gb", d.String())
+
+	err = d.Set("nvme:12mb,sata:2gb")
+	assert.NoError(t, err)
+
+	assert.Equal(t, "nvme:12mb,sata:2gb", d.String())
+
+	err = d.Set("invalid-no-colon")
 	assert.Error(t, err)
 }

cmd/talosctl/cmd/mgmt/cluster/create/flags/virtiofs.go

@@ -72,7 +72,7 @@ func (f *Virtiofs) Set(value string) error {
 		return err
 	}
 
-	f.requests = append(f.requests, reqs...)
+	f.requests = reqs
 
 	return nil
 }

cmd/talosctl/cmd/mgmt/cluster/create/flags/virtiofs_test.go

@@ -22,8 +22,7 @@ func TestVirtiofsFlag_AccumulatesAndRequests(t *testing.T) {
 	fs.Var(&d, "virtiofs", "")
 
 	args := []string{
-		"--virtiofs", "/mnt/shared/1:/tmp/mnt-shared-1.sock",
-		"--virtiofs", "/mnt/shared/2:/tmp/mnt-shared-2.sock,/mnt/shared/3:/tmp/mnt-shared-3.sock",
+		"--virtiofs", "/mnt/shared/1:/tmp/mnt-shared-1.sock,/mnt/shared/2:/tmp/mnt-shared-2.sock,/mnt/shared/3:/tmp/mnt-shared-3.sock",
 	}
 
 	err := fs.Parse(args)
@@ -52,6 +51,16 @@ func TestVirtiofsFlag_SetInvalid(t *testing.T) {
 
 	var f flags.Virtiofs
 
-	err := f.Set("invalid-no-colon")
+	err := f.Set("/mnt/shared/1:/tmp/mnt-shared-1.sock")
+	assert.NoError(t, err)
+
+	assert.Equal(t, "/mnt/shared/1:/tmp/mnt-shared-1.sock", f.String())
+
+	err = f.Set("/mnt/shared/1:/tmp/mnt-shared-1.sock,/mnt/shared/2:/tmp/mnt-shared-2.sock")
+	assert.NoError(t, err)
+
+	assert.Equal(t, "/mnt/shared/1:/tmp/mnt-shared-1.sock,/mnt/shared/2:/tmp/mnt-shared-2.sock", f.String())
+
+	err = f.Set("invalid-no-colon")
 	assert.Error(t, err)
 }