Bug 49281 - Add UCRV dhcpd/ldap/debug to activate /var/log/dhcp-ldap-startup.log
Add UCRV dhcpd/ldap/debug to activate /var/log/dhcp-ldap-startup.log
Status: CLOSED FIXED
Product: UCS
Classification: Unclassified
Component: DHCP
UCS 4.4
Other Linux
: P5 normal (vote)
: UCS 4.4-3-errata
Assigned To: Max Pohle
Philipp Hahn
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2019-04-11 20:09 CEST by Arvid Requate
Modified: 2020-02-05 17:41 CET (History)
3 users (show)

See Also:
What kind of report is it?: Feature Request
What type of bug is this?: ---
Who will be affected by this bug?: ---
How will those affected feel about the bug?: ---
User Pain:
Enterprise Customer affected?:
School Customer affected?:
ISV affected?:
Waiting Support:
Flags outvoted (downgraded) after PO Review:
Ticket number:
Bug group (optional):
Max CVSS v3 score:
hahn: Patch_Available+


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Arvid Requate univentionstaff 2019-04-11 20:09:15 CEST
In /etc/dhcpd.conf there is a line

 #ldap-debug-file "/var/log/dhcp-ldap-startup.log";

Judging from some tickets, it's "common" knowledge to uncomment that line in case of problems. I would suggest to make this configurable in the usual way, by defining a UCS variable, say dhcpd/ldap/debug, which can simply be set to 'yes' to enable logging to that file.
Comment 1 Jannik Ahlers univentionstaff 2019-04-12 10:21:18 CEST
It would probably be better to name the ucrv something like dhcpd/ldap/debug/enable, to make clear that it has to be set to 'yes' instead of some logging level
Comment 2 Philipp Hahn univentionstaff 2019-08-14 11:19:14 CEST
<git:phahn/43688-dhcp-systemd> (part of Bug #43688)
Comment 3 Max Pohle univentionstaff 2020-01-16 11:45:09 CET
I agree with Jannik, that the name of the variable could also express, for what it is used. But I see no additional value in enforcing a hard coded file name. For that reason my solution introduces dhcpd/ldap/debug/file as variable and if that is missing, we will still see the well known comment line in the config file.
Comment 4 Philipp Hahn univentionstaff 2020-01-24 10:35:32 CET
FAIL: univention-dhcp.yaml
 Please re-phrase the text to something admin understandable like:
  "The new UCR variable `dhcpd/ldap/debug/file` has been added and can be set to a filename, into which the configuration from LDAP is written to simplify debugging the DHCP server."

FYI: Please include this next time:
 Package: univention-dhcp
 Version: 13.0.0-3A~4.4.0.202001161132
 Branch: ucs_4.4-0
 Scope: errata4.4-3

OK: apt install -t apt univention-dhcp

RFC: I see no value in making the name of the file configurable: For UCR variables related to debugging (`ucr search debug`), most of them are '*/debug/**/level' if the expect an "int" or just "*/debug" if they expect a "bool". Using "*/file" would be the only exception which makes the naming inconsistent (IMHO).
Also each UCRV should associated with a descriptive message:

> # grep -A4 dhcpd/ldap/debug/file /etc/univention/registry.info/variables/univention-dhcp.cfg 
> [dhcpd/ldap/debug/file]
> Description[de]=Mit diesen Variablen kann die Erstellung einer Logdatei für Debuginformationen aktiviert werden.
> Description[en]=With this variable the creation of a debug log file can be activated.
> Type=str
> Categories=service-dhcp

Yours I find too terse as it does not tell "howto" enable it exactly ("specify the name of a file to which the information should be logged.")and "what" is logged here exactly ("the configuration for DHCPd as read from LDAP").

See git:559dde0bc7 from my branch in comment 2 for an example.

FAIL: grep -n log /etc/dhcp/dhcpd.conf
> 27:# debug logging is configurable via univention registry: dhcpd/ldap/debug/file

Please call it "Univention Configuration Registry variable" or "UCR variable".
And also call it "LDAP [configuration] debug logging" to make it clears its only about the configuration as read from LDAP.

FAIL: dhcpd -t
> /etc/dhcp/dhcpd.conf line 28: semicolon expected.
Comment 5 Philipp Hahn univentionstaff 2020-01-24 10:39:10 CET
FAIL:     print('ldap-debug-file "%s"\n' % "@%@dhcpd/ldap/debug/file@%@")
There is no need to an explicit "\n" as "print()" adds it implicitly (unless explicitly disabled by "print(..., end='')").

You forgot to import and build the package after git:76489e9d; Don't forget to update the YAML file afterwards.
Comment 6 Max Pohle univentionstaff 2020-01-29 16:13:33 CET
All issued fixed. Please review the change.

version:
version_build: 13.0.0-6A~4.4.0.202001291523

all commits:
git log --pretty=oneline --since=2020-01-01 --grep='Bug #49281'

changed files:
git log --name-only --pretty='' --since=2020-01-01 --grep='Bug #49281' | sort | uniq
Comment 7 Philipp Hahn univentionstaff 2020-01-29 17:12:16 CET
OK: apt install -t apt univention-dhcp # 13.0.0-6A~4.4.0.202001291523
OK: /etc/dhcp/dhcpd.conf
OK: ucr info dhcpd/ldap/debug
OK: ucr set dhcpd/ldap/debug=1
OK: dhcpd -t

OK: errata-announce -V --onyl univention-dhcp.yaml
FIXED: univention-dhcp.yaml → be7e720f58

OK: git diff 4.4-3@{2020-01-01}..4.4-3 -- services/univention-dhcp/ doc/errata/staging/univention-dhcp.yaml
Comment 8 Erik Damrose univentionstaff 2020-02-05 17:41:14 CET
<http://errata.software-univention.de/ucs/4.4/438.html>