[kea-dev] Initial proposal for Kea Control API

[Marcin Siodelski]

On 25.05.2016 01:27, Tomek Mrugalski wrote:
> Hi,
> 
> I have just finished an initial proposal for Control API for Kea. The
> proposal is available here:
> 
> http://kea.isc.org/wiki/ControlAPIRequirements
> 
> I'm very eager to hear your opinions on this. If you're an existing or
> prospective user, can you also share your perceived priorities of those
> proposed calls? Sharing your opinion on kea-dev is the best way to
> respond, but sending your comments privately off the list is ok, too.
> 
> The complete API is huge and there is no way ISC could implement all of
> it, at least not in one release. Therefore it's very useful to
> prioritize which calls should be implemented first.
> 
> Note that ISC does not plan to make actual implementation of the API in
> Kea 1.1. We had preliminary discussions whether it would be a good
> leading topic for Kea 1.2, but we did not make any specific decisions yet.
> 

Tomek,

This is a very good first stab for the Control API requirements. I have
a couple comments which we may want to discuss.

General comment:

Using normative language for requirements is odd. If we consider
something a useful feature for Control API we should assume we'll
implement it. Whether we do implement it or not for a particular release
should be decided as a part of the roadmap discussion, not as part of
the requirements.

A.2. The entire discussion for this requirement should be below the
requirement because otherwise it looks like a description above the A.2.
refers to requirement A.1. I was pretty confused initially reading it,
until I found that the text refers to A.2.

A.2. I think we should better reword this requirement to state how large
commands we’ll actually support, rather than vaguely state that they
will be larger than 1500 bytes. Perhaps we could take a typical
deployment which we’re going to satisfy and based on the command size
try to assess how large command (in terms of bytes) is required to issue
a command that takes the larger number of parameters (the worst case
command) and go from there?

A.3. It may be good to clarify in the requirement that it pertains to
configurations stored in the JSON file and not to other configuration
storages. Admittedly, we don’t support other configuration sources (e.g.
database) but at this point we should think if „reload-config” will be a
generic command for any source of configuration or if you stick to a
database, another command will be provided. Also, the requirement itself
could say that the location of the configuration storage can be changed,
e.g. „Kea MUST support reload-config to reload configuration from a
specified file"

A.4. Perhaps more accurate to say: „Kea MUST support leases-reclaim
command to trigger reclamation of expired leases”. Note that expiration
is something that you don’t force but it is something that happens over
time by itself.

We may want to consider additional requirement:
A.X. Kea MUST support "test-config" command to dry run server
reconfiguration.

Looking at all requirements for „Administrative Actions” I realized that
it is unclear to me if those requirements pertain to „Kea” as a
collection of all modules or to individual modules respectively. I
suppose, the latter as we don’t currently have any common module/process
handling commands for all Kea processes. I suppose it should be more
obvious from reading the requirements if we’re planning to provide
ability for the administrator to „config-reload” configuration for all
services with a single command (DHCPv4 + DHCPv6 + D2) or it will be
required to reload configuration for each of them individually. Note
that in some cases, e.g. DHCPv4o6 or DHCPv4+DDNS it doesn’t make sense
to reload configuration for a single process. I assume that whatever we
currently support we will also extend to be supported in D2.

L4. and L6. While we have everything structured by subnet-id, from the
user perspective it would be probably much more convenient if he
wouldn’t have to specify subnet-id to retrieve a lease. The problem is
that the subnet-id is a value that has to be retrieved from the
configuration. If the configuration is large or subnet-id is auto
generated, this may pose issues with finding subnet-id. So, maybe we
should also consider providing a command: get-leasesX(identifier-type,
identifier) to find all leases for a particular client. In many (most)
cases it would return a single lease.

To answer question, I’d suggest we stick to a single command name -
maybe call it get-leasesX to support a general case that multiple leases
can be returned and depending on what arguments are specified, the
command would perform searches by ip-addr, identifier,
identifer+subnet-id etc. I just find it more convenient to have a single
command name.

L9. The text says that the only parameter required will be IP address. I
wonder if we should really make such a constraint. We talked about the
possibility to extend Kea to assign the same address within multiple
subnets. Or…. there could be a shared IP address case and port-set
assignment …. In such case, you’d need to be more specific which lease
you want to delete.

As for the note about retrieving all leases within a subnet, we do have
a lease-dump tool that can dump the database into a CSV file. We could
potentially extend it to filter out leases belonging to a particular
subnet and such. But, this is out of scope. Though, I think we need to
have ability to retrieve multiple leases for a single client as I stated
above… or multiple leases for the same IP address.

Host Reservations (HR) Management:
The „Note” says that the change in the host reservations will be
reflected "in-memory” only. I think the intent is to say that if you’re
using a backend that can’t store reservations in the DB, you’d simply
keep reservations in-memory, but this is not what this text really says.

In the following sentence:
"It will be possible to specify IPv4 address, IPv6 addresses (typically
one, but eventually it will be possible to specify more), IPv6 prefixes
(typically one, but eventually it will be possible to specify more),"

I am not sure what is meant by „eventually it will be possible to
specify more”. Do we treat multiple addresses/prefixes for a single host
as something optional, something that will be implemented in some
subsequent release? My feeling is that it should be available from day
one, simply because HR implementation already allows for multiple
reservations so users should have tools to make use of this capability.
That means that H.3 and H.4. should be updated.

We may want to consider using „add-host”, „get-host” naming rather than
„add-reservation”, „get-reservation” etc. to be consistent with the
naming in the HR code. Note that in this case the „Host” holds a
collection of various reservations, such as IPv4 address, IPv6
address/prefix, hostname etc. Whereas in this document the „reservation”
is actually describing a host with all its reservations. I am not going
to argue about it, because one may think that „reservation” is more
descriptive, but apart from consistency the „host” is actually easier to
type.

H.10. I don’t see why MAY for circuit-id, rather than MUST? We do
support circuit-id and we have added this support because users had
asked us to do so. I realize that MAY doesn’t mean we’re not going to
implement it (simply trying to prioritize) but I don’t see why this is
less important than any other requirement, especially less important
than H.10 (client-id) ?

H.12. We aren’t supporting remote-id at the moment.

H.16. I don’t understand why this is less important than having ability
to retrieve hosts by IP address. I also suggest that this variant of
get-reservation allows for not specifying a subnet-id, in which case
reservations for all subnets are returned for the particular identifier.
Note that „subnet-id” may sometimes be hard to find.

H.17. So there is an interesting use case whereby a client already has
an IPv6 reservation(s) and you want to add one more IPv6 address for it.
When calling the „update-reservation” would you have to specify the
entire specification for all existing reservations (including existing
IPv6 addresses, hostname, options etc) plus the IPv6 address reservation
being added? I think this is probably the case, because otherwise you
can’t tell whether you’re simply adding a new reservation, or this new
reservation replaces an existing reservation etc. But it strikes me that
it would be also useful to have variants of the update-reservation that
specifically allow for modifying specific piece of the host data, such
as „add IPv6 address to the reservations”, „delete IPv6 address from the
reservations”, „add single option for the host” etc. This comment may be
not relevant for the requirements but rather for the design document,
but at this stage we should know if the „update-reservation” command is
intended to perform partial updates to the existing reservations or we
should come up with additional commands that handle partial updates.

Now I see that you’re actually asking a question about it… I think the
answer for your question is „yes” - we should add something like
add-ipv6-reservation etc. but…. perhaps this may be somehow handled
within „update-reservation, rather than with a new command. For example,
we could be providing extra parameter (subcommand) along with the IPv6
address being added, e.g. operation: ADD, REPLACE, DELETE”. Depending on
this parameter the address would be appended to existing reservations,
would replace the existing addresses or would be deleted (assuming it is
already reserved). BTW, there is similar problem with options, not only
IPv6 addresses/prefixes.

H.19. In case the client has multiple IPv6 reservations, what would the
„delete-reservation by IPv6 address” actually do? Would it remove this
particular address from the group of reserved addresses for this host
and leave the host information in place (including existing options,
other IPv6 reservations, IPv4 reservations)? Or, it would erase the host
information from the database entirely?

Subnets management
So, wouldn’t it be actually useful to have a command that retrieves
subnets by a subnet prefix? Such as get-subnet(„2001:db8:1::/64”) ? I am
not going to say that getting subnet by index is not going to have its
use cases, but if you have many subnets you may end up iterating quite
long before you find the subnet you’re looking for

S.7. and S.8. Perhaps we may consider adding a „subcommand” which would
allow for incremental changes, such as:
update-subnet:
{
….
„pools”: [ „2001:db8:1::100 - 2001:db8:1::200” ],
„pools-action”: „append"
...
}

which would indicate that the pool should be inserted into existing
pools. Otherwise, you’d need to specify all existing pools to not
override them with a single (new) pool.

S.11. and S.12. As for your question regarding deletion of subnets it
seems to me that the gradual deletion is the one that should be
supported for sure. I also think that relying on „reconfigure” is not
going to work for the reasons you mentioned. Plus, DHCPv4 doesn’t
support reconfigure. Option 2) is in fact a variant of option 1)
(gradual removal) but provides less control over the deletion. I think
we could provide a parameter controlling whether the subnet is
„force-deleted” or „gradually-deleted”. I would not vote for option 3) -
deletion of leases together with a subnet as it would cause
inconsistency between the client and server states.

O.3. and O.4. Why do we have a commands for adding multiple option
definitions in a single command but not for adding multiple hosts or
subnets? I presume we have a command to set multiple options because it
may be the case that some options do not make sense for a client without
some other options and you want to make sure they are set in a single
transaction? (all or nothing). For option definitions it is probably ok
to set a single option definition at the time?

There is also an interesting question what happens if you have multiple
options specified for existing option definitions and then you use
„set-options4-def” to change formats of those options? Would this
invalidate all existing options specifications? This should be a part of
the sanity check and the conflicting option definitions should not be
set until option-data using old data formats are removed.

Another question is whether we’re planning to implement some
„transactional” model of setting multiple options/option definitions, in
which if setting one of the option definitions/options fails, all other
option definitions/options are rejected? That’s what I’d think.

I think that it is better to rename „set-options4-def” (and alike) to
„set-option4-defs” or to „set-multiple-option4-defs” because it is easy
to confuse „set-options4-def” with „set-option4-def” („s” is in the
middle of a command name). In addition it sounds like you’re adding a
single definition for multiple options, whereas you’re adding multiple
definitions.

Interfaces management - would it be worth to provide a command that adds
a single interface to a service and a command that disables an interface?

Marcin