[pve-devel] [PATCH common/pve-manager/pmg-api] unify task filter api

[pve-devel] [PATCH common/pve-manager/pmg-api] unify task filter api

[Dominik Csapak]

this series aims to get some baseline for the task listing filters in
the api across products. the most filters are currently in PBS,
so we add the missing ones in pve/pmg

this changes only the api yet

pve-manager patch 2/2 conflicts with Fabian Ebners patch
"api/cli: tasks: don't treat warnings status as an error status"

pve-common:

Dominik Csapak (2):
  PVE/JSONSchema: add pve-task-status-type
  PVE/Tools: add 'upid_get_status_type'

 src/PVE/JSONSchema.pm | 12 ++++++++++++
 src/PVE/Tools.pm      | 20 ++++++++++++++++++++
 2 files changed, 32 insertions(+)

pve-manager:

Dominik Csapak (2):
  PVE/API2/Tasks: add since/until filter for the task list
  PVE/API2/Tasks: add statusfilter to task list

 PVE/API2/Tasks.pm | 45 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 44 insertions(+), 1 deletion(-)

pmg-api:

Dominik Csapak (3):
  PMG/API2/Tasks: add typefilter
  PMG/API2/Tasks: add since and until filter
  PMG/API2/Tasks: add statusfilter

 src/PMG/API2/Tasks.pm | 53 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 52 insertions(+), 1 deletion(-)

-- 
2.20.1

[pve-devel] [PATCH common 1/2] PVE/JSONSchema: add pve-task-status-type

[Dominik Csapak]

to have a format that contains the possible worker task states

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/PVE/JSONSchema.pm | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/src/PVE/JSONSchema.pm b/src/PVE/JSONSchema.pm
index 6297fff..71df690 100644
--- a/src/PVE/JSONSchema.pm
+++ b/src/PVE/JSONSchema.pm
@@ -689,6 +689,18 @@ sub pve_verify_tfa_secret {
     die "unable to decode TFA secret\n";
 }
 
+
+PVE::JSONSchema::register_format('pve-task-status-type', \&verify_task_status_type);
+sub verify_task_status_type {
+    my ($value, $noerr) = @_;
+
+    return $value if $value =~ m/^(ok|error|warning|unknown)$/i;
+
+    return undef if $noerr;
+
+    die "invalid status '$value'\n";
+}
+
 sub check_format {
     my ($format, $value, $path) = @_;
 
-- 
2.20.1

[pve-devel] applied: [pmg-devel] [PATCH common 1/2] PVE/JSONSchema: add pve-task-status-type

[Thomas Lamprecht]

On 24.06.21 09:10, Dominik Csapak wrote:
> to have a format that contains the possible worker task states
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  src/PVE/JSONSchema.pm | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
>

applied, thanks! Maybe we should name those things a bit more product
agnostic, especially if intended to be used in PMG too.

[pve-devel] [PATCH common 2/2] PVE/Tools: add 'upid_get_status_type'

[Dominik Csapak]

as a single point where we get the type of upid status

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/PVE/Tools.pm | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/src/PVE/Tools.pm b/src/PVE/Tools.pm
index 8946e93..7cca4a4 100644
--- a/src/PVE/Tools.pm
+++ b/src/PVE/Tools.pm
@@ -1179,6 +1179,26 @@ sub upid_status_is_error {
     return !($status eq 'OK' || $status =~ m/^WARNINGS: \d+$/);
 }
 
+# takes the parsed status and returns the type,
+# either ok, warning, error or unknown
+sub upid_get_status_type {
+    my ($status) = @_;
+
+    if (!$status) {
+	return 'unknown';
+    }
+
+    if ($status eq 'OK') {
+	return 'ok';
+    } elsif ($status =~ m/^WARNINGS: \d+$/) {
+	return 'warning';
+    } elsif ($status eq 'unexpected status') {
+	return 'unknown';
+    } else {
+	return 'error';
+    }
+}
+
 # useful functions to store comments in config files
 sub encode_text {
     my ($text) = @_;
-- 
2.20.1

[pve-devel] applied: [pmg-devel] [PATCH common 2/2] PVE/Tools: add 'upid_get_status_type'

[Thomas Lamprecht]

On 24.06.21 09:10, Dominik Csapak wrote:
> as a single point where we get the type of upid status
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  src/PVE/Tools.pm | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
>

applied, thanks! Renamed it to `upid_normalize_status_type` though, as this does not
really "gets" (as in loads it from somewhere) but rather transforms a argument passed,
so the caller has to get the status previously.

[pve-devel] [PATCH manager 1/2] PVE/API2/Tasks: add since/until filter for the task list

[Dominik Csapak]

similar to pbs

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 PVE/API2/Tasks.pm | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/PVE/API2/Tasks.pm b/PVE/API2/Tasks.pm
index 8f6ab603..8df701e5 100644
--- a/PVE/API2/Tasks.pm
+++ b/PVE/API2/Tasks.pm
@@ -90,6 +90,16 @@ __PACKAGE__->register_method({
 		optional => 1,
 		description => 'List archived, active or all tasks.',
 	    },
+	    since => {
+		type => 'integer',
+		description => "Only list tasks since this UNIX epoch.",
+		optional => 1,
+	    },
+	    until => {
+		type => 'integer',
+		description => "Only list tasks until this UNIX epoch.",
+		optional => 1,
+	    },
 	},
     },
     returns => {
@@ -128,6 +138,8 @@ __PACKAGE__->register_method({
 	my $typefilter = $param->{typefilter};
 	my $errors = $param->{errors} // 0;
 	my $source = $param->{source} // 'archive';
+	my $since = $param->{since};
+	my $until = $param->{until};
 
 	my $count = 0;
 	my $line;
@@ -145,6 +157,9 @@ __PACKAGE__->register_method({
 	    return 1 if $errors && $task->{status} && $task->{status} eq 'OK';
 	    return 1 if $param->{vmid} && (!$task->{id} || $task->{id} ne $param->{vmid});
 
+	    return 1 if defined($since) && $task->{starttime} < $since;
+	    return 1 if defined($until) && $task->{starttime} > $until;
+
 	    return 1 if $count++ < $start;
 	    return 1 if $limit <= 0;
 
-- 
2.20.1

[pve-devel] applied: [pmg-devel] [PATCH manager 1/2] PVE/API2/Tasks: add since/until filter for the task list

[Thomas Lamprecht]

On 24.06.21 09:10, Dominik Csapak wrote:
> similar to pbs
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  PVE/API2/Tasks.pm | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
>

applied, thanks!

[pve-devel] [PATCH manager 2/2] PVE/API2/Tasks: add statusfilter to task list

[Dominik Csapak]

similar to pbs. the 'errors' filter parameter still overrides this

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 PVE/API2/Tasks.pm | 30 +++++++++++++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/PVE/API2/Tasks.pm b/PVE/API2/Tasks.pm
index 8df701e5..eca92715 100644
--- a/PVE/API2/Tasks.pm
+++ b/PVE/API2/Tasks.pm
@@ -100,6 +100,12 @@ __PACKAGE__->register_method({
 		description => "Only list tasks until this UNIX epoch.",
 		optional => 1,
 	    },
+	    statusfilter => {
+		type => 'string',
+		format => 'pve-task-status-type-list',
+		optional => 1,
+		description => 'List of Task States that should be returned.',
+	    },
 	},
     },
     returns => {
@@ -140,6 +146,26 @@ __PACKAGE__->register_method({
 	my $source = $param->{source} // 'archive';
 	my $since = $param->{since};
 	my $until = $param->{until};
+	my $statusfilter = {
+	    ok => 1,
+	    warning => 1,
+	    error => 1,
+	    unknown => 1,
+	};
+
+	if (defined($param->{statusfilter}) && !$errors) {
+	    $statusfilter = {
+		ok => 0,
+		warning => 0,
+		error => 0,
+		unknown => 0,
+	    };
+	    for my $filter (PVE::Tools::split_list($param->{statusfilter})) {
+		$statusfilter->{lc($filter)} = 1 ;
+	    }
+	} elsif ($errors) {
+	    $statusfilter->{ok} = 0;
+	}
 
 	my $count = 0;
 	my $line;
@@ -154,12 +180,14 @@ __PACKAGE__->register_method({
 
 	    return 1 if $typefilter && $task->{type} ne $typefilter;
 
-	    return 1 if $errors && $task->{status} && $task->{status} eq 'OK';
 	    return 1 if $param->{vmid} && (!$task->{id} || $task->{id} ne $param->{vmid});
 
 	    return 1 if defined($since) && $task->{starttime} < $since;
 	    return 1 if defined($until) && $task->{starttime} > $until;
 
+	    my $type = PVE::Tools::upid_get_status_type($task->{status});
+	    return 1 if !$statusfilter->{$type};
+
 	    return 1 if $count++ < $start;
 	    return 1 if $limit <= 0;
 
-- 
2.20.1

[pve-devel] applied: [pmg-devel] [PATCH manager 2/2] PVE/API2/Tasks: add statusfilter to task list

[Thomas Lamprecht]

On 24.06.21 09:10, Dominik Csapak wrote:
> similar to pbs. the 'errors' filter parameter still overrides this
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  PVE/API2/Tasks.pm | 30 +++++++++++++++++++++++++++++-
>  1 file changed, 29 insertions(+), 1 deletion(-)
> 
>

applied, with adapting it to the renamed task status normalize helper, thanks!

[pve-devel] [PATCH pmg-api 1/3] PMG/API2/Tasks: add typefilter

[Dominik Csapak]

like we have one in PVE/PBS

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/PMG/API2/Tasks.pm | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/src/PMG/API2/Tasks.pm b/src/PMG/API2/Tasks.pm
index 17a043e..85edabf 100644
--- a/src/PMG/API2/Tasks.pm
+++ b/src/PMG/API2/Tasks.pm
@@ -43,6 +43,11 @@ __PACKAGE__->register_method({
 		type => 'boolean',
 		optional => 1,
 	    },
+	    typefilter => {
+		type => 'string',
+		optional => 1,
+		description => 'Only list tasks of this type (e.g., aptupdate, saupdate).',
+	    },
 	},
     },
     returns => {
@@ -68,6 +73,7 @@ __PACKAGE__->register_method({
 	my $start = $param->{start} || 0;
 	my $limit = $param->{limit} || 50;
 	my $userfilter = $param->{userfilter};
+	my $typefilter = $param->{typefilter};
 	my $errors = $param->{errors};
 
 	my $count = 0;
@@ -81,6 +87,8 @@ __PACKAGE__->register_method({
 		if ((my $task = PVE::Tools::upid_decode($upid, 1))) {
 		    return if $userfilter && $task->{user} !~ m/\Q$userfilter\E/i;
 		    return if $errors && $status && $status eq 'OK';
+		    return if $typefilter && $task->{type} ne $typefilter;
+
 		    return if $count++ < $start;
 		    return if $limit <= 0;
 
-- 
2.20.1

[pve-devel] [PATCH pmg-api 2/3] PMG/API2/Tasks: add since and until filter

[Dominik Csapak]

like in PVE/PBS

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/PMG/API2/Tasks.pm | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/src/PMG/API2/Tasks.pm b/src/PMG/API2/Tasks.pm
index 85edabf..a369c2a 100644
--- a/src/PMG/API2/Tasks.pm
+++ b/src/PMG/API2/Tasks.pm
@@ -48,6 +48,16 @@ __PACKAGE__->register_method({
 		optional => 1,
 		description => 'Only list tasks of this type (e.g., aptupdate, saupdate).',
 	    },
+	    since => {
+		type => 'integer',
+		description => "Only list tasks since this UNIX epoch.",
+		optional => 1,
+	    },
+	    until => {
+		type => 'integer',
+		description => "Only list tasks until this UNIX epoch.",
+		optional => 1,
+	    },
 	},
     },
     returns => {
@@ -74,6 +84,8 @@ __PACKAGE__->register_method({
 	my $limit = $param->{limit} || 50;
 	my $userfilter = $param->{userfilter};
 	my $typefilter = $param->{typefilter};
+	my $since = $param->{since};
+	my $until = $param->{until};
 	my $errors = $param->{errors};
 
 	my $count = 0;
@@ -87,6 +99,8 @@ __PACKAGE__->register_method({
 		if ((my $task = PVE::Tools::upid_decode($upid, 1))) {
 		    return if $userfilter && $task->{user} !~ m/\Q$userfilter\E/i;
 		    return if $errors && $status && $status eq 'OK';
+		    return if defined($since) && $task->{starttime} < $since;
+		    return if defined($until) && $task->{starttime} > $until;
 		    return if $typefilter && $task->{type} ne $typefilter;
 
 		    return if $count++ < $start;
-- 
2.20.1

[pve-devel] [PATCH pmg-api 3/3] PMG/API2/Tasks: add statusfilter

[Dominik Csapak]

like in PVE/PBS

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/PMG/API2/Tasks.pm | 31 ++++++++++++++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/src/PMG/API2/Tasks.pm b/src/PMG/API2/Tasks.pm
index a369c2a..b3d4279 100644
--- a/src/PMG/API2/Tasks.pm
+++ b/src/PMG/API2/Tasks.pm
@@ -58,6 +58,12 @@ __PACKAGE__->register_method({
 		description => "Only list tasks until this UNIX epoch.",
 		optional => 1,
 	    },
+	    statusfilter => {
+		type => 'string',
+		format => 'pve-task-status-type-list',
+		optional => 1,
+		description => 'List of Task States that should be returned.',
+	    },
 	},
     },
     returns => {
@@ -88,6 +94,27 @@ __PACKAGE__->register_method({
 	my $until = $param->{until};
 	my $errors = $param->{errors};
 
+	my $statusfilter = {
+	    ok => 1,
+	    warning => 1,
+	    error => 1,
+	    unknown => 1,
+	};
+
+	if (defined($param->{statusfilter}) && !$errors) {
+	    $statusfilter = {
+		ok => 0,
+		warning => 0,
+		error => 0,
+		unknown => 0,
+	    };
+	    for my $filter (PVE::Tools::split_list($param->{statusfilter})) {
+		$statusfilter->{lc($filter)} = 1 ;
+	    }
+	} elsif ($errors) {
+	    $statusfilter->{ok} = 0;
+	}
+
 	my $count = 0;
 	my $line;
 
@@ -98,11 +125,13 @@ __PACKAGE__->register_method({
 		my $status = $5;
 		if ((my $task = PVE::Tools::upid_decode($upid, 1))) {
 		    return if $userfilter && $task->{user} !~ m/\Q$userfilter\E/i;
-		    return if $errors && $status && $status eq 'OK';
 		    return if defined($since) && $task->{starttime} < $since;
 		    return if defined($until) && $task->{starttime} > $until;
 		    return if $typefilter && $task->{type} ne $typefilter;
 
+		    my $statustype = PVE::Tools::upid_get_status_type($status);
+		    return if !$statusfilter->{$statustype};
+
 		    return if $count++ < $start;
 		    return if $limit <= 0;
 
-- 
2.20.1

[pve-devel] partially-applied: [pmg-devel] [PATCH common/pve-manager/pmg-api] unify task filter api

[Thomas Lamprecht]

On 24.06.21 09:10, Dominik Csapak wrote:
> pmg-api:
> 
> Dominik Csapak (3):
>   PMG/API2/Tasks: add typefilter
>   PMG/API2/Tasks: add since and until filter
>   PMG/API2/Tasks: add statusfilter
> 

applied above, but adapted commit subject as it made not much sense to me, as:
* PMG is rather obvious in a pmg-api repo, so not much use for the human reader
* s!API2/Tasks!api: tasks!; as the "subsystem" is api, affecting the tasks endpoints,
  we do not really care which API version

This almost looks like you just copied over the file changed, which is not really
useful as I can just use `git log --stat` for that, the subject should rather be
as friendly as possible for the human eye, ideally be written such that it can be
copied over 1:1 in d/changelog and be also digestible easily in `git log --oneline`.