Re: [pdm-devel] [PATCH datacenter-manager v2 09/14] config: add auto-installer configuration module

["Christoph Heiss" <c.heiss@proxmox.com>]

Thanks for the review!

On Tue Dec 9, 2025 at 11:22 AM CET, Lukas Wagner wrote:
> One note inline, otherwise this looks good to me.
>
> On Fri Dec 5, 2025 at 12:25 PM CET, Christoph Heiss wrote:
[..]
>> diff --git a/lib/pdm-config/src/auto_install.rs b/lib/pdm-config/src/auto_install.rs
>> new file mode 100644
>> index 0000000..0374a70
>> --- /dev/null
>> +++ b/lib/pdm-config/src/auto_install.rs
>> @@ -0,0 +1,85 @@
>> +//! Implements configuration for the auto-installer integration.
>> +
>> +use anyhow::Result;
>> +use proxmox_schema::ApiType;
>> +use proxmox_section_config::{SectionConfig, SectionConfigData, SectionConfigPlugin};
>> +use std::{fs::File, sync::LazyLock};
>> +
>> +use pdm_api_types::{
>> +    auto_installer::{Installation, PreparedInstallationConfig, PREPARED_INSTALL_CONFIG_ID_SCHEMA},
>> +    ConfigDigest,
>> +};
>> +use proxmox_product_config::{open_api_lockfile, replace_config, ApiLockGuard};
>> +use proxmox_sys::error::SysError;
>> +
>> +pub const CONFIG_PATH: &str = pdm_buildcfg::configdir!("/autoinst");
>> +
>> +static CONFIG: LazyLock<SectionConfig> = LazyLock::new(|| {
>> +    let mut config = SectionConfig::new(&PREPARED_INSTALL_CONFIG_ID_SCHEMA);
>> +
>> +    config.register_plugin(SectionConfigPlugin::new(
>> +        "prepared-answer".into(),
>> +        Some("id".into()),
>> +        PreparedInstallationConfig::API_SCHEMA.unwrap_object_schema(),
>> +    ));
>> +    config
>> +});
>> +
>> +const PREPARED_CONF_FILE: &str = pdm_buildcfg::configdir!("/autoinst/prepared.cfg");
>> +const PREPARED_LOCK_FILE: &str = pdm_buildcfg::configdir!("/autoinst/.prepared.lock");
>> +
>> +// Information about installations themselves are not configuration files and thus are saved
>> +// as JSON.
>> +const INSTALLATIONS_CONF_FILE: &str = pdm_buildcfg::configdir!("/autoinst/installations.json");
>> +const INSTALLATIONS_LOCK_FILE: &str = pdm_buildcfg::configdir!("/autoinst/.installations.lock");
>
> Any particular reason why you put `installations.json` in the config
> directory and not in /var/lib/proxmox-datacenter-manager/... as 'state'?
> To me this more seems like application state rather than configuration.

Good point, they should definitely live there! I'll change it for v2.

Honestly, I just wasn't aware that we already have and use
/var/lib/proxmox-datacenter-manager :^)


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