[pbs-devel] [PATCH proxmox-backup/widget-toolkit] Handle optional services and expose more details

[pbs-devel] [PATCH proxmox-backup/widget-toolkit] Handle optional services and expose more details

[Christoph Heiss]

Add the `unit-state` property for services to the PBS api, much like
it's PVE counterpart. This avoids erroneously marking services as dead
that might be just not installed, e.g. systemd-timesyncd.

On the GUI side, this functionality already exists (from PVE), just the
appropriate cookie name needs to be added.

See also #4406.

proxmox-backup:

Christoph Heiss (1):
      api2/node/services: Handle optional services and expose unit-state

 src/api2/node/services.rs | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

proxmox-widget-toolkit:

Christoph Heiss (1):
      node/ServiceView: Show unit-state column in PBS too

 src/node/ServiceView.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.30.2

[pbs-devel] [PATCH proxmox-backup] api2/node/services: Handle optional services and expose unit-state

[Christoph Heiss]

.. in the same way the PVE api does, esp. regarding the logic to handle
oneshot and missing services.

This then allows re-using the GUI parts from there as well, so that the
services page in PVE and PBS looks the same.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 src/api2/node/services.rs | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/src/api2/node/services.rs b/src/api2/node/services.rs
index 0cc1857e..534c41e8 100644
--- a/src/api2/node/services.rs
+++ b/src/api2/node/services.rs
@@ -76,12 +76,27 @@ fn get_full_service_state(service: &str) -> Result<Value, Error> {
 fn json_service_state(service: &str, status: Value) -> Value {
     if let Some(desc) = status["Description"].as_str() {
         let name = status["Name"].as_str().unwrap_or(service);
-        let state = status["SubState"].as_str().unwrap_or("unknown");
+
+        let state = if status["Type"] == "oneshot" && status["SubState"] == "dead" {
+            &status["Result"]
+        } else {
+            &status["SubState"]
+        }
+        .as_str()
+        .unwrap_or("unknown");
+
+        let unit_state = if status["LoadState"] == "not-found" {
+            "not-found"
+        } else {
+            status["UnitFileState"].as_str().unwrap_or("unknown")
+        };
+
         return json!({
             "service": service,
             "name": name,
             "desc": desc,
             "state": state,
+            "unit-state": unit_state,
         });
     }
 
@@ -117,6 +132,10 @@ fn json_service_state(service: &str, status: Value) -> Value {
                     type: String,
                     description: "systemd service 'SubState'.",
                 },
+                "unit-state": {
+                    type: String,
+                    description: "systemd service unit state.",
+                },
             },
         },
     },
-- 
2.30.2

Re: [pbs-devel] [PATCH proxmox-backup] api2/node/services: Handle optional services and expose unit-state

[Wolfgang Bumiller]

On Wed, Dec 14, 2022 at 10:42:59AM +0100, Christoph Heiss wrote:
> .. in the same way the PVE api does, esp. regarding the logic to handle
> oneshot and missing services.
> 
> This then allows re-using the GUI parts from there as well, so that the
> services page in PVE and PBS looks the same.
> 
> Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
> ---
>  src/api2/node/services.rs | 21 ++++++++++++++++++++-
>  1 file changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/src/api2/node/services.rs b/src/api2/node/services.rs
> index 0cc1857e..534c41e8 100644
> --- a/src/api2/node/services.rs
> +++ b/src/api2/node/services.rs
> @@ -76,12 +76,27 @@ fn get_full_service_state(service: &str) -> Result<Value, Error> {
>  fn json_service_state(service: &str, status: Value) -> Value {
>      if let Some(desc) = status["Description"].as_str() {
>          let name = status["Name"].as_str().unwrap_or(service);
> -        let state = status["SubState"].as_str().unwrap_or("unknown");
> +
> +        let state = if status["Type"] == "oneshot" && status["SubState"] == "dead" {
> +            &status["Result"]

Reading the perl equivalent, if we end up here but `Result` does not
exist we'd see 'dead' in perl as it falls back to SubState (which at
this point is known to be 'dead'), while we'd show 'unknown' in this
case from the 'unwrap_or' below.
Not sure if this can happen though?

> +        } else {
> +            &status["SubState"]
> +        }
> +        .as_str()
> +        .unwrap_or("unknown");

> +
> +        let unit_state = if status["LoadState"] == "not-found" {
> +            "not-found"
> +        } else {
> +            status["UnitFileState"].as_str().unwrap_or("unknown")
> +        };
> +
>          return json!({
>              "service": service,
>              "name": name,
>              "desc": desc,
>              "state": state,
> +            "unit-state": unit_state,
>          });
>      }
>  
> @@ -117,6 +132,10 @@ fn json_service_state(service: &str, status: Value) -> Value {
>                      type: String,
>                      description: "systemd service 'SubState'.",
>                  },
> +                "unit-state": {
> +                    type: String,
> +                    description: "systemd service unit state.",
> +                },
>              },
>          },
>      },
> -- 
> 2.30.2

Re: [pbs-devel] [PATCH proxmox-backup] api2/node/services: Handle optional services and expose unit-state

[Christoph Heiss]

On 12/15/22 10:29, Wolfgang Bumiller wrote:
> On Wed, Dec 14, 2022 at 10:42:59AM +0100, Christoph Heiss wrote:
>> .. in the same way the PVE api does, esp. regarding the logic to handle
>> oneshot and missing services.
>>
>> This then allows re-using the GUI parts from there as well, so that the
>> services page in PVE and PBS looks the same.
>>
>> Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
>> ---
>>   src/api2/node/services.rs | 21 ++++++++++++++++++++-
>>   1 file changed, 20 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/api2/node/services.rs b/src/api2/node/services.rs
>> index 0cc1857e..534c41e8 100644
>> --- a/src/api2/node/services.rs
>> +++ b/src/api2/node/services.rs
>> @@ -76,12 +76,27 @@ fn get_full_service_state(service: &str) -> Result<Value, Error> {
>>   fn json_service_state(service: &str, status: Value) -> Value {
>>       if let Some(desc) = status["Description"].as_str() {
>>           let name = status["Name"].as_str().unwrap_or(service);
>> -         let state = status["SubState"].as_str().unwrap_or("unknown");
>> +
>> +         let state = if status["Type"] == "oneshot" && status["SubState"] == "dead" {
>> +             &status["Result"]
> 
> Reading the perl equivalent, if we end up here but `Result` does not
> exist we'd see 'dead' in perl as it falls back to SubState (which at
> this point is known to be 'dead'), while we'd show 'unknown' in this
> case from the 'unwrap_or' below.
> Not sure if this can happen though?

Not sure either, but if it is a finished one-shot service, systemd
*should* always provide a result. That's IMHO a very reasonable
assumption and was my thought while working on it too.

Playing a bit with `systemctl show` and looking at some services, it
really seems that the Result property is always present, even for e.g.
non-existing services.

But if you want, I can send a v2 and add the fallback to SubState here
as well, to align it with the Perl equivalent.

>> [..]
>> -- 
>> 2.30.2

[pbs-devel] [PATCH widget-toolkit] node/ServiceView: Show unit-state column in PBS too

[Christoph Heiss]

The PBS api now reports `unit-state` for services as well, thus enable
the column for it.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 src/node/ServiceView.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/node/ServiceView.js b/src/node/ServiceView.js
index 75a5c28..19cfc18 100644
--- a/src/node/ServiceView.js
+++ b/src/node/ServiceView.js
@@ -201,7 +201,7 @@ Ext.define('Proxmox.node.ServiceView', {
 		    header: gettext('Unit'),
 		    width: 120,
 		    sortable: true,
-		    hidden: Proxmox?.Setup?.auth_cookie_name !== 'PVEAuthCookie', // FIXME currently only PVE supports it
+		    hidden: !Ext.Array.contains(['PVEAuthCookie', 'PBSAuthCookie'], Proxmox?.Setup?.auth_cookie_name),
 		    dataIndex: 'unit-state',
 		},
 		{
-- 
2.30.2