]> review.fuel-infra Code Review - openstack-build/heat-build.git/commitdiff
heat engine : Autoscaling reload Loadbalancer correctly
authorSteven Hardy <shardy@redhat.com>
Wed, 27 Mar 2013 16:08:41 +0000 (16:08 +0000)
committerSteven Hardy <shardy@redhat.com>
Thu, 28 Mar 2013 07:58:37 +0000 (07:58 +0000)
Since the change implemented as part of bug #1136148 we no longer
wait for nova to create the instance before reloading the loadbalancer
which means we race nova assigning an IP to the instance and typically
the loadbalancer config ends up with the 0.0.0.0 default value
which the loadbalancer implementation uses when nova returns no IP.

This patch moves the LB reload into a separate function, which is called
after instances are active on group creation or adjustment

Since our loadbalancer is broken without this fix, this is a candidate
for backporting to grizzly milestone-proposed.

fixes bug #1160407

Change-Id: I50423883dead9a615aa79b8765d5480a9345686d

heat/engine/resources/autoscaling.py

index dc0b1023dd8c2f2279115006fd7b46070cd1e721..31914023a1b7194d2fb8658bf450fc79d1897465 100644 (file)
@@ -86,6 +86,8 @@ class InstanceGroup(resource.Resource):
         active = all(i.check_active(override=False) for i in self._activating)
         if active:
             self._activating = []
+            # When all instances are active, reload the LB config
+            self._lb_reload()
         return active
 
     def _wait_for_activation(self):
@@ -202,9 +204,16 @@ class InstanceGroup(resource.Resource):
                 inst_list.remove(victim)
                 self.resource_id_set(','.join(inst_list))
 
+    def _lb_reload(self):
         # notify the LoadBalancer to reload it's config to include
-        # the changes in instances we have just made.
+        # the changes in instances we have just made, this must be after
+        # activation (instance in ACTIVE state), or we may not get network
+        # details for the instance, resulting in incorrect 0.0.0.0 config
+        # being updated via the LoadBalancer resource
         if self.properties['LoadBalancerNames']:
+            inst_list = []
+            if self.resource_id is not None:
+                inst_list = sorted(self.resource_id.split(','))
             # convert the list of instance names into a list of instance id's
             id_list = []
             for inst_name in inst_list:
@@ -326,8 +335,8 @@ class AutoScalingGroup(InstanceGroup, CooldownMixin):
         return self.UPDATE_COMPLETE
 
     def adjust(self, adjustment, adjustment_type='ChangeInCapacity'):
-        self._adjust(adjustment, adjustment_type, False)
-        self._wait_for_activation()
+        if self._adjust(adjustment, adjustment_type, False) is True:
+            self._wait_for_activation()
 
     def _adjust(self, adjustment, adjustment_type='ExactCapacity',
                 raise_on_error=True):
@@ -335,7 +344,7 @@ class AutoScalingGroup(InstanceGroup, CooldownMixin):
         if self._cooldown_inprogress():
             logger.info("%s NOT performing scaling adjustment, cooldown %s" %
                         (self.name, self.properties['Cooldown']))
-            return
+            return False
 
         inst_list = []
         if self.resource_id is not None:
@@ -352,19 +361,21 @@ class AutoScalingGroup(InstanceGroup, CooldownMixin):
 
         if new_capacity > int(self.properties['MaxSize']):
             logger.warn('can not exceed %s' % self.properties['MaxSize'])
-            return
+            return False
         if new_capacity < int(self.properties['MinSize']):
             logger.warn('can not be less than %s' % self.properties['MinSize'])
-            return
+            return False
 
         if new_capacity == capacity:
             logger.debug('no change in capacity %d' % capacity)
-            return
+            return False
 
         self.resize(new_capacity, raise_on_error=raise_on_error)
 
         self._cooldown_timestamp("%s : %s" % (adjustment_type, adjustment))
 
+        return True
+
     def FnGetRefId(self):
         return unicode(self.name)