From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 02F4E1FF139 for ; Tue, 24 Feb 2026 13:48:57 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 3333A999D; Tue, 24 Feb 2026 13:49:50 +0100 (CET) Message-ID: Date: Tue, 24 Feb 2026 13:49:45 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH pve-manager 1/1] sdn: add dry-run view for sdn apply To: pve-devel@lists.proxmox.com References: <20260203160246.353351-1-g.goller@proxmox.com> <20260203160246.353351-23-g.goller@proxmox.com> Content-Language: en-US From: Stefan Hanreich In-Reply-To: <20260203160246.353351-23-g.goller@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.344 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 1.179 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.717 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.236 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: 5H72SNUY2PRFU6VT4MXVJOFVONZQFHID X-Message-ID-Hash: 5H72SNUY2PRFU6VT4MXVJOFVONZQFHID X-MailFrom: s.hanreich@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: The panel is still a bit barebones imo, the diff should at least be in a monospaced font. Colored output would be better, side-by-side comparison really great. I don't think we have anything like that in our ExtJS widget toolkit though - so this would have to be implemented from scratch. Maybe @Dominik as ideas / thoughts on that? On 2/3/26 5:03 PM, Gabriel Goller wrote: > Introduce a new SdnDiffView modal that runs a dry-run and shows the frr > configuration changes which will be made when clicking apply. Now the > user knows which frr config options will be set without needing to apply > the config. > > Signed-off-by: Gabriel Goller > --- > www/manager6/Makefile | 1 + > www/manager6/sdn/SdnDiffView.js | 123 ++++++++++++++++++++++++++++++++ > www/manager6/sdn/StatusView.js | 8 +++ > 3 files changed, 132 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..2de10efd14b6 > --- /dev/null > +++ b/www/manager6/sdn/SdnDiffView.js > @@ -0,0 +1,123 @@ > +Ext.define('PVE.sdn.SdnDiffView', { > + extend: 'Ext.window.Window', > + > + maxWidth: 1000, > + maxHeight: 1000, > + minWidth: 600, > + minHeight: 400, > + scrollable: true, > + modal: true, > + title: gettext('FRR config diff view'), Potentially improve the title here, sounds a bit half-baked imo. 'Pending FRR [configuration] changes' or something like that? Or 'FRR configuration preview'. Some combination of Pending / Preview and FRR configuration... > + > + node: undefined, > + > + viewModel: { > + data: { > + diff: undefined, > + }, > + }, > + > + items: [ > + { > + xtype: 'displayfield', > + padding: 10, > + scrollable: true, > + bind: { > + value: '{diff}', > + }, > + }, > + ], > + buttons: [ > + { > + handler: function () { > + this.up('window').close(); > + }, > + text: gettext('OK'), 'OK' might give the impression to some users that the config will be applied when pressing the button - maybe close is better here? > + }, > + ], > + > + loadDiff: async function () { > + let me = this; > + > + let req = await Proxmox.Async.api2({ > + url: `/cluster/sdn/dry-apply`, > + 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('diff', ''); > + } else { > + this.getViewModel().set('diff', diff['frr-diff'].replaceAll('\n', '
')); potentially use a 
 tag and escape the returned config instead of
replacing new lines with a 
?

> +                }
> +            })
> +            .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'),

Dry-run might be misleading as well, since currently it is limited to
the FRR configuration only. Although it might not be too hard to extend
this to ifupdown2 - maybe that could be an additional quick win as
follow-up?

That reminds me, that we should also implement rolling back of pending
changes (and potentially previewing the changes there as well) in the UI
- this would play quite nicely with this addition...

> +                        handler: function () {
> +                            Ext.create('PVE.sdn.SdnDiffView', {
> +                                autoShow: true,
> +                            });
> +                        },
> +                    },
>                  ],
>                  viewConfig: {
>                      trackOver: false,