diff --git a/heat/engine/resource.py b/heat/engine/resource.py index 98e3fd684..45f81683b 100644 --- a/heat/engine/resource.py +++ b/heat/engine/resource.py @@ -499,6 +499,12 @@ class Resource(object): 'res': self.type(), 'name': self.name} raise exception.NotSupported(feature=mesg) + if changed_properties_set and self.needs_replace_with_prop_diff( + changed_properties_set, + after_props, + before_props): + raise exception.UpdateReplace(self) + if not changed_properties_set.issubset(update_allowed_set): raise exception.UpdateReplace(self.name) @@ -847,6 +853,19 @@ class Resource(object): resource_data.get('resource_data'), resource_data.get('metadata')) + def needs_replace(self, after_props): + """Mandatory replace based on certain properties.""" + return False + + def needs_replace_with_prop_diff(self, changed_properties_set, + after_props, before_props): + """Needs replace based on prop_diff.""" + return False + + def needs_replace_with_tmpl_diff(self, tmpl_diff): + """Needs replace based on tmpl_diff.""" + return False + def _needs_update(self, after, before, after_props, before_props, prev_resource, check_init_complete=True): if self.status == self.FAILED: @@ -856,6 +875,9 @@ class Resource(object): and self.status == self.COMPLETE): raise exception.UpdateReplace(self) + if self.needs_replace(after_props): + raise exception.UpdateReplace(self) + if before != after.freeze(): return True @@ -955,10 +977,15 @@ class Resource(object): self.updated_time = datetime.utcnow() with self._action_recorder(action, exception.UpdateReplace): after_props.validate() + tmpl_diff = self.update_template_diff(function.resolve(after), before) + if tmpl_diff and self.needs_replace_with_tmpl_diff(tmpl_diff): + raise exception.UpdateReplace(self) + prop_diff = self.update_template_diff_properties(after_props, before_props) + yield self.action_handler_task(action, args=[after, tmpl_diff, prop_diff]) diff --git a/heat/engine/resources/aws/autoscaling/launch_config.py b/heat/engine/resources/aws/autoscaling/launch_config.py index 70c78a677..13b9bb223 100644 --- a/heat/engine/resources/aws/autoscaling/launch_config.py +++ b/heat/engine/resources/aws/autoscaling/launch_config.py @@ -216,19 +216,8 @@ class LaunchConfiguration(resource.Resource): self.properties_schema, self.context) self._update_stored_properties() - def _needs_update(self, after, before, after_props, before_props, - prev_resource, check_init_complete=True): - result = super(LaunchConfiguration, self)._needs_update( - after, before, after_props, before_props, prev_resource, - check_init_complete=check_init_complete) - - tmpl_diff = self.update_template_diff(function.resolve(after), - before) - - if 'Metadata' in tmpl_diff: - raise exception.UpdateReplace(self.name) - - return result + def needs_replace_with_tmpl_diff(self, tmpl_diff): + return 'Metadata' in tmpl_diff def get_reference_id(self): return self.physical_resource_name_or_FnGetRefId() diff --git a/heat/engine/resources/aws/ec2/eip.py b/heat/engine/resources/aws/ec2/eip.py index 74f194cfe..ffb231e80 100644 --- a/heat/engine/resources/aws/ec2/eip.py +++ b/heat/engine/resources/aws/ec2/eip.py @@ -406,26 +406,16 @@ class ElasticIpAssociation(resource.Resource): port_id=None, ignore_not_found=True) - def _needs_update(self, after, before, after_props, before_props, - prev_resource, check_init_complete=True): - result = super(ElasticIpAssociation, self)._needs_update( - after, before, after_props, before_props, prev_resource, - check_init_complete=check_init_complete) - - prop_diff = self.update_template_diff_properties(after_props, - before_props) - - # according to aws doc, when update allocation_id or eip, - # if you also change the InstanceId or NetworkInterfaceId, - # should go to Replacement flow - if self.ALLOCATION_ID in prop_diff or self.EIP in prop_diff: - instance_id = prop_diff.get(self.INSTANCE_ID) - ni_id = prop_diff.get(self.NETWORK_INTERFACE_ID) - - if instance_id or ni_id: - raise exception.UpdateReplace(self.name) - - return result + def needs_replace_with_prop_diff(self, changed_properties_set, + after_props, before_props): + if (self.ALLOCATION_ID in changed_properties_set or + self.EIP in changed_properties_set): + instance_id, ni_id = None, None + if self.INSTANCE_ID in changed_properties_set: + instance_id = after_props.get(self.INSTANCE_ID) + if self.NETWORK_INTERFACE_ID in changed_properties_set: + ni_id = after_props.get(self.NETWORK_INTERFACE_ID) + return bool(instance_id or ni_id) def handle_update(self, json_snippet, tmpl_diff, prop_diff): if prop_diff: diff --git a/heat/engine/resources/openstack/heat/test_resource.py b/heat/engine/resources/openstack/heat/test_resource.py index 83376a4b2..2a6aff7bc 100644 --- a/heat/engine/resources/openstack/heat/test_resource.py +++ b/heat/engine/resources/openstack/heat/test_resource.py @@ -16,7 +16,6 @@ import eventlet from oslo_utils import timeutils import six -from heat.common import exception from heat.common.i18n import _ from heat.engine import attributes from heat.engine import properties @@ -157,21 +156,10 @@ class TestResource(resource.Resource): return timeutils.utcnow(), self._wait_secs() - def _needs_update(self, after, before, after_props, before_props, - prev_resource, check_init_complete=True): - result = super(TestResource, self)._needs_update( - after, before, after_props, before_props, prev_resource, - check_init_complete=check_init_complete) - - prop_diff = self.update_template_diff_properties(after_props, - before_props) - - if self.UPDATE_REPLACE in prop_diff: - update_replace = prop_diff.get(self.UPDATE_REPLACE) - if update_replace: - raise exception.UpdateReplace(self.name) - - return result + def needs_replace_with_prop_diff(self, changed_properties_set, + after_props, before_props): + if self.UPDATE_REPLACE in changed_properties_set: + return bool(after_props.get(self.UPDATE_REPLACE)) def handle_update(self, json_snippet, tmpl_diff, prop_diff): self.properties = json_snippet.properties(self.properties_schema, diff --git a/heat/engine/resources/openstack/neutron/neutron.py b/heat/engine/resources/openstack/neutron/neutron.py index 4f6e66b88..190b0b9d5 100644 --- a/heat/engine/resources/openstack/neutron/neutron.py +++ b/heat/engine/resources/openstack/neutron/neutron.py @@ -21,11 +21,6 @@ class NeutronResource(resource.Resource): default_client_name = 'neutron' - # Subclasses provide a list of properties which, although - # update_allowed in the schema, should be excluded from the - # call to neutron, because they are handled in _needs_update - update_exclude_properties = [] - def validate(self): """Validate any of the provided params.""" res = super(NeutronResource, self).validate() diff --git a/heat/engine/resources/openstack/neutron/port.py b/heat/engine/resources/openstack/neutron/port.py index 054443e89..8832a01a9 100644 --- a/heat/engine/resources/openstack/neutron/port.py +++ b/heat/engine/resources/openstack/neutron/port.py @@ -15,7 +15,6 @@ from oslo_log import log as logging from oslo_serialization import jsonutils import six -from heat.common import exception from heat.common.i18n import _ from heat.common.i18n import _LW from heat.engine import attributes @@ -93,7 +92,6 @@ class Port(neutron.NeutronResource): constraints=[ constraints.CustomConstraint('neutron.network') ], - update_allowed=True, ), NETWORK: properties.Schema( @@ -108,7 +106,6 @@ class Port(neutron.NeutronResource): constraints=[ constraints.CustomConstraint('neutron.network') ], - update_allowed=True, ), DEVICE_ID: properties.Schema( properties.Schema.STRING, @@ -325,10 +322,6 @@ class Port(neutron.NeutronResource): ), } - # The network property can be updated, but only to switch between - # a name and ID for the same network, which is handled in _needs_update - update_exclude_properties = [NETWORK, NETWORK_ID] - def __init__(self, name, definition, stack): """Overloaded init in case of merging two schemas to one.""" self.properties_schema.update(self.extra_properties_schema) @@ -458,29 +451,24 @@ class Port(neutron.NeutronResource): return subnets return super(Port, self)._resolve_attribute(name) - def _needs_update(self, after, before, after_props, before_props, - prev_resource, check_init_complete=True): + def needs_replace(self, after_props): + """Mandatory replace based on props """ + return after_props.get(self.REPLACEMENT_POLICY) == 'REPLACE_ALWAYS' - if after_props.get(self.REPLACEMENT_POLICY) == 'REPLACE_ALWAYS': - raise exception.UpdateReplace(self.name) - - if after_props != before_props: - # Switching between name and ID is OK, provided the value resolves - # to the same network. If the network changes, port is replaced. - # Support NETWORK and NETWORK_ID, as switching between those is OK. - before_id = self.client_plugin().find_neutron_resource( - before_props, self.NETWORK, 'network') + def needs_replace_with_prop_diff(self, changed_properties_set, + after_props, before_props): + """Needs replace based on prop_diff """ + # Switching between name and ID is OK, provided the value resolves + # to the same network. If the network changes, port is replaced. + if self.NETWORK in changed_properties_set: after_id = self.client_plugin().find_neutron_resource( after_props, self.NETWORK, 'network') - if before_id != after_id: - raise exception.UpdateReplace(self.name) - - return super(Port, self)._needs_update( - after, before, after_props, before_props, prev_resource, - check_init_complete) + before_id = self.client_plugin().find_neutron_resource( + before_props, self.NETWORK, 'network') + changed_properties_set.remove(self.NETWORK) + return before_id != after_id def handle_update(self, json_snippet, tmpl_diff, prop_diff): - prop_diff.pop(self.NETWORK, None) if prop_diff: self.prepare_update_properties(prop_diff) if self.QOS_POLICY in prop_diff: diff --git a/heat/engine/resources/openstack/nova/server.py b/heat/engine/resources/openstack/nova/server.py index da46ad439..52d7c2f7c 100644 --- a/heat/engine/resources/openstack/nova/server.py +++ b/heat/engine/resources/openstack/nova/server.py @@ -1038,30 +1038,22 @@ class Server(stack_user.StackUser, sh.SchedulerHintsMixin, return updaters - def _needs_update(self, after, before, after_props, before_props, - prev_resource, check_init_complete=True): - result = super(Server, self)._needs_update( - after, before, after_props, before_props, prev_resource, - check_init_complete=check_init_complete) - - prop_diff = self.update_template_diff_properties(after_props, - before_props) - - if self.FLAVOR in prop_diff: + def needs_replace_with_prop_diff(self, changed_properties_set, + after_props, before_props): + """Needs replace based on prop_diff """ + if self.FLAVOR in changed_properties_set: flavor_update_policy = ( - prop_diff.get(self.FLAVOR_UPDATE_POLICY) or - self.properties[self.FLAVOR_UPDATE_POLICY]) + after_props.get(self.FLAVOR_UPDATE_POLICY) or + before_props.get(self.FLAVOR_UPDATE_POLICY)) if flavor_update_policy == 'REPLACE': - raise exception.UpdateReplace(self.name) + return True - if self.IMAGE in prop_diff: + if self.IMAGE in changed_properties_set: image_update_policy = ( - prop_diff.get(self.IMAGE_UPDATE_POLICY) or - self.properties[self.IMAGE_UPDATE_POLICY]) + after_props.get(self.IMAGE_UPDATE_POLICY) or + before_props.get(self.IMAGE_UPDATE_POLICY)) if image_update_policy == 'REPLACE': - raise exception.UpdateReplace(self.name) - - return result + return True def handle_update(self, json_snippet, tmpl_diff, prop_diff): if 'Metadata' in tmpl_diff: diff --git a/heat/tests/aws/test_eip.py b/heat/tests/aws/test_eip.py index b8bbb0453..b9d2ac75b 100644 --- a/heat/tests/aws/test_eip.py +++ b/heat/tests/aws/test_eip.py @@ -908,15 +908,13 @@ class AllocTest(common.HeatTestCase): t = template_format.parse(eip_template_ipassoc) stack = utils.parse_stack(t) self.create_eip(t, stack, 'IPAddress') - before_props = {'InstanceId': {'Ref': 'WebServer'}, - 'EIP': '11.0.0.1'} - after_props = {'InstanceId': {'Ref': 'WebServer2'}, + after_props = {'InstanceId': '5678', 'EIP': '11.0.0.2'} before = self.create_association(t, stack, 'IPAssoc') after = rsrc_defn.ResourceDefinition(before.name, before.type(), after_props) - self.assertRaises(exception.UpdateReplace, before._needs_update, - after, before, after_props, before_props, None) + updater = scheduler.TaskRunner(before.update, after) + self.assertRaises(exception.UpdateReplace, updater) def test_update_association_with_NetworkInterfaceId_or_InstanceId(self): self.mock_create_floatingip() diff --git a/heat/tests/openstack/neutron/test_neutron_port.py b/heat/tests/openstack/neutron/test_neutron_port.py index 5b2f56413..f3cf92287 100644 --- a/heat/tests/openstack/neutron/test_neutron_port.py +++ b/heat/tests/openstack/neutron/test_neutron_port.py @@ -11,6 +11,7 @@ # License for the specific language governing permissions and limitations # under the License. +import copy import mock import mox from neutronclient.common import exceptions as qe @@ -474,7 +475,7 @@ class NeutronPortTest(common.HeatTestCase): def test_port_needs_update_network(self): props = {'network_id': u'net1234', - 'name': utils.PhysName('test_stack', 'port'), + 'name': 'test_port', 'admin_state_up': True, 'device_owner': u'network:dhcp'} neutronV20.find_resourceid_by_name_or_id( @@ -482,7 +483,21 @@ class NeutronPortTest(common.HeatTestCase): 'network', 'net1234', cmd_resource=None, - ).AndReturn('net1234') + ).MultipleTimes().AndReturn('net1234') + + neutronV20.find_resourceid_by_name_or_id( + mox.IsA(neutronclient.Client), + 'network', + 'old_network', + cmd_resource=None, + ).MultipleTimes().AndReturn('net1234') + neutronV20.find_resourceid_by_name_or_id( + mox.IsA(neutronclient.Client), + 'network', + 'new_network', + cmd_resource=None, + ).MultipleTimes().AndReturn('net5678') + create_props = props.copy() neutronclient.Client.create_port( {'port': create_props} @@ -500,36 +515,43 @@ class NeutronPortTest(common.HeatTestCase): "ip_address": "10.0.0.2" } }}) - neutronV20.find_resourceid_by_name_or_id( - mox.IsA(neutronclient.Client), - 'network', - 'net1234', - cmd_resource=None, - ).MultipleTimes().AndReturn('net1234') - neutronV20.find_resourceid_by_name_or_id( - mox.IsA(neutronclient.Client), - 'network', - 'old_network', - cmd_resource=None, - ).AndReturn('net1234') - neutronV20.find_resourceid_by_name_or_id( - mox.IsA(neutronclient.Client), - 'network', - 'net1234', - cmd_resource=None, - ).MultipleTimes().AndReturn('net1234') - neutronV20.find_resourceid_by_name_or_id( - mox.IsA(neutronclient.Client), - 'network', - 'new_network', - cmd_resource=None, - ).AndReturn('net5678') + + call_dict = copy.deepcopy(props) + call_dict['security_groups'] = [ + '0389f747-7785-4757-b7bb-2ab07e4b09c3'] + del call_dict['network_id'] + + neutronclient.Client.show_port( + 'fc68ea2c-b60b-4b4f-bd82-94ec81110766' + ).AndReturn({'port': { + "status": "ACTIVE", + "id": "fc68ea2c-b60b-4b4f-bd82-94ec81110766" + }}) + + neutronclient.Client.show_port( + 'fc68ea2c-b60b-4b4f-bd82-94ec81110766' + ).AndReturn({'port': { + "status": "ACTIVE", + "id": "fc68ea2c-b60b-4b4f-bd82-94ec81110766" + }}) + + neutronclient.Client.show_port( + 'fc68ea2c-b60b-4b4f-bd82-94ec81110766' + ).AndReturn({'port': { + "status": "ACTIVE", + "id": "fc68ea2c-b60b-4b4f-bd82-94ec81110766" + }}) + neutronclient.Client.update_port( + 'fc68ea2c-b60b-4b4f-bd82-94ec81110766', + {'port': {'fixed_ips': []}} + ).AndReturn(None) self.m.ReplayAll() # create port t = template_format.parse(neutron_port_template) t['resources']['port']['properties'].pop('fixed_ips') + t['resources']['port']['properties']['name'] = 'test_port' stack = utils.parse_stack(t) port = stack['port'] @@ -538,28 +560,25 @@ class NeutronPortTest(common.HeatTestCase): # Switch from network_id=ID to network=ID (no replace) new_props = props.copy() new_props['network'] = new_props.pop('network_id') - new_props['network_id'] = None update_snippet = rsrc_defn.ResourceDefinition(port.name, port.type(), new_props) - self.assertTrue(port._needs_update(update_snippet, - port.frozen_definition(), - new_props, port.properties, None)) + scheduler.TaskRunner(port.update, update_snippet)() + self.assertEqual((port.UPDATE, port.COMPLETE), port.state) # Switch from network=ID to network=NAME (no replace) new_props['network'] = 'old_network' update_snippet = rsrc_defn.ResourceDefinition(port.name, port.type(), new_props) - self.assertTrue(port._needs_update(update_snippet, - port.frozen_definition(), - new_props, port.properties, None)) + + scheduler.TaskRunner(port.update, update_snippet)() + self.assertEqual((port.UPDATE, port.COMPLETE), port.state) # Switch to a different network (replace) new_props['network'] = 'new_network' update_snippet = rsrc_defn.ResourceDefinition(port.name, port.type(), new_props) - self.assertRaises(exception.UpdateReplace, port._needs_update, - update_snippet, port.frozen_definition(), - new_props, port.properties, None) + updater = scheduler.TaskRunner(port.update, update_snippet) + self.assertRaises(exception.UpdateReplace, updater) self.m.VerifyAll() diff --git a/heat/tests/openstack/nova/test_server.py b/heat/tests/openstack/nova/test_server.py index efcb146aa..90363fd6b 100644 --- a/heat/tests/openstack/nova/test_server.py +++ b/heat/tests/openstack/nova/test_server.py @@ -3650,6 +3650,7 @@ class ServersTest(common.HeatTestCase): update_template['Properties']['image_update_policy'] = 'REPLACE' # update + self.stub_ImageConstraint_validate() updater = scheduler.TaskRunner(server.update, update_template) self.assertRaises(exception.UpdateReplace, updater)