Skip to content

Commit

Permalink
Merge pull request ovn-kubernetes#4722 from jcaamano/udn-gw-fixes
Browse files Browse the repository at this point in the history
UDN gateway & test fixes
  • Loading branch information
tssurya authored Sep 13, 2024
2 parents a83b6be + 14afcbf commit c960d47
Show file tree
Hide file tree
Showing 6 changed files with 97 additions and 100 deletions.
8 changes: 5 additions & 3 deletions contrib/kind-common
Original file line number Diff line number Diff line change
Expand Up @@ -394,9 +394,11 @@ install_kubevirt_ipam_controller() {
}

install_multus() {
echo "Installing multus-cni daemonset ..."
multus_manifest="https://raw.githubusercontent.com/k8snetworkplumbingwg/multus-cni/master/deployments/multus-daemonset.yml"
run_kubectl apply -f "$multus_manifest"
local version="v4.1.0"
echo "Installing multus-cni $version daemonset ..."
wget -qO- "https://raw.githubusercontent.com/k8snetworkplumbingwg/multus-cni/${version}/deployments/multus-daemonset.yml" |\
sed -e "s|multus-cni:snapshot|multus-cni:${version}|g" |\
run_kubectl apply -f -
}

install_mpolicy_crd() {
Expand Down
18 changes: 0 additions & 18 deletions go-controller/pkg/config/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -263,24 +263,6 @@ func (cs *ConfigSubnets) checkIPFamilies() (usingIPv4, usingIPv6 bool, err error
return false, false, fmt.Errorf("illegal network configuration: %s", netConfig)
}

func ContainsJoinIP(ip net.IP) bool {
var joinSubnetsConfig []string
if IPv4Mode {
joinSubnetsConfig = append(joinSubnetsConfig, Gateway.V4JoinSubnet)
}
if IPv6Mode {
joinSubnetsConfig = append(joinSubnetsConfig, Gateway.V6JoinSubnet)
}

for _, subnet := range joinSubnetsConfig {
_, joinSubnet, _ := net.ParseCIDR(subnet)
if joinSubnet.Contains(ip) {
return true
}
}
return false
}

// masqueradeIP represents the masqueradeIPs used by the masquerade subnets for host to service traffic
type MasqueradeIPsConfig struct {
V4OVNMasqueradeIP net.IP
Expand Down
50 changes: 36 additions & 14 deletions go-controller/pkg/ovn/gateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,20 +159,15 @@ func (gw *GatewayManager) cleanupStalePodSNATs(nodeName string, nodeIPs []*net.I
if !config.Gateway.DisableSNATMultipleGWs {
return nil
}

pods, err := gw.kube.GetPods(metav1.NamespaceAll, metav1.ListOptions{
FieldSelector: fields.OneTermEqualSelector("spec.nodeName", nodeName).String(),
})
if err != nil {
return fmt.Errorf("unable to list existing pods on node: %s, %w",
nodeName, err)
}
gatewayRouter := nbdb.LogicalRouter{
Name: gw.gwRouterName,
}
routerNats, err := libovsdbops.GetRouterNATs(gw.nbClient, &gatewayRouter)
if err != nil && errors.Is(err, libovsdbclient.ErrNotFound) {
return fmt.Errorf("unable to get NAT entries for router %s on node %s: %w", gatewayRouter.Name, nodeName, err)
}

podIPsOnNode := sets.NewString() // collects all podIPs on node
for _, pod := range pods {
pod := *pod
Expand Down Expand Up @@ -204,18 +199,37 @@ func (gw *GatewayManager) cleanupStalePodSNATs(nodeName string, nodeIPs []*net.I
podIPsOnNode.Insert(podIP.String())
}
}

gatewayRouter := nbdb.LogicalRouter{
Name: gw.gwRouterName,
}
routerNats, err := libovsdbops.GetRouterNATs(gw.nbClient, &gatewayRouter)
if err != nil && errors.Is(err, libovsdbclient.ErrNotFound) {
return fmt.Errorf("unable to get NAT entries for router %s on node %s: %w", gatewayRouter.Name, nodeName, err)
}

nodeIPset := sets.New(util.IPNetsIPToStringSlice(nodeIPs)...)
natsToDelete := []*nbdb.NAT{}
for _, routerNat := range routerNats {
routerNat := routerNat
if routerNat.Type != nbdb.NATTypeSNAT {
continue
}
for _, nodeIP := range nodeIPs {
logicalIP := net.ParseIP(routerNat.LogicalIP)
if routerNat.ExternalIP == nodeIP.IP.String() && !config.ContainsJoinIP(logicalIP) && !podIPsOnNode.Has(routerNat.LogicalIP) {
natsToDelete = append(natsToDelete, routerNat)
}
if !nodeIPset.Has(routerNat.ExternalIP) {
continue
}
if podIPsOnNode.Has(routerNat.LogicalIP) {
continue
}
logicalIP := net.ParseIP(routerNat.LogicalIP)
if logicalIP == nil {
// this is probably a CIDR so not a pod IP
continue
}
if gw.containsJoinIP(logicalIP) {
continue
}
natsToDelete = append(natsToDelete, routerNat)
}
if len(natsToDelete) > 0 {
err := libovsdbops.DeleteNATs(gw.nbClient, &gatewayRouter, natsToDelete...)
Expand Down Expand Up @@ -592,7 +606,7 @@ func (gw *GatewayManager) GatewayInit(
if gw.clusterRouterName != "" {
p := func(item *nbdb.LogicalRouterStaticRoute) bool {
return item.IPPrefix == lrsr.IPPrefix && item.Policy != nil && *item.Policy == *lrsr.Policy &&
config.ContainsJoinIP(net.ParseIP(item.Nexthop))
gw.containsJoinIP(net.ParseIP(item.Nexthop))
}
err := libovsdbops.DeleteLogicalRouterStaticRoutesWithPredicate(gw.nbClient, gw.clusterRouterName, p)
if err != nil {
Expand Down Expand Up @@ -643,7 +657,7 @@ func (gw *GatewayManager) GatewayInit(
// note, nat.LogicalIP may be a CIDR or IP, we don't care unless it's an IP
parsedLogicalIP := net.ParseIP(nat.LogicalIP)
// check if join ip changed
if config.ContainsJoinIP(parsedLogicalIP) {
if gw.containsJoinIP(parsedLogicalIP) {
// is a join SNAT, check if IP needs updating
joinIP, err := util.MatchFirstIPFamily(utilnet.IsIPv6(parsedLogicalIP), gwLRPIPs)
if err != nil {
Expand Down Expand Up @@ -1152,6 +1166,14 @@ func (gw *GatewayManager) removeLRPolicies(nodeName string) {
}
}

func (gw *GatewayManager) containsJoinIP(ip net.IP) bool {
ipNet := &net.IPNet{
IP: ip,
Mask: util.GetIPFullMask(ip),
}
return util.IsContainedInAnyCIDR(ipNet, gw.netInfo.JoinSubnets()...)
}

func (gw *GatewayManager) syncGatewayLogicalNetwork(
node *kapi.Node,
l3GatewayConfig *util.L3GatewayConfig,
Expand Down
43 changes: 17 additions & 26 deletions go-controller/pkg/ovn/multihoming_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ import (

v1 "k8s.io/api/core/v1"

iputils "github.com/containernetworking/plugins/pkg/ip"

nadapi "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1"

libovsdbclient "github.com/ovn-org/libovsdb/client"
Expand Down Expand Up @@ -345,7 +343,7 @@ func hostPhysicalIP(gwConfig util.L3GatewayConfig) string {

func hostIPsFromGWConfig(gwConfig util.L3GatewayConfig) []string {
var hostIPs []string
for _, ip := range append(gwConfig.IPAddresses, dummyJoinIP()) {
for _, ip := range append(gwConfig.IPAddresses, dummyMasqueradeIP()) {
hostIPs = append(hostIPs, ip.IP.String())
}
return hostIPs
Expand Down Expand Up @@ -404,8 +402,8 @@ func icClusterWithDisableSNATTestConfiguration() testConfiguration {
}
}

func newMultiHomedPod(namespace, name, node, podIP string, multiHomingConfigs ...secondaryNetInfo) *v1.Pod {
pod := newPod(namespace, name, node, podIP)
func newMultiHomedPod(testPod testPod, multiHomingConfigs ...secondaryNetInfo) *v1.Pod {
pod := newPod(testPod.namespace, testPod.podName, testPod.nodeName, testPod.podIP)
var secondaryNetworks []nadapi.NetworkSelectionElement
for _, multiHomingConf := range multiHomingConfigs {
if multiHomingConf.isPrimary {
Expand All @@ -427,23 +425,24 @@ func newMultiHomedPod(namespace, name, node, podIP string, multiHomingConfigs ..
serializedNetworkSelectionElements, _ := json.Marshal(secondaryNetworks)
pod.Annotations = map[string]string{nadapi.NetworkAttachmentAnnot: string(serializedNetworkSelectionElements)}
if config.OVNKubernetesFeature.EnableInterconnect {
dummyOVNNetAnnotations := dummyOVNPodNetworkAnnotations(multiHomingConfigs)
dummyOVNNetAnnotations := dummyOVNPodNetworkAnnotations(testPod.secondaryPodInfos, multiHomingConfigs)
if dummyOVNNetAnnotations != "{}" {
pod.Annotations["k8s.ovn.org/pod-networks"] = dummyOVNNetAnnotations
}
}
return pod
}

func dummyOVNPodNetworkAnnotations(multiHomingConfigs []secondaryNetInfo) string {
func dummyOVNPodNetworkAnnotations(secondaryPodInfos map[string]*secondaryPodInfo, multiHomingConfigs []secondaryNetInfo) string {
var ovnPodNetworksAnnotations []byte
podAnnotations := map[string]podAnnotation{}
for i, netConfig := range multiHomingConfigs {
// we need to inject a dummy OVN annotation into the pods for each multihoming config
// for layer2 topology since allocating the annotation for this cluster configuration
// is performed by cluster manager - which doesn't exist in the unit tests.
if netConfig.topology == ovntypes.Layer2Topology {
podAnnotations[netConfig.nadName] = dummyOVNPodNetworkAnnotationForNetwork(netConfig, i+1)
portInfo := secondaryPodInfos[netConfig.netName].allportInfo[netConfig.nadName]
podAnnotations[netConfig.nadName] = dummyOVNPodNetworkAnnotationForNetwork(portInfo, netConfig, i+1)
}
}

Expand All @@ -455,40 +454,32 @@ func dummyOVNPodNetworkAnnotations(multiHomingConfigs []secondaryNetInfo) string
return string(ovnPodNetworksAnnotations)
}

func dummyOVNPodNetworkAnnotationForNetwork(netConfig secondaryNetInfo, tunnelID int) podAnnotation {
func dummyOVNPodNetworkAnnotationForNetwork(portInfo portInfo, netConfig secondaryNetInfo, tunnelID int) podAnnotation {
role := ovntypes.NetworkRoleSecondary
if netConfig.isPrimary {
role = ovntypes.NetworkRolePrimary
}
var (
gateways []string
ips []string
)
var gateways []string
for _, subnetStr := range strings.Split(netConfig.clustersubnets, ",") {
subnet := testing.MustParseIPNet(subnetStr)
ips = append(ips, GetWorkloadSecondaryNetworkDummyIP(subnet).String())
gateways = append(gateways, util.GetNodeGatewayIfAddr(subnet).IP.String())
}
ip := testing.MustParseIP(portInfo.podIP)
_, maskSize := util.GetIPFullMask(ip).Size()
ipNet := net.IPNet{
IP: ip,
Mask: net.CIDRMask(portInfo.prefixLen, maskSize),
}
return podAnnotation{
IPs: ips,
MAC: util.IPAddrToHWAddr(testing.MustParseIPNet(ips[0]).IP).String(),
IPs: []string{ipNet.String()},
MAC: util.IPAddrToHWAddr(ip).String(),
Gateways: gateways,
Routes: nil, // TODO: must add here the expected routes.
TunnelID: tunnelID,
Role: role,
}
}

// GetWorkloadSecondaryNetworkDummyIP returns the workload logical switch port
// address (the ".3" address), return nil if the subnet is invalid
func GetWorkloadSecondaryNetworkDummyIP(subnet *net.IPNet) *net.IPNet {
mgmtIfAddr := util.GetNodeManagementIfAddr(subnet)
if mgmtIfAddr == nil {
return nil
}
return &net.IPNet{IP: iputils.NextIP(mgmtIfAddr.IP), Mask: subnet.Mask}
}

// Internal struct used to marshal PodAnnotation to the pod annotationç
// Copied from pkg/util/pod_annotation.go
type podAnnotation struct {
Expand Down
35 changes: 17 additions & 18 deletions go-controller/pkg/ovn/secondary_layer2_network_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ var _ = Describe("OVN Multi-Homed pod operations for layer2 network", func() {
&v1.NodeList{Items: []v1.Node{*testNode}},
&v1.PodList{
Items: []v1.Pod{
*newMultiHomedPod(podInfo.namespace, podInfo.podName, podInfo.nodeName, podInfo.podIP, netInfo),
*newMultiHomedPod(podInfo, netInfo),
},
},
&nadapi.NetworkAttachmentDefinitionList{
Expand Down Expand Up @@ -160,21 +160,21 @@ var _ = Describe("OVN Multi-Homed pod operations for layer2 network", func() {
nonICClusterTestConfiguration(),
),

table.Entry("pod on a user defined primary network on an IC cluster",
table.Entry("pod on a user defined primary network",
dummyPrimaryLayer2UserDefinedNetwork("100.200.0.0/16"),
icClusterTestConfiguration(),
nonICClusterTestConfiguration(),
),

table.Entry("pod on a user defined secondary network",
table.Entry("pod on a user defined secondary network on an IC cluster",
dummySecondaryLayer2UserDefinedNetwork("100.200.0.0/16"),
nonICClusterTestConfiguration(),
icClusterTestConfiguration(),
),

table.Entry("pod on a user defined primary network on an IC cluster",
dummyPrimaryLayer2UserDefinedNetwork("100.200.0.0/16"),
icClusterTestConfiguration(),
),
table.Entry("pod on a user defined primary network on an IC cluster",
table.Entry("pod on a user defined primary network on an IC cluster with per-pod SNATs enabled",
dummyPrimaryLayer2UserDefinedNetwork("100.200.0.0/16"),
icClusterWithDisableSNATTestConfiguration(),
),
Expand Down Expand Up @@ -230,7 +230,7 @@ var _ = Describe("OVN Multi-Homed pod operations for layer2 network", func() {
},
&v1.PodList{
Items: []v1.Pod{
*newMultiHomedPod(podInfo.namespace, podInfo.podName, podInfo.nodeName, podInfo.podIP, netInfo),
*newMultiHomedPod(podInfo, netInfo),
},
},
&nadapi.NetworkAttachmentDefinitionList{
Expand Down Expand Up @@ -283,11 +283,11 @@ var _ = Describe("OVN Multi-Homed pod operations for layer2 network", func() {
dummyLayer2PrimaryUserDefinedNetwork("192.168.0.0/16"),
nonICClusterTestConfiguration(),
),
table.Entry("pod on a user defined primary network on an interconnect cluster",
table.Entry("pod on a user defined primary network on an IC cluster",
dummyLayer2PrimaryUserDefinedNetwork("192.168.0.0/16"),
icClusterTestConfiguration(),
),
table.Entry("pod on a user defined primary network on an interconnect cluster",
table.Entry("pod on a user defined primary network on an IC cluster with per-pod SNATs enabled",
dummyLayer2PrimaryUserDefinedNetwork("192.168.0.0/16"),
icClusterWithDisableSNATTestConfiguration(),
),
Expand Down Expand Up @@ -387,7 +387,7 @@ func expectedLayer2EgressEntities(netInfo util.NetInfo, gwConfig util.L3GatewayC

var nat []string
if config.Gateway.DisableSNATMultipleGWs {
nat = append(nat, perPodSNAT)
nat = append(nat, nat1, perPodSNAT)
} else {
nat = append(nat, nat1, nat2, nat3)
}
Expand All @@ -402,7 +402,7 @@ func expectedLayer2EgressEntities(netInfo util.NetInfo, gwConfig util.L3GatewayC
Options: gwRouterOptions(gwConfig),
Policies: []string{routerPolicyUUID1},
},
expectedGWToNetworkSwitchRouterPort(gwRouterToNetworkSwitchPortName, netInfo, gwRouterIPAddress(), layer2SubnetGWAddr()),
expectedGWToNetworkSwitchRouterPort(gwRouterToNetworkSwitchPortName, netInfo, gwRouterJoinIPAddress(), layer2SubnetGWAddr()),
expectedGRStaticRoute(sr1, dummyMasqueradeSubnet().String(), nextHopMasqueradeIP().String(), nil, &staticRouteOutputPort, netInfo),
expectedGRStaticRoute(sr2, ipv4DefaultRoute().String(), nodeGateway().IP.String(), nil, &staticRouteOutputPort, netInfo),
expectedGRToExternalSwitchLRP(gwRouterName, netInfo, nodePhysicalIPAddress(), udnGWSNATAddress()),
Expand All @@ -411,15 +411,14 @@ func expectedLayer2EgressEntities(netInfo util.NetInfo, gwConfig util.L3GatewayC
expectedLogicalRouterPolicy(routerPolicyUUID1, netInfo, nodeName, nodeIP().IP.String(), managementPortIP(layer2Subnet()).String()),
}

for _, entity := range expectedExternalSwitchAndLSPs(netInfo, gwConfig, nodeName) {
expectedEntities = append(expectedEntities, entity)
}
expectedEntities = append(expectedEntities, expectedExternalSwitchAndLSPs(netInfo, gwConfig, nodeName)...)
if config.Gateway.DisableSNATMultipleGWs {
expectedEntities = append(expectedEntities, newNATEntry(perPodSNAT, dummyJoinIP().IP.String(), dummyL2TestPodAdditionalNetworkIP(), nil))
expectedEntities = append(expectedEntities, newNATEntry(nat1, dummyMasqueradeIP().IP.String(), gwRouterJoinIPAddress().IP.String(), standardNonDefaultNetworkExtIDs(netInfo)))
expectedEntities = append(expectedEntities, newNATEntry(perPodSNAT, dummyMasqueradeIP().IP.String(), dummyL2TestPodAdditionalNetworkIP(), nil))
} else {
expectedEntities = append(expectedEntities, newNATEntry(nat1, dummyJoinIP().IP.String(), gwRouterIPAddress().IP.String(), standardNonDefaultNetworkExtIDs(netInfo)))
expectedEntities = append(expectedEntities, newNATEntry(nat2, dummyJoinIP().IP.String(), layer2Subnet().String(), standardNonDefaultNetworkExtIDs(netInfo)))
expectedEntities = append(expectedEntities, newNATEntry(nat3, dummyJoinIP().IP.String(), layer2SubnetGWAddr().IP.String(), standardNonDefaultNetworkExtIDs(netInfo)))
expectedEntities = append(expectedEntities, newNATEntry(nat1, dummyMasqueradeIP().IP.String(), gwRouterJoinIPAddress().IP.String(), standardNonDefaultNetworkExtIDs(netInfo)))
expectedEntities = append(expectedEntities, newNATEntry(nat2, dummyMasqueradeIP().IP.String(), layer2Subnet().String(), standardNonDefaultNetworkExtIDs(netInfo)))
expectedEntities = append(expectedEntities, newNATEntry(nat3, dummyMasqueradeIP().IP.String(), layer2SubnetGWAddr().IP.String(), standardNonDefaultNetworkExtIDs(netInfo)))
}
return expectedEntities
}
Expand Down
Loading

0 comments on commit c960d47

Please sign in to comment.