diff --git a/data/data/install.openshift.io_installconfigs.yaml b/data/data/install.openshift.io_installconfigs.yaml index 388545e1407..00e10a27edb 100644 --- a/data/data/install.openshift.io_installconfigs.yaml +++ b/data/data/install.openshift.io_installconfigs.yaml @@ -629,8 +629,16 @@ spec: gibibytes (GiB). Required type: integer type: - description: Type defines the type of the volume. Required + description: 'Type defines the type of the volume. Deprecated: + Use Types instead.' type: string + types: + description: Types is the list of the volume types of + the root volumes. This is mutually exclusive with + Type. + items: + type: string + type: array zones: description: Zones is the list of availability zones where the root volumes should be deployed. If no zones @@ -641,7 +649,7 @@ spec: type: array required: - size - - type + - types type: object serverGroupPolicy: description: ServerGroupPolicy will be used to create the @@ -1381,8 +1389,15 @@ spec: (GiB). Required type: integer type: - description: Type defines the type of the volume. Required + description: 'Type defines the type of the volume. Deprecated: + Use Types instead.' type: string + types: + description: Types is the list of the volume types of + the root volumes. This is mutually exclusive with Type. + items: + type: string + type: array zones: description: Zones is the list of availability zones where the root volumes should be deployed. If no zones are @@ -1393,7 +1408,7 @@ spec: type: array required: - size - - type + - types type: object serverGroupPolicy: description: ServerGroupPolicy will be used to create the @@ -3152,8 +3167,15 @@ spec: (GiB). Required type: integer type: - description: Type defines the type of the volume. Required + description: 'Type defines the type of the volume. Deprecated: + Use Types instead.' type: string + types: + description: Types is the list of the volume types of + the root volumes. This is mutually exclusive with Type. + items: + type: string + type: array zones: description: Zones is the list of availability zones where the root volumes should be deployed. If no zones are @@ -3164,7 +3186,7 @@ spec: type: array required: - size - - type + - types type: object serverGroupPolicy: description: ServerGroupPolicy will be used to create the diff --git a/data/data/openstack/bootstrap/main.tf b/data/data/openstack/bootstrap/main.tf index 5b2656eb6b5..e1a30939565 100644 --- a/data/data/openstack/bootstrap/main.tf +++ b/data/data/openstack/bootstrap/main.tf @@ -73,7 +73,7 @@ resource "openstack_blockstorage_volume_v3" "bootstrap_volume" { description = local.description size = var.openstack_master_root_volume_size - volume_type = var.openstack_master_root_volume_type + volume_type = var.openstack_master_root_volume_types[0] image_id = data.openstack_images_image_v2.base_image.id availability_zone = var.openstack_master_root_volume_availability_zones[0] diff --git a/data/data/openstack/masters/main.tf b/data/data/openstack/masters/main.tf index 3642f5595cf..0a733460ad0 100644 --- a/data/data/openstack/masters/main.tf +++ b/data/data/openstack/masters/main.tf @@ -64,7 +64,7 @@ resource "openstack_blockstorage_volume_v3" "master_volume" { count = var.openstack_master_root_volume_size == null ? 0 : var.master_count size = var.openstack_master_root_volume_size - volume_type = var.openstack_master_root_volume_type + volume_type = var.openstack_master_root_volume_types[count.index] image_id = data.openstack_images_image_v2.base_image.id availability_zone = var.openstack_master_root_volume_availability_zones[count.index] diff --git a/data/data/openstack/variables-openstack.tf b/data/data/openstack/variables-openstack.tf index e0bfbcbbc05..ccb9a201d71 100644 --- a/data/data/openstack/variables-openstack.tf +++ b/data/data/openstack/variables-openstack.tf @@ -1,9 +1,3 @@ -variable "openstack_master_root_volume_type" { - type = string - default = null - description = "The type of volume for the root block device of master nodes." -} - variable "openstack_master_root_volume_size" { type = number default = null @@ -389,6 +383,12 @@ variable "openstack_master_root_volume_availability_zones" { description = "List of availability Zones to Schedule the masters root volumes on." } +variable "openstack_master_root_volume_types" { + type = list(string) + default = [""] + description = "List of volume types used by the masters root volumes." +} + variable "openstack_worker_server_group_names" { type = set(string) description = "Names of the server groups for the worker nodes." diff --git a/docs/user/openstack/control-plane-machine-set.md b/docs/user/openstack/control-plane-machine-set.md index 776e177dcbd..1cf9ea9bb2c 100644 --- a/docs/user/openstack/control-plane-machine-set.md +++ b/docs/user/openstack/control-plane-machine-set.md @@ -322,3 +322,88 @@ spec: When reconciling the Machines, cluster-control-plane-machine-set-operator will match their spec against the template, after substituting `availabilityZone` and `rootVolume.availabilityZone` for each of them. The three Control plane Machines will all be provisioned on a different availability zone and have their `rootVolume` provisioned on a different availability zone. + +--- + +## Example 5: three Compute availability zones, three Storage types + +The storage types apply to the root volume. The `providerSpec` must contain a `rootVolume` property. + +```yaml +apiVersion: machine.openshift.io/v1 +kind: ControlPlaneMachineSet +metadata: + creationTimestamp: null + labels: + machine.openshift.io/cluster-api-cluster: ocp1-2g2xs + name: cluster + namespace: openshift-machine-api +spec: + replicas: 3 + selector: + matchLabels: + machine.openshift.io/cluster-api-cluster: ocp1-2g2xs + machine.openshift.io/cluster-api-machine-role: master + machine.openshift.io/cluster-api-machine-type: master + state: Active + strategy: {} + template: + machineType: machines_v1beta1_machine_openshift_io + machines_v1beta1_machine_openshift_io: + failureDomains: + openstack: + - availabilityZone: nova-one + rootVolume: + volumeType: fastpool-1 + - availabilityZone: nova-two + rootVolume: + volumeType: fastpool-2 + - availabilityZone: nova-three + rootVolume: + volumeType: fastpool-3 + platform: OpenStack + metadata: + labels: + machine.openshift.io/cluster-api-cluster: ocp1-2g2xs + machine.openshift.io/cluster-api-machine-role: master + machine.openshift.io/cluster-api-machine-type: master + spec: + lifecycleHooks: {} + metadata: {} + providerSpec: + value: # <-- The OpenStack providerSpec + apiVersion: machine.openshift.io/v1alpha1 + cloudName: openstack + cloudsSecret: + name: openstack-cloud-credentials + namespace: openshift-machine-api + flavor: m1.xlarge + image: ocp1-2g2xs-rhcos + kind: OpenstackProviderSpec + metadata: + creationTimestamp: null + networks: + - filter: {} + subnets: + - filter: + name: ocp1-2g2xs-nodes + tags: openshiftClusterID=ocp1-2g2xs + rootVolume: + diskSize: 30 + securityGroups: + - filter: {} + name: ocp1-2g2xs-master + serverGroupName: ocp1-2g2xs-master + serverMetadata: + Name: ocp1-2g2xs-master + openshiftClusterID: ocp1-2g2xs + tags: + - openshiftClusterID=ocp1-2g2xs + trunk: true + userDataSecret: + name: master-user-data +``` + +When reconciling the Machines, cluster-control-plane-machine-set-operator will match their spec against the template, after substituting `availabilityZone` and `rootVolume.volumeType` for each of them. + +The three Control plane Machines will all be provisioned on a different availability zone and have their `rootVolume` provisioned with a different volume type. \ No newline at end of file diff --git a/docs/user/openstack/customization.md b/docs/user/openstack/customization.md index de378224fe1..5592e9642bf 100644 --- a/docs/user/openstack/customization.md +++ b/docs/user/openstack/customization.md @@ -47,7 +47,8 @@ Beyond the [platform-agnostic `install-config.yaml` properties](../customization * `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. Must be set to at least 25. - * `type` (required string): The volume pool to create the volume from. + * `type` (optional string): The volume pool to create the volume from. It was replaced by `types`. + * `types` (required list of strings): The volume pool to create the volume from. If compute `zones` are defined with more than one type, the number of zones must match the number of types. * `zones` (optional list of strings): The names of the availability zones you want to install your root volumes on. If unset, the installer will use your default volume zone. If compute `zones` contains at least one value, `rootVolume.zones` must also contain at least one value. Indeed, when a machine is created with a compute availability zone and a storage root volume with no specified rootVolume.availabilityZone, [CAPO](https://github.com/kubernetes-sigs/cluster-api-provider-openstack/blob/9d183bd479fe9aed4f6e7ac3d5eee46681c518e7/pkg/cloud/services/compute/instance.go#L439-L442) will use the compute AZ for the volume AZ. @@ -106,7 +107,8 @@ compute: type: ml.large rootVolume: size: 30 - type: performance + types: + - performance replicas: 3 metadata: name: test-cluster diff --git a/pkg/asset/installconfig/openstack/validation/machinepool.go b/pkg/asset/installconfig/openstack/validation/machinepool.go index f7b72fba224..9c06d0593fd 100644 --- a/pkg/asset/installconfig/openstack/validation/machinepool.go +++ b/pkg/asset/installconfig/openstack/validation/machinepool.go @@ -42,7 +42,7 @@ func ValidateMachinePool(p *openstack.MachinePool, ci *CloudInfo, controlPlane b var checkStorageFlavor bool // Validate Root Volumes if p.RootVolume != nil { - allErrs = append(allErrs, validateVolumeTypes(p.RootVolume.Type, ci.VolumeTypes, fldPath.Child("rootVolume").Child("type"))...) + allErrs = append(allErrs, validateVolumeTypes(p.RootVolume.Types, ci.VolumeTypes, fldPath.Child("rootVolume").Child("types"))...) 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 GB to use root volumes, had %d GB", minimumStorage, p.RootVolume.Size))) } else if p.RootVolume.Size < recommendedStorage { @@ -89,15 +89,16 @@ func validateZones(input []string, available []string, fldPath *field.Path) fiel return allErrs } -func validateVolumeTypes(input string, available []string, fldPath *field.Path) field.ErrorList { +func validateVolumeTypes(input []string, available []string, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} - if input == "" { - allErrs = append(allErrs, field.Invalid(fldPath, input, "Volume type must be specified to use root volumes")) + if len(input) == 0 { return allErrs } - volumeTypes := sets.NewString(available...) - if !volumeTypes.Has(input) { - allErrs = append(allErrs, field.Invalid(fldPath, input, "Volume Type either does not exist in this cloud, or is not available")) + volumeTypes := sets.New[string](available...) + for _, volumeType := range input { + if !volumeTypes.Has(volumeType) { + allErrs = append(allErrs, field.Invalid(fldPath, volumeType, "Volume type either does not exist in this cloud, or is not available")) + } } return allErrs diff --git a/pkg/asset/installconfig/openstack/validation/machinepool_test.go b/pkg/asset/installconfig/openstack/validation/machinepool_test.go index f74b0368a4c..be1ec03283e 100644 --- a/pkg/asset/installconfig/openstack/validation/machinepool_test.go +++ b/pkg/asset/installconfig/openstack/validation/machinepool_test.go @@ -27,13 +27,16 @@ const ( baremetalFlavor = "baremetal-flavor" - volumeType = "performance" invalidType = "invalid-type" volumeSmallSize = 10 volumeMediumSize = 40 volumeLargeSize = 100 ) +var volumeTypes = []string{"performance", "standard"} +var invalidVolumeTypes = []string{"performance", "invalid-type"} +var volumeType = volumeTypes[0] + func validMachinePool() *openstack.MachinePool { return &openstack.MachinePool{ FlavorName: validCtrlPlaneFlavor, @@ -46,8 +49,8 @@ func invalidMachinePoolSmallVolume() *openstack.MachinePool { FlavorName: validCtrlPlaneFlavor, Zones: []string{""}, RootVolume: &openstack.RootVolume{ - Type: volumeType, Size: volumeSmallSize, + Types: volumeTypes, Zones: []string{""}, }, } @@ -58,8 +61,8 @@ func warningMachinePoolMediumVolume() *openstack.MachinePool { FlavorName: validCtrlPlaneFlavor, Zones: []string{""}, RootVolume: &openstack.RootVolume{ - Type: volumeType, Size: volumeMediumSize, + Types: volumeTypes, Zones: []string{""}, }, } @@ -70,8 +73,8 @@ func validMachinePoolLargeVolume() *openstack.MachinePool { FlavorName: validCtrlPlaneFlavor, Zones: []string{""}, RootVolume: &openstack.RootVolume{ - Type: volumeType, Size: volumeLargeSize, + Types: volumeTypes, Zones: []string{validZone}, }, } @@ -144,9 +147,7 @@ func validMpoolCloudInfo() *CloudInfo { VolumeZones: []string{ validZone, }, - VolumeTypes: []string{ - volumeType, - }, + VolumeTypes: volumeTypes, } } @@ -370,28 +371,41 @@ func TestOpenStackMachinepoolValidation(t *testing.T) { expectedErrMsg: `compute\[0\].platform.openstack.rootVolume.zones: Invalid value: \[\]string{"AZ1", "AZ2"}: there must be either just one volume availability zone common to all nodes or the number of compute and volume availability zones must be equal`, }, { - name: "empty volume type", - controlPlane: false, + name: "invalid volume types", + controlPlane: true, mpool: func() *openstack.MachinePool { mp := validMachinePoolLargeVolume() - mp.RootVolume.Type = "" + mp.RootVolume.Types = invalidVolumeTypes return mp }(), cloudInfo: validMpoolCloudInfo(), expectedError: true, - expectedErrMsg: `compute\[0\].platform.openstack.rootVolume.type: Invalid value: \"\": Volume type must be specified to use root volumes`, + expectedErrMsg: "controlPlane.platform.openstack.rootVolume.types: Invalid value: \"invalid-type\": Volume type either does not exist in this cloud, or is not available", }, { - name: "invalid volume type", - controlPlane: false, + name: "valid volume type", + controlPlane: true, mpool: func() *openstack.MachinePool { mp := validMachinePoolLargeVolume() - mp.RootVolume.Type = invalidType + mp.RootVolume.DeprecatedType = volumeType + mp.RootVolume.Types = []string{} return mp }(), cloudInfo: validMpoolCloudInfo(), - expectedError: true, - expectedErrMsg: `compute\[0\].platform.openstack.rootVolume.type: Invalid value: \"invalid-type\": Volume Type either does not exist in this cloud, or is not available`, + expectedError: false, + expectedErrMsg: "", + }, + { + name: "valid volume types", + controlPlane: true, + mpool: func() *openstack.MachinePool { + mp := validMachinePoolLargeVolume() + mp.RootVolume.Types = volumeTypes + return mp + }(), + cloudInfo: validMpoolCloudInfo(), + expectedError: false, + expectedErrMsg: "", }, } diff --git a/pkg/asset/machines/openstack/machines.go b/pkg/asset/machines/openstack/machines.go index f530a0d3a15..90d0115cc1a 100644 --- a/pkg/asset/machines/openstack/machines.go +++ b/pkg/asset/machines/openstack/machines.go @@ -57,7 +57,7 @@ func Machines(clusterID string, config *types.InstallConfig, pool *types.Machine for idx := int64(0); idx < total; idx++ { failureDomain := failureDomains[uint(idx)%uint(len(failureDomains))] - provider, err := generateProvider( + providerSpec := generateProviderSpec( clusterID, platform, mpool, @@ -67,9 +67,7 @@ func Machines(clusterID string, config *types.InstallConfig, pool *types.Machine trunkSupport, failureDomain, ) - if err != nil { - return nil, nil, fmt.Errorf("failed to generate the Machine providerSpec for replica %d: %w", idx, err) - } + machine := machineapi.Machine{ TypeMeta: metav1.TypeMeta{ APIVersion: "machine.openshift.io/v1beta1", @@ -86,7 +84,7 @@ func Machines(clusterID string, config *types.InstallConfig, pool *types.Machine }, Spec: machineapi.MachineSpec{ ProviderSpec: machineapi.ProviderSpec{ - Value: &runtime.RawExtension{Object: provider}, + Value: &runtime.RawExtension{Object: providerSpec}, }, // we don't need to set Versions, because we control those via operators. }, @@ -94,7 +92,7 @@ func Machines(clusterID string, config *types.InstallConfig, pool *types.Machine machines = append(machines, machine) } - machineSetProvider, err := generateProvider( + machineSetProviderSpec := generateProviderSpec( clusterID, platform, mpool, @@ -104,9 +102,6 @@ func Machines(clusterID string, config *types.InstallConfig, pool *types.Machine trunkSupport, machinev1.OpenStackFailureDomain{RootVolume: &machinev1.RootVolume{}}, ) - if err != nil { - return nil, nil, fmt.Errorf("failed to generate the CPMS providerSpec: %w", err) - } replicas := int32(total) @@ -144,7 +139,7 @@ func Machines(clusterID string, config *types.InstallConfig, pool *types.Machine }, Spec: machineapi.MachineSpec{ ProviderSpec: machineapi.ProviderSpec{ - Value: &runtime.RawExtension{Object: machineSetProvider}, + Value: &runtime.RawExtension{Object: machineSetProviderSpec}, }, }, }, @@ -161,7 +156,7 @@ func Machines(clusterID string, config *types.InstallConfig, pool *types.Machine return machines, controlPlaneMachineSet, nil } -func generateProvider(clusterID string, platform *openstack.Platform, mpool *openstack.MachinePool, osImage string, role, userDataSecret string, trunkSupport bool, failureDomain machinev1.OpenStackFailureDomain) (*machinev1alpha1.OpenstackProviderSpec, error) { +func generateProviderSpec(clusterID string, platform *openstack.Platform, mpool *openstack.MachinePool, osImage string, role, userDataSecret string, trunkSupport bool, failureDomain machinev1.OpenStackFailureDomain) *machinev1alpha1.OpenstackProviderSpec { var controlPlaneNetwork machinev1alpha1.NetworkParam additionalNetworks := make([]machinev1alpha1.NetworkParam, 0, len(mpool.AdditionalNetworkIDs)) primarySubnet := "" @@ -253,13 +248,13 @@ func generateProvider(clusterID string, platform *openstack.Platform, mpool *ope spec.RootVolume = &machinev1alpha1.RootVolume{ Size: mpool.RootVolume.Size, SourceUUID: osImage, - VolumeType: mpool.RootVolume.Type, + VolumeType: failureDomain.RootVolume.VolumeType, Zone: failureDomain.RootVolume.AvailabilityZone, } } else { spec.Image = osImage } - return &spec, nil + return &spec } // failureDomainIsEmpty returns true if the failure domain only contains nil or @@ -269,7 +264,7 @@ func failureDomainIsEmpty(failureDomain machinev1.OpenStackFailureDomain) bool { if failureDomain.RootVolume == nil { return true } - if failureDomain.RootVolume.AvailabilityZone == "" { + if failureDomain.RootVolume.AvailabilityZone == "" && failureDomain.RootVolume.VolumeType == "" { return true } } @@ -291,29 +286,32 @@ func pruneFailureDomains(failureDomains []machinev1.OpenStackFailureDomain) []ma // domain slice is guaranteed to have at least one element. func failureDomainsFromSpec(mpool openstack.MachinePool) []machinev1.OpenStackFailureDomain { var numberOfFailureDomains int - { - numberOfFailureDomains = len(mpool.Zones) - - if mpool.RootVolume != nil { - // At this point, after validation, there can't be a - // number of Compute zones different from the nunmber - // of Root volumes zones. However, we want to consider - // the case where one of them is left as its default - // (length zero), or is set to one value (length one). - // - // As a consequence, one of these is true: - // - // * there are as many Compute zones as Root volumes zones - // * there are zero or one Compute zones - // * there are zero or one Root volumes zones - if computes, volumes := len(mpool.Zones), len(mpool.RootVolume.Zones); computes > 1 && volumes > 1 && computes != volumes { - panic("Compute and Storage availability zones in the machine-pool should have been validated to have equal length") - } - - if volumes := len(mpool.RootVolume.Zones); volumes > numberOfFailureDomains { - numberOfFailureDomains = volumes + if mpool.RootVolume != nil { + // At this point, after validation, compute availability zones, + // storage avalability zones and root volume types must all be + // equal in number. However, we want to accept case where any + // of them has zero or one value (which means: apply the same + // value to all failure domains). + var ( + highestCardinality int + highestCardinalityField string + ) + for field, cardinality := range map[string]int{ + "compute availability zones": len(mpool.Zones), + "storage availability zones": len(mpool.RootVolume.Zones), + "root volume types": len(mpool.RootVolume.Types), + } { + if cardinality > 1 { + if highestCardinality > 1 && cardinality != highestCardinality { + panic(highestCardinalityField + " and " + field + " should have equal length") + } + highestCardinality = cardinality + highestCardinalityField = field } } + numberOfFailureDomains = highestCardinality + } else { + numberOfFailureDomains = len(mpool.Zones) } // No failure domain is exactly like one failure domain with the default values. @@ -348,6 +346,15 @@ func failureDomainsFromSpec(mpool openstack.MachinePool) []machinev1.OpenStackFa AvailabilityZone: mpool.RootVolume.Zones[i], } } + + switch len(mpool.RootVolume.Types) { + case 0: + panic("Root volume types should have been validated to have at least one element") + case 1: + failureDomains[i].RootVolume.VolumeType = mpool.RootVolume.Types[0] + default: + failureDomains[i].RootVolume.VolumeType = mpool.RootVolume.Types[i] + } } } return failureDomains diff --git a/pkg/asset/machines/openstack/machines_test.go b/pkg/asset/machines/openstack/machines_test.go index 43c98ff0006..35da7fbc095 100644 --- a/pkg/asset/machines/openstack/machines_test.go +++ b/pkg/asset/machines/openstack/machines_test.go @@ -2,6 +2,7 @@ package openstack import ( "fmt" + "strings" "testing" machinev1 "github.com/openshift/api/machine/v1" @@ -16,7 +17,21 @@ func mpWithZones(zones ...string) func(*openstack.MachinePool) { func mpWithRootVolumeZones(zones ...string) func(*openstack.MachinePool) { return func(mpool *openstack.MachinePool) { - mpool.RootVolume = &openstack.RootVolume{Zones: zones} + if mpool.RootVolume != nil { + mpool.RootVolume.Zones = zones + } else { + mpool.RootVolume = &openstack.RootVolume{Zones: zones} + } + } +} + +func mpWithRootVolumeTypes(types ...string) func(*openstack.MachinePool) { + return func(mpool *openstack.MachinePool) { + if mpool.RootVolume != nil { + mpool.RootVolume.Types = types + } else { + mpool.RootVolume = &openstack.RootVolume{Types: types} + } } } @@ -32,6 +47,15 @@ func TestFailureDomains(t *testing.T) { type checkFunc func([]machinev1.OpenStackFailureDomain, error) error check := func(fns ...checkFunc) []checkFunc { return fns } + hasNFailureDomains := func(want int) checkFunc { + return func(fds []machinev1.OpenStackFailureDomain, _ error) error { + if have := len(fds); want != have { + return fmt.Errorf("expected %d failure domains, got %d", want, have) + } + return nil + } + } + hasComputeZones := func(wantZones ...string) checkFunc { return func(fds []machinev1.OpenStackFailureDomain, _ error) error { haveZones := make([]string, len(fds)) @@ -86,6 +110,30 @@ func TestFailureDomains(t *testing.T) { } } + hasRootVolumeTypes := func(wantTypes ...string) checkFunc { + return func(fds []machinev1.OpenStackFailureDomain, _ error) error { + haveTypes := make([]string, len(fds)) + for i := range fds { + if fds[i].RootVolume == nil { + return fmt.Errorf("failure domain %d has unexpectedly nil RootVolume", i) + } + haveTypes[i] = fds[i].RootVolume.VolumeType + } + + if wantLen, haveLen := len(wantTypes), len(haveTypes); wantLen != haveLen { + return fmt.Errorf("expected root volume types %v, got %v", wantTypes, haveTypes) + } + + for i := range fds { + if want, have := wantTypes[i], haveTypes[i]; want != have { + return fmt.Errorf("expected root volume types %v, got %v", wantTypes, haveTypes) + } + } + + return nil + } + } + doesNotPanic := func(_ []machinev1.OpenStackFailureDomain, have error) error { if have != nil { return fmt.Errorf("unexpected panic: %w", have) @@ -98,7 +146,7 @@ func TestFailureDomains(t *testing.T) { if have == nil { return fmt.Errorf("unexpectedly, didn't panic") } - if have := fmt.Sprintf("%v", have); want != have { + if have := fmt.Sprintf("%v", have); !strings.Contains(have, want) { return fmt.Errorf("expected panic with %q, got %q", want, have) } return nil @@ -114,6 +162,7 @@ func TestFailureDomains(t *testing.T) { "no_zones", generateMachinePool(), check( + hasNFailureDomains(1), hasComputeZones(""), hasNilRootVolume, doesNotPanic, @@ -125,6 +174,7 @@ func TestFailureDomains(t *testing.T) { mpWithZones("one"), ), check( + hasNFailureDomains(1), hasComputeZones("one"), hasNilRootVolume, doesNotPanic, @@ -136,6 +186,7 @@ func TestFailureDomains(t *testing.T) { mpWithZones("one", "two", "three"), ), check( + hasNFailureDomains(3), hasComputeZones("one", "two", "three"), hasNilRootVolume, doesNotPanic, @@ -146,10 +197,13 @@ func TestFailureDomains(t *testing.T) { generateMachinePool( mpWithZones("one", "two", "three"), mpWithRootVolumeZones("volume_one"), + mpWithRootVolumeTypes("type-1"), ), check( + hasNFailureDomains(3), hasComputeZones("one", "two", "three"), hasRootVolumeZones("volume_one", "volume_one", "volume_one"), + hasRootVolumeTypes("type-1", "type-1", "type-1"), doesNotPanic, ), }, @@ -158,33 +212,55 @@ func TestFailureDomains(t *testing.T) { generateMachinePool( mpWithZones("one"), mpWithRootVolumeZones("volume_one", "volume_two", "volume_three"), + mpWithRootVolumeTypes("type-1"), ), check( + hasNFailureDomains(3), hasComputeZones("one", "one", "one"), hasRootVolumeZones("volume_one", "volume_two", "volume_three"), + hasRootVolumeTypes("type-1", "type-1", "type-1"), doesNotPanic, ), }, { - "three_compute_zone_three_root_volume_zones", + "three_compute_zone_two_root_volume_zones_panics", + generateMachinePool( + mpWithZones("one", "two", "three"), + mpWithRootVolumeZones("volume_one", "volume_two"), + ), + check( + // We have to check for a partial result here, because the mapping + // of compute zones to root volume zones is handled in a map therefore + // the order is not deterministic. + panicsWith("availability zones should have equal length"), + ), + }, + { + "three_compute_zones_three_root_volume_types", generateMachinePool( mpWithZones("one", "two", "three"), mpWithRootVolumeZones("volume_one", "volume_two", "volume_three"), + mpWithRootVolumeTypes("type-1", "type-2", "type-3"), ), check( + hasNFailureDomains(3), hasComputeZones("one", "two", "three"), hasRootVolumeZones("volume_one", "volume_two", "volume_three"), + hasRootVolumeTypes("type-1", "type-2", "type-3"), doesNotPanic, ), }, { - "three_compute_zone_two_root_volume_zones_panics", + "three_root_volume_types", generateMachinePool( - mpWithZones("one", "two", "three"), - mpWithRootVolumeZones("volume_one", "volume_two"), + mpWithRootVolumeTypes("type-1", "type-2", "type-3"), ), check( - panicsWith("Compute and Storage availability zones in the machine-pool should have been validated to have equal length"), + hasNFailureDomains(3), + hasComputeZones("", "", ""), + hasRootVolumeZones("", "", ""), + hasRootVolumeTypes("type-1", "type-2", "type-3"), + doesNotPanic, ), }, } { diff --git a/pkg/asset/machines/openstack/machinesets.go b/pkg/asset/machines/openstack/machinesets.go index be4e51a16d7..f269b813a4d 100644 --- a/pkg/asset/machines/openstack/machinesets.go +++ b/pkg/asset/machines/openstack/machinesets.go @@ -8,14 +8,22 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" - machinev1 "github.com/openshift/api/machine/v1" clusterapi "github.com/openshift/api/machine/v1beta1" "github.com/openshift/installer/pkg/types" "github.com/openshift/installer/pkg/types/openstack" - openstackdefaults "github.com/openshift/installer/pkg/types/openstack/defaults" ) -// MachineSets returns a list of machinesets for a machinepool. +const maxInt32 int64 = int64(^uint32(0)) >> 1 + +// MachineSets returns the MachineSets encoded by the given machine-pool. The +// number of returned MachineSets, while being capped to the number of +// replicas, depends on the variable-length fields in the machine-pool. Each +// MachineSet generates a set of identical Machines; to encode for Machines +// spread on, say, three availability zones, three MachineSets must be +// produced. Note that for each variable-length field (currently: Compute +// availability zones, Storage availability zones and Root volume types), when +// more than one is specified, values of identical index are grouped in the +// same MachineSet. func MachineSets(clusterID string, config *types.InstallConfig, pool *types.MachinePool, osImage, role, userDataSecret string, clientOpts *clientconfig.ClientOpts) ([]*clusterapi.MachineSet, error) { if configPlatform := config.Platform.Name(); configPlatform != openstack.Name { return nil, fmt.Errorf("non-OpenStack configuration: %q", configPlatform) @@ -23,6 +31,9 @@ func MachineSets(clusterID string, config *types.InstallConfig, pool *types.Mach if poolPlatform := pool.Platform.Name(); poolPlatform != openstack.Name { return nil, fmt.Errorf("non-OpenStack machine-pool: %q", poolPlatform) } + if pool.Replicas == nil || *pool.Replicas < 1 { + return nil, nil + } platform := config.Platform.OpenStack mpool := pool.Platform.OpenStack trunkSupport, err := checkNetworkExtensionAvailability(platform.Cloud, "trunk", clientOpts) @@ -30,25 +41,24 @@ func MachineSets(clusterID string, config *types.InstallConfig, pool *types.Mach return nil, err } - volumeAZs := []string{openstackdefaults.DefaultRootVolumeAZ()} - if mpool.RootVolume != nil && len(mpool.RootVolume.Zones) != 0 { - volumeAZs = mpool.RootVolume.Zones - } - - total := int32(0) - if pool.Replicas != nil { - total = int32(*pool.Replicas) - } - - numOfAZs := int32(len(mpool.Zones)) - var machinesets []*clusterapi.MachineSet + failureDomains := failureDomainsFromSpec(*mpool) + numberOfFailureDomains := int64(len(failureDomains)) - for idx, az := range mpool.Zones { - replicas := int32(total / numOfAZs) - if int32(idx) < total%numOfAZs { - replicas++ + machinesets := make([]*clusterapi.MachineSet, len(failureDomains)) + for idx := range machinesets { + var replicaNumber int32 + { + replicas := *pool.Replicas / numberOfFailureDomains + if int64(idx) < *pool.Replicas%numberOfFailureDomains { + replicas++ + } + if replicas > maxInt32 { + return nil, fmt.Errorf("the number of requested worker replicas (%d) is too high. Each MachineSet can hold %d replicas; the install-config encodes for %d MachineSets, which gives us a replica number of %d", *pool.Replicas, maxInt32, numberOfFailureDomains, replicas) + } + replicaNumber = int32(replicas) } - provider, err := generateProvider( + + providerSpec := generateProviderSpec( clusterID, platform, mpool, @@ -56,21 +66,13 @@ func MachineSets(clusterID string, config *types.InstallConfig, pool *types.Mach role, userDataSecret, trunkSupport, - machinev1.OpenStackFailureDomain{ - AvailabilityZone: az, - RootVolume: &machinev1.RootVolume{ - AvailabilityZone: volumeAZs[idx%len(volumeAZs)], - }, - }, + failureDomains[idx], ) - if err != nil { - return nil, err - } // Set unique name for the machineset name := fmt.Sprintf("%s-%s-%d", clusterID, pool.Name, idx) - mset := &clusterapi.MachineSet{ + machinesets[idx] = &clusterapi.MachineSet{ TypeMeta: metav1.TypeMeta{ APIVersion: "machine.openshift.io/v1beta1", Kind: "MachineSet", @@ -85,7 +87,7 @@ func MachineSets(clusterID string, config *types.InstallConfig, pool *types.Mach }, }, Spec: clusterapi.MachineSetSpec{ - Replicas: &replicas, + Replicas: &replicaNumber, Selector: metav1.LabelSelector{ MatchLabels: map[string]string{ "machine.openshift.io/cluster-api-machineset": name, @@ -103,14 +105,13 @@ func MachineSets(clusterID string, config *types.InstallConfig, pool *types.Mach }, Spec: clusterapi.MachineSpec{ ProviderSpec: clusterapi.ProviderSpec{ - Value: &runtime.RawExtension{Object: provider}, + Value: &runtime.RawExtension{Object: providerSpec}, }, // we don't need to set Versions, because we control those via cluster operators. }, }, }, } - machinesets = append(machinesets, mset) } return machinesets, nil diff --git a/pkg/tfvars/openstack/openstack.go b/pkg/tfvars/openstack/openstack.go index ee5af5f22b4..3e1bd188058 100644 --- a/pkg/tfvars/openstack/openstack.go +++ b/pkg/tfvars/openstack/openstack.go @@ -120,6 +120,14 @@ func TFVars( } } + // storageVolumeTypes is a slice where each index targets a master. + storageVolumeTypes := make([]string, len(masterSpecs)) + for i := range storageVolumeTypes { + if masterSpecs[i].RootVolume != nil { + storageVolumeTypes[i] = masterSpecs[i].RootVolume.VolumeType + } + } + // Normally baseImage contains a URL that we will use to create a new Glance image, but for testing // purposes we also allow to set a custom Glance image name to skip the uploading. Here we check // whether baseImage is a URL or not. If this is the first case, it means that the image should be @@ -149,10 +157,8 @@ func TFVars( } var rootVolumeSize int - var rootVolumeType string if rootVolume := masterSpecs[0].RootVolume; rootVolume != nil { rootVolumeSize = rootVolume.Size - rootVolumeType = rootVolume.VolumeType } masterServerGroupPolicy := getServerGroupPolicy(mastermpool, defaultmpool, types_openstack.SGPolicySoftAntiAffinity) @@ -237,7 +243,6 @@ func TFVars( TrunkSupport bool `json:"openstack_trunk_support,omitempty"` OctaviaSupport bool `json:"openstack_octavia_support,omitempty"` RootVolumeSize int `json:"openstack_master_root_volume_size,omitempty"` - RootVolumeType string `json:"openstack_master_root_volume_type,omitempty"` BootstrapShim string `json:"openstack_bootstrap_shim_ignition,omitempty"` ExternalDNS []string `json:"openstack_external_dns,omitempty"` MasterServerGroupName string `json:"openstack_master_server_group_name,omitempty"` @@ -251,6 +256,7 @@ func TFVars( MachinesPorts []*terraformPort `json:"openstack_machines_ports"` MasterAvailabilityZones []string `json:"openstack_master_availability_zones,omitempty"` MasterRootVolumeAvailabilityZones []string `json:"openstack_master_root_volume_availability_zones,omitempty"` + MasterRootVolumeTypes []string `json:"openstack_master_root_volume_types,omitempty"` UserManagedLoadBalancer bool `json:"openstack_user_managed_load_balancer"` }{ BaseImageName: imageName, @@ -264,7 +270,6 @@ func TFVars( TrunkSupport: masterSpecs[0].Trunk, OctaviaSupport: octaviaSupport, RootVolumeSize: rootVolumeSize, - RootVolumeType: rootVolumeType, BootstrapShim: bootstrapShim, ExternalDNS: installConfig.Config.Platform.OpenStack.ExternalDNS, MasterServerGroupName: masterServerGroupName, @@ -278,6 +283,7 @@ func TFVars( MachinesPorts: machinesPorts, MasterAvailabilityZones: computeAvailabilityZones, MasterRootVolumeAvailabilityZones: storageAvailabilityZones, + MasterRootVolumeTypes: storageVolumeTypes, UserManagedLoadBalancer: userManagedLoadBalancer, }, "", " ") } diff --git a/pkg/types/conversion/installconfig.go b/pkg/types/conversion/installconfig.go index ab4895f4383..6d7fda620ae 100644 --- a/pkg/types/conversion/installconfig.go +++ b/pkg/types/conversion/installconfig.go @@ -168,6 +168,35 @@ func convertOpenStack(config *types.InstallConfig) error { config.Platform.OpenStack.DefaultMachinePlatform.FlavorName = config.Platform.OpenStack.DeprecatedFlavorName } + // type has been deprecated in favor of types in the machinePools. + if config.ControlPlane != nil && config.ControlPlane.Platform.OpenStack.RootVolume != nil && config.ControlPlane.Platform.OpenStack.RootVolume.DeprecatedType != "" { + if len(config.ControlPlane.Platform.OpenStack.RootVolume.Types) > 0 { + // Return error if both type and types of rootVolume are specified in the config + return field.Forbidden(field.NewPath("controlPlane").Child("platform").Child("openstack").Child("rootVolume").Child("type"), "cannot specify type and types in rootVolume together") + } + config.ControlPlane.Platform.OpenStack.RootVolume.Types = []string{config.ControlPlane.Platform.OpenStack.RootVolume.DeprecatedType} + config.ControlPlane.Platform.OpenStack.RootVolume.DeprecatedType = "" + } + for _, pool := range config.Compute { + mpool := pool.Platform.OpenStack + if mpool.RootVolume != nil && mpool.RootVolume.DeprecatedType != "" { + if mpool.RootVolume.Types != nil && len(mpool.RootVolume.Types) > 0 { + // Return error if both type and types of rootVolume are specified in the config + return field.Forbidden(field.NewPath("compute").Child("platform").Child("openstack").Child("rootVolume").Child("type"), "cannot specify type and types in rootVolume together") + } + mpool.RootVolume.Types = []string{mpool.RootVolume.DeprecatedType} + mpool.RootVolume.DeprecatedType = "" + } + } + if config.Platform.OpenStack.DefaultMachinePlatform != nil && config.Platform.OpenStack.DefaultMachinePlatform.RootVolume != nil && config.Platform.OpenStack.DefaultMachinePlatform.RootVolume.DeprecatedType != "" { + if len(config.Platform.OpenStack.DefaultMachinePlatform.RootVolume.Types) > 0 { + // Return error if both type and types of defaultMachinePlatform are specified in the config + return field.Forbidden(field.NewPath("platform").Child("openstack").Child("type"), "cannot specify type and types in defaultMachinePlatform together") + } + config.Platform.OpenStack.DefaultMachinePlatform.RootVolume.Types = []string{config.Platform.OpenStack.DefaultMachinePlatform.RootVolume.DeprecatedType} + config.Platform.OpenStack.DefaultMachinePlatform.RootVolume.DeprecatedType = "" + } + if err := upconvertVIP(&config.Platform.OpenStack.APIVIPs, config.Platform.OpenStack.DeprecatedAPIVIP, "apiVIP", "apiVIPs", field.NewPath("platform").Child("openstack")); err != nil { return err } diff --git a/pkg/types/conversion/installconfig_test.go b/pkg/types/conversion/installconfig_test.go index 6b2e786758b..04eb9d1b902 100644 --- a/pkg/types/conversion/installconfig_test.go +++ b/pkg/types/conversion/installconfig_test.go @@ -393,6 +393,39 @@ func TestConvertInstallConfig(t *testing.T) { }, expectedError: "cannot specify computeFlavor and type in defaultMachinePlatform together", }, + { + name: "deprecated OpenStack controlPlane with type in rootVolume", + config: &types.InstallConfig{ + TypeMeta: metav1.TypeMeta{ + APIVersion: types.InstallConfigVersion, + }, + Platform: types.Platform{OpenStack: &openstack.Platform{}}, + ControlPlane: &types.MachinePool{ + Platform: types.MachinePoolPlatform{ + OpenStack: &openstack.MachinePool{ + RootVolume: &openstack.RootVolume{ + DeprecatedType: "fast", + }, + }, + }, + }, + }, + expected: &types.InstallConfig{ + TypeMeta: metav1.TypeMeta{ + APIVersion: types.InstallConfigVersion, + }, + Platform: types.Platform{OpenStack: &openstack.Platform{}}, + ControlPlane: &types.MachinePool{ + Platform: types.MachinePoolPlatform{ + OpenStack: &openstack.MachinePool{ + RootVolume: &openstack.RootVolume{ + Types: []string{"fast"}, + }, + }, + }, + }, + }, + }, { name: "openstack deprecated apiVIP", config: &types.InstallConfig{ diff --git a/pkg/types/openstack/machinepool.go b/pkg/types/openstack/machinepool.go index 85ef9937d57..02e66182f33 100644 --- a/pkg/types/openstack/machinepool.go +++ b/pkg/types/openstack/machinepool.go @@ -48,7 +48,15 @@ func (o *MachinePool) Set(required *MachinePool) { o.RootVolume = new(RootVolume) } o.RootVolume.Size = required.RootVolume.Size - o.RootVolume.Type = required.RootVolume.Type + o.RootVolume.DeprecatedType = required.RootVolume.DeprecatedType + + if required.RootVolume.DeprecatedType != "" { + o.RootVolume.DeprecatedType = "" + o.RootVolume.Types = []string{required.RootVolume.DeprecatedType} + } else if len(required.RootVolume.Types) > 0 { + o.RootVolume.Types = required.RootVolume.Types + } + if len(required.RootVolume.Zones) > 0 { o.RootVolume.Zones = required.RootVolume.Zones } @@ -77,8 +85,14 @@ type RootVolume struct { // Required Size int `json:"size"` // Type defines the type of the volume. - // Required - Type string `json:"type"` + // Deprecated: Use Types instead. + // +optional + DeprecatedType string `json:"type,omitempty"` + + // Types is the list of the volume types of the root volumes. + // This is mutually exclusive with Type. + // +required + Types []string `json:"types"` // Zones is the list of availability zones where the root volumes should be deployed. // If no zones are provided, all instances will be deployed on OpenStack Cinder default availability zone diff --git a/pkg/types/openstack/validation/machinepool.go b/pkg/types/openstack/validation/machinepool.go index 9203caebb27..6fc002aa4d5 100644 --- a/pkg/types/openstack/validation/machinepool.go +++ b/pkg/types/openstack/validation/machinepool.go @@ -25,8 +25,39 @@ func ValidateMachinePool(_ *openstack.Platform, machinePool *openstack.MachinePo errs = append(errs, field.NotSupported(fldPath.Child("serverGroupPolicy"), machinePool.ServerGroupPolicy, validServerGroupPolicies)) } - if len(machinePool.Zones) > 0 && machinePool.RootVolume != nil && len(machinePool.RootVolume.Zones) == 0 { - errs = append(errs, field.Required(fldPath.Child("rootVolume").Child("zones"), "root volume availability zones must be specified when compute availability zones are specified")) + if machinePool.RootVolume != nil { + if len(machinePool.Zones) > 0 && len(machinePool.RootVolume.Zones) == 0 { + errs = append(errs, field.Required(fldPath.Child("rootVolume").Child("zones"), "root volume availability zones must be specified when compute availability zones are specified")) + } + + rootVolumeType := machinePool.RootVolume.DeprecatedType + rootVolumeTypes := machinePool.RootVolume.Types + typePath := fldPath.Child("rootVolume").Child("type") + typesPath := fldPath.Child("rootVolume").Child("types") + + if rootVolumeType != "" && len(rootVolumeTypes) > 0 { + errs = append(errs, field.Invalid(typePath, rootVolumeType, "Only one of type or types can be specified")) + errs = append(errs, field.Invalid(typesPath, rootVolumeTypes, "Only one of type or types can be specified")) + } + + if rootVolumeType == "" && len(rootVolumeTypes) == 0 { + errs = append(errs, field.Invalid(typePath, rootVolumeType, "Either type or types must be specified")) + errs = append(errs, field.Invalid(typesPath, rootVolumeTypes, "Either type or types must be specified")) + } + + // When distributing the Root volumes across multiple failure domains, we suggest using multiple Storage types so they use a different backend. + // Storage availability zones are purely cosmetic for now and can also be used to define where a volume should be created, however + // we don't want to force a user to define a Storage availability zone when using multiple Storage types. + // Therefore we decided to require as many Storage types as there are Compute availability zones, if there are multiple Compute availability zones + // and more than one Storage type is defined. + // Even if we support a single Storage type across multiple failure domains, we still allow doing it. + // e.g. it would not make sense to have 3 Compute availability zones and 2 Storage types, because one of the Storage types would be used twice and + // therefore the number of failure domains would not be 3 anymore. + if machinePool.RootVolume.Types != nil { + if computes, volumes := len(machinePool.Zones), len(machinePool.RootVolume.Types); computes > 1 && volumes > 1 && volumes != computes { + errs = append(errs, field.Invalid(typesPath, rootVolumeTypes, "Compute and Storage availability zones in a MachinePool should have been validated to have equal length when more than one Storage type is defined")) + } + } } return errs diff --git a/pkg/types/openstack/validation/machinepool_test.go b/pkg/types/openstack/validation/machinepool_test.go index 1e1820455f7..d004559d772 100644 --- a/pkg/types/openstack/validation/machinepool_test.go +++ b/pkg/types/openstack/validation/machinepool_test.go @@ -13,11 +13,11 @@ func withServerGroupPolicy(serverGroupPolicy string) func(*openstack.MachinePool return func(mp *openstack.MachinePool) { mp.ServerGroupPolicy = openstack.ServerGroupPolicy(serverGroupPolicy) } } -func withRootVolume(rootVolume openstack.RootVolume) func(*openstack.MachinePool) { - return func(mp *openstack.MachinePool) { mp.RootVolume = &rootVolume } +func withRootVolume(rootVolume *openstack.RootVolume) func(*openstack.MachinePool) { + return func(mp *openstack.MachinePool) { mp.RootVolume = rootVolume } } -func withAvailabilityZones(zones []string) func(*openstack.MachinePool) { +func withAvailabilityZone(zones []string) func(*openstack.MachinePool) { return func(mp *openstack.MachinePool) { mp.Zones = zones } } @@ -80,14 +80,65 @@ func TestValidateMachinePool(t *testing.T) { ), }, { - "with availability zone and valid root volume", - testMachinePool(withAvailabilityZones([]string{"az0", "az1", "az2"}), withRootVolume(openstack.RootVolume{Zones: []string{"az0", "az1", "az2"}, Size: 100})), + "with no rootVolume type nor types", + testMachinePool( + withRootVolume(&openstack.RootVolume{ + Size: 10, + }), + ), + "default", + check( + someErrorType(field.ErrorTypeInvalid), + exactlyNErrors(2), + ), + }, + { + "with both rootVolume type and types", + testMachinePool( + withRootVolume(&openstack.RootVolume{ + Size: 10, + DeprecatedType: "fast", + Types: []string{"fast"}, + }), + ), + "default", + check( + someErrorType(field.ErrorTypeInvalid), + exactlyNErrors(2), + ), + }, + { + "with three compute zones and one root volume type", + testMachinePool( + withRootVolume(&openstack.RootVolume{ + Size: 10, + Types: []string{"fast"}, + Zones: []string{"az1", "az2", "az3"}, + }), + withAvailabilityZone([]string{"az1", "az2", "az3"}), + ), "default", check(noError), }, { - "with availability zone and invalid root volume missing zones", - testMachinePool(withAvailabilityZones([]string{"az0", "az1", "az2"}), withRootVolume(openstack.RootVolume{Size: 100})), + "with three compute zones and two root volume types", + testMachinePool( + withRootVolume(&openstack.RootVolume{ + Size: 10, + Types: []string{"fast", "slow"}, + Zones: []string{"az1", "az2", "az3"}, + }), + withAvailabilityZone([]string{"az1", "az2", "az3"}), + ), + "default", + check( + someErrorType(field.ErrorTypeInvalid), + exactlyNErrors(1), + ), + }, + { + "with three compute zones and invalid root volume missing zones", + testMachinePool(withAvailabilityZone([]string{"az0", "az1", "az2"}), withRootVolume(&openstack.RootVolume{Size: 100, Types: []string{"fast"}})), "default", check( someErrorType(field.ErrorTypeRequired), diff --git a/scripts/openstack/manifest-tests/root-volume-types/install-config.yaml b/scripts/openstack/manifest-tests/root-volume-types/install-config.yaml new file mode 100644 index 00000000000..67803846d93 --- /dev/null +++ b/scripts/openstack/manifest-tests/root-volume-types/install-config.yaml @@ -0,0 +1,40 @@ +apiVersion: v1 +baseDomain: shiftstack.example.com +controlPlane: + architecture: amd64 + name: master + platform: + openstack: + rootVolume: + size: 100 + types: ["type-1", "type-2", "type-3"] + zones: ["VolumeAZ1", "VolumeAZ2", "VolumeAZ3"] + type: ${COMPUTE_FLAVOR} + zones: ["MasterAZ1", "MasterAZ2", "MasterAZ3"] + replicas: 3 +compute: +- name: worker + platform: + openstack: + rootVolume: + size: 100 + types: ["type-A", "type-B", "type-C"] + type: ${COMPUTE_FLAVOR} + replicas: 1000 +metadata: + name: manifests1 +networking: + clusterNetwork: + - cidr: 10.128.0.0/14 + hostPrefix: 23 + machineNetwork: + - cidr: 10.0.128.0/17 + networkType: OVNKubernetes + serviceNetwork: + - 172.30.0.0/16 +platform: + openstack: + apiFloatingIP: ${API_FIP} + cloud: ${OS_CLOUD} + externalNetwork: ${EXTERNAL_NETWORK} +pullSecret: ${PULL_SECRET} diff --git a/scripts/openstack/manifest-tests/root-volume-types/test_cpms.py b/scripts/openstack/manifest-tests/root-volume-types/test_cpms.py new file mode 100755 index 00000000000..5a4e527c9f2 --- /dev/null +++ b/scripts/openstack/manifest-tests/root-volume-types/test_cpms.py @@ -0,0 +1,81 @@ +#!/usr/bin/env python +# -*- coding: utf-8 -*- + +import unittest +import xmlrunner + +import os +import sys +import yaml + +ASSETS_DIR = "" + +EXPECTED_MASTER_ZONE_NAMES = ["MasterAZ1", "MasterAZ2", "MasterAZ3"] +EXPECTED_MASTER_ROOT_VOLUME_TYPE_NAMES = ["type-1", "type-2", "type-3"] +EXPECTED_MASTER_ROOT_VOLUME_ZONE_NAMES = ["VolumeAZ1", "VolumeAZ2", "VolumeAZ3"] + +class ControlPlaneMachineSet(unittest.TestCase): + def setUp(self): + """Parse the CPMS into a Python data structure.""" + with open(f'{ASSETS_DIR}/openshift/99_openshift-machine-api_master-control-plane-machine-set.yaml') as f: + self.cpms = yaml.load(f, Loader=yaml.FullLoader) + + def test_providerspec_failuredomain_fields(self): + """Assert that the failure-domain-managed fields in the CPMS providerSpec are omitted.""" + provider_spec = self.cpms["spec"]["template"]["machines_v1beta1_machine_openshift_io"]["spec"]["providerSpec"]["value"] + self.assertNotIn("availabilityZone", provider_spec) + self.assertNotIn("availabilityZone", provider_spec["rootVolume"]) + + def test_compute_zones(self): + """Assert that the CPMS failure domain zones match the expected machine-pool zones.""" + self.assertIn("failureDomains", self.cpms["spec"]["template"]["machines_v1beta1_machine_openshift_io"]) + failure_domains = self.cpms["spec"]["template"]["machines_v1beta1_machine_openshift_io"]["failureDomains"]["openstack"] + + compute_zones = [] + for failure_domain in failure_domains: + zone = failure_domain["availabilityZone"] + compute_zones.append(zone) + self.assertIn(zone, EXPECTED_MASTER_ZONE_NAMES) + + for expected_zone in EXPECTED_MASTER_ZONE_NAMES: + self.assertIn(expected_zone, compute_zones) + + def test_storage_types(self): + """Assert that the CPMS storage types match the expected machine-pool storage types.""" + self.assertIn("failureDomains", self.cpms["spec"]["template"]["machines_v1beta1_machine_openshift_io"]) + self.assertIn("openstack", self.cpms["spec"]["template"]["machines_v1beta1_machine_openshift_io"]["failureDomains"]) + failure_domains = self.cpms["spec"]["template"]["machines_v1beta1_machine_openshift_io"]["failureDomains"]["openstack"] + + storage_types = [] + for failure_domain in failure_domains: + self.assertIn("rootVolume", failure_domain) + self.assertIn("volumeType", failure_domain["rootVolume"]) + storage_type = failure_domain["rootVolume"]["volumeType"] + storage_types.append(storage_type) + self.assertIn(storage_type, EXPECTED_MASTER_ROOT_VOLUME_TYPE_NAMES) + + for expected_type in EXPECTED_MASTER_ROOT_VOLUME_TYPE_NAMES: + self.assertIn(expected_type, storage_types) + + def test_storage_zones(self): + """Assert that the CPMS failure domain root volume zones match the expected machine-pool zones.""" + self.assertIn("failureDomains", self.cpms["spec"]["template"]["machines_v1beta1_machine_openshift_io"]) + self.assertIn("openstack", self.cpms["spec"]["template"]["machines_v1beta1_machine_openshift_io"]["failureDomains"]) + failure_domains = self.cpms["spec"]["template"]["machines_v1beta1_machine_openshift_io"]["failureDomains"]["openstack"] + + storage_zones = [] + for failure_domain in failure_domains: + self.assertIn("rootVolume", failure_domain) + self.assertIn("availabilityZone", failure_domain["rootVolume"]) + rootVolumeZone = failure_domain["rootVolume"]["availabilityZone"] + storage_zones.append(rootVolumeZone) + self.assertIn(rootVolumeZone, EXPECTED_MASTER_ROOT_VOLUME_ZONE_NAMES) + + for expected_zone in EXPECTED_MASTER_ROOT_VOLUME_ZONE_NAMES: + self.assertIn(expected_zone, storage_zones) + + +if __name__ == '__main__': + ASSETS_DIR = sys.argv.pop() + with open(os.environ.get('JUNIT_FILE', '/dev/null'), 'wb') as output: + unittest.main(testRunner=xmlrunner.XMLTestRunner(output=output), failfast=False, buffer=False, catchbreak=False, verbosity=2) diff --git a/scripts/openstack/manifest-tests/root-volume-types/test_machines.py b/scripts/openstack/manifest-tests/root-volume-types/test_machines.py new file mode 100755 index 00000000000..70cc24d795f --- /dev/null +++ b/scripts/openstack/manifest-tests/root-volume-types/test_machines.py @@ -0,0 +1,119 @@ +#!/usr/bin/env python +# -*- coding: utf-8 -*- + +import unittest +import xmlrunner + +import os +import sys +import glob +import yaml + +ASSETS_DIR = "" + +EXPECTED_MASTER_REPLICAS = 3 +EXPECTED_MASTER_ROOT_VOLUME_TYPE_NAMES = ["type-1", "type-2", "type-3"] +EXPECTED_MASTER_ROOT_VOLUME_ZONE_NAMES = ["VolumeAZ1", "VolumeAZ2", "VolumeAZ3"] +EXPECTED_MASTER_ZONE_NAMES = ["MasterAZ1", "MasterAZ2", "MasterAZ3"] + +EXPECTED_WORKER_REPLICAS = 1000 +EXPECTED_WORKER_ROOT_VOLUME_TYPES = ["type-A", "type-B", "type-C"] + + + +class RootVolumeTypeMachines(unittest.TestCase): + def setUp(self): + """Parse the Machines into a Python data structure.""" + self.machines = [] + for machine_path in glob.glob( + f'{ASSETS_DIR}/openshift/99_openshift-cluster-api_master-machines-*.yaml' + ): + with open(machine_path) as f: + self.machines.append(yaml.load(f, Loader=yaml.FullLoader)) + + def test_zone_names(self): + """Assert that all machines have one valid compute az that matches volume az.""" + for machine in self.machines: + master_zone = machine["spec"]["providerSpec"]["value"]["availabilityZone"] + volume_zone = machine["spec"]["providerSpec"]["value"]["rootVolume"]["availabilityZone"] + self.assertIn(master_zone, EXPECTED_MASTER_ZONE_NAMES) + self.assertIn(volume_zone, EXPECTED_MASTER_ROOT_VOLUME_ZONE_NAMES) + self.assertEqual(master_zone[-3:], volume_zone[-3:]) + + def test_type_names(self): + """Assert that all machines have one valid volume type.""" + for machine in self.machines: + volume_type = machine["spec"]["providerSpec"]["value"]["rootVolume"]["volumeType"] + self.assertIn(volume_type, EXPECTED_MASTER_ROOT_VOLUME_TYPE_NAMES) + + def test_total_instance_number(self): + """Assert that there are as many Machines as required ControlPlane replicas.""" + self.assertEqual(len(self.machines), EXPECTED_MASTER_REPLICAS) + + def test_replica_distribution(self): + """Assert that machines are evenly distributed across failure domains.""" + volume_zones = {} + volume_types = {} + for machine in self.machines: + volume_zone = machine["spec"]["providerSpec"]["value"]["rootVolume"]["availabilityZone"] + volume_zones[volume_zone] = volume_zones.get(volume_zone, 0) + 1 + volume_type = machine["spec"]["providerSpec"]["value"]["rootVolume"]["volumeType"] + volume_types[volume_type] = volume_types.get(volume_zone, 0) + 1 + + setpoint = 0 + for replicas in volume_zones.values(): + if setpoint == 0: + setpoint = replicas + else: + self.assertTrue(-2 < replicas - setpoint < 2, msg="volume zone mismatch") + + setpoint = 0 + for replicas in volume_types.values(): + if setpoint == 0: + setpoint = replicas + else: + self.assertTrue(-2 < replicas - setpoint < 2, msg="volume type mismatch") + + +class RootVolumeTypesMachinesets(unittest.TestCase): + def setUp(self): + """Parse the MachineSets into a Python data structure.""" + self.machinesets = [] + for machineset_path in glob.glob( + f'{ASSETS_DIR}/openshift/99_openshift-cluster-api_worker-machineset-*.yaml' + ): + with open(machineset_path) as f: + self.machinesets.append(yaml.load(f, Loader=yaml.FullLoader)) + + def test_machineset_zone_name(self): + """Assert that there are as many MachineSets as failure domains.""" + self.assertEqual(len(self.machinesets), len(EXPECTED_WORKER_ROOT_VOLUME_TYPES)) + + def test_machineset_volume_type_name(self): + """Assert that each MachineSet has a valid volume type.""" + for machineset in self.machinesets: + volume_type = machineset["spec"]["template"]["spec"]["providerSpec"]["value"]["rootVolume"]["volumeType"] + self.assertIn(volume_type, EXPECTED_WORKER_ROOT_VOLUME_TYPES) + + def test_total_replica_number(self): + """Assert that replicas spread across the MachineSets add up to the expected number.""" + total_found = 0 + for machineset in self.machinesets: + total_found += machineset["spec"]["replicas"] + self.assertEqual(total_found, EXPECTED_WORKER_REPLICAS) + + def test_replica_distribution(self): + """Assert that MachineSets are evenly distributed across failure domains.""" + setpoint = 0 + for machineset in self.machinesets: + replicas = machineset["spec"]["replicas"] + if setpoint == 0: + setpoint = replicas + else: + self.assertTrue(-2 < replicas - setpoint < 2) + + +if __name__ == '__main__': + ASSETS_DIR = sys.argv.pop() + with open(os.environ.get('JUNIT_FILE', '/dev/null'), 'wb') as output: + unittest.main(testRunner=xmlrunner.XMLTestRunner(output=output), failfast=False, buffer=False, catchbreak=False, verbosity=2)