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

@@ -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

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
 

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')