From 32c4de3612c1fbaca0e62fc6ea689b8975e63042 Mon Sep 17 00:00:00 2001 From: zhangzujian Date: Thu, 13 Jun 2024 10:15:35 +0000 Subject: [PATCH] fix reconcile routes Signed-off-by: zhangzujian --- pkg/daemon/controller_linux.go | 39 +++++++---- pkg/daemon/controller_windows.go | 3 +- pkg/daemon/ovs_linux.go | 15 ++++- test/e2e/framework/iproute/iproute.go | 16 ++++- test/e2e/kube-ovn/node/node.go | 94 +++++++++++++-------------- 5 files changed, 100 insertions(+), 67 deletions(-) diff --git a/pkg/daemon/controller_linux.go b/pkg/daemon/controller_linux.go index f58e29897de..bcf9c136385 100644 --- a/pkg/daemon/controller_linux.go +++ b/pkg/daemon/controller_linux.go @@ -16,6 +16,7 @@ import ( "github.com/kubeovn/felix/ipsets" "github.com/kubeovn/go-iptables/iptables" "github.com/vishvananda/netlink" + "golang.org/x/sys/unix" v1 "k8s.io/api/core/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/labels" @@ -270,13 +271,18 @@ func (c *Controller) reconcileRouters(event *subnetEvent) error { return err } nodeIPv4, nodeIPv6 := util.GetNodeInternalIP(*node) + var joinIPv4, joinIPv6 string + if len(node.Annotations) != 0 { + joinIPv4, joinIPv6 = util.SplitStringIP(node.Annotations[util.IPAddressAnnotation]) + } joinCIDR := make([]string, 0, 2) cidrs := make([]string, 0, len(subnets)*2) for _, subnet := range subnets { // The route for overlay subnet cidr via ovn0 should not be deleted even though subnet.Status has changed to not ready if subnet.Spec.Vpc != c.config.ClusterRouter || - (subnet.Spec.Vlan != "" && !subnet.Spec.LogicalGateway && (!subnet.Spec.U2OInterconnection || (subnet.Spec.EnableLb != nil && *subnet.Spec.EnableLb))) { + (subnet.Spec.Vlan != "" && !subnet.Spec.LogicalGateway && (!subnet.Spec.U2OInterconnection || (subnet.Spec.EnableLb != nil && *subnet.Spec.EnableLb))) || + (subnet.Name != c.config.NodeSwitch && !subnet.Status.IsReady()) { continue } @@ -319,7 +325,7 @@ func (c *Controller) reconcileRouters(event *subnetEvent) error { klog.Error(err) return err } - toAdd, toDel := routeDiff(nodeNicRoutes, allRoutes, cidrs, joinCIDR, gateway, net.ParseIP(nodeIPv4), net.ParseIP(nodeIPv6)) + toAdd, toDel := routeDiff(nodeNicRoutes, allRoutes, cidrs, joinCIDR, joinIPv4, joinIPv6, gateway, net.ParseIP(nodeIPv4), net.ParseIP(nodeIPv6)) for _, r := range toDel { if err = netlink.RouteDel(&netlink.Route{Dst: r.Dst}); err != nil { klog.Errorf("failed to del route %v", err) @@ -353,7 +359,7 @@ func getNicExistRoutes(nic netlink.Link, gateway string) ([]netlink.Route, error return existRoutes, nil } -func routeDiff(nodeNicRoutes, allRoutes []netlink.Route, cidrs, joinCIDR []string, gateway string, srcIPv4, srcIPv6 net.IP) (toAdd, toDel []netlink.Route) { +func routeDiff(nodeNicRoutes, allRoutes []netlink.Route, cidrs, joinCIDR []string, joinIPv4, joinIPv6, gateway string, srcIPv4, srcIPv6 net.IP) (toAdd, toDel []netlink.Route) { for _, route := range nodeNicRoutes { if route.Scope == netlink.SCOPE_LINK || route.Dst == nil || route.Dst.IP.IsLinkLocalUnicast() { continue @@ -382,16 +388,12 @@ func routeDiff(nodeNicRoutes, allRoutes []netlink.Route, cidrs, joinCIDR []strin } } if len(toDel) > 0 { - klog.Infof("route to del %v", toDel) + klog.Infof("routes to delete: %v", toDel) } ipv4, ipv6 := util.SplitStringIP(gateway) gwV4, gwV6 := net.ParseIP(ipv4), net.ParseIP(ipv6) for _, c := range cidrs { - if slices.Contains(joinCIDR, c) { - continue - } - var src, gw net.IP switch util.CheckProtocol(c) { case kubeovnv1.ProtocolIPv4: @@ -427,16 +429,27 @@ func routeDiff(nodeNicRoutes, allRoutes []netlink.Route, cidrs, joinCIDR []strin } if !found { _, cidr, _ := net.ParseCIDR(c) + var proto netlink.RouteProtocol + scope := netlink.SCOPE_UNIVERSE + if slices.Contains(joinCIDR, c) { + ip := joinIPv4 + if util.CheckProtocol(c) == kubeovnv1.ProtocolIPv6 { + ip = joinIPv6 + } + gw, src, scope = nil, net.ParseIP(ip), netlink.SCOPE_LINK + proto = netlink.RouteProtocol(unix.RTPROT_KERNEL) + } toAdd = append(toAdd, netlink.Route{ - Dst: cidr, - Src: src, - Gw: gw, - Scope: netlink.SCOPE_UNIVERSE, + Dst: cidr, + Src: src, + Gw: gw, + Protocol: proto, + Scope: scope, }) } } if len(toAdd) > 0 { - klog.Infof("route to add %v", toAdd) + klog.Infof("routes to add: %v", toAdd) } return } diff --git a/pkg/daemon/controller_windows.go b/pkg/daemon/controller_windows.go index 0406949e78f..7e94d2b745b 100644 --- a/pkg/daemon/controller_windows.go +++ b/pkg/daemon/controller_windows.go @@ -55,7 +55,8 @@ func (c *Controller) reconcileRouters(_ *subnetEvent) error { for _, subnet := range subnets { // The route for overlay subnet cidr via ovn0 should not be deleted even though subnet.Status has changed to not ready if (subnet.Spec.Vlan != "" && !subnet.Spec.LogicalGateway) || - subnet.Spec.Vpc != c.config.ClusterRouter { + subnet.Spec.Vpc != c.config.ClusterRouter || + (subnet.Name != c.config.NodeSwitch && !subnet.Status.IsReady()) { continue } diff --git a/pkg/daemon/ovs_linux.go b/pkg/daemon/ovs_linux.go index 8d709cd6eb6..cec8c1837db 100644 --- a/pkg/daemon/ovs_linux.go +++ b/pkg/daemon/ovs_linux.go @@ -607,9 +607,19 @@ func configureNodeNic(portName, ip, gw, joinCIDR string, macAddr net.HardwareAdd } if !found { _, cidr, _ := net.ParseCIDR(c) + protocol := util.CheckProtocol(c) + var src net.IP + for _, ip := range strings.Split(ip, ",") { + if util.CheckProtocol(ip) == protocol { + src = net.ParseIP(ip) + break + } + } toAdd = append(toAdd, netlink.Route{ - Dst: cidr, - Scope: netlink.SCOPE_UNIVERSE, + Dst: cidr, + Src: src, + Protocol: netlink.RouteProtocol(unix.RTPROT_KERNEL), + Scope: netlink.SCOPE_LINK, }) } } @@ -619,6 +629,7 @@ func configureNodeNic(portName, ip, gw, joinCIDR string, macAddr net.HardwareAdd for _, r := range toAdd { r.LinkIndex = hostLink.Attrs().Index + klog.Infof("adding route %q on %s", r.String(), hostLink.Attrs().Name) if err = netlink.RouteReplace(&r); err != nil && !errors.Is(err, syscall.EEXIST) { klog.Errorf("failed to replace route %v: %v", r, err) } diff --git a/test/e2e/framework/iproute/iproute.go b/test/e2e/framework/iproute/iproute.go index 34ef7722b95..7ac28c5d564 100644 --- a/test/e2e/framework/iproute/iproute.go +++ b/test/e2e/framework/iproute/iproute.go @@ -101,8 +101,10 @@ func (e *execer) exec(cmd string, result interface{}) error { return fmt.Errorf("failed to exec cmd %q: %v\nstdout:\n%s\nstderr:\n%s", cmd, err, stdout, stderr) } - if err = json.Unmarshal(stdout, result); err != nil { - return fmt.Errorf("failed to decode json %q: %v", string(stdout), err) + if result != nil { + if err = json.Unmarshal(stdout, result); err != nil { + return fmt.Errorf("failed to decode json %q: %v", string(stdout), err) + } } return nil @@ -150,6 +152,16 @@ func RouteShow(table, device string, execFunc ExecFunc) ([]Route, error) { return append(routes, routes6...), nil } +func RouteDel(table, dst string, execFunc ExecFunc) error { + e := execer{fn: execFunc} + args := dst + if table != "" { + args = " table " + table + } + + return e.exec("ip route del "+args, nil) +} + func RuleShow(device string, execFunc ExecFunc) ([]Rule, error) { e := execer{fn: execFunc} diff --git a/test/e2e/kube-ovn/node/node.go b/test/e2e/kube-ovn/node/node.go index 35c9a129379..8b7b129f319 100644 --- a/test/e2e/kube-ovn/node/node.go +++ b/test/e2e/kube-ovn/node/node.go @@ -7,9 +7,9 @@ import ( "net" "strconv" "strings" + "time" corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" clientset "k8s.io/client-go/kubernetes" e2enode "k8s.io/kubernetes/test/e2e/framework/node" @@ -90,8 +90,8 @@ var _ = framework.OrderedDescribe("[group:node]", func() { pod.Spec.HostNetwork = true pod = podClient.CreateSync(pod) - ginkgo.By("Checking ip addresses on ovn0") - links, err := iproute.AddressShow("ovn0", func(cmd ...string) ([]byte, []byte, error) { + ginkgo.By("Checking ip addresses on " + util.NodeNic) + links, err := iproute.AddressShow(util.NodeNic, func(cmd ...string) ([]byte, []byte, error) { return framework.KubectlExec(pod.Namespace, pod.Name, cmd...) }) framework.ExpectNoError(err) @@ -107,8 +107,8 @@ var _ = framework.OrderedDescribe("[group:node]", func() { } }) - framework.ConformanceIt("should add default route on node for join subnet", func() { - f.SkipVersionPriorTo(1, 13, "This feature was introduced in v1.13") + framework.ConformanceIt("should add missing routes on node for the join subnet", func() { + f.SkipVersionPriorTo(1, 9, "This feature was introduced in v1.9") ginkgo.By("Getting join subnet") join := subnetClient.Get("join") @@ -127,66 +127,62 @@ var _ = framework.OrderedDescribe("[group:node]", func() { podName = "pod-" + framework.RandomSuffix() ginkgo.By("Creating pod " + podName + " with host network") cmd := []string{"sh", "-c", "sleep infinity"} - pod := framework.MakePod(namespaceName, podName, nil, nil, image, cmd, nil) + pod := framework.MakePrivilegedPod(namespaceName, podName, nil, nil, image, cmd, nil) pod.Spec.NodeName = node.Name pod.Spec.HostNetwork = true pod = podClient.CreateSync(pod) - ginkgo.By("Getting node routes") - nodeRoutes, err := iproute.RouteShow("", "ovn0", func(cmd ...string) ([]byte, []byte, error) { + ginkgo.By("Getting node routes on " + util.NodeNic) + cidrs := strings.Split(join.Spec.CIDRBlock, ",") + execFunc := func(cmd ...string) ([]byte, []byte, error) { return framework.KubectlExec(pod.Namespace, pod.Name, cmd...) - }) + } + routes, err := iproute.RouteShow("", util.NodeNic, execFunc) framework.ExpectNoError(err) - for _, joinCidr := range strings.Split(join.Spec.CIDRBlock, ",") { - found := false - for _, nodeRoute := range nodeRoutes { - if nodeRoute.Dst == joinCidr { - ginkgo.By("Getting ovn0 default route for cidr " + nodeRoute.Dst) - found = true - break - } - if found { + found := make([]bool, len(cidrs)) + for i, cidr := range cidrs { + for _, route := range routes { + if route.Dst == cidr { + framework.Logf("Found route for cidr " + cidr + " on " + util.NodeNic) + found[i] = true break } } - framework.ExpectTrue(found) + } + for i, cidr := range cidrs { + framework.ExpectTrue(found[i], "Route for cidr "+cidr+" not found on "+util.NodeNic) } - ginkgo.By("Getting and recreate kube-ovn-cni pod") - kubePodClient := f.PodClientNS(framework.KubeOvnNamespace) - pods, err := kubePodClient.List(context.Background(), metav1.ListOptions{LabelSelector: metav1.FormatLabelSelector(&metav1.LabelSelector{MatchLabels: map[string]string{"app": "kube-ovn-cni"}}), FieldSelector: fmt.Sprintf("spec.nodeName=%s", node.Name)}) - framework.ExpectNoError(err) - framework.ExpectNotNil(pods) - framework.Logf("Delete kube-ovn-cni pod: %s", pods.Items[0].Name) - err = kubePodClient.Delete(pods.Items[0].Name) - framework.ExpectNoError(err) - kubePodClient.WaitForNotFound(pods.Items[0].Name) + for _, cidr := range strings.Split(join.Spec.CIDRBlock, ",") { + ginkgo.By("Deleting route for " + cidr + " on node " + node.Name) + err = iproute.RouteDel("", cidr, execFunc) + framework.ExpectNoError(err) + } - ginkgo.By("Getting new created kube-ovn-cni pod") - pods, err = kubePodClient.List(context.Background(), metav1.ListOptions{LabelSelector: metav1.FormatLabelSelector(&metav1.LabelSelector{MatchLabels: map[string]string{"app": "kube-ovn-cni"}}), FieldSelector: fmt.Sprintf("spec.nodeName=%s", node.Name)}) - framework.ExpectNoError(err) - framework.ExpectNotNil(pods) - kubePodClient.WaitForRunning(pods.Items[0].Name) - framework.Logf("Get new kube-ovn-cni pod: %s", pods.Items[0].Name) + ginkgo.By("Waiting for routes for subnet " + join.Name + " to be created") + framework.WaitUntil(2*time.Second, 10*time.Second, func(_ context.Context) (bool, error) { + if routes, err = iproute.RouteShow("", util.NodeNic, execFunc); err != nil { + return false, err + } - nodeRoutes, err = iproute.RouteShow("", "ovn0", func(cmd ...string) ([]byte, []byte, error) { - return framework.KubectlExec(pod.Namespace, pod.Name, cmd...) - }) - framework.ExpectNoError(err) - for _, joinCidr := range strings.Split(join.Spec.CIDRBlock, ",") { - found := false - for _, nodeRoute := range nodeRoutes { - if nodeRoute.Dst == joinCidr { - ginkgo.By("Getting ovn0 default route for cidr " + nodeRoute.Dst) - found = true - break + found = make([]bool, len(cidrs)) + for i, cidr := range cidrs { + for _, route := range routes { + if route.Dst == cidr { + framework.Logf("Found route for cidr " + cidr + " on " + util.NodeNic) + found[i] = true + break + } } - if found { - break + } + for i, cidr := range cidrs { + if !found[i] { + framework.Logf("Route for cidr " + cidr + " not found on " + util.NodeNic) + return false, nil } } - framework.ExpectTrue(found) - } + return true, nil + }, "") err = podClient.Delete(podName) framework.ExpectNoError(err)