Merge pull request #470 from mansam/vmware-tools-validation

Add validation that VMWare Tools are present

cluster/manifests/vcsim.yaml.in

@@ -17,7 +17,7 @@ spec:
       serviceAccountName: cdi-sa
       containers:
       - name: vcsim
-        image: mansam/vcsim:latest
+        image: quay.io/slucidi/vcsim:latest
         imagePullPolicy: IfNotPresent
         ports:
         - containerPort: 8989

pkg/providers/vmware/provider.go

@@ -3,6 +3,7 @@ package vmware
 import (
 	"encoding/xml"
 	"fmt"
+	"strings"
 
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	cdiv1 "kubevirt.io/containerized-data-importer/pkg/apis/core/v1alpha1"
@@ -200,6 +201,7 @@ func (r *VmwareProvider) LoadVM(sourceSpec v1beta1.VirtualMachineImportSourceSpe
 		return err
 	}
 	r.vm = vm.(*object.VirtualMachine)
+
 	return nil
 }
 
@@ -369,19 +371,29 @@ func (r *VmwareProvider) TestConnection() error {
 
 // Validate checks whether the source VM and resource mapping is valid.
 func (r *VmwareProvider) Validate() ([]v1beta1.VirtualMachineImportCondition, error) {
+	vmProperties, err := r.getVmProperties()
+	if err != nil {
+		return nil, err
+	}
+
 	validCondition := conditions.NewCondition(v1beta1.Valid, string(v1beta1.ValidationCompleted), "Validation completed successfully", corev1.ConditionTrue)
 	mappingCondition := conditions.NewCondition(v1beta1.MappingRulesVerified, string(v1beta1.MappingRulesVerificationCompleted), "All mapping rules checks passed", corev1.ConditionTrue)
+	validationFailures := make([]string, 0)
+
+	if vmProperties.Guest.ToolsStatus != types.VirtualMachineToolsStatusToolsOk && vmProperties.Runtime.PowerState != types.VirtualMachinePowerStatePoweredOff {
+		validationFailures = append(validationFailures, "VM must be powered off, or up to date VMWare Tools must be installed and running to allow the guest to be shutdown gracefully")
+	}
 
 	if r.instance.Spec.Warm {
-		vmProperties, err := r.getVmProperties()
-		if err != nil {
-			return nil, err
-		}
 		if vmProperties.Config.ChangeTrackingEnabled == nil || !*vmProperties.Config.ChangeTrackingEnabled {
-			//validCondition = conditions.NewCondition(v1beta1.Valid, string(v1beta1.ValidationFailed), "Changed Block Tracking must be enabled to allow warm import", corev1.ConditionFalse)
+			validationFailures = append(validationFailures, "Changed Block Tracking must be enabled to allow warm import")
 		}
 	}
 
+	if len(validationFailures) > 0 {
+		validCondition = conditions.NewCondition(v1beta1.Valid, string(v1beta1.ValidationFailed), strings.Join(validationFailures, "; "), corev1.ConditionFalse)
+	}
+
 	return []v1beta1.VirtualMachineImportCondition{validCondition, mappingCondition}, nil
 }
 

pkg/providers/vmware/provider_test.go

@@ -82,6 +82,129 @@ func getSimulatorVMIdentifiers(vm *simulator.VirtualMachine) (string, string, st
 	return vm.Reference().Value, vm.Config.Uuid, vm.Name
 }
 
+var _ = Describe("Validation", func() {
+	var provider *VmwareProvider
+	var model *simulator.Model
+	var server *simulator.Server
+
+	BeforeEach(func() {
+		model, server, provider = makeProvider()
+	})
+
+	AfterEach(func() {
+		server.Close()
+		model.Remove()
+	})
+
+	It("should throw a validation failure if a warm import was requested but CBT is disabled", func() {
+		vm := getSimulatorVM()
+		_, uuid, _ := getSimulatorVMIdentifiers(vm)
+
+		vm.Runtime.PowerState = types.VirtualMachinePowerStatePoweredOff
+		cbtEnabled := false
+		vm.Config.ChangeTrackingEnabled = &cbtEnabled
+		provider.instance.Spec.Warm = true
+		provider.instance.Spec.Source = v1beta1.VirtualMachineImportSourceSpec{
+			Vmware: &v1beta1.VirtualMachineImportVmwareSourceSpec{
+				VM: v1beta1.VirtualMachineImportVmwareSourceVMSpec{
+					ID:   &uuid,
+					Name: nil,
+				},
+			},
+		}
+
+		conditions, err := provider.Validate()
+		Expect(err).To(BeNil())
+
+		// valid condition, then mapping condition
+		Expect(len(conditions)).To(Equal(2))
+		Expect(conditions[0].Type).To(Equal(v1beta1.Valid))
+		Expect(*conditions[0].Reason).To(Equal(string(v1beta1.ValidationFailed)))
+		Expect(conditions[0].Status).To(Equal(v1.ConditionFalse))
+		Expect(*conditions[0].Message).To(ContainSubstring("Changed Block Tracking must be enabled to allow warm import"))
+	})
+
+	It("should not throw a validation failure if a warm import was requested and CBT is enabled", func() {
+		vm := getSimulatorVM()
+		_, uuid, _ := getSimulatorVMIdentifiers(vm)
+
+		vm.Runtime.PowerState = types.VirtualMachinePowerStatePoweredOff
+		cbtEnabled := true
+		vm.Config.ChangeTrackingEnabled = &cbtEnabled
+		provider.instance.Spec.Warm = true
+		provider.instance.Spec.Source = v1beta1.VirtualMachineImportSourceSpec{
+			Vmware: &v1beta1.VirtualMachineImportVmwareSourceSpec{
+				VM: v1beta1.VirtualMachineImportVmwareSourceVMSpec{
+					ID:   &uuid,
+					Name: nil,
+				},
+			},
+		}
+
+		conditions, err := provider.Validate()
+		Expect(err).To(BeNil())
+
+		// valid condition, then mapping condition
+		Expect(len(conditions)).To(Equal(2))
+		Expect(conditions[0].Type).To(Equal(v1beta1.Valid))
+		Expect(*conditions[0].Reason).To(Equal(string(v1beta1.ValidationCompleted)))
+		Expect(conditions[0].Status).To(Equal(v1.ConditionTrue))
+		Expect(*conditions[0].Message).ToNot(ContainSubstring("Changed Block Tracking must be enabled to allow warm import"))
+	})
+
+	It("should throw a validation failure the VM is powered on but the VMware tools aren't installed", func() {
+		vm := getSimulatorVM()
+		_, uuid, _ := getSimulatorVMIdentifiers(vm)
+
+		vm.Runtime.PowerState = types.VirtualMachinePowerStatePoweredOn
+		vm.Guest.ToolsStatus = types.VirtualMachineToolsStatusToolsNotInstalled
+		provider.instance.Spec.Source = v1beta1.VirtualMachineImportSourceSpec{
+			Vmware: &v1beta1.VirtualMachineImportVmwareSourceSpec{
+				VM: v1beta1.VirtualMachineImportVmwareSourceVMSpec{
+					ID:   &uuid,
+					Name: nil,
+				},
+			},
+		}
+
+		conditions, err := provider.Validate()
+		Expect(err).To(BeNil())
+
+		// valid condition, then mapping condition
+		Expect(len(conditions)).To(Equal(2))
+		Expect(conditions[0].Type).To(Equal(v1beta1.Valid))
+		Expect(*conditions[0].Reason).To(Equal(string(v1beta1.ValidationFailed)))
+		Expect(conditions[0].Status).To(Equal(v1.ConditionFalse))
+		Expect(*conditions[0].Message).To(ContainSubstring("VM must be powered off, or up to date VMWare Tools must be installed and running to allow the guest to be shutdown gracefully"))
+	})
+
+	It("should not throw a validation failure the VM is powered on but the VMware tools are installed", func() {
+		vm := getSimulatorVM()
+		_, uuid, _ := getSimulatorVMIdentifiers(vm)
+
+		vm.Runtime.PowerState = types.VirtualMachinePowerStatePoweredOn
+		vm.Guest.ToolsStatus = types.VirtualMachineToolsStatusToolsOk
+		provider.instance.Spec.Source = v1beta1.VirtualMachineImportSourceSpec{
+			Vmware: &v1beta1.VirtualMachineImportVmwareSourceSpec{
+				VM: v1beta1.VirtualMachineImportVmwareSourceVMSpec{
+					ID:   &uuid,
+					Name: nil,
+				},
+			},
+		}
+
+		conditions, err := provider.Validate()
+		Expect(err).To(BeNil())
+
+		// valid condition, then mapping condition
+		Expect(len(conditions)).To(Equal(2))
+		Expect(conditions[0].Type).To(Equal(v1beta1.Valid))
+		Expect(*conditions[0].Reason).To(Equal(string(v1beta1.ValidationCompleted)))
+		Expect(conditions[0].Status).To(Equal(v1.ConditionTrue))
+		Expect(*conditions[0].Message).ToNot(ContainSubstring("VM must be powered off, or up to date VMWare Tools must be installed and running to allow the guest to be shutdown gracefully"))
+	})
+})
+
 var _ = Describe("Initialization", func() {
 	provider := VmwareProvider{}
 	instance := &v1beta1.VirtualMachineImport{}