disk: Refactor image pull and cache dir creation

This creates two new structs, image and cache, and removes the code to
pull the image and create the cache directory from bootc_disk. This is
done in order to check if the VM is running before creating the disk to
fail early. We need the pulled container image ID before checking
if the VM is running.

This also gets us closer to separately managing the cache dir which should
simplify some of the lock code.

This is in preparation for the code to clear the cached
disk image when modifying the disk image size or filesystem type. Prior
to these changes, the disk could be recreated before checking if the VM
is running.

Signed-off-by: Chris Kyrouac <ckyrouac@redhat.com>

Author: ckyrouac

cmd/run.go

@@ -9,7 +9,9 @@ import (
 	"time"
 
 	"gitlab.com/bootc-org/podman-bootc/pkg/bootc"
+	"gitlab.com/bootc-org/podman-bootc/pkg/cache"
 	"gitlab.com/bootc-org/podman-bootc/pkg/config"
+	"gitlab.com/bootc-org/podman-bootc/pkg/container"
 	"gitlab.com/bootc-org/podman-bootc/pkg/user"
 	"gitlab.com/bootc-org/podman-bootc/pkg/utils"
 	"gitlab.com/bootc-org/podman-bootc/pkg/vm"
@@ -100,24 +102,23 @@ func doRun(flags *cobra.Command, args []string) error {
 		return err
 	}
 
-	// create the disk image
-	idOrName := args[0]
-	bootcDisk := bootc.NewBootcDisk(idOrName, ctx, user)
-	err = bootcDisk.Install(vmConfig.Quiet, diskImageConfigInstance)
-
+	// pull the container image
+	containerImage := container.NewContainerImage(args[0], ctx)
+	err = containerImage.Pull()
 	if err != nil {
-		return fmt.Errorf("unable to install bootc image: %w", err)
+		return fmt.Errorf("unable to pull container image: %w", err)
 	}
 
-	//start the VM
-	println("Booting the VM...")
-	sshPort, err := utils.GetFreeLocalTcpPort()
+	// create the cache directory
+	cache := cache.NewCache(containerImage.GetId(), user)
+	err = cache.Create()
 	if err != nil {
-		return fmt.Errorf("unable to get free port for SSH: %w", err)
+		return fmt.Errorf("unable to create cache: %w", err)
 	}
 
+	// check if the vm is already running
 	bootcVM, err := vm.NewVM(vm.NewVMParameters{
-		ImageID:    bootcDisk.GetImageId(),
+		ImageID:    containerImage.GetId(),
 		User:       user,
 		LibvirtUri: config.LibvirtUri,
 		Locking:    utils.Shared,
@@ -127,11 +128,34 @@ func doRun(flags *cobra.Command, args []string) error {
 		return fmt.Errorf("unable to initialize VM: %w", err)
 	}
 
+	isRunning, err := bootcVM.IsRunning()
+	if err != nil {
+		return fmt.Errorf("unable to check if VM is running: %w", err)
+	}
+	if isRunning {
+		return fmt.Errorf("VM already running, use the ssh command to connect to it")
+	}
+
+	// create the disk image
+	bootcDisk := bootc.NewBootcDisk(containerImage, ctx, user, cache)
+	err = bootcDisk.Install(vmConfig.Quiet, diskImageConfigInstance)
+
+	if err != nil {
+		return fmt.Errorf("unable to install bootc image: %w", err)
+	}
+
+	//start the VM
+	println("Booting the VM...")
+	sshPort, err := utils.GetFreeLocalTcpPort()
+	if err != nil {
+		return fmt.Errorf("unable to get free port for SSH: %w", err)
+	}
+
 	// Let's be explicit instead of relying on the defer exec order
 	defer func() {
 		bootcVM.CloseConnection()
 		if err := bootcVM.Unlock(); err != nil {
-			logrus.Warningf("unable to unlock VM %s: %v", bootcDisk.GetImageId(), err)
+			logrus.Warningf("unable to unlock VM %s: %v", containerImage.GetId(), err)
 		}
 	}()
 
@@ -153,7 +177,7 @@ func doRun(flags *cobra.Command, args []string) error {
 	}
 
 	// write down the config file
-	if err = bootcVM.WriteConfig(*bootcDisk); err != nil {
+	if err = bootcVM.WriteConfig(*bootcDisk, containerImage); err != nil {
 		return err
 	}
 

pkg/bootc/bootc_disk.go

@@ -13,12 +13,11 @@ import (
 	"syscall"
 	"time"
 
-	"gitlab.com/bootc-org/podman-bootc/pkg/config"
+	"gitlab.com/bootc-org/podman-bootc/pkg/cache"
+	"gitlab.com/bootc-org/podman-bootc/pkg/container"
 	"gitlab.com/bootc-org/podman-bootc/pkg/user"
-	"gitlab.com/bootc-org/podman-bootc/pkg/utils"
 
 	"github.com/containers/podman/v5/pkg/bindings/containers"
-	"github.com/containers/podman/v5/pkg/bindings/images"
 	"github.com/containers/podman/v5/pkg/domain/entities/types"
 	"github.com/containers/podman/v5/pkg/specgen"
 	"github.com/docker/go-units"
@@ -62,14 +61,11 @@ type diskFromContainerMeta struct {
 }
 
 type BootcDisk struct {
-	ImageNameOrId           string
+	ContainerImage          container.ContainerImage
+	Cache                   cache.Cache
 	User                    user.User
 	Ctx                     context.Context
-	ImageId                 string
-	imageData               *types.ImageInspectReport
-	RepoTag                 string
 	CreatedAt               time.Time
-	Directory               string
 	file                    *os.File
 	bootcInstallContainerId string
 }
@@ -80,40 +76,34 @@ var (
 	instanceOnce sync.Once
 )
 
-func NewBootcDisk(imageNameOrId string, ctx context.Context, user user.User) *BootcDisk {
+// NewBootcDisk creates a new BootcDisk instance
+//
+// Parameters:
+//   - imageNameOrId: the name or id of the container image
+//   - ctx: context for the podman machine connection
+//   - user: the user who is running the command, determines where the disk image is stored
+func NewBootcDisk(containerImage container.ContainerImage, ctx context.Context, user user.User, cache cache.Cache) *BootcDisk {
 	instanceOnce.Do(func() {
 		instance = &BootcDisk{
-			ImageNameOrId: imageNameOrId,
-			Ctx:           ctx,
-			User:          user,
+			ContainerImage: containerImage,
+			Ctx:            ctx,
+			User:           user,
+			Cache:          cache,
 		}
 	})
 	return instance
 }
 
-func (p *BootcDisk) GetDirectory() string {
-	return p.Directory
-}
-
-func (p *BootcDisk) GetImageId() string {
-	return p.ImageId
-}
-
 // GetSize returns the virtual size of the disk in bytes;
 // this may be larger than the actual disk usage
 func (p *BootcDisk) GetSize() (int64, error) {
-	st, err := os.Stat(filepath.Join(p.Directory, config.DiskImage))
+	st, err := os.Stat(p.Cache.GetDiskPath())
 	if err != nil {
 		return 0, err
 	}
 	return st.Size(), nil
 }
 
-// GetRepoTag returns the repository of the container image
-func (p *BootcDisk) GetRepoTag() string {
-	return p.RepoTag
-}
-
 // GetCreatedAt returns the creation time of the disk image
 func (p *BootcDisk) GetCreatedAt() time.Time {
 	return p.CreatedAt
@@ -122,32 +112,6 @@ func (p *BootcDisk) GetCreatedAt() time.Time {
 func (p *BootcDisk) Install(quiet bool, config DiskImageConfig) (err error) {
 	p.CreatedAt = time.Now()
 
-	err = p.pullImage()
-	if err != nil {
-		return
-	}
-
-	// Create VM cache dir; one per oci bootc image
-	p.Directory = filepath.Join(p.User.CacheDir(), p.ImageId)
-	lock := utils.NewCacheLock(p.User.RunDir(), p.Directory)
-	locked, err := lock.TryLock(utils.Exclusive)
-	if err != nil {
-		return fmt.Errorf("error locking the VM cache path: %w", err)
-	}
-	if !locked {
-		return fmt.Errorf("unable to lock the VM cache path")
-	}
-
-	defer func() {
-		if err := lock.Unlock(); err != nil {
-			logrus.Errorf("unable to unlock VM %s: %v", p.ImageId, err)
-		}
-	}()
-
-	if err := os.MkdirAll(p.Directory, os.ModePerm); err != nil {
-		return fmt.Errorf("error while making bootc disk directory: %w", err)
-	}
-
 	err = p.getOrInstallImageToDisk(quiet, config)
 	if err != nil {
 		return
@@ -173,7 +137,7 @@ func (p *BootcDisk) Cleanup() (err error) {
 
 // getOrInstallImageToDisk checks if the disk is present and if not, installs the image to a new disk
 func (p *BootcDisk) getOrInstallImageToDisk(quiet bool, diskConfig DiskImageConfig) error {
-	diskPath := filepath.Join(p.Directory, config.DiskImage)
+	diskPath := p.Cache.GetDiskPath()
 	f, err := os.Open(diskPath)
 	if err != nil {
 		if !errors.Is(err, os.ErrNotExist) {
@@ -199,8 +163,8 @@ func (p *BootcDisk) getOrInstallImageToDisk(quiet bool, diskConfig DiskImageConf
 		return p.bootcInstallImageToDisk(quiet, diskConfig)
 	}
 
-	logrus.Debugf("previous disk digest: %s current digest: %s", serializedMeta.ImageDigest, p.ImageId)
-	if serializedMeta.ImageDigest == p.ImageId {
+	logrus.Debugf("previous disk digest: %s current digest: %s", serializedMeta.ImageDigest, p.ContainerImage.GetId())
+	if serializedMeta.ImageDigest == p.ContainerImage.GetId() {
 		return nil
 	}
 
@@ -217,12 +181,12 @@ func align(size int64, align int64) int64 {
 
 // bootcInstallImageToDisk creates a disk image from a bootc container
 func (p *BootcDisk) bootcInstallImageToDisk(quiet bool, diskConfig DiskImageConfig) (err error) {
-	fmt.Printf("Executing `bootc install to-disk` from container image %s to create disk image\n", p.RepoTag)
-	p.file, err = os.CreateTemp(p.Directory, "podman-bootc-tempdisk")
+	fmt.Printf("Executing `bootc install to-disk` from container image %s to create disk image\n", p.ContainerImage.GetRepoTag())
+	p.file, err = os.CreateTemp(p.Cache.GetDirectory(), "podman-bootc-tempdisk")
 	if err != nil {
 		return err
 	}
-	size := p.imageData.Size * containerSizeToDiskSizeMultiplier
+	size := p.ContainerImage.GetSize() * containerSizeToDiskSizeMultiplier
 	if size < diskSizeMinimum {
 		size = diskSizeMinimum
 	}
@@ -237,7 +201,7 @@ func (p *BootcDisk) bootcInstallImageToDisk(quiet bool, diskConfig DiskImageConf
 	}
 	// Round up to 4k; loopback wants at least 512b alignment
 	size = align(size, 4096)
-	humanContainerSize := units.HumanSize(float64(p.imageData.Size))
+	humanContainerSize := units.HumanSize(float64(p.ContainerImage.GetSize()))
 	humanSize := units.HumanSize(float64(size))
 	logrus.Infof("container size: %s, disk size: %s", humanContainerSize, humanSize)
 
@@ -257,7 +221,7 @@ func (p *BootcDisk) bootcInstallImageToDisk(quiet bool, diskConfig DiskImageConf
 		return fmt.Errorf("failed to create disk image: %w", err)
 	}
 	serializedMeta := diskFromContainerMeta{
-		ImageDigest: p.ImageId,
+		ImageDigest: p.ContainerImage.GetId(),
 	}
 	buf, err := json.Marshal(serializedMeta)
 	if err != nil {
@@ -266,8 +230,8 @@ func (p *BootcDisk) bootcInstallImageToDisk(quiet bool, diskConfig DiskImageConf
 	if err := unix.Fsetxattr(int(p.file.Fd()), imageMetaXattr, buf, 0); err != nil {
 		return fmt.Errorf("failed to set xattr: %w", err)
 	}
-	diskPath := filepath.Join(p.Directory, config.DiskImage)
 
+	diskPath := p.Cache.GetDiskPath()
 	if err := os.Rename(p.file.Name(), diskPath); err != nil {
 		return fmt.Errorf("failed to rename to %s: %w", diskPath, err)
 	}
@@ -276,39 +240,10 @@ func (p *BootcDisk) bootcInstallImageToDisk(quiet bool, diskConfig DiskImageConf
 	return nil
 }
 
-// pullImage fetches the container image if not present
-func (p *BootcDisk) pullImage() (err error) {
-	pullPolicy := "missing"
-	ids, err := images.Pull(p.Ctx, p.ImageNameOrId, &images.PullOptions{Policy: &pullPolicy})
-	if err != nil {
-		return fmt.Errorf("failed to pull image: %w", err)
-	}
-
-	if len(ids) == 0 {
-		return fmt.Errorf("no ids returned from image pull")
-	}
-
-	if len(ids) > 1 {
-		return fmt.Errorf("multiple ids returned from image pull")
-	}
-
-	image, err := images.GetImage(p.Ctx, p.ImageNameOrId, &images.GetOptions{})
-	if err != nil {
-		return fmt.Errorf("failed to get image: %w", err)
-	}
-	p.imageData = image
-
-	imageId := ids[0]
-	p.ImageId = imageId
-	p.RepoTag = image.RepoTags[0]
-
-	return
-}
-
 // runInstallContainer runs the bootc installer in a container to create a disk image
 func (p *BootcDisk) runInstallContainer(quiet bool, config DiskImageConfig) (err error) {
 	// Create a temporary external shell script with the contents of our losetup wrapper
-	losetupTemp, err := os.CreateTemp(p.Directory, "losetup-wrapper")
+	losetupTemp, err := os.CreateTemp(p.Cache.GetDirectory(), "losetup-wrapper")
 	if err != nil {
 		return fmt.Errorf("temp losetup wrapper: %w", err)
 	}
@@ -393,7 +328,7 @@ func (p *BootcDisk) createInstallContainer(config DiskImageConfig, tempLosetup s
 			Terminal:    &trueDat,
 		},
 		ContainerStorageConfig: specgen.ContainerStorageConfig{
-			Image: p.ImageNameOrId,
+			Image: p.ContainerImage.ImageNameOrId,
 			Mounts: []specs.Mount{
 				{
 					Source:      "/var/lib/containers",
@@ -406,7 +341,7 @@ func (p *BootcDisk) createInstallContainer(config DiskImageConfig, tempLosetup s
 					Type:        "bind",
 				},
 				{
-					Source:      p.Directory,
+					Source:      p.Cache.GetDirectory(),
 					Destination: "/output",
 					Type:        "bind",
 				},

pkg/cache/cache.go

@@ -0,0 +1,66 @@
+package cache
+
+import (
+	"fmt"
+	"os"
+	"path/filepath"
+
+	"github.com/sirupsen/logrus"
+	"gitlab.com/bootc-org/podman-bootc/pkg/user"
+	"gitlab.com/bootc-org/podman-bootc/pkg/utils"
+)
+
+func NewCache(imageId string, user user.User) Cache {
+	return Cache{
+		ImageId: imageId,
+		User:    user,
+	}
+}
+
+type Cache struct {
+	User      user.User
+	ImageId   string
+	Directory string
+	Created	  bool
+}
+
+// Create VM cache dir; one per oci bootc image
+func (p *Cache) Create() (err error) {
+	p.Directory = filepath.Join(p.User.CacheDir(), p.ImageId)
+	lock := utils.NewCacheLock(p.User.RunDir(), p.Directory)
+	locked, err := lock.TryLock(utils.Exclusive)
+	if err != nil {
+		return fmt.Errorf("error locking the VM cache path: %w", err)
+	}
+	if !locked {
+		return fmt.Errorf("unable to lock the VM cache path")
+	}
+
+	defer func() {
+		if err := lock.Unlock(); err != nil {
+			logrus.Errorf("unable to unlock VM %s: %v", p.ImageId, err)
+		}
+	}()
+
+	if err := os.MkdirAll(p.Directory, os.ModePerm); err != nil {
+		return fmt.Errorf("error while making bootc disk directory: %w", err)
+	}
+
+	p.Created = true
+
+	return
+}
+
+func (p *Cache) GetDirectory() string {
+	if !p.Created {
+		panic("cache not created")
+	}
+	return p.Directory
+}
+
+func (p *Cache) GetDiskPath() string {
+	if !p.Created {
+		panic("cache not created")
+	}
+	return filepath.Join(p.GetDirectory(), "disk.raw")
+}