From abb21efbae9d9ae28cad0db5bd16aa0a0ce8fee4 Mon Sep 17 00:00:00 2001 From: Samuel Walladge Date: Thu, 16 Jun 2022 14:55:20 +0930 Subject: [PATCH] Optionally ignore LACP activity bit in port check If the LACP bond on the switch side is configured in passive mode, the activity mode bit on the port states will usually be mismatched. Add an option to the charm config to inform when this is expected, so that the charm will ignore this mismatched bit when checking ports. Closes-Bug: #1958327 Change-Id: I784bb410110813472e14b3477247fe138db5c214 --- src/config.yaml | 4 ++ src/lib/charms/layer/magpie_tools.py | 14 ++++- unit_tests/test_magpie_tools.py | 81 ++++++++++++++++++++++++++++ 3 files changed, 98 insertions(+), 1 deletion(-) diff --git a/src/config.yaml b/src/config.yaml index 9067c87..ed1a802 100644 --- a/src/config.yaml +++ b/src/config.yaml @@ -35,6 +35,10 @@ options: default: 3 description: Timeout in seconds per DNS query try type: int + lacp_passive_mode: + default: false + description: Set to true if switches are in LACP passive mode. + type: boolean ping_timeout: default: 2 description: Timeout in seconds per ICMP request diff --git a/src/lib/charms/layer/magpie_tools.py b/src/lib/charms/layer/magpie_tools.py index c2c9981..1d0480d 100644 --- a/src/lib/charms/layer/magpie_tools.py +++ b/src/lib/charms/layer/magpie_tools.py @@ -517,12 +517,24 @@ def check_aggregator_id(bond_iface, slave_iface): def check_lacp_port_state(iface): + cfg = hookenv.config() iface_dir = "/sys/class/net/{}/bonding_slave".format(iface) with open("{}/ad_actor_oper_port_state".format(iface_dir)) as fos: actor_port_state = fos.read() with open("{}/ad_partner_oper_port_state".format(iface_dir)) as fos: partner_port_state = fos.read() - if actor_port_state != partner_port_state: + + if ( + actor_port_state != partner_port_state + # check if this is an acceptable mismatch in the LACP activity mode + and not ( + cfg.get('lacp_passive_mode') + # and the only difference is the LACP activity bit + # (1111_1110 bitmask to ignore LACP activity bit in comparison) + and (int(actor_port_state) & 254) == + (int(partner_port_state) & 254) + ) + ): return "lacp_port_state_mismatch" return None diff --git a/unit_tests/test_magpie_tools.py b/unit_tests/test_magpie_tools.py index 20c1bc6..51d9945 100644 --- a/unit_tests/test_magpie_tools.py +++ b/unit_tests/test_magpie_tools.py @@ -1,7 +1,32 @@ +from unittest.mock import ( + patch, + mock_open, +) + import lib.charms.layer.magpie_tools as magpie_tools import unit_tests.test_utils +LACP_STATE_SLOW_ACTIVE = '61' +LACP_STATE_FAST_ACTIVE = '63' +LACP_STATE_SLOW_PASSIVE = '60' + + +def mocked_open_lacp_port_state(actor, partner): + def the_actual_mock(path): + if ( + path == + "/sys/class/net/test/bonding_slave/ad_actor_oper_port_state" + ): + return mock_open(read_data=actor)(path) + elif ( + path == + "/sys/class/net/test/bonding_slave/ad_partner_oper_port_state" + ): + return mock_open(read_data=partner)(path) + return the_actual_mock + + class TestMagpieTools(unit_tests.test_utils.CharmTestCase): def setUp(self): @@ -60,3 +85,59 @@ class TestMagpieTools(unit_tests.test_utils.CharmTestCase): magpie_tools.status_for_speed_check('50%', 300, -1), ', speed failed: link speed undefined' ) + + @patch('lib.charms.layer.magpie_tools.open', + mock_open(read_data=LACP_STATE_SLOW_ACTIVE)) + def test_check_lacp_port_state_match_default(self): + self.hookenv.config.return_value = {} + self.assertIsNone(magpie_tools.check_lacp_port_state('test')) + + @patch('lib.charms.layer.magpie_tools.open', + mock_open(read_data=LACP_STATE_SLOW_ACTIVE)) + def test_check_lacp_port_state_match_explicit_active(self): + self.hookenv.config.return_value = {'lacp_passive_mode': False} + self.assertIsNone(magpie_tools.check_lacp_port_state('test')) + + @patch('lib.charms.layer.magpie_tools.open', + mock_open(read_data=LACP_STATE_SLOW_ACTIVE)) + def test_check_lacp_port_state_match_passive(self): + self.hookenv.config.return_value = {'lacp_passive_mode': True} + self.assertIsNone(magpie_tools.check_lacp_port_state('test')) + + @patch('lib.charms.layer.magpie_tools.open') + def test_check_lacp_port_state_passive_expected_mismatch(self, open_): + open_.side_effect = mocked_open_lacp_port_state( + LACP_STATE_SLOW_ACTIVE, LACP_STATE_SLOW_PASSIVE + ) + self.hookenv.config.return_value = {'lacp_passive_mode': True} + self.assertIsNone(magpie_tools.check_lacp_port_state('test')) + + @patch('lib.charms.layer.magpie_tools.open') + def test_check_lacp_port_state_passive_default(self, open_): + open_.side_effect = mocked_open_lacp_port_state( + LACP_STATE_SLOW_ACTIVE, LACP_STATE_SLOW_PASSIVE + ) + self.hookenv.config.return_value = {} + self.assertEqual( + magpie_tools.check_lacp_port_state('test'), + 'lacp_port_state_mismatch') + + @patch('lib.charms.layer.magpie_tools.open') + def test_check_lacp_port_state_passive_configured_active(self, open_): + open_.side_effect = mocked_open_lacp_port_state( + LACP_STATE_SLOW_ACTIVE, LACP_STATE_SLOW_PASSIVE + ) + self.hookenv.config.return_value = {'lacp_passive_mode': False} + self.assertEqual( + magpie_tools.check_lacp_port_state('test'), + 'lacp_port_state_mismatch') + + @patch('lib.charms.layer.magpie_tools.open') + def test_check_lacp_port_state_passive_unexpected_mismatch(self, open_): + open_.side_effect = mocked_open_lacp_port_state( + LACP_STATE_FAST_ACTIVE, LACP_STATE_SLOW_PASSIVE + ) + self.hookenv.config.return_value = {'lacp_passive_mode': True} + self.assertEqual( + magpie_tools.check_lacp_port_state('test'), + 'lacp_port_state_mismatch')