Convergence: Fix restore on rollback for server and port

Implements the special handling required for server and port resources
rollback to be possible.

Change-Id: If1b39df070f5e5394304d3e2b31ee7226ec2f270
Partial-Implements: blueprint rich-network-prop
Closes-Bug: #1498660

heat/engine/resource.py

@@ -883,6 +883,10 @@ class Resource(object):
         with self.lock(engine_id):
             new_temp = template.Template.load(self.context, template_id)
             new_res_def = new_temp.resource_definitions(self.stack)[self.name]
+            if self.stack.action == self.stack.ROLLBACK and \
+                    self.stack.status == self.stack.IN_PROGRESS \
+                    and self.replaced_by:
+                self.restore_prev_rsrc(convergence=True)
             runner = scheduler.TaskRunner(self.update, new_res_def)
             try:
                 runner(timeout=timeout)
@@ -947,10 +951,11 @@ class Resource(object):
         except exception.UpdateReplace as ex:
             # catch all UpdateReplace expections
             if (self.stack.action == 'ROLLBACK' and
-                    self.stack.status == 'IN_PROGRESS'):
+                    self.stack.status == 'IN_PROGRESS' and
+                    not cfg.CONF.convergence_engine):
                 # handle case, when it's rollback and we should restore
                 # old resource
-                self.restore_after_rollback()
+                self.restore_prev_rsrc()
             else:
                 self.prepare_for_replace()
             raise ex
@@ -964,7 +969,7 @@ class Resource(object):
         """
         pass
 
-    def restore_after_rollback(self):
+    def restore_prev_rsrc(self, convergence=False):
         """Restore resource after rollback.
 
         Some resources requires additional actions after rollback.

heat/engine/resources/openstack/neutron/port.py

@@ -21,6 +21,7 @@ from heat.common.i18n import _LW
 from heat.engine import attributes
 from heat.engine import constraints
 from heat.engine import properties
+from heat.engine import resource
 from heat.engine.resources.openstack.neutron import neutron
 from heat.engine.resources.openstack.neutron import subnet
 from heat.engine import support
@@ -441,16 +442,30 @@ class Port(neutron.NeutronResource):
         props = {'fixed_ips': []}
         self.client().update_port(self.resource_id, {'port': props})
 
-    def restore_after_rollback(self):
+    def restore_prev_rsrc(self, convergence=False):
-        old_port = self.stack._backup_stack().resources.get(self.name)
+        # In case of convergence, during rollback, the previous rsrc is
-        fixed_ips = old_port.data().get('port_fip', [])
+        # already selected and is being acted upon.
-        # restore fixed_ips for this port by setting fixed_ips to []
+        prev_port = self if convergence else \
+            self.stack._backup_stack().resources.get(self.name)
+        fixed_ips = prev_port.data().get('port_fip', [])
+
         props = {'fixed_ips': []}
-        old_props = {'fixed_ips': jsonutils.loads(fixed_ips)}
+        if convergence:
-        # remove ip from new port
+            existing_port, stack = resource.Resource.load(
-        self.client().update_port(self.resource_id, {'port': props})
+                prev_port.context, prev_port.replaced_by, True,
-        # restore ip for old port
+                prev_port.stack.cache_data
-        self.client().update_port(old_port.resource_id, {'port': old_props})
+            )
+            existing_port_id = existing_port.resource_id
+        else:
+            existing_port_id = self.resource_id
+        if existing_port_id:
+            # reset fixed_ips to [] for new resource
+            self.client().update_port(existing_port_id, {'port': props})
+        if fixed_ips and prev_port.resource_id:
+            # restore ip for old port
+            prev_port_props = {'fixed_ips': jsonutils.loads(fixed_ips)}
+            self.client().update_port(prev_port.resource_id,
+                                      {'port': prev_port_props})
 
 
 def resource_mapping():

heat/engine/resources/openstack/nova/server.py

@@ -1411,8 +1411,8 @@ class Server(stack_user.StackUser, sh.SchedulerHintsMixin,
     def prepare_for_replace(self):
         self.prepare_ports_for_replace()
 
-    def restore_after_rollback(self):
+    def restore_prev_rsrc(self, convergence=False):
-        self.restore_ports_after_rollback()
+        self.restore_ports_after_rollback(convergence=convergence)
 
 
 def resource_mapping():

heat/engine/resources/openstack/nova/server_network_mixin.py

@@ -20,6 +20,7 @@ from oslo_utils import netutils
 from heat.common import exception
 from heat.common.i18n import _
 from heat.common.i18n import _LI
+from heat.engine import resource
 
 LOG = logging.getLogger(__name__)
 
@@ -355,22 +356,38 @@ class ServerNetworkMixin(object):
             self.client('neutron').update_port(
                 port['id'], {'port': {'fixed_ips': []}})
 
-    def restore_ports_after_rollback(self):
+    def restore_ports_after_rollback(self, convergence):
         if not self.is_using_neutron():
             return
 
-        old_server = self.stack._backup_stack().resources.get(self.name)
+        # In case of convergence, during rollback, the previous rsrc is
+        # already selected and is being acted upon.
+        prev_server = self if convergence else \
+            self.stack._backup_stack().resources.get(self.name)
 
-        port_data = itertools.chain(self._data_get_ports(),
+        if convergence:
-                                    self._data_get_ports('external_ports'))
+            rsrc, stack = resource.Resource.load(
+                prev_server.context, prev_server.replaced_by, True,
+                prev_server.stack.cache_data
+            )
+            existing_server = rsrc
+        else:
+            existing_server = self
+
+        port_data = itertools.chain(
+            existing_server._data_get_ports(),
+            existing_server._data_get_ports('external_ports')
+        )
         for port in port_data:
+            # reset fixed_ips to [] for new resource
             self.client('neutron').update_port(port['id'],
                                                {'port': {'fixed_ips': []}})
 
-        old_port_data = itertools.chain(
+        # restore ip for old port
-            old_server._data_get_ports(),
+        prev_port_data = itertools.chain(
-            old_server._data_get_ports('external_ports'))
+            prev_server._data_get_ports(),
-        for port in old_port_data:
+            prev_server._data_get_ports('external_ports'))
+        for port in prev_port_data:
             fixed_ips = port['fixed_ips']
             self.client('neutron').update_port(
                 port['id'], {'port': {'fixed_ips': fixed_ips}})

heat/tests/neutron/test_neutron_port.py

@@ -748,7 +748,7 @@ class NeutronPortTest(common.HeatTestCase):
         n_client.update_port.assert_called_once_with('test_res_id',
                                                      expected_props)
 
-    def test_restore_after_rollback_port(self):
+    def test_restore_prev_rsrc(self):
         t = template_format.parse(neutron_port_template)
         stack = utils.parse_stack(t)
         new_port = stack['port']
@@ -769,7 +769,7 @@ class NeutronPortTest(common.HeatTestCase):
         new_port.client = mock.Mock(return_value=n_client)
 
         # execute prepare_for_replace
-        new_port.restore_after_rollback()
+        new_port.restore_prev_rsrc()
 
         # check, that ports were updated: old port get ip and
         # same ip was removed from old port
@@ -778,3 +778,40 @@ class NeutronPortTest(common.HeatTestCase):
         n_client.update_port.assert_has_calls([
             mock.call('new_res_id', expected_new_props),
             mock.call('old_res_id', expected_old_props)])
+
+    def test_restore_prev_rsrc_convergence(self):
+        t = template_format.parse(neutron_port_template)
+        stack = utils.parse_stack(t)
+        stack.store()
+
+        # mock resource from previous template
+        prev_rsrc = stack['port']
+        prev_rsrc.resource_id = 'prev-rsrc'
+        # store in db
+        prev_rsrc.state_set(prev_rsrc.UPDATE, prev_rsrc.COMPLETE)
+
+        # mock resource from existing template and store in db
+        existing_rsrc = stack['port']
+        existing_rsrc.current_template_id = stack.t.id
+        existing_rsrc.resource_id = 'existing-rsrc'
+        existing_rsrc.state_set(existing_rsrc.UPDATE, existing_rsrc.COMPLETE)
+
+        # mock previous resource was replaced by existing resource
+        prev_rsrc.replaced_by = existing_rsrc.id
+        _value = {
+            'subnet_id': 'test_subnet',
+            'ip_address': '42.42.42.42'
+        }
+        prev_rsrc._data = {'port_fip': jsonutils.dumps(_value)}
+
+        n_client = mock.Mock()
+        prev_rsrc.client = mock.Mock(return_value=n_client)
+
+        # execute prepare_for_replace
+        prev_rsrc.restore_prev_rsrc(convergence=True)
+
+        expected_existing_props = {'port': {'fixed_ips': []}}
+        expected_prev_props = {'port': {'fixed_ips': _value}}
+        n_client.update_port.assert_has_calls([
+            mock.call(existing_rsrc.resource_id, expected_existing_props),
+            mock.call(prev_rsrc.resource_id, expected_prev_props)])

heat/tests/nova/test_server.py

@@ -4275,7 +4275,86 @@ class ServerInternalPortTest(common.HeatTestCase):
         stack._backup_stack().resources.get.return_value = old_server
         old_server._data_get_ports.side_effect = [port_ids, external_port_ids]
 
-        server.restore_after_rollback()
+        server.restore_prev_rsrc()
+
+        # check, that all ip were removed from new_ports
+        empty_fixed_ips = {'port': {'fixed_ips': []}}
+        self.port_update.assert_has_calls([
+            mock.call(1122, empty_fixed_ips),
+            mock.call(3344, empty_fixed_ips),
+            mock.call(5566, empty_fixed_ips)])
+
+        # check, that all ip were restored for old_ports
+        self.port_update.assert_has_calls([
+            mock.call(1122, {'port': port1_fixed_ip}),
+            mock.call(3344, {'port': port2_fixed_ip}),
+            mock.call(5566, {'port': port3_fixed_ip})])
+
+    def test_restore_ports_after_rollback_convergence(self):
+        tmpl = """
+        heat_template_version: 2015-10-15
+        resources:
+          server:
+            type: OS::Nova::Server
+            properties:
+              flavor: m1.small
+              image: F17-x86_64-gold
+              networks:
+                - network: 4321
+        """
+        t = template_format.parse(tmpl)
+        stack = utils.parse_stack(t)
+        stack.store()
+
+        # mock resource from previous template
+        prev_rsrc = stack['server']
+        prev_rsrc.resource_id = 'prev-rsrc'
+        # store in db
+        prev_rsrc.state_set(prev_rsrc.UPDATE, prev_rsrc.COMPLETE)
+
+        # mock resource from existing template, store in db, and set _data
+        existing_rsrc = stack['server']
+        existing_rsrc.current_template_id = stack.t.id
+        existing_rsrc.resource_id = 'existing-rsrc'
+        existing_rsrc.state_set(existing_rsrc.UPDATE, existing_rsrc.COMPLETE)
+
+        port_ids = [{'id': 1122}, {'id': 3344}]
+        external_port_ids = [{'id': 5566}]
+        existing_rsrc.data_set("internal_ports", jsonutils.dumps(port_ids))
+        existing_rsrc.data_set("external_ports",
+                               jsonutils.dumps(external_port_ids))
+
+        # mock previous resource was replaced by existing resource
+        prev_rsrc.replaced_by = existing_rsrc.id
+
+        port1_fixed_ip = {
+            'fixed_ips': {
+                'subnet_id': 'test_subnet1',
+                'ip_address': '41.41.41.41'
+            }
+        }
+        port2_fixed_ip = {
+            'fixed_ips': {
+                'subnet_id': 'test_subnet2',
+                'ip_address': '42.42.42.42'
+            }
+        }
+        port3_fixed_ip = {
+            'fixed_ips': {
+                'subnet_id': 'test_subnet3',
+                'ip_address': '43.43.43.43'
+            }
+        }
+        port_ids[0].update(port1_fixed_ip)
+        port_ids[1].update(port2_fixed_ip)
+        external_port_ids[0].update(port3_fixed_ip)
+        # add data to old server
+        prev_rsrc._data = {
+            "internal_ports": jsonutils.dumps(port_ids),
+            "external_ports": jsonutils.dumps(external_port_ids)
+        }
+
+        prev_rsrc.restore_prev_rsrc(convergence=True)
 
         # check, that all ip were removed from new_ports
         empty_fixed_ips = {'port': {'fixed_ips': []}}

heat/tests/test_resource.py

@@ -406,13 +406,13 @@ class ResourceTest(common.HeatTestCase):
         self.stack.state_set('ROLLBACK', 'IN_PROGRESS', 'Simulate rollback')
         res = TestResource('test_resource', tmpl, self.stack)
 
-        res.restore_after_rollback = mock.Mock()
+        res.restore_prev_rsrc = mock.Mock()
 
         utmpl = rsrc_defn.ResourceDefinition('test_resource', 'TestResource',
                                              {'a_string': 'foo'})
         self.assertRaises(
             exception.UpdateReplace, scheduler.TaskRunner(res.update, utmpl))
-        self.assertTrue(res.restore_after_rollback.called)
+        self.assertTrue(res.restore_prev_rsrc.called)
 
     def test_update_replace_in_failed_without_nested(self):
         tmpl = rsrc_defn.ResourceDefinition('test_resource',