Re: [pdm-devel] [PATCH proxmox-datacenter-manager 3/5] ui: add sdn status report to dashboard

[Stefan Hanreich <s.hanreich@proxmox.com>]

thanks! will address both in a new version

On 9/9/25 3:10 PM, Dominik Csapak wrote:
> two comments inline
> 
> On 9/9/25 12:08 PM, Stefan Hanreich wrote:
>> This also includes support for external links, searching and
>> navigating to the dedicated SDN overview. For now, add it to the task
>> summary row, since there are issues with only showing one element in a
>> row due to the CSS rules. While this breaks a little bit with the
>> current grouping, the widget is quite small so it would look weird in
>> a single row and we can always decide to move it around as soon as we
>> add more elements to the dashboard.
>>
>> Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
>> ---
>>   ui/src/dashboard/mod.rs            |  17 +++-
>>   ui/src/dashboard/sdn_zone_panel.rs | 155 +++++++++++++++++++++++++++++
>>   ui/src/lib.rs                      |  14 ++-
>>   ui/src/pve/utils.rs                |  16 ++-
>>   ui/src/renderer.rs                 |   4 +-
>>   5 files changed, 199 insertions(+), 7 deletions(-)
>>   create mode 100644 ui/src/dashboard/sdn_zone_panel.rs
>>
>> diff --git a/ui/src/dashboard/mod.rs b/ui/src/dashboard/mod.rs
>> index 0659dc0..626a6bf 100644
>> --- a/ui/src/dashboard/mod.rs
>> +++ b/ui/src/dashboard/mod.rs
>> @@ -25,7 +25,7 @@ use pwt::{
>>   };
>>     use pdm_api_types::{
>> -    resource::{GuestStatusCount, NodeStatusCount, ResourcesStatus},
>> +    resource::{GuestStatusCount, NodeStatusCount, ResourcesStatus,
>> SdnZoneCount},
>>       TaskStatistics,
>>   };
>>   use pdm_client::types::TopEntity;
>> @@ -46,6 +46,9 @@ use remote_panel::RemotePanel;
>>   mod guest_panel;
>>   use guest_panel::GuestPanel;
>>   +mod sdn_zone_panel;
>> +use sdn_zone_panel::SdnZonePanel;
>> +
>>   mod status_row;
>>   use status_row::DashboardStatusRow;
>>   @@ -242,6 +245,15 @@ impl PdmDashboard {
>>               ))
>>       }
>>   +    fn create_sdn_panel(&self, status: &SdnZoneCount) -> Panel {
>> +        Panel::new()
>> +            .flex(1.0)
>> +            .width(200)
>> +            .title(self.create_title_with_icon("sdn", tr!("SDN Zones")))
>> +            .border(true)
>> +            .with_child(SdnZonePanel::new((!
>> self.loading).then_some(status.clone())))
>> +    }
>> +
>>       fn create_task_summary_panel(
>>           &self,
>>           statistics: &StatisticsOptions,
>> @@ -620,7 +632,8 @@ impl Component for PdmDashboard {
>>                       .class(pwt::css::Flex::Fill)
>>                       .class(FlexWrap::Wrap)
>>                       .with_child(self.create_task_summary_panel(&self.statistics, None))
>> -
>>                     .with_child(self.create_task_summary_panel(&self.statistics, Some(5))),
>> +                    .with_child(self.create_task_summary_panel(&self.statistics, Some(5)))
>> +                    .with_child(self.create_sdn_panel(&self.status.sdn_zones)),
>>               );
>>             Panel::new()
>> diff --git a/ui/src/dashboard/sdn_zone_panel.rs b/ui/src/dashboard/
>> sdn_zone_panel.rs
>> new file mode 100644
>> index 0000000..bcac36b
>> --- /dev/null
>> +++ b/ui/src/dashboard/sdn_zone_panel.rs
>> @@ -0,0 +1,155 @@
>> +use std::rc::Rc;
>> +
>> +use pdm_api_types::resource::{ResourceType, SdnStatus, SdnZoneCount};
>> +use pdm_search::{Search, SearchTerm};
>> +use pwt::{
>> +    css::{self, FontColor, TextAlign},
>> +    prelude::*,
>> +    widget::{Container, Fa, List, ListTile},
>> +};
>> +use yew::{
>> +    virtual_dom::{VComp, VNode},
>> +    Properties,
>> +};
>> +
>> +use crate::search_provider::get_search_provider;
>> +
>> +use super::loading_column;
>> +
>> +#[derive(PartialEq, Clone, Properties)]
>> +pub struct SdnZonePanel {
>> +    status: Option<SdnZoneCount>,
>> +}
>> +
>> +impl SdnZonePanel {
>> +    pub fn new(status: Option<SdnZoneCount>) -> Self {
>> +        yew::props!(Self { status })
>> +    }
>> +}
>> +
>> +impl From<SdnZonePanel> for VNode {
>> +    fn from(value: SdnZonePanel) -> Self {
>> +        let comp =
>> VComp::new::<SdnZonePanelComponent>(Rc::new(value), None);
>> +        VNode::from(comp)
>> +    }
>> +}
>> +
>> +#[derive(PartialEq, Clone)]
>> +pub enum StatusRow {
>> +    State(SdnStatus, u64),
>> +    All(u64),
>> +}
>> +
>> +impl StatusRow {
>> +    fn icon(&self) -> Fa {
>> +        let (icon, color) = match self {
>> +            Self::All(_) => ("th", None),
>> +            Self::State(SdnStatus::Available, _) => ("check",
>> Some(FontColor::Success)),
>> +            Self::State(SdnStatus::Error, _) => ("times-circle",
>> Some(FontColor::Error)),
>> +            Self::State(SdnStatus::Unknown, _) => ("question", None),
>> +        };
>> +
>> +        let mut icon = Fa::new(icon);
>> +
>> +        if let Some(color) = color {
>> +            icon = icon.class(color);
>> +        }
>> +
>> +        icon
>> +    }
>> +}
>> +
>> +pub struct SdnZonePanelComponent {}
>> +
>> +impl yew::Component for SdnZonePanelComponent {
>> +    type Message = Search;
>> +    type Properties = SdnZonePanel;
>> +
>> +    fn create(_ctx: &yew::Context<Self>) -> Self {
>> +        Self {}
>> +    }
>> +
>> +    fn update(&mut self, ctx: &Context<Self>, msg: Self::Message) ->
>> bool {
>> +        if let Some(provider) = get_search_provider(ctx) {
>> +            provider.search(msg);
>> +        }
>> +
>> +        false
>> +    }
>> +
>> +    fn view(&self, ctx: &yew::Context<Self>) -> yew::Html {
>> +        let props = ctx.props();
>> +
>> +        let Some(status) = &props.status else {
>> +            return loading_column().into();
>> +        };
>> +
>> +        let data = vec![
>> +            StatusRow::State(SdnStatus::Available, status.available),
>> +            StatusRow::State(SdnStatus::Error, status.error),
>> +            StatusRow::All(status.available + status.error +
>> status.unknown),
>> +        ];
>> +
>> +        let tiles: Vec<_> = data
>> +            .into_iter()
>> +            .filter_map(|row| create_list_tile(ctx.link(), row))
>> +            .collect();
>> +
>> +        let list = List::new(tiles.len() as u64, move |idx: u64| {
>> +            tiles[idx as usize].clone()
>> +        })
>> +        .padding(4)
>> +        .class(css::Flex::Fill)
>> +        .grid_template_columns("auto auto 1fr auto");
>> +
>> +        list.into()
>> +    }
>> +}
>> +
>> +fn create_list_tile(
>> +    link: &html::Scope<SdnZonePanelComponent>,
>> +    status_row: StatusRow,
>> +) -> Option<ListTile> {
>> +    let (icon, status, count) = match status_row {
>> +        StatusRow::State(sdn_status, count) => (status_row.icon(),
>> Some(sdn_status), count),
>> +        StatusRow::All(count) => (status_row.icon(), None, count),
>> +    };
>> +
>> +    let name = status
>> +        .map(|status| status.to_string())
>> +        .unwrap_or_else(|| "All".to_string());
>> +
>> +    Some(
>> +        ListTile::new()
>> +            .tabindex(0)
>> +            .interactive(true)
>> +            .with_child(icon)
>> +            .with_child(Container::new().padding_x(2).with_child(name))
>> +            .with_child(
>> +                Container::new()
>> +                    .class(TextAlign::Right)
>> +                    .padding_end(2)
>> +                    .with_child(count),
>> +            )
>> +            .with_child(Fa::new("search"))
>> +            .onclick(link.callback(move |_|
>> create_sdn_zone_search_term(status)))
>> +            .onkeydown(link.batch_callback(
>> +                move |event: KeyboardEvent| match event.key().as_str() {
>> +                    "Enter" | " " =>
>> Some(create_sdn_zone_search_term(status)),
>> +                    _ => None,
>> +                },
>> +            )),
>> +    )
>> +}
>> +
>> +fn create_sdn_zone_search_term(status: Option<SdnStatus>) -> Search {
>> +    let resource_type: ResourceType = ResourceType::PveSdnZone;
>> +
>> +    let mut terms = vec!
>> [SearchTerm::new(resource_type.as_str()).category(Some("type"))];
>> +
>> +    if let Some(status) = status {
>> +       
>> terms.push(SearchTerm::new(status.to_string()).category(Some("status")));
>> +    }
>> +
>> +    Search::with_terms(terms)
>> +}
>> diff --git a/ui/src/lib.rs b/ui/src/lib.rs
>> index 5ffbff3..e4bfbb7 100644
>> --- a/ui/src/lib.rs
>> +++ b/ui/src/lib.rs
>> @@ -1,4 +1,4 @@
>> -use pdm_api_types::resource::{PveLxcResource, PveQemuResource};
>> +use pdm_api_types::resource::{PveLxcResource, PveQemuResource,
>> PveSdnResource};
>>   use pdm_client::types::Resource;
>>   use serde::{Deserialize, Serialize};
>>   @@ -151,13 +151,21 @@ pub(crate) fn navigate_to<C: yew::Component>(
>>                       pdm_client::types::Resource::PveStorage(storage)
>> => {
>>                           format!("storage+{}+{}", storage.node,
>> storage.storage)
>>                       }
>> +                   
>> pdm_client::types::Resource::PveSdn(PveSdnResource::Zone(_)) => {
>> +                        "sdn/zones".to_string()
>> +                    }
>>                       pdm_client::types::Resource::PbsDatastore(store)
>> => store.name.clone(),
>>                       // FIXME: implement
>>                       _ => return None,
>>                   })
>>               })
>>               .unwrap_or_default();
>> -        nav.push(&yew_router::AnyRoute::new(format!("/remote-
>> {remote}/{id}")));
>> +
>> +        if let Some(pdm_client::types::Resource::PveSdn(_)) = resource {
>> +            nav.push(&yew_router::AnyRoute::new(format!("/{id}")));
>> +        } else {
>> +            nav.push(&yew_router::AnyRoute::new(format!("/remote-
>> {remote}/{id}")));
>> +        }
> 
> i don't really like the special casing of sdn here. I think it would be
> better, e.g. if the match would return a tuple of the (optional) remote
> and the id

Yeah, makes sense I'll try to adjust it

>>       }
>>   }
>>   @@ -167,7 +175,7 @@ pub(crate) fn get_resource_node(resource:
>> &Resource) -> Option<&str> {
>>           Resource::PveQemu(qemu) => Some(&qemu.node),
>>           Resource::PveLxc(lxc) => Some(&lxc.node),
>>           Resource::PveNode(node) => Some(&node.node),
>> -        Resource::PveSdn(sdn) => Some(&sdn.sdn),
>> +        Resource::PveSdn(sdn) => Some(sdn.node()),
>>           Resource::PbsNode(_) => None,
>>           Resource::PbsDatastore(_) => None,
>>       }
>> diff --git a/ui/src/pve/utils.rs b/ui/src/pve/utils.rs
>> index 7663734..a49205d 100644
>> --- a/ui/src/pve/utils.rs
>> +++ b/ui/src/pve/utils.rs
>> @@ -1,6 +1,7 @@
>>   use anyhow::Error;
>>   use pdm_api_types::resource::{
>> -    PveLxcResource, PveNodeResource, PveQemuResource,
>> PveStorageResource,
>> +    PveLxcResource, PveNodeResource, PveQemuResource,
>> PveStorageResource, SdnStatus,
>> +    SdnZoneResource,
>>   };
>>   use pdm_client::types::{
>>       LxcConfig, LxcConfigMp, LxcConfigRootfs, LxcConfigUnused,
>> PveQmIde, QemuConfig, QemuConfigSata,
>> @@ -88,6 +89,19 @@ pub fn render_node_status_icon(node:
>> &PveNodeResource) -> Container {
>>           .with_child(Fa::from(extra).fixed_width().class("status-icon"))
>>   }
>>   +/// Renders the status icon for a PveNode
> 
> it's not for a PveNode ;)

copy-paste mistake :(

> 
>> +pub fn render_sdn_status_icon(zone: &SdnZoneResource) -> Container {
>> +    let extra = match zone.status {
>> +        SdnStatus::Available => NodeState::Online,
>> +        SdnStatus::Error => NodeState::Offline,
>> +        _ => NodeState::Unknown,
>> +    };
>> +    Container::new()
>> +        .class("pdm-type-icon")
>> +        .with_child(Fa::new("th").fixed_width())
>> +        .with_child(Fa::from(extra).fixed_width().class("status-icon"))
>> +}
>> +
>>   /// Renders the status icon for a PveStorage
>>   pub fn render_storage_status_icon(node: &PveStorageResource) ->
>> Container {
>>       let extra = match node.status.as_str() {
>> diff --git a/ui/src/renderer.rs b/ui/src/renderer.rs
>> index 5ebd9a3..e179cd5 100644
>> --- a/ui/src/renderer.rs
>> +++ b/ui/src/renderer.rs
>> @@ -1,3 +1,4 @@
>> +use pdm_api_types::resource::PveSdnResource;
>>   use pwt::{
>>       css,
>>       prelude::*,
>> @@ -17,7 +18,7 @@ pub fn render_resource_name(resource: &Resource,
>> vmid_first: bool) -> String {
>>           Resource::PveQemu(qemu) =>
>> pve::utils::render_qemu_name(qemu, vmid_first),
>>           Resource::PveLxc(lxc) => pve::utils::render_lxc_name(lxc,
>> vmid_first),
>>           Resource::PveNode(node) => node.node.clone(),
>> -        Resource::PveSdn(sdn) => sdn.sdn.clone(),
>> +        Resource::PveSdn(sdn) => sdn.name().to_string(),

while I'm at it will also amend this change into the earlier commit
since this line is touched twice in the patch series and a remnant of an
earlier version of the Resource types..

>>           Resource::PbsNode(node) => node.name.clone(),
>>           Resource::PbsDatastore(store) => store.name.clone(),
>>       }
>> @@ -43,6 +44,7 @@ pub fn render_status_icon(resource: &Resource) ->
>> Container {
>>           Resource::PveQemu(qemu) =>
>> pve::utils::render_qemu_status_icon(qemu),
>>           Resource::PveLxc(lxc) =>
>> pve::utils::render_lxc_status_icon(lxc),
>>           Resource::PveNode(node) =>
>> pve::utils::render_node_status_icon(node),
>> +        Resource::PveSdn(PveSdnResource::Zone(zone)) =>
>> pve::utils::render_sdn_status_icon(zone),
>>           // FIXME: implement remaining types
>>           _ =>
>> Container::new().with_child(render_resource_icon(resource)),
>>       }
> 



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