Lease storage API

[Natolumin]

Following #110 and #108 (comment) I want to start a discussion about what the lease storage API would/should look like. This is pretty long but I have no real conclusion at the end, I hope we can build feedback and some PoCs to see where we should go

Quick note about terminology:

• consumer plugin: Uses the interface to query/store leases
• provider plugin: Implements the interface, stores and maintains lease information

Basics

The base interface should act like a map, indexed by a client ID, returning leases

What's a client ID ?

Probably not the same for v4 and v6.
v4 usually identifies client by hardware address, but there's also a client-identifier option (61) that probably almost nobody actually uses
v6 client-ID option is mandatory and is probably much safer to use
However even in v6 administrators often care more about the MAC addresses. Which the RFC forbids but that ship has sailed

What's a lease ?

There's an expiration date. That's easy (not)(We'll come back to this later)
There's an IP. Or multiple (v6, multiple IA_ADDR in a single IA_NA). Or a prefix (or multiple)
There can be multiple leases per client. This is extremely common in v6 but not nearly as much in v4

Can we need to search/index by other than a client ID ?

For things like ad-hoc replication, or leasequery, we may need a bulk read(/write) interface

Do we need additional data in addition ?

• Information on the validity on the lease ? Eg if there is relay info, a
  lease could be linked to a network segment, and if the client moves the
  lease becomes invalid.
  It might not need to be stored in the lease though

• A solution could be to have optional auxiliary interface{} data. However
  this makes it harder to share a lease storage across several plugins (is
  that something we want? need?). Maybe index the additional data by
  plugin name/instance, and plugins can only retrieve leases they stored
  themselves

Usage (and locking)

The most common operation of a consumer plugin is a read-modify-update loop on a single client:

• Receive the client request, fetch leases for the client
• Extend and/or allocate one or more leases for the client
• Write back the lease(s) to the provider

This means we need to prevent another plugin (or the same plugin in another
request) from modifying leases for a client we've "looked up" and intend to update.

Interface

Looking up a client gives, along with the info, a token (opaque, managed by the plugin), which has to be given during updates to ensure consistency of the results.
On the backend this can map nicely to multiple constructs:

• For a key-value backend like eg consul, the token can be a modification
  index, which is atomically compared when updating the value, and the
  modification rejected if the base isn't the expected one
• For an SQL backend this maps to a SELECT ... FOR UPDATE (or equivalent),
  where the backend opens a transaction, and the token can be an
  identifier/index to find the transaction later to complete it
• For a simpler "in-memory map" backend it could also be ignored by the
  provider plugin and have Lookup/Update take a lock and block instead

type Token struct{
	pluginRef *plugins.Plugin // or just an index or something
	token     interface{}
}

type LeaseStorage interface {
	Lookup(clientID) (Token, []Lease, error)
	Update(clientID, Token, []Lease) error

	ReleaseToken(Token) // This is only necessary if Update is never called
}

Notice that some of these operations, instead of actually "locking", return
an error on races at the time of the update.

Errors and retrying

Due to the diversity of backends, there are many cases where some calls can
fail in retryable ways. We'll need to define, along with the interface, error
types that have to be handled in a specific way by the clients.
We can define the base errors and providers will wrap them, and clients test with the go 1.13 error API if they match (errors.Is)

At least:

• A "Transport" related error (network issues, etc): Doesn't invalidate the
  token, retryable
• A "Concurrency" related error: Invalidates the token, retryable (but needs a new lookup)

(This doesn't preclude other errors, eg for bad parameters)

Lease Expiration

Leases expire, and something needs to clean them up at some point.

Either the lease storage plugin or the calling plugin has to clean up expired
leases. I'd rather the storage plugin handle it so that it doesn't need to be
reimplemented for multiple plugins. Additionally it will probably need some
specific knowledge of the backend to find/clean the leases

However, there may be additional handling to do on lease cleanup. At least if
there is IP allocation, IPs (may) need to be freed. I can think of 2 solutions:

• The lease contains an optional Allocator from which the IP(s) were obtained
  (q: can we have multiple IPs in a single lease from multiple allocators ?
  how do we handle that then)
• The lease contains a callback to free it (interface to be defined. Would you
  pass the entire lease object ? How do you know which allocator to use to free it ?)

Anyway, First Draft

const (
	CidHWAddress = iota
	CidOpt61
	CidDUID
)

// clientID I'm not sure about and don't like too much
// We'd basically be using this as a tagged enum
type ClientID struct {
	// Variant will be one of a few constants identifying types of clientIDs
	Variant uint8
	// data here is a string only used as a read-only []byte that we can use to index maps
	Data string
}

// Lease holds data for a single lease to a client
type Lease struct {
	// Elements is a type generic enough that we can hold any known type of
	// lease (one or multiple IPs or prefixes)
	Elements []net.IPNet

	// Expire is the expiration date of the lease
	Expire time.Time

	// Owner keeps a reference to the plugin that inserted this.
	// It may be used for filtering leases and not touching those from other plugins
	Owner *plugins.Plugin

	// ExpireAction is the callback invoked on lease expiration. It receives
	// Elements and Expire as parameters
	ExpireAction func(elements []net.IPNet, expiredAt time.Time)

	// Here we may need to add something like (and pass it to ExpireAction at least)
	// AdditionalData interface{}
}

type Token interface{}

type LeaseStorage interface{
	// Lookup obtains the leases for a client and prepares an update to them
	Lookup(ClientID) ([]Lease, Token, error)

	// Update attempts to update the leases for ClientID.
	// It may fail and invalidate the Token, after which Lookup() needs to be
	// performed again (and in general the whole operation restarted)
	// On success, the Token is invalidated
	// It may also fail without invalidating the token and be retried
	Update(ClientID, []Lease, Token) error

	// ReleaseToken discards an existing token, after which it can't be used in
	// an Update() anymore
	ReleaseToken(Token) error

	// Possibly, if especially useful, a read that only reads and doesn't create a token:
	// ReadOnlyLookup(ClientID) ([]Lease, error)
}

[skoef]

@Natolumin this lease storage API seems like a really good idea, sorry to see that it didn't get more momentum!

In a local build of CoreDHCP I experimented with using fsnotify to update the StaticRecords in the file plugin when something changed to the leases file. Would this be something worth polishing up and sending in as a PR, while this API is still a draft?

[Natolumin]

This is definitely a draft - I have a somewhat working branch but we haven't managed to review either the design or the code yet and I haven't worked on it in a while either so it's not moving fast at all

Certainly feel free to send your improvements - we will probably even be able to reuse them in this API if it ever goes forward

[dulitz]

Just wanted to make a note here that I read your design and it seems thoughtful. I had questions.

• Do you envision multiple plugins cooperating to allocate addresses? I do. For example, first a plugin would try to allocate an address bound to the MAC/ClientID. If there are still unallocated nontemporary requests after that plugin, a second plugin might allocate from a general pool of semistable addresses. Finally, any requests for privacy addresses (IA_TA) would be allocated by a specialized allocator that switches them up on the regular.
• What is the problem that the Lease Storage API is meant to address? If it's an issue of plugins cooperating, the stack I imagine above doesn't have a coordination problem. The first plugin that can handle it does. Or does it provide a set of helpful primitives that many allocator plugins would need to deal with (such as lease expiration, or attaching a blob of data to a client so we can see it on subsequent requests like a server-side "cookie")?
• Would you envision that allocating plugins normatively ought to use this API, even if they don't need to deal with expiration or storing a blob associated with each client? Consider the file plugin. Those leases don't really have "expirations" that the server cares about anyway. If the static mapping file changes on the server and the client requests another address then it's okay for the address to change at that point, and if the static mapping file doesn't change the client is just gonna get the same address as last time.

I also have a comment, which is that one of the things that appealed to me about coredhcp is that you don't impose any requirements on how expiration is supposed to work. In our use case as an ISP, we typically assign one address per interface. The subscriber is allowed to connect only one device at a time, so whatever device asks for an address/prefix on that interface we will give it the same address/prefix even if a different device just got that same lease 30 seconds ago. I don't think ours is a common use case but it's nice that it fits well here.