Merge pull request #104 from sp-yduck/feature/storage

fix #93 : support different types of storages

Author: sp-yduck

Makefile

@@ -196,7 +196,7 @@ RELEASE_DIR := $(shell pwd)/out
 $(RELEASE_DIR):
 	mkdir -p $(RELEASE_DIR)
 
-RELEASE_TAG := $(shell git describe --abbrev=0 2>/dev/null)
+RELEASE_TAG := $(shell git describe --abbrev=0 --tags)
 
 .PHONY: release
 release: ## Builds all the manifests/config files to publish with a release

api/v1beta1/proxmoxcluster_types.go

@@ -35,9 +35,8 @@ type ProxmoxClusterSpec struct {
 	// ServerRef is used for configuring Proxmox client
 	ServerRef ServerRef `json:"serverRef"`
 
-	// storage is for proxmox storage used by vm instances
-	// +optional
-	Storage Storage `json:"storage"`
+	// storage is used for storing cloud init snippet
+	Storage Storage `json:"storage,omitempty"`
 }
 
 // ProxmoxClusterStatus defines the observed state of ProxmoxCluster

api/v1beta1/proxmoxmachine_types.go

@@ -34,20 +34,22 @@ type ProxmoxMachineSpec struct {
 	ProviderID *string `json:"providerID,omitempty"`
 
 	// Node is proxmox node hosting vm instance which used for ProxmoxMachine
-	// +optional
 	Node string `json:"node,omitempty"`
 
+	// Storage is name of proxmox storage used by this node.
+	// The storage must support "images(VM Disks)" type of content.
+	// cappx will use random storage if empty
+	Storage string `json:"storage,omitempty"`
+
 	// +kubebuilder:validation:Minimum:=0
 	// VMID is proxmox qemu's id
-	// +optional
 	VMID *int `json:"vmID,omitempty"`
 
 	// Image is the image to be provisioned
 	Image Image `json:"image"`
 
 	// CloudInit defines options related to the bootstrapping systems where
 	// CloudInit is used.
-	// +optional
 	CloudInit CloudInit `json:"cloudInit,omitempty"`
 
 	// Hardware
@@ -56,8 +58,7 @@ type ProxmoxMachineSpec struct {
 	// Network
 	Network Network `json:"network,omitempty"`
 
-	// Options
-	// +optional
+	// Options for QEMU instance
 	Options Options `json:"options,omitempty"`
 
 	// FailureDomain is the failure domain unique identifier this Machine should be attached to, as defined in Cluster API.
@@ -94,7 +95,9 @@ type ProxmoxMachineStatus struct {
 //+kubebuilder:subresource:status
 // +kubebuilder:printcolumn:name="Cluster",type="string",JSONPath=".metadata.labels.cluster\\.x-k8s\\.io/cluster-name",description="Cluster to which this VSphereMachine belongs"
 // +kubebuilder:printcolumn:name="Machine",type="string",JSONPath=".metadata.ownerReferences[?(@.kind==\"Machine\")].name",description="Machine object which owns with this ProxmoxMachine",priority=1
-// +kubebuilder:printcolumn:name="vmid",type=string,JSONPath=`.spec.vmID`,priority=1
+// +kubebuilder:printcolumn:name="VMID",type=string,JSONPath=`.spec.vmID`,priority=1
+// +kubebuilder:printcolumn:name="Node",type=string,JSONPath=`.spec.node`,priority=1
+// +kubebuilder:printcolumn:name="Storage",type=string,JSONPath=`.spec.storage`,priority=1
 // +kubebuilder:printcolumn:name="ProviderID",type=string,JSONPath=`.spec.providerID`
 // +kubebuilder:printcolumn:name="Status",type=string,JSONPath=`.status.instanceStatus`
 // +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp",description="Time duration since creation of Machine"

cloud/interfaces.go

@@ -57,7 +57,8 @@ type MachineGetter interface {
 	GetProviderID() string
 	GetBootstrapData() (string, error)
 	GetInstanceStatus() *infrav1.InstanceStatus
-	GetStorage() infrav1.Storage
+	GetClusterStorage() infrav1.Storage
+	GetStorage() string
 	GetCloudInit() infrav1.CloudInit
 	GetNetwork() infrav1.Network
 	GetHardware() infrav1.Hardware
@@ -72,6 +73,7 @@ type MachineSetter interface {
 	SetNodeName(name string)
 	SetVMID(vmid int)
 	SetConfigStatus(config api.VirtualMachineConfig)
+	SetStorage(name string)
 	// SetFailureMessage(v error)
 	// SetFailureReason(v capierrors.MachineStatusError)
 	// SetAnnotation(key, value string)

cloud/scope/cluster.go

@@ -18,6 +18,7 @@ package scope
 
 import (
 	"context"
+	"fmt"
 
 	"github.com/pkg/errors"
 	"github.com/sp-yduck/proxmox-go/proxmox"
@@ -26,6 +27,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	infrav1 "github.com/sp-yduck/cluster-api-provider-proxmox/api/v1beta1"
+	"github.com/sp-yduck/cluster-api-provider-proxmox/cloud/services/compute/storage"
 )
 
 type ClusterScopeParams struct {
@@ -92,7 +94,14 @@ func (s *ClusterScope) ControlPlaneEndpoint() clusterv1.APIEndpoint {
 	return s.ProxmoxCluster.Spec.ControlPlaneEndpoint
 }
 
+// return default values if they are not specified
 func (s *ClusterScope) Storage() infrav1.Storage {
+	if s.ProxmoxCluster.Spec.Storage.Name == "" {
+		s.ProxmoxCluster.Spec.Storage.Name = fmt.Sprintf("local-dir-%s", s.Name())
+	}
+	if s.ProxmoxCluster.Spec.Storage.Path == "" {
+		s.ProxmoxCluster.Spec.Storage.Path = fmt.Sprintf("%s/%s", storage.DefaultBasePath, s.ProxmoxCluster.Spec.Storage.Name)
+	}
 	return s.ProxmoxCluster.Spec.Storage
 }
 

cloud/scope/machine.go

@@ -83,8 +83,12 @@ func (m *MachineScope) CloudClient() *proxmox.Service {
 	return m.ClusterGetter.CloudClient()
 }
 
-func (m *MachineScope) GetStorage() infrav1.Storage {
-	return m.ClusterGetter.ProxmoxCluster.Spec.Storage
+func (m *MachineScope) GetClusterStorage() infrav1.Storage {
+	return m.ClusterGetter.Storage()
+}
+
+func (m *MachineScope) GetStorage() string {
+	return m.ProxmoxMachine.Spec.Storage
 }
 
 func (m *MachineScope) Name() string {
@@ -103,6 +107,10 @@ func (m *MachineScope) SetNodeName(name string) {
 	m.ProxmoxMachine.Spec.Node = name
 }
 
+func (m *MachineScope) SetStorage(name string) {
+	m.ProxmoxMachine.Spec.Storage = name
+}
+
 // func (m *MachineScope) Client() Compute {
 // 	return m.ClusterGetter.Client()
 // }

cloud/services/compute/instance/cloudinit.go

@@ -30,7 +30,7 @@ func (s *Service) reconcileCloudInit(ctx context.Context) error {
 
 // delete CloudConfig
 func (s *Service) deleteCloudConfig(ctx context.Context) error {
-	storageName := s.scope.GetStorage().Name
+	storageName := s.scope.GetClusterStorage().Name
 	path := userSnippetPath(s.scope.Name())
 	volumeID := fmt.Sprintf("%s:%s", storageName, path)
 
@@ -79,7 +79,7 @@ func (s *Service) reconcileCloudInitUser(ctx context.Context) error {
 		return err
 	}
 	defer vnc.Close()
-	filePath := fmt.Sprintf("%s/%s", s.scope.GetStorage().Path, userSnippetPath(vmName))
+	filePath := fmt.Sprintf("%s/%s", s.scope.GetClusterStorage().Path, userSnippetPath(vmName))
 	if err := vnc.WriteFile(context.TODO(), configYaml, filePath); err != nil {
 		return errors.Errorf("failed to write file error : %v", err)
 	}

cloud/services/compute/instance/image.go

@@ -36,16 +36,14 @@ func (s *Service) reconcileBootDevice(ctx context.Context, vm *proxmox.VirtualMa
 }
 
 // setCloudImage downloads OS image into Proxmox node
-// and then sets it to specified storage
+// so that proxmox can import image to the storage from there
 func (s *Service) setCloudImage(ctx context.Context) error {
 	log := log.FromContext(ctx)
 	log.Info("setting cloud image")
 
 	image := s.scope.GetImage()
 	rawImageFilePath := rawImageFilePath(image)
 
-	// workaround
-	// API does not support something equivalent of "qm importdisk"
 	vnc, err := s.vncClient(s.scope.NodeName())
 	if err != nil {
 		return errors.Errorf("failed to create vnc client: %v", err)
@@ -68,14 +66,6 @@ func (s *Service) setCloudImage(ctx context.Context) error {
 			return errors.Errorf("failed to confirm checksum: %v", err)
 		}
 	}
-
-	// convert downloaded image to raw format and set it to storage
-	vmid := s.scope.GetVMID()
-	destPath := fmt.Sprintf("%s/images/%d/vm-%d-disk-0.raw", s.scope.GetStorage().Path, *vmid, *vmid)
-	out, _, err := vnc.Exec(context.TODO(), fmt.Sprintf("/usr/bin/qemu-img convert -O raw %s %s", rawImageFilePath, destPath))
-	if err != nil {
-		return errors.Errorf("failed to convert iamge : %s : %v", out, err)
-	}
 	return nil
 }
 

cloud/services/compute/instance/qemu.go

@@ -48,6 +48,10 @@ func (s *Service) getQEMU(ctx context.Context, vmid *int) (*proxmox.VirtualMachi
 func (s *Service) createQEMU(ctx context.Context, nodeName string, vmid *int) (*proxmox.VirtualMachine, error) {
 	log := log.FromContext(ctx)
 
+	if err := s.ensureStorageAvailable(ctx); err != nil {
+		return nil, err
+	}
+
 	// get node
 	if nodeName == "" {
 		// temp solution
@@ -107,10 +111,15 @@ func (s *Service) getRandomNode(ctx context.Context) (*api.Node, error) {
 
 func (s *Service) generateVMOptions() api.VirtualMachineCreateOptions {
 	vmName := s.scope.Name()
-	storageName := s.scope.GetStorage().Name
+	snippetStorageName := s.scope.GetClusterStorage().Name
+	imageStorageName := s.scope.GetStorage()
 	network := s.scope.GetNetwork()
 	hardware := s.scope.GetHardware()
 	options := s.scope.GetOptions()
+	cicustom := fmt.Sprintf("user=%s:%s", snippetStorageName, userSnippetPath(vmName))
+	ide2 := fmt.Sprintf("file=%s:cloudinit,media=cdrom", imageStorageName)
+	scsi0 := fmt.Sprintf("%s:0,import-from=%s", imageStorageName, rawImageFilePath(s.scope.GetImage()))
+	net0 := "model=virtio,bridge=vmbr0,firewall=1"
 
 	vmoptions := api.VirtualMachineCreateOptions{
 		ACPI:          boolToInt8(options.ACPI),
@@ -119,12 +128,12 @@ func (s *Service) generateVMOptions() api.VirtualMachineCreateOptions {
 		Balloon:       options.Balloon,
 		BIOS:          string(hardware.BIOS),
 		Boot:          fmt.Sprintf("order=%s", bootDvice),
-		CiCustom:      fmt.Sprintf("user=%s:%s", storageName, userSnippetPath(vmName)),
+		CiCustom:      cicustom,
 		Cores:         hardware.CPU,
 		CpuLimit:      hardware.CPULimit,
 		Description:   options.Description,
 		HugePages:     options.HugePages.String(),
-		Ide:           api.Ide{Ide2: fmt.Sprintf("file=%s:cloudinit,media=cdrom", storageName)},
+		Ide:           api.Ide{Ide2: ide2},
 		IPConfig:      api.IPConfig{IPConfig0: network.IPConfig.String()},
 		KeepHugePages: boolToInt8(options.KeepHugePages),
 		KVM:           boolToInt8(options.KVM),
@@ -133,13 +142,13 @@ func (s *Service) generateVMOptions() api.VirtualMachineCreateOptions {
 		Memory:        hardware.Memory,
 		Name:          vmName,
 		NameServer:    network.NameServer,
-		Net:           api.Net{Net0: "model=virtio,bridge=vmbr0,firewall=1"},
+		Net:           api.Net{Net0: net0},
 		Numa:          boolToInt8(options.NUMA),
 		OnBoot:        boolToInt8(options.OnBoot),
 		OSType:        api.OSType(options.OSType),
 		Protection:    boolToInt8(options.Protection),
 		Reboot:        int(boolToInt8(options.Reboot)),
-		Scsi:          api.Scsi{Scsi0: fmt.Sprintf("file=%s:8", storageName)},
+		Scsi:          api.Scsi{Scsi0: scsi0},
 		ScsiHw:        api.VirtioScsiPci,
 		SearchDomain:  network.SearchDomain,
 		Serial:        api.Serial{Serial0: "socket"},

cloud/services/compute/instance/storage.go

@@ -0,0 +1,49 @@
+package instance
+
+import (
+	"context"
+	"fmt"
+	"strings"
+
+	"github.com/sp-yduck/proxmox-go/api"
+)
+
+// make sure storage exists and supports "images" type of content
+func (s *Service) ensureStorageAvailable(ctx context.Context) error {
+	storageName := s.scope.GetStorage()
+	if storageName == "" { // no storage specified, find available storage
+		storage, err := s.findVMStorage(ctx)
+		if err != nil {
+			return err
+		}
+		storageName = storage.Storage
+		s.scope.SetStorage(storageName)
+	} else { // storage specified, check if it supports "images" type of content
+		storage, err := s.client.RESTClient().GetStorage(ctx, storageName)
+		if err != nil {
+			return err
+		}
+		if supportsImage(storage) {
+			return fmt.Errorf("storage %s does not support \"images\" type of content", storageName)
+		}
+	}
+	return nil
+}
+
+// get one storage supporting "images" type of content
+func (s *Service) findVMStorage(ctx context.Context) (*api.Storage, error) {
+	storages, err := s.client.RESTClient().GetStorages(ctx)
+	if err != nil {
+		return nil, err
+	}
+	for _, storage := range storages {
+		if supportsImage(storage) {
+			return storage, nil
+		}
+	}
+	return nil, fmt.Errorf("no available storage")
+}
+
+func supportsImage(storage *api.Storage) bool {
+	return strings.Contains(storage.Content, "images")
+}