summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--python/vyos/ifconfig/bond.py22
-rwxr-xr-xsmoketest/scripts/cli/test_interfaces_bonding.py24
-rwxr-xr-xsrc/conf_mode/interfaces-bonding.py42
3 files changed, 59 insertions, 29 deletions
diff --git a/python/vyos/ifconfig/bond.py b/python/vyos/ifconfig/bond.py
index 67dcd2b69..c33cf30bf 100644
--- a/python/vyos/ifconfig/bond.py
+++ b/python/vyos/ifconfig/bond.py
@@ -1,4 +1,4 @@
-# Copyright 2019 VyOS maintainers and contributors <maintainers@vyos.io>
+# Copyright 2019-2020 VyOS maintainers and contributors <maintainers@vyos.io>
#
# This library is free software; you can redistribute it and/or
# modify it under the terms of the GNU Lesser General Public
@@ -381,9 +381,14 @@ class BondIf(Interface):
# Some interface options can only be changed if the interface is
# administratively down
if self.get_admin_state() == 'down':
- # Delete bond member port(s)
+ # Remove ALL bond member interfaces
for interface in self.get_slaves():
self.del_port(interface)
+ # Removing an interface from a bond will always place the underlaying
+ # physical interface in admin-down state! If physical interface is
+ # not disabled, re-enable it.
+ if not vyos_dict_search(f'member.interface_remove.{interface}.disable', config):
+ Interface(interface).set_admin_state('up')
# Bonding policy/mode
value = config.get('mode')
@@ -391,13 +396,12 @@ class BondIf(Interface):
# Add (enslave) interfaces to bond
value = vyos_dict_search('member.interface', config)
- if value:
- for interface in value:
- # if we've come here we already verified the interface
- # does not have an addresses configured so just flush
- # any remaining ones
- Interface(interface).flush_addrs()
- self.add_port(interface)
+ for interface in (value or []):
+ # if we've come here we already verified the interface
+ # does not have an addresses configured so just flush
+ # any remaining ones
+ Interface(interface).flush_addrs()
+ self.add_port(interface)
# Primary device interface - must be set after 'mode'
value = config.get('primary')
diff --git a/smoketest/scripts/cli/test_interfaces_bonding.py b/smoketest/scripts/cli/test_interfaces_bonding.py
index e3d3b25ee..b165883b9 100755
--- a/smoketest/scripts/cli/test_interfaces_bonding.py
+++ b/smoketest/scripts/cli/test_interfaces_bonding.py
@@ -20,6 +20,7 @@ import unittest
from base_interfaces_test import BasicInterfaceTest
from vyos.ifconfig import Section
+from vyos.ifconfig.interface import Interface
from vyos.configsession import ConfigSessionError
from vyos.util import read_file
@@ -57,5 +58,28 @@ class BondingInterfaceTest(BasicInterfaceTest.BaseTest):
slaves = read_file(f'/sys/class/net/{interface}/bonding/slaves').split()
self.assertListEqual(slaves, self._members)
+ def test_remove_member(self):
+ """ T2515: when removing a bond member the interface must be admin-up again """
+
+ # configure member interfaces
+ for interface in self._interfaces:
+ for option in self._options.get(interface, []):
+ self.session.set(self._base_path + [interface] + option.split())
+
+ self.session.commit()
+
+ # remove single bond member port
+ for interface in self._interfaces:
+ remove_member = self._members[0]
+ self.session.delete(self._base_path + [interface, 'member', 'interface', remove_member])
+
+ self.session.commit()
+
+ # removed member port must be admin-up
+ for interface in self._interfaces:
+ remove_member = self._members[0]
+ state = Interface(remove_member).get_admin_state()
+ self.assertEqual('up', state)
+
if __name__ == '__main__':
unittest.main()
diff --git a/src/conf_mode/interfaces-bonding.py b/src/conf_mode/interfaces-bonding.py
index 16e6e4f6e..a9679b47c 100755
--- a/src/conf_mode/interfaces-bonding.py
+++ b/src/conf_mode/interfaces-bonding.py
@@ -29,6 +29,7 @@ from vyos.configverify import verify_source_interface
from vyos.configverify import verify_vlan_config
from vyos.configverify import verify_vrf
from vyos.ifconfig import BondIf
+from vyos.ifconfig import Section
from vyos.validate import is_member
from vyos.validate import has_address_configured
from vyos import ConfigError
@@ -69,31 +70,33 @@ def get_config(config=None):
# into a dictionary - we will use this to add additional information
# later on for wach member
if 'member' in bond and 'interface' in bond['member']:
- # first convert it to a list if only one member is given
- if isinstance(bond['member']['interface'], str):
- bond['member']['interface'] = [bond['member']['interface']]
-
- tmp={}
- for interface in bond['member']['interface']:
- tmp.update({interface: {}})
-
- bond['member']['interface'] = tmp
+ # convert list if member interfaces to a dictionary
+ bond['member']['interface'] = dict.fromkeys(
+ bond['member']['interface'], {})
if 'mode' in bond:
bond['mode'] = get_bond_mode(bond['mode'])
tmp = leaf_node_changed(conf, ['mode'])
- if tmp:
- bond.update({'shutdown_required': ''})
+ if tmp: bond.update({'shutdown_required': {}})
# determine which members have been removed
- tmp = leaf_node_changed(conf, ['member', 'interface'])
- if tmp:
- bond.update({'shutdown_required': ''})
- if 'member' in bond:
- bond['member'].update({'interface_remove': tmp })
- else:
- bond.update({'member': {'interface_remove': tmp }})
+ interfaces_removed = leaf_node_changed(conf, ['member', 'interface'])
+ if interfaces_removed:
+ bond.update({'shutdown_required': {}})
+ if 'member' not in bond:
+ bond.update({'member': {}})
+
+ tmp = {}
+ for interface in interfaces_removed:
+ section = Section.section(interface) # this will be 'ethernet' for 'eth0'
+ if conf.exists(['insterfaces', section, interface, 'disable']):
+ tmp.update({interface : {'disable': ''}})
+ else:
+ tmp.update({interface : {}})
+
+ # also present the interfaces to be removed from the bond as dictionary
+ bond['member'].update({'interface_remove': tmp})
if 'member' in bond and 'interface' in bond['member']:
for interface, interface_config in bond['member']['interface'].items():
@@ -109,8 +112,7 @@ def get_config(config=None):
# bond members must not have an assigned address
tmp = has_address_configured(conf, interface)
- if tmp:
- interface_config.update({'has_address' : ''})
+ if tmp: interface_config.update({'has_address' : ''})
return bond