[pve-devel] [PATCH ifupdown2 v2 1/1] fix #5197: do not run scripts ending with .dpkg-{old, new, tmp, dist}

[Stefan Hanreich]

This can lead to issue when upgrading from ifupdown to ifupdown2. The
particular issue this fixes occurs in the following scenario:

* Suppose there is a legacy Debian host with ifupdown and ifenslave
  installed that has a bond configured in /etc/network/interfaces.
* ifenslave installs a script /etc/network/if-pre-up.d/ifenslave.
* Now, an upgrade creates a second script
  /etc/network/if-pre-up.d/ifenslave.dpkg-new. As ifupdown executes
  network scripts via run-parts which ignores scripts with . in their
  name, ifenslave.dpkg-new has no effect.
* If the host switches over to ifupdown2 by installing it (removing
  ifupdown, keeping ifenslave) and reboots, the network will not come
  up:
  /etc/network/if-pre-up.d/ifenslave still exists, but is ignored
  by ifupdown2's bond addon [1]
  /etc/network/if-pre-up.d/ifenslave.dpkg-new is executed by ifupdown2
  because it executes all scripts in /etc/network/if-pre-up.d, even if
  their name contains a dot

This leads to ifreload failing on upgrades, which in turn causes
issues with the networking of upgraded hosts.

Also submitted upstream at [2]

[1] https://github.com/CumulusNetworks/ifupdown2/blob/ccdc386cfab70703b657fe7c0ffceb95448a9c2b/ifupdown2/addons/bond.py#L45
[2] https://github.com/CumulusNetworks/ifupdown2/pull/304

Signed-off-by: Stefan Hanreich <s.hanreich at proxmox.com>
Tested-by: Friedrich Weber <f.weber at proxmox.com>
Reviewed-by: Fabian Grünbichler <f.gruenbichler at proxmox.com>
---

Changes from v1 -> v2:
* Improved commit message of patch (thanks @Fabian!)

 ...dpkg-files-when-running-hook-scripts.patch | 55 +++++++++++++++++++
 debian/patches/series                         |  1 +
 2 files changed, 56 insertions(+)
 create mode 100644 debian/patches/pve/0010-main-ignore-dpkg-files-when-running-hook-scripts.patch

diff --git a/debian/patches/pve/0010-main-ignore-dpkg-files-when-running-hook-scripts.patch b/debian/patches/pve/0010-main-ignore-dpkg-files-when-running-hook-scripts.patch
new file mode 100644
index 0000000..4298c76
--- /dev/null
+++ b/debian/patches/pve/0010-main-ignore-dpkg-files-when-running-hook-scripts.patch
@@ -0,0 +1,55 @@
+From 1c4ef3796e18625f8f93aa49f071e759120a72ea Mon Sep 17 00:00:00 2001
+From: Stefan Hanreich <s.hanreich at proxmox.com>
+Date: Tue, 4 Jun 2024 16:17:54 +0200
+Subject: [PATCH] main: ignore dpkg files when running hook scripts
+
+Currently ifupdown2 executes scripts that are backed up by dpkg (e.g.
+foo.dpkg-old). This can lead to issues with hook scripts getting
+executed after upgrading ifupdown2 or packages that ship hook scripts
+(e.g. ifenslave).
+
+This also brings the behavior of ifupdown2 more in line with the
+behavior of ifupdown. ifupdown used run-parts for executing the hook
+scripts, which ignores dpkg-created files (among others).
+
+Signed-off-by: Stefan Hanreich <s.hanreich at proxmox.com>
+---
+ ifupdown2/ifupdown/ifupdownmain.py | 4 +++-
+ ifupdown2/ifupdown/utils.py        | 6 ++++++
+ 2 files changed, 9 insertions(+), 1 deletion(-)
+
+diff --git a/ifupdown2/ifupdown/ifupdownmain.py b/ifupdown2/ifupdown/ifupdownmain.py
+index 51f5460..e6622f0 100644
+--- a/ifupdown2/ifupdown/ifupdownmain.py
++++ b/ifupdown2/ifupdown/ifupdownmain.py
+@@ -1540,7 +1540,9 @@ class ifupdownMain:
+             try:
+                 module_list = os.listdir(msubdir)
+                 for module in module_list:
+-                    if self.modules.get(module) or module in self.overridden_ifupdown_scripts:
++                    if (self.modules.get(module)
++                        or module in self.overridden_ifupdown_scripts
++                        or utils.is_dpkg_file(module)):
+                         continue
+                     self.script_ops[op].append(msubdir + '/' + module)
+             except Exception:
+diff --git a/ifupdown2/ifupdown/utils.py b/ifupdown2/ifupdown/utils.py
+index 05c7e48..3085e82 100644
+--- a/ifupdown2/ifupdown/utils.py
++++ b/ifupdown2/ifupdown/utils.py
+@@ -212,6 +212,12 @@ class utils():
+         # what we have in the cache (data retrieved via a netlink dump by
+         # nlmanager). nlmanager return all macs in lower-case
+ 
++    _dpkg_suffixes = (".dpkg-old", ".dpkg-dist", ".dpkg-new", ".dpkg-tmp")
++
++    @staticmethod
++    def is_dpkg_file(name):
++        return any(name.endswith(suffix) for suffix in utils._dpkg_suffixes)
++
+     @classmethod
+     def importName(cls, modulename, name):
+         """ Import a named object """
+-- 
+2.39.2
+
diff --git a/debian/patches/series b/debian/patches/series
index 557aa7f..d5772c9 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -7,6 +7,7 @@ pve/0006-openvswitch-ovs-ports-condone-regex-exclude-tap-veth.patch
 pve/0007-allow-vlan-tag-inside-vxlan-tunnel.patch
 pve/0008-lacp-bond-remove-bond-min-links-0-warning.patch
 pve/0009-gvgeb-fix-python-interpreter-shebang.patch
+pve/0010-main-ignore-dpkg-files-when-running-hook-scripts.patch
 upstream/0001-add-ipv6-slaac-support-inet6-auto-accept_ra.patch
 upstream/0001-addons-ethtool-add-rx-vlan-filter.patch
 upstream/0001-scheduler-import-traceback.patch
-- 
2.39.2