* [pve-devel] [PATCH ifupdown2] debian: postinst: ensure /etc/iproute2/rt_tables.d exists @ 2025-07-08 16:43 Friedrich Weber 2025-07-08 17:01 ` Thomas Lamprecht 2025-07-08 17:32 ` [pve-devel] " Michael Köppl 0 siblings, 2 replies; 5+ messages in thread From: Friedrich Weber @ 2025-07-08 16:43 UTC (permalink / raw) To: pve-devel When upgrading to Debian Trixie, iproute2 is also upgraded. Its postinst deletes /etc/iproute2/rt_tables.d if it is currently empty (or only contains a README file) [1]. After that, ifreload -a will always print a warning: > error: [Errno 2] No such file or directory: '/etc/iproute2/rt_tables.d/ifupdown2_vrf_map.conf' because it cannot create the file if the parent directory does not exist. To avoid that, create the directory in ifupdown2's postinst if it does not exist yet. Commit b115f7f ("add /etc/iproute2/rt_tables.d/ to managed directories") already added the directory to ifupdown2.dirs, but this does not seem to be effective, due to the following sequence of events on upgrade: 1) iproute2 is unpacked 2) ifupdown2 is unpacked. The directory still exists, so the ifupdown2.dirs entry does not have an effect 3) ifroute2 is set up, and its postinst empties and removes the directory 4) ifupdown2 is set up [1] https://sources.debian.org/src/iproute2/6.15.0-1/debian/iproute2.postinst/ Signed-off-by: Friedrich Weber <f.weber@proxmox.com> --- Notes: Please check the reasoning for plausibility, my understanding of dpkg is very basic, this is just what I could infer from running strace on the upgrade. An alternative solution would be to patch ifupdown2 itself to create the directory (if needed) before trying to write to the ifupdown2_vrf_map.conf file. debian/ifupdown2.postinst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/debian/ifupdown2.postinst b/debian/ifupdown2.postinst index f9a719c..13cc987 100644 --- a/debian/ifupdown2.postinst +++ b/debian/ifupdown2.postinst @@ -111,6 +111,9 @@ case "$1" in process_udev chmod +x /usr/share/ifupdown2/__main__.py postinst_remove_diverts + if [ ! -d /etc/iproute2/rt_tables.d ]; then + mkdir /etc/iproute2/rt_tables.d || echo "could not create /etc/iproute2/rt_tables.d" + fi if [ -z "$2" ] && [ ! -e /proxmox_install_mode ]; then proxmox_compatibility echo "Reloading network config on first install" -- 2.47.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] 5+ messages in thread
* Re: [pve-devel] [PATCH ifupdown2] debian: postinst: ensure /etc/iproute2/rt_tables.d exists 2025-07-08 16:43 [pve-devel] [PATCH ifupdown2] debian: postinst: ensure /etc/iproute2/rt_tables.d exists Friedrich Weber @ 2025-07-08 17:01 ` Thomas Lamprecht 2025-07-09 6:40 ` Friedrich Weber 2025-07-08 17:32 ` [pve-devel] " Michael Köppl 1 sibling, 1 reply; 5+ messages in thread From: Thomas Lamprecht @ 2025-07-08 17:01 UTC (permalink / raw) To: Proxmox VE development discussion, Friedrich Weber Am 08.07.25 um 18:43 schrieb Friedrich Weber: > When upgrading to Debian Trixie, iproute2 is also upgraded. Its > postinst deletes /etc/iproute2/rt_tables.d if it is currently empty > (or only contains a README file) [1]. After that, ifreload -a will > always print a warning: > >> error: [Errno 2] No such file or directory: '/etc/iproute2/rt_tables.d/ifupdown2_vrf_map.conf' > > because it cannot create the file if the parent directory does not > exist. > > To avoid that, create the directory in ifupdown2's postinst if it > does not exist yet. > > Commit b115f7f ("add /etc/iproute2/rt_tables.d/ to managed > directories") already added the directory to ifupdown2.dirs, but this > does not seem to be effective, due to the following sequence of events > on upgrade: > > 1) iproute2 is unpacked > 2) ifupdown2 is unpacked. The directory still exists, so the > ifupdown2.dirs entry does not have an effect > 3) ifroute2 is set up, and its postinst empties and removes the s/if/ip/ (can be fixed up on applying). > directory > 4) ifupdown2 is set up > > [1] https://sources.debian.org/src/iproute2/6.15.0-1/debian/iproute2.postinst/ > > Signed-off-by: Friedrich Weber <f.weber@proxmox.com> > --- > > Notes: > Please check the reasoning for plausibility, my understanding of dpkg > is very basic, this is just what I could infer from running strace on > the upgrade. Seems plausible as unpacking happens before configuring the pacakges. > > An alternative solution would be to patch ifupdown2 itself to create > the directory (if needed) before trying to write to the > ifupdown2_vrf_map.conf file. That might be slightly more robust and should be IMO what upstream should do already, that said, it's a very specific case and not having to touch the python code has its advantages, so the patch here would be fine for me in general. > > debian/ifupdown2.postinst | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/debian/ifupdown2.postinst b/debian/ifupdown2.postinst > index f9a719c..13cc987 100644 > --- a/debian/ifupdown2.postinst > +++ b/debian/ifupdown2.postinst > @@ -111,6 +111,9 @@ case "$1" in > process_udev > chmod +x /usr/share/ifupdown2/__main__.py > postinst_remove_diverts > + if [ ! -d /etc/iproute2/rt_tables.d ]; then > + mkdir /etc/iproute2/rt_tables.d || echo "could not create /etc/iproute2/rt_tables.d" > + fi > if [ -z "$2" ] && [ ! -e /proxmox_install_mode ]; then > proxmox_compatibility > echo "Reloading network config on first install" _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [pve-devel] [PATCH ifupdown2] debian: postinst: ensure /etc/iproute2/rt_tables.d exists 2025-07-08 17:01 ` Thomas Lamprecht @ 2025-07-09 6:40 ` Friedrich Weber 2025-07-09 8:01 ` [pve-devel] applied: " Thomas Lamprecht 0 siblings, 1 reply; 5+ messages in thread From: Friedrich Weber @ 2025-07-09 6:40 UTC (permalink / raw) To: Thomas Lamprecht, Proxmox VE development discussion On 08/07/2025 19:01, Thomas Lamprecht wrote: > Am 08.07.25 um 18:43 schrieb Friedrich Weber: >> When upgrading to Debian Trixie, iproute2 is also upgraded. Its >> postinst deletes /etc/iproute2/rt_tables.d if it is currently empty >> (or only contains a README file) [1]. After that, ifreload -a will >> always print a warning: >> >>> error: [Errno 2] No such file or directory: '/etc/iproute2/rt_tables.d/ifupdown2_vrf_map.conf' >> >> because it cannot create the file if the parent directory does not >> exist. >> >> To avoid that, create the directory in ifupdown2's postinst if it >> does not exist yet. >> >> Commit b115f7f ("add /etc/iproute2/rt_tables.d/ to managed >> directories") already added the directory to ifupdown2.dirs, but this >> does not seem to be effective, due to the following sequence of events >> on upgrade: >> >> 1) iproute2 is unpacked >> 2) ifupdown2 is unpacked. The directory still exists, so the >> ifupdown2.dirs entry does not have an effect >> 3) ifroute2 is set up, and its postinst empties and removes the > > s/if/ip/ (can be fixed up on applying). Thanks for catching that. If nothing else comes up, I won't send a v2 though. > >> directory >> 4) ifupdown2 is set up >> >> [1] https://sources.debian.org/src/iproute2/6.15.0-1/debian/iproute2.postinst/ >> >> Signed-off-by: Friedrich Weber <f.weber@proxmox.com> >> --- >> >> Notes: >> Please check the reasoning for plausibility, my understanding of dpkg >> is very basic, this is just what I could infer from running strace on >> the upgrade. > > Seems plausible as unpacking happens before configuring the pacakges. > >> >> An alternative solution would be to patch ifupdown2 itself to create >> the directory (if needed) before trying to write to the >> ifupdown2_vrf_map.conf file. > > That might be slightly more robust and should be IMO what upstream should > do already, that said, it's a very specific case and not having to touch > the python code has its advantages, so the patch here would be fine for > me in general. Yeah, I started working on a Python patch first, but as this file is opened at several different places [1], I realized testing such a patch thoroughly (to hit all call sites) would be more difficult for me, and it really is more of an edge case, so the postinst patch also seemed OK to me. I can send this postinst patch upstream too to raise the issue, then we can see whether they'd rather go with a Python patch. [1] https://github.com/CumulusNetworks/ifupdown2/blob/master/ifupdown2/addons/vrf.py > >> >> debian/ifupdown2.postinst | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/debian/ifupdown2.postinst b/debian/ifupdown2.postinst >> index f9a719c..13cc987 100644 >> --- a/debian/ifupdown2.postinst >> +++ b/debian/ifupdown2.postinst >> @@ -111,6 +111,9 @@ case "$1" in >> process_udev >> chmod +x /usr/share/ifupdown2/__main__.py >> postinst_remove_diverts >> + if [ ! -d /etc/iproute2/rt_tables.d ]; then >> + mkdir /etc/iproute2/rt_tables.d || echo "could not create /etc/iproute2/rt_tables.d" >> + fi >> if [ -z "$2" ] && [ ! -e /proxmox_install_mode ]; then >> proxmox_compatibility >> echo "Reloading network config on first install" > _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 5+ messages in thread
* [pve-devel] applied: [PATCH ifupdown2] debian: postinst: ensure /etc/iproute2/rt_tables.d exists 2025-07-09 6:40 ` Friedrich Weber @ 2025-07-09 8:01 ` Thomas Lamprecht 0 siblings, 0 replies; 5+ messages in thread From: Thomas Lamprecht @ 2025-07-09 8:01 UTC (permalink / raw) To: Proxmox VE development discussion, Friedrich Weber Am 09.07.25 um 08:40 schrieb Friedrich Weber: > On 08/07/2025 19:01, Thomas Lamprecht wrote: >> Am 08.07.25 um 18:43 schrieb Friedrich Weber: >>> When upgrading to Debian Trixie, iproute2 is also upgraded. Its >>> postinst deletes /etc/iproute2/rt_tables.d if it is currently empty >>> (or only contains a README file) [1]. After that, ifreload -a will >>> always print a warning: >>> >>>> error: [Errno 2] No such file or directory: '/etc/iproute2/rt_tables.d/ifupdown2_vrf_map.conf' >>> >>> because it cannot create the file if the parent directory does not >>> exist. >>> >>> To avoid that, create the directory in ifupdown2's postinst if it >>> does not exist yet. >>> >>> Commit b115f7f ("add /etc/iproute2/rt_tables.d/ to managed >>> directories") already added the directory to ifupdown2.dirs, but this >>> does not seem to be effective, due to the following sequence of events >>> on upgrade: >>> >>> 1) iproute2 is unpacked >>> 2) ifupdown2 is unpacked. The directory still exists, so the >>> ifupdown2.dirs entry does not have an effect >>> 3) ifroute2 is set up, and its postinst empties and removes the >> >> s/if/ip/ (can be fixed up on applying). > > Thanks for catching that. If nothing else comes up, I won't send a v2 > though. applied it and obviously forgot to fix this up, meh, if I at least wouldn't have noticed it earlier I could better save face ;-) > Yeah, I started working on a Python patch first, but as this file is > opened at several different places [1], I realized testing such a patch > thoroughly (to hit all call sites) would be more difficult for me, and > it really is more of an edge case, so the postinst patch also seemed OK > to me. I can send this postinst patch upstream too to raise the issue, > then we can see whether they'd rather go with a Python patch. > > [1] > https://github.com/CumulusNetworks/ifupdown2/blob/master/ifupdown2/addons/vrf.py Yes, this would be a bigger change, if upstream would be more active I'd favor it, but this way I do not think it will help us. Btw., after missing the fixup for the typo today I question also this approach again, after all configure scripts have no strict ordering on their own – but Debian policy got us covered here [0], as ifupdown2 declares a dependency on iproute2, so the following policy section applies: "The Depends field should also be used if the postinst or prerm scripts require the depended-on package to be unpacked or configured in order to run. In the case of postinst configure, the depended-on packages will be unpacked and configured first" So your solution here is sound: applied, thanks! [0]: https://www.debian.org/doc/debian-policy/ch-relationships.html#binary-dependencies-depends-recommends-suggests-enhances-pre-depends _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [pve-devel] [PATCH ifupdown2] debian: postinst: ensure /etc/iproute2/rt_tables.d exists 2025-07-08 16:43 [pve-devel] [PATCH ifupdown2] debian: postinst: ensure /etc/iproute2/rt_tables.d exists Friedrich Weber 2025-07-08 17:01 ` Thomas Lamprecht @ 2025-07-08 17:32 ` Michael Köppl 1 sibling, 0 replies; 5+ messages in thread From: Michael Köppl @ 2025-07-08 17:32 UTC (permalink / raw) To: Proxmox VE development discussion, Friedrich Weber Since I was just not encountering this problem (again), gave this patch a quick spin on my Trixie dev VM. Was able to `ifreload -a` without warnings again and apply networking changes through the UI without encountering errors in the task log. As before, any changes are applied successfully, just without the warnings. Tested-by: Michael Köppl <m.koeppl@proxmox.com> On 7/8/25 18:43, Friedrich Weber wrote: > When upgrading to Debian Trixie, iproute2 is also upgraded. Its > postinst deletes /etc/iproute2/rt_tables.d if it is currently empty > (or only contains a README file) [1]. After that, ifreload -a will > always print a warning: > >> error: [Errno 2] No such file or directory: '/etc/iproute2/rt_tables.d/ifupdown2_vrf_map.conf' > > because it cannot create the file if the parent directory does not > exist. > > To avoid that, create the directory in ifupdown2's postinst if it > does not exist yet. > > Commit b115f7f ("add /etc/iproute2/rt_tables.d/ to managed > directories") already added the directory to ifupdown2.dirs, but this > does not seem to be effective, due to the following sequence of events > on upgrade: > > 1) iproute2 is unpacked > 2) ifupdown2 is unpacked. The directory still exists, so the > ifupdown2.dirs entry does not have an effect > 3) ifroute2 is set up, and its postinst empties and removes the > directory > 4) ifupdown2 is set up > > [1] https://sources.debian.org/src/iproute2/6.15.0-1/debian/iproute2.postinst/ > > Signed-off-by: Friedrich Weber <f.weber@proxmox.com> > --- > > Notes: > Please check the reasoning for plausibility, my understanding of dpkg > is very basic, this is just what I could infer from running strace on > the upgrade. > > An alternative solution would be to patch ifupdown2 itself to create > the directory (if needed) before trying to write to the > ifupdown2_vrf_map.conf file. > > debian/ifupdown2.postinst | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/debian/ifupdown2.postinst b/debian/ifupdown2.postinst > index f9a719c..13cc987 100644 > --- a/debian/ifupdown2.postinst > +++ b/debian/ifupdown2.postinst > @@ -111,6 +111,9 @@ case "$1" in > process_udev > chmod +x /usr/share/ifupdown2/__main__.py > postinst_remove_diverts > + if [ ! -d /etc/iproute2/rt_tables.d ]; then > + mkdir /etc/iproute2/rt_tables.d || echo "could not create /etc/iproute2/rt_tables.d" > + fi > if [ -z "$2" ] && [ ! -e /proxmox_install_mode ]; then > proxmox_compatibility > echo "Reloading network config on first install" _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-07-09 8:01 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2025-07-08 16:43 [pve-devel] [PATCH ifupdown2] debian: postinst: ensure /etc/iproute2/rt_tables.d exists Friedrich Weber 2025-07-08 17:01 ` Thomas Lamprecht 2025-07-09 6:40 ` Friedrich Weber 2025-07-09 8:01 ` [pve-devel] applied: " Thomas Lamprecht 2025-07-08 17:32 ` [pve-devel] " Michael Köppl
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox