Merge pull request vmware-tanzu#1378 from bryanv/bryanv/just-instance-storage-use-placemvs

🐛 Use PlaceVmsXCluster for most placement decisions

Author: bryanv

pkg/providers/vsphere/placement/cluster_placement.go

@@ -190,14 +190,14 @@ func PlaceVMForCreate(
 		ConfigSpec:    &configSpec,
 	}
 
-	vmCtx.Logger.V(4).Info("PlaceVMForCreate request", "placementSpec", vimtypes.ToString(placementSpec))
+	vmCtx.Logger.V(4).Info("PlaceVM request", "placementSpec", vimtypes.ToString(placementSpec))
 
 	resp, err := cluster.PlaceVm(vmCtx, placementSpec)
 	if err != nil {
 		return nil, err
 	}
 
-	vmCtx.Logger.V(6).Info("PlaceVMForCreate response", "resp", vimtypes.ToString(resp))
+	vmCtx.Logger.V(4).Info("PlaceVM response", "placementResult", vimtypes.ToString(resp))
 
 	for _, r := range resp.Recommendations {
 		if r.Reason != string(vimtypes.RecommendationReasonCodeXvmotionPlacement) {
@@ -216,21 +216,21 @@ func PlaceVMForCreate(
 	return nil, nil
 }
 
-// ClusterPlaceVMForCreate determines the suitable cluster placement among the specified ResourcePools.
-func ClusterPlaceVMForCreate(
+// getClusterPlacementRecommendations calls DRS PlaceVmsXCluster to get the placement
+// recommendations for the given VMs.
+func getClusterPlacementRecommendations(
 	ctx context.Context,
 	vcClient *vim25.Client,
 	finder *find.Finder,
 	resourcePoolsMoRefs []vimtypes.ManagedObjectReference,
 	configSpecs []vimtypes.VirtualMachineConfigSpec,
-	needHostPlacement, needDatastorePlacement bool) (map[string]Recommendation, error) {
+	needDatastorePlacement bool) (map[string]Recommendation, error) {
 
 	logger := pkglog.FromContextOrDefault(ctx)
 	placementSpec := vimtypes.PlaceVmsXClusterSpec{
 		PlacementType:           string(vimtypes.PlaceVmsXClusterSpecPlacementTypeCreateAndPowerOn),
 		ResourcePools:           resourcePoolsMoRefs,
 		VmPlacementSpecs:        make([]vimtypes.PlaceVmsXClusterSpecVmPlacementSpec, len(configSpecs)),
-		HostRecommRequired:      &needHostPlacement,
 		DatastoreRecommRequired: &needDatastorePlacement,
 	}
 
@@ -262,7 +262,7 @@ func ClusterPlaceVMForCreate(
 		return nil, fmt.Errorf("faults: %v", faultMgs)
 	}
 
-	recommendations := make(map[string]Recommendation)
+	recommendations := make(map[string]Recommendation, len(results.PlacementInfos))
 
 	for _, info := range results.PlacementInfos {
 		if info.Recommendation.Reason != string(vimtypes.RecommendationReasonCodeXClusterPlacement) {

pkg/providers/vsphere/placement/group_placement.go

@@ -6,6 +6,7 @@ package placement
 
 import (
 	"context"
+	"fmt"
 
 	"github.com/vmware/govmomi/find"
 	"github.com/vmware/govmomi/vim25"
@@ -23,30 +24,49 @@ func GroupPlacement(
 	namespace, childRPName string,
 	configSpecs []vimtypes.VirtualMachineConfigSpec) (map[string]Result, error) {
 
-	candidates, resourcePoolToZoneName, err := getPlacementCandidates(ctx, client, vcClient, "", namespace, childRPName)
+	candidates, err := getPlacementCandidates(ctx, client, vcClient, "", namespace, childRPName)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to get placement candidates: %w", err)
 	}
 
 	if len(candidates) == 0 {
 		return nil, ErrNoPlacementCandidates
 	}
 
-	recommendations, err := getGroupPlacementRecommendations(ctx, vcClient, finder, candidates, configSpecs)
+	needDatastorePlacement := pkgcfg.FromContext(ctx).Features.FastDeploy
+	recommendations, err := getGroupPlacementRecommendations(
+		ctx,
+		vcClient,
+		finder,
+		candidates,
+		configSpecs,
+		needDatastorePlacement)
 	if err != nil {
 		return nil, err
 	}
 
-	results := map[string]Result{}
+	resourcePoolToZoneName := make(map[string]string, len(candidates))
+	for zoneName, rpMoIDs := range candidates {
+		for _, rpMoID := range rpMoIDs {
+			resourcePoolToZoneName[rpMoID] = zoneName
+		}
+	}
+
+	results := make(map[string]Result, len(recommendations))
 	for vmName, recommendation := range recommendations {
-		if pkgcfg.FromContext(ctx).Features.FastDeploy {
+		if needDatastorePlacement {
 			// Get the name and type of the datastores.
 			if err := getDatastoreProperties(ctx, vcClient, &recommendation); err != nil {
 				return nil, err
 			}
 		}
 
-		zoneName := resourcePoolToZoneName[recommendation.PoolMoRef.Value]
+		zoneName, ok := resourcePoolToZoneName[recommendation.PoolMoRef.Value]
+		if !ok {
+			// This should never happen: placement returned a non-candidate RP.
+			return nil, fmt.Errorf("no zone assignment for ResourcePool %s",
+				recommendation.PoolMoRef.Value)
+		}
 
 		result := Result{
 			ZoneName:   zoneName,
@@ -66,7 +86,8 @@ func getGroupPlacementRecommendations(
 	vcClient *vim25.Client,
 	finder *find.Finder,
 	candidates map[string][]string,
-	configSpecs []vimtypes.VirtualMachineConfigSpec) (map[string]Recommendation, error) {
+	configSpecs []vimtypes.VirtualMachineConfigSpec,
+	needDatastorePlacement bool) (map[string]Recommendation, error) {
 
 	var candidateRPMoRefs []vimtypes.ManagedObjectReference
 
@@ -80,12 +101,11 @@ func getGroupPlacementRecommendations(
 		}
 	}
 
-	return ClusterPlaceVMForCreate(
+	return getClusterPlacementRecommendations(
 		ctx,
 		vcClient,
 		finder,
 		candidateRPMoRefs,
 		configSpecs,
-		false,
-		true)
+		needDatastorePlacement)
 }