[pve-devel] [PATCH-SERIES manager/proxmox-ve] warn against/prevent using virtual console for major upgrade

[pve-devel] [PATCH-SERIES manager/proxmox-ve] warn against/prevent using virtual console for major upgrade

[Fabian Ebner]

Quoting from the upgrade notes:

> Perform the actions via console or ssh; preferably via console to avoid
> interrupted ssh connections. Do not carry out the upgrade when connected
> via the virtual console offered by the GUI; as this will get interrupted
> during the upgrade.

But some users still seem to miss this, so let's be more direct.

One part is proxmox-ve patches #1 and #2, just mentioning it up front.

The other two patches (sent as RFC, as I'm not sure this is the best
approach), would make it a hard error when a console started via
API/GUI is detected upon attempting a major upgrade.


All patches are also intended for stable-6. Note that proxmox-ve does
not currently have a stable-6 branch, I used
286285a9a441ad5b1a3c1869373bfbaadbb70bb4 as a base when testing.


proxmox-ve depends on pve-manager for the new behavior to take effect,
but it's not a hard dependency.


pve-manager:

Fabian Ebner (1):
  api: nodes: set environment variable for shells started via the API

 PVE/API2/Nodes.pm | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)


proxmox-ve:

Fabian Ebner (3):
  apt hook: avoid long line and fix typo
  apt hook: mention that console/ssh should be used for major upgrade
  apt hook: disallow major upgrade via virtual console from API/UI

 debian/apthook/pve-apt-hook | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)
-- 
2.20.1

[pve-devel] [RFC manager 1/1] api: nodes: set environment variable for shells started via the API

[Fabian Ebner]

so that proxmox-ve's apt hook script can detect this.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 PVE/API2/Nodes.pm | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/PVE/API2/Nodes.pm b/PVE/API2/Nodes.pm
index e58d9c10..c57ad995 100644
--- a/PVE/API2/Nodes.pm
+++ b/PVE/API2/Nodes.pm
@@ -843,13 +843,13 @@ my $sslcert;
 
 my $shell_cmd_map = {
     'login' => {
-	cmd => [ '/bin/login', '-f', 'root' ],
+	cmd => [ '/bin/login', '-f', 'root', 'PVE_API_SHELL=1' ],
     },
     'upgrade' => {
-	cmd => [ '/usr/bin/pveupgrade', '--shell' ],
+	cmd => [ '/usr/bin/env', 'PVE_API_SHELL=1', '/usr/bin/pveupgrade', '--shell' ],
     },
     'ceph_install' => {
-	cmd => [ '/usr/bin/pveceph', 'install' ],
+	cmd => [ '/usr/bin/env', 'PVE_API_SHELL=1', '/usr/bin/pveceph', 'install' ],
 	allow_args => 1,
     },
 };
@@ -866,11 +866,11 @@ sub get_shell_command  {
 		push @$cmd, split("\0", $args);
 	    }
 	} else {
-	    $cmd = [ '/bin/login', '-f', 'root' ];
+	    $cmd = [ '/bin/login', '-f', 'root', 'PVE_API_SHELL=1' ];
 	}
     } else {
 	# non-root must always login for now, we do not have a superuser role!
-	$cmd = [ '/bin/login' ];
+	$cmd = [ '/bin/login', 'PVE_API_SHELL=1' ];
     }
     return $cmd;
 }
-- 
2.20.1

[pve-devel] [PATCH proxmox-ve 1/3] apt hook: avoid long line and fix typo

[Fabian Ebner]

by making 'upgrade' lowercase.

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

diff --git a/debian/apthook/pve-apt-hook b/debian/apthook/pve-apt-hook
index 1f77a1a..d79b6c0 100755
--- a/debian/apthook/pve-apt-hook
+++ b/debian/apthook/pve-apt-hook
@@ -74,7 +74,8 @@ while (my $line = <$fh>) {
       }
     } elsif ($action eq '**CONFIGURE**' && $dir eq '<' && $old =~ /^6\./ && $new =~ /^7\./) {
       $log->("!! ATTENTION !!\n");
-      $log->("You are attempting to upgrade from proxmox-ve '$old' to proxmox-ve '$new'. Please make sure to read the Upgrade notes at\n");
+      $log->("You are attempting to upgrade from proxmox-ve '$old' to proxmox-ve '$new'.\n");
+      $log->("Please make sure to read the upgrade notes at\n");
       $log->("\thttps://pve.proxmox.com/wiki/Upgrade_from_6.x_to_7.0\n");
       $log->("before proceeding with this operation.\n");
       $log->("\n");
-- 
2.20.1

[pve-devel] [PATCH proxmox-ve 2/3] apt hook: mention that console/ssh should be used for major upgrade

[Fabian Ebner]

There were a few reports of people trying to upgrade via the virtual
console running into problems, because it would be interrupted. The
latest one is [0], although I'm speculating that it might not have
been the cause of all the reported problems in this case.

[0]: https://forum.proxmox.com/threads/help-please-problem-upgrading-to-pve7.95941/

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

diff --git a/debian/apthook/pve-apt-hook b/debian/apthook/pve-apt-hook
index d79b6c0..72a38ca 100755
--- a/debian/apthook/pve-apt-hook
+++ b/debian/apthook/pve-apt-hook
@@ -77,7 +77,8 @@ while (my $line = <$fh>) {
       $log->("You are attempting to upgrade from proxmox-ve '$old' to proxmox-ve '$new'.\n");
       $log->("Please make sure to read the upgrade notes at\n");
       $log->("\thttps://pve.proxmox.com/wiki/Upgrade_from_6.x_to_7.0\n");
-      $log->("before proceeding with this operation.\n");
+      $log->("and that you are are connected directly via console or ssh (not the virtual\n");
+      $log->("console offered by the GUI!) before proceeding with this operation.\n");
       $log->("\n");
       $log->("Press enter to continue, or C^c to abort.\n");
       $cleanup->(0, 1);
-- 
2.20.1

[pve-devel] [RFC proxmox-ve 3/3] apt hook: disallow major upgrade via virtual console from API/UI

[Fabian Ebner]

and adapt the output to avoid too much redundancy.

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

diff --git a/debian/apthook/pve-apt-hook b/debian/apthook/pve-apt-hook
index 72a38ca..0eed6eb 100755
--- a/debian/apthook/pve-apt-hook
+++ b/debian/apthook/pve-apt-hook
@@ -77,11 +77,16 @@ while (my $line = <$fh>) {
       $log->("You are attempting to upgrade from proxmox-ve '$old' to proxmox-ve '$new'.\n");
       $log->("Please make sure to read the upgrade notes at\n");
       $log->("\thttps://pve.proxmox.com/wiki/Upgrade_from_6.x_to_7.0\n");
-      $log->("and that you are are connected directly via console or ssh (not the virtual\n");
-      $log->("console offered by the GUI!) before proceeding with this operation.\n");
+      $log->("and that you are are connected directly via console or ssh before proceeding\n");
+      $log->("with this operation.\n");
       $log->("\n");
-      $log->("Press enter to continue, or C^c to abort.\n");
-      $cleanup->(0, 1);
+      if ($ENV{PVE_API_SHELL}) {
+          $log->("Error: Refusing to carry out the major upgrade via GUI/API virtual console.\n");
+          $cleanup->(1);
+      } else {
+          $log->("Press enter to continue, or C^c to abort.\n");
+          $cleanup->(0, 1);
+      }
     }
   }
 }
-- 
2.20.1

Re: [pve-devel] [RFC manager 1/1] api: nodes: set environment variable for shells started via the API

[Fabian Ebner]

Am 13.09.21 um 14:04 schrieb Fabian Ebner:
> so that proxmox-ve's apt hook script can detect this.
> 
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
>   PVE/API2/Nodes.pm | 10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/PVE/API2/Nodes.pm b/PVE/API2/Nodes.pm
> index e58d9c10..c57ad995 100644
> --- a/PVE/API2/Nodes.pm
> +++ b/PVE/API2/Nodes.pm
> @@ -843,13 +843,13 @@ my $sslcert;
>   
>   my $shell_cmd_map = {
>       'login' => {
> -	cmd => [ '/bin/login', '-f', 'root' ],
> +	cmd => [ '/bin/login', '-f', 'root', 'PVE_API_SHELL=1' ],
>       },
>       'upgrade' => {
> -	cmd => [ '/usr/bin/pveupgrade', '--shell' ],
> +	cmd => [ '/usr/bin/env', 'PVE_API_SHELL=1', '/usr/bin/pveupgrade', '--shell' ],
>       },
>       'ceph_install' => {
> -	cmd => [ '/usr/bin/pveceph', 'install' ],
> +	cmd => [ '/usr/bin/env', 'PVE_API_SHELL=1', '/usr/bin/pveceph', 'install' ],
>   	allow_args => 1,
>       },
>   };
> @@ -866,11 +866,11 @@ sub get_shell_command  {
>   		push @$cmd, split("\0", $args);
>   	    }
>   	} else {
> -	    $cmd = [ '/bin/login', '-f', 'root' ];
> +	    $cmd = [ '/bin/login', '-f', 'root', 'PVE_API_SHELL=1' ];
>   	}
>       } else {
>   	# non-root must always login for now, we do not have a superuser role!
> -	$cmd = [ '/bin/login' ];
> +	$cmd = [ '/bin/login', 'PVE_API_SHELL=1' ];

Sorry, I think the PVE_API_SHELL=1 gets interpreted as the username in 
this case. I'll fix that in v2 if we even go with this approach.

>       }
>       return $cmd;
>   }
> 

Re: [pve-devel] [PATCH-SERIES manager/proxmox-ve] warn against/prevent using virtual console for major upgrade

[Fabian Ebner]

Could I get some feedback for this?

Am 13.09.21 um 14:04 schrieb Fabian Ebner:
> Quoting from the upgrade notes:
> 
>> Perform the actions via console or ssh; preferably via console to avoid
>> interrupted ssh connections. Do not carry out the upgrade when connected
>> via the virtual console offered by the GUI; as this will get interrupted
>> during the upgrade.
> 
> But some users still seem to miss this, so let's be more direct.
> 
> One part is proxmox-ve patches #1 and #2, just mentioning it up front.
> 
> The other two patches (sent as RFC, as I'm not sure this is the best
> approach), would make it a hard error when a console started via
> API/GUI is detected upon attempting a major upgrade.
> 
> 
> All patches are also intended for stable-6. Note that proxmox-ve does
> not currently have a stable-6 branch, I used
> 286285a9a441ad5b1a3c1869373bfbaadbb70bb4 as a base when testing.
> 
> 
> proxmox-ve depends on pve-manager for the new behavior to take effect,
> but it's not a hard dependency.
> 
> 
> pve-manager:
> 
> Fabian Ebner (1):
>    api: nodes: set environment variable for shells started via the API
> 
>   PVE/API2/Nodes.pm | 10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)
> 
> 
> proxmox-ve:
> 
> Fabian Ebner (3):
>    apt hook: avoid long line and fix typo
>    apt hook: mention that console/ssh should be used for major upgrade
>    apt hook: disallow major upgrade via virtual console from API/UI
> 
>   debian/apthook/pve-apt-hook | 15 +++++++++++----
>   1 file changed, 11 insertions(+), 4 deletions(-)
>