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,
next prev parent 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