From 625932f5c345e08f49581973bb88acb25d0bca10 Mon Sep 17 00:00:00 2001 From: Emilien Macchi Date: Fri, 27 Jan 2023 10:40:55 -0500 Subject: [PATCH] Support for External LB as Tech Preview --- .../install.openshift.io_installconfigs.yaml | 110 +++++++++++++++++ data/data/openstack/bootstrap/main.tf | 7 +- .../data/openstack/masters/private-network.tf | 24 ++-- data/data/openstack/variables-openstack.tf | 5 + pkg/asset/installconfig/ovirt/validation.go | 2 +- pkg/asset/installconfig/vsphere/validation.go | 2 +- pkg/asset/manifests/infrastructure.go | 6 + pkg/tfvars/openstack/openstack.go | 8 ++ pkg/types/baremetal/platform.go | 6 + pkg/types/baremetal/validation/platform.go | 24 ++++ .../baremetal/validation/platform_test.go | 46 ++++++- pkg/types/nutanix/platform.go | 9 ++ pkg/types/nutanix/validation/platform.go | 27 ++++- pkg/types/nutanix/validation/platform_test.go | 89 +++++++++++++- pkg/types/openstack/platform.go | 9 ++ pkg/types/openstack/validation/platform.go | 24 ++++ .../openstack/validation/platform_test.go | 104 ++++++++++++++-- pkg/types/ovirt/platform.go | 9 ++ pkg/types/ovirt/validation/platform.go | 27 ++++- pkg/types/ovirt/validation/platform_test.go | 114 ++++++++++++++++-- pkg/types/validation/installconfig.go | 6 +- pkg/types/vsphere/platform.go | 9 ++ pkg/types/vsphere/validation/platform.go | 28 ++++- pkg/types/vsphere/validation/platform_test.go | 102 ++++++++++++++-- .../default-stable-lb/install-config.yaml | 31 +++++ .../default-stable-lb/test_cluster-infra.py | 27 +++++ .../install-config.yaml | 32 +++++ .../test_cluster-infra.py | 27 +++++ .../openshift-managed-lb/install-config.yaml | 34 ++++++ .../test_cluster-infra.py | 32 +++++ .../user-managed-lb/install-config.yaml | 34 ++++++ .../user-managed-lb/test_cluster-infra.py | 32 +++++ 32 files changed, 997 insertions(+), 49 deletions(-) create mode 100644 scripts/openstack/manifest-tests/default-stable-lb/install-config.yaml create mode 100755 scripts/openstack/manifest-tests/default-stable-lb/test_cluster-infra.py create mode 100644 scripts/openstack/manifest-tests/default-techpreview-lb/install-config.yaml create mode 100755 scripts/openstack/manifest-tests/default-techpreview-lb/test_cluster-infra.py create mode 100644 scripts/openstack/manifest-tests/openshift-managed-lb/install-config.yaml create mode 100755 scripts/openstack/manifest-tests/openshift-managed-lb/test_cluster-infra.py create mode 100644 scripts/openstack/manifest-tests/user-managed-lb/install-config.yaml create mode 100755 scripts/openstack/manifest-tests/user-managed-lb/test_cluster-infra.py diff --git a/data/data/install.openshift.io_installconfigs.yaml b/data/data/install.openshift.io_installconfigs.yaml index a097707ea2..21cc7f23d9 100644 --- a/data/data/install.openshift.io_installconfigs.yaml +++ b/data/data/install.openshift.io_installconfigs.yaml @@ -2225,6 +2225,28 @@ spec: must be reachable from the host where the installer is run. Default is qemu:///system type: string + loadBalancer: + description: LoadBalancer defines how the load balancer used by + the cluster is configured. LoadBalancer is available in TechPreview. + properties: + type: + default: OpenShiftManagedDefault + description: type defines the type of load balancer used by + the cluster on BareMetal platform which can be a user-managed + or openshift-managed load balancer that is to be used for + the OpenShift API and Ingress endpoints. When set to OpenShiftManagedDefault + the static pods in charge of API and Ingress traffic load-balancing + defined in the machine config operator will be deployed. + When set to UserManaged these static pods will not be deployed + and it is expected that the load balancer is configured + out of band by the deployer. When omitted, this means no + opinion and the platform is left to choose a reasonable + default. The default value is OpenShiftManagedDefault. + enum: + - OpenShiftManagedDefault + - UserManaged + type: string + type: object provisioningBridge: description: Provisioning bridge is used for provisioning nodes, on the host that will run the bootstrap VM. @@ -2627,6 +2649,28 @@ spec: maxItems: 2 type: array uniqueItems: true + loadBalancer: + description: LoadBalancer defines how the load balancer used by + the cluster is configured. LoadBalancer is available in TechPreview. + properties: + type: + default: OpenShiftManagedDefault + description: type defines the type of load balancer used by + the cluster on Nutanix platform which can be a user-managed + or openshift-managed load balancer that is to be used for + the OpenShift API and Ingress endpoints. When set to OpenShiftManagedDefault + the static pods in charge of API and Ingress traffic load-balancing + defined in the machine config operator will be deployed. + When set to UserManaged these static pods will not be deployed + and it is expected that the load balancer is configured + out of band by the deployer. When omitted, this means no + opinion and the platform is left to choose a reasonable + default. The default value is OpenShiftManagedDefault. + enum: + - OpenShiftManagedDefault + - UserManaged + type: string + type: object prismCentral: description: PrismCentral is the endpoint (address and port) and credentials to connect to the Prism Central. @@ -2871,6 +2915,28 @@ spec: IP in your OpenStack cluster to associate with the OpenShift load balancer. Deprecated: this value has been renamed to apiFloatingIP.' type: string + loadBalancer: + description: LoadBalancer defines how the load balancer used by + the cluster is configured. LoadBalancer is available in TechPreview. + properties: + type: + default: OpenShiftManagedDefault + description: type defines the type of load balancer used by + the cluster on OpenStack platform which can be a user-managed + or openshift-managed load balancer that is to be used for + the OpenShift API and Ingress endpoints. When set to OpenShiftManagedDefault + the static pods in charge of API and Ingress traffic load-balancing + defined in the machine config operator will be deployed. + When set to UserManaged these static pods will not be deployed + and it is expected that the load balancer is configured + out of band by the deployer. When omitted, this means no + opinion and the platform is left to choose a reasonable + default. The default value is OpenShiftManagedDefault. + enum: + - OpenShiftManagedDefault + - UserManaged + type: string + type: object machinesSubnet: description: MachinesSubnet is the UUIDv4 of an openstack subnet. This subnet will be used by all nodes created by the installer. @@ -3065,6 +3131,28 @@ spec: maxItems: 2 type: array uniqueItems: true + loadBalancer: + description: LoadBalancer defines how the load balancer used by + the cluster is configured. LoadBalancer is available in TechPreview. + properties: + type: + default: OpenShiftManagedDefault + description: type defines the type of load balancer used by + the cluster on Ovirt platform which can be a user-managed + or openshift-managed load balancer that is to be used for + the OpenShift API and Ingress endpoints. When set to OpenShiftManagedDefault + the static pods in charge of API and Ingress traffic load-balancing + defined in the machine config operator will be deployed. + When set to UserManaged these static pods will not be deployed + and it is expected that the load balancer is configured + out of band by the deployer. When omitted, this means no + opinion and the platform is left to choose a reasonable + default. The default value is OpenShiftManagedDefault. + enum: + - OpenShiftManagedDefault + - UserManaged + type: string + type: object ovirt_cluster_id: description: The target cluster under which all VMs will run type: string @@ -3372,6 +3460,28 @@ spec: maxItems: 2 type: array uniqueItems: true + loadBalancer: + description: LoadBalancer defines how the load balancer used by + the cluster is configured. LoadBalancer is available in TechPreview. + properties: + type: + default: OpenShiftManagedDefault + description: type defines the type of load balancer used by + the cluster on VSphere platform which can be a user-managed + or openshift-managed load balancer that is to be used for + the OpenShift API and Ingress endpoints. When set to OpenShiftManagedDefault + the static pods in charge of API and Ingress traffic load-balancing + defined in the machine config operator will be deployed. + When set to UserManaged these static pods will not be deployed + and it is expected that the load balancer is configured + out of band by the deployer. When omitted, this means no + opinion and the platform is left to choose a reasonable + default. The default value is OpenShiftManagedDefault. + enum: + - OpenShiftManagedDefault + - UserManaged + type: string + type: object network: description: 'Network specifies the name of the network to be used by the cluster. Deprecated: Use FailureDomains.Topology.Network' diff --git a/data/data/openstack/bootstrap/main.tf b/data/data/openstack/bootstrap/main.tf index 667a2848e4..4818a4634d 100644 --- a/data/data/openstack/bootstrap/main.tf +++ b/data/data/openstack/bootstrap/main.tf @@ -52,8 +52,11 @@ resource "openstack_networking_port_v2" "bootstrap_port" { subnet_id = var.nodes_subnet_id } - allowed_address_pairs { - ip_address = var.openstack_api_int_ip + dynamic "allowed_address_pairs" { + for_each = var.openstack_user_managed_load_balancer ? [] : [1] + content { + ip_address = var.openstack_api_int_ip + } } depends_on = [var.master_port_ids] diff --git a/data/data/openstack/masters/private-network.tf b/data/data/openstack/masters/private-network.tf index 2e108673f4..20c8cb16ca 100644 --- a/data/data/openstack/masters/private-network.tf +++ b/data/data/openstack/masters/private-network.tf @@ -62,18 +62,25 @@ resource "openstack_networking_port_v2" "masters" { subnet_id = local.nodes_subnet_id } - allowed_address_pairs { - ip_address = var.openstack_api_int_ip + dynamic "allowed_address_pairs" { + for_each = var.openstack_user_managed_load_balancer ? [] : [1] + content { + ip_address = var.openstack_api_int_ip + } } - allowed_address_pairs { - ip_address = var.openstack_ingress_ip + dynamic "allowed_address_pairs" { + for_each = var.openstack_user_managed_load_balancer ? [] : [1] + content { + ip_address = var.openstack_ingress_ip + } } depends_on = [openstack_networking_port_v2.api_port, openstack_networking_port_v2.ingress_port] } resource "openstack_networking_port_v2" "api_port" { + count = var.openstack_user_managed_load_balancer ? 0 : 1 name = "${var.cluster_id}-api-port" description = local.description @@ -89,6 +96,7 @@ resource "openstack_networking_port_v2" "api_port" { } resource "openstack_networking_port_v2" "ingress_port" { + count = var.openstack_user_managed_load_balancer ? 0 : 1 name = "${var.cluster_id}-ingress-port" description = local.description @@ -134,15 +142,15 @@ resource "openstack_networking_trunk_v2" "masters" { // as expected. resource "openstack_networking_floatingip_associate_v2" "api_fip" { - count = length(var.openstack_api_floating_ip) == 0 ? 0 : 1 - port_id = openstack_networking_port_v2.api_port.id + count = (var.openstack_user_managed_load_balancer || length(var.openstack_api_floating_ip) == 0) ? 0 : 1 + port_id = openstack_networking_port_v2.api_port[0].id floating_ip = var.openstack_api_floating_ip depends_on = [openstack_networking_router_interface_v2.nodes_router_interface] } resource "openstack_networking_floatingip_associate_v2" "ingress_fip" { - count = length(var.openstack_ingress_floating_ip) == 0 ? 0 : 1 - port_id = openstack_networking_port_v2.ingress_port.id + count = (var.openstack_user_managed_load_balancer || length(var.openstack_ingress_floating_ip) == 0) ? 0 : 1 + port_id = openstack_networking_port_v2.ingress_port[0].id floating_ip = var.openstack_ingress_floating_ip depends_on = [openstack_networking_router_interface_v2.nodes_router_interface] } diff --git a/data/data/openstack/variables-openstack.tf b/data/data/openstack/variables-openstack.tf index 20b52125c9..6dc27cbd93 100644 --- a/data/data/openstack/variables-openstack.tf +++ b/data/data/openstack/variables-openstack.tf @@ -374,3 +374,8 @@ variable "openstack_worker_server_group_policy" { type = string description = "Policy of the server groups for the worker nodes." } + +variable "openstack_user_managed_load_balancer" { + type = bool + description = "True if the load balancer that is used for the control plane VIPs is managed by the user." +} diff --git a/pkg/asset/installconfig/ovirt/validation.go b/pkg/asset/installconfig/ovirt/validation.go index 57096a168a..60145adcd8 100644 --- a/pkg/asset/installconfig/ovirt/validation.go +++ b/pkg/asset/installconfig/ovirt/validation.go @@ -27,7 +27,7 @@ func Validate(ic *types.InstallConfig) error { allErrs = append( allErrs, - validation.ValidatePlatform(ic.Platform.Ovirt, ovirtPlatformPath)...) + validation.ValidatePlatform(ic.Platform.Ovirt, ovirtPlatformPath, ic)...) con, err := NewConnection() if err != nil { diff --git a/pkg/asset/installconfig/vsphere/validation.go b/pkg/asset/installconfig/vsphere/validation.go index c4b3cb9ce8..2292facd37 100644 --- a/pkg/asset/installconfig/vsphere/validation.go +++ b/pkg/asset/installconfig/vsphere/validation.go @@ -59,7 +59,7 @@ func Validate(ic *types.InstallConfig) error { if ic.Platform.VSphere == nil { return errors.New(field.Required(field.NewPath("platform", "vsphere"), "vSphere validation requires a vSphere platform configuration").Error()) } - return validation.ValidatePlatform(ic.Platform.VSphere, field.NewPath("platform").Child("vsphere")).ToAggregate() + return validation.ValidatePlatform(ic.Platform.VSphere, field.NewPath("platform").Child("vsphere"), ic).ToAggregate() } func getVCenterClient(failureDomain vsphere.FailureDomain, ic *types.InstallConfig) (*validationContext, ClientLogout, error) { diff --git a/pkg/asset/manifests/infrastructure.go b/pkg/asset/manifests/infrastructure.go index 278863bdd0..bc81c8923c 100644 --- a/pkg/asset/manifests/infrastructure.go +++ b/pkg/asset/manifests/infrastructure.go @@ -160,6 +160,7 @@ func (i *Infrastructure) Generate(dependencies asset.Parents) error { IngressIP: installConfig.Config.Platform.BareMetal.IngressVIPs[0], APIServerInternalIPs: installConfig.Config.Platform.BareMetal.APIVIPs, IngressIPs: installConfig.Config.Platform.BareMetal.IngressVIPs, + LoadBalancer: installConfig.Config.Platform.BareMetal.LoadBalancer, } case gcp.Name: config.Spec.PlatformSpec.Type = configv1.GCPPlatformType @@ -210,6 +211,7 @@ func (i *Infrastructure) Generate(dependencies asset.Parents) error { IngressIP: installConfig.Config.OpenStack.IngressVIPs[0], APIServerInternalIPs: installConfig.Config.OpenStack.APIVIPs, IngressIPs: installConfig.Config.OpenStack.IngressVIPs, + LoadBalancer: installConfig.Config.OpenStack.LoadBalancer, } case vsphere.Name: config.Spec.PlatformSpec.Type = configv1.VSpherePlatformType @@ -219,8 +221,10 @@ func (i *Infrastructure) Generate(dependencies asset.Parents) error { IngressIP: installConfig.Config.VSphere.IngressVIPs[0], APIServerInternalIPs: installConfig.Config.VSphere.APIVIPs, IngressIPs: installConfig.Config.VSphere.IngressVIPs, + LoadBalancer: installConfig.Config.VSphere.LoadBalancer, } } + config.Spec.PlatformSpec.VSphere = vsphereinfra.GetInfraPlatformSpec(installConfig) if _, exists := cloudproviderconfig.ConfigMap.Data["vsphere.conf"]; exists { @@ -234,6 +238,7 @@ func (i *Infrastructure) Generate(dependencies asset.Parents) error { IngressIP: installConfig.Config.Ovirt.IngressVIPs[0], APIServerInternalIPs: installConfig.Config.Ovirt.APIVIPs, IngressIPs: installConfig.Config.Ovirt.IngressVIPs, + LoadBalancer: installConfig.Config.Ovirt.LoadBalancer, } case powervs.Name: config.Spec.PlatformSpec.Type = configv1.PowerVSPlatformType @@ -303,6 +308,7 @@ func (i *Infrastructure) Generate(dependencies asset.Parents) error { IngressIP: installConfig.Config.Nutanix.IngressVIPs[0], APIServerInternalIPs: installConfig.Config.Nutanix.APIVIPs, IngressIPs: installConfig.Config.Nutanix.IngressVIPs, + LoadBalancer: installConfig.Config.Nutanix.LoadBalancer, } } default: diff --git a/pkg/tfvars/openstack/openstack.go b/pkg/tfvars/openstack/openstack.go index c2e78c28af..5d494d9971 100644 --- a/pkg/tfvars/openstack/openstack.go +++ b/pkg/tfvars/openstack/openstack.go @@ -12,6 +12,7 @@ import ( "github.com/gophercloud/gophercloud/openstack/networking/v2/subnets" "github.com/gophercloud/utils/openstack/clientconfig" + configv1 "github.com/openshift/api/config/v1" machinev1alpha1 "github.com/openshift/api/machine/v1alpha1" "github.com/openshift/installer/pkg/asset/installconfig" installconfig_openstack "github.com/openshift/installer/pkg/asset/installconfig/openstack" @@ -95,6 +96,11 @@ func TFVars( workermpool = installConfig.Config.Compute[0].Platform.OpenStack } + var userManagedLoadBalancer bool + if lb := installConfig.Config.Platform.OpenStack.LoadBalancer; lb != nil && lb.Type == configv1.LoadBalancerTypeUserManaged { + userManagedLoadBalancer = true + } + var zones []string { seen := make(map[string]struct{}) @@ -211,6 +217,7 @@ func TFVars( MachinesNetwork string `json:"openstack_machines_network_id,omitempty"` MasterAvailabilityZones []string `json:"openstack_master_availability_zones,omitempty"` MasterRootVolumeAvailabilityZones []string `json:"openstack_master_root_volume_availability_zones,omitempty"` + UserManagedLoadBalancer bool `json:"openstack_user_managed_load_balancer"` }{ BaseImageName: imageName, ExternalNetwork: installConfig.Config.Platform.OpenStack.ExternalNetwork, @@ -236,6 +243,7 @@ func TFVars( MachinesNetwork: machinesNetwork, MasterAvailabilityZones: zones, MasterRootVolumeAvailabilityZones: masterRootVolumeAvailabilityZones, + UserManagedLoadBalancer: userManagedLoadBalancer, }, "", " ") } diff --git a/pkg/types/baremetal/platform.go b/pkg/types/baremetal/platform.go index dd0827a151..4da227dccb 100644 --- a/pkg/types/baremetal/platform.go +++ b/pkg/types/baremetal/platform.go @@ -3,6 +3,7 @@ package baremetal import ( apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + configv1 "github.com/openshift/api/config/v1" "github.com/openshift/installer/pkg/ipnet" ) @@ -226,4 +227,9 @@ type Platform struct { // +kubebuilder:validation:Format=ip // +optional BootstrapExternalStaticGateway string `json:"bootstrapExternalStaticGateway,omitempty"` + + // LoadBalancer defines how the load balancer used by the cluster is configured. + // LoadBalancer is available in TechPreview. + // +optional + LoadBalancer *configv1.BareMetalPlatformLoadBalancer `json:"loadBalancer,omitempty"` } diff --git a/pkg/types/baremetal/validation/platform.go b/pkg/types/baremetal/validation/platform.go index eaa87a3057..b6c2e66a33 100644 --- a/pkg/types/baremetal/validation/platform.go +++ b/pkg/types/baremetal/validation/platform.go @@ -19,6 +19,7 @@ import ( "k8s.io/apimachinery/pkg/util/validation" "k8s.io/apimachinery/pkg/util/validation/field" + configv1 "github.com/openshift/api/config/v1" "github.com/openshift/installer/pkg/ipnet" "github.com/openshift/installer/pkg/types" "github.com/openshift/installer/pkg/types/baremetal" @@ -425,9 +426,32 @@ func ValidatePlatform(p *baremetal.Platform, n *types.Networking, fldPath *field allErrs = append(allErrs, validateHostsName(p.Hosts, fldPath.Child("Hosts"))...) } + // Platform fields only allowed in TechPreviewNoUpgrade + if c.FeatureSet != configv1.TechPreviewNoUpgrade { + if c.BareMetal.LoadBalancer != nil { + allErrs = append(allErrs, field.Forbidden(fldPath.Child("loadBalancer"), "load balancer is not supported in this feature set")) + } + } + + if c.BareMetal.LoadBalancer != nil { + if !validateLoadBalancer(c.BareMetal.LoadBalancer.Type) { + allErrs = append(allErrs, field.Invalid(fldPath.Child("loadBalancer", "type"), c.BareMetal.LoadBalancer.Type, "invalid load balancer type")) + } + } + return allErrs } +// validateLoadBalancer returns an error if the load balancer is not valid. +func validateLoadBalancer(lbType configv1.PlatformLoadBalancerType) bool { + switch lbType { + case configv1.LoadBalancerTypeOpenShiftManagedDefault, configv1.LoadBalancerTypeUserManaged: + return true + default: + return false + } +} + // ValidateProvisioning checks that provisioning network requirements specified is valid. func ValidateProvisioning(p *baremetal.Platform, n *types.Networking, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} diff --git a/pkg/types/baremetal/validation/platform_test.go b/pkg/types/baremetal/validation/platform_test.go index 02fd1c69de..bcf22d9069 100644 --- a/pkg/types/baremetal/validation/platform_test.go +++ b/pkg/types/baremetal/validation/platform_test.go @@ -10,6 +10,7 @@ import ( "github.com/stretchr/testify/assert" "k8s.io/apimachinery/pkg/util/validation/field" + configv1 "github.com/openshift/api/config/v1" "github.com/openshift/installer/pkg/ipnet" "github.com/openshift/installer/pkg/types" "github.com/openshift/installer/pkg/types/baremetal" @@ -180,6 +181,35 @@ func TestValidatePlatform(t *testing.T) { ControlPlane(machinePool().Replicas(1)).build(), expected: "baremetal.hosts\\[0\\].Name: Required value: missing Name", }, + { + name: "forbidden_feature_loadbalancer", + config: installConfig(). + BareMetalPlatform( + platform().LoadBalancerType("OpenShiftManagedDefault")).build(), + expected: "baremetal.loadBalancer: Forbidden: load balancer is not supported in this feature set", + }, + { + name: "allowed_feature_loadbalancer_openshift_managed_default", + config: installConfig(). + BareMetalPlatform( + platform().LoadBalancerType("OpenShiftManagedDefault")). + FeatureSet(configv1.TechPreviewNoUpgrade).build(), + }, + { + name: "allowed_feature_loadbalancer_user_managed", + config: installConfig(). + BareMetalPlatform( + platform().LoadBalancerType("UserManaged")). + FeatureSet(configv1.TechPreviewNoUpgrade).build(), + }, + { + name: "allowed_feature_loadbalancer_invalid", + config: installConfig(). + BareMetalPlatform( + platform().LoadBalancerType("FooBar")). + FeatureSet(configv1.TechPreviewNoUpgrade).build(), + expected: "baremetal.loadBalancer.type: Invalid value: \"FooBar\": invalid load balancer type", + }, { name: "missing_mac", platform: platform(). @@ -312,7 +342,7 @@ interfaces: for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - //Build default wrapping installConfig + // Build default wrapping installConfig if tc.config == nil { tc.config = installConfig().build() tc.config.BareMetal = tc.platform @@ -656,7 +686,7 @@ func TestValidateProvisioning(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - //Build default wrapping installConfig + // Build default wrapping installConfig if tc.config == nil { tc.config = installConfig().build() } @@ -861,6 +891,13 @@ func (pb *platformBuilder) Hosts(builders ...*hostBuilder) *platformBuilder { return pb } +func (pb *platformBuilder) LoadBalancerType(value string) *platformBuilder { + pb.Platform.LoadBalancer = &configv1.BareMetalPlatformLoadBalancer{ + Type: configv1.PlatformLoadBalancerType(value), + } + return pb +} + func (pb *platformBuilder) LibvirtURI(value string) *platformBuilder { pb.Platform.LibvirtURI = value return pb @@ -916,6 +953,11 @@ func (icb *installConfigBuilder) BareMetalPlatform(builder *platformBuilder) *in return icb } +func (icb *installConfigBuilder) FeatureSet(value configv1.FeatureSet) *installConfigBuilder { + icb.InstallConfig.FeatureSet = value + return icb +} + func (icb *installConfigBuilder) ControlPlane(builder *machinePoolBuilder) *installConfigBuilder { icb.InstallConfig.ControlPlane = builder.build() diff --git a/pkg/types/nutanix/platform.go b/pkg/types/nutanix/platform.go index 897d1c29ad..ff86e0e94d 100644 --- a/pkg/types/nutanix/platform.go +++ b/pkg/types/nutanix/platform.go @@ -1,5 +1,9 @@ package nutanix +import ( + configv1 "github.com/openshift/api/config/v1" +) + // Platform stores any global configuration used for Nutanix platforms. type Platform struct { // PrismCentral is the endpoint (address and port) and credentials to @@ -56,6 +60,11 @@ type Platform struct { // SubnetUUIDs identifies the network subnets to be used by the cluster. // Currently we only support one subnet for an OpenShift cluster. SubnetUUIDs []string `json:"subnetUUIDs"` + + // LoadBalancer defines how the load balancer used by the cluster is configured. + // LoadBalancer is available in TechPreview. + // +optional + LoadBalancer *configv1.NutanixPlatformLoadBalancer `json:"loadBalancer,omitempty"` } // PrismCentral holds the endpoint and credentials data used to connect to the Prism Central diff --git a/pkg/types/nutanix/validation/platform.go b/pkg/types/nutanix/validation/platform.go index 73975c339c..250e05145a 100644 --- a/pkg/types/nutanix/validation/platform.go +++ b/pkg/types/nutanix/validation/platform.go @@ -3,13 +3,15 @@ package validation import ( "k8s.io/apimachinery/pkg/util/validation/field" + configv1 "github.com/openshift/api/config/v1" + "github.com/openshift/installer/pkg/types" "github.com/openshift/installer/pkg/types/nutanix" "github.com/openshift/installer/pkg/validate" ) // ValidatePlatform checks that the specified platform is valid. // TODO(nutanix): Revisit for further expanding the validation logic -func ValidatePlatform(p *nutanix.Platform, fldPath *field.Path) field.ErrorList { +func ValidatePlatform(p *nutanix.Platform, fldPath *field.Path, c *types.InstallConfig) field.ErrorList { allErrs := field.ErrorList{} if len(p.PrismCentral.Endpoint.Address) == 0 { @@ -69,5 +71,28 @@ func ValidatePlatform(p *nutanix.Platform, fldPath *field.Path) field.ErrorList allErrs = append(allErrs, field.Required(fldPath.Child("subnet"), "must specify the subnet")) } + // Platform fields only allowed in TechPreviewNoUpgrade + if c.FeatureSet != configv1.TechPreviewNoUpgrade { + if c.Nutanix.LoadBalancer != nil { + allErrs = append(allErrs, field.Forbidden(fldPath.Child("loadBalancer"), "load balancer is not supported in this feature set")) + } + } + + if c.Nutanix.LoadBalancer != nil { + if !validateLoadBalancer(c.Nutanix.LoadBalancer.Type) { + allErrs = append(allErrs, field.Invalid(fldPath.Child("loadBalancer", "type"), c.Nutanix.LoadBalancer.Type, "invalid load balancer type")) + } + } + return allErrs } + +// validateLoadBalancer returns an error if the load balancer is not valid. +func validateLoadBalancer(lbType configv1.PlatformLoadBalancerType) bool { + switch lbType { + case configv1.LoadBalancerTypeOpenShiftManagedDefault, configv1.LoadBalancerTypeUserManaged: + return true + default: + return false + } +} diff --git a/pkg/types/nutanix/validation/platform_test.go b/pkg/types/nutanix/validation/platform_test.go index e76f25dc16..e34f6963d8 100644 --- a/pkg/types/nutanix/validation/platform_test.go +++ b/pkg/types/nutanix/validation/platform_test.go @@ -6,6 +6,8 @@ import ( "github.com/stretchr/testify/assert" "k8s.io/apimachinery/pkg/util/validation/field" + configv1 "github.com/openshift/api/config/v1" + "github.com/openshift/installer/pkg/types" "github.com/openshift/installer/pkg/types/nutanix" ) @@ -27,6 +29,7 @@ func validPlatform() *nutanix.Platform { func TestValidatePlatform(t *testing.T) { cases := []struct { name string + config *types.InstallConfig platform *nutanix.Platform expectedError string }{ @@ -43,6 +46,71 @@ func TestValidatePlatform(t *testing.T) { }(), expectedError: `^test-path\.prismCentral\.endpoint\.address: Required value: must specify the Prism Central endpoint address$`, }, + { + name: "forbidden load balancer field", + platform: validPlatform(), + config: &types.InstallConfig{ + Platform: types.Platform{ + Nutanix: func() *nutanix.Platform { + p := validPlatform() + p.LoadBalancer = &configv1.NutanixPlatformLoadBalancer{ + Type: configv1.LoadBalancerTypeOpenShiftManagedDefault, + } + return p + }(), + }, + }, + expectedError: `^test-path\.loadBalancer: Forbidden: load balancer is not supported in this feature set`, + }, + { + name: "allowed load balancer field with OpenShift managed default", + platform: validPlatform(), + config: &types.InstallConfig{ + FeatureSet: configv1.TechPreviewNoUpgrade, + Platform: types.Platform{ + Nutanix: func() *nutanix.Platform { + p := validPlatform() + p.LoadBalancer = &configv1.NutanixPlatformLoadBalancer{ + Type: configv1.LoadBalancerTypeOpenShiftManagedDefault, + } + return p + }(), + }, + }, + }, + { + name: "allowed load balancer field with user-managed", + platform: validPlatform(), + config: &types.InstallConfig{ + FeatureSet: configv1.TechPreviewNoUpgrade, + Platform: types.Platform{ + Nutanix: func() *nutanix.Platform { + p := validPlatform() + p.LoadBalancer = &configv1.NutanixPlatformLoadBalancer{ + Type: configv1.LoadBalancerTypeUserManaged, + } + return p + }(), + }, + }, + }, + { + name: "allowed load balancer field invalid type", + platform: validPlatform(), + config: &types.InstallConfig{ + FeatureSet: configv1.TechPreviewNoUpgrade, + Platform: types.Platform{ + Nutanix: func() *nutanix.Platform { + p := validPlatform() + p.LoadBalancer = &configv1.NutanixPlatformLoadBalancer{ + Type: "FooBar", + } + return p + }(), + }, + }, + expectedError: `^test-path\.loadBalancer.type: Invalid value: "FooBar": invalid load balancer type`, + }, { name: "missing Prism Central username", platform: func() *nutanix.Platform { @@ -118,7 +186,12 @@ func TestValidatePlatform(t *testing.T) { } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - err := ValidatePlatform(tc.platform, field.NewPath("test-path")).ToAggregate() + // Build default wrapping installConfig + if tc.config == nil { + tc.config = installConfig().build() + tc.config.Nutanix = tc.platform + } + err := ValidatePlatform(tc.platform, field.NewPath("test-path"), tc.config).ToAggregate() if tc.expectedError == "" { assert.NoError(t, err) } else { @@ -127,3 +200,17 @@ func TestValidatePlatform(t *testing.T) { }) } } + +type installConfigBuilder struct { + types.InstallConfig +} + +func installConfig() *installConfigBuilder { + return &installConfigBuilder{ + InstallConfig: types.InstallConfig{}, + } +} + +func (icb *installConfigBuilder) build() *types.InstallConfig { + return &icb.InstallConfig +} diff --git a/pkg/types/openstack/platform.go b/pkg/types/openstack/platform.go index 9a864429c8..fe06190e55 100644 --- a/pkg/types/openstack/platform.go +++ b/pkg/types/openstack/platform.go @@ -1,5 +1,9 @@ package openstack +import ( + configv1 "github.com/openshift/api/config/v1" +) + // Platform stores all the global configuration that all // machinesets use. type Platform struct { @@ -114,4 +118,9 @@ type Platform struct { // The subnet and network specified in MachinesSubnet will not be deleted or modified by the installer. // +optional MachinesSubnet string `json:"machinesSubnet,omitempty"` + + // LoadBalancer defines how the load balancer used by the cluster is configured. + // LoadBalancer is available in TechPreview. + // +optional + LoadBalancer *configv1.OpenStackPlatformLoadBalancer `json:"loadBalancer,omitempty"` } diff --git a/pkg/types/openstack/validation/platform.go b/pkg/types/openstack/validation/platform.go index 82f73edfe0..87d7ee511b 100644 --- a/pkg/types/openstack/validation/platform.go +++ b/pkg/types/openstack/validation/platform.go @@ -3,6 +3,7 @@ package validation import ( "k8s.io/apimachinery/pkg/util/validation/field" + configv1 "github.com/openshift/api/config/v1" "github.com/openshift/installer/pkg/types" "github.com/openshift/installer/pkg/types/openstack" "github.com/openshift/installer/pkg/validate" @@ -20,5 +21,28 @@ func ValidatePlatform(p *openstack.Platform, n *types.Networking, fldPath *field allErrs = append(allErrs, ValidateMachinePool(p, p.DefaultMachinePlatform, "default", fldPath.Child("defaultMachinePlatform"))...) + // Platform fields only allowed in TechPreviewNoUpgrade + if c.FeatureSet != configv1.TechPreviewNoUpgrade { + if c.OpenStack.LoadBalancer != nil { + allErrs = append(allErrs, field.Forbidden(fldPath.Child("loadBalancer"), "load balancer is not supported in this feature set")) + } + } + + if c.OpenStack.LoadBalancer != nil { + if !validateLoadBalancer(c.OpenStack.LoadBalancer.Type) { + allErrs = append(allErrs, field.Invalid(fldPath.Child("loadBalancer", "type"), c.OpenStack.LoadBalancer.Type, "invalid load balancer type")) + } + } + return allErrs } + +// validateLoadBalancer returns an error if the load balancer is not valid. +func validateLoadBalancer(lbType configv1.PlatformLoadBalancerType) bool { + switch lbType { + case configv1.LoadBalancerTypeOpenShiftManagedDefault, configv1.LoadBalancerTypeUserManaged: + return true + default: + return false + } +} diff --git a/pkg/types/openstack/validation/platform_test.go b/pkg/types/openstack/validation/platform_test.go index dc11b2eca3..96bf2a2a57 100644 --- a/pkg/types/openstack/validation/platform_test.go +++ b/pkg/types/openstack/validation/platform_test.go @@ -6,6 +6,7 @@ import ( "github.com/stretchr/testify/assert" "k8s.io/apimachinery/pkg/util/validation/field" + configv1 "github.com/openshift/api/config/v1" "github.com/openshift/installer/pkg/ipnet" "github.com/openshift/installer/pkg/types" "github.com/openshift/installer/pkg/types/openstack" @@ -33,6 +34,7 @@ func validNetworking() *types.Networking { func TestValidatePlatform(t *testing.T) { cases := []struct { name string + config *types.InstallConfig platform *openstack.Platform networking *types.Networking noClouds bool @@ -41,6 +43,7 @@ func TestValidatePlatform(t *testing.T) { validMachinesSubnet bool invalidMachinesSubnet bool valid bool + expectedError string }{ { name: "minimal", @@ -48,6 +51,75 @@ func TestValidatePlatform(t *testing.T) { networking: validNetworking(), valid: true, }, + { + name: "forbidden load balancer field", + platform: validPlatform(), + config: &types.InstallConfig{ + Platform: types.Platform{ + OpenStack: func() *openstack.Platform { + p := validPlatform() + p.LoadBalancer = &configv1.OpenStackPlatformLoadBalancer{ + Type: configv1.LoadBalancerTypeOpenShiftManagedDefault, + } + return p + }(), + }, + }, + valid: false, + expectedError: `^test-path\.loadBalancer: Forbidden: load balancer is not supported in this feature set`, + }, + { + name: "allowed load balancer field with OpenShift managed default", + platform: validPlatform(), + config: &types.InstallConfig{ + FeatureSet: configv1.TechPreviewNoUpgrade, + Platform: types.Platform{ + OpenStack: func() *openstack.Platform { + p := validPlatform() + p.LoadBalancer = &configv1.OpenStackPlatformLoadBalancer{ + Type: configv1.LoadBalancerTypeOpenShiftManagedDefault, + } + return p + }(), + }, + }, + valid: true, + }, + { + name: "allowed load balancer field with user-managed", + platform: validPlatform(), + config: &types.InstallConfig{ + FeatureSet: configv1.TechPreviewNoUpgrade, + Platform: types.Platform{ + OpenStack: func() *openstack.Platform { + p := validPlatform() + p.LoadBalancer = &configv1.OpenStackPlatformLoadBalancer{ + Type: configv1.LoadBalancerTypeUserManaged, + } + return p + }(), + }, + }, + valid: true, + }, + { + name: "allowed load balancer field invalid type", + platform: validPlatform(), + config: &types.InstallConfig{ + FeatureSet: configv1.TechPreviewNoUpgrade, + Platform: types.Platform{ + OpenStack: func() *openstack.Platform { + p := validPlatform() + p.LoadBalancer = &configv1.OpenStackPlatformLoadBalancer{ + Type: "FooBar", + } + return p + }(), + }, + }, + expectedError: `^test-path\.loadBalancer.type: Invalid value: "FooBar": invalid load balancer type`, + valid: false, + }, { name: "non IP external dns", platform: func() *openstack.Platform { @@ -57,8 +129,9 @@ func TestValidatePlatform(t *testing.T) { } return p }(), - networking: validNetworking(), - valid: false, + networking: validNetworking(), + valid: false, + expectedError: `\"invalid\" is not a valid IP`, }, { name: "valid external dns", @@ -75,15 +148,32 @@ func TestValidatePlatform(t *testing.T) { } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - testConfig := types.InstallConfig{} - testConfig.ObjectMeta.Name = "test" + // Build default wrapping installConfig + if tc.config == nil { + tc.config = installConfig().build() + tc.config.OpenStack = tc.platform + } - err := ValidatePlatform(tc.platform, tc.networking, field.NewPath("test-path"), &testConfig).ToAggregate() - if tc.valid { + err := ValidatePlatform(tc.platform, tc.networking, field.NewPath("test-path"), tc.config).ToAggregate() + if tc.expectedError == "" { assert.NoError(t, err) } else { - assert.Error(t, err) + assert.Regexp(t, tc.expectedError, err) } }) } } + +type installConfigBuilder struct { + types.InstallConfig +} + +func installConfig() *installConfigBuilder { + return &installConfigBuilder{ + InstallConfig: types.InstallConfig{}, + } +} + +func (icb *installConfigBuilder) build() *types.InstallConfig { + return &icb.InstallConfig +} diff --git a/pkg/types/ovirt/platform.go b/pkg/types/ovirt/platform.go index 60040c49be..d54e6cf500 100644 --- a/pkg/types/ovirt/platform.go +++ b/pkg/types/ovirt/platform.go @@ -1,5 +1,9 @@ package ovirt +import ( + configv1 "github.com/openshift/api/config/v1" +) + // Platform stores all the global configuration that all // machinesets use. type Platform struct { @@ -66,6 +70,11 @@ type Platform struct { // AffinityGroups contains the RHV affinity groups that the installer will create. // +optional AffinityGroups []AffinityGroup `json:"affinityGroups"` + + // LoadBalancer defines how the load balancer used by the cluster is configured. + // LoadBalancer is available in TechPreview. + // +optional + LoadBalancer *configv1.OvirtPlatformLoadBalancer `json:"loadBalancer,omitempty"` } // AffinityGroup defines the affinity group that the installer will create diff --git a/pkg/types/ovirt/validation/platform.go b/pkg/types/ovirt/validation/platform.go index 46dbbd8c18..34d3583595 100644 --- a/pkg/types/ovirt/validation/platform.go +++ b/pkg/types/ovirt/validation/platform.go @@ -6,12 +6,14 @@ import ( "github.com/pkg/errors" "k8s.io/apimachinery/pkg/util/validation/field" + configv1 "github.com/openshift/api/config/v1" + "github.com/openshift/installer/pkg/types" "github.com/openshift/installer/pkg/types/ovirt" "github.com/openshift/installer/pkg/validate" ) // ValidatePlatform checks that the specified platform is valid. -func ValidatePlatform(p *ovirt.Platform, fldPath *field.Path) field.ErrorList { +func ValidatePlatform(p *ovirt.Platform, fldPath *field.Path, c *types.InstallConfig) field.ErrorList { allErrs := field.ErrorList{} if err := validate.UUID(p.ClusterID); err != nil { allErrs = append(allErrs, field.Invalid(fldPath.Child("ovirt_cluster_id"), p.ClusterID, err.Error())) @@ -32,6 +34,19 @@ func ValidatePlatform(p *ovirt.Platform, fldPath *field.Path) field.ErrorList { allErrs = append(allErrs, ValidateMachinePool(p.DefaultMachinePlatform, fldPath.Child("defaultMachinePlatform"))...) } + // Platform fields only allowed in TechPreviewNoUpgrade + if c.FeatureSet != configv1.TechPreviewNoUpgrade { + if c.Ovirt.LoadBalancer != nil { + allErrs = append(allErrs, field.Forbidden(fldPath.Child("loadBalancer"), "load balancer is not supported in this feature set")) + } + } + + if c.Ovirt.LoadBalancer != nil { + if !validateLoadBalancer(c.Ovirt.LoadBalancer.Type) { + allErrs = append(allErrs, field.Invalid(fldPath.Child("loadBalancer", "type"), c.Ovirt.LoadBalancer.Type, "invalid load balancer type")) + } + } + return allErrs } @@ -78,3 +93,13 @@ func validateAffinityGroupDuplicate(agList []ovirt.AffinityGroup) field.ErrorLis } return allErrs } + +// validateLoadBalancer returns an error if the load balancer is not valid. +func validateLoadBalancer(lbType configv1.PlatformLoadBalancerType) bool { + switch lbType { + case configv1.LoadBalancerTypeOpenShiftManagedDefault, configv1.LoadBalancerTypeUserManaged: + return true + default: + return false + } +} diff --git a/pkg/types/ovirt/validation/platform_test.go b/pkg/types/ovirt/validation/platform_test.go index a5f067080c..3f6c432b15 100644 --- a/pkg/types/ovirt/validation/platform_test.go +++ b/pkg/types/ovirt/validation/platform_test.go @@ -6,6 +6,8 @@ import ( "github.com/stretchr/testify/assert" "k8s.io/apimachinery/pkg/util/validation/field" + configv1 "github.com/openshift/api/config/v1" + "github.com/openshift/installer/pkg/types" "github.com/openshift/installer/pkg/types/ovirt" ) @@ -23,15 +25,86 @@ func validPlatform() *ovirt.Platform { func TestValidatePlatform(t *testing.T) { cases := []struct { - name string - platform *ovirt.Platform - valid bool + name string + config *types.InstallConfig + platform *ovirt.Platform + valid bool + expectedError string }{ { name: "minimal", platform: validPlatform(), valid: true, }, + { + name: "forbidden load balancer field", + platform: validPlatform(), + config: &types.InstallConfig{ + Platform: types.Platform{ + Ovirt: func() *ovirt.Platform { + p := validPlatform() + p.LoadBalancer = &configv1.OvirtPlatformLoadBalancer{ + Type: configv1.LoadBalancerTypeOpenShiftManagedDefault, + } + return p + }(), + }, + }, + valid: false, + expectedError: `^test-path\.loadBalancer: Forbidden: load balancer is not supported in this feature set`, + }, + { + name: "allowed load balancer field with OpenShift managed default", + platform: validPlatform(), + config: &types.InstallConfig{ + FeatureSet: configv1.TechPreviewNoUpgrade, + Platform: types.Platform{ + Ovirt: func() *ovirt.Platform { + p := validPlatform() + p.LoadBalancer = &configv1.OvirtPlatformLoadBalancer{ + Type: configv1.LoadBalancerTypeOpenShiftManagedDefault, + } + return p + }(), + }, + }, + valid: true, + }, + { + name: "allowed load balancer field with user-managed", + platform: validPlatform(), + config: &types.InstallConfig{ + FeatureSet: configv1.TechPreviewNoUpgrade, + Platform: types.Platform{ + Ovirt: func() *ovirt.Platform { + p := validPlatform() + p.LoadBalancer = &configv1.OvirtPlatformLoadBalancer{ + Type: configv1.LoadBalancerTypeUserManaged, + } + return p + }(), + }, + }, + valid: true, + }, + { + name: "allowed load balancer field invalid type", + platform: validPlatform(), + config: &types.InstallConfig{ + FeatureSet: configv1.TechPreviewNoUpgrade, + Platform: types.Platform{ + Ovirt: func() *ovirt.Platform { + p := validPlatform() + p.LoadBalancer = &configv1.OvirtPlatformLoadBalancer{ + Type: "FooBar", + } + return p + }(), + }, + }, + expectedError: `^test-path\.loadBalancer.type: Invalid value: "FooBar": invalid load balancer type`, + valid: false, + }, { name: "invalid when empty cluster ID", platform: func() *ovirt.Platform { @@ -39,7 +112,8 @@ func TestValidatePlatform(t *testing.T) { p.ClusterID = "" return p }(), - valid: false, + expectedError: `^test-path.ovirt_cluster_id: Invalid value: "": invalid UUID length: 0`, + valid: false, }, { name: "invalid when empty storage ID", @@ -48,7 +122,8 @@ func TestValidatePlatform(t *testing.T) { p.StorageDomainID = "" return p }(), - valid: false, + expectedError: `^test-path.ovirt_storage_domain_id: Invalid value: "": invalid UUID length: 0`, + valid: false, }, { name: "malformed vnic profile id", @@ -57,7 +132,8 @@ func TestValidatePlatform(t *testing.T) { p.VNICProfileID = "abcd-sdf" return p }(), - valid: false, + expectedError: `^test-path.vnicProfileID: Invalid value: "abcd-sdf": invalid UUID length: 8`, + valid: false, }, { name: "valid empty vnic profile id", @@ -80,12 +156,32 @@ func TestValidatePlatform(t *testing.T) { } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - err := ValidatePlatform(tc.platform, field.NewPath("test-path")).ToAggregate() - if tc.valid { + // Build default wrapping installConfig + if tc.config == nil { + tc.config = installConfig().build() + tc.config.Ovirt = tc.platform + } + + err := ValidatePlatform(tc.platform, field.NewPath("test-path"), tc.config).ToAggregate() + if tc.expectedError == "" { assert.NoError(t, err) } else { - assert.Error(t, err) + assert.Regexp(t, tc.expectedError, err) } }) } } + +type installConfigBuilder struct { + types.InstallConfig +} + +func installConfig() *installConfigBuilder { + return &installConfigBuilder{ + InstallConfig: types.InstallConfig{}, + } +} + +func (icb *installConfigBuilder) build() *types.InstallConfig { + return &icb.InstallConfig +} diff --git a/pkg/types/validation/installconfig.go b/pkg/types/validation/installconfig.go index 0cb3cc22cb..bb0e32c20f 100644 --- a/pkg/types/validation/installconfig.go +++ b/pkg/types/validation/installconfig.go @@ -722,7 +722,7 @@ func validatePlatform(platform *types.Platform, fldPath *field.Path, network *ty } if platform.VSphere != nil { validate(vsphere.Name, platform.VSphere, func(f *field.Path) field.ErrorList { - return vspherevalidation.ValidatePlatform(platform.VSphere, f) + return vspherevalidation.ValidatePlatform(platform.VSphere, f, c) }) } if platform.BareMetal != nil { @@ -732,12 +732,12 @@ func validatePlatform(platform *types.Platform, fldPath *field.Path, network *ty } if platform.Ovirt != nil { validate(ovirt.Name, platform.Ovirt, func(f *field.Path) field.ErrorList { - return ovirtvalidation.ValidatePlatform(platform.Ovirt, f) + return ovirtvalidation.ValidatePlatform(platform.Ovirt, f, c) }) } if platform.Nutanix != nil { validate(nutanix.Name, platform.Nutanix, func(f *field.Path) field.ErrorList { - return nutanixvalidation.ValidatePlatform(platform.Nutanix, f) + return nutanixvalidation.ValidatePlatform(platform.Nutanix, f, c) }) } return allErrs diff --git a/pkg/types/vsphere/platform.go b/pkg/types/vsphere/platform.go index 1bb67ad7ca..e87b1b434b 100644 --- a/pkg/types/vsphere/platform.go +++ b/pkg/types/vsphere/platform.go @@ -1,5 +1,9 @@ package vsphere +import ( + configv1 "github.com/openshift/api/config/v1" +) + // DiskType is a disk provisioning type for vsphere. // +kubebuilder:validation:Enum="";thin;thick;eagerZeroedThick type DiskType string @@ -116,6 +120,11 @@ type Platform struct { // If this is omitted failure domains (regions and zones) will not be used. // +kubebuilder:validation:Optional FailureDomains []FailureDomain `json:"failureDomains,omitempty"` + + // LoadBalancer defines how the load balancer used by the cluster is configured. + // LoadBalancer is available in TechPreview. + // +optional + LoadBalancer *configv1.VSpherePlatformLoadBalancer `json:"loadBalancer,omitempty"` } // FailureDomain holds the region and zone failure domain and diff --git a/pkg/types/vsphere/validation/platform.go b/pkg/types/vsphere/validation/platform.go index 3801086651..a2f452e23e 100644 --- a/pkg/types/vsphere/validation/platform.go +++ b/pkg/types/vsphere/validation/platform.go @@ -9,13 +9,14 @@ import ( "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/validation/field" + configv1 "github.com/openshift/api/config/v1" + "github.com/openshift/installer/pkg/types" "github.com/openshift/installer/pkg/types/vsphere" "github.com/openshift/installer/pkg/validate" ) // ValidatePlatform checks that the specified platform is valid. -func ValidatePlatform(p *vsphere.Platform, fldPath *field.Path) field.ErrorList { - +func ValidatePlatform(p *vsphere.Platform, fldPath *field.Path, c *types.InstallConfig) field.ErrorList { isLegacyUpi := false // This is to cover existing UPI non-zonal case // where neither network or cluster is required. @@ -44,6 +45,19 @@ func ValidatePlatform(p *vsphere.Platform, fldPath *field.Path) field.ErrorList allErrs = append(allErrs, validateFailureDomains(p, fldPath.Child("failureDomains"), isLegacyUpi)...) } + // Platform fields only allowed in TechPreviewNoUpgrade + if c.FeatureSet != configv1.TechPreviewNoUpgrade { + if c.VSphere.LoadBalancer != nil { + allErrs = append(allErrs, field.Forbidden(fldPath.Child("loadBalancer"), "load balancer is not supported in this feature set")) + } + } + + if c.VSphere.LoadBalancer != nil { + if !validateLoadBalancer(c.VSphere.LoadBalancer.Type) { + allErrs = append(allErrs, field.Invalid(fldPath.Child("loadBalancer", "type"), c.VSphere.LoadBalancer.Type, "invalid load balancer type")) + } + } + return allErrs } @@ -197,3 +211,13 @@ func validateDiskType(p *vsphere.Platform, fldPath *field.Path) field.ErrorList return allErrs } + +// validateLoadBalancer returns an error if the load balancer is not valid. +func validateLoadBalancer(lbType configv1.PlatformLoadBalancerType) bool { + switch lbType { + case configv1.LoadBalancerTypeOpenShiftManagedDefault, configv1.LoadBalancerTypeUserManaged: + return true + default: + return false + } +} diff --git a/pkg/types/vsphere/validation/platform_test.go b/pkg/types/vsphere/validation/platform_test.go index 3fbe268159..6994d32336 100644 --- a/pkg/types/vsphere/validation/platform_test.go +++ b/pkg/types/vsphere/validation/platform_test.go @@ -7,6 +7,8 @@ import ( "github.com/stretchr/testify/assert" "k8s.io/apimachinery/pkg/util/validation/field" + configv1 "github.com/openshift/api/config/v1" + "github.com/openshift/installer/pkg/types" "github.com/openshift/installer/pkg/types/vsphere" ) @@ -57,10 +59,10 @@ func validPlatform() *vsphere.Platform { func TestValidatePlatform(t *testing.T) { cases := []struct { - name string - platform *vsphere.Platform - postValidationFunction func(*vsphere.Platform) error - expectedError string + name string + config *types.InstallConfig + platform *vsphere.Platform + expectedError string }{ { name: "Valid diskType", @@ -213,21 +215,99 @@ func TestValidatePlatform(t *testing.T) { }(), expectedError: `^test-path\.failureDomains\.zone: Required value: must specify zone tag value`, }, + { + name: "forbidden load balancer field", + platform: validPlatform(), + config: &types.InstallConfig{ + Platform: types.Platform{ + VSphere: func() *vsphere.Platform { + p := validPlatform() + p.LoadBalancer = &configv1.VSpherePlatformLoadBalancer{ + Type: configv1.LoadBalancerTypeOpenShiftManagedDefault, + } + return p + }(), + }, + }, + expectedError: `^test-path\.loadBalancer: Forbidden: load balancer is not supported in this feature set`, + }, + { + name: "allowed load balancer field with OpenShift managed default", + platform: validPlatform(), + config: &types.InstallConfig{ + FeatureSet: configv1.TechPreviewNoUpgrade, + Platform: types.Platform{ + VSphere: func() *vsphere.Platform { + p := validPlatform() + p.LoadBalancer = &configv1.VSpherePlatformLoadBalancer{ + Type: configv1.LoadBalancerTypeOpenShiftManagedDefault, + } + return p + }(), + }, + }, + }, + { + name: "allowed load balancer field with user-managed", + platform: validPlatform(), + config: &types.InstallConfig{ + FeatureSet: configv1.TechPreviewNoUpgrade, + Platform: types.Platform{ + VSphere: func() *vsphere.Platform { + p := validPlatform() + p.LoadBalancer = &configv1.VSpherePlatformLoadBalancer{ + Type: configv1.LoadBalancerTypeUserManaged, + } + return p + }(), + }, + }, + }, + { + name: "allowed load balancer field invalid type", + platform: validPlatform(), + config: &types.InstallConfig{ + FeatureSet: configv1.TechPreviewNoUpgrade, + Platform: types.Platform{ + VSphere: func() *vsphere.Platform { + p := validPlatform() + p.LoadBalancer = &configv1.VSpherePlatformLoadBalancer{ + Type: "FooBar", + } + return p + }(), + }, + }, + expectedError: `^test-path\.loadBalancer.type: Invalid value: "FooBar": invalid load balancer type`, + }, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - err := ValidatePlatform(tc.platform, field.NewPath("test-path")).ToAggregate() + // Build default wrapping installConfig + if tc.config == nil { + tc.config = installConfig().build() + tc.config.VSphere = tc.platform + } + err := ValidatePlatform(tc.platform, field.NewPath("test-path"), tc.config).ToAggregate() if tc.expectedError == "" { assert.NoError(t, err) } else { assert.Regexp(t, regexp.MustCompile(tc.expectedError), err) } - if tc.postValidationFunction != nil { - err := tc.postValidationFunction(tc.platform) - if err != nil { - assert.NoError(t, err) - } - } }) } } + +type installConfigBuilder struct { + types.InstallConfig +} + +func installConfig() *installConfigBuilder { + return &installConfigBuilder{ + InstallConfig: types.InstallConfig{}, + } +} + +func (icb *installConfigBuilder) build() *types.InstallConfig { + return &icb.InstallConfig +} diff --git a/scripts/openstack/manifest-tests/default-stable-lb/install-config.yaml b/scripts/openstack/manifest-tests/default-stable-lb/install-config.yaml new file mode 100644 index 0000000000..fb92c405aa --- /dev/null +++ b/scripts/openstack/manifest-tests/default-stable-lb/install-config.yaml @@ -0,0 +1,31 @@ +apiVersion: v1 +baseDomain: shiftstack.example.com +controlPlane: + hyperthreading: Enabled + architecture: amd64 + name: master + platform: + openstack: + type: ${COMPUTE_FLAVOR} + replicas: 3 +compute: +- name: worker + platform: + openstack: + type: ${COMPUTE_FLAVOR} + replicas: 3 +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: + cloud: ${OS_CLOUD} +pullSecret: ${PULL_SECRET} diff --git a/scripts/openstack/manifest-tests/default-stable-lb/test_cluster-infra.py b/scripts/openstack/manifest-tests/default-stable-lb/test_cluster-infra.py new file mode 100755 index 0000000000..147254663b --- /dev/null +++ b/scripts/openstack/manifest-tests/default-stable-lb/test_cluster-infra.py @@ -0,0 +1,27 @@ +#!/usr/bin/env python +# -*- coding: utf-8 -*- + +import unittest + +import sys +import glob +import yaml + +ASSETS_DIR = "" + +class TestClusterInfraObject(unittest.TestCase): + def setUp(self): + """Parse the Cluster Infrastructure object into a Python data structure.""" + self.machines = [] + cluster_infra = f'{ASSETS_DIR}/manifests/cluster-infrastructure-02-config.yml' + with open(cluster_infra) as f: + self.cluster_infra = yaml.load(f, Loader=yaml.FullLoader) + + def test_load_balancer(self): + """Assert that the Cluster infrastructure object does not contain the LoadBalancer configuration.""" + self.assertNotIn("loadBalancer", self.cluster_infra["status"]["platformStatus"]["openstack"]) + + +if __name__ == '__main__': + ASSETS_DIR = sys.argv.pop() + unittest.main(verbosity=2) diff --git a/scripts/openstack/manifest-tests/default-techpreview-lb/install-config.yaml b/scripts/openstack/manifest-tests/default-techpreview-lb/install-config.yaml new file mode 100644 index 0000000000..d7d8f591a0 --- /dev/null +++ b/scripts/openstack/manifest-tests/default-techpreview-lb/install-config.yaml @@ -0,0 +1,32 @@ +apiVersion: v1 +baseDomain: shiftstack.example.com +controlPlane: + hyperthreading: Enabled + architecture: amd64 + name: master + platform: + openstack: + type: ${COMPUTE_FLAVOR} + replicas: 3 +compute: +- name: worker + platform: + openstack: + type: ${COMPUTE_FLAVOR} + replicas: 3 +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: + cloud: ${OS_CLOUD} +featureSet: TechPreviewNoUpgrade +pullSecret: ${PULL_SECRET} diff --git a/scripts/openstack/manifest-tests/default-techpreview-lb/test_cluster-infra.py b/scripts/openstack/manifest-tests/default-techpreview-lb/test_cluster-infra.py new file mode 100755 index 0000000000..147254663b --- /dev/null +++ b/scripts/openstack/manifest-tests/default-techpreview-lb/test_cluster-infra.py @@ -0,0 +1,27 @@ +#!/usr/bin/env python +# -*- coding: utf-8 -*- + +import unittest + +import sys +import glob +import yaml + +ASSETS_DIR = "" + +class TestClusterInfraObject(unittest.TestCase): + def setUp(self): + """Parse the Cluster Infrastructure object into a Python data structure.""" + self.machines = [] + cluster_infra = f'{ASSETS_DIR}/manifests/cluster-infrastructure-02-config.yml' + with open(cluster_infra) as f: + self.cluster_infra = yaml.load(f, Loader=yaml.FullLoader) + + def test_load_balancer(self): + """Assert that the Cluster infrastructure object does not contain the LoadBalancer configuration.""" + self.assertNotIn("loadBalancer", self.cluster_infra["status"]["platformStatus"]["openstack"]) + + +if __name__ == '__main__': + ASSETS_DIR = sys.argv.pop() + unittest.main(verbosity=2) diff --git a/scripts/openstack/manifest-tests/openshift-managed-lb/install-config.yaml b/scripts/openstack/manifest-tests/openshift-managed-lb/install-config.yaml new file mode 100644 index 0000000000..6aa00a0ea2 --- /dev/null +++ b/scripts/openstack/manifest-tests/openshift-managed-lb/install-config.yaml @@ -0,0 +1,34 @@ +apiVersion: v1 +baseDomain: shiftstack.example.com +controlPlane: + hyperthreading: Enabled + architecture: amd64 + name: master + platform: + openstack: + type: ${COMPUTE_FLAVOR} + replicas: 3 +compute: +- name: worker + platform: + openstack: + type: ${COMPUTE_FLAVOR} + replicas: 3 +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: + cloud: ${OS_CLOUD} + loadBalancer: + type: OpenShiftManagedDefault +featureSet: TechPreviewNoUpgrade +pullSecret: ${PULL_SECRET} diff --git a/scripts/openstack/manifest-tests/openshift-managed-lb/test_cluster-infra.py b/scripts/openstack/manifest-tests/openshift-managed-lb/test_cluster-infra.py new file mode 100755 index 0000000000..f337450542 --- /dev/null +++ b/scripts/openstack/manifest-tests/openshift-managed-lb/test_cluster-infra.py @@ -0,0 +1,32 @@ +#!/usr/bin/env python +# -*- coding: utf-8 -*- + +import unittest + +import sys +import glob +import yaml + +ASSETS_DIR = "" + +class TestClusterInfraObject(unittest.TestCase): + def setUp(self): + """Parse the Cluster Infrastructure object into a Python data structure.""" + self.machines = [] + cluster_infra = f'{ASSETS_DIR}/manifests/cluster-infrastructure-02-config.yml' + with open(cluster_infra) as f: + self.cluster_infra = yaml.load(f, Loader=yaml.FullLoader) + + def test_load_balancer(self): + """Assert that the Cluster infrastructure object contains the LoadBalancer configuration.""" + self.assertIn("loadBalancer", self.cluster_infra["status"]["platformStatus"]["openstack"]) + + loadBalancer = self.cluster_infra["status"]["platformStatus"]["openstack"]["loadBalancer"] + + self.assertIn("type", loadBalancer) + self.assertEqual("OpenShiftManagedDefault", loadBalancer["type"]) + + +if __name__ == '__main__': + ASSETS_DIR = sys.argv.pop() + unittest.main(verbosity=2) diff --git a/scripts/openstack/manifest-tests/user-managed-lb/install-config.yaml b/scripts/openstack/manifest-tests/user-managed-lb/install-config.yaml new file mode 100644 index 0000000000..04402ddd67 --- /dev/null +++ b/scripts/openstack/manifest-tests/user-managed-lb/install-config.yaml @@ -0,0 +1,34 @@ +apiVersion: v1 +baseDomain: shiftstack.example.com +controlPlane: + hyperthreading: Enabled + architecture: amd64 + name: master + platform: + openstack: + type: ${COMPUTE_FLAVOR} + replicas: 3 +compute: +- name: worker + platform: + openstack: + type: ${COMPUTE_FLAVOR} + replicas: 3 +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: + cloud: ${OS_CLOUD} + loadBalancer: + type: UserManaged +featureSet: TechPreviewNoUpgrade +pullSecret: ${PULL_SECRET} diff --git a/scripts/openstack/manifest-tests/user-managed-lb/test_cluster-infra.py b/scripts/openstack/manifest-tests/user-managed-lb/test_cluster-infra.py new file mode 100755 index 0000000000..56db471a38 --- /dev/null +++ b/scripts/openstack/manifest-tests/user-managed-lb/test_cluster-infra.py @@ -0,0 +1,32 @@ +#!/usr/bin/env python +# -*- coding: utf-8 -*- + +import unittest + +import sys +import glob +import yaml + +ASSETS_DIR = "" + +class TestClusterInfraObject(unittest.TestCase): + def setUp(self): + """Parse the Cluster Infrastructure object into a Python data structure.""" + self.machines = [] + cluster_infra = f'{ASSETS_DIR}/manifests/cluster-infrastructure-02-config.yml' + with open(cluster_infra) as f: + self.cluster_infra = yaml.load(f, Loader=yaml.FullLoader) + + def test_load_balancer(self): + """Assert that the Cluster infrastructure object contains the LoadBalancer configuration.""" + self.assertIn("loadBalancer", self.cluster_infra["status"]["platformStatus"]["openstack"]) + + loadBalancer = self.cluster_infra["status"]["platformStatus"]["openstack"]["loadBalancer"] + + self.assertIn("type", loadBalancer) + self.assertEqual("UserManaged", loadBalancer["type"]) + + +if __name__ == '__main__': + ASSETS_DIR = sys.argv.pop() + unittest.main(verbosity=2)