Since I'm new here, and was just aiming for a (somewhat) minimal inotify bug-fix, I defer and won't comment.   But I was unaware of the capability and appreciate the pointer.


On Mon, Mar 28, 2022, at 3:32 PM, Jon Murphy wrote:
Sorry for interrupting this train of thought.  Wouldn’t it be easier to use the tools available from dhcpd?

dhcpd seems to have a way execute commands.  And I would guess this would be cleaner than "watching" a file.

There are three execute commands:
on commit
on release
on expiry

and these could be used to launch the needed unbound bridge command.

Here is where I found this:
https://jpmens.net/2011/07/06/execute-a-script-when-isc-dhcp-hands-out-a-new-lease/

And here is additional info:
https://wiki.samba.org/index.php/Configure_DHCP_to_update_DNS_records

(Search for "on commit" on this page)


Jon

On Mar 28, 2022, at 12:00 PM, Michael Tremer <michael.tremer@ipfire.org> wrote:

Hello Anthony,

Thank you very much for submitting this patch and welcome to the list.

I understand that it is easier to track any changes in the directory, but I don’t quite understand why the logic had to be changed that the changes will be applied at the end only.

Is this just to avoid that multiple updates happen one after the other or what are you trying to achieve?

-Michael

On 22 Mar 2022, at 03:47, Anthony Heading <ajrh@ajrh.net> wrote:

Switch from inotify watching individual files to monitoring the
containing directories, as because dhcpd renames its leases file into a
backup, monitoring the single inode does not work well.  Additionally,
python appears to have a bug with replacing expired inotify watches on
single files.
---
unbound-dhcp-leases-bridge | 47 +++++++++++++++++++++++++-------------
1 file changed, 31 insertions(+), 16 deletions(-)

diff --git unbound-dhcp-leases-bridge unbound-dhcp-leases-bridge
index a2df5f1..6e22066 100644
--- unbound-dhcp-leases-bridge
+++ unbound-dhcp-leases-bridge
@@ -72,6 +72,15 @@ class UnboundDHCPLeasesBridge(object):
self.fix_leases_file = fix_leases_file
self.hosts_file = hosts_file

+ # base mask for a completed file change
+ mask = inotify.constants.IN_CLOSE_WRITE | inotify.constants.IN_MOVED_TO
+ # IN_MODIFY since dhcpd appends lease updates to an open file
+ self.watches = {
+    self.leases_file: mask | inotify.constants.IN_MODIFY,
+    self.fix_leases_file: mask,
+    self.hosts_file: mask
+ }
+
self.unbound = UnboundConfigWriter(unbound_leases_file)
self.running = False

@@ -80,36 +89,42 @@ class UnboundDHCPLeasesBridge(object):
self.running = True

# Initial setup
- self.hosts = self.read_static_hosts()
- self.update_dhcp_leases()
+ update_hosts = True
+ update_leases = True
+
+ i = inotify.adapters.Inotify()

- i = inotify.adapters.Inotify([
- self.leases_file,
- self.fix_leases_file,
- self.hosts_file,
- ])
+ for f in self.watches:
+ i.add_watch(os.path.dirname(f), self.watches[f])

for event in i.event_gen():
# End if we are requested to terminate
if not self.running:
break

+ # Make pending updates once inotify queue is empty
if event is None:
+ if update_hosts:
+ self.hosts = self.read_static_hosts()
+ update_hosts = False
+ if update_leases:
+ self.update_dhcp_leases()
+ update_leases = False
continue

header, type_names, watch_path, filename = event

- # Update leases after leases file has been modified
- if "IN_MODIFY" in type_names:
- # Reload hosts
- if watch_path == self.hosts_file:
- self.hosts = self.read_static_hosts()
+ file = os.path.join(watch_path, filename)
+
+ if not file in self.watches:
+ continue
+
+ log.debug("Inotify %s: %s", file, " ".join(type_names))

- self.update_dhcp_leases()
+ update_leases = True

- # If the file is deleted, we re-add the watcher
- if "IN_IGNORED" in type_names:
- i.add_watch(watch_path)
+ if file == self.hosts_file:
+ update_hosts = True

log.info("Unbound DHCP Leases Bridge terminated")

--
2.34.1