Merge pull request openshift#8671 from jcpowermac/OCPBUGS-36242

OCPBUGS-36242: vSphere - If the folder pre-exists do not tag

Author: openshift-merge-bot[bot]

pkg/asset/installconfig/vsphere/folder.go

@@ -1,17 +1,17 @@
-//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.
@@ -23,9 +23,14 @@ func folderExists(validationCtx *validationContext, folderPath string, fldPath *
 	defer cancel()
 
 	folder, err := finder.Folder(ctx, folderPath)
-	if err != nil {
+	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)

pkg/asset/installconfig/vsphere/folder_altinfra.go

@@ -361,27 +361,15 @@ func TestValidateFailureDomains(t *testing.T) {
 			checkTags:        false,
 			expectErr:        `^platform.vsphere.failureDomains.topology: Invalid value: "invalid-network": unable to find network provided$`,
 		}, {
-			name: "multi-zone validation - invalid folder",
+			name: "multi-zone validation - create missing folder",
 			failureDomain: func() *vsphere.FailureDomain {
 				failureDomain := &validMultiVCenterPlatform().FailureDomains[0]
-				failureDomain.Topology.Folder = "/DC0/vm/invalid-folder"
+				failureDomain.Topology.Folder = "/DC0/vm/create-missing-folder"
 				return failureDomain
 			}(),
 			validationMethod: validateFailureDomain,
 			checkTags:        false,
-			expectErr:        `^platform.vsphere.failureDomains.topology.folder: Invalid value: "/DC0/vm/invalid-folder": folder '/DC0/vm/invalid-folder' not found$`,
-		},
-
-		{
-			name: "multi-zone validation - invalid folder",
-			failureDomain: func() *vsphere.FailureDomain {
-				failureDomain := &validMultiVCenterPlatform().FailureDomains[0]
-				failureDomain.Topology.Folder = "/DC0/vm/invalid-folder"
-				return failureDomain
-			}(),
-			validationMethod: validateFailureDomain,
-			checkTags:        false,
-			expectErr:        `^platform.vsphere.failureDomains.topology.folder: Invalid value: "/DC0/vm/invalid-folder": folder '/DC0/vm/invalid-folder' not found$`,
+			expectErr:        ``,
 		}, {
 			name:             "multi-zone tag categories present and tags attached",
 			validationMethod: validateFailureDomain,

pkg/asset/installconfig/vsphere/validation_test.go

@@ -6,6 +6,7 @@ import (
 	"path"
 	"strings"
 
+	"github.com/vmware/govmomi/object"
 	"sigs.k8s.io/cluster-api-provider-vsphere/apis/v1beta1"
 	"sigs.k8s.io/cluster-api-provider-vsphere/pkg/session"
 
@@ -52,20 +53,26 @@ func initializeFoldersAndTemplates(ctx context.Context, cachedImage string, fail
 		folderPath = folder
 	}
 
-	folderMo, err := createFolder(ctx, folderPath, session)
-	if err != nil {
-		return fmt.Errorf("unable to create folder: %w", err)
-	}
+	var folderObj *object.Folder
 
-	// attach tag to folder
-	err = session.TagManager.AttachTag(ctx, tagID, folderMo.Reference())
-	if err != nil {
-		return fmt.Errorf("unable to attach tag to folder: %w", err)
+	// Only createFolder() and attach the tag if the folder does not exist prior to installing
+	if folderObj, err = folderExists(ctx, folderPath, session); folderObj == nil && err == nil {
+		folderObj, err = createFolder(ctx, folderPath, session)
+		if err != nil {
+			return fmt.Errorf("unable to create folder: %w", err)
+		}
+		// attach tag to folder
+		err = session.TagManager.AttachTag(ctx, tagID, folderObj.Reference())
+		if err != nil {
+			return fmt.Errorf("unable to attach tag to folder: %w", err)
+		}
+	} else if err != nil {
+		return fmt.Errorf("unable to get 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,
+		if err = importRhcosOva(ctx, session, folderObj,
 			cachedImage, clusterID, tagID, string(diskType), failureDomain); err != nil {
 			return fmt.Errorf("failed to import ova: %w", err)
 		}

pkg/infrastructure/vsphere/clusterapi/clusterapi.go

@@ -10,35 +10,44 @@ import (
 	"sigs.k8s.io/cluster-api-provider-vsphere/pkg/session"
 )
 
-func createFolder(ctx context.Context, fullpath string, session *session.Session) (*object.Folder, error) {
+func folderExists(ctx context.Context, dir string, session *session.Session) (*object.Folder, error) {
+	/* scenarios:
+	 * 1. folder exists and returns
+	 * 2. folder does not exist and err and folder are nil
+	 * 3. finder.Folder fails and returns folder nil and error
+	 */
 	var notFoundError *find.NotFoundError
+	folder, err := session.Finder.Folder(ctx, dir)
 
-	dir := path.Dir(fullpath)
-	base := path.Base(fullpath)
-	finder := session.Finder
-
-	folder, err := finder.Folder(ctx, fullpath)
-	if err != nil && !errors.As(err, &notFoundError) {
+	// scenario two
+	if folder == nil && errors.As(err, &notFoundError) {
+		return nil, nil
+	}
+	// scenario three
+	if err != nil {
 		return nil, err
 	}
+	// scenario one
+	return folder, nil
+}
+
+func createFolder(ctx context.Context, fullpath string, session *session.Session) (*object.Folder, error) {
+	var folder *object.Folder
+	var err error
+
+	dir := path.Dir(fullpath)
+	base := path.Base(fullpath)
 
 	// if folder is nil the fullpath does not exist
-	if folder == nil {
-		folder, err = finder.Folder(ctx, dir)
-
-		if errors.As(err, &notFoundError) {
-			folder, err = createFolder(ctx, dir, session)
-			if err != nil {
-				return folder, err
-			}
+	if folder, err = folderExists(ctx, dir, session); err == nil && folder == nil {
+		folder, err = createFolder(ctx, dir, session)
+		if err != nil {
+			return nil, err
 		}
+	}
 
-		if folder != nil {
-			folder, err = folder.CreateFolder(ctx, base)
-			if err != nil {
-				return folder, err
-			}
-		}
+	if folder != nil && err == nil {
+		return folder.CreateFolder(ctx, base)
 	}
 	return folder, err
 }