[kea-dev] Initial proposal for Kea Control API

Tomek Mrugalski tomasz at isc.org
Tue Aug 16 18:49:14 UTC 2016


Ok, it seems I finally managed to go through the comments. See my
responses below.

W dniu 11.07.2016 o 11:17, Marcin Siodelski pisze:
> 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.
Rearranged the text. Hopefully it's more obvious which text relates to
which requirement.
> 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?
Initially I was opposed to specifying any concrete number, because no
matter which value we would pick, someone would complain that he want to
set more leases/subnets/whatever than the value allows. On the other
hand, if we allowed dynamic buffers that would grow as the data comes
in, it would open up a DoS vector. So I picked the lesser of two evils.
The buffer size of 64K is now explicitly mentioned.

> 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"
Updated as requested, with extra clarification: "from a specified JSON
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.
Updated as suggested.
> We may want to consider additional requirement:
> A.X. Kea MUST support "test-config" command to dry run server
> reconfiguration.
Added A.10 and a text explaining that in some isolated cases the
test-config is not fool proof. Example of opening sockets and connecting
to the database is given.
> 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 suppose which process exposes the API is an implementation detail. At
this stage the document says "X will be possible", without going into
much detail. What you mentioned will probably be a significant aspect of
the design that we will work on in the near future.

> I assume that whatever we
> currently support we will also extend to be supported in D2.
That's a good point. Currently D2 does not support command API at all.
That's going to be changed for sure.
>
> 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.
I think the auto-generation of subnet-id is a convenience for simpler
deployments. If you want to tie reservations and leases from the
database with your configuration, you'd better specify the ids
explicitly. And if you really want Kea to generate them, you could use
get-subnet4-count and get-subnet4 to retrieve the information, before
you start fiddling with leases or reservations.

> 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.
Yes, we talked about that, but haven't made any decisions. In
particular, we have different understandings of what multi-tenancy
really means and how to implement it. This is a big feature on its own
right and I don't want to make any assumptions that would prevent us
from implementing one approach or excluding another. So there are no
provisions for multi-tenancy in the API. If you want to add them, please
design the multi-tenancy support first.

> 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.
We certainly could do lots of things. But could does not imply should :)
Let's start with what we have requirements for now and see if users try
them out and then request extra capabilities.
> 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.
Reworded. Let me know the text is better now.
> 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.
The text was written before allocation engine was able to handle
multiple IPv6 address/prefix reservations. Since it can do that now, the
text became a bit outdated. Updated. Updated H.3 and H.4 to say "zero,
one or more IPv6 addresses/prefixes". It sounds a bit odd, but I wanted
to emphasize couple things. First, that you don't have to reserve
anything (zero). Second, that you can do reservation similar as you can
do in IPv4 (one) or you can go fancy and reserve many.
> 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.
No, I would prefer to stick with reservation. The consistency between
naming convention in the code and this API is meaningless. And people
could get confused if we call them 'hosts'. For example, some could
assume that for every device in their network the need (or kea creates)
a host, which is not true.
> 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) ?
Because most people are using client-id and only some people use
circuit-id? Because you can use client-id everywhere and you can use
circuit-id only if you use relays and those relays support circuit-id?
> H.12. We aren’t supporting remote-id at the moment.
I can't disclose publicly why remote-id is here :)
> 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.
It is MUST now.
> 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.
Yeah, I was thinking about this. Since the reservations in v6 don't have
any unique identifiers, we would need something like
get-reservations6-count and then reference them by index. We may
implement API like this if we get actual requests from the users asking
for this. For the time being I think updating the whole thing (with
'thing' being v6 reservations, v4 options or v6 options) at once is the
way to go. I don't want to over specify the API unless there are actual
deployments who would use that specific calls.

> 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.
I was thinking about something similar, but no matter how you slice the
problem, it adds a lot of complexity for something that's probably is
not that useful. I can imagine that typical use cases will be either a
GUI that edits existing reservation (it already has all the details, so
pushing those bits that haven't changed is not a big deal) or an IPAM
(that has its own database and would simply push the whole host info
when it changes).
> 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?
That's a good question. I would think it would delete the whole
reservation, because the IPv6 address is simply used as a selector.
> 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
Ok, added S.3.1 and S.5.1. (I will renumber them one day when we receive
all the external feedback we're waiting 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.
Again, we may implement that if there are users interested in this.
Otherwise I feel we may be over-engineering this.
>
> 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. 
RFC3203 dares to claim otherwise :)
> 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.
Agree regarding 3, unless used with care. For example you're turning off
a lab. You already shutdown or dismantled the hardware and want to get
rid of the whole subnet. There may be other use cases. I think the "burn
it to the ground" approach should definitely not be the default, but it
may be nice option for thorough clean up.

> 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?
Because of radically different use cases. How often do you think people
would add new option definitions? Once, twice a year maybe, each time
adding one or two definitions. For hosts, the answer may be as high as
"every 5 minutes" or "a thousand each day".

Do I get your comment correctly that you're suggesting to remove O.3 and
O.4? If that is so, I'm very much ok with that.

> 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.
We can go down this rabbit hole as deep as you want. In particular,
there is the problem of retrieving all host reservations and making sure
that each of them is consistent with the new option definition. And if
it isn't, what should the code do? Reject the update? Invalidate the
host somehow? Maybe Just notify the user that the definitions are
inconsistent? On the other hand, maybe it wouldn't be the best idea to
suddenly retrieve millions of reservations from the DB when some adds
new option definition, because we can't enforce the consistency there?
At any given time, a sysadmin can connect to the DB and add options that
are not consistent with the definitions.

Anyway, I don't want to solve all those problems at the requirements
level. These are the topics good for the design phase.
> 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.
Avoiding this kind of problems is an argument for going with one option
per call approach. It would either succeed completely or entirely fail.
So if we go with the remove O.3 and O.4, this transactional model would
be done with the regular sanity check.
> 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.
Renamed as suggested.
> 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?
Yes. Added I.5 and I.6.

Ok, that's it. Thanks for this thorough review and sorry it took me so
long to get back to you, but the requirements type of documents are
often lesser priority if they're not to be implemented in the current
milestone.

Tomek




More information about the kea-dev mailing list