Merge pull request #215 from tsorya/igal/ovn_ip_filter

OCPBUGS-6004: filter ovn fd69::2 ip from list of ips in case ovn

cmd/runtimecfg/node-ip.go

@@ -24,9 +24,11 @@ const (
 	nodeIpNotMatchesVipsFile = "/run/nodeip-configuration/remote-worker"
 	crioSvcOverridePath      = "/etc/systemd/system/crio.service.d/20-nodenet.conf"
 	remoteWorkerLabel        = "node.openshift.io/remote-worker"
+	ovn                      = "OVNKubernetes"
 )
 
 var retry, preferIPv6 bool
+var networkType string
 var nodeIPCmd = &cobra.Command{
 	Use:                   "node-ip",
 	DisableFlagsInUseLine: true,
@@ -66,6 +68,7 @@ func init() {
 	nodeIPCmd.AddCommand(nodeIPSetCmd)
 	nodeIPCmd.PersistentFlags().BoolVarP(&retry, "retry-on-failure", "r", false, "Keep retrying until it finds a suitable IP address. System errors will still abort")
 	nodeIPCmd.PersistentFlags().BoolVarP(&preferIPv6, "prefer-ipv6", "6", false, "Prefer IPv6 addresses to IPv4")
+	nodeIPCmd.PersistentFlags().StringVarP(&networkType, "network-type", "n", ovn, "CNI network type")
 	rootCmd.AddCommand(nodeIPCmd)
 }
 
@@ -75,7 +78,7 @@ func show(cmd *cobra.Command, args []string) error {
 		return err
 	}
 
-	chosenAddresses, _, err := getSuitableIPs(retry, vips, preferIPv6)
+	chosenAddresses, _, err := getSuitableIPs(retry, vips, preferIPv6, networkType)
 	if err != nil {
 		return err
 	}
@@ -98,7 +101,7 @@ func set(cmd *cobra.Command, args []string) error {
 		return err
 	}
 
-	chosenAddresses, matchesVips, err := getSuitableIPs(retry, vips, preferIPv6)
+	chosenAddresses, matchesVips, err := getSuitableIPs(retry, vips, preferIPv6, networkType)
 	if err != nil {
 		return err
 	}
@@ -202,12 +205,13 @@ func checkAddressUsable(chosen []net.IP) (err error) {
 	return err
 }
 
-func getSuitableIPs(retry bool, vips []net.IP, preferIPv6 bool) (chosen []net.IP, matchesVips bool, err error) {
+func getSuitableIPs(retry bool, vips []net.IP, preferIPv6 bool, networkType string) (chosen []net.IP, matchesVips bool, err error) {
 	// Enable debug logging in utils package
 	utils.SetDebugLogLevel()
+	ipFilterFunc := utils.ValidNodeAddress
 	for {
 		if len(vips) > 0 {
-			chosen, err = utils.AddressesRouting(vips, utils.ValidNodeAddress)
+			chosen, err = utils.AddressesRouting(vips, ipFilterFunc)
 			if len(chosen) > 0 || err != nil {
 				if err == nil {
 					err = checkAddressUsable(chosen)
@@ -223,7 +227,11 @@ func getSuitableIPs(retry bool, vips []net.IP, preferIPv6 bool) (chosen []net.IP
 			}
 		}
 		if len(chosen) == 0 {
-			chosen, err = utils.AddressesDefault(preferIPv6, utils.ValidNodeAddress)
+			// we should check ovn specific in case vips were not set
+			if networkType == ovn {
+				ipFilterFunc = utils.ValidOVNNodeAddress
+			}
+			chosen, err = utils.AddressesDefault(preferIPv6, ipFilterFunc)
 			if len(chosen) > 0 || err != nil {
 				if err == nil {
 					err = checkAddressUsable(chosen)

pkg/utils/addresses.go

@@ -100,6 +100,16 @@ func ValidNodeAddress(address netlink.Addr) bool {
 	return true
 }
 
+// ValidOVNNodeAddress returns true if the address is suitable for a node's primary IP
+// and is not fd69::2
+// we intentionally don't filter ipv4 because it isn't necessary.
+func ValidOVNNodeAddress(address netlink.Addr) bool {
+	if IsIPv6(address.IP) && address.IP.String() == "fd69::2" {
+		return false
+	}
+	return ValidNodeAddress(address)
+}
+
 // usableIPv6Route returns true if the passed route is acceptable for AddressesRouting
 func usableIPv6Route(route netlink.Route) bool {
 	// Ignore default routes

pkg/utils/addresses_test.go

@@ -98,6 +98,7 @@ func addIPv6Addrs(addrs map[netlink.Link][]netlink.Addr, af AddressFilter) {
 	maybeAddAddress(addrs, af, lo, "::1/128", false)
 	maybeAddAddress(addrs, af, eth0, "fd00::5/64", false)
 	maybeAddAddress(addrs, af, eth0, "fe80::1234/64", false)
+	maybeAddAddress(addrs, af, eth1, "fd69::2/125", false)
 	maybeAddAddress(addrs, af, eth1, "fd01::3/64", true)
 	maybeAddAddress(addrs, af, eth1, "fd01::4/64", true)
 	maybeAddAddress(addrs, af, eth1, "fd01::5/64", false)
@@ -110,6 +111,15 @@ func addIPv6Routes(routes map[int][]netlink.Route, rf RouteFilter) {
 	maybeAddRoute(routes, rf, eth1, "fd01::/64", false, 100, "")
 }
 
+func addIPv6AddrsOVN(addrs map[netlink.Link][]netlink.Addr, af AddressFilter) {
+	maybeAddAddress(addrs, af, eth0, "fd69::2/125", false)
+}
+
+func addIPv6RoutesWithOVN(routes map[int][]netlink.Route, rf RouteFilter) {
+	maybeAddRoute(routes, rf, eth0, "", false, 100, "")
+	maybeAddRoute(routes, rf, eth0, "fd69::/64", false, 48, "")
+}
+
 func addOverlappingIPv6Addrs(addrs map[netlink.Link][]netlink.Addr, af AddressFilter) {
 	maybeAddAddress(addrs, af, lo, "127.0.0.1/8", false)
 	maybeAddAddress(addrs, af, lo, "::1/128", false)
@@ -189,6 +199,17 @@ func ipv6AddrMap(af AddressFilter) (map[netlink.Link][]netlink.Addr, error) {
 	return addrs, nil
 }
 
+func ipv6AddrMapOVN(af AddressFilter) (map[netlink.Link][]netlink.Addr, error) {
+	addrs := make(map[netlink.Link][]netlink.Addr)
+	addIPv6AddrsOVN(addrs, af)
+	return addrs, nil
+}
+func ipv6RouteMapOVN(rf RouteFilter) (map[int][]netlink.Route, error) {
+	routes := make(map[int][]netlink.Route)
+	addIPv6RoutesWithOVN(routes, rf)
+	return routes, nil
+}
+
 func ipv6RouteMap(rf RouteFilter) (map[int][]netlink.Route, error) {
 	routes := make(map[int][]netlink.Route)
 	addIPv6Routes(routes, rf)
@@ -394,6 +415,30 @@ var _ = Describe("addresses", func() {
 		Expect(addrs).To(Equal([]net.IP{net.ParseIP("fd00::5")}))
 	})
 
+	It("finds an interface with a default route in ovn in an IPv6 cluster", func() {
+
+		By("Verify ovn ip will be chosen without filter")
+		addrs, err := addressesDefaultInternal(
+			true,
+			ValidNodeAddress,
+			ipv6AddrMapOVN,
+			ipv6RouteMapOVN,
+		)
+		Expect(err).NotTo(HaveOccurred())
+		Expect(addrs).To(Equal([]net.IP{net.ParseIP("fd69::2")}))
+
+		By("Verify ovn ip will not be chosen with filter")
+		addrs, err = addressesDefaultInternal(
+			true,
+			ValidOVNNodeAddress,
+			ipv6AddrMapOVN,
+			ipv6RouteMapOVN,
+		)
+		Expect(err).NotTo(HaveOccurred())
+		Expect(addrs).To(Equal([]net.IP{}))
+
+	})
+
 	It("finds an interface with a default route in a dual-stack cluster", func() {
 		addrs, err := addressesDefaultInternal(
 			false,