BIND 10 #2140: Create abstract API for lease database access

[BIND 10 Development]

#2140: Create abstract API for lease database access
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  tomek
  stephen                            |                Status:  reviewing
                       Type:  task   |             Milestone:  Sprint-
                   Priority:         |  DHCP-20121004
  medium                             |            Resolution:
                  Component:  dhcp   |             Sensitive:  0
                   Keywords:         |           Sub-Project:  DHCP
            Defect Severity:  N/A    |  Estimated Difficulty:  0
Feature Depending on Ticket:         |           Total Hours:  0
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Comment (by sar):

 I have one architectural level question and a set of simpler questions or
 comments.

 The architectural level issue deal with getting information from the db
 via the lease_mgr api.  In the current code a given key may map to several
 different leases on different subnets.  These could either be somewhat
 fixed,
 for example a laptop used for testing that has a set of fixed addresses
 and can be carried from subnet to subnet.  Or they could by dynamic, for
 example if a laptop is moved around a campus and acquires multiple leases.

 The functions that find a lease via some key (clientid, hwaddr etc)
 don't seem to allow for either finding a lease within a subnet or
 walking through all of the leases with that key until one for
 the proper subnet is found.


         Comments on DUID (dhid.h, dhid.cc)
 The return of a pointer to the private vector in the get functions causes
 me
 a certain amount of concern.  Howerver I don't have any concrete problems
 with
 it and therefore it's probably fine as is until how the rest of code might
 usae the pointer becomes more clear.

 I have a larger concern about the typing in the structures.  Perhaps it
 won't
 happen but it seems like there is some potential for confusion between the
 different types if the structures can be mixed.  It also seems that
 deciding
 a 4 byte id is an IPv4 address might not always be true.

 I'm not sure about putting an IPv4 address into the same structure - do
 you
 have a use case for that already?

 Lastly did you consider adding functions to create the various types of
 DUIDs?
 (LL, LLT etc) It seems to be that such functions would be useful and would
 go into this class.

         Comments on lease_mgr (lease_mgr.h, lease_mgr.cc)
 Is there a description of how the lease structures will be used (in a
 general sense)?  I'm imagining the lease_mgr  structure being used as a
 temporary holder to get the data from the backend db, manipulate it
 and then push the changes back to the backend.  Is that somewhat
 correct?

 Are you planning on including the type of the hardware addess as the
 first element in the hardware address vector?

 There are several additional timestamps associated with failover.
 I suppose they can be deferred for now and added if/when we do failover.

 I'm curious about the choice for valid_lft.  Making it seconds since cltt
 means that most uses of it will require getting cltt and doing an
 addition.
 Storing it as a time_t in its own right might take more space but might
 make
 the code more efficient.

 Does recylce_time need to be in the structure?  If the lease structures
 aren't
 permanent I can see the use of this functionality to provide a "grace"
 period
 for a user to return and reclaim an address.  However I'm a bit iffy on
 needing
 it on a lease by lease basis vs server wide basis.

 I'm not sure we need the DDNS flags here.  In the current DDNS
 requirements it
 appears that there will probably be a separate DDNS structure to track
 DDNS
 information - the names in use, if A, AAAA or PTR have been updated etc.
 (Such an arrangement is likely required by the desire to be able to remove
 DDNS
 information when a lease expires even if the range has already been
 removed.
 We might be able to keep the lease structures around but that seems like
 it
 might be overly complicated.)  In my mind I'm thinking of the DDNS module
 as
 a somewhat separate entity.  It would be informed of all updates to the
 lease
 (requests, renews, rebinds, releases, expires etc) and would update its
 own db
 and the DNS server as necessary.  In this model the flags woudld go in
 that
 other structure not in the lease.  One downside of this is that the lease
 won't
 always have the same name information as the DNS.

 The v6 lease structure includes t1 and t2 is there a reason not to include
 them
 in the v4 structure?  The trend in the current code is to handle them in a
 similar fashion and it also makes serving leasequeries easier.

 Comments on the test code:

 It would probably be good to add a short description to each of
 the tests to indicate what they are trying to test.

 While it wouldn't matter all that much to  the tests it might be
 useful to use something that is closer to the full format
 for an LLT or LL.

-- 
Ticket URL: <http://bind10.isc.org/ticket/2140#comment:5>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development