Bug 1826100: vSphere allow users to specify existing folder

data/data/vsphere/main.tf

@@ -1,3 +1,7 @@
+locals {
+  folder = var.vsphere_preexisting_folder ? var.vsphere_folder : vsphere_folder.folder[0].path
+}
+
 provider "vsphere" {
   user                 = var.vsphere_username
   password             = var.vsphere_password
@@ -43,7 +47,7 @@ resource "vsphereprivate_import_ova" "import" {
   datacenter = var.vsphere_datacenter
   datastore  = var.vsphere_datastore
   network    = var.vsphere_network
-  folder     = vsphere_folder.folder.path
+  folder     = local.folder
   tag        = vsphere_tag.tag.id
 }
 
@@ -66,6 +70,8 @@ resource "vsphere_tag" "tag" {
 }
 
 resource "vsphere_folder" "folder" {
+  count = var.vsphere_preexisting_folder ? 0 : 1
+
   path          = var.vsphere_folder
   type          = "vm"
   datacenter_id = data.vsphere_datacenter.datacenter.id
@@ -79,7 +85,7 @@ module "bootstrap" {
   ignition      = var.ignition_bootstrap
   resource_pool = data.vsphere_compute_cluster.cluster.resource_pool_id
   datastore     = data.vsphere_datastore.datastore.id
-  folder        = vsphere_folder.folder.path
+  folder        = local.folder
   network       = data.vsphere_network.network.id
   datacenter    = data.vsphere_datacenter.datacenter.id
   template      = data.vsphere_virtual_machine.template.id
@@ -101,7 +107,7 @@ module "master" {
 
   resource_pool = data.vsphere_compute_cluster.cluster.resource_pool_id
   datastore     = data.vsphere_datastore.datastore.id
-  folder        = vsphere_folder.folder.path
+  folder        = local.folder
   network       = data.vsphere_network.network.id
   datacenter    = data.vsphere_datacenter.datacenter.id
   template      = data.vsphere_virtual_machine.template.id

data/data/vsphere/variables-vsphere.tf

@@ -48,7 +48,13 @@ variable "vsphere_network" {
 }
 
 variable "vsphere_folder" {
-  type = string
+  type        = string
+  description = "The relative path to the folder which should be used or created for VMs."
+}
+
+variable "vsphere_preexisting_folder" {
+  type        = bool
+  description = "If false, creates a top-level folder with the name from vsphere_folder_rel_path."
 }
 
 ///////////

docs/user/vsphere/customization.md

@@ -9,6 +9,7 @@ Beyond the [platform-agnostic `install-config.yaml` properties](../customization
 * `password` (required string): The password to use to connect to the vCenter.
 * `datacenter` (required string): The name of the datacenter to use in the vCenter.
 * `defaultDatastore` (required string): The default datastore to use for provisioning volumes.
+* `folder` (optional string): The absolute path of an existing folder where the installer should create VMs. The absolute path is of the form `/example_datacenter/vm/example_folder/example_subfolder`. If a value is specified, the folder must exist. If no value is specified, a folder named with the cluster ID will be created in the `datacenter` VM folder.
 
 ## Machine pools
 

pkg/asset/cluster/tfvars.go

@@ -461,13 +461,18 @@ func (t *TerraformVariables) Generate(parents asset.Parents) error {
 		for i, c := range controlPlanes {
 			controlPlaneConfigs[i] = c.Spec.ProviderSpec.Value.Object.(*vsphereprovider.VSphereMachineProviderSpec)
 		}
+
+		// Set this flag to use an existing folder specified in the install-config. Otherwise, create one.
+		preexistingFolder := installConfig.Config.Platform.VSphere.Folder != ""
+
 		data, err = vspheretfvars.TFVars(
 			vspheretfvars.TFVarsSources{
 				ControlPlaneConfigs: controlPlaneConfigs,
 				Username:            installConfig.Config.VSphere.Username,
 				Password:            installConfig.Config.VSphere.Password,
 				Cluster:             installConfig.Config.VSphere.Cluster,
 				ImageURL:            string(*rhcosImage),
+				PreexistingFolder:   preexistingFolder,
 			},
 		)
 		if err != nil {

pkg/asset/installconfig/vsphere/validation.go

@@ -1,10 +1,15 @@
 package vsphere
 
 import (
+	"context"
+	"time"
+
 	"github.com/pkg/errors"
+	"github.com/vmware/govmomi/find"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 
 	"github.com/openshift/installer/pkg/types"
+	vspheretypes "github.com/openshift/installer/pkg/types/vsphere"
 	"github.com/openshift/installer/pkg/types/vsphere/validation"
 )
 
@@ -16,6 +21,7 @@ func Validate(ic *types.InstallConfig) error {
 	}
 
 	allErrs = append(allErrs, validation.ValidatePlatform(ic.Platform.VSphere, field.NewPath("platform").Child("vsphere"))...)
+	allErrs = append(allErrs, folderExists(ic, field.NewPath("platform").Child("vsphere").Child("folder"))...)
 
 	return allErrs.ToAggregate()
 }
@@ -33,3 +39,30 @@ func ValidateForProvisioning(ic *types.InstallConfig) error {
 
 	return allErrs.ToAggregate()
 }
+
+// folderExists returns an error if a folder is specified in the vSphere platform but a folder with that name is not found in the datacenter.
+func folderExists(ic *types.InstallConfig, fldPath *field.Path) field.ErrorList {
+	allErrs := field.ErrorList{}
+	cfg := ic.VSphere
+
+	// If no folder is specified, skip this check as the folder will be created.
+	if cfg.Folder == "" {
+		return allErrs
+	}
+
+	vim25Client, _, err := vspheretypes.CreateVSphereClients(context.TODO(), cfg.VCenter, cfg.Username, cfg.Password)
+	if err != nil {
+		err = errors.Wrap(err, "unable to connect to vCenter API")
+		return append(allErrs, field.InternalError(fldPath, err))
+	}
+
+	finder := find.NewFinder(vim25Client)
+
+	ctx, cancel := context.WithTimeout(context.TODO(), 60*time.Second)
+	defer cancel()
+
+	if _, err = finder.Folder(ctx, cfg.Folder); err != nil {
+		return append(allErrs, field.Invalid(fldPath, cfg.Folder, err.Error()))
+	}
+	return nil
+}

pkg/asset/machines/vsphere/machines.go

@@ -64,6 +64,11 @@ func Machines(clusterID string, config *types.InstallConfig, pool *types.Machine
 }
 
 func provider(clusterID string, platform *vsphere.Platform, mpool *vsphere.MachinePool, osImage string, userDataSecret string) (*vsphereapis.VSphereMachineProviderSpec, error) {
+	folder := fmt.Sprintf("/%s/vm/%s", platform.Datacenter, clusterID)
+	if platform.Folder != "" {
+		folder = platform.Folder
+	}
+
 	return &vsphereapis.VSphereMachineProviderSpec{
 		TypeMeta: metav1.TypeMeta{
 			APIVersion: vsphereapis.SchemeGroupVersion.String(),
@@ -83,7 +88,7 @@ func provider(clusterID string, platform *vsphere.Platform, mpool *vsphere.Machi
 			Server:     platform.VCenter,
 			Datacenter: platform.Datacenter,
 			Datastore:  platform.DefaultDatastore,
-			Folder:     clusterID,
+			Folder:     folder,
 		},
 		NumCPUs:           mpool.NumCPUs,
 		NumCoresPerSocket: mpool.NumCoresPerSocket,

pkg/asset/manifests/cloudproviderconfig.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"io/ioutil"
 	"path/filepath"
+	"strings"
 
 	"github.com/ghodss/yaml"
 	"github.com/pkg/errors"
@@ -146,8 +147,14 @@ func (cpc *CloudProviderConfig) Generate(dependencies asset.Parents) error {
 		}
 		cm.Data[cloudProviderConfigDataKey] = gcpConfig
 	case vspheretypes.Name:
+		var folderRelPath string
+		if len(installConfig.Config.Platform.VSphere.Folder) != 0 {
+			folderRelPath = strings.SplitAfterN(installConfig.Config.Platform.VSphere.Folder, "vm/", 2)[1]
+		}
+
 		vsphereConfig, err := vspheremanifests.CloudProviderConfig(
-			installConfig.Config.ObjectMeta.Name,
+			clusterID.InfraID,
+			folderRelPath,
 			installConfig.Config.Platform.VSphere,
 		)
 		if err != nil {

pkg/asset/manifests/vsphere/cloudproviderconfig.go

@@ -14,7 +14,7 @@ func printIfNotEmpty(buf *bytes.Buffer, k, v string) {
 }
 
 // CloudProviderConfig generates the cloud provider config for the vSphere platform.
-func CloudProviderConfig(clusterName string, p *vspheretypes.Platform) (string, error) {
+func CloudProviderConfig(clusterID, folderRelPath string, p *vspheretypes.Platform) (string, error) {
 	buf := new(bytes.Buffer)
 
 	fmt.Fprintln(buf, "[Global]")
@@ -27,9 +27,9 @@ func CloudProviderConfig(clusterName string, p *vspheretypes.Platform) (string,
 	printIfNotEmpty(buf, "server", p.VCenter)
 	printIfNotEmpty(buf, "datacenter", p.Datacenter)
 	printIfNotEmpty(buf, "default-datastore", p.DefaultDatastore)
-	printIfNotEmpty(buf, "folder", p.Folder)
+	printIfNotEmpty(buf, "folder", folderRelPath)
 	if p.Folder == "" {
-		printIfNotEmpty(buf, "folder", clusterName)
+		printIfNotEmpty(buf, "folder", clusterID)
 	}
 	fmt.Fprintln(buf, "")
 

pkg/asset/manifests/vsphere/cloudproviderconfig_test.go

@@ -10,6 +10,7 @@ import (
 
 func TestCloudProviderConfig(t *testing.T) {
 	clusterName := "test-cluster"
+	folderRelPath := ""
 	platform := &vspheretypes.Platform{
 		VCenter:          "test-name",
 		Username:         "test-username",
@@ -31,7 +32,7 @@ folder = "test-cluster"
 [VirtualCenter "test-name"]
 datacenters = "test-datacenter"
 `
-	actualConfig, err := CloudProviderConfig(clusterName, platform)
+	actualConfig, err := CloudProviderConfig(clusterName, folderRelPath, platform)
 	assert.NoError(t, err, "failed to create cloud provider config")
 	assert.Equal(t, expectedConfig, actualConfig, "unexpected cloud provider config")
 }

pkg/destroy/vsphere/vsphere.go

@@ -52,32 +52,29 @@ func New(logger logrus.FieldLogger, metadata *installertypes.ClusterMetadata) (p
 	}, nil
 }
 
-func deleteVirtualMachines(ctx context.Context, client *vim25.Client, virtualMachineMoList []mo.VirtualMachine, parentFolder mo.Folder, logger logrus.FieldLogger) error {
+func deleteVirtualMachines(ctx context.Context, client *vim25.Client, virtualMachineMoList []mo.VirtualMachine, logger logrus.FieldLogger) error {
 	ctx, cancel := context.WithTimeout(ctx, time.Minute*30)
 	defer cancel()
 
 	if len(virtualMachineMoList) != 0 {
 		for _, vmMO := range virtualMachineMoList {
 			virtualMachineLogger := logger.WithField("VirtualMachine", vmMO.Name)
-			// Checking if the Folder mobRef is the same as the VirtualMachine parent mobRef
-			if parentFolder.Reference() == vmMO.Parent.Reference() {
-				vm := object.NewVirtualMachine(client, vmMO.Reference())
-				if vmMO.Summary.Runtime.PowerState == "poweredOn" {
-					task, err := vm.PowerOff(ctx)
-					if err != nil {
-						return err
-					}
-					task.Wait(ctx)
-					virtualMachineLogger.Debug("Powered off")
-				}
-
-				task, err := vm.Destroy(ctx)
+			vm := object.NewVirtualMachine(client, vmMO.Reference())
+			if vmMO.Summary.Runtime.PowerState == "poweredOn" {
+				task, err := vm.PowerOff(ctx)
 				if err != nil {
 					return err
 				}
 				task.Wait(ctx)
-				virtualMachineLogger.Info("Destroyed")
+				virtualMachineLogger.Debug("Powered off")
+			}
+
+			task, err := vm.Destroy(ctx)
+			if err != nil {
+				return err
 			}
+			task.Wait(ctx)
+			virtualMachineLogger.Info("Destroyed")
 		}
 	}
 	return nil
@@ -157,31 +154,32 @@ func (o *ClusterUninstaller) Run() error {
 		}
 	}
 
-	// There should be exactly one Folder which is created by
-	// terraform.  That is the parent to the VirtualMachines.
+	// The installer should create at most one parent,
+	// the parent to the VirtualMachines.
 	// If there are more or less fail with error message.
-	if len(folderList) != 1 {
+	if len(folderList) > 1 {
 		return errors.Errorf("Expected 1 Folder per tag but got %d", len(folderList))
 	}
 
-	o.Logger.Debug("find Folder objects")
-	folderMoList, err := getFolderManagedObjects(context.TODO(), o.Client, folderList)
-
 	o.Logger.Debug("find VirtualMachine objects")
 	virtualMachineMoList, err := getVirtualMachineManagedObjects(context.TODO(), o.Client, virtualMachineList)
 	if err != nil {
 		return err
 	}
 	o.Logger.Debug("delete VirtualMachines")
-	err = deleteVirtualMachines(context.TODO(), o.Client, virtualMachineMoList, folderMoList[0], o.Logger)
+	err = deleteVirtualMachines(context.TODO(), o.Client, virtualMachineMoList, o.Logger)
 	if err != nil {
 		return err
 	}
-	// When removing virtual machines the mo.Folder object does not change
-	// We need to re-retrieve the list again
+
+	// In this case, folder was user-provided
+	// and should not be deleted so we are done.
+	if len(folderList) == 0 {
+		return nil
+	}
+
 	o.Logger.Debug("find Folder objects")
-	folderMoList = nil
-	folderMoList, err = getFolderManagedObjects(context.TODO(), o.Client, folderList)
+	folderMoList, err := getFolderManagedObjects(context.TODO(), o.Client, folderList)
 	if err != nil {
 		o.Logger.Errorln(err)
 		return err

pkg/terraform/exec/plugins/vsphereprivate/resource_vsphereprivate_import_ova.go

@@ -114,9 +114,9 @@ func findImportOvaParams(client *vim25.Client, datacenter, cluster, datastore, n
 	}
 	importOvaParams.Datacenter = dcObj
 
-	// Find the newly created folder based on the path
-	// provided
-	folderObj, err := finder.Folder(ctx, folder)
+	// Create an absolute path to the folder in case the provided folder is nested.
+	folderPath := fmt.Sprintf("/%s/vm/%s", datacenter, folder)
+	folderObj, err := finder.Folder(ctx, folderPath)
 	if err != nil {
 		return nil, err
 	}

pkg/tfvars/vsphere/vsphere.go

@@ -2,6 +2,7 @@ package vsphere
 
 import (
 	"encoding/json"
+	"strings"
 
 	vsphereapis "github.com/openshift/machine-api-operator/pkg/apis/vsphereprovider/v1alpha1"
 	"github.com/pkg/errors"
@@ -24,6 +25,7 @@ type config struct {
 	Network           string `json:"vsphere_network"`
 	Template          string `json:"vsphere_template"`
 	OvaFilePath       string `json:"vsphere_ova_filepath"`
+	PreexistingFolder bool   `json:"vsphere_preexisting_folder"`
 }
 
 // TFVarsSources contains the parameters to be converted into Terraform variables
@@ -33,6 +35,7 @@ type TFVarsSources struct {
 	Password            string
 	Cluster             string
 	ImageURL            string
+	PreexistingFolder   bool
 }
 
 //TFVars generate vSphere-specific Terraform variables
@@ -44,6 +47,11 @@ func TFVars(sources TFVarsSources) ([]byte, error) {
 		return nil, errors.Wrap(err, "failed to use cached vsphere image")
 	}
 
+	// The vSphere provider needs the relativepath of the folder,
+	// so get the relPath from the absolute path. Absolute path is always of the form
+	// /<datacenter>/vm/<folder_path> so we can split on "vm/".
+	folderRelPath := strings.SplitAfterN(controlPlaneConfig.Workspace.Folder, "vm/", 2)[1]
+
 	cfg := &config{
 		VSphereURL:        controlPlaneConfig.Workspace.Server,
 		VSphereUsername:   sources.Username,
@@ -55,10 +63,11 @@ func TFVars(sources TFVarsSources) ([]byte, error) {
 		Cluster:           sources.Cluster,
 		Datacenter:        controlPlaneConfig.Workspace.Datacenter,
 		Datastore:         controlPlaneConfig.Workspace.Datastore,
-		Folder:            controlPlaneConfig.Workspace.Folder,
+		Folder:            folderRelPath,
 		Network:           controlPlaneConfig.Network.Devices[0].NetworkName,
 		Template:          controlPlaneConfig.Template,
 		OvaFilePath:       cachedImage,
+		PreexistingFolder: sources.PreexistingFolder,
 	}
 
 	return json.MarshalIndent(cfg, "", "  ")

pkg/types/validation/installconfig_test.go

@@ -690,6 +690,18 @@ func TestValidateInstallConfig(t *testing.T) {
 			}(),
 			expectedError: `^platform\.vsphere.vCenter: Required value: must specify the name of the vCenter$`,
 		},
+		{
+			name: "invalid vsphere folder",
+			installConfig: func() *types.InstallConfig {
+				c := validInstallConfig()
+				c.Platform = types.Platform{
+					VSphere: validVSpherePlatform(),
+				}
+				c.Platform.VSphere.Folder = "my-folder"
+				return c
+			}(),
+			expectedError: `^platform\.vsphere.folder: Invalid value: \"my-folder\": folder must be absolute path: expected prefix /test-datacenter/vm/$`,
+		},
 		{
 			name: "empty proxy settings",
 			installConfig: func() *types.InstallConfig {