Merge pull request #4508 from shiftstack/bz/1899161

Bug 1899161: openstack: consider volumes for storage requirements checks

docs/user/openstack/README.md

@@ -73,19 +73,19 @@ openstack quota set --secgroups 8 --secgroup-rules 100 <project>`
 
 ### Master Nodes
 
-The default deployment stands up 3 master nodes, which is the minimum amount required for a cluster. For each master node you stand up, you will need 1 instance, and 1 port available in your quota. They should be assigned a flavor with at least 16 GB RAM, 4 vCPUs, and 25 GB Disk. It is theoretically possible to run with a smaller flavor, but be aware that if it takes too long to stand up services, or certain essential services crash, the installer could time out, leading to a failed install.
+The default deployment stands up 3 master nodes, which is the minimum amount required for a cluster. For each master node you stand up, you will need 1 instance, and 1 port available in your quota. They should be assigned a flavor with at least 16 GB RAM, 4 vCPUs, and 25 GB Disk (or Root Volume). It is theoretically possible to run with a smaller flavor, but be aware that if it takes too long to stand up services, or certain essential services crash, the installer could time out, leading to a failed install.
 
 The Master Nodes are placed in a single Server Group with "soft anti-affinity" policy; the machines will therefore be creted on separate hosts when possible.
 
 ### Worker Nodes
 
-The default deployment stands up 3 worker nodes. Worker nodes host the applications you run on OpenShift. The flavor assigned to the worker nodes should have at least 2 vCPUs, 8 GB RAM and 25 GB Disk. However, if you are experiencing `Out Of Memory` issues, or your installs are timing out, try increasing the size of your flavor to match the master nodes: 4 vCPUs and 16 GB RAM.
+The default deployment stands up 3 worker nodes. Worker nodes host the applications you run on OpenShift. The flavor assigned to the worker nodes should have at least 2 vCPUs, 8 GB RAM and 25 GB Disk (or Root Volume). However, if you are experiencing `Out Of Memory` issues, or your installs are timing out, try increasing the size of your flavor to match the master nodes: 4 vCPUs and 16 GB RAM.
 
 See the [OpenShift documentation](https://docs.openshift.com/container-platform/4.4/architecture/control-plane.html#defining-workers_control-plane) for more information on the worker nodes.
 
 ### Bootstrap Node
 
-The bootstrap node is a temporary node that is responsible for standing up the control plane on the masters. Only one bootstrap node will be stood up and it will be deprovisioned once the production control plane is ready. To do so, you need 1 instance, and 1 port. We recommend a flavor with a minimum of 16 GB RAM, 4 vCPUs, and 25 GB Disk.
+The bootstrap node is a temporary node that is responsible for standing up the control plane on the masters. Only one bootstrap node will be stood up and it will be deprovisioned once the production control plane is ready. To do so, you need 1 instance, and 1 port. We recommend a flavor with a minimum of 16 GB RAM, 4 vCPUs, and 25 GB Disk (or Root Volume).
 
 ### Image Registry Requirements
 

docs/user/openstack/customization.md

@@ -40,7 +40,7 @@ Beyond the [platform-agnostic `install-config.yaml` properties](../customization
 * `additionalSecurityGroupIDs` (optional list of strings): IDs of additional security groups for machines.
 * `type` (optional string): The OpenStack flavor name for machines in the pool.
 * `rootVolume` (optional object): Defines the root volume for instances in the machine pool. The instances use ephemeral disks if not set.
-  * `size` (required integer): Size of the root volume in GB.
+  * `size` (required integer): Size of the root volume in GB. Must be set to at least 25.
   * `type` (required string): The volume pool to create the volume from.
 * `zones` (optional list of strings): The names of the availability zones you want to install your nodes on. If unset, the installer will use your default compute zone.
 

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

@@ -14,37 +14,45 @@ type flavorRequirements struct {
 	RAM, VCPUs, Disk int
 }
 
+const (
+	minimumStorage = 25
+)
+
 var (
 	ctrlPlaneFlavorMinimums = flavorRequirements{
 		RAM:   16,
 		VCPUs: 4,
-		Disk:  25,
+		Disk:  minimumStorage,
 	}
 	computeFlavorMinimums = flavorRequirements{
 		RAM:   8,
 		VCPUs: 2,
-		Disk:  25,
+		Disk:  minimumStorage,
 	}
 )
 
 // ValidateMachinePool checks that the specified machine pool is valid.
 func ValidateMachinePool(p *openstack.MachinePool, ci *CloudInfo, controlPlane bool, fldPath *field.Path) field.ErrorList {
 	allErrs := field.ErrorList{}
 
+	var checkStorageFlavor bool
 	// Validate Root Volumes
 	if p.RootVolume != nil {
 		if p.RootVolume.Type == "" {
 			allErrs = append(allErrs, field.Invalid(fldPath.Child("rootVolume").Child("type"), p.RootVolume.Type, "Volume type must be specified to use root volumes"))
 		}
-		if p.RootVolume.Size <= 0 {
-			allErrs = append(allErrs, field.Invalid(fldPath.Child("rootVolume").Child("size"), p.RootVolume.Size, "Volume size must be greater than zero to use root volumes"))
+		if p.RootVolume.Size < minimumStorage {
+			allErrs = append(allErrs, field.Invalid(fldPath.Child("rootVolume").Child("size"), p.RootVolume.Size, fmt.Sprintf("Volume size must be greater than %d to use root volumes, had %d", minimumStorage, p.RootVolume.Size)))
 		}
+	} else {
+		// Not using root volume, so must check flavor
+		checkStorageFlavor = true
 	}
 
 	if controlPlane {
-		allErrs = append(allErrs, validateFlavor(p.FlavorName, ci, ctrlPlaneFlavorMinimums, fldPath.Child("type"))...)
+		allErrs = append(allErrs, validateFlavor(p.FlavorName, ci, ctrlPlaneFlavorMinimums, fldPath.Child("type"), checkStorageFlavor)...)
 	} else {
-		allErrs = append(allErrs, validateFlavor(p.FlavorName, ci, computeFlavorMinimums, fldPath.Child("type"))...)
+		allErrs = append(allErrs, validateFlavor(p.FlavorName, ci, computeFlavorMinimums, fldPath.Child("type"), checkStorageFlavor)...)
 	}
 
 	allErrs = append(allErrs, validateZones(p.Zones, ci.Zones, fldPath.Child("zones"))...)
@@ -100,7 +108,7 @@ func validUUIDv4(s string) bool {
 
 // validate flavor checks to make sure that a given flavor exists and meets the minimum requrement to run a cluster
 // this function does not validate proper install config usage
-func validateFlavor(flavorName string, ci *CloudInfo, req flavorRequirements, fldPath *field.Path) field.ErrorList {
+func validateFlavor(flavorName string, ci *CloudInfo, req flavorRequirements, fldPath *field.Path, storage bool) field.ErrorList {
 	if flavorName == "" {
 		return nil
 	}
@@ -123,7 +131,7 @@ func validateFlavor(flavorName string, ci *CloudInfo, req flavorRequirements, fl
 	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 {
+	if flavor.Disk < req.Disk && storage {
 		errs = append(errs, fmt.Sprintf("Must have minimum of %d GB Disk, had %d GB", req.Disk, flavor.Disk))
 	}
 

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

@@ -23,6 +23,10 @@ const (
 	invalidCtrlPlaneFlavor = "invalid-control-plane-flavor"
 
 	baremetalFlavor = "baremetal-flavor"
+
+	volumeType      = "performance"
+	volumeSmallSize = 10
+	volumeLargeSize = 25
 )
 
 func validMachinePool() *openstack.MachinePool {
@@ -32,6 +36,28 @@ func validMachinePool() *openstack.MachinePool {
 	}
 }
 
+func invalidMachinePoolSmallVolume() *openstack.MachinePool {
+	return &openstack.MachinePool{
+		FlavorName: validCtrlPlaneFlavor,
+		Zones:      []string{""},
+		RootVolume: &openstack.RootVolume{
+			Type: volumeType,
+			Size: volumeSmallSize,
+		},
+	}
+}
+
+func validMachinePoolLargeVolume() *openstack.MachinePool {
+	return &openstack.MachinePool{
+		FlavorName: validCtrlPlaneFlavor,
+		Zones:      []string{""},
+		RootVolume: &openstack.RootVolume{
+			Type: volumeType,
+			Size: volumeLargeSize,
+		},
+	}
+}
+
 func validMpoolCloudInfo() *CloudInfo {
 	return &CloudInfo{
 		Flavors: map[string]Flavor{
@@ -197,6 +223,30 @@ func TestOpenStackMachinepoolValidation(t *testing.T) {
 			expectedError:  false,
 			expectedErrMsg: "",
 		},
+		{
+			name:         "volume too small",
+			controlPlane: false,
+			mpool: func() *openstack.MachinePool {
+				mp := invalidMachinePoolSmallVolume()
+				mp.FlavorName = invalidCtrlPlaneFlavor
+				return mp
+			}(),
+			cloudInfo:      validMpoolCloudInfo(),
+			expectedError:  true,
+			expectedErrMsg: "Volume size must be greater than 25 to use root volumes, had 10",
+		},
+		{
+			name:         "volume big enough",
+			controlPlane: false,
+			mpool: func() *openstack.MachinePool {
+				mp := validMachinePoolLargeVolume()
+				mp.FlavorName = invalidCtrlPlaneFlavor
+				return mp
+			}(),
+			cloudInfo:      validMpoolCloudInfo(),
+			expectedError:  false,
+			expectedErrMsg: "",
+		},
 	}
 
 	for _, tc := range cases {