Skip to content

Commit 7588fd3

Browse files
martinkennellymartinkennelly
committed
EgressIP: fix startup sync to add metadata
Initial implementations erroneously assumed a CIDR for NATs logicalIP. Also, eip controller expects all OVN constructs that support EIP to have this metadata so if we cannot build this metadata then add dummy data so its cleaned up later by EIP controller. This was not caught by unit tests because the unit test also contained the assumption of only logical IP with no mask. It was not caught by upstream CI because we have no reboot tests. Signed-off-by: Martin Kennelly <[email protected]>
1 parent f25f775 commit 7588fd3

File tree

4 files changed

+42
-33
lines changed

4 files changed

+42
-33
lines changed

‎go-controller/pkg/ovn/external_ids_syncer/logical_router_policy/logical_router_policy_sync.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,8 @@ func (syncer *LRPSyncer) syncEgressIPReRoutes() error {
101101
podInfo, err := cache.getPod(podIP)
102102
if err != nil {
103103
klog.Infof("Failed to find Logical Switch Port cache entry for pod IP %s: %v", podIP.String(), err)
104-
continue
104+
// pod not found, add dummy metadata that will be cleaned up by EIP controller sync.
105+
podInfo = podNetInfo{namespace: "UNKNOWN", name: "UNKNOWN"}
105106
}
106107
ipFamily := getIPFamily(isIPv6)
107108
lrp.ExternalIDs = getEgressIPLRPReRouteDbIDs(eipName, podInfo.namespace, podInfo.name, ipFamily, defaultNetworkName, syncer.controllerName).GetExternalIDs()

‎go-controller/pkg/ovn/external_ids_syncer/logical_router_policy/logical_router_policy_sync_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ var _ = ginkgo.Describe("OVN Logical Router Syncer", func() {
122122
map[string]string{"name": egressIPName},
123123
defaultNetworkControllerName)},
124124
finalLRPs: []*nbdb.LogicalRouterPolicy{getReRouteLRP(podNamespace, podName, v4PodIPStr, 0, v4IPFamilyValue, v4PodNextHops,
125-
map[string]string{"name": egressIPName},
125+
getEgressIPLRPReRouteDbIDs(egressIPName, "UNKNOWN", "UNKNOWN", v4IPFamilyValue, defaultNetworkName, defaultNetworkControllerName).GetExternalIDs(),
126126
defaultNetworkControllerName)},
127127
v4ClusterSubnets: []*net.IPNet{v4PodClusterSubnet},
128128
v4JoinSubnet: v4JoinSubnet,

‎go-controller/pkg/ovn/external_ids_syncer/nat/nat_sync.go‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
libovsdbclient "github.com/ovn-org/libovsdb/client"
1111
"github.com/ovn-org/libovsdb/ovsdb"
1212

13+
"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/config"
1314
libovsdbops "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/libovsdb/ops"
1415
"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/nbdb"
1516
ovntypes "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/types"
@@ -86,10 +87,10 @@ func (n *NATSyncer) syncEgressIPNATs() error {
8687
klog.Errorf("Expected NAT %s to contain 'name' as a key within its external IDs", nat.UUID)
8788
continue
8889
}
89-
podIP, _, err := net.ParseCIDR(nat.LogicalIP)
90-
if err != nil {
91-
klog.Errorf("Failed to process logical IP %q of NAT %s", nat.LogicalIP, nat.UUID)
92-
continue
90+
// for egress IP, the logicalIP does not contain a mask.
91+
podIP := net.ParseIP(nat.LogicalIP)
92+
if podIP == nil {
93+
return fmt.Errorf("failed to process logical IP %q of NAT %s", nat.LogicalIP, nat.UUID)
9394
}
9495
isV6 := utilsnet.IsIPv6(podIP)
9596
var ipFamily egressIPFamilyValue
@@ -103,15 +104,15 @@ func (n *NATSyncer) syncEgressIPNATs() error {
103104
pod, found = v4PodCache.getPodByIP(podIP)
104105
}
105106
if !found {
106-
klog.Errorf("Failed to find logical switch port that contains IP address %s", podIP.String())
107-
continue
107+
// set it to unknown and the egress IP controller syncer will take care of removing it.
108+
pod = podNetInfo{namespace: "UNKNOWN", name: "UNKNOWN"}
109+
ipFamily = getFirstSupportIPFamily()
108110
}
109111
nat.ExternalIDs = getEgressIPNATDbIDs(eIPName, pod.namespace, pod.name, ipFamily, n.controllerName).GetExternalIDs()
110112
ops, err = libovsdbops.UpdateNATOps(n.nbClient, ops, nat)
111113
if err != nil {
112114
klog.Errorf("Failed to generate NAT ops for NAT %s: %v", nat.UUID, err)
113115
}
114-
klog.Infof("## martin found %d nats", len(ops))
115116
}
116117

117118
_, err = libovsdbops.TransactAndCheck(n.nbClient, ops)
@@ -176,3 +177,10 @@ func getEgressIPNATDbIDs(eIPName, podNamespace, podName string, ipFamily egressI
176177
libovsdbops.IPFamilyKey: string(ipFamily),
177178
})
178179
}
180+
181+
func getFirstSupportIPFamily() egressIPFamilyValue {
182+
if config.IPv4Mode {
183+
return ipFamilyValueV4
184+
}
185+
return ipFamilyValueV6
186+
}

‎go-controller/pkg/ovn/external_ids_syncer/nat/nat_sync_test.go‎

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -26,22 +26,22 @@ const (
2626
egressIP = "10.10.10.10"
2727
nat1UUID = "nat-1-UUID"
2828
nat2UUID = "nat-2-UUID"
29-
pod1V4CIDRStr = "10.128.0.5/32"
30-
pod1V6CIDRStr = "2001:0000:130F:0000:0000:09C0:876A:130B/128"
29+
pod1V4Str = "10.128.0.5"
30+
pod1V6Str = "2001:0000:130F:0000:0000:09C0:876A:130B"
3131
pod1Namespace = "ns1"
3232
pod1Name = "pod1"
33-
pod2V4CIDRStr = "10.128.0.6/32"
34-
pod2V6CIDRStr = "2001:0000:130F:0000:0000:09C0:876A:130A/128"
33+
pod2V4Str = "10.128.0.6"
34+
pod2V6Str = "2001:0000:130F:0000:0000:09C0:876A:130A"
3535
pod2Namespace = "ns1"
3636
pod2Name = "pod2"
3737
defaultNetworkControllerName = "default-network-controller"
3838
)
3939

4040
var (
41-
pod1V4IPNet = testing.MustParseIPNet(pod1V4CIDRStr)
42-
pod1V6IPNet = testing.MustParseIPNet(pod1V6CIDRStr)
43-
pod2V4IPNet = testing.MustParseIPNet(pod2V4CIDRStr)
44-
pod2V6IPNet = testing.MustParseIPNet(pod2V6CIDRStr)
41+
pod1V4IP = testing.MustParseIP(pod1V4Str)
42+
pod1V6IP = testing.MustParseIP(pod1V6Str)
43+
pod2V4IP = testing.MustParseIP(pod2V4Str)
44+
pod2V6IP = testing.MustParseIP(pod2V6Str)
4545
legacyExtIDs = map[string]string{legacyEIPNameExtIDKey: egressIPName}
4646
pod1V4ExtIDs = getEgressIPNATDbIDs(egressIPName, pod1Namespace, pod1Name, ipFamilyValueV4, defaultNetworkControllerName).GetExternalIDs()
4747
pod1V6ExtIDs = getEgressIPNATDbIDs(egressIPName, pod1Namespace, pod1Name, ipFamilyValueV6, defaultNetworkControllerName).GetExternalIDs()
@@ -54,64 +54,64 @@ var _ = ginkgo.Describe("NAT Syncer", func() {
5454
ginkgo.DescribeTable("egress NATs", func(sync natSync) {
5555
performTest(defaultNetworkControllerName, sync.initialNATs, sync.finalNATs, sync.pods)
5656
}, ginkgo.Entry("converts legacy IPv4 NATs", natSync{
57-
initialNATs: []*nbdb.NAT{getSNAT(nat1UUID, pod1V4CIDRStr, egressIP, legacyExtIDs)},
58-
finalNATs: []*nbdb.NAT{getSNAT(nat1UUID, pod1V4CIDRStr, egressIP, pod1V4ExtIDs)},
57+
initialNATs: []*nbdb.NAT{getSNAT(nat1UUID, pod1V4Str, egressIP, legacyExtIDs)},
58+
finalNATs: []*nbdb.NAT{getSNAT(nat1UUID, pod1V4Str, egressIP, pod1V4ExtIDs)},
5959
pods: podsNetInfo{
6060
{
61-
[]net.IP{pod1V4IPNet.IP},
61+
[]net.IP{pod1V4IP},
6262
pod1Namespace,
6363
pod1Name,
6464
},
6565
{
66-
[]net.IP{pod2V4IPNet.IP},
66+
[]net.IP{pod2V4IP},
6767
pod2Namespace,
6868
pod2Name,
6969
},
7070
},
7171
}),
7272
ginkgo.Entry("converts legacy IPv6 NATs", natSync{
73-
initialNATs: []*nbdb.NAT{getSNAT(nat1UUID, pod1V6CIDRStr, egressIP, legacyExtIDs)},
74-
finalNATs: []*nbdb.NAT{getSNAT(nat1UUID, pod1V6CIDRStr, egressIP, pod1V6ExtIDs)},
73+
initialNATs: []*nbdb.NAT{getSNAT(nat1UUID, pod1V6Str, egressIP, legacyExtIDs)},
74+
finalNATs: []*nbdb.NAT{getSNAT(nat1UUID, pod1V6Str, egressIP, pod1V6ExtIDs)},
7575
pods: podsNetInfo{
7676
{
77-
[]net.IP{pod1V6IPNet.IP},
77+
[]net.IP{pod1V6IP},
7878
pod1Namespace,
7979
pod1Name,
8080
},
8181
{
82-
[]net.IP{pod2V6IPNet.IP},
82+
[]net.IP{pod2V6IP},
8383
pod2Namespace,
8484
pod2Name,
8585
},
8686
},
8787
}),
8888
ginkgo.Entry("converts legacy dual stack NATs", natSync{
89-
initialNATs: []*nbdb.NAT{getSNAT(nat1UUID, pod1V4CIDRStr, egressIP, legacyExtIDs), getSNAT(nat2UUID, pod1V6CIDRStr, egressIP, legacyExtIDs)},
90-
finalNATs: []*nbdb.NAT{getSNAT(nat1UUID, pod1V4CIDRStr, egressIP, pod1V4ExtIDs), getSNAT(nat2UUID, pod1V6CIDRStr, egressIP, pod1V6ExtIDs)},
89+
initialNATs: []*nbdb.NAT{getSNAT(nat1UUID, pod1V4Str, egressIP, legacyExtIDs), getSNAT(nat2UUID, pod1V6Str, egressIP, legacyExtIDs)},
90+
finalNATs: []*nbdb.NAT{getSNAT(nat1UUID, pod1V4Str, egressIP, pod1V4ExtIDs), getSNAT(nat2UUID, pod1V6Str, egressIP, pod1V6ExtIDs)},
9191
pods: podsNetInfo{
9292
{
93-
[]net.IP{pod1V4IPNet.IP, pod1V6IPNet.IP},
93+
[]net.IP{pod1V4IP, pod1V6IP},
9494
pod1Namespace,
9595
pod1Name,
9696
},
9797
{
98-
[]net.IP{pod2V4IPNet.IP, pod2V6IPNet.IP},
98+
[]net.IP{pod2V4IP, pod2V6IP},
9999
pod2Namespace,
100100
pod2Name,
101101
},
102102
},
103103
}),
104104
ginkgo.Entry("doesn't alter NAT with correct external IDs", natSync{
105-
initialNATs: []*nbdb.NAT{getSNAT(nat1UUID, pod1V6CIDRStr, egressIP, pod1V6ExtIDs)},
106-
finalNATs: []*nbdb.NAT{getSNAT(nat1UUID, pod1V6CIDRStr, egressIP, pod1V6ExtIDs)},
105+
initialNATs: []*nbdb.NAT{getSNAT(nat1UUID, pod1V6Str, egressIP, pod1V6ExtIDs)},
106+
finalNATs: []*nbdb.NAT{getSNAT(nat1UUID, pod1V6Str, egressIP, pod1V6ExtIDs)},
107107
pods: podsNetInfo{
108108
{
109-
[]net.IP{pod1V4IPNet.IP, pod1V6IPNet.IP},
109+
[]net.IP{pod1V4IP, pod1V6IP},
110110
pod1Namespace,
111111
pod1Name,
112112
},
113113
{
114-
[]net.IP{pod2V4IPNet.IP, pod2V6IPNet.IP},
114+
[]net.IP{pod2V4IP, pod2V6IP},
115115
pod2Namespace,
116116
pod2Name,
117117
},

0 commit comments

Comments
 (0)