BIND 10 #3151: PD: Configuration parsers much be able to parse PD configuration

[BIND 10 Development]

#3151: PD: Configuration parsers much be able to parse PD configuration
-------------------------------------+-------------------------------------
            Reporter:  tomek         |                        Owner:  tmark
                Type:  enhancement   |                       Status:
            Priority:  medium        |  reviewing
           Component:  dhcpconf      |                    Milestone:
            Keywords:                |  Sprint-DHCP-20130918
           Sensitive:  0             |                   Resolution:
         Sub-Project:  DHCP          |                 CVSS Scoring:
Estimated Difficulty:  0             |              Defect Severity:  N/A
         Total Hours:  0             |  Feature Depending on Ticket:
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by tomek):

 * owner:  tomek => tmark


Comment:

 Replying to [comment:6 tmark]:
 > It would be useful, but why start a trend?  I added the class comment
 (thought I had done that...) which includes an example.
 It's almost good, but I think the initial triple curly braces are not
 necessary.

 > > So what happens if there is address-only configuration? The parser is
 not created at all? It is created, but an empty list passed?
 > If a subnet has no "pd_pools" entry, this parser will not be created. If
 it is created but passed an empty list, it will simply produce no pools.
 Ok.

 > > '''src/bin/dhcp6/tests/config_parser_unittest.cc'''
 > >
 > > Dhcp6ParserTest.pdPoolBasics: the test should confirm that the last
 address in a pool is is ok (in words checking if 'prefix-len' was
 processed correctly). You can calculate it manually or use
 lastAddrInPrefix(prefix, prefix_len).
 > Done.
 Thanks.

 > > It is likely that the second test is already there, so just a comment
 added that address-only test doubles as a test that verifies that a prefix
 can remain not-configured.
 > >
 >
 > The first and fourth tests I rolled into one new test,
 "subnetAndPrefixDelegated".  I think this tests what you are after.
 >
 > The second test does exist, in several places, so I added a comment
 above TEST_F(Dhcp7ParserTest, poolPrefixLen).
 >
 > The third test you asked for is verified by pdPoolBasics, so I added a
 comment for that as well.
 >
 > Lastly, I added an NA pool to the pdPoolList test, which demonstrates
 that a subnet may be a combination of pool types.
 >
 > If you have specific tests that are not covered by these, how about
 sending me the config snippets and I can add tests for them.
 No, the tests you developed are very thorough and they cover the feature
 adequately.

 > > '''src/lib/dhcpsrv/tests/test_libraries.h'''
 > > You modified its content and replaced /home/thomson/... with
 > > /Users/tmark/... I'm not sure if I like that change :)
 > > In fact, why they test_libraries.h file is in the repo at all?
 >
 > It should not be there at all.  I pulled a fresh copy of trac3151 and it
 isn't there, nor will git let me "rm" it.

 thomson at billabong:~/devel/bind10$ git status
 # On branch trac3151
 # Changes not staged for commit:
 #   (use "git add <file>..." to update what will be committed)
 #   (use "git checkout -- <file>..." to discard changes in working
 directory)
 #
 #       modified:   src/lib/dhcpsrv/tests/test_libraries.h
 {{{
 git blame src/lib/dhcpsrv/tests/test_libraries.h
 }}}
 shows that it was added in commit a26a75c9 (by me) and later updated in
 commit f67708b1 (by you).

 I don't understand why it is on my trac3151 branch, but not on yours.
 That's odd.

 > I added a new section, 18.2.4 for this.  It is bare bones and marked
 with a todo.
 That was exactly what I was looking for. Short enough to not waste too
 much time and informative enough, so Włodek can get his tests rolling.

 This addresses all my concerns. Please try to find out how it is possible
 that there are test_libraries.h on my branch, but not on yours. But don't
 spend too much time on it. We can live with that file in repo for now.

 If you agree to remove the curly braces , then the ticket is ready to go.
 I don't need to see it again. Please merge.

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