public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pbs-devel] [RFC proxmox/proxmox-backup] refactor common server code from pbs
@ 2023-10-18 10:39 Dominik Csapak
  2023-10-18 10:39 ` [pbs-devel] [RFC proxmox 1/3] new proxmox-server-config crate Dominik Csapak
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Dominik Csapak @ 2023-10-18 10:39 UTC (permalink / raw)
  To: pbs-devel

# General

Sending this as RFC because of the multiple ways (see 'Alternatives'
section) we could implement this (or something similar). I'm not sure
what the general opinion about this pattern is.

# Goal

When creating a new daemon/server, we currently have much code in pbs
that would have to be duplicated, mostly relating to setting up the
server directories/files. Some crates features already have an interface
to be slightly configurable, but not all of them.

This change adds a new crate 'proxmox-server-config' that provides an
interface which should be easy to use and reusable across multiple
daemons/server applications for our use-cases.

In this series i opted to factor out the two main remaining crates that
are not seperated yet: the jobstates and the certificate management.
(Though i guess there are more, depending on what another daemon might
want to do)

# Implementation

I use a static OnceLock to save a global `ServerConfig` struct, that
contains some configurations available, which are either setup, or
derived from the minimum required configuration (base_dir, name, user).

This can then be used to access them from any point in the program or in
crates where needed.

With this, we don't have to carry around the different directories as
parameters, nor do we have to sprinkle things like `config!("foo.bar")`,
around the code to get the path we want.

# Alternatives

The main alternatives to this approaches are:

* copy the code across different server/daemons:
  obviously the easiest way, but doesn't scale well and prone to
  diverging implementations.

* parameters:
  this has two ways of working:
  - all functions must take the configuration (dir(s)/user(s))
    blows up the parameters, and we have to use our current dir macros
    even more
  - each crate/module has an initializer that takes the config, which
    would then e.g. save that into a static OnceLock once again,
    disadvantage here is that we may duplicate values across multiple
    crates (should not be much though)

There are probably more ways to implement this (e.g. maybe a global
trait object that must be implemented by each server?)

# Future work

We could extend this pattern for other crates (e.g. rest-server) so
that we don't have to carry around the parameters anymore.

Also we could make the ServerConfig extendable, where each server could
configure it's own necessary variables (e.g with a hashmap)

proxmox:

Dominik Csapak (3):
  new proxmox-server-config crate
  new proxmox-jobstate crate
  new proxmox-cert-management crate

 Cargo.toml                                   |   5 +
 proxmox-cert-management/Cargo.toml           |  23 ++
 proxmox-cert-management/debian/changelog     |   5 +
 proxmox-cert-management/debian/control       |  53 ++++
 proxmox-cert-management/debian/copyright     |  18 ++
 proxmox-cert-management/debian/debcargo.toml |   7 +
 proxmox-cert-management/src/lib.rs           | 182 +++++++++++
 proxmox-jobstate/Cargo.toml                  |  26 ++
 proxmox-jobstate/debian/changelog            |   5 +
 proxmox-jobstate/debian/control              |  55 ++++
 proxmox-jobstate/debian/copyright            |  18 ++
 proxmox-jobstate/debian/debcargo.toml        |   7 +
 proxmox-jobstate/src/api_types.rs            |  40 +++
 proxmox-jobstate/src/jobstate.rs             | 315 +++++++++++++++++++
 proxmox-jobstate/src/lib.rs                  |  46 +++
 proxmox-server-config/Cargo.toml             |  16 +
 proxmox-server-config/debian/changelog       |   5 +
 proxmox-server-config/debian/control         |  37 +++
 proxmox-server-config/debian/copyright       |  18 ++
 proxmox-server-config/debian/debcargo.toml   |   7 +
 proxmox-server-config/src/lib.rs             | 302 ++++++++++++++++++
 21 files changed, 1190 insertions(+)
 create mode 100644 proxmox-cert-management/Cargo.toml
 create mode 100644 proxmox-cert-management/debian/changelog
 create mode 100644 proxmox-cert-management/debian/control
 create mode 100644 proxmox-cert-management/debian/copyright
 create mode 100644 proxmox-cert-management/debian/debcargo.toml
 create mode 100644 proxmox-cert-management/src/lib.rs
 create mode 100644 proxmox-jobstate/Cargo.toml
 create mode 100644 proxmox-jobstate/debian/changelog
 create mode 100644 proxmox-jobstate/debian/control
 create mode 100644 proxmox-jobstate/debian/copyright
 create mode 100644 proxmox-jobstate/debian/debcargo.toml
 create mode 100644 proxmox-jobstate/src/api_types.rs
 create mode 100644 proxmox-jobstate/src/jobstate.rs
 create mode 100644 proxmox-jobstate/src/lib.rs
 create mode 100644 proxmox-server-config/Cargo.toml
 create mode 100644 proxmox-server-config/debian/changelog
 create mode 100644 proxmox-server-config/debian/control
 create mode 100644 proxmox-server-config/debian/copyright
 create mode 100644 proxmox-server-config/debian/debcargo.toml
 create mode 100644 proxmox-server-config/src/lib.rs

proxmox-backup:

Dominik Csapak (1):
  use proxmox_jobstate and proxmox_cert_management crates

 Cargo.toml                        |   8 +
 pbs-api-types/Cargo.toml          |   1 +
 pbs-api-types/src/jobs.rs         |  41 +---
 src/auth_helpers.rs               | 184 +---------------
 src/bin/proxmox-backup-api.rs     |   8 +-
 src/bin/proxmox-backup-manager.rs |   2 +
 src/bin/proxmox-backup-proxy.rs   |   2 +
 src/server/jobstate.rs            | 347 +-----------------------------
 src/server/mod.rs                 |  21 ++
 9 files changed, 52 insertions(+), 562 deletions(-)

-- 
2.30.2





^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2023-10-30 11:41 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-18 10:39 [pbs-devel] [RFC proxmox/proxmox-backup] refactor common server code from pbs Dominik Csapak
2023-10-18 10:39 ` [pbs-devel] [RFC proxmox 1/3] new proxmox-server-config crate Dominik Csapak
2023-10-25 16:38   ` Thomas Lamprecht
2023-10-30 11:05     ` Dominik Csapak
2023-10-30 11:41       ` Thomas Lamprecht
2023-10-18 10:39 ` [pbs-devel] [RFC proxmox 2/3] new proxmox-jobstate crate Dominik Csapak
2023-10-18 10:39 ` [pbs-devel] [RFC proxmox 3/3] new proxmox-cert-management crate Dominik Csapak
2023-10-18 10:39 ` [pbs-devel] [RFC proxmox-backup 1/1] use proxmox_jobstate and proxmox_cert_management crates Dominik Csapak
2023-10-24  8:17 ` [pbs-devel] [RFC proxmox/proxmox-backup] refactor common server code from pbs Lukas Wagner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal