Merge pull request #793 from pjbreaux/liberty

Pool will not be recreated if neutron-lbaas show it as 'ACTIVE' and pool does not exist on device

f5_openstack_agent/lbaasv2/drivers/bigip/lbaas_builder.py

@@ -150,8 +150,8 @@ def _assure_pools_created(self, service):
 
                 try:
                     # create or update pool
-                    if pool['provisioning_status'] == \
-                            plugin_const.PENDING_CREATE:
+                    if pool['provisioning_status'] \
+                            != plugin_const.PENDING_UPDATE:
                         self.pool_builder.create_pool(svc, bigips)
                     else:
                         self.pool_builder.update_pool(svc, bigips)

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

@@ -24,10 +24,30 @@
 
 from requests import HTTPError
 
+POOL_BLDR_PATH = 'f5_openstack_agent.lbaasv2.drivers.bigip.pool_service.' \
+    'PoolServiceBuilder'
+
 
 @pytest.fixture
 def service():
     return {
+        u"listeners": [{
+            u"admin_state_up": True,
+            u"connection_limit": -1,
+            u"default_pool_id": None,
+            u"default_tls_container_id": None,
+            u"description": u"",
+            u"id": u"5108c2fe-29bd-4e5b-94de-99034c561a15",
+            u"l7_policies": [],
+            u"loadbalancer_id": u"75e9326d-67ec-42b7-9647-3fc1c51c0091",
+            u"name": u"listener2",
+            u"operating_status": u"ONLINE",
+            u"protocol": u"HTTPS",
+            u"protocol_port": 443,
+            u"provisioning_status": u"ACTIVE",
+            u"sni_containers": [],
+            u"tenant_id": u"980e3f914f3e40359c3c2d9470fb2e8a"
+        }],
         u'loadbalancer': {
             u'admin_state_up': True,
             u'description': u'',
@@ -106,11 +126,13 @@ def service():
                     u'id': u'2dbca6cd-30d8-4013-9c9a-df0850fabf52',
                     u'l7_policies': [],
                     u'lb_algorithm': u'ROUND_ROBIN',
+                    u'listeners':[
+                        {u'id': '5108c2fe-29bd-4e5b-94de-99034c561a15'}],
                     u'loadbalancer_id': u'd5a0396e-e862-4cbf-8eb9-25c7fbc4d59',
                     u'name': u'',
                     u'operating_status': u'ONLINE',
                     u'protocol': u'HTTP',
-                    u'provisioning_status': u'PENDING_DELETE',
+                    u'provisioning_status': u'ACTIVE',
                     u'session_persistence': None,
                     u'sessionpersistence': None,
                     u'tenant_id': u'd9ed216f67f04a84bf8fd97c155855cd'}],
@@ -143,6 +165,7 @@ def test_assure_members_deleted(self, service):
         delete_member_mock = mock.MagicMock()
         pool_builder_mock.delete_member = delete_member_mock
         builder.pool_builder = pool_builder_mock
+        service['pools'][0]['provisioning_status'] = 'PENDING_DELETE'
 
         builder._assure_members(service, mock.MagicMock())
         assert delete_member_mock.called
@@ -159,7 +182,6 @@ def test_assure_members_not_deleted(self, service):
         pool_builder_mock.delete_member = delete_member_mock
         builder.pool_builder = pool_builder_mock
 
-        service['pools'][0]['provisioning_status'] = 'ACTIVE'
         builder._assure_members(service, mock.MagicMock())
         assert not delete_member_mock.called
 
@@ -184,20 +206,13 @@ def test_assure_members_update_exception(self, service):
         updated, LBaaSBuilder must catch any exception from PoolServiceBuilder
         update_member() and set member provisioning_status to ERROR.
         """
-        with mock.patch(
-                target='f5_openstack_agent.lbaasv2.drivers.bigip.'
-                       'pool_service.PoolServiceBuilder.'
-                       'create_member') as mock_create:
-            with mock.patch(
-                    target='f5_openstack_agent.lbaasv2.drivers.'
-                           'bigip.pool_service.PoolServiceBuilder.'
-                           'update_member') as mock_update:
+        with mock.patch(POOL_BLDR_PATH + '.create_member') as mock_create:
+            with mock.patch(POOL_BLDR_PATH + '.update_member') as mock_update:
                 mock_create.side_effect = MockHTTPError(
                     MockHTTPErrorResponse409())
                 mock_update.side_effect = MockHTTPError(
                     MockHTTPErrorResponse409())
                 service['members'][0]['provisioning_status'] = 'PENDING_UPDATE'
-                service['pools'][0]['provisioning_status'] = 'ACTIVE'
                 builder = LBaaSBuilder(mock.MagicMock(), mock.MagicMock())
                 with pytest.raises(f5_ex.MemberUpdateException):
                     builder._assure_members(service, mock.MagicMock())
@@ -239,3 +254,51 @@ def test_assure_member_has_two_ports(self, service):
             builder = LBaaSBuilder(mock.MagicMock(), mock.MagicMock())
             builder._assure_members(service, mock.MagicMock())
             assert mock_log.warning.call_args_list == []
+
+    @mock.patch(POOL_BLDR_PATH + '.create_pool')
+    @mock.patch(POOL_BLDR_PATH + '.update_pool')
+    def test__assure_pools_created_pool_create(
+            self, mock_update, mock_create, service):
+        '''create_pool should be called in pool builder on pool create'''
+        svc = service
+        svc['pools'][0]['provisioning_status'] = 'PENDING_CREATE'
+        builder = LBaaSBuilder(mock.MagicMock(), mock.MagicMock())
+        builder._assure_pools_created(svc)
+        assert mock_create.called
+        assert not mock_update.called
+
+    @mock.patch(POOL_BLDR_PATH + '.create_pool')
+    @mock.patch(POOL_BLDR_PATH + '.update_pool')
+    def test__assure_pools_created_pool_update(
+            self, mock_update, mock_create, service):
+        '''update_pool should be called in pool builder on pool update'''
+        svc = service
+        svc['pools'][0]['provisioning_status'] = 'PENDING_UPDATE'
+        builder = LBaaSBuilder(mock.MagicMock(), mock.MagicMock())
+        builder._assure_pools_created(svc)
+        assert mock_update.called
+        assert not mock_create.called
+
+    @mock.patch(POOL_BLDR_PATH + '.create_pool')
+    @mock.patch(POOL_BLDR_PATH + '.update_pool')
+    def test__assure_pools_created_pool_active(
+            self, mock_update, mock_create, service):
+        '''create_pool should be called in pool builder with active pool'''
+        svc = service
+        svc['pools'][0]['provisioning_status'] = 'ACTIVE'
+        builder = LBaaSBuilder(mock.MagicMock(), mock.MagicMock())
+        builder._assure_pools_created(svc)
+        assert not mock_update.called
+        assert mock_create.called
+
+    @mock.patch(POOL_BLDR_PATH + '.create_pool')
+    @mock.patch(POOL_BLDR_PATH + '.update_pool')
+    def test__assure_pools_created_pool_error(
+            self, mock_update, mock_create, service):
+        '''create_pool should be called in pool builder with errored pool'''
+        svc = service
+        svc['pools'][0]['provisioning_status'] = 'ERROR'
+        builder = LBaaSBuilder(mock.MagicMock(), mock.MagicMock())
+        builder._assure_pools_created(svc)
+        assert not mock_update.called
+        assert mock_create.called