[pve-devel] [PATCH-SERIES v2] improve warnings handling in UI and add to PVE

[pve-devel] [PATCH-SERIES v2] improve warnings handling in UI and add to PVE

[Fabian Ebner]

Changes from v1:
    * dropped already applied patches
    * add format_task_status helper and use that everywhere
    * improve usage of replace() (for the widget-toolkit change)
    * chomp the message in warn() (for the pve-common change)


The first half is just a small UI improvement (but it's all over the place),
replacing 'WARNINGS' with (a language-aware) 'Warnings'.

The later half adds the feature of ending in a WARNINGS state to PVE and
provides a usage example.


proxmox-backup and pve-manager need a dependency bump for proxmox-widget-toolkit
pve-container needs a dependency bump for pve-common


widget-toolkit:

Fabian Ebner (3):
  task status: create helper for formatting
  format task status: rename variable
  format task status: improve replace() usage

 src/Utils.js      | 11 +++++++++++
 src/node/Tasks.js | 10 +---------
 2 files changed, 12 insertions(+), 9 deletions(-)


proxmox-backup:

Fabian Ebner (1):
  ui: tasks: use format_task_status

 www/panel/Tasks.js | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)


manager:

Fabian Ebner (1):
  ui: cluster task log: handle warnings like the node task log does

 www/manager6/dc/Tasks.js | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)


common:

Fabian Ebner (1):
  allow workers to count warnings and finish tasks in a WARNINGS state

 src/PVE/RESTEnvironment.pm | 22 ++++++++++++++++++++--
 src/PVE/Tools.pm           |  2 ++
 2 files changed, 22 insertions(+), 2 deletions(-)


container:

Fabian Ebner (1):
  restore: sanitize config: use new warn() function

 src/PVE/LXC/Create.pm | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

-- 
2.20.1

[pve-devel] [PATCH v2 widget-toolkit 1/7] task status: create helper for formatting

[Fabian Ebner]

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

New in v2.

 src/Utils.js      | 12 ++++++++++++
 src/node/Tasks.js | 10 +---------
 2 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/src/Utils.js b/src/Utils.js
index 3fd8f91..21bdbe3 100644
--- a/src/Utils.js
+++ b/src/Utils.js
@@ -736,6 +736,18 @@ utilities: {
 	return 'error';
     },
 
+    format_task_status: function(value) {
+	let parsed = Proxmox.Utils.parse_task_status(value);
+	switch (parsed) {
+	    case 'unknown': return Proxmox.Utils.unknownText;
+	    case 'error': return Proxmox.Utils.errorText + ': ' + value;
+	    case 'warning': return Proxmox.Utils.warningsText +
+		value.replace('WARNINGS', '');
+	    case 'ok': // fall-through
+	    default: return value;
+	}
+    },
+
     render_duration: function(value) {
 	if (value === undefined) {
 	    return '-';
diff --git a/src/node/Tasks.js b/src/node/Tasks.js
index b01f65e..7f20a8a 100644
--- a/src/node/Tasks.js
+++ b/src/node/Tasks.js
@@ -198,15 +198,7 @@ Ext.define('Proxmox.node.Tasks', {
 			    return '';
 			}
 
-			let parsed = Proxmox.Utils.parse_task_status(value);
-			switch (parsed) {
-			    case 'unknown': return Proxmox.Utils.unknownText;
-			    case 'error': return Proxmox.Utils.errorText + ': ' + value;
-			    case 'warning': return Proxmox.Utils.warningsText +
-				value.replace('WARNINGS', '');
-			    case 'ok': // fall-through
-			    default: return value;
-			}
+			return Proxmox.Utils.format_task_status(value);
 		    },
 		},
 	    ],
-- 
2.20.1

[pve-devel] applied: [PATCH v2 widget-toolkit 1/7] task status: create helper for formatting

[Thomas Lamprecht]

On 09.04.21 10:44, Fabian Ebner wrote:
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
> 
> New in v2.
> 
>  src/Utils.js      | 12 ++++++++++++
>  src/node/Tasks.js | 10 +---------
>  2 files changed, 13 insertions(+), 9 deletions(-)
> 
>

applied, thanks!

[pve-devel] [PATCH v2 widget-toolkit 2/7] format task status: rename variable

[Fabian Ebner]

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

New in v2.

 src/Utils.js | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/Utils.js b/src/Utils.js
index 21bdbe3..155ef49 100644
--- a/src/Utils.js
+++ b/src/Utils.js
@@ -736,15 +736,15 @@ utilities: {
 	return 'error';
     },
 
-    format_task_status: function(value) {
-	let parsed = Proxmox.Utils.parse_task_status(value);
+    format_task_status: function(status) {
+	let parsed = Proxmox.Utils.parse_task_status(status);
 	switch (parsed) {
 	    case 'unknown': return Proxmox.Utils.unknownText;
-	    case 'error': return Proxmox.Utils.errorText + ': ' + value;
+	    case 'error': return Proxmox.Utils.errorText + ': ' + status;
 	    case 'warning': return Proxmox.Utils.warningsText +
-		value.replace('WARNINGS', '');
+		status.replace('WARNINGS', '');
 	    case 'ok': // fall-through
-	    default: return value;
+	    default: return status;
 	}
     },
 
-- 
2.20.1

[pve-devel] applied: [PATCH v2 widget-toolkit 2/7] format task status: rename variable

[Thomas Lamprecht]

On 09.04.21 10:44, Fabian Ebner wrote:
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
> 
> New in v2.
> 
>  src/Utils.js | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
>

applied, thanks!

[pve-devel] [PATCH v2 widget-toolkit 3/7] format task status: improve replace() usage

[Fabian Ebner]

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

New in v2.

 src/Utils.js | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/Utils.js b/src/Utils.js
index 155ef49..1dd227a 100644
--- a/src/Utils.js
+++ b/src/Utils.js
@@ -741,8 +741,7 @@ utilities: {
 	switch (parsed) {
 	    case 'unknown': return Proxmox.Utils.unknownText;
 	    case 'error': return Proxmox.Utils.errorText + ': ' + status;
-	    case 'warning': return Proxmox.Utils.warningsText +
-		status.replace('WARNINGS', '');
+	    case 'warning': return status.replace('WARNINGS', Proxmox.Utils.warningsText);
 	    case 'ok': // fall-through
 	    default: return status;
 	}
-- 
2.20.1

[pve-devel] applied: [PATCH v2 widget-toolkit 3/7] format task status: improve replace() usage

[Thomas Lamprecht]

On 09.04.21 10:44, Fabian Ebner wrote:
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
> 
> New in v2.
> 
>  src/Utils.js | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
>

applied, thanks!

[pve-devel] [PATCH v2 proxmox-backup 4/7] ui: tasks: use format_task_status

[Fabian Ebner]

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

Dependency bump for widget-toolkit is needed.

Changes from v1:
    * use new helper instead of duplicating changes

 www/panel/Tasks.js | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/www/panel/Tasks.js b/www/panel/Tasks.js
index a194e478..a64e0eee 100644
--- a/www/panel/Tasks.js
+++ b/www/panel/Tasks.js
@@ -372,14 +372,7 @@ Ext.define('PBS.node.Tasks', {
 		    return '';
 		}
 
-		let parsed = Proxmox.Utils.parse_task_status(value);
-		switch (parsed) {
-		    case 'unknown': return Proxmox.Utils.unknownText;
-		    case 'error': return Proxmox.Utils.errorText + ': ' + value;
-		    case 'ok': // fall-through
-		    case 'warning': // fall-through
-		    default: return value;
-		}
+		return Proxmox.Utils.format_task_status(value);
 	    },
 	},
     ],
-- 
2.20.1

[pve-devel] [PATCH v2 manager 5/7] ui: cluster task log: handle warnings like the node task log does

[Fabian Ebner]

Copied the relevant code from widget-toolkit.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

Dependency bump for widget-toolkit is needed.

Changes from v1:
    * use new helper

 www/manager6/dc/Tasks.js | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/www/manager6/dc/Tasks.js b/www/manager6/dc/Tasks.js
index c5075dc3..2ecd61ee 100644
--- a/www/manager6/dc/Tasks.js
+++ b/www/manager6/dc/Tasks.js
@@ -61,9 +61,15 @@ Ext.define('PVE.dc.Tasks', {
 		getRowClass: function(record, index) {
 		    var status = record.get('status');
 
-		    if (status && status != 'OK') {
-			return "proxmox-invalid-row";
+		    if (status) {
+			let parsed = Proxmox.Utils.parse_task_status(status);
+			if (parsed === 'error') {
+			    return "proxmox-invalid-row";
+			} else if (parsed === 'warning') {
+			    return "proxmox-warning-row";
+			}
 		    }
+		    return '';
 		},
 	    },
 	    sortableColumns: false,
@@ -122,11 +128,8 @@ Ext.define('PVE.dc.Tasks', {
 			    }
 			    return "";
 			}
-			if (value == 'OK') {
-			    return 'OK';
-			}
-			// metaData.attr = 'style="color:red;"';
-			return Proxmox.Utils.errorText + ': ' + value;
+
+			return Proxmox.Utils.format_task_status(value);
 		    },
 		},
 	    ],
-- 
2.20.1

[pve-devel] [PATCH/RFC v2 common 6/7] allow workers to count warnings and finish tasks in a WARNINGS state

[Fabian Ebner]

as is already supported by the UI (and PBS).

A nice bonus is that warn() can be used by both workers and non-workers. For
workers, the output is redirected/duplicated as set up by {fork,tee}_worker(),
and non-erroring workers that issued a warning will end in a WARNINGS state.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

Changes from v1:
    * use chomp in warn(), so it can also be called with newline-terminated
      strings

 src/PVE/RESTEnvironment.pm | 22 ++++++++++++++++++++--
 src/PVE/Tools.pm           |  2 ++
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/src/PVE/RESTEnvironment.pm b/src/PVE/RESTEnvironment.pm
index d5b84d0..e8a466c 100644
--- a/src/PVE/RESTEnvironment.pm
+++ b/src/PVE/RESTEnvironment.pm
@@ -115,7 +115,10 @@ sub init {
     # priv ... access from private server (pvedaemon)
     # ha   ... access from HA resource manager agent (pve-ha-manager)
 
-    my $self = { type => $type };
+    my $self = {
+	type => $type,
+	warning_count => 0,
+    };
 
     bless $self, $class;
 
@@ -448,7 +451,6 @@ my $tee_worker = sub {
 	    }
 	}
 
-	# get status (error or OK)
 	POSIX::read($ctrlfd, $readbuf, 4096);
 	if ($readbuf =~ m/^TASK OK\n?$/) {
 	    # skip printing to stdout
@@ -456,6 +458,9 @@ my $tee_worker = sub {
 	} elsif ($readbuf =~ m/^TASK ERROR: (.*)\n?$/) {
 	    print STDERR "$1\n";
 	    print $taskfh "\n$readbuf"; # ensure start on new line for webUI
+	} elsif ($readbuf =~ m/^TASK WARNINGS: (\d+)\n?$/) {
+	    print STDERR "Task finished with $1 warning(s)!\n";
+	    print $taskfh "\n$readbuf"; # ensure start on new line for webUI
 	} else {
 	    die "got unexpected control message: $readbuf\n";
 	}
@@ -617,6 +622,9 @@ sub fork_worker {
 	    syslog('err', $err);
 	    $msg = "TASK ERROR: $err\n";
 	    $exitcode = -1;
+	} elsif (my $warnings = $self->{warning_count}) {
+	    $msg = "TASK WARNINGS: $warnings\n";
+	    $exitcode = 0;
 	} else {
 	    $msg = "TASK OK\n";
 	    $exitcode = 0;
@@ -703,6 +711,16 @@ sub fork_worker {
     return wantarray ? ($upid, $res) : $upid;
 }
 
+sub warn {
+    my ($self, $message) = @_;
+
+    chomp($message);
+
+    print STDERR "WARN: $message\n";
+
+    $self->{warning_count}++;
+}
+
 # Abstract function
 
 sub log_cluster_msg {
diff --git a/src/PVE/Tools.pm b/src/PVE/Tools.pm
index 2a91a92..16ae3d2 100644
--- a/src/PVE/Tools.pm
+++ b/src/PVE/Tools.pm
@@ -1156,6 +1156,8 @@ sub upid_read_status {
 	    return 'OK';
 	} elsif ($line =~ m/^TASK ERROR: (.+)$/) {
 	    return $1;
+	} elsif ($line =~ m/^TASK (WARNINGS: \d+)$/) {
+	    return $1;
 	} else {
 	    return "unexpected status";
 	}
-- 
2.20.1

[pve-devel] applied: [PATCH/RFC v2 common 6/7] allow workers to count warnings and finish tasks in a WARNINGS state

[Thomas Lamprecht]

On 09.04.21 10:44, Fabian Ebner wrote:
> as is already supported by the UI (and PBS).
> 
> A nice bonus is that warn() can be used by both workers and non-workers. For
> workers, the output is redirected/duplicated as set up by {fork,tee}_worker(),
> and non-erroring workers that issued a warning will end in a WARNINGS state.
> 
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
> 
> Changes from v1:
>     * use chomp in warn(), so it can also be called with newline-terminated
>       strings
> 
>  src/PVE/RESTEnvironment.pm | 22 ++++++++++++++++++++--
>  src/PVE/Tools.pm           |  2 ++
>  2 files changed, 22 insertions(+), 2 deletions(-)
> 
>

applied, thanks!

[pve-devel] [PATCH/RFC v2 container 7/7] restore: sanitize config: use new warn() function

[Fabian Ebner]

to make it more visible that the task finished with warnings.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

No changes from v1.

Dependency bump for pve-common is needed.

 src/PVE/LXC/Create.pm | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/src/PVE/LXC/Create.pm b/src/PVE/LXC/Create.pm
index 82d7ad9..67b4c7a 100644
--- a/src/PVE/LXC/Create.pm
+++ b/src/PVE/LXC/Create.pm
@@ -6,6 +6,7 @@ use File::Basename;
 use File::Path;
 use Fcntl;
 
+use PVE::RPCEnvironment;
 use PVE::Storage::PBSPlugin;
 use PVE::Storage;
 use PVE::DataCenterConfig;
@@ -306,6 +307,8 @@ sub restore_configuration_from_proxmox_backup {
 sub sanitize_and_merge_config {
     my ($conf, $oldconf, $restricted, $unique) = @_;
 
+    my $rpcenv = PVE::RPCEnvironment::get();
+
     foreach my $key (keys %$oldconf) {
 	next if $key eq 'digest' || $key eq 'rootfs' || $key eq 'snapshots' || $key eq 'unprivileged' || $key eq 'parent';
 	next if $key =~ /^mp\d+$/; # don't recover mountpoints
@@ -316,12 +319,16 @@ sub sanitize_and_merge_config {
 
 	if ($key eq 'lxc' && $restricted) {
 	    my $lxc_list = $oldconf->{'lxc'};
-	    warn "skipping custom lxc options, restore manually as root:\n";
-	    warn "--------------------------------\n";
+
+	    my $msg = "skipping custom lxc options, restore manually as root:\n";
+	    $msg .= "--------------------------------\n";
 	    foreach my $lxc_opt (@$lxc_list) {
-		warn "$lxc_opt->[0]: $lxc_opt->[1]\n"
+		$msg .= "$lxc_opt->[0]: $lxc_opt->[1]\n"
 	    }
-	    warn "--------------------------------\n";
+	    $msg .= "--------------------------------";
+
+	    $rpcenv->warn($msg);
+
 	    next;
 	}
 
-- 
2.20.1