public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [RFC qemu-server] apt hook: warn against using 'upgrade' command
@ 2024-09-06 10:40 Fiona Ebner
  2024-09-06 10:42 ` Fiona Ebner
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Fiona Ebner @ 2024-09-06 10:40 UTC (permalink / raw)
  To: pve-devel

Many people will use 'upgrade' instead of 'full-upgrade' or
'dist-upgrade' (e.g. [0][1]) despite the documentation explicitly
mentioning 'dist-upgrade' [3]. Proxmox VE uses different packaging
guarantees than Debian and using 'upgrade' can lead to a broken
system [2].

The match is kept simple, to not accidentally catch things like
> -o 'foo=bar upgrade baz'
and trip up advanced users.

It does not catch invocations with '-y' either, making it less likely
to break automated user scripts. Although they should not use
'upgrade' either, it still would be bad to break them. If the risk is
still considered too high, this change should wait until a major or
at least point release.

To avoid false positives, it would be necessary to properly parse
options, which is likely not worth the effort.

A downside is that the hook is only invoked after the user confirms
the upgrade, but there doesn't seem to be an early enough hook entry
(DPkg::Pre-Invoke is also too late). Since this is just an additional
safety warning to guide new users, it should still be good enough.

[0]: https://forum.proxmox.com/threads/150217/post-680158
[1]: https://forum.proxmox.com/threads/140580/post-630419
[2]: https://www.reddit.com/r/Proxmox/comments/ujqig9/use_apt_distupgrade_or_the_gui_not_apt_upgrade/
[3]: https://pve.proxmox.com/pve-docs/chapter-sysadmin.html#system_software_updates

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 debian/apthook/pve-apt-hook | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/debian/apthook/pve-apt-hook b/debian/apthook/pve-apt-hook
index 47629bc..b41d0fe 100755
--- a/debian/apthook/pve-apt-hook
+++ b/debian/apthook/pve-apt-hook
@@ -55,6 +55,14 @@ my $blank;
 while (my $line = <$fh>) {
   chomp $line;
 
+  if ($line && $line =~ m/^CommandLine::AsString=apt(-get)?%20upgrade$/) {
+    $log->("!! WARNING !!\n");
+    $log->("Using 'upgrade' can violate packaging assumptions made by Proxmox VE. Use 'dist-upgrade' instead.\n");
+    $log->("\n");
+    $log->("Press enter to continue, or C^c to abort.\n");
+    $cleanup->(0, 1);
+  }
+
   if (!defined($blank)) {
     $blank = 1 if !$line;
     next;
-- 
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] 10+ messages in thread

* Re: [pve-devel] [RFC qemu-server] apt hook: warn against using 'upgrade' command
  2024-09-06 10:40 [pve-devel] [RFC qemu-server] apt hook: warn against using 'upgrade' command Fiona Ebner
@ 2024-09-06 10:42 ` Fiona Ebner
  2024-09-06 11:01 ` Dominik Csapak
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 10+ messages in thread
From: Fiona Ebner @ 2024-09-06 10:42 UTC (permalink / raw)
  To: pve-devel

Sorry, messed up the subject-prefix, since I copy-pasted it in the
.git/config and forgot to change. This is for proxmox-ve of course.


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


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

* Re: [pve-devel] [RFC qemu-server] apt hook: warn against using 'upgrade' command
  2024-09-06 10:40 [pve-devel] [RFC qemu-server] apt hook: warn against using 'upgrade' command Fiona Ebner
  2024-09-06 10:42 ` Fiona Ebner
@ 2024-09-06 11:01 ` Dominik Csapak
  2024-09-06 16:54   ` Thomas Lamprecht
  2024-09-06 12:16 ` Alexander Zeidler
  2024-09-06 16:58 ` Thomas Lamprecht
  3 siblings, 1 reply; 10+ messages in thread
From: Dominik Csapak @ 2024-09-06 11:01 UTC (permalink / raw)
  To: Proxmox VE development discussion, Fiona Ebner

just to mention it (i think it's not widely known):

'apt upgrade' behaves slightly different than 'apt-get upgrade'

while the former also installs new packages if necessary, the latter
doesn't, so an 'apt upgrade' will maybe work ok in most sitations.
I guess we want to filter it out nonetheless?

(none of the commands above will remove packages)


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


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

* Re: [pve-devel] [RFC qemu-server] apt hook: warn against using 'upgrade' command
  2024-09-06 10:40 [pve-devel] [RFC qemu-server] apt hook: warn against using 'upgrade' command Fiona Ebner
  2024-09-06 10:42 ` Fiona Ebner
  2024-09-06 11:01 ` Dominik Csapak
@ 2024-09-06 12:16 ` Alexander Zeidler
  2024-09-06 16:56   ` Thomas Lamprecht
  2024-09-06 16:58 ` Thomas Lamprecht
  3 siblings, 1 reply; 10+ messages in thread
From: Alexander Zeidler @ 2024-09-06 12:16 UTC (permalink / raw)
  To: Proxmox VE development discussion

On Fri Sep 6, 2024 at 12:40 PM CEST, Fiona Ebner wrote:
> (DPkg::Pre-Invoke is also too late). Since this is just an additional
> safety warning to guide new users, it should still be good enough.

> +  if ($line && $line =~ m/^CommandLine::AsString=apt(-get)?%20upgrade$/) {
> +    $log->("!! WARNING !!\n");
> +    $log->("Using 'upgrade' can violate packaging assumptions made by Proxmox VE. Use 'dist-upgrade' instead.\n");
> +    $log->("\n");

As "an additional safety warning to guide new users", we may want to
rephrase this line:
> +    $log->("Press enter to continue, or C^c to abort.\n");
to something like:
"Press CTRL+C to abort, or ENTER to continue anyway."


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


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

* Re: [pve-devel] [RFC qemu-server] apt hook: warn against using 'upgrade' command
  2024-09-06 11:01 ` Dominik Csapak
@ 2024-09-06 16:54   ` Thomas Lamprecht
  0 siblings, 0 replies; 10+ messages in thread
From: Thomas Lamprecht @ 2024-09-06 16:54 UTC (permalink / raw)
  To: Proxmox VE development discussion, Dominik Csapak, Fiona Ebner

Am 06/09/2024 um 13:01 schrieb Dominik Csapak:
> just to mention it (i think it's not widely known):
> 
> 'apt upgrade' behaves slightly different than 'apt-get upgrade'
> 
> while the former also installs new packages if necessary, the latter
> doesn't, so an 'apt upgrade' will maybe work ok in most sitations.
> I guess we want to filter it out nonetheless?
> 
> (none of the commands above will remove packages)

that's all true, but I agree also with your guess, we rely on removals
to happen as we have semi-frequent transitions during a stable release
cycle, so while `apt upgrade` is thankfully way more sensible already,
it still isn't enough for our use case.



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


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

* Re: [pve-devel] [RFC qemu-server] apt hook: warn against using 'upgrade' command
  2024-09-06 12:16 ` Alexander Zeidler
@ 2024-09-06 16:56   ` Thomas Lamprecht
  0 siblings, 0 replies; 10+ messages in thread
From: Thomas Lamprecht @ 2024-09-06 16:56 UTC (permalink / raw)
  To: Proxmox VE development discussion, Alexander Zeidler

Am 06/09/2024 um 14:16 schrieb Alexander Zeidler:
> On Fri Sep 6, 2024 at 12:40 PM CEST, Fiona Ebner wrote:
>> (DPkg::Pre-Invoke is also too late). Since this is just an additional
>> safety warning to guide new users, it should still be good enough.
> 
>> +  if ($line && $line =~ m/^CommandLine::AsString=apt(-get)?%20upgrade$/) {
>> +    $log->("!! WARNING !!\n");
>> +    $log->("Using 'upgrade' can violate packaging assumptions made by Proxmox VE. Use 'dist-upgrade' instead.\n");
>> +    $log->("\n");
> 
> As "an additional safety warning to guide new users", we may want to
> rephrase this line:
>> +    $log->("Press enter to continue, or C^c to abort.\n");
> to something like:
> "Press CTRL+C to abort, or ENTER to continue anyway."

FWIW, if we do this then I probably would keep enter spelled lower case,
simply to avoid it sticking so much out to user to trigger reflexes and
pressing enter without fully reading the message.


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


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

* Re: [pve-devel] [RFC qemu-server] apt hook: warn against using 'upgrade' command
  2024-09-06 10:40 [pve-devel] [RFC qemu-server] apt hook: warn against using 'upgrade' command Fiona Ebner
                   ` (2 preceding siblings ...)
  2024-09-06 12:16 ` Alexander Zeidler
@ 2024-09-06 16:58 ` Thomas Lamprecht
  2024-09-09  7:48   ` Fabian Grünbichler
  3 siblings, 1 reply; 10+ messages in thread
From: Thomas Lamprecht @ 2024-09-06 16:58 UTC (permalink / raw)
  To: Proxmox VE development discussion, Fiona Ebner

Am 06/09/2024 um 12:40 schrieb Fiona Ebner:
> Many people will use 'upgrade' instead of 'full-upgrade' or
> 'dist-upgrade' (e.g. [0][1]) despite the documentation explicitly
> mentioning 'dist-upgrade' [3]. Proxmox VE uses different packaging
> guarantees than Debian and using 'upgrade' can lead to a broken
> system [2].
> 
> The match is kept simple, to not accidentally catch things like
>> -o 'foo=bar upgrade baz'
> and trip up advanced users.
> 
> It does not catch invocations with '-y' either, making it less likely
> to break automated user scripts. Although they should not use
> 'upgrade' either, it still would be bad to break them. If the risk is
> still considered too high, this change should wait until a major or
> at least point release.
> 
> To avoid false positives, it would be necessary to properly parse
> options, which is likely not worth the effort.
> 
> A downside is that the hook is only invoked after the user confirms
> the upgrade, but there doesn't seem to be an early enough hook entry
> (DPkg::Pre-Invoke is also too late). Since this is just an additional
> safety warning to guide new users, it should still be good enough.
> 
> [0]: https://forum.proxmox.com/threads/150217/post-680158
> [1]: https://forum.proxmox.com/threads/140580/post-630419
> [2]: https://www.reddit.com/r/Proxmox/comments/ujqig9/use_apt_distupgrade_or_the_gui_not_apt_upgrade/
> [3]: https://pve.proxmox.com/pve-docs/chapter-sysadmin.html#system_software_updates
> 

yeah, it's something I considered here and then but never pulled through,
as it just somehow doesn't feel right...

But it's definitively a real problem, and so I surely won't block this on
the basis of some gut feeling, I'd rather like to hear Fabian's opinion on
it.


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


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

* Re: [pve-devel] [RFC qemu-server] apt hook: warn against using 'upgrade' command
  2024-09-06 16:58 ` Thomas Lamprecht
@ 2024-09-09  7:48   ` Fabian Grünbichler
  2024-09-09  7:52     ` Fiona Ebner
  0 siblings, 1 reply; 10+ messages in thread
From: Fabian Grünbichler @ 2024-09-09  7:48 UTC (permalink / raw)
  To: Thomas Lamprecht, Proxmox VE development discussion, Fiona Ebner

> Thomas Lamprecht <t.lamprecht@proxmox.com> hat am 06.09.2024 18:58 CEST geschrieben:  
> Am 06/09/2024 um 12:40 schrieb Fiona Ebner:
> > Many people will use 'upgrade' instead of 'full-upgrade' or
> > 'dist-upgrade' (e.g. [0][1]) despite the documentation explicitly
> > mentioning 'dist-upgrade' [3]. Proxmox VE uses different packaging
> > guarantees than Debian and using 'upgrade' can lead to a broken
> > system [2].

just a slight nit here: you should only end up with a broken system if we miss properly tracking some inter-package relationship. it can happen happen (and probably does, from time to time), but in the vast majority of cases "apt[-get] upgrade" should at most leave you stuck with an outdated system (with APT telling you that there are still packages to be upgraded), not a broken one. we did get a lot better about accounting for these things over the past few years (but of course, we don't have anywhere close to the infrastructure that Debian has for automated tracking and testing).

> > The match is kept simple, to not accidentally catch things like
> >> -o 'foo=bar upgrade baz'
> > and trip up advanced users.
> > 
> > It does not catch invocations with '-y' either, making it less likely
> > to break automated user scripts. Although they should not use
> > 'upgrade' either, it still would be bad to break them. If the risk is
> > still considered too high, this change should wait until a major or
> > at least point release.
> > 
> > To avoid false positives, it would be necessary to properly parse
> > options, which is likely not worth the effort.
> > 
> > A downside is that the hook is only invoked after the user confirms
> > the upgrade, but there doesn't seem to be an early enough hook entry
> > (DPkg::Pre-Invoke is also too late). Since this is just an additional
> > safety warning to guide new users, it should still be good enough.
> > 
> > [0]: https://forum.proxmox.com/threads/150217/post-680158
> > [1]: https://forum.proxmox.com/threads/140580/post-630419
> > [2]: https://www.reddit.com/r/Proxmox/comments/ujqig9/use_apt_distupgrade_or_the_gui_not_apt_upgrade/
> > [3]: https://pve.proxmox.com/pve-docs/chapter-sysadmin.html#system_software_updates
> > 
> 
> yeah, it's something I considered here and then but never pulled through,
> as it just somehow doesn't feel right...
> 
> But it's definitively a real problem, and so I surely won't block this on
> the basis of some gut feeling, I'd rather like to hear Fabian's opinion on
> it.

given that I also use `apt upgrade` from time to time (habit from being an unstable user ;)), and that it might alienate power users coming from Debian, I'd prefer this to be a non-interactive warning with the text "disarmed" a bit?

something like

!! WARNING !!
Since Proxmox VE follows a rolling release model, using 'upgrade' can lead to a system being stuck on outdated versions, or in rare cases, break upon upgrading. Use 'dist-upgrade' or 'full-upgrade' instead.
!! WARNING !!

with or without a prompt (it's a pity that the hook is not executed with the config before the regular confirmation prompt, else we could just depend on that)?


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


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

* Re: [pve-devel] [RFC qemu-server] apt hook: warn against using 'upgrade' command
  2024-09-09  7:48   ` Fabian Grünbichler
@ 2024-09-09  7:52     ` Fiona Ebner
  2024-09-09  7:57       ` Thomas Lamprecht
  0 siblings, 1 reply; 10+ messages in thread
From: Fiona Ebner @ 2024-09-09  7:52 UTC (permalink / raw)
  To: Fabian Grünbichler, Thomas Lamprecht,
	Proxmox VE development discussion

Am 09.09.24 um 09:48 schrieb Fabian Grünbichler:
>> Thomas Lamprecht <t.lamprecht@proxmox.com> hat am 06.09.2024 18:58 CEST geschrieben:  
>> Am 06/09/2024 um 12:40 schrieb Fiona Ebner:
>>> Many people will use 'upgrade' instead of 'full-upgrade' or
>>> 'dist-upgrade' (e.g. [0][1]) despite the documentation explicitly
>>> mentioning 'dist-upgrade' [3]. Proxmox VE uses different packaging
>>> guarantees than Debian and using 'upgrade' can lead to a broken
>>> system [2].
> 
> just a slight nit here: you should only end up with a broken system if we miss properly tracking some inter-package relationship. it can happen happen (and probably does, from time to time), but in the vast majority of cases "apt[-get] upgrade" should at most leave you stuck with an outdated system (with APT telling you that there are still packages to be upgraded), not a broken one. we did get a lot better about accounting for these things over the past few years (but of course, we don't have anywhere close to the infrastructure that Debian has for automated tracking and testing).

Okay, will change the commit message in v2.

> 
>>> The match is kept simple, to not accidentally catch things like
>>>> -o 'foo=bar upgrade baz'
>>> and trip up advanced users.
>>>
>>> It does not catch invocations with '-y' either, making it less likely
>>> to break automated user scripts. Although they should not use
>>> 'upgrade' either, it still would be bad to break them. If the risk is
>>> still considered too high, this change should wait until a major or
>>> at least point release.
>>>
>>> To avoid false positives, it would be necessary to properly parse
>>> options, which is likely not worth the effort.
>>>
>>> A downside is that the hook is only invoked after the user confirms
>>> the upgrade, but there doesn't seem to be an early enough hook entry
>>> (DPkg::Pre-Invoke is also too late). Since this is just an additional
>>> safety warning to guide new users, it should still be good enough.
>>>
>>> [0]: https://forum.proxmox.com/threads/150217/post-680158
>>> [1]: https://forum.proxmox.com/threads/140580/post-630419
>>> [2]: https://www.reddit.com/r/Proxmox/comments/ujqig9/use_apt_distupgrade_or_the_gui_not_apt_upgrade/
>>> [3]: https://pve.proxmox.com/pve-docs/chapter-sysadmin.html#system_software_updates
>>>
>>
>> yeah, it's something I considered here and then but never pulled through,
>> as it just somehow doesn't feel right...
>>
>> But it's definitively a real problem, and so I surely won't block this on
>> the basis of some gut feeling, I'd rather like to hear Fabian's opinion on
>> it.
> 
> given that I also use `apt upgrade` from time to time (habit from being an unstable user ;)), and that it might alienate power users coming from Debian, I'd prefer this to be a non-interactive warning with the text "disarmed" a bit?
> 
> something like
> 
> !! WARNING !!
> Since Proxmox VE follows a rolling release model, using 'upgrade' can lead to a system being stuck on outdated versions, or in rare cases, break upon upgrading. Use 'dist-upgrade' or 'full-upgrade' instead.
> !! WARNING !!
> 
> with or without a prompt (it's a pity that the hook is not executed with the config before the regular confirmation prompt, else we could just depend on that)?

Sounds good. And since it is so rare, I'd just leave out the additional
confirmation prompt then.


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

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

* Re: [pve-devel] [RFC qemu-server] apt hook: warn against using 'upgrade' command
  2024-09-09  7:52     ` Fiona Ebner
@ 2024-09-09  7:57       ` Thomas Lamprecht
  0 siblings, 0 replies; 10+ messages in thread
From: Thomas Lamprecht @ 2024-09-09  7:57 UTC (permalink / raw)
  To: Fiona Ebner, Fabian Grünbichler, Proxmox VE development discussion

Am 09/09/2024 um 09:52 schrieb Fiona Ebner:
> Am 09.09.24 um 09:48 schrieb Fabian Grünbichler:
>> given that I also use `apt upgrade` from time to time (habit from being an unstable user ;)), and that it might alienate power users coming from Debian, I'd prefer this to be a non-interactive warning with the text "disarmed" a bit?
>>
>> something like
>>
>> !! WARNING !!
>> Since Proxmox VE follows a rolling release model, using 'upgrade' can lead to a system being stuck on outdated versions, or in rare cases, break upon upgrading. Use 'dist-upgrade' or 'full-upgrade' instead.
>> !! WARNING !!

All right, albeit I find the "!! WARNING !!" still rather a bit to scary then.

Maybe just an extra newline above and below and then something like your wording:

NOTE: Proxmox VE follows a rolling release model, so using the 'upgrade' command [...]

FWIW, you could also make this more generic for easier re-use, like:

NOTE: Proxmox projects follow [...]

and ship it in a common package, maybe even an extra one that is only recommended,
not hard-dependency, so the power user can remove the warning if they don't care
(or are daring)


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

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

end of thread, other threads:[~2024-09-09  7:57 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-09-06 10:40 [pve-devel] [RFC qemu-server] apt hook: warn against using 'upgrade' command Fiona Ebner
2024-09-06 10:42 ` Fiona Ebner
2024-09-06 11:01 ` Dominik Csapak
2024-09-06 16:54   ` Thomas Lamprecht
2024-09-06 12:16 ` Alexander Zeidler
2024-09-06 16:56   ` Thomas Lamprecht
2024-09-06 16:58 ` Thomas Lamprecht
2024-09-09  7:48   ` Fabian Grünbichler
2024-09-09  7:52     ` Fiona Ebner
2024-09-09  7:57       ` 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