Fix: size limit with disk image set

internal/controllers/machine_controller_test.go

@@ -4,13 +4,16 @@
 package controllers_test
 
 import (
+	"os"
+	"path/filepath"
 	"time"
 
 	"github.com/digitalocean/go-libvirt"
 	"github.com/ironcore-dev/libvirt-provider/api"
 	libvirtutils "github.com/ironcore-dev/libvirt-provider/internal/libvirt/utils"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+	utilstrings "k8s.io/utils/strings"
 	"libvirt.org/go/libvirtxml"
 )
 
@@ -144,6 +147,50 @@ var _ = Describe("MachineController", func() {
 			))
 		})
 
+		It("should handle machine with boot image and size limit set", func(ctx SpecContext) {
+			By("creating a machine with boot volume")
+			image := osImage
+
+			machine, err := createMachine(api.MachineSpec{
+				Power:       api.PowerStatePowerOn,
+				Cpu:         4,
+				MemoryBytes: 2147483648,
+				Volumes: []*api.VolumeSpec{
+					{
+						Name: "disk-0",
+						LocalDisk: &api.LocalDiskSpec{
+							Image: &image,
+							Size:  10 * 1024 * 1024 * 1024, // 10GB
+						},
+						Device: "oda",
+					},
+				},
+			})
+			Expect(err).NotTo(HaveOccurred())
+			Expect(machine).NotTo(BeNil())
+
+			DeferCleanup(cleanupMachine(machine.ID))
+
+			diskPath := filepath.Join(
+				providerHost.MachineVolumeDir(
+					machine.ID,
+					utilstrings.EscapeQualifiedName("libvirt-provider.ironcore.dev/local-disk"),
+					"disk-0",
+				),
+				"disk.raw",
+			)
+			var stat os.FileInfo
+			Eventually(func(g Gomega) {
+				stat, err = os.Stat(diskPath)
+				g.Expect(err).NotTo(HaveOccurred())
+			}).
+				WithTimeout(5 * time.Minute).WithPolling(5 * time.Second).
+				Should(Succeed())
+
+			Expect(stat.Size()).To(BeNumerically("==", int64(10*1024*1024*1024)),
+				"disc size should be as configured")
+		})
+
 		It("should update machine power state", func(ctx SpecContext) {
 			By("creating a machine in powered on state")
 			machine, err := createMachine(api.MachineSpec{

internal/plugins/volume/localdisk/localdisk.go

@@ -87,26 +87,36 @@ func (p *plugin) Apply(ctx context.Context, spec *api.VolumeSpec, machine *api.M
 	}
 
 	if !ok {
-		var createOption raw.CreateOption
+		if spec.LocalDisk.Size < 0 {
+			return nil, fmt.Errorf("local disk size must not be negative, got %d", spec.LocalDisk.Size)
+		}
+
+		var createOpts []raw.CreateOption
+
 		if imgRef := spec.LocalDisk.Image; imgRef != nil {
 			img, err := p.imageCache.Get(ctx, *imgRef)
 			if err != nil {
 				return nil, err
 			}
 
 			log.V(2).Info("Create disk with rootfs from img", "file", img.RootFS.Path)
-			createOption = raw.WithSourceFile(img.RootFS.Path)
+			createOpts = append(createOpts, raw.WithSourceFile(img.RootFS.Path))
+
+			if spec.LocalDisk.Size > 0 {
+				log.V(2).Info("Grow disk to configured size", "size", spec.LocalDisk.Size)
+				createOpts = append(createOpts, raw.WithSize(spec.LocalDisk.Size))
+			}
 		} else {
 			size := spec.LocalDisk.Size
 			if size == 0 {
 				size = defaultSize
 			}
 
 			log.V(2).Info("Create disk", "size", size)
-			createOption = raw.WithSize(size)
+			createOpts = append(createOpts, raw.WithSize(size))
 		}
 
-		if err := p.raw.Create(diskFilename, createOption); err != nil {
+		if err := p.raw.Create(diskFilename, createOpts...); err != nil {
 			return nil, fmt.Errorf("error creating disk %w", err)
 		}
 		if err := os.Chmod(diskFilename, filePerm); err != nil {

internal/plugins/volume/localdisk/localdisk_suite_test.go

@@ -0,0 +1,16 @@
+// SPDX-FileCopyrightText: SAP SE or an SAP affiliate company and IronCore contributors
+// SPDX-License-Identifier: Apache-2.0
+
+package localdisk_test
+
+import (
+	"testing"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+)
+
+func TestLocalDisk(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "LocalDisk Suite")
+}

internal/plugins/volume/localdisk/localdisk_test.go

@@ -0,0 +1,111 @@
+// SPDX-FileCopyrightText: SAP SE or an SAP affiliate company and IronCore contributors
+// SPDX-License-Identifier: Apache-2.0
+
+package localdisk_test
+
+import (
+	"context"
+	"os"
+	"path/filepath"
+
+	"github.com/ironcore-dev/libvirt-provider/api"
+	"github.com/ironcore-dev/libvirt-provider/internal/plugins/volume"
+	"github.com/ironcore-dev/libvirt-provider/internal/plugins/volume/localdisk"
+	"github.com/ironcore-dev/libvirt-provider/internal/raw"
+	apiutils "github.com/ironcore-dev/provider-utils/apiutils/api"
+	ociutils "github.com/ironcore-dev/provider-utils/ociutils/oci"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+)
+
+const (
+	machineID          = "1a2b3c"
+	volumeName         = "disk-1"
+	imageSize    int64 = 4096
+	grownSize    int64 = imageSize * 4
+	negativeSize int64 = -1
+)
+
+var _ = Describe("Plugin", func() {
+	Describe("Apply", func() {
+		var volumeDir, diskFile string
+		var plugin volume.Plugin
+
+		BeforeEach(func() {
+			volumeDir = GinkgoT().TempDir()
+			diskFile = filepath.Join(volumeDir, "disk.raw")
+
+			plugin = localdisk.NewPlugin(raw.Exec{}, fakeImageCache{rootFSPath: writeImageRootFS()})
+			Expect(plugin.Init(fakeHost{volumeDir: volumeDir})).To(Succeed())
+		})
+
+		When("local disk with image", func() {
+			It("rejects a negative size", func(ctx SpecContext) {
+				_, err := plugin.Apply(ctx, imageBackedVolume(negativeSize), testMachine())
+
+				Expect(err).To(HaveOccurred())
+
+				Expect(diskFile).ToNot(BeAnExistingFile(),
+					"disk with the wrong size gets stuck and will not be fixed on reapply")
+			})
+
+			It("keeps the image size when no size is set", func(ctx SpecContext) {
+				vol, err := plugin.Apply(ctx, imageBackedVolume(0), testMachine())
+
+				Expect(err).ToNot(HaveOccurred())
+				Expect(vol.EffectiveStorageBytesSize).To(Equal(imageSize))
+			})
+
+			It("grows the disk to the configured size", func(ctx SpecContext) {
+				vol, err := plugin.Apply(ctx, imageBackedVolume(grownSize), testMachine())
+
+				Expect(err).ToNot(HaveOccurred())
+				Expect(vol.EffectiveStorageBytesSize).To(Equal(grownSize))
+			})
+		})
+	})
+})
+
+func imageBackedVolume(size int64) *api.VolumeSpec {
+	image := "example.org/os-foo/gardenlinux:latest"
+
+	return &api.VolumeSpec{
+		Name: volumeName,
+		LocalDisk: &api.LocalDiskSpec{
+			Size:  size,
+			Image: &image,
+		},
+	}
+}
+
+func testMachine() *api.Machine {
+	return &api.Machine{Metadata: apiutils.Metadata{ID: machineID}}
+}
+
+func writeImageRootFS() string {
+	GinkgoHelper()
+
+	rootFS := filepath.Join(GinkgoT().TempDir(), "rootfs.raw")
+
+	Expect(os.WriteFile(rootFS, make([]byte, imageSize), 0o600)).To(Succeed())
+	return rootFS
+}
+
+type fakeHost struct {
+	volumeDir string
+}
+
+func (h fakeHost) PluginDir(string) string                        { return h.volumeDir }
+func (h fakeHost) MachinePluginDir(string, string) string         { return h.volumeDir }
+func (h fakeHost) MachineVolumeDir(string, string, string) string { return h.volumeDir }
+
+type fakeImageCache struct {
+	rootFSPath string
+}
+
+func (c fakeImageCache) Get(context.Context, string) (*ociutils.Image, error) {
+	return &ociutils.Image{RootFS: &ociutils.FileLayer{Path: c.rootFSPath}}, nil
+}
+
+func (fakeImageCache) AddListener(ociutils.Listener) {}

internal/raw/raw_exec.go

@@ -17,24 +17,60 @@ type Exec struct{}
 
 const filePerm = 0660
 
-func (Exec) Create(filename string, opts ...CreateOption) error {
+// Create writes a raw disk image at filename.
+// A source file, if given, is copied and then extended to the requested size.
+// It returns an error if the requested size is smaller than the source.
+func (Exec) Create(filename string, opts ...CreateOption) (err error) {
 	o := &CreateOptions{}
 	o.ApplyOptions(opts)
 	log := ctrl.Log.WithName("raw-disk").WithValues("filename", filename)
 
-	if o.SourceFile == "" {
-		if o.Size == nil {
-			return fmt.Errorf("must specify Size when creating without source file")
+	if o.Size != nil && *o.Size <= 0 {
+		return fmt.Errorf("size must be greater than zero, got %d", *o.Size)
+	}
+
+	if o.SourceFile == "" && o.Size == nil {
+		return fmt.Errorf("must specify Size when creating without source file")
+	}
+
+	var wantSize int64
+	if o.Size != nil {
+		wantSize = *o.Size
+	}
+
+	if o.SourceFile != "" && wantSize > 0 {
+		fi, err := os.Stat(o.SourceFile)
+		if err != nil {
+			return fmt.Errorf("could not stat %q: %w", o.SourceFile, err)
+		}
+		if fi.Size() > wantSize {
+			return fmt.Errorf("cannot create %q at %d: source file %q is already %d", filename, wantSize, o.SourceFile, fi.Size())
+		}
+	}
+
+	defer func() {
+		if err != nil {
+			os.Remove(filename)
 		}
-		seek := *o.Size
+	}()
+
+	if o.SourceFile == "" {
 		// Position the file cursor one byte before the desired seek position to write a single byte,
 		// to ensure that data is written at the exact byte position specified by seek.
-		if err := createEmptyFileWithSeek(log, filename, seek-1); err != nil {
+		if err := createEmptyFileWithSeek(log, filename, wantSize-1); err != nil {
 			return fmt.Errorf("failed creating the empty ephemeral disk at %s: %w", filename, err)
 		}
-	} else {
-		if err := copyFile(log, o.SourceFile, filename); err != nil {
-			return fmt.Errorf("failed creating virtual disk image, source: %s, destination: %s: %w", o.SourceFile, filename, err)
+
+		return nil
+	}
+
+	if err := copyFile(log, o.SourceFile, filename); err != nil {
+		return fmt.Errorf("failed creating virtual disk image, source: %s, destination: %s: %w", o.SourceFile, filename, err)
+	}
+
+	if wantSize > 0 {
+		if err := os.Truncate(filename, wantSize); err != nil {
+			return fmt.Errorf("resizing file: %w", err)
 		}
 	}