From d0ae65f5c864350b490ac8363be4f00a6060fd42 Mon Sep 17 00:00:00 2001
From: Jakub Libosvar <libosvar@redhat.com>
Date: Wed, 31 Jul 2024 20:19:53 +0000
Subject: [PATCH] Remove _check_ip_associated method from Event

The method should be a helper function as it does not modify any state
of the Event instance.

Change-Id: Id744b40d3203fa864bbdf6c541ca7c311c16f9ce
Signed-off-by: Jakub Libosvar <libosvar@redhat.com>
(cherry picked from commit eaa906e2e534f03ae9219d99aaa7a3876a9279fa)
---
 ovn_bgp_agent/drivers/openstack/utils/port.py | 17 +++++++++
 .../openstack/watchers/base_watcher.py        |  6 ---
 .../drivers/openstack/watchers/bgp_watcher.py | 17 +++++----
 .../openstack/watchers/evpn_watcher.py        |  9 +++--
 .../openstack/watchers/nb_bgp_watcher.py      | 13 ++++---
 .../unit/drivers/openstack/utils/test_port.py | 37 +++++++++++++++++++
 .../openstack/watchers/test_base_watcher.py   | 33 -----------------
 7 files changed, 75 insertions(+), 57 deletions(-)
 create mode 100644 ovn_bgp_agent/drivers/openstack/utils/port.py
 create mode 100644 ovn_bgp_agent/tests/unit/drivers/openstack/utils/test_port.py

diff --git a/ovn_bgp_agent/drivers/openstack/utils/port.py b/ovn_bgp_agent/drivers/openstack/utils/port.py
new file mode 100644
index 00000000..fca9e17f
--- /dev/null
+++ b/ovn_bgp_agent/drivers/openstack/utils/port.py
@@ -0,0 +1,17 @@
+# Copyright 2024 Red Hat, Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+
+def has_ip_address_defined(address):
+    return ' ' in address.strip()
diff --git a/ovn_bgp_agent/drivers/openstack/watchers/base_watcher.py b/ovn_bgp_agent/drivers/openstack/watchers/base_watcher.py
index 05512c19..8cfc4a73 100644
--- a/ovn_bgp_agent/drivers/openstack/watchers/base_watcher.py
+++ b/ovn_bgp_agent/drivers/openstack/watchers/base_watcher.py
@@ -41,9 +41,6 @@ class PortBindingChassisEvent(Event):
         super().__init__(bgp_agent, events, table)
         self.event_name = self.__class__.__name__
 
-    def _check_ip_associated(self, mac):
-        return len(mac.strip().split(' ')) > 1
-
 
 class OVNLBEvent(Event):
     def __init__(self, bgp_agent, events):
@@ -98,9 +95,6 @@ class LSPChassisEvent(Event):
         super().__init__(bgp_agent, events, table)
         self.event_name = self.__class__.__name__
 
-    def _check_ip_associated(self, mac):
-        return len(mac.strip().split(' ')) > 1
-
     def _get_chassis(self, row, default_type=constants.OVN_VM_VIF_PORT_TYPE):
         return driver_utils.get_port_chassis(row, self.agent.chassis,
                                              default_port_type=default_type)
diff --git a/ovn_bgp_agent/drivers/openstack/watchers/bgp_watcher.py b/ovn_bgp_agent/drivers/openstack/watchers/bgp_watcher.py
index 8d394d6d..e23c855f 100644
--- a/ovn_bgp_agent/drivers/openstack/watchers/bgp_watcher.py
+++ b/ovn_bgp_agent/drivers/openstack/watchers/bgp_watcher.py
@@ -17,6 +17,7 @@ from oslo_config import cfg
 from oslo_log import log as logging
 
 from ovn_bgp_agent import constants
+from ovn_bgp_agent.drivers.openstack.utils import port as port_utils
 from ovn_bgp_agent.drivers.openstack.watchers import base_watcher
 from ovn_bgp_agent.utils import helpers
 
@@ -34,7 +35,7 @@ class PortBindingChassisCreatedEvent(base_watcher.PortBindingChassisEvent):
     def match_fn(self, event, row, old):
         try:
             # single and dual-stack format
-            if not self._check_ip_associated(row.mac[0]):
+            if not port_utils.has_ip_address_defined(row.mac[0]):
                 return False
             if not bool(row.up[0]):
                 return False
@@ -67,7 +68,7 @@ class PortBindingChassisDeletedEvent(base_watcher.PortBindingChassisEvent):
     def match_fn(self, event, row, old):
         try:
             # single and dual-stack format
-            if not self._check_ip_associated(row.mac[0]):
+            if not port_utils.has_ip_address_defined(row.mac[0]):
                 return False
             if event == self.ROW_DELETE:
                 return row.chassis[0].name == self.agent.chassis
@@ -181,7 +182,7 @@ class SubnetRouterAttachedEvent(base_watcher.PortBindingChassisEvent):
     def match_fn(self, event, row, old):
         try:
             # single and dual-stack format
-            if not self._check_ip_associated(row.mac[0]):
+            if not port_utils.has_ip_address_defined(row.mac[0]):
                 return False
             return (not row.chassis and
                     row.logical_port.startswith('lrp-') and
@@ -212,8 +213,8 @@ class SubnetRouterUpdateEvent(base_watcher.PortBindingChassisEvent):
         # mac = [ff:ff:ff:ff:ff:ff subnet1/cidr subnet2/cidr [...]]
         try:
             # single and dual-stack format
-            if (not self._check_ip_associated(row.mac[0]) and
-                    not self._check_ip_associated(old.mac[0])):
+            if (not port_utils.has_ip_address_defined(row.mac[0]) and
+                    not port_utils.has_ip_address_defined(old.mac[0])):
                 return False
             return (
                 not row.chassis and
@@ -240,7 +241,7 @@ class SubnetRouterDetachedEvent(base_watcher.PortBindingChassisEvent):
     def match_fn(self, event, row, old):
         try:
             # single and dual-stack format
-            if not self._check_ip_associated(row.mac[0]):
+            if not port_utils.has_ip_address_defined(row.mac[0]):
                 return False
             return (not row.chassis and
                     row.logical_port.startswith('lrp-') and
@@ -272,7 +273,7 @@ class TenantPortCreatedEvent(base_watcher.PortBindingChassisEvent):
                 if not n_cidrs:
                     return False
             # single and dual-stack format
-            elif not self._check_ip_associated(row.mac[0]):
+            elif not port_utils.has_ip_address_defined(row.mac[0]):
                 return False
             return (not old.chassis and row.chassis and
                     self.agent.ovn_local_lrps != [])
@@ -311,7 +312,7 @@ class TenantPortDeletedEvent(base_watcher.PortBindingChassisEvent):
                 if not n_cidrs:
                     return False
             # single and dual-stack format
-            elif not self._check_ip_associated(row.mac[0]):
+            elif not port_utils.has_ip_address_defined(row.mac[0]):
                 return False
             if event == self.ROW_UPDATE:
                 return (old.chassis and not row.chassis and
diff --git a/ovn_bgp_agent/drivers/openstack/watchers/evpn_watcher.py b/ovn_bgp_agent/drivers/openstack/watchers/evpn_watcher.py
index ddbbe88e..6a089882 100644
--- a/ovn_bgp_agent/drivers/openstack/watchers/evpn_watcher.py
+++ b/ovn_bgp_agent/drivers/openstack/watchers/evpn_watcher.py
@@ -16,6 +16,7 @@ from oslo_concurrency import lockutils
 from oslo_log import log as logging
 
 from ovn_bgp_agent import constants
+from ovn_bgp_agent.drivers.openstack.utils import port as port_utils
 from ovn_bgp_agent.drivers.openstack.watchers import base_watcher
 
 
@@ -35,7 +36,7 @@ class PortBindingChassisCreatedEvent(base_watcher.PortBindingChassisEvent):
             if row.type != constants.OVN_CHASSISREDIRECT_VIF_PORT_TYPE:
                 return False
             # single and dual-stack format
-            if not self._check_ip_associated(row.mac[0]):
+            if not port_utils.has_ip_address_defined(row.mac[0]):
                 return False
             return (row.chassis[0].name == self.agent.chassis and
                     not old.chassis)
@@ -60,7 +61,7 @@ class PortBindingChassisDeletedEvent(base_watcher.PortBindingChassisEvent):
             if row.type != constants.OVN_CHASSISREDIRECT_VIF_PORT_TYPE:
                 return False
             # single and dual-stack format
-            if not self._check_ip_associated(row.mac[0]):
+            if not port_utils.has_ip_address_defined(row.mac[0]):
                 return False
             if event == self.ROW_UPDATE:
                 return (old.chassis[0].name == self.agent.chassis and
@@ -156,7 +157,7 @@ class TenantPortCreatedEvent(base_watcher.PortBindingChassisEvent):
     def match_fn(self, event, row, old):
         try:
             # single and dual-stack format
-            if not self._check_ip_associated(row.mac[0]):
+            if not port_utils.has_ip_address_defined(row.mac[0]):
                 return False
             return (not old.chassis and row.chassis and
                     self.agent.ovn_local_lrps != [])
@@ -181,7 +182,7 @@ class TenantPortDeletedEvent(base_watcher.PortBindingChassisEvent):
     def match_fn(self, event, row, old):
         try:
             # single and dual-stack format
-            if not self._check_ip_associated(row.mac[0]):
+            if not port_utils.has_ip_address_defined(row.mac[0]):
                 return False
             if event == self.ROW_UPDATE:
                 return (old.chassis and not row.chassis and
diff --git a/ovn_bgp_agent/drivers/openstack/watchers/nb_bgp_watcher.py b/ovn_bgp_agent/drivers/openstack/watchers/nb_bgp_watcher.py
index e3889fe5..de094767 100644
--- a/ovn_bgp_agent/drivers/openstack/watchers/nb_bgp_watcher.py
+++ b/ovn_bgp_agent/drivers/openstack/watchers/nb_bgp_watcher.py
@@ -17,6 +17,7 @@ from oslo_log import log as logging
 
 from ovn_bgp_agent import constants
 from ovn_bgp_agent.drivers.openstack.utils import driver_utils
+from ovn_bgp_agent.drivers.openstack.utils import port as port_utils
 from ovn_bgp_agent.drivers.openstack.watchers import base_watcher
 
 
@@ -49,7 +50,7 @@ class LogicalSwitchPortProviderCreateEvent(base_watcher.LSPChassisEvent):
             return
         try:
             # single and dual-stack format
-            if not self._check_ip_associated(row.addresses[0]):
+            if not port_utils.has_ip_address_defined(row.addresses[0]):
                 return False
 
             current_chassis = self._get_chassis(row)
@@ -100,7 +101,7 @@ class LogicalSwitchPortProviderDeleteEvent(base_watcher.LSPChassisEvent):
             return
         try:
             # single and dual-stack format
-            if not self._check_ip_associated(row.addresses[0]):
+            if not port_utils.has_ip_address_defined(row.addresses[0]):
                 return False
 
             ips = row.addresses[0].split(' ')[1:]
@@ -180,7 +181,7 @@ class LogicalSwitchPortFIPCreateEvent(base_watcher.LSPChassisEvent):
             return
         try:
             # single and dual-stack format
-            if not self._check_ip_associated(row.addresses[0]):
+            if not port_utils.has_ip_address_defined(row.addresses[0]):
                 return False
 
             current_chassis = self._get_chassis(row)
@@ -258,7 +259,7 @@ class LogicalSwitchPortFIPDeleteEvent(base_watcher.LSPChassisEvent):
             return
         try:
             # single and dual-stack format
-            if not self._check_ip_associated(row.addresses[0]):
+            if not port_utils.has_ip_address_defined(row.addresses[0]):
                 return False
 
             current_port_fip = self._get_port_fip(row)
@@ -579,7 +580,7 @@ class LogicalSwitchPortTenantCreateEvent(base_watcher.LSPChassisEvent):
     def match_fn(self, event, row, old):
         try:
             # single and dual-stack format
-            if not self._check_ip_associated(row.addresses[0]):
+            if not port_utils.has_ip_address_defined(row.addresses[0]):
                 return False
 
             if not bool(row.up[0]):
@@ -627,7 +628,7 @@ class LogicalSwitchPortTenantDeleteEvent(base_watcher.LSPChassisEvent):
     def match_fn(self, event, row, old):
         try:
             # single and dual-stack format
-            if not self._check_ip_associated(row.addresses[0]):
+            if not port_utils.has_ip_address_defined(row.addresses[0]):
                 return False
 
             current_network = self._get_network(row)
diff --git a/ovn_bgp_agent/tests/unit/drivers/openstack/utils/test_port.py b/ovn_bgp_agent/tests/unit/drivers/openstack/utils/test_port.py
new file mode 100644
index 00000000..7f216b06
--- /dev/null
+++ b/ovn_bgp_agent/tests/unit/drivers/openstack/utils/test_port.py
@@ -0,0 +1,37 @@
+# Copyright 2024 Red Hat, Inc.
+# All Rights Reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+from ovn_bgp_agent.drivers.openstack.utils import port
+from ovn_bgp_agent.tests import base as test_base
+
+
+class TestHasIpAddressDefined(test_base.TestCase):
+    def test_no_ip_address(self):
+        self.assertFalse(
+            port.has_ip_address_defined('aa:bb:cc:dd:ee:ff'))
+
+    def test_one_ip_address(self):
+        self.assertTrue(
+            port.has_ip_address_defined('aa:bb:cc:dd:ee:ff 10.10.1.16'))
+
+    def test_two_ip_addresses(self):
+        self.assertTrue(
+            port.has_ip_address_defined(
+                'aa:bb:cc:dd:ee:ff 10.10.1.16 10.10.1.17'))
+
+    def test_three_ip_addresses(self):
+        self.assertTrue(
+            port.has_ip_address_defined(
+                'aa:bb:cc:dd:ee:ff 10.10.1.16 10.10.1.17 10.10.1.18'))
diff --git a/ovn_bgp_agent/tests/unit/drivers/openstack/watchers/test_base_watcher.py b/ovn_bgp_agent/tests/unit/drivers/openstack/watchers/test_base_watcher.py
index 9243634c..b54eea91 100644
--- a/ovn_bgp_agent/tests/unit/drivers/openstack/watchers/test_base_watcher.py
+++ b/ovn_bgp_agent/tests/unit/drivers/openstack/watchers/test_base_watcher.py
@@ -21,29 +21,6 @@ from ovn_bgp_agent.tests import base as test_base
 from ovn_bgp_agent.tests import utils
 
 
-class FakePortBindingChassisEvent(base_watcher.PortBindingChassisEvent):
-    def run(self):
-        pass
-
-
-class TestPortBindingChassisEvent(test_base.TestCase):
-
-    def setUp(self):
-        super(TestPortBindingChassisEvent, self).setUp()
-        self.pb_event = FakePortBindingChassisEvent(
-            mock.Mock(), [mock.Mock()])
-
-    def test__check_ip_associated(self):
-        self.assertTrue(self.pb_event._check_ip_associated(
-            'aa:bb:cc:dd:ee:ff 10.10.1.16'))
-        self.assertTrue(self.pb_event._check_ip_associated(
-            'aa:bb:cc:dd:ee:ff 10.10.1.16 10.10.1.17'))
-        self.assertFalse(self.pb_event._check_ip_associated(
-            'aa:bb:cc:dd:ee:ff'))
-        self.assertTrue(self.pb_event._check_ip_associated(
-            'aa:bb:cc:dd:ee:ff 10.10.1.16 10.10.1.17 10.10.1.18'))
-
-
 class FakeOVNLBEvent(base_watcher.OVNLBEvent):
     def run(self):
         pass
@@ -115,16 +92,6 @@ class TestLSPChassisEvent(test_base.TestCase):
         self.lsp_event = FakeLSPChassisEvent(
             mock.Mock(), [mock.Mock()])
 
-    def test__check_ip_associated(self):
-        self.assertTrue(self.lsp_event._check_ip_associated(
-            'aa:bb:cc:dd:ee:ff 10.10.1.16'))
-        self.assertTrue(self.lsp_event._check_ip_associated(
-            'aa:bb:cc:dd:ee:ff 10.10.1.16 10.10.1.17'))
-        self.assertFalse(self.lsp_event._check_ip_associated(
-            'aa:bb:cc:dd:ee:ff'))
-        self.assertTrue(self.lsp_event._check_ip_associated(
-            'aa:bb:cc:dd:ee:ff 10.10.1.16 10.10.1.17 10.10.1.18'))
-
     def test__has_additional_binding(self):
         row = utils.create_row(
             options={constants.OVN_REQUESTED_CHASSIS: 'host1,host2'})