Opened 10 years ago

Closed 3 years ago

#102 closed enhancement (complete)

reporting versions (from command-line and via command channel)

Reported by: jreed Owned by: tomek
Priority: low Milestone: Kea1.2
Component: dhcp Version: git
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: DHCP Feature Depending on Ticket:
Estimated Difficulty: 0.0 Add Hours to Ticket: 6
Total Hours: 11 Internal?: no

Description (last modified by tomek)

Make sure all executables report versions.

And have bind10 report versions for all its managed components when running modules as daemons or justy to get versions without running serbices.

Subtickets

Change History (36)

comment:1 Changed 10 years ago by jreed

This is the same as #166.

comment:2 Changed 9 years ago by jreed

  • billable set to 0
  • Estimated Difficulty set to 0.0
  • Internal? unset

(from #166)
Every tool should return a consistent version number, both for BIND 10's version, and for the tool itself.

this should be high priority after the June 2010 release.

(backlog item 45)

comment:3 Changed 6 years ago by shane

  • Defect Severity set to N/A
  • Sub-Project set to DNS

Do we still want such version reporting?

If so, is this for the programs installed in bin and sbin, or also for the ones installed in libexec? (Many of the programs installed in libexec can't really be run by themselves... if we want this capability for each of them - such as just saying "don't run by itself" - then that should be a separate ticket.)

We could relatively easily do this for the bin/sbin files.

comment:4 Changed 5 years ago by tomek

  • Milestone set to Remaining BIND10 tickets

comment:5 Changed 5 years ago by tomek

  • Milestone changed from Remaining BIND10 tickets to DHCP Outstanding Tasks
  • Sub-Project changed from DNS to DHCP
  • Version set to git

All Kea components report their versions (see #3508).

It would be useful if:

  • keactrl could report versions of all components.
  • it would be possible to remotely check versions.

comment:6 Changed 5 years ago by tomek

  • Component changed from Unclassified to dhcp

comment:7 Changed 5 years ago by tomek

  • Milestone changed from DHCP Outstanding Tasks to Kea0.9.1

comment:8 Changed 5 years ago by tomek

Once #3513 is done, we can tweak keactrl and be close this one quickly.

comment:9 Changed 5 years ago by hschempf

  • Milestone changed from Kea0.9.1 to Kea0.9.2

comment:10 Changed 5 years ago by hschempf

  • Milestone changed from Kea0.9.2 to DHCP Outstanding Tasks

comment:11 Changed 4 years ago by tomek

  • Milestone changed from DHCP Outstanding Tasks to Outstanding Tasks

Milestone renamed

comment:12 Changed 3 years ago by tomek

  • Description modified (diff)
  • Milestone changed from Outstanding Tasks to Kea1.2
  • Summary changed from reporting versions to reporting versions (from command-line and via command channel)

comment:13 Changed 3 years ago by fdupont

For keactrl it should be added to #3475. For the command channel there are 2 things to do:

  • get a command name
  • encapsulate the answer in JSON (e.g. with isc::config::createAnswer)

If nobody suggests something more urgent (> 1.2 low) or wants this ticket I'll do that.

comment:14 Changed 3 years ago by fdupont

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

comment:15 Changed 3 years ago by fdupont

  • Add Hours to Ticket set to 4
  • Owner changed from fdupont to UnAssigned
  • Status changed from accepted to reviewing
Done. Please review. It requires a ChangeLog entry like "Added a "get-version" command to get over the control channel what command line -vVW arguments display."

comment:16 Changed 3 years ago by fdupont

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

Take it back as now I think there should be 3 different commands.

comment:17 Changed 3 years ago by fdupont

  • Add Hours to Ticket changed from 4 to 5
  • Owner changed from fdupont to UnAssigned
  • Status changed from accepted to reviewing

Split get-version into 3 different commands. The proposed ChangeLog is now "Added get-version, get-extended-version and get-config-report commands to get over the control channel what command line -v, -V and -W arguments display."
Please review.

comment:18 Changed 3 years ago by fdupont

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

Taking it back to add D2 and CA support though src/lib/process.

comment:19 Changed 3 years ago by fdupont

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

I added these new commands and fixed list-commands in lib process so in D2 (where it is tested) and CA.
Ready again for review.

comment:20 Changed 3 years ago by fdupont

Take it back to implement commands in the CA which has a non standard control channel.

comment:21 Changed 3 years ago by fdupont

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

comment:22 Changed 3 years ago by fdupont

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

Done: now the control agent supports the version commands through HTTP.
Ready again for review.

comment:23 Changed 3 years ago by fdupont

The branch to review is trac102a

comment:24 Changed 3 years ago by fdupont

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

Change command names...

comment:25 Changed 3 years ago by fdupont

Done for trac102a, still available for review.

comment:26 Changed 3 years ago by fdupont

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

comment:27 Changed 3 years ago by fdupont

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

Good proposals for command rename/reorg. Taking it to apply last proposals.

comment:28 Changed 3 years ago by fdupont

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

Updated again. The extended part of version-get could be improved...
Ready for review (note that the branch is trac102a).

comment:29 Changed 3 years ago by tomek

  • Owner changed from UnAssigned to tomek

comment:30 follow-up: Changed 3 years ago by tomek

  • Add Hours to Ticket changed from 5 to 6
  • Owner changed from tomek to fdupont
  • Total Hours set to 11

I did review the code on trac102a. The changes related to DHCPv4 and DHCPv6 are good. However, the code updating D2 and CA was too complex. It's not that the changes were incorrect, they simply touched areas of the code that was already broken. Perhaps 'broken' is not the right word here. The problem is that the existing code was written back in BIND10 days. Thomas wrote the code to mimic commands handling in DHCP. Back when the code was written we didn't have CommandMgr? at all. We now have it and it is used in CA.

All commands should be registered in the CommandMgr? and all commands handling should be done there. As such, there's no need to maintain a list of commands (CommandMgr? already takes care of that). Also, the convention in DHCP (which should be followed in CA and D2) is that each command has its own handler.

I did update the code. It split your changes into 3 methods, one for each (version-get, build-report and shutdown) commands. Please review. If you're ok with the changes, we need to confirm with Marcin whether he's ok with the shutdown command being exposed in CA. If he is, the code will be ready for merge.

I pushed my changes to trac102a.

This change definitely needs a changelog. Here's my proposal:

12XX.	[func]		fdupont, tomek
	Additional commands: version-get, build-report have been
	implemented for DHCPv4, DHCPv6 and Control Agent
	components. Control Agent also now supports shutdown command.
	(Trac #102, git tbd)

On a related note, we will be discussing the scope of work for 1.3. I think unifying how DHCP and CA/D2 components are handling configuration and commands should be done in 1.3.

comment:31 in reply to: ↑ 30 Changed 3 years ago by fdupont

Replying to tomek:

I did review the code on trac102a. The changes related to DHCPv4 and DHCPv6 are good. However, the code updating D2 and CA was too complex.

=> I was not sure I used the right way so I asked on Jabber... Thomas answered my proposal (register everything to use the internal executeCommand() and its extensions) made sense. Note I insisted to get this being reviewed first.

I did update the code. It split your changes into 3 methods, one for each (version-get, build-report and shutdown) commands. Please review. If you're ok with the changes, we need to confirm with Marcin whether he's ok with the shutdown command being exposed in CA.

=> IMHO there is no problem to exposed shutdown to CA. BTW the comment saying it is universal is old (for instance it is in master).

I pushed my changes to trac102a.

=> I shall review them.

This change definitely needs a changelog. Here's my proposal:

12XX.	[func]		fdupont, tomek
	Additional commands: version-get, build-report have been
	implemented for DHCPv4, DHCPv6 and Control Agent
	components. Control Agent also now supports shutdown command.
	(Trac #102, git tbd)

comment:32 Changed 3 years ago by fdupont

I removed the command method and did a few cleanup (mainly cosmetic).
I rebased it to trac102b so it should be easier to merge.

comment:33 Changed 3 years ago by tomek

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

I reviewed the changes on trac102b. They look good. The code is now merged to master.
Closing ticket.

comment:34 Changed 3 years ago by tomek

  • Resolution complete deleted
  • Status changed from closed to reopened

comment:35 Changed 3 years ago by tomek

  • Owner changed from fdupont to tomek
  • Status changed from reopened to assigned

comment:36 Changed 3 years ago by tomek

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