Agent logs virtual address delete error

Issues:
Fixes F5Networks#1318

Problem: Agent encounters errors when trying to update virtual address.
Additionally, errors are logged in BIG-IP mpcd associated with creating
virtual servers and pools.

Analysis: Modified assure functions to first check if object exists
before attempting to create.

Tests: Unit tests.

f5_openstack_agent/lbaasv2/drivers/bigip/lbaas_builder.py

@@ -282,7 +282,7 @@ def _assure_members(self, service, all_subnet_hints):
 
             if 'port' not in member and \
                member['provisioning_status'] != constants_v2.F5_PENDING_DELETE:
-                LOG.warning("Member definition does not include Neutron port")
+                LOG.debug("Member definition does not include Neutron port")
 
             pool_id = member.get('pool_id', None)
             if not pool_id:

f5_openstack_agent/lbaasv2/drivers/bigip/listener_service.py

@@ -75,27 +75,17 @@ def create_listener(self, service, bigips, esd=None):
                 self._add_cookie_persist_rule(vip, persist, bigip)
 
             try:
-                self.vs_helper.create(bigip, vip)
-            except HTTPError as err:
-                if err.response.status_code == 409:
+                if self.vs_helper.exists(bigip, vip):
                     LOG.debug("Virtual server already exists...updating")
-                    try:
-                        self.vs_helper.update(bigip, vip)
-                    except Exception as err:
-                        error = f5_ex.VirtualServerUpdateException(
-                            err.message)
-                        LOG.error("Virtual server update error: %s" %
-                                  error.message)
+                    self.vs_helper.update(bigip, vip)
                 else:
-                    error = f5_ex.VirtualServerCreationException(
-                        err.message)
-                    LOG.error("Virtual server creation error: %s" %
-                              error.message)
+                    LOG.debug("Virtual server does not exist...creating")
+                    self.vs_helper.create(bigip, vip)
 
             except Exception as err:
                 error = f5_ex.VirtualServerCreationException(
                     err.message)
-                LOG.error("Virtual server creation error: %s" %
+                LOG.error("Failed to create virtual server: %s" %
                           error.message)
 
             if not persist:

f5_openstack_agent/lbaasv2/drivers/bigip/pool_service.py

@@ -62,20 +62,12 @@ def create_pool(self, service, bigips):
 
         for bigip in bigips:
             try:
-                self.pool_helper.create(bigip, pool)
-            except HTTPError as err:
-                if err.response.status_code == 409:
+                if self.pool_helper.exists(bigip, pool):
                     LOG.debug("Pool already exists...updating")
-                    try:
-                        self.pool_helper.update(bigip, pool)
-                    except Exception as err:
-                        error = f5_ex.PoolUpdateException(err.message)
-                        LOG.error("Failed to assure pool %s on %s: %s",
-                                  pool['name'], bigip, error.message)
+                    self.pool_helper.update(bigip, pool)
                 else:
-                    error = f5_ex.PoolCreationException(err.message)
-                    LOG.error("Failed to assure pool %s on %s: %s",
-                              pool['name'], bigip, error.message)
+                    LOG.debug("Pool does not exist...creating")
+                    self.pool_helper.create(bigip, pool)
             except Exception as err:
                 error = f5_ex.PoolCreationException(err.message)
                 LOG.error("Failed to assure pool %s on %s: %s",
@@ -144,19 +136,12 @@ def create_healthmonitor(self, service, bigips):
 
         for bigip in bigips:
             try:
-                hm_helper.create(bigip, hm)
-            except HTTPError as err:
-                if err.response.status_code == 409:
-                    try:
-                        hm_helper.update(bigip, hm)
-                    except Exception as err:
-                        error = f5_ex.MonitorUpdateException(err.message)
-                        LOG.error("Failed to update monitor %s on %s: %s",
-                                  hm['name'], bigip, error.message)
+                if hm_helper.exists(bigip, hm):
+                    LOG.debug("Health monitor already exists...updating")
+                    hm_helper.update(bigip, hm)
                 else:
-                    error = f5_ex.MonitorCreationException(err.message)
-                    LOG.error("Failed to create monitor %s on %s: %s",
-                              hm['name'], bigip, error.message)
+                    LOG.debug("Health monitor does not exist...creating")
+                    hm_helper.create(bigip, hm)
             except Exception as err:
                 error = f5_ex.MonitorCreationException(err.message)
                 LOG.error("Failed to create monitor %s on %s: %s",

f5_openstack_agent/lbaasv2/drivers/bigip/test/test_lbaas_builder.py

@@ -1456,7 +1456,7 @@ def test_assure_member_has_no_port(self, service):
             service['members'][1].pop('port', None)
             builder._assure_members(service, mock.MagicMock())
             assert \
-                mock_log.warning.call_args_list == \
+                mock_log.debug.call_args_list == \
                 [mock.call('Member definition does not include Neutron port'),
                  mock.call('Member definition does not include Neutron port')]
 
@@ -1467,7 +1467,7 @@ def test_assure_member_has_one_port(self, service):
             service['members'][0].pop('port', None)
             builder._assure_members(service, mock.MagicMock())
             assert \
-                mock_log.warning.call_args_list == \
+                mock_log.debug.call_args_list == \
                 [mock.call('Member definition does not include Neutron port')]
 
     def test_assure_member_has_two_ports(self, service):

f5_openstack_agent/lbaasv2/drivers/bigip/test/test_listener_services.py

@@ -147,7 +147,9 @@ def create_basic_listener_200(target, service_with_listener):
             tls = dict()
             target.service_adapter.get_virtual.return_value = vs
             target.service_adapter.get_tls.return_value = tls
+            target.vs_helper.exists = Mock(return_value=False)
             bigips = [Mock()]
+
             retval = target.create_listener(service_with_listener, bigips)
 
             assert not retval
@@ -159,6 +161,7 @@ def create_basic_listener_200_two_bigip(target, service_with_listener):
             tls = dict()
             target.service_adapter.get_virtual.return_value = vs
             target.service_adapter.get_tls.return_value = tls
+            target.vs_helper.exists = Mock(return_value=False)
             bigips = [Mock(), Mock()]
             retval = target.create_listener(service_with_listener, bigips)
 
@@ -171,8 +174,7 @@ def create_basic_listener_409(target, service_with_listener):
             tls = dict()
             target.service_adapter.get_virtual.return_value = vs
             target.service_adapter.get_tls.return_value = tls
-            target.vs_helper.create.side_effect = MockHTTPError(
-                MockHTTPErrorResponse409())
+            target.vs_helper.exists = Mock(return_value=True)
             bigips = [Mock()]
             retval = target.create_listener(service_with_listener, bigips)
 
@@ -185,12 +187,11 @@ def create_basic_listener_400(target, service_with_listener):
             tls = dict()
             target.service_adapter.get_virtual.return_value = vs
             target.service_adapter.get_tls.return_value = tls
-            target.vs_helper.create.side_effect = MockHTTPError(
-                MockHTTPErrorResponse400())
+            target.vs_helper.exists = Mock(return_value=False)
             bigips = [Mock()]
             retval = target.create_listener(service_with_listener, bigips)
 
-            assert retval
+            assert not retval
             assert not target.vs_helper.update.called
 
         def create_basic_listener_exception(target, service_with_listener):
@@ -214,8 +215,7 @@ def create_basic_listener_update_exception(target,
             tls = dict()
             target.service_adapter.get_virtual.return_value = vs
             target.service_adapter.get_tls.return_value = tls
-            target.vs_helper.create.side_effect = MockHTTPError(
-                MockHTTPErrorResponse409())
+            target.vs_helper.exists = Mock(return_value=True)
             target.vs_helper.update.side_effect = MockError()
 
             bigips = [Mock()]
@@ -334,6 +334,7 @@ def create_listener_two_bigip_noerror(target, service_with_listener):
             tls = dict()
             target.service_adapter.get_virtual.return_value = vs
             target.service_adapter.get_tls.return_value = tls
+            target.vs_helper.exists = Mock(return_value=False)
             bigips = [Mock(), Mock()]
             retval = target.create_listener(service_with_listener, bigips)
 
@@ -361,8 +362,7 @@ def create_listener_two_bigip_409_noerror(target,
             tls = dict()
             target.service_adapter.get_virtual.return_value = vs
             target.service_adapter.get_tls.return_value = tls
-            target.vs_helper.create.side_effect = [
-                MockHTTPError(MockHTTPErrorResponse409()), None]
+            target.vs_helper.exists = Mock(return_value=True)
             bigips = [Mock(), Mock()]
             retval = target.create_listener(service_with_listener, bigips)
 
@@ -375,9 +375,7 @@ def create_listener_two_bigip_409_409(target, service_with_listener):
             tls = dict()
             target.service_adapter.get_virtual.return_value = vs
             target.service_adapter.get_tls.return_value = tls
-            target.vs_helper.create.side_effect = [
-                MockHTTPError(MockHTTPErrorResponse409()),
-                MockHTTPError(MockHTTPErrorResponse409())]
+            target.vs_helper.exists = Mock(return_value=True)
             bigips = [Mock(), Mock()]
             retval = target.create_listener(service_with_listener, bigips)
 

f5_openstack_agent/lbaasv2/drivers/bigip/test/test_pool_services.py

@@ -133,6 +133,8 @@ def create_pool_200(target, service_with_pool):
             pool = dict(name='name', partition='partition')
             target.service_adapter.get_pool.return_value = pool
             bigips = [Mock()]
+
+            target.pool_helper.exists = Mock(return_value=False)
             retval = target.create_pool(service_with_pool, bigips)
 
             assert not retval
@@ -143,8 +145,7 @@ def create_pool_409(target, service_with_pool):
             target.service_adapter.get_pool.return_value = pool
             bigips = [Mock()]
 
-            target.pool_helper.create.side_effect = MockHTTPError(
-                MockHTTPErrorResponse409())
+            target.pool_helper.exists = Mock(return_value=True)
 
             retval = target.create_pool(service_with_pool, bigips)
 
@@ -156,19 +157,19 @@ def create_pool_400(target, service_with_pool):
             target.service_adapter.get_pool.return_value = pool
             bigips = [Mock()]
 
-            target.pool_helper.create.side_effect = MockHTTPError(
-                MockHTTPErrorResponse400())
+            target.pool_helper.exists = Mock(return_value=False)
 
             retval = target.create_pool(service_with_pool, bigips)
 
-            assert retval
+            assert not retval
             assert not target.pool_helper.update.called
 
         def create_pool_error(target, service_with_pool):
             pool = dict(name='name', partition='partition')
             target.service_adapter.get_pool.return_value = pool
             bigips = [Mock()]
 
+            target.pool_helper.exists = Mock(return_value=False)
             target.pool_helper.create.side_effect = MockError()
 
             retval = target.create_pool(service_with_pool, bigips)
@@ -181,8 +182,7 @@ def create_pool_409_and_error(target, service_with_pool):
             target.service_adapter.get_pool.return_value = pool
             bigips = [Mock()]
 
-            target.pool_helper.create.side_effect = MockHTTPError(
-                MockHTTPErrorResponse409())
+            target.pool_helper.exists = Mock(return_value=True)
             target.pool_helper.update.side_effect = MockError()
 
             retval = target.create_pool(service_with_pool, bigips)
@@ -319,6 +319,7 @@ def create_monitor_200(target, service_with_pool):
             target.service_adapter.get_healthmonitor.return_value = \
                 monitor
             hm_helper = Mock()
+            hm_helper.exists = Mock(return_value=False)
             target._get_monitor_helper = Mock()
             target._get_monitor_helper.return_value = hm_helper
 
@@ -333,13 +334,12 @@ def create_monitor_409(target, service_with_pool):
             target.service_adapter.get_healthmonitor.return_value = \
                 monitor
             hm_helper = Mock()
+            hm_helper.exists = Mock(return_value=True)
             target._get_monitor_helper = Mock()
             target._get_monitor_helper.return_value = hm_helper
 
-            hm_helper.create.side_effect = MockHTTPError(
-                MockHTTPErrorResponse409())
-
             bigips = [Mock()]
+
             retval = target.create_healthmonitor(service_with_pool, bigips)
 
             assert not retval
@@ -350,23 +350,22 @@ def create_monitor_400(target, service_with_pool):
             target.service_adapter.get_healthmonitor.return_value = \
                 monitor
             hm_helper = Mock()
+            hm_helper.exists = Mock(return_value=False)
             target._get_monitor_helper = Mock()
             target._get_monitor_helper.return_value = hm_helper
 
-            hm_helper.create.side_effect = MockHTTPError(
-                MockHTTPErrorResponse400())
-
             bigips = [Mock()]
             retval = target.create_healthmonitor(service_with_pool, bigips)
 
-            assert retval
+            assert not retval
             assert not hm_helper.update.called
 
         def create_monitor_error(target, service_with_pool):
             monitor = dict(name='name', partition='partition')
             target.service_adapter.get_healthmonitor.return_value = \
                 monitor
             hm_helper = Mock()
+            hm_helper.exists = Mock(return_value=False)
             target._get_monitor_helper = Mock()
             target._get_monitor_helper.return_value = hm_helper
 
@@ -383,11 +382,11 @@ def create_monitor_409_error(target, service_with_pool):
             target.service_adapter.get_healthmonitor.return_value = \
                 monitor
             hm_helper = Mock()
+            hm_helper.exists = Mock(return_value=True)
+
             target._get_monitor_helper = Mock()
             target._get_monitor_helper.return_value = hm_helper
 
-            hm_helper.create.side_effect = MockHTTPError(
-                MockHTTPErrorResponse409())
             hm_helper.update.side_effect = MockError()
 
             bigips = [Mock()]

f5_openstack_agent/lbaasv2/drivers/bigip/virtual_address.py

@@ -121,8 +121,5 @@ def assure(self, bigip, delete=False):
 
         if delete:
             self.delete(bigip)
-        else:
-            if self.exists(bigip):
-                self.update(bigip)
-            else:
-                self.create(bigip)
+        elif not self.exists(bigip):
+            self.create(bigip)