public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Stefan Hanreich <s.hanreich@proxmox.com>
To: Gabriel Goller <g.goller@proxmox.com>, pve-devel@lists.proxmox.com
Subject: Re: [PATCH pve-manager v3 1/1] sdn: add dry-run diff view for sdn apply
Date: Fri, 6 Mar 2026 10:24:42 +0100	[thread overview]
Message-ID: <5b84e2db-7855-4bca-9c61-dc481e5a6f40@proxmox.com> (raw)
In-Reply-To: <20260305100331.80741-20-g.goller@proxmox.com>

still looks a bit barebones imo, but at least its monospaced now ;)
Maybe we could make the '{FRR,Interfaces} Config' headings rows, instead
of taking up a whole column with lots of unused whitespace.
I think it should also show a nicer message when there are no changes -
stating explicitly that nothing changed - instead of just an empty panel.

Not a big blocker, since we can always improve it in a follow-up patch.


On 3/5/26 11:04 AM, Gabriel Goller wrote:
> Introduce a new SdnDiffView modal that runs a dry-run and shows the frr
> and ifupdown2 configuration changes which will be made when clicking
> apply. Now the user knows which config options will be set without
> needing to apply the config.
> 
> Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
> ---
>  www/manager6/Makefile           |   1 +
>  www/manager6/sdn/SdnDiffView.js | 139 ++++++++++++++++++++++++++++++++
>  www/manager6/sdn/StatusView.js  |   8 ++
>  3 files changed, 148 insertions(+)
>  create mode 100644 www/manager6/sdn/SdnDiffView.js
> 
> diff --git a/www/manager6/Makefile b/www/manager6/Makefile
> index 4558d53e54be..da602523b27a 100644
> --- a/www/manager6/Makefile
> +++ b/www/manager6/Makefile
> @@ -286,6 +286,7 @@ JSSRC= 							\
>  	sdn/ControllerView.js				\
>  	sdn/Status.js					\
>  	sdn/StatusView.js				\
> +	sdn/SdnDiffView.js				\
>  	sdn/VnetEdit.js					\
>  	sdn/VnetView.js					\
>  	sdn/VnetACLView.js				\
> diff --git a/www/manager6/sdn/SdnDiffView.js b/www/manager6/sdn/SdnDiffView.js
> new file mode 100644
> index 000000000000..bfff3675950d
> --- /dev/null
> +++ b/www/manager6/sdn/SdnDiffView.js
> @@ -0,0 +1,139 @@
> +Ext.define('PVE.sdn.SdnDiffView', {
> +    extend: 'Ext.window.Window',
> +
> +    width: 800,
> +    height: 900,
> +
> +    scrollable: true,
> +    modal: true,
> +    title: gettext('Pending SDN configuration changes'),
> +
> +    node: undefined,
> +
> +    viewModel: {
> +        data: {
> +            frr_diff: undefined,
> +            interfaces_diff: undefined,
> +        },
> +    },
> +
> +    items: [
> +        {
> +            xtype: 'displayfield',
> +            padding: 10,
> +            fieldLabel: gettext('FRR config'),
> +            bind: {
> +                value: '{frr_diff}',
> +            },
> +        },
> +        {
> +            xtype: 'displayfield',
> +            padding: 10,
> +            fieldLabel: gettext('Interfaces config'),
> +            bind: {
> +                value: '{interfaces_diff}',
> +            },
> +        },
> +    ],
> +    buttons: [
> +        {
> +            handler: function () {
> +                this.up('window').close();
> +            },
> +            text: gettext('Close'),
> +        },
> +    ],
> +
> +    loadDiff: async function () {
> +        let me = this;
> +
> +        let req = await Proxmox.Async.api2({
> +            url: `/cluster/sdn/dry-run`,
> +            params: { node: me.node },
> +            method: 'PUT',
> +        });
> +
> +        return req.result.data;
> +    },
> +
> +    load: function () {
> +        let me = this;
> +
> +        me.setLoading('fetching node diff');
> +
> +        me.loadDiff()
> +            .catch(Proxmox.Utils.alertResponseFailure)
> +            .then((diff) => {
> +                if (diff['frr-diff'] === null) {
> +                    this.getViewModel().set('frr_diff', '');
> +                } else {
> +                    this.getViewModel().set('frr_diff', '<pre>' + diff['frr-diff'] + '</pre>');
> +                }
> +                if (diff['interfaces-diff'] === null) {
> +                    this.getViewModel().set('interfaces_diff', '');
> +                } else {
> +                    this.getViewModel().set(
> +                        'interfaces_diff',
> +                        '<pre>' + diff['interfaces-diff'] + '</pre>',
> +                    );
> +                }
> +            })
> +            .finally(() => {
> +                me.setLoading(false);
> +            });
> +    },
> +
> +    getNodeSelector: function () {
> +        let me = this;
> +
> +        return Ext.create('PVE.form.NodeSelector', {
> +            xtype: 'pveNodeSelector',
> +            reference: 'nodeselector',
> +            fieldLabel: gettext('Node'),
> +            padding: 10,
> +            labelWidth: 120,
> +            name: 'node',
> +            allowBlank: false,
> +            listeners: {
> +                change: function (f, value) {
> +                    me.node = value;
> +                    me.load();
> +                },
> +            },
> +            listConfig: {
> +                columns: [
> +                    {
> +                        header: gettext('Node'),
> +                        dataIndex: 'node',
> +                        sortable: true,
> +                        hideable: false,
> +                        flex: 1,
> +                    },
> +                ],
> +            },
> +            store: {
> +                fields: ['node'],
> +                proxy: {
> +                    type: 'proxmox',
> +                    url: '/api2/json/nodes',
> +                },
> +                sorters: [
> +                    {
> +                        property: 'node',
> +                        direction: 'ASC',
> +                    },
> +                ],
> +            },
> +        });
> +    },
> +
> +    initComponent: function () {
> +        let me = this;
> +
> +        me.nodeSelector = me.getNodeSelector();
> +
> +        me.items = [me.nodeSelector, ...me.items];
> +
> +        me.callParent();
> +    },
> +});
> diff --git a/www/manager6/sdn/StatusView.js b/www/manager6/sdn/StatusView.js
> index fbc712c6cf6b..fada50411e91 100644
> --- a/www/manager6/sdn/StatusView.js
> +++ b/www/manager6/sdn/StatusView.js
> @@ -69,6 +69,14 @@ Ext.define(
>                              });
>                          },
>                      },
> +                    {
> +                        text: gettext('Dry-Run'),
> +                        handler: function () {
> +                            Ext.create('PVE.sdn.SdnDiffView', {
> +                                autoShow: true,
> +                            });
> +                        },
> +                    },
>                  ],
>                  viewConfig: {
>                      trackOver: false,





  reply	other threads:[~2026-03-06  9:24 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-05 10:03 [PATCH manager/network/proxmox{-ve-rs,-perl-rs} v3 00/19] Generate frr config using jinja templates and rust types Gabriel Goller
2026-03-05 10:03 ` [PATCH proxmox-ve-rs v3 1/8] ve-config: firewall: cargo fmt Gabriel Goller
2026-03-05 10:03 ` [PATCH proxmox-ve-rs v3 2/8] frr: add proxmox-frr-templates package that contains templates Gabriel Goller
2026-03-05 10:03 ` [PATCH proxmox-ve-rs v3 3/8] ve-config: remove FrrConfigBuilder struct Gabriel Goller
2026-03-05 10:03 ` [PATCH proxmox-ve-rs v3 4/8] sdn-types: support variable-length NET identifier Gabriel Goller
2026-03-05 10:03 ` [PATCH proxmox-ve-rs v3 5/8] frr: add template serializer and serialize fabrics using templates Gabriel Goller
2026-03-05 10:03 ` [PATCH proxmox-ve-rs v3 6/8] frr: add isis configuration and templates Gabriel Goller
2026-03-05 10:03 ` [PATCH proxmox-ve-rs v3 7/8] frr: support custom frr configuration lines Gabriel Goller
2026-03-05 10:03 ` [PATCH proxmox-ve-rs v3 8/8] frr: add bgp support with templates and serialization Gabriel Goller
2026-03-05 10:03 ` [PATCH proxmox-perl-rs v3 1/1] sdn: add function to generate the frr config for all daemons Gabriel Goller
2026-03-05 10:03 ` [PATCH pve-network v3 1/9] tests: use Test::Differences to make test assertions Gabriel Goller
2026-03-05 10:03 ` [PATCH pve-network v3 2/9] test: add test for frr.conf.local merging Gabriel Goller
2026-03-06  9:27   ` Stefan Hanreich
2026-03-06 10:51     ` Gabriel Goller
2026-03-05 10:03 ` [PATCH pve-network v3 3/9] test: bgp: add some various integration tests Gabriel Goller
2026-03-05 10:03 ` [PATCH pve-network v3 4/9] sdn: write structured frr config that can be rendered using templates Gabriel Goller
2026-03-05 10:03 ` [PATCH pve-network v3 5/9] sdn: remove duplicate comment line '!' in frr config Gabriel Goller
2026-03-05 10:03 ` [PATCH pve-network v3 6/9] tests: rearrange some statements in the " Gabriel Goller
2026-03-05 10:03 ` [PATCH pve-network v3 7/9] sdn: adjust frr.conf.local merging to rust template types Gabriel Goller
2026-03-06 10:35   ` Stefan Hanreich
2026-03-06 10:54     ` Gabriel Goller
2026-03-05 10:03 ` [PATCH pve-network v3 8/9] test: adjust frr_local_merge test for new template generation Gabriel Goller
2026-03-05 10:03 ` [PATCH pve-network v3 9/9] api: add dry-run endpoint for sdn apply to preview changes Gabriel Goller
2026-03-06  9:24   ` Stefan Hanreich
2026-03-06 12:08     ` Gabriel Goller
2026-03-05 10:03 ` [PATCH pve-manager v3 1/1] sdn: add dry-run diff view for sdn apply Gabriel Goller
2026-03-06  9:24   ` Stefan Hanreich [this message]
2026-03-06 12:37     ` Gabriel Goller
2026-03-06 12:58 ` [PATCH manager/network/proxmox{-ve-rs,-perl-rs} v3 00/19] Generate frr config using jinja templates and rust types Gabriel Goller

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=5b84e2db-7855-4bca-9c61-dc481e5a6f40@proxmox.com \
    --to=s.hanreich@proxmox.com \
    --cc=g.goller@proxmox.com \
    --cc=pve-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