Added retries in 'network_set_host' function

In concurrent transactions in 'network_set_host' function
could happen a deadlock or returned rows_updated value
could be equal to 0.

In these cases we should retry the transaction to get
another row and try to set the host again.

Closes-Bug: #1422610

Change-Id: I374b12e62f76810ffdb1e2d35af72a738ead5591

nova/db/sqlalchemy/api.py

@@ -2900,12 +2900,30 @@ def network_get_all_by_host(context, host):
 
 
 @require_admin_context
+@_retry_on_deadlock
+@retrying.retry(stop_max_attempt_number=5, retry_on_exception=
+                lambda e: isinstance(e, exception.NetworkSetHostFailed))
 def network_set_host(context, network_id, host_id):
-    return _network_get_query(context).\
+    network_ref = _network_get_query(context).\
+        filter_by(id=network_id).\
+        first()
+
+    if not network_ref:
+        raise exception.NetworkNotFound(network_id=network_id)
+
+    if network_ref.host:
+        return None
+
+    rows_updated = _network_get_query(context).\
         filter_by(id=network_id).\
         filter_by(host=None).\
         update({'host': host_id})
 
+    if not rows_updated:
+        LOG.debug('The row was updated in a concurrent transaction, '
+                  'we will fetch another row')
+        raise exception.NetworkSetHostFailed(network_id=network_id)
+
 
 @require_context
 def network_update(context, network_id, values):

nova/exception.py

@@ -636,6 +636,10 @@ class NetworkInUse(NovaException):
     msg_fmt = _("Network %(network_id)s is still in use.")
 
 
+class NetworkSetHostFailed(NovaException):
+    msg_fmt = _("Network set host failed for network %(network_id)s.")
+
+
 class NetworkNotCreated(Invalid):
     msg_fmt = _("%(req)s is required to create a network.")
 

nova/tests/unit/db/test_db_api.py

@@ -5629,23 +5629,69 @@ class NetworkTestCase(test.TestCase, ModelsObjectComparatorMixin):
         self.assertEqual(2, network_new.vlan)
 
     def test_network_set_host_nonexistent_network(self):
-        self.assertEqual(0, db.network_set_host(
-            self.ctxt, 123456, 'nonexistent'))
+        self.assertRaises(exception.NetworkNotFound, db.network_set_host,
+                          self.ctxt, 123456, 'nonexistent')
 
-    def test_network_set_host_already_set(self):
+    def test_network_set_host_already_set_correct(self):
         values = {'host': 'example.com', 'project_id': 'project1'}
         network = db.network_create_safe(self.ctxt, values)
-        self.assertEqual(0, db.network_set_host(
-            self.ctxt, network.id, 'new.example.com'))
+        self.assertIsNone(db.network_set_host(self.ctxt, network.id,
+                          'example.com'))
+
+    def test_network_set_host_already_set_incorrect(self):
+        values = {'host': 'example.com', 'project_id': 'project1'}
+        network = db.network_create_safe(self.ctxt, values)
+        self.assertIsNone(db.network_set_host(self.ctxt, network.id,
+                                              'new.example.com'))
 
     def test_network_set_host_with_initially_no_host(self):
         values = {'project_id': 'project1'}
         network = db.network_create_safe(self.ctxt, values)
-        self.assertEqual(1, db.network_set_host(
-            self.ctxt, network.id, 'example.com'))
+        db.network_set_host(self.ctxt, network.id, 'example.com')
         self.assertEqual('example.com',
             db.network_get(self.ctxt, network.id).host)
 
+    def test_network_set_host_succeeds_retry_on_deadlock(self):
+        values = {'project_id': 'project1'}
+        network = db.network_create_safe(self.ctxt, values)
+
+        def fake_update(params):
+            if mock_update.call_count == 1:
+                raise db_exc.DBDeadlock()
+            else:
+                return 1
+
+        with mock.patch('sqlalchemy.orm.query.Query.update',
+                        side_effect=fake_update) as mock_update:
+            db.network_set_host(self.ctxt, network.id, 'example.com')
+            self.assertEqual(2, mock_update.call_count)
+
+    def test_network_set_host_succeeds_retry_on_no_rows_updated(self):
+        values = {'project_id': 'project1'}
+        network = db.network_create_safe(self.ctxt, values)
+
+        def fake_update(params):
+            if mock_update.call_count == 1:
+                return 0
+            else:
+                return 1
+
+        with mock.patch('sqlalchemy.orm.query.Query.update',
+                        side_effect=fake_update) as mock_update:
+            db.network_set_host(self.ctxt, network.id, 'example.com')
+            self.assertEqual(2, mock_update.call_count)
+
+    def test_network_set_host_failed_with_retry_on_no_rows_updated(self):
+        values = {'project_id': 'project1'}
+        network = db.network_create_safe(self.ctxt, values)
+
+        with mock.patch('sqlalchemy.orm.query.Query.update',
+                        return_value=0) as mock_update:
+            self.assertRaises(exception.NetworkSetHostFailed,
+                              db.network_set_host, self.ctxt, network.id,
+                              'example.com')
+            self.assertEqual(5, mock_update.call_count)
+
     def test_network_get_all_by_host(self):
         self.assertEqual([],
             db.network_get_all_by_host(self.ctxt, 'example.com'))