[pbs-devel] [PATCH proxmox-backup 1/4] GC: flatten existing status into job status

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

to avoid drifting definitions and reduce duplication. with the next major
release, the 'upid' field could then be renamed and aliased to be in line with
the other jobs, which all use 'last-run-upid'. doing it now would break
existing callers of the GC status endpoint (or consumers of the on-disk status
file).

the main difference is that the GC status fields are now not optional (except
for the UPID) in the job status, since flattening an optional value is not
possible. this only affects datastores that were never GCed at all, and only
direct API consumers, since the UI handles those fields correctly.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
 pbs-api-types/src/datastore.rs | 26 +++++---------------
 src/api2/admin/datastore.rs    | 43 ++++++++++------------------------
 www/Utils.js                   |  8 ++++++-
 www/config/GCView.js           | 12 +++++-----
 4 files changed, 31 insertions(+), 58 deletions(-)

diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs
index 872c4bc7..5d787369 100644
--- a/pbs-api-types/src/datastore.rs
+++ b/pbs-api-types/src/datastore.rs
@@ -1246,7 +1246,7 @@ pub struct TypeCounts {
         },
     },
 )]
-#[derive(Clone, Default, Serialize, Deserialize, PartialEq)]
+#[derive(Clone, Debug, Default, Serialize, Deserialize, PartialEq)]
 #[serde(rename_all = "kebab-case")]
 /// Garbage collection status.
 pub struct GarbageCollectionStatus {
@@ -1275,11 +1275,10 @@ pub struct GarbageCollectionStatus {
 
 #[api(
     properties: {
-        "last-run-upid": {
-            optional: true,
-            type: UPID,
+        "status": {
+            type: GarbageCollectionStatus,
         },
-    },
+    }
 )]
 #[derive(Clone, Debug, Default, Serialize, Deserialize, PartialEq)]
 #[serde(rename_all = "kebab-case")]
@@ -1287,21 +1286,8 @@ pub struct GarbageCollectionStatus {
 pub struct GarbageCollectionJobStatus {
     /// Datastore
     pub store: String,
-    /// upid of the last run gc job
-    #[serde(skip_serializing_if = "Option::is_none")]
-    pub last_run_upid: Option<String>,
-    /// Sum of removed bytes.
-    #[serde(skip_serializing_if = "Option::is_none")]
-    pub removed_bytes: Option<u64>,
-    /// Number of removed chunks
-    #[serde(skip_serializing_if = "Option::is_none")]
-    pub removed_chunks: Option<usize>,
-    /// Sum of pending bytes
-    #[serde(skip_serializing_if = "Option::is_none")]
-    pub pending_bytes: Option<u64>,
-    /// Number of pending chunks
-    #[serde(skip_serializing_if = "Option::is_none")]
-    pub pending_chunks: Option<usize>,
+    #[serde(flatten)]
+    pub status: GarbageCollectionStatus,
     /// Schedule of the gc job
     #[serde(skip_serializing_if = "Option::is_none")]
     pub schedule: Option<String>,
diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 77f9fe3d..da2b545a 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -35,13 +35,13 @@ use pxar::EntryKind;
 use pbs_api_types::{
     print_ns_and_snapshot, print_store_and_ns, Authid, BackupContent, BackupNamespace, BackupType,
     Counts, CryptMode, DataStoreConfig, DataStoreListItem, DataStoreStatus,
-    GarbageCollectionJobStatus, GarbageCollectionStatus, GroupListItem, JobScheduleStatus,
-    KeepOptions, Operation, PruneJobOptions, RRDMode, RRDTimeFrame, SnapshotListItem,
-    SnapshotVerifyState, BACKUP_ARCHIVE_NAME_SCHEMA, BACKUP_ID_SCHEMA, BACKUP_NAMESPACE_SCHEMA,
-    BACKUP_TIME_SCHEMA, BACKUP_TYPE_SCHEMA, DATASTORE_SCHEMA, IGNORE_VERIFIED_BACKUPS_SCHEMA,
-    MAX_NAMESPACE_DEPTH, NS_MAX_DEPTH_SCHEMA, PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_BACKUP,
-    PRIV_DATASTORE_MODIFY, PRIV_DATASTORE_PRUNE, PRIV_DATASTORE_READ, PRIV_DATASTORE_VERIFY, UPID,
-    UPID_SCHEMA, VERIFICATION_OUTDATED_AFTER_SCHEMA,
+    GarbageCollectionJobStatus, GroupListItem, JobScheduleStatus, KeepOptions, Operation,
+    PruneJobOptions, RRDMode, RRDTimeFrame, SnapshotListItem, SnapshotVerifyState,
+    BACKUP_ARCHIVE_NAME_SCHEMA, BACKUP_ID_SCHEMA, BACKUP_NAMESPACE_SCHEMA, BACKUP_TIME_SCHEMA,
+    BACKUP_TYPE_SCHEMA, DATASTORE_SCHEMA, IGNORE_VERIFIED_BACKUPS_SCHEMA, MAX_NAMESPACE_DEPTH,
+    NS_MAX_DEPTH_SCHEMA, PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_BACKUP, PRIV_DATASTORE_MODIFY,
+    PRIV_DATASTORE_PRUNE, PRIV_DATASTORE_READ, PRIV_DATASTORE_VERIFY, UPID, UPID_SCHEMA,
+    VERIFICATION_OUTDATED_AFTER_SCHEMA,
 };
 use pbs_client::pxar::{create_tar, create_zip};
 use pbs_config::CachedUserInfo;
@@ -1273,35 +1273,15 @@ pub fn garbage_collection_job_status(
     let datastore = DataStore::lookup_datastore(&store, Some(Operation::Read))?;
     let status_in_memory = datastore.last_gc_status();
     let state_file = JobState::load("garbage_collection", &store)
-        .map_err(|err| {
-            log::error!(
-                "could not open statefile for {:?}: {}",
-                info.last_run_upid,
-                err
-            )
-        })
+        .map_err(|err| log::error!("could not open GC statefile for {store}: {err}"))
         .ok();
 
-    let mut selected_upid = None;
-    if status_in_memory.upid.is_some() {
-        selected_upid = status_in_memory.upid;
-    } else if let Some(JobState::Finished { upid, .. }) = &state_file {
-        selected_upid = Some(upid.to_owned());
-    }
-
-    info.last_run_upid = selected_upid.clone();
-
-    match selected_upid {
-        Some(upid) => {
-            info.removed_bytes = Some(status_in_memory.removed_bytes);
-            info.removed_chunks = Some(status_in_memory.removed_chunks);
-            info.pending_bytes = Some(status_in_memory.pending_bytes);
-            info.pending_chunks = Some(status_in_memory.pending_chunks);
-
+    match status_in_memory.upid {
+        Some(ref upid) => {
             let mut computed_schedule: JobScheduleStatus = JobScheduleStatus::default();
             let mut duration = None;
             if let Some(state) = state_file {
-                if let Ok(cs) = compute_schedule_status(&state, info.last_run_upid.as_deref()) {
+                if let Ok(cs) = compute_schedule_status(&state, Some(&upid)) {
                     computed_schedule = cs;
                 }
             }
@@ -1327,6 +1307,7 @@ pub fn garbage_collection_job_status(
                 }
             }
 
+            info.status = status_in_memory;
             info.next_run = computed_schedule.next_run;
             info.last_run_endtime = computed_schedule.last_run_endtime;
             info.last_run_state = computed_schedule.last_run_state;
diff --git a/www/Utils.js b/www/Utils.js
index acd6e0d8..0c1f78b6 100644
--- a/www/Utils.js
+++ b/www/Utils.js
@@ -200,7 +200,13 @@ Ext.define('PBS.Utils', {
     },
 
     render_task_status: function(value, metadata, record, rowIndex, colIndex, store) {
-	if (!record.data['last-run-upid'] && !store.getById('last-run-upid')?.data.value) {
+	// GC tasks use 'upid' for backwards-compat, rest use 'last-run-upid'
+	if (
+	    !record.data['last-run-upid'] &&
+	    !store.getById('last-run-upid')?.data.value &&
+	    !record.data.upid &&
+	    !store.getById('upid')?.data.value
+	) {
 	    return '-';
 	}
 
diff --git a/www/config/GCView.js b/www/config/GCView.js
index 8cdf63f9..982ab939 100644
--- a/www/config/GCView.js
+++ b/www/config/GCView.js
@@ -1,14 +1,14 @@
 Ext.define('pbs-gc-jobs-status', {
     extend: 'Ext.data.Model',
     fields: [
-	'store', 'last-run-upid', 'removed-bytes', 'pending-bytes', 'schedule',
+	'store', 'upid', 'removed-bytes', 'pending-bytes', 'schedule',
 	'next-run', 'last-run-endtime', 'last-run-state',
 	{
 	    name: 'duration',
 	    calculate: function(data) {
 		let endtime = data['last-run-endtime'];
 		if (!endtime) return undefined;
-		let task = Proxmox.Utils.parse_task_upid(data['last-run-upid']);
+		let task = Proxmox.Utils.parse_task_upid(data['upid']);
 		return endtime - task.starttime;
 	    },
 	},
@@ -97,7 +97,7 @@ Ext.define('PBS.config.GCJobView', {
 	showTaskLog: function() {
 	    let me = this;
 
-	    let upid = this.getData()['last-run-upid'];
+	    let upid = this.getData().upid;
 	    if (!upid) return;
 
 	    Ext.create('Proxmox.window.TaskViewer', { upid }).show();
@@ -147,7 +147,7 @@ Ext.define('PBS.config.GCJobView', {
 	    xtype: 'proxmoxButton',
 	    text: gettext('Show Log'),
 	    handler: 'showTaskLog',
-	    enableFn: (rec) => !!rec.data["last-run-upid"],
+	    enableFn: (rec) => !!rec.data.upid,
 	    disabled: true,
 	},
 	{
@@ -214,7 +214,7 @@ Ext.define('PBS.config.GCJobView', {
 	{
 	    header: gettext('Removed Data'),
 	    dataIndex: 'removed-bytes',
-	    renderer: (value) => value !== undefined
+	    renderer: (value, meta, record) => record.data.upid !== null
 		? Proxmox.Utils.format_size(value, true) : "-",
 	    sortable: false,
 	    minWidth: 85,
@@ -223,7 +223,7 @@ Ext.define('PBS.config.GCJobView', {
 	{
 	    header: gettext('Pending Data'),
 	    dataIndex: 'pending-bytes',
-	    renderer: (value) => value !== undefined
+	    renderer: (value, meta, record) => record.data.upid !== null
 		? Proxmox.Utils.format_size(value, true) : "-",
 	    sortable: false,
 	    minWidth: 80,
-- 
2.39.2



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