Code review comment for lp:~rconradharris/nova/lp794672

Revision history for this message
Rick Harris (rconradharris) wrote :

Brian Lamar--

> I feel like we're trying to program in a solution where a better solution already exists. What is
> the problem with making glance_host == a load balanced IP and using conventional load balancing to
> do the job here? Load balancing logic doesn't belong in Nova in my opinion. It's just going to
> complicate the code unnecessarily.

I really can't argue with this because, for the most part, this is exactly right. In fact, as mentioned above, that was the original idea behind having a single IP-- we just shove load-balancing off to something that can do it well.

But, leading into your next observation...

> I'd really prefer to leave this out as the only time I'd want to use this is in development...where you don't need load balancing.

There's the rub. According to Ant, they're running into issues testing where Glance is the bottleneck. Of course, we could always say, 'just toss a LB in there'.

The issue with that is, they're going to be spinning-up and tearing-down test envs left and right. It could be a burden to constantly have to configure an external component each time. In this case, dirt-simple `random.choice` is good 'nuff though.

> What's next? Weighted-least-connections algorithm support? Automatic health checks? Caching?
> Adding/removing nodes on the fly?

If each one of those had compelling arguments to why they should be included for testing and dev, then yeah, maybe should consider adding rudimentary versions of those. But, none of the above seem very compelling :).

I think the take-away is, from my perspective, we may need to add some less-than-ideal versions of components to facilitate testing. Sure, it will complicate the code somewhat. Hopefully reduced friction amortized over hundreds of test iterations make the trade-off worth it.

« Back to merge proposal