[PATCH manager 0/3] add opt-in IPMI SEL log tab for nodes

[PATCH manager 0/3] add opt-in IPMI SEL log tab for nodes

[Luca Vornheder]

This series adds a per-node, opt-in view of the IPMI System Event Log
(SEL) to the GUI, as requested in
https://bugzilla.proxmox.com/show_bug.cgi?id=7656.

A new 'IPMI' tab next to the system log of a node lists the entries of
the SEL of the node's local BMC, read in-band via 'ipmitool sel elist',
with reload and export buttons. The feature is off by default and is
enabled per node through a new 'ipmi-sel' node option, either in the
node's options in the GUI or with 'pvenode config set --ipmi-sel 1'.
While it is disabled, the API returns 501 and the tab shows a hint
instead of an error.

Design notes / open questions:

* Only the local BMC of the node itself is supported (no remote,
  out-of-band access with stored BMC credentials).
* The endpoint uses the existing Sys.Syslog privilege, like the other
  log endpoints. Enabling the option needs Sys.Modify, as for all node
  options.
* 'ipmitool' is only a Recommends of pve-manager, as the feature is
  opt-in. The GUI marks the feature as technology preview.
* Only reading the log is implemented, there is no way to clear it.
* I did not add documentation to pve-docs yet, I'm happy to do that in
  a follow-up once the general approach is fine.

Tested on a PVE 9.2 node: option toggling in the GUI and via pvenode,
the disabled/enabled behaviour of the tab, and the export. 'make check'
passes, apart from test-ceph-key-migration (2 failing sub-tests in
CephKeyMigrationScript_test.pl), which fails the same way on an
unmodified master and is unrelated to this series.

Luca Vornheder (3):
  api: nodes: add opt-in endpoint to read the local IPMI SEL
  fix #7656: ui: node: add opt-in IPMI SEL log tab
  d/control: recommend ipmitool

 PVE/API2/Nodes.pm                    | 111 ++++++++++++++++++++++
 PVE/NodeConfig.pm                    |   8 ++
 debian/control                       |   3 +-
 www/manager6/Makefile                |   1 +
 www/manager6/node/Config.js          |   9 ++
 www/manager6/node/IPMI.js            | 134 +++++++++++++++++++++++++++
 www/manager6/node/NodeOptionsView.js |  31 +++++++
 7 files changed, 296 insertions(+), 1 deletion(-)
 create mode 100644 www/manager6/node/IPMI.js

-- 
2.50.1 (Apple Git-155)

[PATCH manager 1/3] api: nodes: add opt-in endpoint to read the local IPMI SEL

[Luca Vornheder]

Add a per-node 'ipmi-sel' option to the node config and a new
GET /nodes/{node}/ipmi-sel endpoint, which returns the System Event Log
of the node's local BMC as parsed entries, read via 'ipmitool sel
elist'.

The feature is opt-in. As long as the option is not set, the endpoint
fails with 501 (Not Implemented), so clients can tell a disabled
feature apart from a real error. Access requires Sys.Syslog on the
node, like the other log endpoints.

With the 'download' parameter the raw ipmitool output is streamed as a
file instead, analogous to the task log download.

Signed-off-by: Luca Vornheder <luca@vornheder.cloud>
---
 PVE/API2/Nodes.pm | 111 ++++++++++++++++++++++++++++++++++++++++++++++
 PVE/NodeConfig.pm |   8 ++++
 2 files changed, 119 insertions(+)

diff --git a/PVE/API2/Nodes.pm b/PVE/API2/Nodes.pm
index 2ca3244..bc3008a 100644
--- a/PVE/API2/Nodes.pm
+++ b/PVE/API2/Nodes.pm
@@ -239,6 +239,7 @@ __PACKAGE__->register_method({
             { name => 'firewall' },
             { name => 'hardware' },
             { name => 'hosts' },
+            { name => 'ipmi-sel' },
             { name => 'journal' },
             { name => 'lxc' },
             { name => 'migrateall' },
@@ -1124,6 +1125,116 @@ __PACKAGE__->register_method({
     },
 });
 
+my $ipmitool_bin = '/usr/bin/ipmitool';
+
+__PACKAGE__->register_method({
+    name => 'ipmi_sel',
+    path => 'ipmi-sel',
+    method => 'GET',
+    description =>
+        "Read this node's local IPMI System Event Log (SEL), as reported by its BMC."
+        . " Requires 'ipmitool' to be installed and a local IPMI interface to be present,"
+        . " and must be enabled first via the node's 'ipmi-sel' option.",
+    proxyto => 'node',
+    permissions => {
+        check => ['perm', '/nodes/{node}', ['Sys.Syslog']],
+    },
+    protected => 1,
+    download_allowed => 1,
+    parameters => {
+        additionalProperties => 0,
+        properties => {
+            node => get_standard_option('pve-node'),
+            download => {
+                type => 'boolean',
+                optional => 1,
+                description => "Whether to return the raw 'ipmitool sel elist' output as a"
+                    . " downloadable file, instead of parsed JSON entries.",
+            },
+        },
+    },
+    returns => {
+        type => 'array',
+        items => {
+            type => 'object',
+            properties => {
+                id => { type => 'string' },
+                timestamp => { type => 'string', optional => 1 },
+                sensor => { type => 'string' },
+                description => { type => 'string' },
+                direction => { type => 'string', optional => 1 },
+            },
+        },
+    },
+    code => sub {
+        my ($param) = @_;
+
+        my $node = $param->{node};
+
+        my $conf = PVE::NodeConfig::load_config($node);
+        raise(
+            "IPMI SEL log is not enabled for this node, enable it in the node's"
+                . " 'Options' first.\n",
+            code => HTTP_NOT_IMPLEMENTED,
+        ) if !$conf->{'ipmi-sel'};
+
+        die "'ipmitool' is not installed\n" if !-x $ipmitool_bin;
+
+        if ($param->{download}) {
+            # needs a real pipe (fd), not an in-memory filehandle: the async
+            # streaming code in pve-http-server needs to select()/poll() on it
+            open(my $fh, '-|', $ipmitool_bin, 'sel', 'elist')
+                or die "could not run 'ipmitool sel elist' for download - $!\n";
+
+            return {
+                download => {
+                    fh => $fh,
+                    stream => 1,
+                    'content-type' => 'text/plain',
+                    'content-disposition' => "attachment; filename=\"ipmi-sel-${node}.log\"",
+                },
+            };
+        }
+
+        my $raw = '';
+        PVE::Tools::run_command(
+            [$ipmitool_bin, 'sel', 'elist'],
+            outfunc => sub {
+                my ($line) = @_;
+                $raw .= "$line\n";
+            },
+        );
+
+        my $entries = [];
+        for my $line (split(/\n/, $raw)) {
+            my @fields = map { s/^\s+|\s+$//gr } split(/\|/, $line);
+            my $id = shift @fields;
+            next if !defined($id) || $id !~ /^[0-9a-fA-F]+$/;
+
+            my $entry = { id => $id };
+
+            # normal format is '<id> | <date> | <time> | <sensor> | <description>
+            # [| <direction>]'; right after a BMC reset entries can lack a resolvable
+            # date and print a single 'Pre-Init Time-stamp' field instead.
+            if (@fields >= 2 && $fields[1] =~ m/^\d{2}:\d{2}:\d{2}$/) {
+                $entry->{timestamp} = (shift @fields) . ' ' . (shift @fields);
+            } elsif (@fields) {
+                $entry->{timestamp} = shift @fields;
+            }
+
+            $entry->{direction} = pop(@fields)
+                if @fields > 1 && $fields[-1] =~ m/^(?:Asserted|Deasserted)$/;
+
+            $entry->{sensor} = shift(@fields) // '';
+            $entry->{description} = join(' | ', @fields);
+
+            push @$entries, $entry;
+        }
+
+        return $entries;
+    },
+});
+
 my $sslcert;
 
 my $shell_cmd_map = {
diff --git a/PVE/NodeConfig.pm b/PVE/NodeConfig.pm
index cc1a2a2..d99a3fd 100644
--- a/PVE/NodeConfig.pm
+++ b/PVE/NodeConfig.pm
@@ -108,6 +108,14 @@ my $confdesc = {
         default => 80,
         optional => 1,
     },
+    'ipmi-sel' => {
+        description => "Enable the IPMI System Event Log (SEL) view for this node in the"
+            . " web interface. Reads the SEL of this node's local BMC via 'ipmitool', so"
+            . " that binary and a local IPMI interface need to be available on the node.",
+        type => 'boolean',
+        default => 0,
+        optional => 1,
+    },
 };
 
 my $wakeonlan_desc = {
-- 
2.50.1 (Apple Git-155)

[PATCH manager 2/3] fix #7656: ui: node: add opt-in IPMI SEL log tab

[Luca Vornheder]

Add an 'IPMI' tab next to the system log of a node, listing the entries
of the node's IPMI System Event Log, with a reload and an export
button.

The tab is enabled per node through a new 'IPMI SEL Log' row in the
node options. Its edit window notes that this is a technology preview
and that 'ipmitool' needs to be installed. While the feature is
disabled, the tab shows a hint instead of an error.

Signed-off-by: Luca Vornheder <luca@vornheder.cloud>
---
 www/manager6/Makefile                |   1 +
 www/manager6/node/Config.js          |   9 ++
 www/manager6/node/IPMI.js            | 134 +++++++++++++++++++++++++++
 www/manager6/node/NodeOptionsView.js |  31 +++++++
 4 files changed, 175 insertions(+)
 create mode 100644 www/manager6/node/IPMI.js

diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index d2ea786..d83fa20 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -244,6 +244,7 @@ JSSRC= 							\
 	node/CmdMenu.js					\
 	node/Config.js					\
 	node/Directory.js				\
+	node/IPMI.js					\
 	node/LVM.js					\
 	node/LVMThin.js					\
 	node/StatusView.js				\
diff --git a/www/manager6/node/Config.js b/www/manager6/node/Config.js
index 217ee28..ee8277e 100644
--- a/www/manager6/node/Config.js
+++ b/www/manager6/node/Config.js
@@ -257,6 +257,15 @@ Ext.define('PVE.node.Config', {
                 url: '/api2/extjs/nodes/' + nodename + '/journal',
             });
 
+            me.items.push({
+                xtype: 'pveNodeIPMIView',
+                title: gettext('IPMI'),
+                iconCls: 'fa fa-server',
+                groups: ['services'],
+                itemId: 'ipmi-sel',
+                nodename: nodename,
+            });
+
             if (caps.nodes['Sys.Modify']) {
                 me.items.push({
                     xtype: 'proxmoxNodeAPT',
diff --git a/www/manager6/node/IPMI.js b/www/manager6/node/IPMI.js
new file mode 100644
index 0000000..48228ae
--- /dev/null
+++ b/www/manager6/node/IPMI.js
@@ -0,0 +1,134 @@
+Ext.define('PVE.node.IPMIView', {
+    extend: 'Ext.grid.Panel',
+    alias: 'widget.pveNodeIPMIView',
+
+    onlineHelp: 'proxmox_node_management',
+
+    stateful: true,
+    stateId: 'grid-node-ipmi-sel',
+
+    // we drive all loading/error/disabled feedback ourselves via emptyText and
+    // setErrorMask below, so the grid's own spinner must stay out of the way
+    loadMask: false,
+
+    viewConfig: {
+        loadMask: false,
+        emptyText: gettext('No entries found'),
+        deferEmptyText: false,
+    },
+
+    columns: [
+        {
+            header: gettext('ID'),
+            dataIndex: 'id',
+            width: 60,
+        },
+        {
+            header: gettext('Timestamp'),
+            dataIndex: 'timestamp',
+            width: 160,
+        },
+        {
+            header: gettext('Sensor'),
+            dataIndex: 'sensor',
+            width: 200,
+        },
+        {
+            header: gettext('Event'),
+            dataIndex: 'description',
+            flex: 1,
+        },
+        {
+            header: gettext('Direction'),
+            dataIndex: 'direction',
+            width: 100,
+        },
+    ],
+
+    tbar: [
+        {
+            text: gettext('Reload'),
+            iconCls: 'fa fa-refresh',
+            handler: function () {
+                this.up('grid').reload();
+            },
+        },
+        {
+            itemId: 'exportBtn',
+            text: gettext('Export'),
+            iconCls: 'fa fa-download',
+            disabled: true,
+            handler: function () {
+                let me = this.up('grid');
+                Proxmox.Utils.downloadAsFile(
+                    `/api2/json/nodes/${me.nodename}/ipmi-sel?download=1`,
+                );
+            },
+        },
+    ],
+
+    reload: function () {
+        let me = this;
+        me.getStore().load();
+    },
+
+    listeners: {
+        activate: function () {
+            this.reload();
+        },
+    },
+
+    initComponent: function () {
+        let me = this;
+
+        me.nodename = me.pveSelNode.data.node;
+        if (!me.nodename) {
+            throw 'no node name specified';
+        }
+
+        Ext.apply(me, {
+            store: {
+                fields: ['id', 'timestamp', 'sensor', 'description', 'direction'],
+                proxy: {
+                    type: 'proxmox',
+                    url: `/api2/json/nodes/${me.nodename}/ipmi-sel`,
+                },
+            },
+        });
+
+        me.callParent();
+
+        let store = me.getStore();
+        let view = me.getView();
+        let exportBtn = me.down('#exportBtn');
+
+        me.mon(store, 'beforeload', () => Proxmox.Utils.setErrorMask(me, false));
+
+        // only works with the 'proxmox' proxy
+        me.mon(store.proxy, 'afterload', (proxy, request, success) => {
+            if (success) {
+                Proxmox.Utils.setErrorMask(me, false);
+                exportBtn.setDisabled(false);
+                return;
+            }
+
+            exportBtn.setDisabled(true);
+
+            let error = request._operation.getError();
+
+            if (error && error.status === 501) {
+                // feature disabled for this node -- not an error, just say so
+                view.emptyText = Ext.htmlEncode(
+                    gettext(
+                        "IPMI SEL log is not enabled for this node. Enable it in the" +
+                            " node's 'Options' tab first.",
+                    ),
+                );
+                view.refresh();
+                return;
+            }
+
+            Proxmox.Utils.setErrorMask(me, Proxmox.Utils.getResponseErrorMessage(error));
+        });
+    },
+});
diff --git a/www/manager6/node/NodeOptionsView.js b/www/manager6/node/NodeOptionsView.js
index 5e9ce79..98fdb70 100644
--- a/www/manager6/node/NodeOptionsView.js
+++ b/www/manager6/node/NodeOptionsView.js
@@ -55,6 +55,37 @@ Ext.define('Proxmox.node.NodeOptionsView', {
                 xtype: 'pmxLocationEditWindow',
             },
         },
+        'ipmi-sel': {
+            required: true,
+            defaultValue: 0,
+            header: gettext('IPMI SEL Log'),
+            renderer: Proxmox.Utils.format_boolean,
+            editor: {
+                xtype: 'proxmoxWindowEdit',
+                subject: gettext('IPMI SEL Log'),
+                fieldDefaults: {
+                    labelWidth: 130,
+                },
+                items: [
+                    {
+                        xtype: 'displayfield',
+                        userCls: 'pmx-hint',
+                        value: gettext(
+                            "Technology preview. Requires 'ipmitool' to be installed on" +
+                                ' this node.',
+                        ),
+                    },
+                    {
+                        xtype: 'proxmoxcheckbox',
+                        name: 'ipmi-sel',
+                        uncheckedValue: 0,
+                        defaultValue: 0,
+                        checked: false,
+                        fieldLabel: gettext('IPMI SEL Log'),
+                    },
+                ],
+            },
+        },
     },
 
     gridRows: [
-- 
2.50.1 (Apple Git-155)

[PATCH manager 3/3] d/control: recommend ipmitool

[Luca Vornheder]

The new IPMI SEL log feature of the node needs 'ipmitool'. Only
recommend it, as the feature is opt-in.

Signed-off-by: Luca Vornheder <luca@vornheder.cloud>
---
 debian/control | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/debian/control b/debian/control
index 728f660..05fa949 100644
--- a/debian/control
+++ b/debian/control
@@ -104,7 +104,8 @@ Depends: apt (>= 1.5~),
          zstd,
          ${misc:Depends},
          ${perl:Depends},
-Recommends: proxmox-firewall,
+Recommends: ipmitool,
+            proxmox-firewall,
             proxmox-offline-mirror-helper,
             pve-nvidia-vgpu-helper,
             skopeo,
-- 
2.50.1 (Apple Git-155)