#5100 closed task (complete)

Decouple CommandMgr's unix sockets from commands handling

Reported by: marcin Owned by: marcin
Priority: medium Milestone: Kea1.2
Component: remote-management 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

The Control API design calls for restructuring CommandMgr class to decouple unix sockets functionality from commands handling. See http://kea.isc.org/wiki/ControlAPIDesign#RestructuringCommandMgr for the details.

Subtickets

Change History (7)

comment:1 Changed 11 months ago by marcin

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

comment:2 Changed 11 months ago by marcin

  • Owner changed from marcin to UnAssigned
  • Status changed from accepted to assigned

I split the CommandMgr to three different classes: BaseCommandMgr, HookedCommandMgr and CommandMgr. The first one is meant to be used (derived from) in the hooks libraries to implement commands handling within hooks libraries. The latter adds ability to call callouts and must be used within Kea modules (servers and CA) as a base class to support hooks in command channel. The CommandMgr is a derivation of HookedCommandMgr and it extends it with support for unix domain socket. The CA will also derive from HookedCommandMgr but since it doesn't receive commands over unix domain sockets but HTTP it will be a separate derived class.

Initially I planned to have two hooks implemented within HookedCommandMgr, one for 'list-commands' command and one for all other commands. Eventually, I decided to go with a single hook point which handles all commands which appears to be less complex solution.

Proposed ChangeLog entry:

12xx.   [func]         marcin
	Added 'control_command_receive' hook point to Kea Command
	Manager.
        (Trac #5100, git abcd)

comment:3 Changed 11 months ago by marcin

  • Status changed from assigned to reviewing

comment:4 follow-up: Changed 11 months ago by fdupont

  • Owner changed from UnAssigned to marcin

Fixed spelling. Removed ignored params.

In the hooked code, COMMAND_HOOK_RECEIVE_SKIP is logged in the else branch? IMHO it must be before returning the response and the (now empty) else branch removed.

The design document says the response is an ElementPtr, the code uses a ConstElementPtr.: at least one is wrong.

BTW do you have an idea how we can hook the "set-config" command?

comment:5 in reply to: ↑ 4 Changed 11 months ago by marcin

  • Owner changed from marcin to fdupont

Replying to fdupont:

Fixed spelling. Removed ignored params.

Thanks.

In the hooked code, COMMAND_HOOK_RECEIVE_SKIP is logged in the else branch? IMHO it must be before returning the response and the (now empty) else branch removed.

You're right. I don't know what I was thinking! :-)

The design document says the response is an ElementPtr, the code uses a ConstElementPtr.: at least one is wrong.

The design will be updated.

BTW do you have an idea how we can hook the "set-config" command?

This is an interesting question. I created the test and slightly modified the code to support this case. The simplest case seems to be having the callout which retrieves the argument of the command, alters the arguments and returns without setting the SKIP command. The Command Manager takes the command with modified set of arguments and runs native 'set-config' implementation. The new test demonstrates how this can be done. Though, it doesn't run 'set-config', but rather the test callout.

comment:6 Changed 11 months ago by fdupont

  • Owner changed from fdupont to marcin

Looks OK. BTW Jenkins got a warning because of #5090 bug so I added a branch for it and pushed the ticket on the review queue.

comment:7 Changed 11 months ago by marcin

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

Merged with commit d0c7cb29a7df3588c540afb4ca56de55f26142e0

Note: See TracTickets for help on using tickets.