public inbox for pdm-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: "Fabian Grünbichler" <f.gruenbichler@proxmox.com>
To: Dominik Csapak <d.csapak@proxmox.com>, pdm-devel@lists.proxmox.com
Subject: Re: [pdm-devel] [RFC datacenter-manager 3/3] api: add subscription endpoints
Date: Mon, 01 Dec 2025 13:52:07 +0100	[thread overview]
Message-ID: <1764593361.1bcpnuv4t7.astroid@yuna.none> (raw)
In-Reply-To: <ec96f8d7-8fa0-4dbb-a883-9315e7970267@proxmox.com>

On December 1, 2025 1:42 pm, Dominik Csapak wrote:
> some comments inline
> 
> rest LGTM
> 
> On 12/1/25 12:38 PM, Fabian Grünbichler wrote:
>> for the PDM system itself, by proxy of how many of the remote nodes have valid
>> subscriptions above a certain level.
>> 
>> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
>> ---
>> the thresholds here are rather arbitrary of course, as is the refresh
>> interval for cached subscription information..
>> 
>>   server/src/api/nodes/mod.rs          |   2 +
>>   server/src/api/nodes/subscription.rs | 191 +++++++++++++++++++++++++++
>>   server/src/api/resources.rs          |   2 +-
>>   3 files changed, 194 insertions(+), 1 deletion(-)
>>   create mode 100644 server/src/api/nodes/subscription.rs
>> 
>> diff --git a/server/src/api/nodes/mod.rs b/server/src/api/nodes/mod.rs
>> index a0fe14a..3a795d6 100644
>> --- a/server/src/api/nodes/mod.rs
>> +++ b/server/src/api/nodes/mod.rs
>> @@ -11,6 +11,7 @@ pub mod journal;
>>   pub mod network;
>>   pub mod rrddata;
>>   pub mod status;
>> +pub mod subscription;
>>   pub mod syslog;
>>   pub mod tasks;
>>   pub mod termproxy;
>> @@ -45,6 +46,7 @@ pub const SUBDIRS: SubdirMap = &sorted!([
>>       ("journal", &journal::ROUTER),
>>       ("network", &network::ROUTER),
>>       ("rrdata", &rrddata::ROUTER),
>> +    ("subscription", &subscription::ROUTER),
>>       ("status", &status::ROUTER),
>>       ("syslog", &syslog::ROUTER),
>>       ("tasks", &tasks::ROUTER),
>> diff --git a/server/src/api/nodes/subscription.rs b/server/src/api/nodes/subscription.rs
>> new file mode 100644
>> index 0000000..343a2c8
>> --- /dev/null
>> +++ b/server/src/api/nodes/subscription.rs
>> @@ -0,0 +1,191 @@
>> +use std::collections::HashMap;
>> +
>> +use anyhow::{bail, Error};
>> +
>> +use proxmox_router::{Permission, Router};
>> +use proxmox_schema::api;
>> +use proxmox_schema::api_types::NODE_SCHEMA;
>> +use proxmox_subscription::files::update_apt_auth;
>> +use proxmox_subscription::{SubscriptionInfo, SubscriptionStatus};
>> +use proxmox_sys::fs::CreateOptions;
>> +
>> +use pdm_api_types::remotes::RemoteType;
>> +use pdm_api_types::subscription::{
>> +    NodeSubscriptionInfo, SubscriptionLevel, SubscriptionStatistics,
>> +};
>> +use pdm_api_types::PRIV_SYS_MODIFY;
>> +
>> +use crate::api::resources::get_subscription_info_for_remote;
>> +
>> +const PRODUCT_URL: &str = "https://www.proxmox.com/en/proxmox-datacenter-maanger/pricing";
> 
> typo:
> 
> s/maanger/manager/

yes. not sure what the URL is supposed to be in any case? ;)

> 
>> +const APT_AUTH_FN: &str = "/etc/apt/auth.conf.d/pdm.conf";
>> +const APT_AUTH_URL: &str = "enterprise.proxmox.com/debian/pdm";
>> +
>> +// minimum ratio of nodes with active subscriptions
>> +const SUBSCRIPTION_THRESHOLD: f64 = 0.9;
>> +// max ratio of nodes with community subscriptions, among nodes with subscriptions
>> +const COMMUNITY_THRESHOLD: f64 = 0.4;
>> +
>> +fn apt_auth_file_opts() -> CreateOptions {
>> +    let mode = nix::sys::stat::Mode::from_bits_truncate(0o0600);
>> +    CreateOptions::new().perm(mode).owner(nix::unistd::ROOT)
>> +}
>> +
>> +async fn get_all_subscription_infos(
>> +) -> Result<HashMap<String, (RemoteType, HashMap<String, Option<NodeSubscriptionInfo>>)>, Error> {
>> +    let (remotes_config, _digest) = pdm_config::remotes::config()?;
>> +
>> +    let mut subscription_info = HashMap::new();
>> +    for (remote_name, remote) in remotes_config.iter() {
>> +        match get_subscription_info_for_remote(remote, 24 * 60 * 60).await {
>> +            Ok(info) => {
>> +                subscription_info.insert(remote_name.to_string(), (remote.ty, info));
>> +            }
>> +            Err(err) => {
>> +                log::debug!("Failed to get subscription info for remote {remote_name} - {err}");
>> +                subscription_info.insert(remote_name.to_string(), (remote.ty, HashMap::new()));
>> +            }
>> +        }
>> +    }
>> +    Ok(subscription_info)
>> +}
>> +
>> +fn count_subscriptions(
>> +    subscription_infos: &HashMap<
>> +        String,
>> +        (RemoteType, HashMap<String, Option<NodeSubscriptionInfo>>),
>> +    >,
>> +) -> SubscriptionStatistics {
>> +    let mut stats = SubscriptionStatistics::default();
>> +    for (_remote, (_remote_type, remote_infos)) in subscription_infos.iter() {
>> +        if remote_infos.is_empty() {
>> +            // count remotes without info as at least one node
>> +            stats.total_nodes += 1;
>> +            continue;
> 
> this would count an unreachable remote as one not subscribed node, or?

I set the cache expiry quite long for that reason, but yeah, counting
remotes that throw an error for subscription info requests is a bit
problematic now matter what you do:
- count them as without subscription (conservative, but if a lot are
  unreachable might treat PDM as without subscription)
- ignore them (might allow easily "cheating")
- count them as with subscription (plain wrong ^^)

we could count them separately and add another threshold for the max.
number of remotes with "unknown" status?

> 
>> +        }
>> +        for (_node, node_info) in remote_infos.iter() {
>> +            stats.total_nodes += 1;
>> +            if let Some(info) = node_info {
>> +                if info.status == SubscriptionStatus::Active {
>> +                    stats.active_subscriptions += 1;
>> +                    if info.level == SubscriptionLevel::Community {
>> +                        stats.community += 1;
>> +                    }
>> +                }
>> +            }
>> +        }
>> +    }
>> +    stats
>> +}
>> +
>> +fn check_counts(stats: SubscriptionStatistics) -> Result<(), Error> {
>> +    let subscribed_ratio = stats.active_subscriptions as f64 / stats.total_nodes as f64;
>> +    let community_ratio = stats.community as f64 / stats.active_subscriptions as f64;
>> +
>> +    if subscribed_ratio > SUBSCRIPTION_THRESHOLD {
>> +        if community_ratio < COMMUNITY_THRESHOLD {
>> +            return Ok(());
>> +        } else {
>> +            bail!("Too many remote nodes with community level subscription!");
>> +        }
>> +    } else {
>> +        bail!("Too many remote nodes without active subscription!");
>> +    }
>> +}
>> +
>> +#[api(
>> +    access: { permission: &Permission::Anybody, },
>> +    input: {
>> +        properties: {
>> +            node: {
>> +                schema: NODE_SCHEMA,
>> +            },
>> +        },
>> +    },
>> +    returns: {
>> +        type: SubscriptionInfo,
>> +    }
>> +)]
>> +/// Return subscription status
>> +pub async fn get_subscription() -> Result<SubscriptionInfo, Error> {
>> +    let infos = get_all_subscription_infos().await?;
>> +
>> +    let stats = count_subscriptions(&infos);
>> +
>> +    if let Err(err) = check_counts(stats) {
>> +        Ok(SubscriptionInfo {
>> +            status: SubscriptionStatus::Invalid,
>> +            message: Some(format!("{err}")),
>> +            serverid: None,
>> +            url: Some(PRODUCT_URL.into()),
>> +            ..Default::default()
>> +        })
>> +    } else {
>> +        Ok(SubscriptionInfo {
>> +            status: SubscriptionStatus::Active,
>> +            url: Some(PRODUCT_URL.into()),
>> +            ..Default::default()
>> +        })
>> +    }
>> +}
>> +
>> +#[api(
>> +    input: {
>> +        properties: {
>> +            node: {
>> +                schema: NODE_SCHEMA,
>> +            },
>> +        },
>> +    },
>> +    protected: true,
>> +    access: {
>> +        permission: &Permission::Privilege(&["system"], PRIV_SYS_MODIFY, false),
>> +    },
>> +)]
>> +/// Update subscription information
>> +pub async fn check_subscription() -> Result<(), Error> {
>> +    let infos = get_all_subscription_infos().await?;
>> +    let stats = count_subscriptions(&infos);
>> +
>> +    if let Err(err) = check_counts(stats) {
>> +        update_apt_auth(APT_AUTH_FN, apt_auth_file_opts(), APT_AUTH_URL, None, None)?;
>> +        return Err(err);
>> +    }
>> +
>> +    let mut found = false;
>> +    'outer: for (remote, (remote_type, remote_info)) in infos.iter() {
>> +        if *remote_type != RemoteType::Pve {
>> +            continue;
>> +        }
>> +        for (node, node_info) in remote_info.iter() {
>> +            if let Some(info) = node_info {
>> +                if info.status == SubscriptionStatus::Active
>> +                    && info.key.is_some()
>> +                    && info.serverid.is_some()
>> +                {
>> +                    log::info!("Using subscription of node '{node}' of remote '{remote}' for enterprise repository access");
>> +                    update_apt_auth(
>> +                        APT_AUTH_FN,
>> +                        apt_auth_file_opts(),
>> +                        APT_AUTH_URL,
>> +                        info.key.clone(),
>> +                        info.serverid.clone(),
>> +                    )?;
>> +                    found = true;
>> +                    break 'outer;
>> +                }
>> +            }
>> +        }
>> +    }
>> +
>> +    if !found {
>> +        log::warn!("No valid subscription found for configuring enterprise repository access..");
>> +        update_apt_auth(APT_AUTH_FN, apt_auth_file_opts(), APT_AUTH_URL, None, None)?;
>> +    }
>> +
>> +    Ok(())
>> +}
>> +
>> +pub const ROUTER: Router = Router::new()
>> +    .get(&API_METHOD_GET_SUBSCRIPTION)
>> +    .post(&API_METHOD_CHECK_SUBSCRIPTION);
>> diff --git a/server/src/api/resources.rs b/server/src/api/resources.rs
>> index 4beaa54..adab021 100644
>> --- a/server/src/api/resources.rs
>> +++ b/server/src/api/resources.rs
>> @@ -767,7 +767,7 @@ static SUBSCRIPTION_CACHE: LazyLock<RwLock<HashMap<String, CachedSubscriptionSta
>>   ///
>>   /// If recent enough cached data is available, it is returned
>>   /// instead of calling out to the remote.
>> -async fn get_subscription_info_for_remote(
>> +pub async fn get_subscription_info_for_remote(
>>       remote: &Remote,
>>       max_age: u64,
>>   ) -> Result<HashMap<String, Option<NodeSubscriptionInfo>>, Error> {
> 
> 


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

  reply	other threads:[~2025-12-01 12:52 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-01 11:39 [pdm-devel] [RFC datacenter-manager 0/3] PDM subscription status Fabian Grünbichler
2025-12-01 11:39 ` [pdm-devel] [RFC datacenter-manager 1/3] subscription: add serverid field to node subscription info Fabian Grünbichler
2025-12-01 11:39 ` [pdm-devel] [RFC datacenter-manager 2/3] api types: add new SubscriptionStatistics Fabian Grünbichler
2025-12-01 12:36   ` Dominik Csapak
2025-12-01 12:39     ` Thomas Lamprecht
2025-12-01 12:43       ` Dominik Csapak
2025-12-01 12:40     ` Fabian Grünbichler
2025-12-01 11:39 ` [pdm-devel] [RFC datacenter-manager 3/3] api: add subscription endpoints Fabian Grünbichler
2025-12-01 12:42   ` Dominik Csapak
2025-12-01 12:52     ` Fabian Grünbichler [this message]
2025-12-01 12:38 ` [pdm-devel] [RFC datacenter-manager 0/3] PDM subscription status Dominik Csapak
2025-12-01 13:15 ` [pdm-devel] superseded: " Fabian Grünbichler

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1764593361.1bcpnuv4t7.astroid@yuna.none \
    --to=f.gruenbichler@proxmox.com \
    --cc=d.csapak@proxmox.com \
    --cc=pdm-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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