Re: [PATCH pve-manager 1/1] sdn: add dry-run view for sdn apply

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

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 <g.goller@proxmox.com>
> ---
>  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', '<br>'));

potentially use a <pre> tag and escape the returned config instead of
replacing new lines with a <br>?

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