Merge pull request openshift#8059 from jcpowermac/capv-zonal

OCPBUGS-29860: double looping of failuredomains breaks zonal

Author: openshift-merge-bot[bot]

pkg/asset/installconfig/vsphere/folder.go

@@ -0,0 +1,36 @@
+//go:build !altinfra
+// +build !altinfra
+
+package vsphere
+
+import (
+	"context"
+	"time"
+
+	"k8s.io/apimachinery/pkg/util/validation/field"
+)
+
+// 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(validationCtx *validationContext, folderPath string, fldPath *field.Path) field.ErrorList {
+	allErrs := field.ErrorList{}
+	finder := validationCtx.Finder
+	// If no folder is specified, skip this check as the folder will be created.
+	if folderPath == "" {
+		return allErrs
+	}
+
+	ctx, cancel := context.WithTimeout(context.TODO(), 60*time.Second)
+	defer cancel()
+
+	folder, err := finder.Folder(ctx, folderPath)
+	if err != nil {
+		return append(allErrs, field.Invalid(fldPath, folderPath, err.Error()))
+	}
+	permissionGroup := permissions[permissionFolder]
+
+	err = comparePrivileges(ctx, validationCtx, folder.Reference(), permissionGroup)
+	if err != nil {
+		return append(allErrs, field.InternalError(fldPath, err))
+	}
+	return allErrs
+}

pkg/asset/installconfig/vsphere/folder_altinfra.go

@@ -0,0 +1,44 @@
+//go:build altinfra
+// +build altinfra
+
+package vsphere
+
+import (
+	"context"
+	"errors"
+	"time"
+
+	"github.com/vmware/govmomi/find"
+	"k8s.io/apimachinery/pkg/util/validation/field"
+)
+
+// 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(validationCtx *validationContext, folderPath string, fldPath *field.Path) field.ErrorList {
+	var notFoundError *find.NotFoundError
+	allErrs := field.ErrorList{}
+	finder := validationCtx.Finder
+	// If no folder is specified, skip this check as the folder will be created.
+	if folderPath == "" {
+		return allErrs
+	}
+
+	ctx, cancel := context.WithTimeout(context.TODO(), 60*time.Second)
+	defer cancel()
+
+	folder, err := finder.Folder(ctx, folderPath)
+	if err != nil && !errors.As(err, &notFoundError) {
+		return append(allErrs, field.Invalid(fldPath, folderPath, err.Error()))
+	}
+
+	// folder was not found so no privilege check can be performed
+	if folder == nil {
+		return allErrs
+	}
+	permissionGroup := permissions[permissionFolder]
+
+	err = comparePrivileges(ctx, validationCtx, folder.Reference(), permissionGroup)
+	if err != nil {
+		return append(allErrs, field.InternalError(fldPath, err))
+	}
+	return allErrs
+}

pkg/asset/installconfig/vsphere/validation.go

@@ -190,31 +190,6 @@ func validateFailureDomain(validationCtx *validationContext, failureDomain *vsph
 	return allErrs
 }
 
-// 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(validationCtx *validationContext, folderPath string, fldPath *field.Path) field.ErrorList {
-	allErrs := field.ErrorList{}
-	finder := validationCtx.Finder
-	// If no folder is specified, skip this check as the folder will be created.
-	if folderPath == "" {
-		return allErrs
-	}
-
-	ctx, cancel := context.WithTimeout(context.TODO(), 60*time.Second)
-	defer cancel()
-
-	folder, err := finder.Folder(ctx, folderPath)
-	if err != nil {
-		return append(allErrs, field.Invalid(fldPath, folderPath, err.Error()))
-	}
-	permissionGroup := permissions[permissionFolder]
-
-	err = comparePrivileges(ctx, validationCtx, folder.Reference(), permissionGroup)
-	if err != nil {
-		return append(allErrs, field.InternalError(fldPath, err))
-	}
-	return allErrs
-}
-
 func validateVCenterVersion(validationCtx *validationContext, fldPath *field.Path) field.ErrorList {
 	allErrs := field.ErrorList{}
 

pkg/infrastructure/vsphere/clusterapi/clusterapi.go

@@ -10,7 +10,6 @@ import (
 	"sigs.k8s.io/cluster-api-provider-vsphere/pkg/session"
 
 	"github.com/openshift/installer/pkg/asset/installconfig"
-	"github.com/openshift/installer/pkg/asset/rhcos"
 	"github.com/openshift/installer/pkg/infrastructure/clusterapi"
 	"github.com/openshift/installer/pkg/rhcos/cache"
 	"github.com/openshift/installer/pkg/types/vsphere"
@@ -28,58 +27,59 @@ func (p Provider) Name() string {
 	return vsphere.Name
 }
 
-func initializeFoldersAndTemplates(ctx context.Context, rhcosImage *rhcos.Image, installConfig *installconfig.InstallConfig, session *session.Session, clusterID, server, tagID string) error {
+func initializeFoldersAndTemplates(ctx context.Context, cachedImage string, failureDomain vsphere.FailureDomain, session *session.Session, diskType vsphere.DiskType, clusterID, tagID string) error {
 	finder := session.Finder
 
-	platform := installConfig.Config.VSphere
-	failureDomains := platform.FailureDomains
-
-	for _, failureDomain := range failureDomains {
-		dc, err := finder.Datacenter(ctx, failureDomain.Topology.Datacenter)
-		if err != nil {
-			return err
-		}
-		dcFolders, err := dc.Folders(ctx)
-		if err != nil {
-			return fmt.Errorf("unable to get datacenter folder: %w", err)
-		}
-
-		folderPath := path.Join(dcFolders.VmFolder.InventoryPath, clusterID)
+	dc, err := finder.Datacenter(ctx, failureDomain.Topology.Datacenter)
+	if err != nil {
+		return err
+	}
+	dcFolders, err := dc.Folders(ctx)
+	if err != nil {
+		return fmt.Errorf("unable to get datacenter folder: %w", err)
+	}
 
-		// we must set the Folder to the infraId somewhere, we will need to remove that.
-		// if we are overwriting folderPath it needs to have a slash (path)
-		folder := failureDomain.Topology.Folder
-		if strings.Contains(folder, "/") {
-			folderPath = folder
-		}
+	folderPath := path.Join(dcFolders.VmFolder.InventoryPath, clusterID)
 
-		folderMo, err := createFolder(ctx, folderPath, session)
-		if err != nil {
-			return fmt.Errorf("unable to create folder: %w", err)
-		}
+	// we must set the Folder to the infraId somewhere, we will need to remove that.
+	// if we are overwriting folderPath it needs to have a slash (path)
+	folder := failureDomain.Topology.Folder
+	if strings.Contains(folder, "/") {
+		folderPath = folder
+	}
 
-		cachedImage, err := cache.DownloadImageFile(string(*rhcosImage), cache.InstallerApplicationName)
-		if err != nil {
-			return fmt.Errorf("failed to use cached vsphere image: %w", err)
-		}
+	folderMo, err := createFolder(ctx, folderPath, session)
+	if err != nil {
+		return fmt.Errorf("unable to create folder: %w", err)
+	}
 
-		// if the template is empty, the ova must be imported
-		if len(failureDomain.Topology.Template) == 0 {
-			if err = importRhcosOva(ctx, session, folderMo,
-				cachedImage, clusterID, tagID, string(platform.DiskType), failureDomain); err != nil {
-				return fmt.Errorf("failed to import ova: %w", err)
-			}
+	// if the template is empty, the ova must be imported
+	if len(failureDomain.Topology.Template) == 0 {
+		if err = importRhcosOva(ctx, session, folderMo,
+			cachedImage, clusterID, tagID, string(diskType), failureDomain); err != nil {
+			return fmt.Errorf("failed to import ova: %w", err)
 		}
 	}
 	return nil
 }
 
 // PreProvision creates the vCenter objects required prior to running capv.
 func (p Provider) PreProvision(ctx context.Context, in clusterapi.PreProvisionInput) error {
+	/*
+	 * one locally cached image
+	 * one tag and tag category per vcenter
+	 * one folder per datacenter
+	 * one template per region/zone aka failuredomain
+	 */
 	installConfig := in.InstallConfig
 	clusterID := &installconfig.ClusterID{InfraID: in.InfraID}
 	var tagID string
 
+	cachedImage, err := cache.DownloadImageFile(string(*in.RhcosImage), cache.InstallerApplicationName)
+	if err != nil {
+		return fmt.Errorf("failed to use cached vsphere image: %w", err)
+	}
+
 	for _, vcenter := range installConfig.Config.VSphere.VCenters {
 		server := vcenter.Server
 		vctrSession, err := installConfig.VSphere.Session(context.TODO(), server)
@@ -105,7 +105,8 @@ func (p Provider) PreProvision(ctx context.Context, in clusterapi.PreProvisionIn
 			if failureDomain.Server != server {
 				continue
 			}
-			if err = initializeFoldersAndTemplates(ctx, in.RhcosImage, installConfig, vctrSession, clusterID.InfraID, server, tagID); err != nil {
+
+			if err = initializeFoldersAndTemplates(ctx, cachedImage, failureDomain, vctrSession, installConfig.Config.VSphere.DiskType, clusterID.InfraID, tagID); err != nil {
 				return fmt.Errorf("unable to initialize folders and templates: %w", err)
 			}
 		}

pkg/infrastructure/vsphere/clusterapi/folder.go

@@ -11,16 +11,21 @@ import (
 )
 
 func createFolder(ctx context.Context, fullpath string, session *session.Session) (*object.Folder, error) {
+	var notFoundError *find.NotFoundError
+
 	dir := path.Dir(fullpath)
 	base := path.Base(fullpath)
 	finder := session.Finder
 
 	folder, err := finder.Folder(ctx, fullpath)
+	if err != nil && !errors.As(err, &notFoundError) {
+		return nil, err
+	}
 
+	// if folder is nil the fullpath does not exist
 	if folder == nil {
 		folder, err = finder.Folder(ctx, dir)
 
-		var notFoundError *find.NotFoundError
 		if errors.As(err, &notFoundError) {
 			folder, err = createFolder(ctx, dir, session)
 			if err != nil {