Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 49 additions & 5 deletions data/data/install.openshift.io_installconfigs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3057,6 +3057,49 @@ spec:
use for instances in this cluster. Deprecated: use FlavorName
in DefaultMachinePlatform to define default flavor.'
type: string
controlPlanePort:
description: ControlPlanePort contains details of the network
attached to the control plane port, with the network either
containing one openstack subnet for IPv4 or two openstack subnets
for dualstack clusters. Providing this configuration will prevent
OpenShift from managing or updating this network and its subnets.
The network and its subnets will be used during creation of
all nodes. This is a TechPreview feature and requires setting
featureSet to TechPreviewNoUpgrade.
properties:
fixedIPs:
description: Specify subnets of the network where control
plane port will be discovered.
items:
description: FixedIP identifies a subnet defined by a subnet
filter.
properties:
subnet:
description: SubnetFilter defines a subnet by ID and/or
name.
properties:
id:
type: string
name:
type: string
type: object
required:
- subnet
type: object
type: array
network:
description: Network is a query for an openstack network that
the port will be discovered on. This will fail if the query
returns more than one network.
properties:
id:
type: string
name:
type: string
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the network required here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, because machinesSubnet only contains info about the subnet and that would be the only info added to controlPlanePort during conversion.

type: object
required:
- fixedIPs
type: object
defaultMachinePlatform:
description: DefaultMachinePlatform is the default configuration
used when installing on OpenStack for machine pools which do
Expand Down Expand Up @@ -3190,11 +3233,12 @@ spec:
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.
By setting this, the installer will no longer create a network
and subnet. The subnet and network specified in MachinesSubnet
will not be deleted or modified by the installer.
description: 'DeprecatedMachinesSubnet is a string of the UUIDv4
of an openstack subnet. This subnet will be used by all nodes
created by the installer. By setting this, the installer will
no longer create a network and subnet. The subnet and network
specified in MachinesSubnet will not be deleted or modified
by the installer. Deprecated: Use ControlPlanePort'
type: string
octaviaSupport:
description: 'OctaviaSupport holds a `0` or `1` value that indicates
Expand Down
94 changes: 65 additions & 29 deletions pkg/asset/installconfig/openstack/validation/cloudinfo.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,23 +27,25 @@ import (

"github.com/openshift/installer/pkg/quota"
"github.com/openshift/installer/pkg/types"
"github.com/openshift/installer/pkg/types/openstack"
openstackdefaults "github.com/openshift/installer/pkg/types/openstack/defaults"
"github.com/openshift/installer/pkg/types/openstack/validation/networkextensions"
)

// CloudInfo caches data fetched from the user's openstack cloud
type CloudInfo struct {
APIFIP *floatingips.FloatingIP
ExternalNetwork *networks.Network
Flavors map[string]Flavor
IngressFIP *floatingips.FloatingIP
MachinesSubnet *subnets.Subnet
OSImage *images.Image
ComputeZones []string
VolumeZones []string
VolumeTypes []string
NetworkExtensions []extensions.Extension
Quotas []quota.Quota
APIFIP *floatingips.FloatingIP
ExternalNetwork *networks.Network
Flavors map[string]Flavor
IngressFIP *floatingips.FloatingIP
ControlPlanePortSubnets []*subnets.Subnet
ControlPlanePortNetwork *networks.Network
OSImage *images.Image
ComputeZones []string
VolumeZones []string
VolumeTypes []string
NetworkExtensions []extensions.Extension
Quotas []quota.Quota

clients *clients
}
Expand Down Expand Up @@ -117,7 +119,7 @@ func GetCloudInfo(ic *types.InstallConfig) (*CloudInfo, error) {
func (ci *CloudInfo) collectInfo(ic *types.InstallConfig, opts *clientconfig.ClientOpts) error {
var err error

ci.ExternalNetwork, err = ci.getNetwork(ic.OpenStack.ExternalNetwork)
ci.ExternalNetwork, err = ci.getNetworkByName(ic.OpenStack.ExternalNetwork)
if err != nil {
return fmt.Errorf("failed to fetch external network info: %w", err)
}
Expand Down Expand Up @@ -177,10 +179,16 @@ func (ci *CloudInfo) collectInfo(ic *types.InstallConfig, opts *clientconfig.Cli
}
}
}

ci.MachinesSubnet, err = ci.getSubnet(ic.OpenStack.MachinesSubnet)
if err != nil {
return fmt.Errorf("failed to fetch machine subnet info: %w", err)
if ic.OpenStack.ControlPlanePort != nil {
controlPlanePort := ic.OpenStack.ControlPlanePort
ci.ControlPlanePortSubnets, err = ci.getSubnets(controlPlanePort)
if err != nil {
return err
}
ci.ControlPlanePortNetwork, err = ci.getNetwork(controlPlanePort.Network.Name, controlPlanePort.Network.ID)
if err != nil {
return err
}
}

ci.APIFIP, err = ci.getFloatingIP(ic.OpenStack.APIFloatingIP)
Expand Down Expand Up @@ -226,20 +234,24 @@ func (ci *CloudInfo) collectInfo(ic *types.InstallConfig, opts *clientconfig.Cli

return nil
}

func (ci *CloudInfo) getSubnet(subnetID string) (*subnets.Subnet, error) {
if subnetID == "" {
return nil, nil
}
subnet, err := subnets.Get(ci.clients.networkClient, subnetID).Extract()
if err != nil {
if isNotFoundError(err) {
return nil, nil
func (ci *CloudInfo) getSubnets(controlPlanePort *openstack.PortTarget) ([]*subnets.Subnet, error) {
controlPlaneSubnets := make([]*subnets.Subnet, 0, len(controlPlanePort.FixedIPs))
for _, fixedIP := range controlPlanePort.FixedIPs {
page, err := subnets.List(ci.clients.networkClient, subnets.ListOpts{ID: fixedIP.Subnet.ID, Name: fixedIP.Subnet.Name}).AllPages()
if err != nil {
return controlPlaneSubnets, err
}
subnetList, err := subnets.ExtractSubnets(page)
if err != nil {
return controlPlaneSubnets, err
}
if len(subnetList) == 1 {
controlPlaneSubnets = append(controlPlaneSubnets, &subnetList[0])
} else if len(subnetList) > 1 {
return controlPlaneSubnets, fmt.Errorf("found multiple subnets")
}
return nil, err
}

return subnet, nil
return controlPlaneSubnets, nil
}

func isNotFoundError(err error) bool {
Expand Down Expand Up @@ -285,7 +297,7 @@ func (ci *CloudInfo) getFlavor(flavorName string) (Flavor, error) {
}, nil
}

func (ci *CloudInfo) getNetwork(networkName string) (*networks.Network, error) {
func (ci *CloudInfo) getNetworkByName(networkName string) (*networks.Network, error) {
if networkName == "" {
return nil, nil
}
Expand All @@ -305,6 +317,30 @@ func (ci *CloudInfo) getNetwork(networkName string) (*networks.Network, error) {
return network, nil
}

func (ci *CloudInfo) getNetwork(networkName, networkID string) (*networks.Network, error) {
opts := networks.ListOpts{
ID: networkID,
Name: networkName,
}
allPages, err := networks.List(ci.clients.networkClient, opts).AllPages()
if err != nil {
return nil, err
}

allNetworks, err := networks.ExtractNetworks(allPages)
if err != nil {
return nil, err
}

if len(allNetworks) == 0 {
return nil, nil
} else if len(allNetworks) > 1 {
return nil, fmt.Errorf("found multiple networks")
}

return &allNetworks[0], nil
}

func (ci *CloudInfo) getFloatingIP(fip string) (*floatingips.FloatingIP, error) {
if fip != "" {
opts := floatingips.ListOpts{
Expand Down
4 changes: 2 additions & 2 deletions pkg/asset/installconfig/openstack/validation/machinepool.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ func validateVolumeTypes(input string, available []string, fldPath *field.Path)
func validateUUIDV4s(input []string, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
for idx, uuid := range input {
if !validUUIDv4(uuid) {
if !ValidUUIDv4(uuid) {
allErrs = append(allErrs, field.Invalid(fldPath.Index(idx), uuid, "valid UUID v4 must be specified"))
}
}
Expand All @@ -116,7 +116,7 @@ func validateUUIDV4s(input []string, fldPath *field.Path) field.ErrorList {

// validUUIDv4 checks if string is in UUID v4 format
// For more information: https://en.wikipedia.org/wiki/Universally_unique_identifier#Version_4_(random)
func validUUIDv4(s string) bool {
func ValidUUIDv4(s string) bool {
uuid, err := guuid.Parse(s)
if err != nil {
return false
Expand Down
95 changes: 76 additions & 19 deletions pkg/asset/installconfig/openstack/validation/platform.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,14 @@ package validation

import (
"bytes"
"errors"
"fmt"
"net"
"net/url"

"github.com/gophercloud/gophercloud/openstack/imageservice/v2/images"
"github.com/gophercloud/gophercloud/openstack/networking/v2/subnets"
"k8s.io/apimachinery/pkg/util/validation/field"
utilsslice "k8s.io/utils/strings/slices"

"github.com/openshift/installer/pkg/types"
"github.com/openshift/installer/pkg/types/openstack"
Expand All @@ -19,8 +20,10 @@ func ValidatePlatform(p *openstack.Platform, n *types.Networking, ci *CloudInfo)
var allErrs field.ErrorList
fldPath := field.NewPath("platform", "openstack")

// validate BYO machinesSubnet usage
allErrs = append(allErrs, validateMachinesSubnet(p, n, ci, fldPath)...)
// validate BYO controlPlanePort usage
if p.ControlPlanePort != nil {
allErrs = append(allErrs, validateControlPlanePort(p, n, ci, fldPath)...)
}

// validate the externalNetwork
allErrs = append(allErrs, validateExternalNetwork(p, ci, fldPath)...)
Expand All @@ -37,26 +40,79 @@ func ValidatePlatform(p *openstack.Platform, n *types.Networking, ci *CloudInfo)
return allErrs
}

// validateMachinesSubnet validates the machines subnet and enforces proper byo subnet usage and returns a list of all validation errors
func validateMachinesSubnet(p *openstack.Platform, n *types.Networking, ci *CloudInfo, fldPath *field.Path) (allErrs field.ErrorList) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This validation function is being removed because the machineSubnets config would be converted to controlPlanePort's network config before it is validated (i.e when the deprecated config is provided by the user and not the new config)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.
The conversion happens when loading the install-config, which happens before the platform validation.

if p.MachinesSubnet != "" {
if len(p.ExternalDNS) > 0 {
allErrs = append(allErrs, field.Invalid(fldPath.Child("externalDNS"), p.ExternalDNS, "externalDNS is set, externalDNS is not supported when machinesSubnet is set"))
}
if ci.MachinesSubnet == nil {
allErrs = append(allErrs, field.NotFound(fldPath.Child("machinesSubnet"), p.MachinesSubnet))
} else if !validUUIDv4(p.MachinesSubnet) {
allErrs = append(allErrs, field.InternalError(fldPath.Child("machinesSubnet"), errors.New("invalid subnet ID")))
// validateControlPlanePort validates the machines subnets and network, while enforcing proper byo subnets usage and returns a list of all validation errors.
func validateControlPlanePort(p *openstack.Platform, n *types.Networking, ci *CloudInfo, fldPath *field.Path) (allErrs field.ErrorList) {
networkID := ""
hasIPv4Subnet := false
hasIPv6Subnet := false
if len(p.ExternalDNS) > 0 {
allErrs = append(allErrs, field.Invalid(fldPath.Child("externalDNS"), p.ExternalDNS, "externalDNS is set, externalDNS is not supported when ControlPlanePort is set"))
return allErrs
}
networkCIDRs := networksCIDRs(n.MachineNetwork)
for _, fixedIP := range p.ControlPlanePort.FixedIPs {
subnet := getSubnet(ci.ControlPlanePortSubnets, fixedIP.Subnet.ID, fixedIP.Subnet.Name)
if subnet == nil {
subnetDetail := fixedIP.Subnet.ID
if subnetDetail == "" {
subnetDetail = fixedIP.Subnet.Name
}
allErrs = append(allErrs, field.NotFound(fldPath.Child("controlPlanePort").Child("fixedIPs"), subnetDetail))
} else {
if n.MachineNetwork[0].CIDR.String() != ci.MachinesSubnet.CIDR {
allErrs = append(allErrs, field.InternalError(fldPath.Child("machinesSubnet"), fmt.Errorf("the first CIDR in machineNetwork, %s, doesn't match the CIDR of the machineSubnet, %s", n.MachineNetwork[0].CIDR.String(), ci.MachinesSubnet.CIDR)))
if subnet.IPVersion == 6 {
hasIPv6Subnet = true
} else {
hasIPv4Subnet = true
}
if !utilsslice.Contains(networkCIDRs, subnet.CIDR) {
allErrs = append(allErrs, field.Invalid(fldPath.Child("controlPlanePort").Child("fixedIPs"), subnet.CIDR, "controlPlanePort CIDR does not match machineNetwork"))
}
if networkID != "" && networkID != subnet.NetworkID {
allErrs = append(allErrs, field.Invalid(fldPath.Child("controlPlanePort").Child("fixedIPs"), subnet.NetworkID, "fixedIPs subnets must be on the same Network"))
}
networkID = subnet.NetworkID
}
}
if !hasIPv4Subnet && hasIPv6Subnet {
allErrs = append(allErrs, field.InternalError(fldPath.Child("controlPlanePort").Child("fixedIPs"), fmt.Errorf("one IPv4 subnet must be specified")))
} else if hasIPv4Subnet && !hasIPv6Subnet && len(p.ControlPlanePort.FixedIPs) == 2 {
allErrs = append(allErrs, field.InternalError(fldPath.Child("controlPlanePort").Child("fixedIPs"), fmt.Errorf("multiple IPv4 subnets is not supported")))
}
controlPlaneNetwork := p.ControlPlanePort.Network
if controlPlaneNetwork.ID != "" || controlPlaneNetwork.Name != "" {
networkDetail := controlPlaneNetwork.ID
if networkDetail == "" {
networkDetail = controlPlaneNetwork.Name
}
// check if the networks does not exist. If it does, verifies if the network contains the subnets
if ci.ControlPlanePortNetwork == nil {
allErrs = append(allErrs, field.NotFound(fldPath.Child("controlPlanePort").Child("network"), networkDetail))
} else if ci.ControlPlanePortNetwork.ID != networkID {
allErrs = append(allErrs, field.Invalid(fldPath.Child("controlPlanePort").Child("network"), networkDetail, "network must contain subnets"))
}
}

return allErrs
}

func networksCIDRs(machineNetwork []types.MachineNetworkEntry) []string {
networks := make([]string, 0, len(machineNetwork))
for _, network := range machineNetwork {
networks = append(networks, network.CIDR.String())
}
return networks
}

func getSubnet(controlPlaneSubnets []*subnets.Subnet, subnetID, subnetName string) *subnets.Subnet {
for _, subnet := range controlPlaneSubnets {
if subnet.ID == subnetID {
return subnet
} else if subnet.Name != "" && subnet.Name == subnetName {
return subnet
}
}
return nil
}

// validateExternalNetwork validates the user's input for the externalNetwork and returns a list of all validation errors
func validateExternalNetwork(p *openstack.Platform, ci *CloudInfo, fldPath *field.Path) (allErrs field.ErrorList) {
// Return an error if external network was specified in the install config, but hasn't been found
Expand Down Expand Up @@ -96,9 +152,10 @@ func validateFloatingIPs(p *openstack.Platform, ci *CloudInfo, fldPath *field.Pa
// platform VIP validation is done in pkg/types/validation/installconfig.go,
// validateAPIAndIngressVIPs().
func validateVIPs(p *openstack.Platform, ci *CloudInfo, fldPath *field.Path) (allErrs field.ErrorList) {
// If the subnet is not found in the CloudInfo object, abandon validation
if ci.MachinesSubnet != nil {
for _, allocationPool := range ci.MachinesSubnet.AllocationPools {
// If the subnet is not found in the CloudInfo object, abandon validation.
// For dual-stack the user needs to pre-create the Port for API and Ingress, so no need for validation.
if len(ci.ControlPlanePortSubnets) == 1 {
for _, allocationPool := range ci.ControlPlanePortSubnets[0].AllocationPools {
start := net.ParseIP(allocationPool.Start)
end := net.ParseIP(allocationPool.End)

Expand Down
Loading