Converger steps for supporting multiple LB implementations - rackerlabs/otter GitHub Wiki

Preface

All work so far in Otter, both job-oriented and convergent, assumes using Cloud Load Balancers. However, we are adding support for RCv3 (#739) As you might expect, going from 1 to 2 means solving a lot of problems that 2 to 3, or 3 to 4 won't have to solve.

This can be problematic because these what would balancer implementations do not necessarily take the same arguments. For example, Cloud Load Balancers uses ServiceNet IP, whereas RCv3 takes a server ID. When trying to adjust old functions to support both systems, a pattern emerges:

  • IPs should be replaced with server dicts (as returned from Nova). This is because these contain sufficient information to identify the server for both CLB and RCv3 (and, presumably, any other LB system, since it contains all identifying information for that server).
  • Load balancer IDs should be replaced with load balancer configs. This is because you need to know the load balancer's type in order to know what endpoint/API to use to manipulate it.

Examples of where this has been done in order to support RCv3: #737, #738.

This document deals with how the converger needs to change in order to support multiple load balancer types, including RCv3.

Current situation

Currently ISteps are fairly specific. E.g. the step for adding or removing from load balancers is specific to CLB. The obvious thing to do to support new things (like RCv3) is to add more such steps. This has issues:

  1. New LB implementations will require implementing a bunch of steps. (not too bad)
  2. This makes testing convergence functions (like load balancer convergence) more difficult, because you have to test it for every kind of load balancer.

The function that plans adding/removing load balancer/server links shouldn't have to know know about the specific kind of load balancer (or, hypothetically, if Nova wasn't the only server API, the specific kind of server). It doesn’t matter if it’s RCv3 or CLB or Neutron or whatever: its inputs should be the LB connections that exist now, and its output should be links that should be made/severed.

Suggested fix: generic steps

One suggested way to fix this is to have a generic AddToLoadBalancer step, which takes a load balancer configuration, a server info dictionary as returned by Nova, and a specific configuration dict (for stuff like draining).

Manish - Will storing dict causing equality issues when writing tests?

lvh - I don't think so. dict eq semantics seem appropriate here. If we do need some particular eq semantics (like ignoring certain attributes), we can always write another class using characteristic. Any particular issue you're thinking of?

lvh - Manish raised the issue of hashing on Slack. Some algorithms require hashing, so that doesn't work if we have dicts. Suggested solution: pyrsistent.freeze to make the regular Python data structures persistent (and hashable).

lvh - Minor note about naming: someone (Manish? Chris?) pointed out that "node" is CLB-speak, and "server" is more generic. Personally I don't care much is it's "server", "node", or nothing, as long as we're consistent. E.g. right now it's RemoveFromLoadBalancer but AddNodesToLoadBalancer. FOLLOW-UP: RCv3 uses "node" too.

This can be compiled down to specific requests, because it contains enough information for any load balancer type (e.g. server id for RCv3, but IPs for CLB; but it also has the entire LB config, so it knows which endpoint to hit).

This has the benefit that load balancer convergence logic becomes easier to test, and independent from specific load balancer types.

So far, when discussing this, this has been called abstract vs concrete. These terms are not very precise. An instance of the "abstract" AddServerToLoadBalancer step describes adding a specific server to a specific load balancer, and is therefore exactly as concrete as a AddServerToCLB step. The only difference is whether or not different LB types get different step types. Because of this, "narrow" and "wide", or "generic" and "specific" steps would be better terms.

lvh -- I like "generic" and "specific".

Compilation to efficient requests

In order to make fewer, more efficient requests, we want to compile many steps (regardless of whether they're generic or specific) to fewer, more specific ones. For example, any amount of AddServerToLoadBalancer steps for any amount of RCv3 load balancers can be reduced to a single request.

Because IStep has an as_request API, a step must map directly to a request. Since we want specific, optimized requests, this either means that we need specific, optimized steps, or that we write an optimization step for requests. I don't think anyone will disagree that writing an optimizer for steps is a lot better than writing an optimizer for requests, because it is a higher level form, and more clearly captures intent.

as_request for optimizable steps

One remaining question is whether or not it is okay for a generic step to produce an unoptimized (but optimizable) request. For example, you could argue that an AddServerToLoadBalancer step for an RCv3 load balancer should never be allowed to produce a request: in the worst case, the bulk API is exactly as efficient. In every other case, it is much more efficient.

One alternative is to allow it to produce such a request, and maybe log it as a warning. The issue with not allowing it to produce a request (or logging it is a warning) is that it duplicates the logic of whether or not a step is optimizable.

Another alternative is to make steps that are actually "abstract" and "concrete", but in the sense that abstract steps can not be turned into requests directly and concrete ones can. In this scenario, abstract steps always refuse to be turned into a request. It is the optimizer's job to turn them into concrete steps. (At that point, "optimizer" should probably be replaced with "compiler": it is no longer about efficiency, but correctness.)

Votes:

  • lvh: Generic steps by default. as_request should probably always work for generic steps, but could raise warnings/errors for things it knows are unoptimized (don't care deeply; preference for warning, but producing a request anyway). Create specific steps only when necessary for optimization (e.g. RCv3 bulk APIs). Yes, this means that for unoptimized APIs, the "generic" step produces the request directly. I don't care very strongly if you make intermediate steps for that as well. Just doesn't seem particularly useful.