From 7014333a1a99d54447ec5b7c62290e7184f2d0b5 Mon Sep 17 00:00:00 2001 From: Mike Fedosin Date: Wed, 28 Oct 2020 19:08:42 +0100 Subject: [PATCH 1/2] Deprecate computeFlavor in OpenStack platform --- .../install.openshift.io_installconfigs.yaml | 3 +- docs/user/openstack/customization.md | 9 ++-- .../installconfig/openstack/openstack.go | 4 +- .../openstack/validation/cloudinfo.go | 12 ++++-- .../openstack/validation/platform.go | 4 ++ pkg/asset/machines/master.go | 2 +- pkg/asset/machines/worker.go | 7 ++- pkg/types/conversion/installconfig.go | 14 ++++++ pkg/types/conversion/installconfig_test.go | 43 +++++++++++++++++++ pkg/types/openstack/platform.go | 6 ++- .../openstack/validation/platform_test.go | 4 +- pkg/types/validation/installconfig_test.go | 4 +- 12 files changed, 92 insertions(+), 20 deletions(-) diff --git a/data/data/install.openshift.io_installconfigs.yaml b/data/data/install.openshift.io_installconfigs.yaml index 07485caf46..33be5d35e9 100644 --- a/data/data/install.openshift.io_installconfigs.yaml +++ b/data/data/install.openshift.io_installconfigs.yaml @@ -1173,7 +1173,7 @@ spec: description: ClusterOSImage is either a URL with `http(s)` or `file` scheme to override the default OS image for cluster nodes, or an existing Glance image name. type: string 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. @@ -1243,7 +1243,6 @@ spec: type: string required: - cloud - - computeFlavor type: object ovirt: description: Ovirt is the configuration used when installing on oVirt. diff --git a/docs/user/openstack/customization.md b/docs/user/openstack/customization.md index b7004943b0..9f14dace49 100644 --- a/docs/user/openstack/customization.md +++ b/docs/user/openstack/customization.md @@ -20,8 +20,7 @@ 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. * `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. @@ -67,7 +66,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" @@ -101,7 +101,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... diff --git a/pkg/asset/installconfig/openstack/openstack.go b/pkg/asset/installconfig/openstack/openstack.go index 7e36401582..098b680227 100644 --- a/pkg/asset/installconfig/openstack/openstack.go +++ b/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 } diff --git a/pkg/asset/installconfig/openstack/validation/cloudinfo.go b/pkg/asset/installconfig/openstack/validation/cloudinfo.go index c7ac811188..298d6cf5c5 100644 --- a/pkg/asset/installconfig/openstack/validation/cloudinfo.go +++ b/pkg/asset/installconfig/openstack/validation/cloudinfo.go @@ -105,10 +105,14 @@ func (ci *CloudInfo) collectInfo(ic *types.InstallConfig) error { } // 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 + } + } } } diff --git a/pkg/asset/installconfig/openstack/validation/platform.go b/pkg/asset/installconfig/openstack/validation/platform.go index 76681effbc..f1a41d44b0 100644 --- a/pkg/asset/installconfig/openstack/validation/platform.go +++ b/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 } diff --git a/pkg/asset/machines/master.go b/pkg/asset/machines/master.go index 99296003bf..ea27173337 100644 --- a/pkg/asset/machines/master.go +++ b/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 diff --git a/pkg/asset/machines/worker.go b/pkg/asset/machines/worker.go index cf8e0ac3b0..7a13d96045 100644 --- a/pkg/asset/machines/worker.go +++ b/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 diff --git a/pkg/types/conversion/installconfig.go b/pkg/types/conversion/installconfig.go index 77742e010f..d395d32315 100644 --- a/pkg/types/conversion/installconfig.go +++ b/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 } diff --git a/pkg/types/conversion/installconfig_test.go b/pkg/types/conversion/installconfig_test.go index ec346f5ee7..64a666cbd8 100644 --- a/pkg/types/conversion/installconfig_test.go +++ b/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 { diff --git a/pkg/types/openstack/platform.go b/pkg/types/openstack/platform.go index 80094d1745..a6a4749371 100644 --- a/pkg/types/openstack/platform.go +++ b/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. diff --git a/pkg/types/openstack/validation/platform_test.go b/pkg/types/openstack/validation/platform_test.go index 329b15b813..f407ffa9d9 100644 --- a/pkg/types/openstack/validation/platform_test.go +++ b/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", + }, } } diff --git a/pkg/types/validation/installconfig_test.go b/pkg/types/validation/installconfig_test.go index 3e5bf7a906..a9514a5bf6 100644 --- a/pkg/types/validation/installconfig_test.go +++ b/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", + }, } } From 4cdf1dc644c244318fdd386bc84cf931afdf78e9 Mon Sep 17 00:00:00 2001 From: Mike Fedosin Date: Fri, 4 Dec 2020 14:48:37 +0100 Subject: [PATCH 2/2] Document defaultMachinePlatform option for OpenStack platform The option exists for a long time, but it is not documented for OpenStack. This commit fixes it. Also it changes ip -> IP in some places to make it consistent across the document. --- docs/user/openstack/customization.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/user/openstack/customization.md b/docs/user/openstack/customization.md index 9f14dace49..a7298f4940 100644 --- a/docs/user/openstack/customization.md +++ b/docs/user/openstack/customization.md @@ -22,7 +22,7 @@ Beyond the [platform-agnostic `install-config.yaml` properties](../customization * `cloud` (required string): The name of the OpenStack cloud to use from `clouds.yaml`. * `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) @@ -32,6 +32,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 @@ -148,7 +149,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).