Merge pull request #388 from jjakob/flush-addrs-T2367

T2367: flush addresses when adding bond/bridge members

3 files changed, 79 insertions, 33 deletions

diff --git a/python/vyos/ifconfig/interface.py b/python/vyos/ifconfig/interface.py
index a156eddd9..6241c83b5 100644
--- a/python/vyos/ifconfig/interface.py
+++ b/python/vyos/ifconfig/interface.py
@@ -219,7 +219,7 @@ class Interface(Control):
             else:
                 raise Exception('interface "{}" not found'.format(self.config['ifname']))
 
-        # list of assigned IP addresses
+        # temporary list of assigned IP addresses
         self._addr = []
 
         self.operational = self.OperationalClass(ifname)
@@ -240,15 +240,11 @@ class Interface(Control):
         >>> i = Interface('eth0')
         >>> i.remove()
         """
-        # stop DHCP(v6) if running
-        self.dhcp.v4.delete()
-        self.dhcp.v6.delete()
 
         # remove all assigned IP addresses from interface - this is a bit redundant
         # as the kernel will remove all addresses on interface deletion, but we
         # can not delete ALL interfaces, see below
-        for addr in self.get_addr():
-            self.del_addr(addr)
+        self.flush_addrs()
 
         # ---------------------------------------------------------------------
         # Any class can define an eternal regex in its definition
@@ -621,7 +617,8 @@ class Interface(Control):
     def add_addr(self, addr):
         """
         Add IP(v6) address to interface. Address is only added if it is not
-        already assigned to that interface.
+        already assigned to that interface. Address format must be validated
+        and compressed/normalized before calling this function.
 
         addr: can be an IPv4 address, IPv6 address, dhcp or dhcpv6!
               IPv4: add IPv4 address to interface
@@ -629,6 +626,7 @@ class Interface(Control):
               dhcp: start dhclient (IPv4) on interface
               dhcpv6: start dhclient (IPv6) on interface
 
+        Returns False if address is already assigned and wasn't re-added.
         Example:
         >>> from vyos.ifconfig import Interface
         >>> j = Interface('eth0')
@@ -637,32 +635,42 @@ class Interface(Control):
         >>> j.get_addr()
         ['192.0.2.1/24', '2001:db8::ffff/64']
         """
-
-        # cache new IP address which is assigned to interface
-        self._addr.append(addr)
-
-        # we can not have both DHCP and static IPv4 addresses assigned to an interface
-        if 'dhcp' in self._addr:
-            for addr in self._addr:
-                # do not change below 'if' ordering esle you will get an exception as:
-                #   ValueError: 'dhcp' does not appear to be an IPv4 or IPv6 address
-                if addr != 'dhcp' and is_ipv4(addr):
-                    raise ConfigError(
-                        "Can't configure both static IPv4 and DHCP address on the same interface")
-
-        if addr == 'dhcp':
-            self.dhcp.v4.set()
-        elif addr == 'dhcpv6':
-            self.dhcp.v6.set()
+        # XXX: normalize/compress with ipaddress if calling functions don't?
+        # is subnet mask always passed, and in the same way?
+        ret = True
+
+        # we can't have both DHCP and static IPv4 addresses assigned
+        for a in self._addr:
+            if ( ( addr == 'dhcp' and a != 'dhcpv6' and is_ipv4(a) ) or
+                    ( a == 'dhcp' and addr != 'dhcpv6' and is_ipv4(addr) ) ):
+                raise ConfigError((
+                    "Can't configure both static IPv4 and DHCP address "
+                    "on the same interface"))
+
+        # do not add same address twice
+        if addr not in self._addr:
+            # add to interface
+            if addr == 'dhcp':
+                self.dhcp.v4.set()
+            elif addr == 'dhcpv6':
+                self.dhcp.v6.set()
+            else:
+                if not is_intf_addr_assigned(self.ifname, addr):
+                    ret = self._cmd(f'ip addr add "{addr}" dev "{self.ifname}"')
+                else:
+                    return False
+            # add to cache
+            self._addr.append(addr)
         else:
-            if not is_intf_addr_assigned(self.config['ifname'], addr):
-                cmd = 'ip addr add "{}" dev "{}"'.format(addr, self.config['ifname'])
-                return self._cmd(cmd)
+            return False
+
+        return ret
 
     def del_addr(self, addr):
         """
-        Delete IP(v6) address to interface. Address is only added if it is
-        assigned to that interface.
+        Delete IP(v6) address from interface. Address is only deleted if it is
+        assigned to that interface. Address format must be exactly the same as
+        was used when adding the address.
 
         addr: can be an IPv4 address, IPv6 address, dhcp or dhcpv6!
               IPv4: delete IPv4 address from interface
@@ -670,6 +678,7 @@ class Interface(Control):
               dhcp: stop dhclient (IPv4) on interface
               dhcpv6: stop dhclient (IPv6) on interface
 
+        Returns False if address isn't already assigned and wasn't deleted.
         Example:
         >>> from vyos.ifconfig import Interface
         >>> j = Interface('eth0')
@@ -681,11 +690,34 @@ class Interface(Control):
         >>> j.get_addr()
         ['2001:db8::ffff/64']
         """
+        ret = True
+
+        # remove from interface
         if addr == 'dhcp':
             self.dhcp.v4.delete()
         elif addr == 'dhcpv6':
             self.dhcp.v6.delete()
         else:
-            if is_intf_addr_assigned(self.config['ifname'], addr):
-                cmd = 'ip addr del "{}" dev "{}"'.format(addr, self.config['ifname'])
-                return self._cmd(cmd)
+            if is_intf_addr_assigned(self.ifname, addr):
+                ret = self._cmd(f'ip addr del "{addr}" dev "{self.ifname}"')
+            else:
+                return False
+
+        if addr in self._addr:
+            # remove from cache
+            self._addr.remove(addr)
+
+        return ret
+
+    def flush_addrs(self):
+        """
+        Flush all addresses from an interface, including DHCP.
+
+        Will raise an exception on error.
+        """
+        # stop DHCP(v6) if running
+        self.dhcp.v4.delete()
+        self.dhcp.v6.delete()
+
+        # flush all addresses
+        self._cmd(f'ip addr flush dev "{self.config["ifname"]}"')
diff --git a/src/conf_mode/interfaces-bonding.py b/src/conf_mode/interfaces-bonding.py
index 34ea4f8f8..a174e33e4 100755
--- a/src/conf_mode/interfaces-bonding.py
+++ b/src/conf_mode/interfaces-bonding.py
@@ -24,7 +24,7 @@ from vyos.ifconfig import BondIf, Section
 from vyos.ifconfig_vlan import apply_vlan_config, verify_vlan_config
 from vyos.configdict import list_diff, intf_to_dict, add_to_dict
 from vyos.config import Config
-from vyos.util import call
+from vyos.util import call, cmd
 from vyos.validate import is_bridge_member
 from vyos import ConfigError
 
@@ -365,6 +365,12 @@ def apply(bond):
 
             # Add (enslave) interfaces to bond
             for intf in bond['member']:
+                # flushes only children of Interfaces class (e.g. vlan are not)
+                if intf in Section.interfaces():
+                    klass = Section.klass(intf, vlan=False)
+                    klass(intf, create=False).flush_addrs()
+                # flushes also vlan interfaces
+                call(f'ip addr flush dev "{intf}"')
                 b.add_port(intf)
 
         # As the bond interface is always disabled first when changing
diff --git a/src/conf_mode/interfaces-bridge.py b/src/conf_mode/interfaces-bridge.py
index 2f92aae09..9d638653c 100755
--- a/src/conf_mode/interfaces-bridge.py
+++ b/src/conf_mode/interfaces-bridge.py
@@ -24,6 +24,7 @@ from vyos.ifconfig import BridgeIf, Section
 from vyos.ifconfig.stp import STP
 from vyos.configdict import list_diff
 from vyos.config import Config
+from vyos.util import cmd
 from vyos import ConfigError
 
 default_config_data = {
@@ -346,6 +347,13 @@ def apply(bridge):
 
         # add interfaces to bridge
         for member in bridge['member']:
+            # flushes address of only children of Interfaces class
+            # (e.g. vlan are not)
+            if member['name'] in Section.interfaces():
+                klass = Section.klass(member['name'], vlan=False)
+                klass(member['name'], create=False).flush_addrs()
+            # flushes all interfaces
+            cmd(f'ip addr flush dev "{member["name"]}"')
             br.add_port(member['name'])
 
         # Change interface MAC address