From 7506d2c22fcd3d6e825ee660a4b20c5947cc7e9a Mon Sep 17 00:00:00 2001 From: Zane Bitter Date: Fri, 10 Oct 2014 15:43:16 -0400 Subject: [PATCH] Don't update a LoadBalancer under autoscaling control Unfortunately, Autoscaling currently uses the update() method of a LoadBalancer resource to do updates, with the result that the current member list gets persisted (good) and used to compare in the event of future *stack* updates (bad). With this patch, we assume that LoadBalancers under the control of Autoscaling will never have a members list property supplied in the template. We then ignore any updates to Autoscaling LoadBalancers that don't actually modify the template. The test changes revert the changes made in order to be able to merge d32370233eaf2a5c32888f269bd1dc5e0e787467, before which LoadBalancers were behaving correctly. Change-Id: I9c02ab3d3dfbee0a8a90dd0ba345a5acdaf8a610 Closes-Bug: #1379619 --- heat/engine/resources/loadbalancer.py | 14 ++++++++++++-- heat/engine/resources/neutron/loadbalancer.py | 19 ++++++++++++++----- heat/tests/test_neutron_autoscaling.py | 9 --------- 3 files changed, 26 insertions(+), 16 deletions(-) diff --git a/heat/engine/resources/loadbalancer.py b/heat/engine/resources/loadbalancer.py index be0658bb0..f265265ff 100644 --- a/heat/engine/resources/loadbalancer.py +++ b/heat/engine/resources/loadbalancer.py @@ -422,7 +422,7 @@ class LoadBalancer(stack_resource.StackResource): servers = [] n = 1 nova_cp = self.client_plugin('nova') - for i in instances: + for i in instances or []: ip = nova_cp.server_to_ipaddress(i) or '0.0.0.0' LOG.debug('haproxy server:%s' % ip) servers.append('%sserver server%d %s:%s %s' % (spaces, n, @@ -481,7 +481,17 @@ class LoadBalancer(stack_resource.StackResource): save it to the db. rely on the cfn-hup to reconfigure HAProxy ''' - if self.INSTANCES in prop_diff: + new_props = json_snippet.properties(self.properties_schema, + self.context) + + # Valid use cases are: + # - Membership controlled by members property in template + # - Empty members property in template; membership controlled by + # "updates" triggered from autoscaling group. + # Mixing the two will lead to undefined behaviour. + if (self.INSTANCES in prop_diff and + (self.properties[self.INSTANCES] is not None or + new_props[self.INSTANCES] is not None)): templ = self.get_parsed_template() cfg = self._haproxy_config(templ, prop_diff[self.INSTANCES]) diff --git a/heat/engine/resources/neutron/loadbalancer.py b/heat/engine/resources/neutron/loadbalancer.py index e9bdbb0cc..3cb913f47 100644 --- a/heat/engine/resources/neutron/loadbalancer.py +++ b/heat/engine/resources/neutron/loadbalancer.py @@ -640,7 +640,6 @@ class LoadBalancer(resource.Resource): MEMBERS: properties.Schema( properties.Schema.LIST, _('The list of Nova server IDs load balanced.'), - default=[], update_allowed=True ), } @@ -652,7 +651,7 @@ class LoadBalancer(resource.Resource): client = self.neutron() protocol_port = self.properties[self.PROTOCOL_PORT] - for member in self.properties.get(self.MEMBERS): + for member in self.properties[self.MEMBERS] or []: address = self.client_plugin('nova').server_to_ipaddress(member) lb_member = client.create_member({ 'member': { @@ -662,8 +661,18 @@ class LoadBalancer(resource.Resource): self.data_set(member, lb_member['id']) def handle_update(self, json_snippet, tmpl_diff, prop_diff): - if self.MEMBERS in prop_diff: - members = set(prop_diff[self.MEMBERS]) + new_props = json_snippet.properties(self.properties_schema, + self.context) + + # Valid use cases are: + # - Membership controlled by members property in template + # - Empty members property in template; membership controlled by + # "updates" triggered from autoscaling group. + # Mixing the two will lead to undefined behaviour. + if (self.MEMBERS in prop_diff and + (self.properties[self.MEMBERS] is not None or + new_props[self.MEMBERS] is not None)): + members = set(new_props[self.MEMBERS] or []) rd_members = self.data() old_members = set(rd_members.keys()) client = self.neutron() @@ -688,7 +697,7 @@ class LoadBalancer(resource.Resource): def handle_delete(self): client = self.neutron() - for member in self.properties.get(self.MEMBERS): + for member in self.properties[self.MEMBERS] or []: member_id = self.data().get(member) try: client.delete_member(member_id) diff --git a/heat/tests/test_neutron_autoscaling.py b/heat/tests/test_neutron_autoscaling.py index 6105287ed..4a266c4d6 100644 --- a/heat/tests/test_neutron_autoscaling.py +++ b/heat/tests/test_neutron_autoscaling.py @@ -122,7 +122,6 @@ class AutoScalingTest(HeatTestCase): self.m.StubOutWithMock(neutronclient.Client, 'show_pool') self.m.StubOutWithMock(neutronclient.Client, 'show_vip') self.m.StubOutWithMock(neutronclient.Client, 'create_member') - self.m.StubOutWithMock(neutronclient.Client, 'delete_member') self.m.StubOutWithMock(neutronclient.Client, 'list_members') self.m.StubOutWithMock(nova.NovaClientPlugin, 'server_to_ipaddress') @@ -310,8 +309,6 @@ class AutoScalingTest(HeatTestCase): nova.NovaClientPlugin.server_to_ipaddress( mox.IgnoreArg()).AndReturn('1.2.3.5') - neutronclient.Client.delete_member(mox.IgnoreArg()).AndReturn(None) - neutronclient.Client.create_member(memberb_block).\ AndReturn(memberb_ret_block) @@ -321,12 +318,6 @@ class AutoScalingTest(HeatTestCase): neutronclient.Client.create_member(memberc_block).\ AndReturn(memberc_ret_block) - nova.NovaClientPlugin.server_to_ipaddress( - mox.IgnoreArg()).AndReturn('1.2.3.4') - - neutronclient.Client.create_member(membera_block).\ - AndReturn(membera_ret_block) - self.m.ReplayAll() # Start of stack create