[PATCH 1/3] unbound-dhcp-leases-bridge: Implement atomic file replacement

[PATCH 1/3] unbound-dhcp-leases-bridge: Implement atomic file replacement

[Michael Tremer]

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

This change no longer renames the file, but removes the old link and
creates a new link for the temporary file. That helps us to jump out of
the code at any point without worrying about cleaning up the temporary
file.

Signed-off-by: Michael Tremer <michael.tremer(a)ipfire.org>
---
 config/unbound/unbound-dhcp-leases-bridge | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/config/unbound/unbound-dhcp-leases-bridge b/config/unbound/unbound-dhcp-leases-bridge
index e9f022aff..5d5696af0 100644
--- a/config/unbound/unbound-dhcp-leases-bridge
+++ b/config/unbound/unbound-dhcp-leases-bridge
@@ -526,16 +526,22 @@ class UnboundConfigWriter(object):
 	def write_dhcp_leases(self, leases):
 		log.debug("Writing DHCP leases...")
 
-		with tempfile.NamedTemporaryFile(mode="w", delete=False) as f:
+		with tempfile.NamedTemporaryFile(mode="w") as f:
 			for l in leases:
 				for rr in l.rrset:
 					f.write("local-data: \"%s\"\n" % " ".join(rr))
 
+			# Flush the file
+			f.flush()
+
 			# Make file readable for everyone
 			os.fchmod(f.fileno(), stat.S_IRUSR|stat.S_IWUSR|stat.S_IRGRP|stat.S_IROTH)
 
+			# Remove the old file
+			os.unlink(self.path)
+
 			# Move the file to its destination
-			os.rename(f.name, self.path)
+			os.link(f.name, self.path)
 
 	def _control(self, *args):
 		command = ["unbound-control"]
-- 
2.39.2

[PATCH 2/3] unbound-dhcp-leases-bridge: Only reload if leases have actually changed

[Michael Tremer]

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

This patches changes that leases will always be written in
alphanumerical order so that we can later compare the newly generated
file with the previous version. If it has not changed, we skip reload
Unbound.

Suggested-by: Nick Howitt <nick(a)howitts.co.uk>
Signed-off-by: Michael Tremer <michael.tremer(a)ipfire.org>
---
 config/unbound/unbound-dhcp-leases-bridge | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/config/unbound/unbound-dhcp-leases-bridge b/config/unbound/unbound-dhcp-leases-bridge
index 5d5696af0..80c8267e8 100644
--- a/config/unbound/unbound-dhcp-leases-bridge
+++ b/config/unbound/unbound-dhcp-leases-bridge
@@ -22,6 +22,7 @@
 import argparse
 import datetime
 import daemon
+import filecmp
 import functools
 import ipaddress
 import logging
@@ -516,24 +517,29 @@ class UnboundConfigWriter(object):
 
 	def update_dhcp_leases(self, leases):
 		# Write out all leases
-		self.write_dhcp_leases(leases)
+		if self.write_dhcp_leases(leases):
+			log.debug("Reloading Unbound...")
 
-		log.debug("Reloading Unbound...")
-
-		# Reload the configuration without dropping the cache
-		self._control("reload_keep_cache")
+			# Reload the configuration without dropping the cache
+			self._control("reload_keep_cache")
 
 	def write_dhcp_leases(self, leases):
 		log.debug("Writing DHCP leases...")
 
 		with tempfile.NamedTemporaryFile(mode="w") as f:
-			for l in leases:
+			for l in sorted(leases, key=lambda x: x.ipaddr):
 				for rr in l.rrset:
 					f.write("local-data: \"%s\"\n" % " ".join(rr))
 
 			# Flush the file
 			f.flush()
 
+			# Compare if the new leases file has changed from the previous version
+			if filecmp.cmp(f.name, self.path, shallow=False):
+				log.debug("The generated leases file has not changed")
+
+				return False
+
 			# Make file readable for everyone
 			os.fchmod(f.fileno(), stat.S_IRUSR|stat.S_IWUSR|stat.S_IRGRP|stat.S_IROTH)
 
@@ -543,6 +549,8 @@ class UnboundConfigWriter(object):
 			# Move the file to its destination
 			os.link(f.name, self.path)
 
+		return True
+
 	def _control(self, *args):
 		command = ["unbound-control"]
 		command.extend(args)
-- 
2.39.2

[PATCH 3/3] unbound-dhcp-leases-bridge: Make comparison work if old file does not exist

[Michael Tremer]

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

This patch catches any errors if the file did not previously exist and
therefore skips the comparison.

Signed-off-by: Michael Tremer <michael.tremer(a)ipfire.org>
---
 config/unbound/unbound-dhcp-leases-bridge | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/config/unbound/unbound-dhcp-leases-bridge b/config/unbound/unbound-dhcp-leases-bridge
index 80c8267e8..7f89f620a 100644
--- a/config/unbound/unbound-dhcp-leases-bridge
+++ b/config/unbound/unbound-dhcp-leases-bridge
@@ -535,17 +535,22 @@ class UnboundConfigWriter(object):
 			f.flush()
 
 			# Compare if the new leases file has changed from the previous version
-			if filecmp.cmp(f.name, self.path, shallow=False):
-				log.debug("The generated leases file has not changed")
+			try:
+				if filecmp.cmp(f.name, self.path, shallow=False):
+					log.debug("The generated leases file has not changed")
 
-				return False
+					return False
+
+				# Remove the old file
+				os.unlink(self.path)
+
+			# If the previous file did not exist, just keep falling through
+			except FileNotFoundError:
+				pass
 
 			# Make file readable for everyone
 			os.fchmod(f.fileno(), stat.S_IRUSR|stat.S_IWUSR|stat.S_IRGRP|stat.S_IROTH)
 
-			# Remove the old file
-			os.unlink(self.path)
-
 			# Move the file to its destination
 			os.link(f.name, self.path)
 
-- 
2.39.2