Open Stack

Hi Winson,

On Fri, Aug 30, 2013 at 02:23:18PM +0000, Chan, Winson C wrote:
> Regarding the last set of comments on the UpdatePolicy, I want to bring your attention to a few items.  I already submitted a new patch set and didn't want to reply on the old patch set so that's why I emailed.

Sorry for the slow response on this, you've already had some good feedback
from Zane and Clint, and I've just reviewed your latest patch:

https://review.openstack.org/#/c/43571/

I'd really like input from the other core guys on this, particularly my
complaints about the changes to the internal interfaces (where we start
passing lists-of-names around again instead of a number)

I'm opposed to spreading this interface change around as in the current
patch (as I said in Patch Set 3, prompting your mail) - the problem is the
interfaces just get much less obvious and far more error-prone IMHO (as has
been proven previously).

> As you are aware, IG/ASG currently create instances by appending group name and #.  On resize, it identifies the newest instances to remove by sorting on the name string and removing from the end of the list.

So I agree with Zane that this is not a requirement for your patch, merely
an implementation detail of the old code.  However if we can support this
mode of replacement (either now or in a subsequent patch) then fine.

> 
> Based on your comments, in the new patch set I have changed the naming of the instances to just a # without prefixing the group name (or self.name).  I also remove the name ranges stuff.  But we still have the following problems…
> 
>   1.  On a decrease in size where the oldest instances should be removed…  Since the naming is still number based, this means we'll have to remove instances starting from 0 (since 0 is the oldest).  This leaves a gap in the beginning of the list.  So on the next resize to increase, where to increase?  Continue the numbering from the end?
>   2.  On replace, I let the UpdateReplace handle the batch replacement.  However, for the use case where we need to support MinInstancesInService (min instances in service = 2, batch size = 2, current size = 2), this means we need to create the new instances first before deleting the old ones instead of letting the instance update to handle it.  Also, with the naming restriction, this means I will have to create the 2 new replacements as '2' and '3'.  After I delete the original '0' and '1', there's a gap in the numbering of the instances…  Then this leads to the same question as above.  What happen on a resize after?
> 
> The ideal I think is to just use some random short id for the name of the instances and then store a creation timestamp somewhere with the resource and use the timestamp to determine the age of the instances for removal.  Thoughts?

+1, using the random short ID seems like a good plan, as does using the DB
timestamp instead of string sorting to decide ordering

However, I still don't think this means we need to pass lists of resource
names around inside the autoscaling implementation - the only place where
we care about the resource names should be in the function creating the
template, e.g not in resize/replace, we just pass the number to replace and
the number to create into _create_template, then update the nested stack
with the new template.

Thanks for the work so far on this, looks like it will be a nice new
feature when we get these final issues sorted out! :)

Steve