Re: [PATCH 1/3] unbound-dhcp-leases-bridge : fix bug 12694 - DHCP hosts not reliably propagated to DNS

[Anthony Heading <ajrh@ajrh.net>]

[-- Attachment #1: Type: text/plain, Size: 4142 bytes --]

Hello Michael,

That was exactly it, just sidestepping an intuitive race with dhcpd writing.  Somewhat hypothetical why that would happen, e.g. clustered updates for multiple NICs or whatever similar non-atomicity,  but it was easy to express that way,  put the update logic in one place (including the initial one at startup), and made the event timestamps sequences a little easier to follow when it wasn't clear why the updates were getting lost at all.  I thought the boolean variables helped document the admittedly simple logic too.   But none of that is critical.

A

On Mon, Mar 28, 2022, at 1:00 PM, Michael Tremer 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(a)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
>>