public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH ifupdown2 v2 1/1] fix #5197: do not run scripts ending with .dpkg-{old, new, tmp, dist}
@ 2024-07-03 12:56 Stefan Hanreich
  2024-07-04  9:31 ` [pve-devel] applied: " Thomas Lamprecht
  0 siblings, 1 reply; 2+ messages in thread
From: Stefan Hanreich @ 2024-07-03 12:56 UTC (permalink / raw)
  To: pve-devel

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@proxmox.com>
Tested-by: Friedrich Weber <f.weber@proxmox.com>
Reviewed-by: Fabian Grünbichler <f.gruenbichler@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@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@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


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

^ permalink raw reply	[flat|nested] 2+ messages in thread

* [pve-devel] applied: [PATCH ifupdown2 v2 1/1] fix #5197: do not run scripts ending with .dpkg-{old, new, tmp, dist}
  2024-07-03 12:56 [pve-devel] [PATCH ifupdown2 v2 1/1] fix #5197: do not run scripts ending with .dpkg-{old, new, tmp, dist} Stefan Hanreich
@ 2024-07-04  9:31 ` Thomas Lamprecht
  0 siblings, 0 replies; 2+ messages in thread
From: Thomas Lamprecht @ 2024-07-04  9:31 UTC (permalink / raw)
  To: Proxmox VE development discussion, Stefan Hanreich

Am 03/07/2024 um 14:56 schrieb 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@proxmox.com>
> Tested-by: Friedrich Weber <f.weber@proxmox.com>
> Reviewed-by: Fabian Grünbichler <f.gruenbichler@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
> 
>

applied, thanks!


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2024-07-04  9:32 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-07-03 12:56 [pve-devel] [PATCH ifupdown2 v2 1/1] fix #5197: do not run scripts ending with .dpkg-{old, new, tmp, dist} Stefan Hanreich
2024-07-04  9:31 ` [pve-devel] applied: " Thomas Lamprecht

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal