Validate OpenStack Flavors

Check that a minimum set of requirements are met for flavors to ensure
that installation failures due to insufficient resources don't occur.

pkg/asset/installconfig/openstack/validate.go

@@ -16,16 +16,16 @@ func Validate(ic *types.InstallConfig) error {
 
 	allErrs := field.ErrorList{}
 
-	allErrs = append(allErrs, validation.ValidatePlatform(ic.OpenStack, ic.Networking, ci)...)
+	allErrs = append(allErrs, validation.ValidatePlatform(ic.Platform.OpenStack, ic.Networking, ci)...)
 	if ic.ControlPlane.Platform.OpenStack != nil {
-		allErrs = append(allErrs, validation.ValidateMachinePool(ic.ControlPlane.Platform.OpenStack, ci, field.NewPath("controlPlane", "platform", "openstack"))...)
+		allErrs = append(allErrs, validation.ValidateMachinePool(ic.ControlPlane.Platform.OpenStack, ci, true, field.NewPath("controlPlane", "platform", "openstack"))...)
 	}
 	for idx, compute := range ic.Compute {
 		fldPath := field.NewPath("compute").Index(idx)
 		if compute.Platform.OpenStack != nil {
 			allErrs = append(
 				allErrs,
-				validation.ValidateMachinePool(compute.Platform.OpenStack, ci, fldPath.Child("platform", "openstack"))...)
+				validation.ValidateMachinePool(compute.Platform.OpenStack, ci, false, fldPath.Child("platform", "openstack"))...)
 		}
 	}
 

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

@@ -16,7 +16,7 @@ import (
 // CloudInfo caches data fetched from the user's openstack cloud
 type CloudInfo struct {
 	ExternalNetwork *networks.Network
-	PlatformFlavor  *flavors.Flavor
+	Flavors         map[string]*flavors.Flavor
 	MachinesSubnet  *subnets.Subnet
 
 	clients *clients
@@ -32,6 +32,7 @@ func GetCloudInfo(ic *types.InstallConfig) (*CloudInfo, error) {
 	var err error
 	ci := CloudInfo{
 		clients: &clients{},
+		Flavors: map[string]*flavors.Flavor{},
 	}
 
 	opts := &clientconfig.ClientOpts{Cloud: ic.OpenStack.Cloud}
@@ -62,11 +63,35 @@ func (ci *CloudInfo) collectInfo(ic *types.InstallConfig) error {
 		return err
 	}
 
-	ci.PlatformFlavor, err = ci.getFlavor(ic.OpenStack.FlavorName)
+	ci.Flavors[ic.OpenStack.FlavorName], err = ci.getFlavor(ic.OpenStack.FlavorName)
 	if err != nil {
 		return err
 	}
 
+	if ic.ControlPlane != nil && ic.ControlPlane.Platform.OpenStack != nil {
+		crtlPlaneFlavor := ic.ControlPlane.Platform.OpenStack.FlavorName
+		if crtlPlaneFlavor != "" {
+			ci.Flavors[crtlPlaneFlavor], err = ci.getFlavor(crtlPlaneFlavor)
+			if err != nil {
+				return err
+			}
+		}
+	}
+
+	for _, machine := range ic.Compute {
+		if machine.Platform.OpenStack != nil {
+			flavorName := machine.Platform.OpenStack.FlavorName
+			if flavorName != "" {
+				if _, seen := ci.Flavors[flavorName]; !seen {
+					ci.Flavors[flavorName], err = ci.getFlavor(flavorName)
+					if err != nil {
+						return err
+					}
+				}
+			}
+		}
+	}
+
 	ci.MachinesSubnet, err = ci.getSubnet(ic.OpenStack.MachinesSubnet)
 	if err != nil {
 		return err

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

@@ -1,14 +1,35 @@
 package validation
 
 import (
+	"fmt"
+
 	"k8s.io/apimachinery/pkg/util/validation/field"
 
 	guuid "github.com/google/uuid"
 	"github.com/openshift/installer/pkg/types/openstack"
+
+	"github.com/gophercloud/gophercloud/openstack/compute/v2/flavors"
+)
+
+type flavorRequirements struct {
+	RAM, VCPUs, DISK int
+}
+
+var (
+	ctrlPlaneFlavorMinimums = flavorRequirements{
+		RAM:   16,
+		VCPUs: 4,
+		DISK:  25,
+	}
+	computeFlavorMinimums = flavorRequirements{
+		RAM:   8,
+		VCPUs: 2,
+		DISK:  25,
+	}
 )
 
 // ValidateMachinePool checks that the specified machine pool is valid.
-func ValidateMachinePool(p *openstack.MachinePool, ci *CloudInfo, fldPath *field.Path) field.ErrorList {
+func ValidateMachinePool(p *openstack.MachinePool, ci *CloudInfo, controlPlane bool, fldPath *field.Path) field.ErrorList {
 	allErrs := field.ErrorList{}
 
 	// Validate Root Volumes
@@ -21,6 +42,14 @@ func ValidateMachinePool(p *openstack.MachinePool, ci *CloudInfo, fldPath *field
 		}
 	}
 
+	if p.FlavorName != "" {
+		if controlPlane {
+			allErrs = append(allErrs, validateMpoolFlavor(ci.Flavors[p.FlavorName], p.FlavorName, ctrlPlaneFlavorMinimums, fldPath)...)
+		} else {
+			allErrs = append(allErrs, validateMpoolFlavor(ci.Flavors[p.FlavorName], p.FlavorName, computeFlavorMinimums, fldPath)...)
+		}
+	}
+
 	allErrs = append(allErrs, validateUUIDV4s(p.AdditionalNetworkIDs, fldPath.Child("additionalNetworkIDs"))...)
 	allErrs = append(allErrs, validateUUIDV4s(p.AdditionalSecurityGroupIDs, fldPath.Child("additionalSecurityGroupIDs"))...)
 
@@ -53,3 +82,34 @@ func validUUIDv4(s string) bool {
 
 	return true
 }
+
+func validateMpoolFlavor(flavor *flavors.Flavor, name string, req flavorRequirements, fldPath *field.Path) field.ErrorList {
+	if flavor == nil {
+		return field.ErrorList{field.NotFound(fldPath.Child("flavorName"), name)}
+	}
+
+	errs := []string{}
+	if flavor.RAM < req.RAM {
+		errs = append(errs, fmt.Sprintf("Must have minimum of %d GB RAM, had %d GB", req.RAM, flavor.RAM))
+	}
+	if flavor.VCPUs < req.VCPUs {
+		errs = append(errs, fmt.Sprintf("Must have minimum of %d VCPUs, had %d", req.VCPUs, flavor.VCPUs))
+	}
+	if flavor.Disk < req.DISK {
+		errs = append(errs, fmt.Sprintf("Must have minimum of %d GB Disk, had %d GB", req.DISK, flavor.Disk))
+	}
+
+	if len(errs) == 0 {
+		return field.ErrorList{}
+	}
+
+	errString := "Flavor did not meet the following minimum requirements: "
+	for i, err := range errs {
+		errString = errString + err
+		if i != len(errs)-1 {
+			errString = errString + "; "
+		}
+	}
+
+	return field.ErrorList{field.Invalid(fldPath.Child("flavorName"), flavor.Name, errString)}
+}

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

@@ -11,44 +11,136 @@ import (
 )
 
 var (
-	validFlavor = "valid-flavor"
+	validCtrlPlaneFlavor = "valid-control-plane-flavor"
+	validComputeFlavor   = "valid-compute-flavor"
+
+	notExistFlavor = "non-existant-flavor"
+
+	invalidComputeFlavor   = "invalid-compute-flavor"
+	invalidCtrlPlaneFlavor = "invalid-control-plane-flavor"
 )
 
 func validMachinePool() *openstack.MachinePool {
 	return &openstack.MachinePool{
-		FlavorName: validFlavor,
+		FlavorName: validCtrlPlaneFlavor,
 	}
 }
 
-func validCloudInfo() *CloudInfo {
+func validMpoolCloudInfo() *CloudInfo {
 	return &CloudInfo{
-		PlatformFlavor: &flavors.Flavor{
-			Name: validFlavor,
+		Flavors: map[string]*flavors.Flavor{
+			validCtrlPlaneFlavor: {
+				Name:  validCtrlPlaneFlavor,
+				RAM:   16,
+				Disk:  25,
+				VCPUs: 4,
+			},
+			validComputeFlavor: {
+				Name:  validComputeFlavor,
+				RAM:   8,
+				Disk:  25,
+				VCPUs: 2,
+			},
+			invalidCtrlPlaneFlavor: {
+				Name:  invalidCtrlPlaneFlavor,
+				RAM:   8, // too low
+				Disk:  25,
+				VCPUs: 2, // too low
+			},
+			invalidComputeFlavor: {
+				Name:  invalidComputeFlavor,
+				RAM:   8,
+				Disk:  10, // too low
+				VCPUs: 2,
+			},
 		},
 	}
 }
 
 func TestOpenStackMachinepoolValidation(t *testing.T) {
 	cases := []struct {
 		name           string
+		controlPlane   bool // only matters for flavor
 		mpool          *openstack.MachinePool
 		cloudInfo      *CloudInfo
 		expectedError  bool
-		expectedErrMsg string
+		expectedErrMsg string // NOTE: this is a REGEXP
 	}{
 		{
-			name:           "valid base case",
+			name:           "valid control plane",
+			controlPlane:   true,
 			mpool:          validMachinePool(),
-			cloudInfo:      validCloudInfo(),
+			cloudInfo:      validMpoolCloudInfo(),
 			expectedError:  false,
 			expectedErrMsg: "",
 		},
+		{
+			name:           "valid compute",
+			controlPlane:   false,
+			mpool:          validMachinePool(),
+			cloudInfo:      validMpoolCloudInfo(),
+			expectedError:  false,
+			expectedErrMsg: "",
+		},
+		{
+			name:         "not found control plane flavorName",
+			controlPlane: true,
+			mpool: func() *openstack.MachinePool {
+				mp := validMachinePool()
+				mp.FlavorName = notExistFlavor
+				return mp
+			}(),
+			cloudInfo:      validMpoolCloudInfo(),
+			expectedError:  true,
+			expectedErrMsg: "controlPlane.platform.openstack.flavorName: Not found: \"non-existant-flavor\"",
+		},
+		{
+			name: "not found control plane flavorName",
+			mpool: func() *openstack.MachinePool {
+				mp := validMachinePool()
+				mp.FlavorName = notExistFlavor
+				return mp
+			}(),
+			cloudInfo:      validMpoolCloudInfo(),
+			expectedError:  true,
+			expectedErrMsg: "compute.0..platform.openstack.flavorName: Not found: \"non-existant-flavor\"",
+		},
+		{
+			name:         "invalid control plane flavorName",
+			controlPlane: true,
+			mpool: func() *openstack.MachinePool {
+				mp := validMachinePool()
+				mp.FlavorName = invalidCtrlPlaneFlavor
+				return mp
+			}(),
+			cloudInfo:      validMpoolCloudInfo(),
+			expectedError:  true,
+			expectedErrMsg: "controlPlane.platform.openstack.flavorName: Invalid value: \"invalid-control-plane-flavor\": Flavor did not meet the following minimum requirements: Must have minimum of 16 GB RAM, had 8 GB; Must have minimum of 4 VCPUs, had 2",
+		},
+		{
+			name:         "invalid compute flavorName",
+			controlPlane: false,
+			mpool: func() *openstack.MachinePool {
+				mp := validMachinePool()
+				mp.FlavorName = invalidComputeFlavor
+				return mp
+			}(),
+			cloudInfo:      validMpoolCloudInfo(),
+			expectedError:  true,
+			expectedErrMsg: "compute.0..platform.openstack.flavorName: Invalid value: \"invalid-compute-flavor\": Flavor did not meet the following minimum requirements: Must have minimum of 25 GB Disk, had 10 GB",
+		},
 	}
 
 	for _, tc := range cases {
 		t.Run(tc.name, func(t *testing.T) {
+			var fieldPath *field.Path
+			if tc.controlPlane {
+				fieldPath = field.NewPath("controlPlane", "platform", "openstack")
+			} else {
+				fieldPath = field.NewPath("compute").Index(0).Child("platform", "openstack")
+			}
 
-			aggregatedErrors := ValidateMachinePool(tc.mpool, tc.cloudInfo, field.NewPath("controlPlane", "platform", "openstack")).ToAggregate()
+			aggregatedErrors := ValidateMachinePool(tc.mpool, tc.cloudInfo, tc.controlPlane, fieldPath).ToAggregate()
 			if tc.expectedError {
 				assert.Regexp(t, tc.expectedErrMsg, aggregatedErrors)
 			} else {

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

@@ -25,7 +25,7 @@ func ValidatePlatform(p *openstack.Platform, n *types.Networking, ci *CloudInfo)
 	allErrs = append(allErrs, validatePlatformFlavor(p, ci, fldPath)...)
 
 	if p.DefaultMachinePlatform != nil {
-		allErrs = append(allErrs, ValidateMachinePool(p.DefaultMachinePlatform, ci, fldPath.Child("defaultMachinePlatform"))...)
+		allErrs = append(allErrs, ValidateMachinePool(p.DefaultMachinePlatform, ci, true, fldPath.Child("defaultMachinePlatform"))...)
 	}
 
 	return allErrs
@@ -60,10 +60,38 @@ func validateExternalNetwork(p *openstack.Platform, ci *CloudInfo, fldPath *fiel
 	return allErrs
 }
 
-// validatePlatformFlavor validates the platform flavor and returns a list of all validatoin errors
+// validatePlatformFlavor validates the platform flavor and returns a list of all validation errors
 func validatePlatformFlavor(p *openstack.Platform, ci *CloudInfo, fldPath *field.Path) (allErrs field.ErrorList) {
-	if ci.PlatformFlavor == nil {
+	flavor := ci.Flavors[p.FlavorName]
+	if flavor == nil {
 		allErrs = append(allErrs, field.NotFound(fldPath.Child("computeFlavor"), p.FlavorName))
+		return allErrs
 	}
+
+	errs := []string{}
+	req := ctrlPlaneFlavorMinimums
+	if flavor.RAM < req.RAM {
+		errs = append(errs, fmt.Sprintf("Must have minimum of %d GB RAM, had %d GB", req.RAM, flavor.RAM))
+	}
+	if flavor.VCPUs < req.VCPUs {
+		errs = append(errs, fmt.Sprintf("Must have minimum of %d VCPUs, had %d", req.VCPUs, flavor.VCPUs))
+	}
+	if flavor.Disk < req.DISK {
+		errs = append(errs, fmt.Sprintf("Must have minimum of %d GB Disk, had %d GB", req.DISK, flavor.Disk))
+	}
+
+	if len(errs) == 0 {
+		return field.ErrorList{}
+	}
+
+	errString := "Flavor did not meet the following minimum requirements: "
+	for i, err := range errs {
+		errString = errString + err
+		if i != len(errs)-1 {
+			errString = errString + "; "
+		}
+	}
+
+	allErrs = append(allErrs, field.Invalid(fldPath.Child("flavorName"), flavor.Name, errString))
 	return allErrs
 }