Validate MTU for custom network

Set the minimum MTU to 1380 when deploying on IPv6. This value corresponds to the minimum MTU for IPv6, which is 1280, and the ovn-kubernetes overhead, which is 100.
openshift · Dec 9, 2024 · 8a97503 · 8a97503
1 parent f6549e3
commit 8a97503
Show file tree

Hide file tree

Showing 6 changed files with 197 additions and 29 deletions.
diff --git a/pkg/asset/installconfig/openstack/validation/cloudinfo.go b/pkg/asset/installconfig/openstack/validation/cloudinfo.go
@@ -15,6 +15,7 @@ import (
 	tokensv3 "github.com/gophercloud/gophercloud/openstack/identity/v3/tokens"
 	"github.com/gophercloud/gophercloud/openstack/imageservice/v2/images"
 	"github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/layer3/floatingips"
+	"github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/mtu"
 	networkquotasets "github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/quotas"
 	"github.com/gophercloud/gophercloud/openstack/networking/v2/networks"
 	"github.com/gophercloud/gophercloud/openstack/networking/v2/subnets"
@@ -35,11 +36,11 @@ import (
 // CloudInfo caches data fetched from the user's openstack cloud
 type CloudInfo struct {
 	APIFIP                  *floatingips.FloatingIP
-	ExternalNetwork         *networks.Network
+	ExternalNetwork         *Network
 	Flavors                 map[string]Flavor
 	IngressFIP              *floatingips.FloatingIP
 	ControlPlanePortSubnets []*subnets.Subnet
-	ControlPlanePortNetwork *networks.Network
+	ControlPlanePortNetwork *Network
 	OSImage                 *images.Image
 	ComputeZones            []string
 	VolumeZones             []string
@@ -50,6 +51,12 @@ type CloudInfo struct {
 	clients *clients
 }
 
+// Network holds a gophercloud network with additional info such as MTU.
+type Network struct {
+	networks.Network
+	mtu.NetworkMTUExt
+}
+
 type clients struct {
 	networkClient  *gophercloud.ServiceClient
 	computeClient  *gophercloud.ServiceClient
@@ -298,7 +305,7 @@ func (ci *CloudInfo) getFlavor(flavorName string) (Flavor, error) {
 	}, nil
 }
 
-func (ci *CloudInfo) getNetworkByName(networkName string) (*networks.Network, error) {
+func (ci *CloudInfo) getNetworkByName(networkName string) (*Network, error) {
 	if networkName == "" {
 		return nil, nil
 	}
@@ -310,15 +317,16 @@ func (ci *CloudInfo) getNetworkByName(networkName string) (*networks.Network, er
 		return nil, err
 	}
 
-	network, err := networks.Get(ci.clients.networkClient, networkID).Extract()
+	var network Network
+	err = networks.Get(ci.clients.networkClient, networkID).ExtractInto(&network)
 	if err != nil {
 		return nil, err
 	}
 
-	return network, nil
+	return &network, nil
 }
 
-func (ci *CloudInfo) getNetwork(controlPlanePort *openstack.PortTarget) (*networks.Network, error) {
+func (ci *CloudInfo) getNetwork(controlPlanePort *openstack.PortTarget) (*Network, error) {
 	networkName := controlPlanePort.Network.Name
 	networkID := controlPlanePort.Network.ID
 	if networkName == "" && networkID == "" {
@@ -336,7 +344,8 @@ func (ci *CloudInfo) getNetwork(controlPlanePort *openstack.PortTarget) (*networ
 		return nil, err
 	}
 
-	allNetworks, err := networks.ExtractNetworks(allPages)
+	var allNetworks []Network
+	err = networks.ExtractNetworksInto(allPages, &allNetworks)
 	if err != nil {
 		return nil, err
 	}

diff --git a/pkg/asset/installconfig/openstack/validation/platform.go b/pkg/asset/installconfig/openstack/validation/platform.go
@@ -15,6 +15,13 @@ import (
 	"github.com/openshift/installer/pkg/types/openstack"
 )
 
+const (
+	// MTU of 1280 is the minimum allowed for IPv6 + 100 for the
+	// OVN-Kubernetes encapsulation overhead
+	// https://issues.redhat.com/browse/OCPBUGS-2921
+	minimumMTUv6 = 1380
+)
+
 // ValidatePlatform checks that the specified platform is valid.
 func ValidatePlatform(p *openstack.Platform, n *types.Networking, ci *CloudInfo) field.ErrorList {
 	var allErrs field.ErrorList
@@ -90,6 +97,9 @@ func validateControlPlanePort(p *openstack.Platform, n *types.Networking, ci *Cl
 		} else if ci.ControlPlanePortNetwork.ID != networkID {
 			allErrs = append(allErrs, field.Invalid(fldPath.Child("controlPlanePort").Child("network"), networkDetail, "network must contain subnets"))
 		}
+		if hasIPv6Subnet && ci.ControlPlanePortNetwork.MTU < minimumMTUv6 {
+			allErrs = append(allErrs, field.InternalError(fldPath.Child("controlPlanePort").Child("network"), fmt.Errorf("network should have an MTU of at least %d", minimumMTUv6)))
+		}
 	}
 	return allErrs
 }

diff --git a/pkg/asset/installconfig/openstack/validation/platform_test.go b/pkg/asset/installconfig/openstack/validation/platform_test.go
@@ -5,6 +5,7 @@ import (
 
 	"github.com/gophercloud/gophercloud/openstack/imageservice/v2/images"
 	"github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/layer3/floatingips"
+	"github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/mtu"
 	"github.com/gophercloud/gophercloud/openstack/networking/v2/networks"
 	"github.com/gophercloud/gophercloud/openstack/networking/v2/subnets"
 	"github.com/stretchr/testify/assert"
@@ -54,17 +55,20 @@ func withControlPlanePortSubnets(subnetCIDR, allocationPoolStart, allocationPool
 				{Start: allocationPoolStart, End: allocationPoolEnd},
 			},
 		}
-		Allsubnets := []*subnets.Subnet{&subnet}
-		ci.ControlPlanePortSubnets = Allsubnets
+		allsubnets := []*subnets.Subnet{&subnet}
+		ci.ControlPlanePortSubnets = allsubnets
 	}
 }
 func validPlatformCloudInfo(options ...func(*CloudInfo)) *CloudInfo {
 	ci := CloudInfo{
-		ExternalNetwork: &networks.Network{
-			ID:           "71b97520-69af-4c35-8153-cdf827z96e60",
-			Name:         validExternalNetwork,
-			AdminStateUp: true,
-			Status:       "ACTIVE",
+		ExternalNetwork: &Network{
+			networks.Network{
+				ID:           "71b97520-69af-4c35-8153-cdf827z96e60",
+				Name:         validExternalNetwork,
+				AdminStateUp: true,
+				Status:       "ACTIVE",
+			},
+			mtu.NetworkMTUExt{},
 		},
 		APIFIP: &floatingips.FloatingIP{
 			ID:     validFIP1,
@@ -412,8 +416,8 @@ func TestMachineSubnet(t *testing.T) {
 				subnet := subnets.Subnet{
 					ID: "00000000-5a89-4465-8d54-3517ec2bad48",
 				}
-				Allsubnets := []*subnets.Subnet{&subnet}
-				ci.ControlPlanePortSubnets = Allsubnets
+				allsubnets := []*subnets.Subnet{&subnet}
+				ci.ControlPlanePortSubnets = allsubnets
 				return ci
 			}(),
 			networking:     validNetworking(),
@@ -434,9 +438,11 @@ func TestMachineSubnet(t *testing.T) {
 					NetworkID: "00000000-1a11-4465-8d54-3517ec2bad48",
 					CIDR:      "172.0.0.1/24",
 				}
-				Allsubnets := []*subnets.Subnet{&subnet}
-				ci.ControlPlanePortSubnets = Allsubnets
-				ci.ControlPlanePortNetwork = &networks.Network{ID: "00000000-2a22-4465-8d54-3517ec2bad48"}
+				allSubnets := []*subnets.Subnet{&subnet}
+				ci.ControlPlanePortSubnets = allSubnets
+				network := Network{}
+				network.ID = "00000000-2a22-4465-8d54-3517ec2bad48"
+				ci.ControlPlanePortNetwork = &network
 				return ci
 			}(),
 			networking: func() *types.Networking {
@@ -462,8 +468,8 @@ func TestMachineSubnet(t *testing.T) {
 					ID:   validSubnetID,
 					CIDR: "172.0.0.1/16",
 				}
-				Allsubnets := []*subnets.Subnet{&subnet}
-				ci.ControlPlanePortSubnets = Allsubnets
+				allsubnets := []*subnets.Subnet{&subnet}
+				ci.ControlPlanePortSubnets = allsubnets
 				return ci
 			}(),
 			networking: func() *types.Networking {
@@ -505,8 +511,8 @@ func TestMachineSubnet(t *testing.T) {
 					CIDR:      "2001:db8::/64",
 					IPVersion: 6,
 				}
-				Allsubnets := []*subnets.Subnet{&subnet, &subnetv6}
-				ci.ControlPlanePortSubnets = Allsubnets
+				allsubnets := []*subnets.Subnet{&subnet, &subnetv6}
+				ci.ControlPlanePortSubnets = allsubnets
 				return ci
 			}(),
 			networking: func() *types.Networking {
@@ -535,8 +541,8 @@ func TestMachineSubnet(t *testing.T) {
 					ID:   validSubnetID,
 					CIDR: "172.0.0.1/16",
 				}
-				Allsubnets := []*subnets.Subnet{&subnet}
-				ci.ControlPlanePortSubnets = Allsubnets
+				allsubnets := []*subnets.Subnet{&subnet}
+				ci.ControlPlanePortSubnets = allsubnets
 				return ci
 			}(),
 			networking: func() *types.Networking {
@@ -576,8 +582,8 @@ func TestMachineSubnet(t *testing.T) {
 					CIDR:      "10.0.0.0/16",
 					IPVersion: 4,
 				}
-				Allsubnets := []*subnets.Subnet{&subnet, &subnetv6}
-				ci.ControlPlanePortSubnets = Allsubnets
+				allsubnets := []*subnets.Subnet{&subnet, &subnetv6}
+				ci.ControlPlanePortSubnets = allsubnets
 				return ci
 			}(),
 			networking: func() *types.Networking {
@@ -609,8 +615,8 @@ func TestMachineSubnet(t *testing.T) {
 					CIDR:      "2001:db8::/64",
 					IPVersion: 6,
 				}
-				Allsubnets := []*subnets.Subnet{&subnetv6}
-				ci.ControlPlanePortSubnets = Allsubnets
+				allsubnets := []*subnets.Subnet{&subnetv6}
+				ci.ControlPlanePortSubnets = allsubnets
 				return ci
 			}(),
 			networking: func() *types.Networking {
@@ -623,6 +629,53 @@ func TestMachineSubnet(t *testing.T) {
 			}(),
 			expectedErrMsg: `platform.openstack.controlPlanePort.fixedIPs: Internal error: one IPv4 subnet must be specified`,
 		},
+		{
+			name: "MTU too low",
+			platform: func() *openstack.Platform {
+				p := validPlatform()
+				fixedIPv6 := openstack.FixedIP{
+					Subnet: openstack.SubnetFilter{ID: "00000000-1111-4465-8d54-3517ec2bad48"},
+				}
+				p.ControlPlanePort = &openstack.PortTarget{
+					FixedIPs: []openstack.FixedIP{fixedIPv6},
+					Network:  openstack.NetworkFilter{ID: "71b97520-69af-4c35-8153-cdf827z96e60"},
+				}
+				return p
+			}(),
+			cloudInfo: func() *CloudInfo {
+				ci := validPlatformCloudInfo()
+				subnetv6 := subnets.Subnet{
+					ID:        "00000000-1111-4465-8d54-3517ec2bad48",
+					CIDR:      "2001:db8::/64",
+					IPVersion: 6,
+					NetworkID: "71b97520-69af-4c35-8153-cdf827z96e60",
+				}
+				allSubnets := []*subnets.Subnet{&subnetv6}
+				ci.ControlPlanePortSubnets = allSubnets
+
+				network := Network{
+					networks.Network{
+						ID:           "71b97520-69af-4c35-8153-cdf827z96e60",
+						Name:         "too-low",
+						AdminStateUp: true,
+						Status:       "ACTIVE",
+						Subnets:      []string{"00000000-1111-4465-8d54-3517ec2bad48"},
+					},
+					mtu.NetworkMTUExt{MTU: 1200},
+				}
+				ci.ControlPlanePortNetwork = &network
+				return ci
+			}(),
+			networking: func() *types.Networking {
+				n := validNetworking()
+				machineNetworkEntry := &types.MachineNetworkEntry{
+					CIDR: *ipnet.MustParseCIDR("2001:db8::/64"),
+				}
+				n.MachineNetwork = []types.MachineNetworkEntry{*machineNetworkEntry}
+				return n
+			}(),
+			expectedErrMsg: "platform.openstack.controlPlanePort.network: Internal error: network should have an MTU of at least 1380",
+		},
 	}
 
 	for _, tc := range cases {

diff --git a/vendor/github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/mtu/requests.go b/vendor/github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/mtu/requests.go
diff --git a/vendor/github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/mtu/results.go b/vendor/github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/mtu/results.go
diff --git a/vendor/modules.txt b/vendor/modules.txt
@@ -678,6 +678,7 @@ github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/attributes
 github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/external
 github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/layer3/floatingips
 github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/layer3/routers
+github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/mtu
 github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/quotas
 github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/security/groups
 github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/security/rules