Re: [PATCH datacenter-manager v4 00/10] subscription key pool registry

[Dominik Csapak <d.csapak@proxmox.com>]

just tested it shortly, very nice!

I have a few high level comments (nothing too grave) and
I'll look into the code afterwards and send separate
notes (I'll mainly focus on the ui)

* there is a bit a lack of ui feedback when something is loading
   I know there is the 'reload' button on the top right, but
   I took a while to actually see that button at all^^

   In my setup I often have some remote that is slow or not
   reachable, which probably increases the response times of these
   apis since they (i guess, didn't look yet) reuse the caching
   mechanisms we have for subscription (and these timeout after
   ~3 seconds per remote i think). so this is quite noticable
   here: I click on auto-assign and nothing is happening for multiple
   seconds.

   I'd propose either masking the panel when something 'blocking' happens
   or make the loading state more prominent (e.g. the progress widget)
   or maybe a combination of both depending what happens

* when adding keys, we might want to (very coarsely) check the syntax
   in the ui to give better feedback? e.g. having a key multiple times
   in there could be easily checked i think (or even stripped in the
   api request). same for general shape of the keys (not too strict
   though, otherwise it could be a problem if we change the backend
   format of the keys)

* the rows in the auto-assign grid are not selectable (probably
   with intent). this feels a bit weird, especially since i can
   change the header columns. Simple improvement would be to
   disable header menus so the grid is completely noninteractive
   (or we make it selectable like you mention below)

* if pending operations fail partially, they won't continue, which
   can be ok, but e.g. if some node is not reachable (e.g. a transient
   error) the whole operation cancels. I'd have expected that all
   will be tried at least.

On 5/22/26 10:51 AM, Thomas Lamprecht wrote:
> v4 of the Subscription Registry. Addresses all review feedback on v3
> from Wolfgang and Lukas, and drops the trailing RFC wizard per the
> consensus on v3-0012.
> 
> For the v2 -> v3 changelog and the original design discussion see the
> v3 cover at:
>    https://lore.proxmox.com/pdm-devel/20260515074623.766766-1-t.lamprecht@proxmox.com
> 
> Notable v3 -> v4 (see per-patch notes for details):
> 
> * RFC wizard (v3-0012) dropped per consensus on the review thread.
> * v4-0009 bundles Adopt Key + Adopt All (was v3-0009 + v3-0010) into
>    one commit since they share the dropdown filter and Source column.
> * Net series count: 12 -> 10.
> * All of Lukas's cover-level UI nits closed: Assign-Key remote/node
>    filters, Key column font-size in proposal/preview tables, per-node
>    Assign dialog footer padding. Verified live on a real fleet.
> * All of Wolfgang's inline review nits applied.
> * Internal: `invalidate_subscription_info_for_remote` ported to the
>    new api_cache abstraction on master.
> 
> R-b and T-b trailers picked up via `b4 trailers -u $msgid`, for patch
> v4-0004 and v4-0005 I skipped applying the Tested-by from Lukas, as
> there was a bit more churn there since the revision that was tested.
> 
> For the record, but IMO no blockers for the initial MVP, the open
> follow-ups, still not in this series:
> * A few UI/UX improvements, like multi-select on auto-import to allow
>    skipping a few remote/nodes - that shouldn't be to hard, but I just
>    wanted to get the v4 out now.
> * Atomic clear-and-assign as one queued change (today swapping a key
>    on a node is Clear / Apply / Assign / Apply; the canonical case is
>    an Expired live subscription that the operator wants to replace).
> * Shop-side full reissue, so PDM can drive the actual key rotation
>    rather than just freeing the pool binding via Clear Key.
> * Shop-bundle import path; the on-disk shadow-file plumbing already
>    accommodates the signed SubscriptionInfo blob.
> * Per-row Auto-Assign overrides for pinning a specific key to a node.
> * Status column filter on the node-status tree.
> 
> Thomas Lamprecht (10):
>    api types: subscription level: render full names
>    pdm-client: add wait_for_local_task helper
>    subscription: pool: add data model and config layer
>    subscription: api: add key pool and node status endpoints
>    ui: registry: add view with key pool and node status
>    cli: client: add subscription key pool management subcommands
>    docs: add subscription registry chapter
>    subscription: add Clear Key action and per-node revert
>    subscription: add Adopt Key action for foreign live subscriptions
>    subscription: add Check Subscription action
> 
>   Cargo.toml                                    |    4 +-
>   cli/client/src/subscriptions.rs               |  401 ++-
>   docs/index.rst                                |    1 +
>   docs/subscription-registry.rst                |   84 +
>   lib/pdm-api-types/Cargo.toml                  |    1 +
>   lib/pdm-api-types/src/subscription.rs         |  493 +++-
>   lib/pdm-api-types/tests/test_import.rs        |  367 +++
>   lib/pdm-client/Cargo.toml                     |    3 +
>   lib/pdm-client/src/lib.rs                     |  330 ++-
>   lib/pdm-config/src/lib.rs                     |    1 +
>   lib/pdm-config/src/setup.rs                   |    7 +
>   lib/pdm-config/src/subscriptions.rs           |  118 +
>   server/src/api/mod.rs                         |    2 +
>   server/src/api/resources.rs                   |   34 +-
>   server/src/api/subscriptions/mod.rs           | 2310 +++++++++++++++++
>   server/src/context.rs                         |    7 +
>   server/src/pbs_client.rs                      |   31 +
>   ui/src/configuration/mod.rs                   |    3 +
>   ui/src/configuration/subscription_assign.rs   |  336 +++
>   ui/src/configuration/subscription_keys.rs     |  583 +++++
>   ui/src/configuration/subscription_registry.rs | 1469 +++++++++++
>   ui/src/dashboard/subscriptions_list.rs        |   18 +-
>   ui/src/main_menu.rs                           |   10 +
>   ui/src/widget/pve_node_selector.rs            |   91 +-
>   ui/src/widget/remote_selector.rs              |   28 +-
>   25 files changed, 6665 insertions(+), 67 deletions(-)
>   create mode 100644 docs/subscription-registry.rst
>   create mode 100644 lib/pdm-api-types/tests/test_import.rs
>   create mode 100644 lib/pdm-config/src/subscriptions.rs
>   create mode 100644 server/src/api/subscriptions/mod.rs
>   create mode 100644 ui/src/configuration/subscription_assign.rs
>   create mode 100644 ui/src/configuration/subscription_keys.rs
>   create mode 100644 ui/src/configuration/subscription_registry.rs
>