Opened 3 years ago

Closed 2 years ago

#3732 closed enhancement (complete)

remove --with-kea-config=BUNDY

Reported by: wlodekwencel Owned by: fdupont
Priority: low Milestone: Kea0.9.2-beta
Component: Unclassified Version: git
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

make[6]: Entering directory `/home/wlodek/dev/kea/src/bin/dhcp4'
  CXX      ctrl_dhcp4_srv.lo
  CXX      json_config_parser.lo
  CXX      bundy_controller.lo
  CXX      dhcp4_srv.lo
  CXX      main.o
bundy_controller.cc:27:40: fatal error: dhcpsrv/dhcp_config_parser.h: No such file or directory
 #include <dhcpsrv/dhcp_config_parser.h>
                                        ^
compilation terminated.
make[6]: *** [bundy_controller.lo] Error 1
make[6]: *** Waiting for unfinished jobs....

Subtickets

Change History (31)

comment:1 Changed 3 years ago by marcin

For me, this ticket should have a summary: "Remove the --with-kea-config=BUNDY". We don't seem to maintain this configuration backend and I am not sure anybody is really using it. The further we go with the development of Kea the more issues we will have like this. Unless we start building Kea with this backend every time we make the change in the server area. But, even if we do, how do we test it in practice, given that we have removed the BIND10 infrastructure?

comment:2 Changed 3 years ago by wlodekwencel

I'm totally agreed with Marcin. We can go for "remove --with-kea-config" :)

comment:3 Changed 3 years ago by hschempf

  • Milestone changed from Kea-proposed to Kea0.9.1

comment:4 Changed 3 years ago by hschempf

  • Milestone changed from Kea0.9.1 to Kea0.9.2

comment:5 Changed 3 years ago by fdupont

  • Summary changed from compilation error for --with-kea-config=BUNDY to remove --with-kea-config=BUNDY

comment:6 Changed 3 years ago by fdupont

Changed the ticket title to follow the rough consensus!

comment:7 Changed 3 years ago by fdupont

  • Owner set to fdupont
  • Status changed from new to accepted

comment:8 Changed 3 years ago by fdupont

Done. Note there are still some code for the control channel.
Ready for review. The ChangeLog should be "Removed --with-kea-config=BUNDY leaving JSON as the only supported way to configure Kea".

comment:9 Changed 3 years ago by fdupont

  • Owner changed from fdupont to UnAssigned
  • Status changed from accepted to reviewing

comment:10 Changed 2 years ago by stephen

  • Owner changed from UnAssigned to stephen

comment:11 Changed 2 years ago by stephen

  • Owner changed from stephen to fdupont

Reviewed commit a5b5c2b77884f7f2d70ff2c2368b39b124dcd474

doc/devel/config-backend.dox
Suggest replacing second paragraph in "Motivation for Different Backends" section with:

While ISC currently (May 2015) maintains only one configuration backend
(a JSON file read from disk), it is quite possible that additional backends
(e.g. using LDAP or XML) will be developed in the future by ISC or other
organizations.

doc/guide/config.xml
Section "Kea configuration". Suggest replacing the first paragraph with:

Kea is designed to allow different methods by which it can be configured,
each method being implemented by a component known as a configuration backend.
At present, only one such backend is available, that allowing configuration
by means of a JSON file.

src/bin/d2/d2.dox
Suggest removing the bullet point:

Support for integrated operation

That was only relevant when talking about integration in the Bundy environment.

Suggest removing the paragraph:

There were two implementations of D2Controller, today the only available
form is JSON.

That there were two implementations is history and is not of interest in a document that describes the software as it is now.

src/bin/dhcp4/dhcp4.dox
I suggest just removing the section dhcpv4Session completely.

src/bin/dhcp6/dhcp6.dox
I suggest just removing the section dhcpv6Session completely.

src/lib/cc/session.cc
Session::establish - The socket file appears not to be used yet (and may go away). However, I suggest that the (environment) variable be named KEA_MSGQ_SOCKET_FILE (rather than MSGQ_SOCKET_FILE) to keep it in the "Kea namespace".

src/lib/cc/session_config.h.pre.in
See comment for src/lib/cc/session.cc

comment:12 Changed 2 years ago by fdupont

Applied all comment:11 items. Does it need a re-review?

comment:13 Changed 2 years ago by fdupont

  • Owner changed from fdupont to stephen

comment:14 Changed 2 years ago by stephen

  • Owner changed from stephen to fdupont

Reviewed commit 6df2818a569c7a797b685a2c879049cf86f0e2ae

All your changes were OK. However, I've modified three files to remove Doxygen references to deleted sections. Please pull and check them. If all is OK, the branch can be merged.

This change is user-visible, so does require a ChangeLog entry. Something brief like:

Remove BUNDY configuration backend

... should be sufficient.

comment:15 Changed 2 years ago by fdupont

Done but the merge highly interfered with lib/{cc,config} cleanup so IMHO there is another pass to perform.
I'll create a trac3732a branch for extra cleanups and keep this ticket opened until the situation becomes clearer.

comment:16 Changed 2 years ago by fdupont

More to review and merge in trac3732a. Perhaps it will need a new ticket and port of changes? BTW I'd like to get comments about the idea to merge cc and config libraries...

comment:17 Changed 2 years ago by fdupont

  • Owner changed from fdupont to UnAssigned

comment:18 Changed 2 years ago by stephen

  • Owner changed from UnAssigned to stephen

comment:19 follow-up: Changed 2 years ago by stephen

  • Owner changed from stephen to fdupont

Reviewed commit c5c9bd839abda7d225fdad2762de18889217c24a

All OK, please merge. Don't forget the ChangeLog entry.

BTW I'd like to get comments about the idea to merge cc and config libraries...

I not sure it's worth the effort at the moment. Once Kea 1.0 has been released, we'll have time to look at the command and configuration interface. Its complexity has been raised in project meetings and there is a desire to simplify it when we have time. When we do, it is likely the cc and config libraries will be merged at that point.

comment:20 in reply to: ↑ 19 Changed 2 years ago by fdupont

Replying to stephen:

BTW I'd like to get comments about the idea to merge cc and config libraries...

I not sure it's worth the effort at the moment. Once Kea 1.0 has been released, we'll have time to look at the command and configuration interface. Its complexity has been raised in project meetings and there is a desire to simplify it when we have time. When we do, it is likely the cc and config libraries will be merged at that point.

=> the idea was for after 0.9.2 so it seems we agree...

comment:21 Changed 2 years ago by fdupont

Merged the trac3732a branch. Creating a ticket for cc/config library merge and closing this one.

comment:22 Changed 2 years ago by fdupont

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

comment:23 Changed 2 years ago by fdupont

The trac3732a branch collided with #3880 so another round is required:

  • removed cc stuff
  • removed pure config stuff.

comment:24 Changed 2 years ago by fdupont

  • Resolution complete deleted
  • Status changed from closed to reopened

comment:25 Changed 2 years ago by fdupont

  • Priority changed from very low to low
  • Status changed from reopened to accepted
  • Type changed from defect to enhancement

comment:26 Changed 2 years ago by fdupont

branch trac3732b ready for review (I've just checked whether it is compatible for #3880 use of src/lib/config/config_log.*).

comment:27 Changed 2 years ago by fdupont

  • Owner changed from fdupont to UnAssigned
  • Status changed from accepted to reviewing

comment:28 Changed 2 years ago by stephen

  • Owner changed from UnAssigned to stephen

comment:29 Changed 2 years ago by stephen

  • Owner changed from stephen to fdupont

Reviewed 49ebe7de96059844dba790c26da148d1a5c2992a

Looks OK, please merge.

comment:30 Changed 2 years ago by fdupont

Done. closing again.

comment:31 Changed 2 years ago by fdupont

  • Resolution set to complete
  • Status changed from reviewing to closed
Note: See TracTickets for help on using tickets.