Fixes machinepools to add the correct number of machines to pool. Als…

…o updates code to handle empty zones more correctly.

data/data/install.openshift.io_installconfigs.yaml

@@ -234,8 +234,10 @@ spec:
                             eg. m1.large
                           type: string
                         zones:
-                          description: Zones contains a list of availabilty zones
-                            that instances can be deployed on
+                          description: Zones is the list of availability zones where
+                            the instances should be deployed. If no zones are provided,
+                            all instances will be deployed on OpenStack Nova default
+                            availability zone
                           items:
                             type: string
                           type: array
@@ -527,8 +529,10 @@ spec:
                           eg. m1.large
                         type: string
                       zones:
-                        description: Zones contains a list of availabilty zones that
-                          instances can be deployed on
+                        description: Zones is the list of availability zones where
+                          the instances should be deployed. If no zones are provided,
+                          all instances will be deployed on OpenStack Nova default
+                          availability zone
                         items:
                           type: string
                         type: array
@@ -1316,8 +1320,10 @@ spec:
                           eg. m1.large
                         type: string
                       zones:
-                        description: Zones contains a list of availabilty zones that
-                          instances can be deployed on
+                        description: Zones is the list of availability zones where
+                          the instances should be deployed. If no zones are provided,
+                          all instances will be deployed on OpenStack Nova default
+                          availability zone
                         items:
                           type: string
                         type: array

pkg/asset/installconfig/openstack/validation/cloudinfo.go

@@ -199,12 +199,12 @@ func (ci *CloudInfo) getZones() ([]string, error) {
 	zones := []string{}
 	allPages, err := availabilityzones.List(ci.clients.computeClient).AllPages()
 	if err != nil {
-		return []string{}, err
+		return nil, err
 	}
 
 	availabilityZoneInfo, err := availabilityzones.ExtractAvailabilityZones(allPages)
 	if err != nil {
-		return []string{}, err
+		return nil, err
 	}
 
 	for _, zoneInfo := range availabilityZoneInfo {
@@ -214,7 +214,7 @@ func (ci *CloudInfo) getZones() ([]string, error) {
 	}
 
 	if len(zones) == 0 {
-		return []string{""}, nil
+		return nil, errors.New("could not find an available compute availability zone")
 	}
 
 	return zones, nil

pkg/asset/installconfig/openstack/validation/machinepool.go

@@ -3,6 +3,7 @@ package validation
 import (
 	"fmt"
 
+	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 
 	guuid "github.com/google/uuid"
@@ -57,25 +58,17 @@ func ValidateMachinePool(p *openstack.MachinePool, ci *CloudInfo, controlPlane b
 	return allErrs
 }
 
-func validateZones(input []string, actual []string, fldPath *field.Path) field.ErrorList {
-	allErrs := field.ErrorList{}
-
-	// Base Case: No user input
-	if len(input) == 1 {
-		if input[0] == "" {
-			return allErrs
-		}
+func validateZones(input []string, available []string, fldPath *field.Path) field.ErrorList {
+	// check if machinepool default
+	if len(input) == 1 && input[0] == "" {
+		return field.ErrorList{}
 	}
-	if len(input) >= 1 {
-		actualZones := map[string]bool{}
-		for _, zone := range actual {
-			actualZones[zone] = true
-		}
 
-		for _, zoneInput := range input {
-			if !actualZones[zoneInput] {
-				allErrs = append(allErrs, field.Invalid(fldPath.Child("zone"), zoneInput, "Zone either does not exist in this cloud, or is not available"))
-			}
+	allErrs := field.ErrorList{}
+	availableZones := sets.NewString(available...)
+	for idx, zone := range input {
+		if !availableZones.Has(zone) {
+			allErrs = append(allErrs, field.Invalid(fldPath.Child("zone").Index(idx), zone, "Zone either does not exist in this cloud, or is not available"))
 		}
 	}
 

pkg/asset/installconfig/openstack/validation/machinepool_test.go

@@ -26,6 +26,7 @@ var (
 func validMachinePool() *openstack.MachinePool {
 	return &openstack.MachinePool{
 		FlavorName: validCtrlPlaneFlavor,
+		Zones:      []string{""},
 	}
 }
 

pkg/asset/machines/openstack/machinesets.go

@@ -33,7 +33,6 @@ func MachineSets(clusterID string, config *types.InstallConfig, pool *types.Mach
 	}
 
 	numOfAZs := int32(len(mpool.Zones))
-	// TODO(flaper87): Add support for availability zones
 	var machinesets []*clusterapi.MachineSet
 
 	for idx, az := range mpool.Zones {
@@ -67,7 +66,7 @@ func MachineSets(clusterID string, config *types.InstallConfig, pool *types.Mach
 				},
 			},
 			Spec: clusterapi.MachineSetSpec{
-				Replicas: &total,
+				Replicas: &replicas,
 				Selector: metav1.LabelSelector{
 					MatchLabels: map[string]string{
 						"machine.openshift.io/cluster-api-machineset": name,

pkg/types/openstack/machinepool.go

@@ -23,8 +23,8 @@ type MachinePool struct {
 	// +optional
 	AdditionalSecurityGroupIDs []string `json:"additionalSecurityGroupIDs,omitempty"`
 
-	// Zones contains a list of availabilty zones that instances can be deployed on
-	//
+	// Zones is the list of availability zones where the instances should be deployed.
+	// If no zones are provided, all instances will be deployed on OpenStack Nova default availability zone
 	// +optional
 	Zones []string `json:"zones,omitempty"`
 }