public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH] pve-rs: fabrics: handle missing running-config and no fabrics config
@ 2025-11-14 10:30 Gabriel Goller
  2025-11-14 10:49 ` Stoiko Ivanov
  2025-11-14 11:01 ` Thomas Lamprecht
  0 siblings, 2 replies; 4+ messages in thread
From: Gabriel Goller @ 2025-11-14 10:30 UTC (permalink / raw)
  To: pve-devel; +Cc: Thomas Lamprecht

Correctly handle if there is a missing `/etc/pve/sdn/.running-config`
file. This happens, e.g. on a clean installation without any sdn "apply"
activation. Also handle a config where there is no fabric configured.
The `status()` functions gets called every few seconds to populate the
network resources in pve, so don't return an error if there is no
running-config or no fabric config. The other functions (neighbors,
interfaces, routes) only get called by the api, so we can return an
error on those.

Reported-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Reported-by: Stoiko Ivanov <s.ivanov@proxmox.com>
Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
 pve-rs/src/bindings/sdn/fabrics.rs | 36 ++++++++++++++++++++++++------
 pve-rs/src/sdn/status.rs           | 10 ++++++---
 2 files changed, 36 insertions(+), 10 deletions(-)

diff --git a/pve-rs/src/bindings/sdn/fabrics.rs b/pve-rs/src/bindings/sdn/fabrics.rs
index c9e5cf526c77..54606e7b6bb9 100644
--- a/pve-rs/src/bindings/sdn/fabrics.rs
+++ b/pve-rs/src/bindings/sdn/fabrics.rs
@@ -595,14 +595,21 @@ pub mod pve_rs_sdn_fabrics {
     /// Read and parse the running-config and get the fabrics section
     ///
     /// This will return a valid FabricConfig. Note that we read the file manually and not through
-    /// the cluster filesystem as with perl, so this will be slower.
-    fn get_fabrics_config() -> Result<Valid<FabricConfig>, anyhow::Error> {
+    /// the cluster filesystem as with perl, so this will be slower. If the running config exists,
+    /// but no fabric is configured, `Ok(None)` is returned.
+    fn get_fabrics_config() -> Result<Option<Valid<FabricConfig>>, anyhow::Error> {
         let raw_config = std::fs::read_to_string("/etc/pve/sdn/.running-config")?;
         let running_config: RunningConfig =
             serde_json::from_str(&raw_config).with_context(|| "error parsing running-config")?;
-        let section_config = SectionConfigData::from_iter(running_config.fabrics.ids);
-        FabricConfig::from_section_config(section_config)
-            .with_context(|| "error converting section config to fabricconfig")
+        let Some(fabrics) = running_config.fabrics else {
+            return Ok(None);
+        };
+        let section_config = SectionConfigData::from_iter(fabrics.ids);
+        Some(
+            FabricConfig::from_section_config(section_config)
+                .with_context(|| "error converting section config to fabricconfig"),
+        )
+        .transpose()
     }
 
     /// Get the routes that have been learned and distributed by this specific fabric on this node.
@@ -614,6 +621,9 @@ pub mod pve_rs_sdn_fabrics {
     fn routes(fabric_id: FabricId) -> Result<Vec<status::RouteStatus>, Error> {
         // Read fabric config to get protocol of fabric
         let config = get_fabrics_config()?;
+        let Some(config) = config else {
+            anyhow::bail!("no fabrics configured");
+        };
 
         let fabric = config.get_fabric(&fabric_id)?;
         match fabric {
@@ -679,6 +689,9 @@ pub mod pve_rs_sdn_fabrics {
     fn neighbors(fabric_id: FabricId) -> Result<status::NeighborStatus, Error> {
         // Read fabric config to get protocol of fabric
         let config = get_fabrics_config()?;
+        let Some(config) = config else {
+            anyhow::bail!("no fabrics configured");
+        };
 
         let fabric = config.get_fabric(&fabric_id)?;
 
@@ -734,6 +747,9 @@ pub mod pve_rs_sdn_fabrics {
     fn interfaces(fabric_id: FabricId) -> Result<status::InterfaceStatus, Error> {
         // Read fabric config to get protocol of fabric
         let config = get_fabrics_config()?;
+        let Some(config) = config else {
+            anyhow::bail!("no fabrics configured");
+        };
 
         let fabric = config.get_fabric(&fabric_id)?;
 
@@ -789,6 +805,14 @@ pub mod pve_rs_sdn_fabrics {
     /// config. If there are, show "ok" as status, otherwise "not ok".
     #[export]
     fn status() -> Result<HashMap<FabricId, status::Status>, Error> {
+        // This gets called every few seconds from pve to get the status of the fabrics into the
+        // network resources. It is possible that the .running-config doesn't exist or no fabric is
+        // configured -- in that case just return nothing.
+        let config = get_fabrics_config();
+        let Ok(Some(config)) = config else {
+            return Ok(HashMap::new());
+        };
+
         let openfabric_ipv4_routes_string = String::from_utf8(
             Command::new("sh")
                 .args(["-c", "vtysh -c 'show ip route openfabric json'"])
@@ -831,8 +855,6 @@ pub mod pve_rs_sdn_fabrics {
                 .with_context(|| "error parsing ospf routes")?
         };
 
-        let config = get_fabrics_config()?;
-
         let route_status = status::RoutesParsed {
             openfabric: openfabric_routes,
             ospf: ospf_routes,
diff --git a/pve-rs/src/sdn/status.rs b/pve-rs/src/sdn/status.rs
index 78b55502b837..e1e336297ac9 100644
--- a/pve-rs/src/sdn/status.rs
+++ b/pve-rs/src/sdn/status.rs
@@ -178,7 +178,7 @@ pub struct RoutesParsed {
 /// Config used to parse the fabric part of the running-config
 #[derive(Deserialize)]
 pub struct RunningConfig {
-    pub fabrics: FabricsRunningConfig,
+    pub fabrics: Option<FabricsRunningConfig>,
 }
 
 /// Map of ids for all the fabrics in the running-config
@@ -647,7 +647,9 @@ mod tests {
 
         let running_config: RunningConfig =
             serde_json::from_str(raw_config).expect("error parsing running-config");
-        let section_config = SectionConfigData::from_iter(running_config.fabrics.ids);
+        let section_config = SectionConfigData::from_iter(
+            running_config.fabrics.expect("no fabrics configured").ids,
+        );
         FabricConfig::from_section_config(section_config)
             .expect("error converting section config to fabricconfig")
     }
@@ -695,7 +697,9 @@ mod tests {
             "#;
         let running_config: RunningConfig =
             serde_json::from_str(raw_config).expect("error parsing running-config");
-        let section_config = SectionConfigData::from_iter(running_config.fabrics.ids);
+        let section_config = SectionConfigData::from_iter(
+            running_config.fabrics.expect("no fabrics configured").ids,
+        );
         FabricConfig::from_section_config(section_config)
             .expect("error converting section config to fabricconfig")
     }
-- 
2.47.3



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


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

* Re: [pve-devel] [PATCH] pve-rs: fabrics: handle missing running-config and no fabrics config
  2025-11-14 10:30 [pve-devel] [PATCH] pve-rs: fabrics: handle missing running-config and no fabrics config Gabriel Goller
@ 2025-11-14 10:49 ` Stoiko Ivanov
  2025-11-14 11:02   ` Gabriel Goller
  2025-11-14 11:01 ` Thomas Lamprecht
  1 sibling, 1 reply; 4+ messages in thread
From: Stoiko Ivanov @ 2025-11-14 10:49 UTC (permalink / raw)
  To: Gabriel Goller; +Cc: pve-devel, Thomas Lamprecht

Thanks for tackling this so quickly!

nit: a subject-prefix for the repository is missing.

As I noticed the messages from pvestatd in my journal - I gave this a
quick spin - it does fix the
```
Nov 14 11:36:26 rosa pvestatd[4474]: network status update error: error parsing running-config: missing field `fabrics` at line 1 column 97
```
logspam for me. I quickly looked through the code and to me it looks like
it does what it says on the tin.

consider this:
Tested-by: Stoiko Ivanov <s.ivanov@proxmox.com>
Reviewed-by: Stoiko Ivanov <s.ivanov@proxmox.com>

On Fri, 14 Nov 2025 11:30:09 +0100
Gabriel Goller <g.goller@proxmox.com> wrote:

> Correctly handle if there is a missing `/etc/pve/sdn/.running-config`
> file. This happens, e.g. on a clean installation without any sdn "apply"
> activation. Also handle a config where there is no fabric configured.
> The `status()` functions gets called every few seconds to populate the
> network resources in pve, so don't return an error if there is no
> running-config or no fabric config. The other functions (neighbors,
> interfaces, routes) only get called by the api, so we can return an
> error on those.
> 
> Reported-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
> Reported-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
> ---
>  pve-rs/src/bindings/sdn/fabrics.rs | 36 ++++++++++++++++++++++++------
>  pve-rs/src/sdn/status.rs           | 10 ++++++---
>  2 files changed, 36 insertions(+), 10 deletions(-)
> 
> diff --git a/pve-rs/src/bindings/sdn/fabrics.rs b/pve-rs/src/bindings/sdn/fabrics.rs
> index c9e5cf526c77..54606e7b6bb9 100644
> --- a/pve-rs/src/bindings/sdn/fabrics.rs
> +++ b/pve-rs/src/bindings/sdn/fabrics.rs
> @@ -595,14 +595,21 @@ pub mod pve_rs_sdn_fabrics {
>      /// Read and parse the running-config and get the fabrics section
>      ///
>      /// This will return a valid FabricConfig. Note that we read the file manually and not through
> -    /// the cluster filesystem as with perl, so this will be slower.
> -    fn get_fabrics_config() -> Result<Valid<FabricConfig>, anyhow::Error> {
> +    /// the cluster filesystem as with perl, so this will be slower. If the running config exists,
> +    /// but no fabric is configured, `Ok(None)` is returned.
> +    fn get_fabrics_config() -> Result<Option<Valid<FabricConfig>>, anyhow::Error> {
>          let raw_config = std::fs::read_to_string("/etc/pve/sdn/.running-config")?;
>          let running_config: RunningConfig =
>              serde_json::from_str(&raw_config).with_context(|| "error parsing running-config")?;
> -        let section_config = SectionConfigData::from_iter(running_config.fabrics.ids);
> -        FabricConfig::from_section_config(section_config)
> -            .with_context(|| "error converting section config to fabricconfig")
> +        let Some(fabrics) = running_config.fabrics else {
> +            return Ok(None);
> +        };
> +        let section_config = SectionConfigData::from_iter(fabrics.ids);
> +        Some(
> +            FabricConfig::from_section_config(section_config)
> +                .with_context(|| "error converting section config to fabricconfig"),
> +        )
> +        .transpose()
>      }
>  
>      /// Get the routes that have been learned and distributed by this specific fabric on this node.
> @@ -614,6 +621,9 @@ pub mod pve_rs_sdn_fabrics {
>      fn routes(fabric_id: FabricId) -> Result<Vec<status::RouteStatus>, Error> {
>          // Read fabric config to get protocol of fabric
>          let config = get_fabrics_config()?;
> +        let Some(config) = config else {
> +            anyhow::bail!("no fabrics configured");
> +        };
>  
>          let fabric = config.get_fabric(&fabric_id)?;
>          match fabric {
> @@ -679,6 +689,9 @@ pub mod pve_rs_sdn_fabrics {
>      fn neighbors(fabric_id: FabricId) -> Result<status::NeighborStatus, Error> {
>          // Read fabric config to get protocol of fabric
>          let config = get_fabrics_config()?;
> +        let Some(config) = config else {
> +            anyhow::bail!("no fabrics configured");
> +        };
>  
>          let fabric = config.get_fabric(&fabric_id)?;
>  
> @@ -734,6 +747,9 @@ pub mod pve_rs_sdn_fabrics {
>      fn interfaces(fabric_id: FabricId) -> Result<status::InterfaceStatus, Error> {
>          // Read fabric config to get protocol of fabric
>          let config = get_fabrics_config()?;
> +        let Some(config) = config else {
> +            anyhow::bail!("no fabrics configured");
> +        };
>  
>          let fabric = config.get_fabric(&fabric_id)?;
>  
> @@ -789,6 +805,14 @@ pub mod pve_rs_sdn_fabrics {
>      /// config. If there are, show "ok" as status, otherwise "not ok".
>      #[export]
>      fn status() -> Result<HashMap<FabricId, status::Status>, Error> {
> +        // This gets called every few seconds from pve to get the status of the fabrics into the
> +        // network resources. It is possible that the .running-config doesn't exist or no fabric is
> +        // configured -- in that case just return nothing.
> +        let config = get_fabrics_config();
> +        let Ok(Some(config)) = config else {
> +            return Ok(HashMap::new());
> +        };
> +
>          let openfabric_ipv4_routes_string = String::from_utf8(
>              Command::new("sh")
>                  .args(["-c", "vtysh -c 'show ip route openfabric json'"])
> @@ -831,8 +855,6 @@ pub mod pve_rs_sdn_fabrics {
>                  .with_context(|| "error parsing ospf routes")?
>          };
>  
> -        let config = get_fabrics_config()?;
> -
>          let route_status = status::RoutesParsed {
>              openfabric: openfabric_routes,
>              ospf: ospf_routes,
> diff --git a/pve-rs/src/sdn/status.rs b/pve-rs/src/sdn/status.rs
> index 78b55502b837..e1e336297ac9 100644
> --- a/pve-rs/src/sdn/status.rs
> +++ b/pve-rs/src/sdn/status.rs
> @@ -178,7 +178,7 @@ pub struct RoutesParsed {
>  /// Config used to parse the fabric part of the running-config
>  #[derive(Deserialize)]
>  pub struct RunningConfig {
> -    pub fabrics: FabricsRunningConfig,
> +    pub fabrics: Option<FabricsRunningConfig>,
>  }
>  
>  /// Map of ids for all the fabrics in the running-config
> @@ -647,7 +647,9 @@ mod tests {
>  
>          let running_config: RunningConfig =
>              serde_json::from_str(raw_config).expect("error parsing running-config");
> -        let section_config = SectionConfigData::from_iter(running_config.fabrics.ids);
> +        let section_config = SectionConfigData::from_iter(
> +            running_config.fabrics.expect("no fabrics configured").ids,
> +        );
>          FabricConfig::from_section_config(section_config)
>              .expect("error converting section config to fabricconfig")
>      }
> @@ -695,7 +697,9 @@ mod tests {
>              "#;
>          let running_config: RunningConfig =
>              serde_json::from_str(raw_config).expect("error parsing running-config");
> -        let section_config = SectionConfigData::from_iter(running_config.fabrics.ids);
> +        let section_config = SectionConfigData::from_iter(
> +            running_config.fabrics.expect("no fabrics configured").ids,
> +        );
>          FabricConfig::from_section_config(section_config)
>              .expect("error converting section config to fabricconfig")
>      }



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


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

* Re: [pve-devel] [PATCH] pve-rs: fabrics: handle missing running-config and no fabrics config
  2025-11-14 10:30 [pve-devel] [PATCH] pve-rs: fabrics: handle missing running-config and no fabrics config Gabriel Goller
  2025-11-14 10:49 ` Stoiko Ivanov
@ 2025-11-14 11:01 ` Thomas Lamprecht
  1 sibling, 0 replies; 4+ messages in thread
From: Thomas Lamprecht @ 2025-11-14 11:01 UTC (permalink / raw)
  To: pve-devel, Gabriel Goller

On Fri, 14 Nov 2025 11:30:09 +0100, Gabriel Goller wrote:
> Correctly handle if there is a missing `/etc/pve/sdn/.running-config`
> file. This happens, e.g. on a clean installation without any sdn "apply"
> activation. Also handle a config where there is no fabric configured.
> The `status()` functions gets called every few seconds to populate the
> network resources in pve, so don't return an error if there is no
> running-config or no fabric config. The other functions (neighbors,
> interfaces, routes) only get called by the api, so we can return an
> error on those.
> 
> [...]

Applied, thanks!

[1/1] pve-rs: fabrics: handle missing running-config and no fabrics config
      commit: 7d68ea0b9a13ba37bca42cb9fe3ad180b3251c1a


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


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

* Re: [pve-devel] [PATCH] pve-rs: fabrics: handle missing running-config and no fabrics config
  2025-11-14 10:49 ` Stoiko Ivanov
@ 2025-11-14 11:02   ` Gabriel Goller
  0 siblings, 0 replies; 4+ messages in thread
From: Gabriel Goller @ 2025-11-14 11:02 UTC (permalink / raw)
  To: Stoiko Ivanov; +Cc: pve-devel, Thomas Lamprecht

On 14.11.2025 11:49, Stoiko Ivanov wrote:
> Thanks for tackling this so quickly!
> 
> nit: a subject-prefix for the repository is missing.

Ah, sorry about that -- this is proxmox-perl-rs obviously :)

> As I noticed the messages from pvestatd in my journal - I gave this a
> quick spin - it does fix the
> ```
> Nov 14 11:36:26 rosa pvestatd[4474]: network status update error: error parsing running-config: missing field `fabrics` at line 1 column 97
> ```
> logspam for me. I quickly looked through the code and to me it looks like
> it does what it says on the tin.
> 
> consider this:
> Tested-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> Reviewed-by: Stoiko Ivanov <s.ivanov@proxmox.com>

Thanks!


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


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

end of thread, other threads:[~2025-11-14 11:01 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-11-14 10:30 [pve-devel] [PATCH] pve-rs: fabrics: handle missing running-config and no fabrics config Gabriel Goller
2025-11-14 10:49 ` Stoiko Ivanov
2025-11-14 11:02   ` Gabriel Goller
2025-11-14 11:01 ` Thomas Lamprecht

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