Merge pull request #4321 from Fedosin/deprecate_computeflavor

Deprecate computeFlavor in OpenStack platform

data/data/install.openshift.io_installconfigs.yaml

@@ -1178,7 +1178,7 @@ spec:
                     description: 'ClusterOSImageProperties is a list of properties to be added to the metadata of the uploaded Glance ClusterOSImage. Default: the default is to not set any properties.'
                     type: object
                   computeFlavor:
-                    description: FlavorName is the name of the compute flavor to use for instances in this cluster.
+                    description: 'DeprecatedFlavorName is the name of the flavor to use for instances in this cluster. Deprecated: use FlavorName in DefaultMachinePlatform to define default flavor.'
                     type: string
                   defaultMachinePlatform:
                     description: DefaultMachinePlatform is the default configuration used when installing on OpenStack for machine pools which do not define their own platform configuration.
@@ -1248,7 +1248,6 @@ spec:
                     type: string
                 required:
                 - cloud
-                - computeFlavor
                 type: object
               ovirt:
                 description: Ovirt is the configuration used when installing on oVirt.

docs/user/openstack/customization.md

@@ -20,10 +20,9 @@ Beyond the [platform-agnostic `install-config.yaml` properties](../customization
 ## Cluster-scoped properties
 
 * `cloud` (required string): The name of the OpenStack cloud to use from `clouds.yaml`.
-* `computeFlavor` (required string): The OpenStack flavor to use for compute and control-plane machines.
-    This is currently required, but has lower precedence than [the `type` property](#machine-pools) on [the `compute` and `controlPlane` machine-pools](../customization.md#platform-customization).
+* `computeFlavor` (deprecated string): The OpenStack flavor to use for compute and control-plane machines.
 * `externalDNS` (optional list of strings): The IP addresses of DNS servers to be used for the DNS resolution of all instances in the cluster
-* `externalNetwork` (optional string): Name of external network the installer will use to provide access to the cluster. If defined, a floating ip from this network will be created and associated with the bootstrap node to facilitate debugging and connection to the bootstrap node during installation. The `apiFloatingIP` property is a floating ip address selected from this network.
+* `externalNetwork` (optional string): Name of external network the installer will use to provide access to the cluster. If defined, a floating IP from this network will be created and associated with the bootstrap node to facilitate debugging and connection to the bootstrap node during installation. The `apiFloatingIP` property is a floating IP address selected from this network.
 * `apiFloatingIP` (optional string): Address of existing Floating IP from externalNetwork the installer will associate with the OpenShift API. This property is only valid if externalNetwork is defined. If externalNetwork is not defined, the installer will throw an error.
 * `ingressFloatingIP` (optional string): Address of an existing Floating IP from externalNetwork the installer will associate with the ingress port. This property is only valid if externalNetwork is defined. If externalNetwork is not defined, the installer will throw an error.
 * `octaviaSupport` (deprecated string): Whether OpenStack supports Octavia (`1` for true or `0` for false)
@@ -34,6 +33,7 @@ Beyond the [platform-agnostic `install-config.yaml` properties](../customization
 * `apiVIP` (optional string): An IP address on the machineNetwork that will be assigned to the API VIP. Be aware that the `10` and `11` of the machineNetwork will be taken by neutron dhcp by default, and wont be available.
 * `ingressVIP` (optional string): An IP address on the machineNetwork that will be assigned to the ingress VIP. Be aware that the `10` and `11` of the machineNetwork will be taken by neutron dhcp by default, and wont be available.
 * `machinesSubnet` (optional string): the UUID of an OpenStack subnet to install the nodes of the cluster onto. For more information on how to install with a custom subnet, see the [custom subnets](#custom-subnets) section of the docs.
+* `defaultMachinePlatform` (optional object): Default [OpenStack-specific machine pool properties](#machine-pools) which apply to [machine pools](../customization.md#machine-pools) that do not define their own OpenStack-specific properties.
 
 ## Machine pools
 
@@ -68,7 +68,8 @@ platform:
   openstack:
     apiFloatingIP: 128.0.0.1
     cloud: mycloud
-    computeFlavor: m1.s2.xlarge
+    defaultMachinePlatform:
+      type: m1.s2.xlarge
     externalNetwork: external
     externalDNS:
       - "8.8.8.8"
@@ -102,7 +103,8 @@ platform:
   openstack:
     apiFloatingIP: 128.0.0.1
     cloud: mycloud
-    computeFlavor: m1.s2.xlarge
+    defaultMachinePlatform:
+      type: m1.s2.xlarge
     externalNetwork: external
 pullSecret: '{"auths": ...}'
 sshKey: ssh-ed25519 AAAA...
@@ -148,7 +150,7 @@ platform:
 
 ## Custom Subnets
 
-Before running the installer with a custom `machinesSubnet`, there are a few things you should check. First, make sure that you have a network and subnet available. Note that the installer will create ports on this network, and some ports will be given a fixed IP addresses in the subnet's CIDR. Make sure that the credentials that you give the installer are authorized to do this! The installer will also apply a tag to the subnet. Lastly, the installer will not create a router, so make sure to do this if you need a router to attach floating ip addresses to the network you want to install the nodes onto. Note that for now, this feature does not fully support provider networks.
+Before running the installer with a custom `machinesSubnet`, there are a few things you should check. First, make sure that you have a network and subnet available. Note that the installer will create ports on this network, and some ports will be given a fixed IP addresses in the subnet's CIDR. Make sure that the credentials that you give the installer are authorized to do this! The installer will also apply a tag to the subnet. Lastly, the installer will not create a router, so make sure to do this if you need a router to attach floating IP addresses to the network you want to install the nodes onto. Note that for now, this feature does not fully support provider networks.
 
 Once that is ironed out, you can pass the ID of the subnet to the install config with the `machinesSubnet` option. This subnet will be tagged as the primary subnet for the cluster. The nodes, and VIP port will be created on this subnet. This does add some complexity to the install process that needs to be accounted for. First of all, the subnet must have DHCP enabled. Also, the CIDR of the custom subnet must match the cidr of `networks.machineNetwork`. Be aware that the API and Ingress VIPs by default will try to take the .5 and .7 of your network CIDR. To avoid other services taking the ports assigned to the api and ingress VIPs, it is recommended that users set the `apiVIP` and `ingressVIP` options in the install config to addresses that are outside of the DHCP allocation pool. Lastly, note that setting `externalDNS` while setting `machinesSubnet` is invalid usage, since the installer will not modify your subnet. If you want to add a DNS to your cluster while using a custom subnet, add it to the subnet in openstack [like this](https://docs.openstack.org/neutron/rocky/admin/config-dns-res.html).
 

pkg/asset/installconfig/openstack/openstack.go

@@ -143,6 +143,8 @@ func Platform() (*openstack.Platform, error) {
 		APIFloatingIP:   apiFloatingIP,
 		Cloud:           cloud,
 		ExternalNetwork: extNet,
-		FlavorName:      flavor,
+		DefaultMachinePlatform: &openstack.MachinePool{
+			FlavorName: flavor,
+		},
 	}, nil
 }

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

@@ -118,10 +118,14 @@ func (ci *CloudInfo) collectInfo(ic *types.InstallConfig, opts *clientconfig.Cli
 	}
 
 	// Get flavor info
-	if flavorName := ic.OpenStack.FlavorName; flavorName != "" {
-		ci.Flavors[flavorName], err = ci.getFlavor(flavorName)
-		if err != nil {
-			return err
+	if ic.Platform.OpenStack.DefaultMachinePlatform != nil {
+		if flavorName := ic.Platform.OpenStack.DefaultMachinePlatform.FlavorName; flavorName != "" {
+			if _, seen := ci.Flavors[flavorName]; !seen {
+				ci.Flavors[flavorName], err = ci.getFlavor(flavorName)
+				if err != nil {
+					return err
+				}
+			}
 		}
 	}
 

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

@@ -29,6 +29,10 @@ func ValidatePlatform(p *openstack.Platform, n *types.Networking, ci *CloudInfo)
 	// validate custom cluster os image
 	allErrs = append(allErrs, validateClusterOSImage(p, ci, fldPath)...)
 
+	if p.DefaultMachinePlatform != nil {
+		allErrs = append(allErrs, ValidateMachinePool(p.DefaultMachinePlatform, ci, true, fldPath.Child("defaultMachinePlatform"))...)
+	}
+
 	return allErrs
 }
 

pkg/asset/machines/master.go

@@ -235,7 +235,7 @@ func (m *Master) Generate(dependencies asset.Parents) error {
 			return errors.Wrap(err, "failed to create master machine objects")
 		}
 	case openstacktypes.Name:
-		mpool := defaultOpenStackMachinePoolPlatform(ic.Platform.OpenStack.FlavorName)
+		mpool := defaultOpenStackMachinePoolPlatform()
 		mpool.Set(ic.Platform.OpenStack.DefaultMachinePlatform)
 		mpool.Set(pool.Platform.OpenStack)
 		pool.Platform.OpenStack = &mpool

pkg/asset/machines/worker.go

@@ -110,10 +110,9 @@ func defaultGCPMachinePoolPlatform() gcptypes.MachinePool {
 	}
 }
 
-func defaultOpenStackMachinePoolPlatform(flavor string) openstacktypes.MachinePool {
+func defaultOpenStackMachinePoolPlatform() openstacktypes.MachinePool {
 	return openstacktypes.MachinePool{
-		FlavorName: flavor,
-		Zones:      []string{""},
+		Zones: []string{""},
 	}
 }
 
@@ -358,7 +357,7 @@ func (w *Worker) Generate(dependencies asset.Parents) error {
 				machineSets = append(machineSets, set)
 			}
 		case openstacktypes.Name:
-			mpool := defaultOpenStackMachinePoolPlatform(ic.Platform.OpenStack.FlavorName)
+			mpool := defaultOpenStackMachinePoolPlatform()
 			mpool.Set(ic.Platform.OpenStack.DefaultMachinePlatform)
 			mpool.Set(pool.Platform.OpenStack)
 			pool.Platform.OpenStack = &mpool

pkg/types/conversion/installconfig.go

@@ -121,5 +121,19 @@ func convertOpenStack(config *types.InstallConfig) error {
 		}
 	}
 
+	// computeFlavor has been deprecated in favor of type in defaultMachinePlatform.
+	if config.Platform.OpenStack.DeprecatedFlavorName != "" {
+		if config.Platform.OpenStack.DefaultMachinePlatform == nil {
+			config.Platform.OpenStack.DefaultMachinePlatform = &openstack.MachinePool{}
+		}
+
+		if config.Platform.OpenStack.DefaultMachinePlatform.FlavorName != "" && config.Platform.OpenStack.DefaultMachinePlatform.FlavorName != config.Platform.OpenStack.DeprecatedFlavorName {
+			// Return error if both computeFlavor and type of defaultMachinePlatform are specified in the config
+			return field.Forbidden(field.NewPath("platform").Child("openstack").Child("computeFlavor"), "cannot specify computeFlavor and type in defaultMachinePlatform together")
+		}
+
+		config.Platform.OpenStack.DefaultMachinePlatform.FlavorName = config.Platform.OpenStack.DeprecatedFlavorName
+	}
+
 	return nil
 }

pkg/types/conversion/installconfig_test.go

@@ -272,6 +272,49 @@ func TestConvertInstallConfig(t *testing.T) {
 			},
 			expectedError: "provisioningHostIP is deprecated; only clusterProvisioningIP needs to be specified",
 		},
+		{
+			name: "deprecated OpenStack computeFlavor",
+			config: &types.InstallConfig{
+				TypeMeta: metav1.TypeMeta{
+					APIVersion: types.InstallConfigVersion,
+				},
+				Platform: types.Platform{
+					OpenStack: &openstack.Platform{
+						DeprecatedFlavorName: "big-flavor",
+					},
+				},
+			},
+			expected: &types.InstallConfig{
+				TypeMeta: metav1.TypeMeta{
+					APIVersion: types.InstallConfigVersion,
+				},
+				Platform: types.Platform{
+					OpenStack: &openstack.Platform{
+						DeprecatedFlavorName: "big-flavor",
+						DefaultMachinePlatform: &openstack.MachinePool{
+							FlavorName: "big-flavor",
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "deprecated OpenStack computeFlavor with type in defaultMachinePlatform",
+			config: &types.InstallConfig{
+				TypeMeta: metav1.TypeMeta{
+					APIVersion: types.InstallConfigVersion,
+				},
+				Platform: types.Platform{
+					OpenStack: &openstack.Platform{
+						DeprecatedFlavorName: "flavor1",
+						DefaultMachinePlatform: &openstack.MachinePool{
+							FlavorName: "flavor2",
+						},
+					},
+				},
+			},
+			expectedError: "cannot specify computeFlavor and type in defaultMachinePlatform together",
+		},
 	}
 
 	for _, tc := range cases {

pkg/types/openstack/platform.go

@@ -21,8 +21,10 @@ type Platform struct {
 	// +optional
 	ExternalNetwork string `json:"externalNetwork,omitempty"`
 
-	// FlavorName is the name of the compute flavor to use for instances in this cluster.
-	FlavorName string `json:"computeFlavor"`
+	// DeprecatedFlavorName is the name of the flavor to use for instances in this cluster.
+	// Deprecated: use FlavorName in DefaultMachinePlatform to define default flavor.
+	// +optional
+	DeprecatedFlavorName string `json:"computeFlavor,omitempty"`
 
 	// LbFloatingIP is the IP address of an available floating IP in your OpenStack cluster
 	// to associate with the OpenShift load balancer.

pkg/types/openstack/validation/platform_test.go

@@ -15,7 +15,9 @@ func validPlatform() *openstack.Platform {
 	return &openstack.Platform{
 		Cloud:           "test-cloud",
 		ExternalNetwork: "test-network",
-		FlavorName:      "test-flavor",
+		DefaultMachinePlatform: &openstack.MachinePool{
+			FlavorName: "test-flavor",
+		},
 	}
 }
 

pkg/types/validation/installconfig_test.go

@@ -120,7 +120,9 @@ func validOpenStackPlatform() *openstack.Platform {
 	return &openstack.Platform{
 		Cloud:           "test-cloud",
 		ExternalNetwork: "test-network",
-		FlavorName:      "test-flavor",
+		DefaultMachinePlatform: &openstack.MachinePool{
+			FlavorName: "test-flavor",
+		},
 	}
 }