Opened 7 years ago

Closed 7 years ago

#2954 closed enhancement (complete)

DHCP-DDNS: Create BIND 10 process skeleton

Reported by: stephen Owned by: tmark
Priority: medium Milestone: Sprint-DHCP-20130523
Component: dhcp Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: DHCP Feature Depending on Ticket:
Estimated Difficulty: 0 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description

Create the basic process framework for the DHCP DDNS module. This work includes creating the directory holding the files, and modifying configure.ac and necessary Makefile.am's to include it in the build process.

Subtickets

Change History (11)

comment:1 Changed 7 years ago by tmark

  • Owner set to tmark
  • Status changed from new to assigned

comment:2 Changed 7 years ago by tmark

  • Owner changed from tmark to UnAssigned
  • Status changed from assigned to reviewing

This ticket represents the first step in developing the D2Server. The following
directory structure and files have been created:

bind10/src/bin/d2srv

Makefile.am
b10-d2srv.xml
d2srv.spec
d2srv_log.cc
d2srv_log.h
d2srv_messages.mes
main.cc
spec_config.h.pre.in

tests/

Makefile.am
d2srv_test.py
d2srv_unittests.cc

Supporting changes to build, test, and install the D2 Server were made to:

bind10/configure.ac
bind10/src/bin/Makefile.am

These changes will yield a b10-d2srv executable module, loadable by bind10.
It will run, sleep for 1000 seconds and exit. It does not establish a msgq
session.

I would recommend the following ChangeLog entry:

6xx. [func] tmark

DHCP-DDNS: Initial D2 Server implemented. Currently it does
nothing useful, except for providing the skeleton implementation
to be expanded in the future.
(Trac #2954, git TBD)

comment:3 Changed 7 years ago by marcin

  • Owner changed from UnAssigned to marcin

comment:4 follow-up: Changed 7 years ago by marcin

  • Owner changed from marcin to tmark

Reviewed commit e448bbba3ecae68b261612954aa9777edc384be4

I have made this comment some time ago via email. I believe that in D2Srv, the ''Srv'' is redundant or even wrong. Please note that none of the modules in BIND10 are called like this. For example, b10-dhcp4 is indeed a server process but it is called b10-dhcp4. Perhaps, simply using d2 would be sufficient. If we are not concerned about the length of the name of this module, we could even try ''dhcpddns''. Even in this stub implementation, there is a lot of ''Srv'' in the code already. Programmers are lazy by their nature, so writing ''Srv'' everywhere might be a pain for some people :-).

b10-d2srv.xml
Copyright date should be corrected in file header.

Errors in the following tags:

  • refentryinfo - invalid date
  • refnamediv - refpurpose is invalid - copied from DHCP module
  • docinfo - invalid date
  • cmdsynopsis - the (-v) option is not supported right now, so there is no reason to document it.
  • refsynopsisdiv - same as above with -v option

d2srv_log.cc
Invalid copyright date.

Comment before ''include'' does not match the module. We define the logger for ''d2'', not b10-dhcp4.

d2srv_log.h
Copyright date is invalid.
Comment before ''extern'' is copy-pasted from DHCP4 and thus invalid in this context.

d2srv_messages.mes
Invalid copyright date.

D2SRV_STARTING & D2SRV_START_INFO: Shouldn't it be ''D2Controller'', instead of ''DController''?

D2SRV_START_INFO: Do we really plan to have the ability to run the server in a standalone mode? If we do, how do we get the configuration?
BTW, if we decide that we need it (for example to run unit tests), it should be documented in b10-d2srv.xml?

main.cc
Invalid copyright date.

usage(): what is the [-p number] parameter? It is not described anywhere, including the error message printed by the usage(). In fact, any attempt to specify this parameter would result in error and usage printing.

Shouldn't verbose mode be only available when standalone is turned on? We control the verbosity by other means, i.e. using bindctl if running a module using bind10.

main(): Why define the ''ret'' variable? You could do:

return (EXIT_SUCCESS);

Makefile.am
Do we really have to link with all these libraries already? I would be in favor to link against the minimal number of libraries and extend the dependencies as we go. The libb10-dhcp++.la is already linked with the number of libs, so I don't know whether explicitly adding these is really needed.

spec_config.h.pre
Copyright date is invalid.

The location of the spec file is also invalid.

tests/Makefile.am
The same comment regarding linking with many libraries as above.

tests/d2srv_unittests.cc
Copyright date in the header is invalid.

tests/d2srv_test.py
Copyright date is invalid.

Do we actually have a ticket to implement ''test_alive''? Currently the comment says it is just a placeholder.

ChangeLog
If we wanted to follow the convention from the ChangeLog we would rather say:

6xx. [func] tmark
    b10-d2srv: Initial DHCP-DDNS (a.k.a. D2) module implemented. Currently it does
    nothing useful, except for providing the skeleton implementation 
    to be expanded in the future.
    (Trac #2954, git TBD)

comment:5 in reply to: ↑ 4 ; follow-up: Changed 7 years ago by tmark

  • Owner changed from tmark to marcin

Replying to marcin:

Reviewed commit e448bbba3ecae68b261612954aa9777edc384be4

I have made this comment some time ago via email. I believe that in D2Srv, the ''Srv'' is redundant or even wrong. Please note that none of the modules in BIND10 are called like this. For example, b10-dhcp4 is indeed a server process but it is called b10-dhcp4. Perhaps, simply using d2 would be sufficient. If we are not concerned about the length of the name of this module, we could even try ''dhcpddns''. Even in this stub implementation, there is a lot of ''Srv'' in the code already. Programmers are lazy by their nature, so writing ''Srv'' everywhere might be a pain for some people :-).

After discussing the issue with the the Dhcp team consensus to remove, "Srv" and "srv" throughout. Attempting to work in dhcp-ddns or permutations thereof is cumbersome at best. The module will be "b10-d2" and files renamed as follows:

bind10/src/bin/d2

Makefile.am
b10-d2.xml
d2.spec
d2_log.cc
d2_log.h
d2_messages.mes
main.cc
spec_config.h.pre.in

tests/

Makefile.am
d2_test.py
d2_unittests.cc

b10-d2srv.xml
Copyright date should be corrected in file header.

Errors in the following tags:

  • refentryinfo - invalid date
  • refnamediv - refpurpose is invalid - copied from DHCP module
  • docinfo - invalid date
  • cmdsynopsis - the (-v) option is not supported right now, so there is no reason to document it.
  • refsynopsisdiv - same as above with -v option

Corrected.

d2srv_log.cc
Invalid copyright date.

Comment before ''include'' does not match the module. We define the logger for ''d2'', not b10-dhcp4.

d2srv_log.h
Copyright date is invalid.
Comment before ''extern'' is copy-pasted from DHCP4 and thus invalid in this context.

d2srv_messages.mes
Invalid copyright date.

Corrected.

D2SRV_STARTING & D2SRV_START_INFO: Shouldn't it be ''D2Controller'', instead of ''DController''?

Corrected.

D2SRV_START_INFO: Do we really plan to have the ability to run the server in a standalone mode? If we do, how do we get the configuration?
BTW, if we decide that we need it (for example to run unit tests), it should be documented in b10-d2srv.xml?

I based this on dhcp4/6. Since they allegedly support "verbose" and "stand-alone" flags, I presumed D2 should as well. It was my understanding that we wanted D2 to be capbale of running without bind10. I have been working under the assumption that the arguments claimed in dhcp4 and dhcp6 function as advertised. Do they not?

I added a comment stating that the parameters are preliminary.

main.cc
Invalid copyright date.

Corrected.

usage(): what is the [-p number] parameter? It is not described anywhere, including the error message printed by the usage(). In fact, any attempt to specify this parameter would result in error and usage printing.

Removed reference to -p.

Shouldn't verbose mode be only available when standalone is turned on? We control the verbosity by other means, i.e. using bindctl if running a module using bind10.

See my comment above regarding standalone mode.

main(): Why define the ''ret'' variable? You could do:

return (EXIT_SUCCESS);

This was residual from the dchp4 version main.cc, which was more involved
than this main is at the moment. I have cleaned it up a bit.

Makefile.am
Do we really have to link with all these libraries already? I would be in favor to link against the minimal number of libraries and extend the dependencies as we go. The libb10-dhcp++.la is already linked with the number of libs, so I don't know whether explicitly adding these is really needed.

Again, residual

spec_config.h.pre
Copyright date is invalid.

The location of the spec file is also invalid.

tests/Makefile.am
The same comment regarding linking with many libraries as above.

tests/d2srv_unittests.cc
Copyright date in the header is invalid.

tests/d2srv_test.py
Copyright date is invalid.

Do we actually have a ticket to implement ''test_alive''? Currently the comment says it is just a placeholder.

I made the test functional.

ChangeLog
If we wanted to follow the convention from the ChangeLog we would rather say:

6xx. [func] tmark
    b10-d2srv: Initial DHCP-DDNS (a.k.a. D2) module implemented. Currently it does
    nothing useful, except for providing the skeleton implementation 
    to be expanded in the future.
    (Trac #2954, git TBD)

This wording is fine.

comment:6 follow-up: Changed 7 years ago by jreed

Why not call the module b10-dhcp-ddns?

comment:7 in reply to: ↑ 6 Changed 7 years ago by tmark

Replying to jreed:

Why not call the module b10-dhcp-ddns?

We were certain there would be many opinions as to the module name, "b10-dhcp-ddns" is certainly a possibility. At least during initial work, b10-d2 is much easier to type and I suppose marketing would likely have an opinion. D2 comes from D for DHCP and D for DDNS, one could go a step further and call it D3 for "Dhcp-Driven Ddns".
In terms of source files, class names etc... dhcp ddns is cumbersome to type as well as to s
say repeatedly. The dhcp team has been using the moniker D2 for discussions and emails tts just much easier to work.

For the time being, we'll leave it a b10-d2. As we get closer to delivery we can decide what to release it under.

comment:8 in reply to: ↑ 5 ; follow-up: Changed 7 years ago by marcin

  • Owner changed from marcin to tmark

Replying to tmark:

Replying to marcin:

Reviewed commit e448bbba3ecae68b261612954aa9777edc384be4

Reviewed commit b3dda76df66adecf3af2ba419a23b4169bfe9541

The doxygen documentation is not generated for D2 module. You should update ''bind10/doc/Doxygen'' file with the path to D2 and verify that there are no errors generating D2 documentation:

cd doc
make devel
less html/doxygen-error.log

I have made this comment some time ago via email. I believe that in D2Srv, the ''Srv'' is redundant or even wrong. Please note that none of the modules in BIND10 are called like this. For example, b10-dhcp4 is indeed a server process but it is called b10-dhcp4. Perhaps, simply using d2 would be sufficient. If we are not concerned about the length of the name of this module, we could even try ''dhcpddns''. Even in this stub implementation, there is a lot of ''Srv'' in the code already. Programmers are lazy by their nature, so writing ''Srv'' everywhere might be a pain for some people :-).

After discussing the issue with the the Dhcp team consensus to remove, "Srv" and "srv" throughout. Attempting to work in dhcp-ddns or permutations thereof is cumbersome at best. The module will be "b10-d2" and files renamed as follows:

bind10/src/bin/d2

Makefile.am
b10-d2.xml
d2.spec
d2_log.cc
d2_log.h
d2_messages.mes
main.cc
spec_config.h.pre.in

tests/

Makefile.am
d2_test.py
d2_unittests.cc

b10-d2srv.xml
Copyright date should be corrected in file header.

Errors in the following tags:

  • refentryinfo - invalid date
  • refnamediv - refpurpose is invalid - copied from DHCP module
  • docinfo - invalid date
  • cmdsynopsis - the (-v) option is not supported right now, so there is no reason to document it.
  • refsynopsisdiv - same as above with -v option

Corrected.

Under ''cmdsynopsis'' the ''-s'' command line option is not documented.

d2srv_log.cc
Invalid copyright date.

Comment before ''include'' does not match the module. We define the logger for ''d2'', not b10-dhcp4.

This hasn't been corrected yet.

d2srv_log.h
Copyright date is invalid.
Comment before ''extern'' is copy-pasted from DHCP4 and thus invalid in this context.

This hasn't been corrected.

d2srv_messages.mes
Invalid copyright date.

Corrected.

There are spurious whitespaces in the file at the end of

It lists some information about the parameters with which the 

and

This is a debug message issued when a D2 process shuts down 


D2SRV_STARTING & D2SRV_START_INFO: Shouldn't it be ''D2Controller'', instead of ''DController''?

Corrected.

D2SRV_START_INFO: Do we really plan to have the ability to run the server in a standalone mode? If we do, how do we get the configuration?
BTW, if we decide that we need it (for example to run unit tests), it should be documented in b10-d2srv.xml?

I based this on dhcp4/6. Since they allegedly support "verbose" and "stand-alone" flags, I presumed D2 should as well. It was my understanding that we wanted D2 to be capbale of running without bind10. I have been working under the assumption that the arguments claimed in dhcp4 and dhcp6 function as advertised. Do they not?

On reflection, I believe running the process without BIND10 infrastructure may be useful for testing. However, running it without the infrastructure implies that you need to provide its configuration by other means and we currently don't have any idea how that should be. The DHCP modules used to be run in standalone mode when we didn't have any configuration capabilities through BIND10. In this case, the server was sending out the same fixed lease to everybody, but that was an early stage of development.

If we are planning to run the module standalone to test it, then that's fine to keep the standalone capability.

I added a comment stating that the parameters are preliminary.

That's cool! The only sugggestion is that any of ''@todo'', ''TODO'' or ''todo'' is added to this comment as a keyword. That way, you will be easily able to grep the code for all outstanding issues that we decided to change or fix at some point. We follow that in the DHCP modules.

main.cc
Invalid copyright date.

Corrected.

usage(): what is the [-p number] parameter? It is not described anywhere, including the error message printed by the usage(). In fact, any attempt to specify this parameter would result in error and usage printing.

bind10/doc

Removed reference to -p.

Shouldn't verbose mode be only available when standalone is turned on? We control the verbosity by other means, i.e. using bindctl if running a module using bind10.

See my comment above regarding standalone mode.

main(): Why define the ''ret'' variable? You could do:

return (EXIT_SUCCESS);

This was residual from the dchp4 version main.cc, which was more involved
than this main is at the moment. I have cleaned it up a bit.

Ok. Thanks.
There are many spurious whitespaces in the code (comments).

Makefile.am
Do we really have to link with all these libraries already? I would be in favor to link against the minimal number of libraries and extend the dependencies as we go. The libb10-dhcp++.la is already linked with the number of libs, so I don't know whether explicitly adding these is really needed.

Again, residual

It looks like the following is required:

b10_d2_LDADD  = $(top_builddir)/src/lib/exceptions/libb10-exceptions.la

otherwise, linker fails.

spec_config.h.pre
Copyright date is invalid.

The location of the spec file is also invalid.

tests/Makefile.am
The same comment regarding linking with many libraries as above.

Can we remove some of the libraries, we are linking the test with?

tests/d2srv_unittests.cc
Copyright date in the header is invalid.

tests/d2srv_test.py
Copyright date is invalid.

Do we actually have a ticket to implement ''test_alive''? Currently the comment says it is just a placeholder.

I made the test functional.

Great!

ChangeLog
If we wanted to follow the convention from the ChangeLog we would rather say:

6xx. [func] tmark
    b10-d2srv: Initial DHCP-DDNS (a.k.a. D2) module implemented. Currently it does
    nothing useful, except for providing the skeleton implementation 
    to be expanded in the future.
    (Trac #2954, git TBD)

This wording is fine.

comment:9 in reply to: ↑ 8 Changed 7 years ago by tmark

  • Owner changed from tmark to marcin

Replying to marcin:

Replying to tmark:

Replying to marcin:

Reviewed commit e448bbba3ecae68b261612954aa9777edc384be4

Reviewed commit b3dda76df66adecf3af2ba419a23b4169bfe9541

The doxygen documentation is not generated for D2 module. You should update ''bind10/doc/Doxygen'' file with the path to D2 and verify that there are no errors generating D2 documentation:

cd doc
make devel
less html/doxygen-error.log

Done. Did not know to do this.

I have made this comment some time ago via email. I believe that in D2Srv, the ''Srv'' is redundant or even wrong. Please note that none of the modules in BIND10 are called like this. For example, b10-dhcp4 is indeed a server process but it is called b10-dhcp4. Perhaps, simply using d2 would be sufficient. If we are not concerned about the length of the name of this module, we could even try ''dhcpddns''. Even in this stub implementation, there is a lot of ''Srv'' in the code already. Programmers are lazy by their nature, so writing ''Srv'' everywhere might be a pain for some people :-).

After discussing the issue with the the Dhcp team consensus to remove, "Srv" and "srv" throughout. Attempting to work in dhcp-ddns or permutations thereof is cumbersome at best. The module will be "b10-d2" and files renamed as follows:

bind10/src/bin/d2

Makefile.am
b10-d2.xml
d2.spec
d2_log.cc
d2_log.h
d2_messages.mes
main.cc
spec_config.h.pre.in

tests/

Makefile.am
d2_test.py
d2_unittests.cc

b10-d2srv.xml
Copyright date should be corrected in file header.

Errors in the following tags:

  • refentryinfo - invalid date
  • refnamediv - refpurpose is invalid - copied from DHCP module
  • docinfo - invalid date
  • cmdsynopsis - the (-v) option is not supported right now, so there is no reason to document it.
  • refsynopsisdiv - same as above with -v option

Corrected.

Under ''cmdsynopsis'' the ''-s'' command line option is not documented.

Added.

d2srv_log.cc
Invalid copyright date.

Comment before ''include'' does not match the module. We define the logger for ''d2'', not b10-dhcp4.

This hasn't been corrected yet.

Done.

d2srv_log.h
Copyright date is invalid.
Comment before ''extern'' is copy-pasted from DHCP4 and thus invalid in this context.

This hasn't been corrected.

Done

d2srv_messages.mes
Invalid copyright date.

Corrected.

There are spurious whitespaces in the file at the end of

It lists some information about the parameters with which the 

and

This is a debug message issued when a D2 process shuts down 

Done


D2SRV_STARTING & D2SRV_START_INFO: Shouldn't it be ''D2Controller'', instead of ''DController''?

Corrected.

D2SRV_START_INFO: Do we really plan to have the ability to run the server in a standalone mode? If we do, how do we get the configuration?
BTW, if we decide that we need it (for example to run unit tests), it should be documented in b10-d2srv.xml?

I based this on dhcp4/6. Since they allegedly support "verbose" and "stand-alone" flags, I presumed D2 should as well. It was my understanding that we wanted D2 to be capbale of running without bind10. I have been working under the assumption that the arguments claimed in dhcp4 and dhcp6 function as advertised. Do they not?

On reflection, I believe running the process without BIND10 infrastructure may be useful for testing. However, running it without the infrastructure implies that you need to provide its configuration by other means and we currently don't have any idea how that should be. The DHCP modules used to be run in standalone mode when we didn't have any configuration capabilities through BIND10. In this case, the server was sending out the same fixed lease to everybody, but that was an early stage of development.

If we are planning to run the module standalone to test it, then that's fine to keep the standalone capability.

I believe that it will be infinitely useful to run stand-alone, certainly
during development. I'll consider the configuration issue while implementing the
controller and server classes.

I added a comment stating that the parameters are preliminary.

That's cool! The only sugggestion is that any of ''@todo'', ''TODO'' or ''todo'' is added to this comment as a keyword. That way, you will be easily able to grep the code for all outstanding issues that we decided to change or fix at some point. We follow that in the DHCP modules.

Added @TODO

main.cc
Invalid copyright date.

Corrected.

usage(): what is the [-p number] parameter? It is not described anywhere, including the error message printed by the usage(). In fact, any attempt to specify this parameter would result in error and usage printing.

bind10/doc

Removed reference to -p.

Shouldn't verbose mode be only available when standalone is turned on? We control the verbosity by other means, i.e. using bindctl if running a module using bind10.

Altered the call to initLogger to set log level to DEBUG only if both
verbose_mode and stand_alone are true.

See my comment above regarding standalone mode.

main(): Why define the ''ret'' variable? You could do:

return (EXIT_SUCCESS);

This was residual from the dchp4 version main.cc, which was more involved
than this main is at the moment. I have cleaned it up a bit.

Ok. Thanks.
There are many spurious whitespaces in the code (comments).

Seriously? Ok, so I added an autocmd to my .vimrc to automagically strip off
trailing whitespace from .cc and .h files.

Makefile.am
Do we really have to link with all these libraries already? I would be in favor to link against the minimal number of libraries and extend the dependencies as we go. The libb10-dhcp++.la is already linked with the number of libs, so I don't know whether explicitly adding these is really needed.

Again, residual

It looks like the following is required:

b10_d2_LDADD  = $(top_builddir)/src/lib/exceptions/libb10-exceptions.la

otherwise, linker fails.

It linked for me under OS-X, but I added the library anyway.

spec_config.h.pre
Copyright date is invalid.

The location of the spec file is also invalid.

tests/Makefile.am
The same comment regarding linking with many libraries as above.

Can we remove some of the libraries, we are linking the test with?

Done.


tests/d2srv_unittests.cc
Copyright date in the header is invalid.

tests/d2srv_test.py
Copyright date is invalid.

Do we actually have a ticket to implement ''test_alive''? Currently the comment says it is just a placeholder.

I made the test functional.

Great!

ChangeLog
If we wanted to follow the convention from the ChangeLog we would rather say:

6xx. [func] tmark
    b10-d2srv: Initial DHCP-DDNS (a.k.a. D2) module implemented. Currently it does
    nothing useful, except for providing the skeleton implementation 
    to be expanded in the future.
    (Trac #2954, git TBD)

This wording is fine.

comment:10 Changed 7 years ago by marcin

  • Owner changed from marcin to tmark

Reviewed 0dcc4f4feab96a9a76d33137adeadc488884b0ec

I run the build on the following systems (to make sure that there are no linking issues):

  • Free, Net, Open BSDs
  • Fedora 18
  • Ubuntu 11.10
  • Debian6
  • Centos 5.3

and the build was successful.

I haven't tested this on Solaris.

Changes are ok and can be merged.

comment:11 Changed 7 years ago by tmark

  • Resolution set to complete
  • Status changed from reviewing to closed

Changes merged, ChangeLog? updated.

Note: See TracTickets for help on using tickets.