Skip to content

Commit

Permalink
Check image name length in preflight phase
Browse files Browse the repository at this point in the history 
... to ensure the resulting name of the `VirtualMachineImage` will match the 63 characters limit.

- Shorten the image display name from `<VMName>-<VMName>-<DiskIndex>` to the name of the disk `<VMName>-<DiskIndex>` to safe characters. This allows the import of VM with longer names.

Related to: harvester/harvester#6463

Signed-off-by: Volker Theile <[email protected]>
  • Loading branch information
@votdev
votdev committed Jan 29, 2025
1 parent 9a44174 commit eddc5ef
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 10 deletions.
24 changes: 17 additions & 7 deletions pkg/controllers/migration/virtualmachine.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,6 @@ func (h *virtualMachineHandler) triggerExport(vm *migration.VirtualMachineImport

// generateVMO is a wrapper to generate a VirtualMachineOperations client
func (h *virtualMachineHandler) generateVMO(vm *migration.VirtualMachineImport) (VirtualMachineOperations, error) {

source, err := h.generateSource(vm)
if err != nil {
return nil, fmt.Errorf("error generating migration interface: %v", err)
Expand Down Expand Up @@ -752,15 +751,19 @@ func generateAnnotations(vm *migration.VirtualMachineImport, vmi *harvesterv1bet
}

func (h *virtualMachineHandler) checkAndCreateVirtualMachineImage(vm *migration.VirtualMachineImport, d migration.DiskInfo) (*harvesterv1beta1.VirtualMachineImage, error) {
// Note, the RFC 1123 compliant name of the disk is used here because
// the variable value is later used as a label.
imageDisplayName := fmt.Sprintf("vm-import-%s", d.Name)

imageList, err := h.vmi.Cache().List(vm.Namespace, labels.SelectorFromSet(map[string]string{
labelImageDisplayName: fmt.Sprintf("vm-import-%s-%s", vm.Name, d.Name),
labelImageDisplayName: imageDisplayName,
}))
if err != nil {
return nil, err
}

if len(imageList) > 1 {
return nil, fmt.Errorf("unexpected error: found %d images with label %s=%s, only expected to find one", len(imageList), labelImageDisplayName, fmt.Sprintf("vm-import-%s-%s", vm.Name, d.Name))
return nil, fmt.Errorf("unexpected error: found %d images with label %s=%s, only expected to find one", len(imageList), labelImageDisplayName, imageDisplayName)
}

if len(imageList) == 1 {
Expand All @@ -781,11 +784,15 @@ func (h *virtualMachineHandler) checkAndCreateVirtualMachineImage(vm *migration.
},
},
Labels: map[string]string{
labelImageDisplayName: fmt.Sprintf("vm-import-%s-%s", vm.Name, d.Name),
// Set the `harvesterhci.io/imageDisplayName` label to be
// able to search for the `VirtualMachineImage` object during
// the reconciliation phase. See code above at the beginning
// of this function.
labelImageDisplayName: imageDisplayName,
},
},
Spec: harvesterv1beta1.VirtualMachineImageSpec{
DisplayName: fmt.Sprintf("vm-import-%s-%s", vm.Name, d.Name),
DisplayName: imageDisplayName,
URL: fmt.Sprintf("http://%s:%d/%s", server.Address(), server.DefaultPort(), d.Name),
SourceType: "download",
},
Expand Down Expand Up @@ -816,16 +823,18 @@ func (h *virtualMachineHandler) sanitizeVirtualMachineImport(vm *migration.Virtu
if err != nil {
vm.Status.Status = migration.VirtualMachineImportInvalid
logrus.WithFields(logrus.Fields{
"kind": vm.Kind,
"name": vm.Name,
"namespace": vm.Namespace,
"spec.virtualMachineName": vm.Spec.VirtualMachineName,
"status.importedVirtualMachineName": vm.Status.ImportedVirtualMachineName,
}).Errorf("Failed to sanitize the '%s' object: %v", vm.Kind, err)
}).Errorf("Failed to sanitize the import spec: %v", err)
} else {
// Make sure the ImportedVirtualMachineName is RFC 1123 compliant.
if errs := validation.IsDNS1123Label(vm.Status.ImportedVirtualMachineName); len(errs) != 0 {
vm.Status.Status = migration.VirtualMachineImportInvalid
logrus.WithFields(logrus.Fields{
"kind": vm.Kind,
"name": vm.Name,
"namespace": vm.Namespace,
"spec.virtualMachineName": vm.Spec.VirtualMachineName,
Expand All @@ -834,11 +843,12 @@ func (h *virtualMachineHandler) sanitizeVirtualMachineImport(vm *migration.Virtu
} else {
vm.Status.Status = migration.SourceReady
logrus.WithFields(logrus.Fields{
"kind": vm.Kind,
"name": vm.Name,
"namespace": vm.Namespace,
"spec.virtualMachineName": vm.Spec.VirtualMachineName,
"status.importedVirtualMachineName": vm.Status.ImportedVirtualMachineName,
}).Infof("The sanitization of the '%s' object was successful", vm.Kind)
}).Info("The sanitization of the import spec was successful")
}
}

Expand Down
27 changes: 26 additions & 1 deletion pkg/source/openstack/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/validation"
kubevirt "kubevirt.io/api/core/v1"

migration "github.com/harvester/vm-import-controller/pkg/apis/migration.harvesterhci.io/v1beta1"
Expand Down Expand Up @@ -185,6 +186,7 @@ func (c *Client) Verify() error {
}

func (c *Client) PreFlightChecks(vm *migration.VirtualMachineImport) (err error) {
// Check the source network mappings.
for _, nm := range vm.Spec.Mapping {
logrus.WithFields(logrus.Fields{
"name": vm.Name,
Expand All @@ -206,6 +208,29 @@ func (c *Client) PreFlightChecks(vm *migration.VirtualMachineImport) (err error)
return fmt.Errorf("source network '%s' not found", nm.SourceNetwork)
}
}

// Make sure the `harvesterhci.io/imageDisplayName` label set to the
// VirtualMachineImages meets the DNS-1123 standard.
// Note, in order to be able to validate the label content, we must
// temporarily bring forward the step `SanitizeVirtualMachineImport`,
// which is actually carried out after the preflight checks, as we
// need the definitive VM name here.
sanitizedVM := vm.DeepCopy()
if err := c.SanitizeVirtualMachineImport(sanitizedVM); err != nil {
return fmt.Errorf("failed to sanitize the object (Kind=%s, Name=%s) during preflight check: %v",
sanitizedVM.Kind, sanitizedVM.Name, err)
}
vmObj, err := c.findVM(sanitizedVM.Spec.VirtualMachineName)
if err != nil {
return err
}
for vIndex := range vmObj.AttachedVolumes {
imageDisplayName := fmt.Sprintf("vm-import-%s-%d.img", sanitizedVM.Status.ImportedVirtualMachineName, vIndex)
if errs := validation.IsDNS1123Label(imageDisplayName); len(errs) != 0 {
return fmt.Errorf("the VM display image name '%s' will not be RFC 1123 compliant: %v", imageDisplayName, errs)
}
}

return nil
}

Expand Down Expand Up @@ -318,7 +343,7 @@ func (c *Client) ExportVirtualMachine(vm *migration.VirtualMachineImport) error
return err
}

rawImageFileName := fmt.Sprintf("%s-%d.img", vmObj.Name, vIndex)
rawImageFileName := fmt.Sprintf("%s-%d.img", vm.Status.ImportedVirtualMachineName, vIndex)

logrus.WithFields(logrus.Fields{
"name": vm.Name,
Expand Down
4 changes: 2 additions & 2 deletions pkg/source/vmware/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ func (c *Client) ExportVirtualMachine(vm *migration.VirtualMachineImport) (err e
// once the disks are converted this needs to be updated to ".img"
// spec for how download_url is generated
// Spec: harvesterv1beta1.VirtualMachineImageSpec{
// DisplayName: fmt.Sprintf("vm-import-%s-%s", vm.Name, d.Name),
// DisplayName: fmt.Sprintf("vm-import-%s", d.Name),
// URL: fmt.Sprintf("http://%s:%d/%s.img", server.Address(), server.DefaultPort(), d.Name),
// },

Expand All @@ -244,7 +244,7 @@ func (c *Client) ExportVirtualMachine(vm *migration.VirtualMachineImport) (err e
destFile := filepath.Join(server.TempDir(), rawDiskName)
err = qemu.ConvertVMDKtoRAW(sourceFile, destFile)
if err != nil {
return fmt.Errorf("error during conversion of vmdk to raw disk %v", err)
return fmt.Errorf("error during conversion of vmdk to raw disk: %v", err)
}
// update fields to reflect final location of raw image file
vm.Status.DiskImportStatus[i].DiskLocalPath = server.TempDir()
Expand Down

0 comments on commit eddc5ef

Please sign in to comment.