openstack/deb-heat
Code Issues Proposed changes

Refactor resource specific replace logic

Browse Source 
Currently we override _needs_update for some resources,
where replacement is required either based on tmpl_diff
or prop_diff. This patch refactors and adds specific functions
(needs_replace, needs_replace_with_prop_diff,
needs_replace_with_tmpl_diff).
These can now be overidden as required.

Change-Id: I15542bef8c9e6ed6b80cd4656d2fd42e8406655b
Blueprint: stack-update-restrict
This commit is contained in:
Rabi Mishra 2015-12-19 07:30:14 +05:30
parent c3440202ff
commit b91eaa803b
10 changed files with 126 additions and 139 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
heat/engine/resource.py
View File

@ -499,6 +499,12 @@ class Resource(object):
'res': self.type(), 'name': self.name} 'res': self.type(), 'name': self.name}
raise exception.NotSupported(feature=mesg) 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): if not changed_properties_set.issubset(update_allowed_set):
raise exception.UpdateReplace(self.name) raise exception.UpdateReplace(self.name)
@ -847,6 +853,19 @@ class Resource(object):
resource_data.get('resource_data'), resource_data.get('resource_data'),
resource_data.get('metadata')) 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, def _needs_update(self, after, before, after_props, before_props,
prev_resource, check_init_complete=True): prev_resource, check_init_complete=True):
if self.status == self.FAILED: if self.status == self.FAILED:
@ -856,6 +875,9 @@ class Resource(object):
and self.status == self.COMPLETE): and self.status == self.COMPLETE):
raise exception.UpdateReplace(self) raise exception.UpdateReplace(self)
if self.needs_replace(after_props):
raise exception.UpdateReplace(self)
if before != after.freeze(): if before != after.freeze():
return True return True
@ -955,10 +977,15 @@ class Resource(object):
self.updated_time = datetime.utcnow() self.updated_time = datetime.utcnow()
with self._action_recorder(action, exception.UpdateReplace): with self._action_recorder(action, exception.UpdateReplace):
after_props.validate() after_props.validate()
tmpl_diff = self.update_template_diff(function.resolve(after), tmpl_diff = self.update_template_diff(function.resolve(after),
before) 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, prop_diff = self.update_template_diff_properties(after_props,
before_props) before_props)
yield self.action_handler_task(action, yield self.action_handler_task(action,
args=[after, tmpl_diff, args=[after, tmpl_diff,
prop_diff]) prop_diff])

15
heat/engine/resources/aws/autoscaling/launch_config.py
View File

@ -216,19 +216,8 @@ class LaunchConfiguration(resource.Resource):
self.properties_schema, self.context) self.properties_schema, self.context)
self._update_stored_properties() self._update_stored_properties()
def _needs_update(self, after, before, after_props, before_props, def needs_replace_with_tmpl_diff(self, tmpl_diff):
prev_resource, check_init_complete=True): return 'Metadata' in tmpl_diff
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 get_reference_id(self): def get_reference_id(self):
return self.physical_resource_name_or_FnGetRefId() return self.physical_resource_name_or_FnGetRefId()

30
heat/engine/resources/aws/ec2/eip.py
View File

@ -406,26 +406,16 @@ class ElasticIpAssociation(resource.Resource):
port_id=None, port_id=None,
ignore_not_found=True) ignore_not_found=True)
def _needs_update(self, after, before, after_props, before_props, def needs_replace_with_prop_diff(self, changed_properties_set,
prev_resource, check_init_complete=True): after_props, before_props):
result = super(ElasticIpAssociation, self)._needs_update( if (self.ALLOCATION_ID in changed_properties_set or
after, before, after_props, before_props, prev_resource, self.EIP in changed_properties_set):
check_init_complete=check_init_complete) instance_id, ni_id = None, None
if self.INSTANCE_ID in changed_properties_set:
prop_diff = self.update_template_diff_properties(after_props, instance_id = after_props.get(self.INSTANCE_ID)
before_props) if self.NETWORK_INTERFACE_ID in changed_properties_set:
ni_id = after_props.get(self.NETWORK_INTERFACE_ID)
# according to aws doc, when update allocation_id or eip, return bool(instance_id or ni_id)
# 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 handle_update(self, json_snippet, tmpl_diff, prop_diff): def handle_update(self, json_snippet, tmpl_diff, prop_diff):
if prop_diff: if prop_diff:

20
heat/engine/resources/openstack/heat/test_resource.py
View File

@ -16,7 +16,6 @@ import eventlet
from oslo_utils import timeutils from oslo_utils import timeutils
import six import six
from heat.common import exception
from heat.common.i18n import _ from heat.common.i18n import _
from heat.engine import attributes from heat.engine import attributes
from heat.engine import properties from heat.engine import properties
@ -157,21 +156,10 @@ class TestResource(resource.Resource):
return timeutils.utcnow(), self._wait_secs() return timeutils.utcnow(), self._wait_secs()
def _needs_update(self, after, before, after_props, before_props, def needs_replace_with_prop_diff(self, changed_properties_set,
prev_resource, check_init_complete=True): after_props, before_props):
result = super(TestResource, self)._needs_update( if self.UPDATE_REPLACE in changed_properties_set:
after, before, after_props, before_props, prev_resource, return bool(after_props.get(self.UPDATE_REPLACE))
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 handle_update(self, json_snippet, tmpl_diff, prop_diff): def handle_update(self, json_snippet, tmpl_diff, prop_diff):
self.properties = json_snippet.properties(self.properties_schema, self.properties = json_snippet.properties(self.properties_schema,

5
heat/engine/resources/openstack/neutron/neutron.py
View File

@ -21,11 +21,6 @@ class NeutronResource(resource.Resource):
default_client_name = 'neutron' 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): def validate(self):
"""Validate any of the provided params.""" """Validate any of the provided params."""
res = super(NeutronResource, self).validate() res = super(NeutronResource, self).validate()

38
heat/engine/resources/openstack/neutron/port.py
View File

@ -15,7 +15,6 @@ from oslo_log import log as logging
from oslo_serialization import jsonutils from oslo_serialization import jsonutils
import six import six
from heat.common import exception
from heat.common.i18n import _ from heat.common.i18n import _
from heat.common.i18n import _LW from heat.common.i18n import _LW
from heat.engine import attributes from heat.engine import attributes
@ -93,7 +92,6 @@ class Port(neutron.NeutronResource):
constraints=[ constraints=[
constraints.CustomConstraint('neutron.network') constraints.CustomConstraint('neutron.network')
], ],
update_allowed=True,
), ),
NETWORK: properties.Schema( NETWORK: properties.Schema(
@ -108,7 +106,6 @@ class Port(neutron.NeutronResource):
constraints=[ constraints=[
constraints.CustomConstraint('neutron.network') constraints.CustomConstraint('neutron.network')
], ],
update_allowed=True,
), ),
DEVICE_ID: properties.Schema( DEVICE_ID: properties.Schema(
properties.Schema.STRING, 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): def __init__(self, name, definition, stack):
"""Overloaded init in case of merging two schemas to one.""" """Overloaded init in case of merging two schemas to one."""
self.properties_schema.update(self.extra_properties_schema) self.properties_schema.update(self.extra_properties_schema)
@ -458,29 +451,24 @@ class Port(neutron.NeutronResource):
return subnets return subnets
return super(Port, self)._resolve_attribute(name) return super(Port, self)._resolve_attribute(name)
def _needs_update(self, after, before, after_props, before_props, def needs_replace(self, after_props):
prev_resource, check_init_complete=True): """Mandatory replace based on props """
return after_props.get(self.REPLACEMENT_POLICY) == 'REPLACE_ALWAYS'
if after_props.get(self.REPLACEMENT_POLICY) == 'REPLACE_ALWAYS': def needs_replace_with_prop_diff(self, changed_properties_set,
raise exception.UpdateReplace(self.name) after_props, before_props):
"""Needs replace based on prop_diff """
if after_props != before_props: # Switching between name and ID is OK, provided the value resolves
# Switching between name and ID is OK, provided the value resolves # to the same network. If the network changes, port is replaced.
# to the same network. If the network changes, port is replaced. if self.NETWORK in changed_properties_set:
# Support NETWORK and NETWORK_ID, as switching between those is OK.
before_id = self.client_plugin().find_neutron_resource(
before_props, self.NETWORK, 'network')
after_id = self.client_plugin().find_neutron_resource( after_id = self.client_plugin().find_neutron_resource(
after_props, self.NETWORK, 'network') after_props, self.NETWORK, 'network')
if before_id != after_id: before_id = self.client_plugin().find_neutron_resource(
raise exception.UpdateReplace(self.name) before_props, self.NETWORK, 'network')
changed_properties_set.remove(self.NETWORK)
return super(Port, self)._needs_update( return before_id != after_id
after, before, after_props, before_props, prev_resource,
check_init_complete)
def handle_update(self, json_snippet, tmpl_diff, prop_diff): def handle_update(self, json_snippet, tmpl_diff, prop_diff):
prop_diff.pop(self.NETWORK, None)
if prop_diff: if prop_diff:
self.prepare_update_properties(prop_diff) self.prepare_update_properties(prop_diff)
if self.QOS_POLICY in prop_diff: if self.QOS_POLICY in prop_diff:

30
heat/engine/resources/openstack/nova/server.py
View File

@ -1038,30 +1038,22 @@ class Server(stack_user.StackUser, sh.SchedulerHintsMixin,
return updaters return updaters
def _needs_update(self, after, before, after_props, before_props, def needs_replace_with_prop_diff(self, changed_properties_set,
prev_resource, check_init_complete=True): after_props, before_props):
result = super(Server, self)._needs_update( """Needs replace based on prop_diff """
after, before, after_props, before_props, prev_resource, if self.FLAVOR in changed_properties_set:
check_init_complete=check_init_complete)
prop_diff = self.update_template_diff_properties(after_props,
before_props)
if self.FLAVOR in prop_diff:
flavor_update_policy = ( flavor_update_policy = (
prop_diff.get(self.FLAVOR_UPDATE_POLICY) or after_props.get(self.FLAVOR_UPDATE_POLICY) or
self.properties[self.FLAVOR_UPDATE_POLICY]) before_props.get(self.FLAVOR_UPDATE_POLICY))
if flavor_update_policy == 'REPLACE': 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 = ( image_update_policy = (
prop_diff.get(self.IMAGE_UPDATE_POLICY) or after_props.get(self.IMAGE_UPDATE_POLICY) or
self.properties[self.IMAGE_UPDATE_POLICY]) before_props.get(self.IMAGE_UPDATE_POLICY))
if image_update_policy == 'REPLACE': if image_update_policy == 'REPLACE':
raise exception.UpdateReplace(self.name) return True
return result
def handle_update(self, json_snippet, tmpl_diff, prop_diff): def handle_update(self, json_snippet, tmpl_diff, prop_diff):
if 'Metadata' in tmpl_diff: if 'Metadata' in tmpl_diff:

8
heat/tests/aws/test_eip.py
View File

@ -908,15 +908,13 @@ class AllocTest(common.HeatTestCase):
t = template_format.parse(eip_template_ipassoc) t = template_format.parse(eip_template_ipassoc)
stack = utils.parse_stack(t) stack = utils.parse_stack(t)
self.create_eip(t, stack, 'IPAddress') self.create_eip(t, stack, 'IPAddress')
before_props = {'InstanceId': {'Ref': 'WebServer'}, after_props = {'InstanceId': '5678',
'EIP': '11.0.0.1'}
after_props = {'InstanceId': {'Ref': 'WebServer2'},
'EIP': '11.0.0.2'} 'EIP': '11.0.0.2'}
before = self.create_association(t, stack, 'IPAssoc') before = self.create_association(t, stack, 'IPAssoc')
after = rsrc_defn.ResourceDefinition(before.name, before.type(), after = rsrc_defn.ResourceDefinition(before.name, before.type(),
after_props) after_props)
self.assertRaises(exception.UpdateReplace, before._needs_update, updater = scheduler.TaskRunner(before.update, after)
after, before, after_props, before_props, None) self.assertRaises(exception.UpdateReplace, updater)
def test_update_association_with_NetworkInterfaceId_or_InstanceId(self): def test_update_association_with_NetworkInterfaceId_or_InstanceId(self):
self.mock_create_floatingip() self.mock_create_floatingip()

91
heat/tests/openstack/neutron/test_neutron_port.py
View File

@ -11,6 +11,7 @@
# License for the specific language governing permissions and limitations # License for the specific language governing permissions and limitations
# under the License. # under the License.
import copy
import mock import mock
import mox import mox
from neutronclient.common import exceptions as qe from neutronclient.common import exceptions as qe
@ -474,7 +475,7 @@ class NeutronPortTest(common.HeatTestCase):
def test_port_needs_update_network(self): def test_port_needs_update_network(self):
props = {'network_id': u'net1234', props = {'network_id': u'net1234',
'name': utils.PhysName('test_stack', 'port'), 'name': 'test_port',
'admin_state_up': True, 'admin_state_up': True,
'device_owner': u'network:dhcp'} 'device_owner': u'network:dhcp'}
neutronV20.find_resourceid_by_name_or_id( neutronV20.find_resourceid_by_name_or_id(
@ -482,7 +483,21 @@ class NeutronPortTest(common.HeatTestCase):
'network', 'network',
'net1234', 'net1234',
cmd_resource=None, 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() create_props = props.copy()
neutronclient.Client.create_port( neutronclient.Client.create_port(
{'port': create_props} {'port': create_props}
@ -500,36 +515,43 @@ class NeutronPortTest(common.HeatTestCase):
"ip_address": "10.0.0.2" "ip_address": "10.0.0.2"
} }
}}) }})
neutronV20.find_resourceid_by_name_or_id(
mox.IsA(neutronclient.Client), call_dict = copy.deepcopy(props)
'network', call_dict['security_groups'] = [
'net1234', '0389f747-7785-4757-b7bb-2ab07e4b09c3']
cmd_resource=None, del call_dict['network_id']
).MultipleTimes().AndReturn('net1234')
neutronV20.find_resourceid_by_name_or_id( neutronclient.Client.show_port(
mox.IsA(neutronclient.Client), 'fc68ea2c-b60b-4b4f-bd82-94ec81110766'
'network', ).AndReturn({'port': {
'old_network', "status": "ACTIVE",
cmd_resource=None, "id": "fc68ea2c-b60b-4b4f-bd82-94ec81110766"
).AndReturn('net1234') }})
neutronV20.find_resourceid_by_name_or_id(
mox.IsA(neutronclient.Client), neutronclient.Client.show_port(
'network', 'fc68ea2c-b60b-4b4f-bd82-94ec81110766'
'net1234', ).AndReturn({'port': {
cmd_resource=None, "status": "ACTIVE",
).MultipleTimes().AndReturn('net1234') "id": "fc68ea2c-b60b-4b4f-bd82-94ec81110766"
neutronV20.find_resourceid_by_name_or_id( }})
mox.IsA(neutronclient.Client),
'network', neutronclient.Client.show_port(
'new_network', 'fc68ea2c-b60b-4b4f-bd82-94ec81110766'
cmd_resource=None, ).AndReturn({'port': {
).AndReturn('net5678') "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() self.m.ReplayAll()
# create port # create port
t = template_format.parse(neutron_port_template) t = template_format.parse(neutron_port_template)
t['resources']['port']['properties'].pop('fixed_ips') t['resources']['port']['properties'].pop('fixed_ips')
t['resources']['port']['properties']['name'] = 'test_port'
stack = utils.parse_stack(t) stack = utils.parse_stack(t)
port = stack['port'] port = stack['port']
@ -538,28 +560,25 @@ class NeutronPortTest(common.HeatTestCase):
# Switch from network_id=ID to network=ID (no replace) # Switch from network_id=ID to network=ID (no replace)
new_props = props.copy() new_props = props.copy()
new_props['network'] = new_props.pop('network_id') new_props['network'] = new_props.pop('network_id')
new_props['network_id'] = None
update_snippet = rsrc_defn.ResourceDefinition(port.name, port.type(), update_snippet = rsrc_defn.ResourceDefinition(port.name, port.type(),
new_props) new_props)
self.assertTrue(port._needs_update(update_snippet, scheduler.TaskRunner(port.update, update_snippet)()
port.frozen_definition(), self.assertEqual((port.UPDATE, port.COMPLETE), port.state)
new_props, port.properties, None))
# Switch from network=ID to network=NAME (no replace) # Switch from network=ID to network=NAME (no replace)
new_props['network'] = 'old_network' new_props['network'] = 'old_network'
update_snippet = rsrc_defn.ResourceDefinition(port.name, port.type(), update_snippet = rsrc_defn.ResourceDefinition(port.name, port.type(),
new_props) new_props)
self.assertTrue(port._needs_update(update_snippet,
port.frozen_definition(), scheduler.TaskRunner(port.update, update_snippet)()
new_props, port.properties, None)) self.assertEqual((port.UPDATE, port.COMPLETE), port.state)
# Switch to a different network (replace) # Switch to a different network (replace)
new_props['network'] = 'new_network' new_props['network'] = 'new_network'
update_snippet = rsrc_defn.ResourceDefinition(port.name, port.type(), update_snippet = rsrc_defn.ResourceDefinition(port.name, port.type(),
new_props) new_props)
self.assertRaises(exception.UpdateReplace, port._needs_update, updater = scheduler.TaskRunner(port.update, update_snippet)
update_snippet, port.frozen_definition(), self.assertRaises(exception.UpdateReplace, updater)
new_props, port.properties, None)
self.m.VerifyAll() self.m.VerifyAll()

1
heat/tests/openstack/nova/test_server.py
View File

@ -3650,6 +3650,7 @@ class ServersTest(common.HeatTestCase):
update_template['Properties']['image_update_policy'] = 'REPLACE' update_template['Properties']['image_update_policy'] = 'REPLACE'
# update # update
self.stub_ImageConstraint_validate()
updater = scheduler.TaskRunner(server.update, update_template) updater = scheduler.TaskRunner(server.update, update_template)
self.assertRaises(exception.UpdateReplace, updater) self.assertRaises(exception.UpdateReplace, updater)