#50700 Issue 50699 - Add Disk Monitor to CLI and UI
Merged 2 months ago by mreynolds. Opened 2 months ago by mreynolds.
mreynolds/389-ds-base issue50699  into  master

@@ -8,7 +8,7 @@ 

      tableCellFormatter,

      noop

  } from "patternfly-react";

- import { DSTable } from "../dsTable.jsx";

+ import { DSTable, DSShortTable } from "../dsTable.jsx";

  import PropTypes from "prop-types";

  import "../../css/ds.css";

  import { get_date_string } from "../tools.jsx";

@@ -1584,6 +1584,121 @@ 

      }

  }

  

+ class DiskTable extends React.Component {

+     constructor(props) {

+         super(props);

+ 

+         this.state = {

+             rowKey: "mount",

+             columns: [

+                 {

+                     property: "mount",

+                     header: {

+                         label: "Disk Partition",

+                         props: {

+                             index: 0,

+                             rowSpan: 1,

+                             colSpan: 1,

+                             sort: true

+                         },

+                         transforms: [],

+                         formatters: [],

+                         customFormatters: [sortableHeaderCellFormatter]

+                     },

+                     cell: {

+                         props: {

+                             index: 0

+                         },

+                         formatters: [tableCellFormatter]

+                     }

+                 },

+                 {

+                     property: "size",

+                     header: {

+                         label: "Disk Size",

+                         props: {

+                             index: 1,

+                             rowSpan: 1,

+                             colSpan: 1,

+                             sort: true

+                         },

+                         transforms: [],

+                         formatters: [],

+                         customFormatters: [sortableHeaderCellFormatter]

+                     },

+                     cell: {

+                         props: {

+                             index: 1

+                         },

+                         formatters: [tableCellFormatter]

+                     }

+                 },

+                 {

+                     property: "used",

+                     header: {

+                         label: "Used Space",

+                         props: {

+                             index: 2,

+                             rowSpan: 1,

+                             colSpan: 1,

+                             sort: true

+                         },

+                         transforms: [],

+                         formatters: [],

+                         customFormatters: [sortableHeaderCellFormatter]

+                     },

+                     cell: {

+                         props: {

+                             index: 2

+                         },

+                         formatters: [tableCellFormatter]

+                     }

+                 },

+                 {

+                     property: "avail",

+                     header: {

+                         label: "Available Space",

+                         props: {

+                             index: 3,

+                             rowSpan: 1,

+                             colSpan: 1,

+                             sort: true

+                         },

+                         transforms: [],

+                         formatters: [],

+                         customFormatters: [sortableHeaderCellFormatter]

+                     },

+                     cell: {

+                         props: {

+                             index: 3

+                         },

+                         formatters: [tableCellFormatter]

+                     }

+                 },

+ 

+             ]

+         };

+         this.getColumns = this.getColumns.bind(this);

+     }

+ 

+     getColumns() {

+         return this.state.columns;

+     }

+ 

+     render() {

+         return (

+             <div className="ds-margin-top-xlg">

+                 <DSShortTable

+                     getColumns={this.getColumns}

+                     rowKey={this.state.rowKey}

+                     rows={this.props.disks}

+                     disableLoadingSpinner

+                 />

+             </div>

+         );

+     }

+ }

+ 

  // Proptypes and defaults

  

  LagReportTable.propTypes = {

@@ -1681,4 +1796,5 @@ 

      AbortCleanALLRUVTable,

      ConflictTable,

      GlueTable,

+     DiskTable,

  };

@@ -3,9 +3,11 @@ 

  import "../../css/ds.css";

  import { get_date_string, get_date_diff } from "../tools.jsx";

  import {

-     ConnectionTable

+     ConnectionTable,

+     DiskTable,

  } from "./monitorTables.jsx";

  import {

+     Button,

      Nav,

      NavItem,

      TabContent,

@@ -66,6 +68,9 @@ 

                              <NavItem eventKey={2}>

                                  <div dangerouslySetInnerHTML={{__html: 'Connection Table'}} />

                              </NavItem>

+                             <NavItem eventKey={3}>

+                                 <div dangerouslySetInnerHTML={{__html: 'Disk Space'}} />

+                             </NavItem>

                          </Nav>

                          <TabContent>

  

@@ -191,6 +196,17 @@ 

                              <TabPane eventKey={2}>

                                  <ConnectionTable conns={this.props.data.connection} />

                              </TabPane>

+                             <TabPane eventKey={3}>

+                                 <DiskTable

+                                     disks={this.props.disks}

+                                 />

+                                 <Button

+                                     className="ds-margin-top"

+                                     onClick={this.props.reloadDisks}

+                                 >

+                                     Refresh

+                                 </Button>

+                             </TabPane>

                          </TabContent>

                      </div>

                  </TabContainer>

@@ -39,6 +39,7 @@ 

              snmpData: {},

              ldbmData: {},

              serverData: {},

+             disks: [],

              loadingMsg: "",

              notifications: [],

              disableTree: false,

@@ -124,6 +125,8 @@ 

          this.loadMonitorServer = this.loadMonitorServer.bind(this);

          this.reloadServer = this.reloadServer.bind(this);

          this.loadMonitorChaining = this.loadMonitorChaining.bind(this);

+         this.loadDiskSpace = this.loadDiskSpace.bind(this);

+         this.reloadDisks = this.reloadDisks.bind(this);

          // Replication

          this.loadMonitorReplication = this.loadMonitorReplication.bind(this);

          this.loadCleanTasks = this.loadCleanTasks.bind(this);

@@ -457,7 +460,7 @@ 

                          replicatedSuffixes: config.items,

                          replSuffix: replSuffix,

                      });

-                 }, this.loadMonitorLDBM());

+                 }, this.loadDiskSpace());

      }

  

      loadMonitorLDBM() {

@@ -528,7 +531,7 @@ 

                      this.setState({

                          serverLoading: false,

                          serverData: config.attrs

-                     });

+                     }, this.reloadDisks());

                  });

      }

  

@@ -548,6 +551,44 @@ 

                  });

      }

  

+     loadDiskSpace() {

+         let cmd = [

+             "dsconf", "-j", "ldapi://%2fvar%2frun%2fslapd-" + this.props.serverId + ".socket",

+             "monitor", "disk"

+         ];

+         log_cmd("loadDiskSpace", "Load disk space info", cmd);

+         cockpit

+                 .spawn(cmd, { superuser: true, err: "message" })

+                 .done(content => {

+                     let disks = JSON.parse(content);

+                     for (let disk of disks.items) {

+                         disk.used = disk.used + " (" + disk.percent + "%)";

+                     }

+                     this.setState({

+                         disks: disks.items

+                     });

+                 }, this.loadMonitorLDBM());

+     }

+ 

+     reloadDisks () {

+         let cmd = [

+             "dsconf", "-j", "ldapi://%2fvar%2frun%2fslapd-" + this.props.serverId + ".socket",

+             "monitor", "disk"

+         ];

+         log_cmd("reloadDisks", "Reload disk stats", cmd);

+         cockpit

+                 .spawn(cmd, { superuser: true, err: "message" })

+                 .done(content => {

+                     let disks = JSON.parse(content);

+                     for (let disk of disks.items) {

+                         disk.used = disk.used + " (" + disk.percent + "%)";

+                     }

+                     this.setState({

+                         disks: disks.items,

+                     });

+                 });

+     }

+ 

      reloadSNMP() {

          this.setState({

              snmpLoading: true

@@ -1053,6 +1094,8 @@ 

                              data={this.state.serverData}

                              reload={this.reloadServer}

                              serverId={this.props.serverId}

+                             disks={this.state.disks}

+                             reloadDisks={this.reloadDisks}

                              enableTree={this.enableTree}

                          />;

                  }

@@ -7,9 +7,11 @@ 

  # See LICENSE for details.

  # --- END COPYRIGHT BLOCK ---

  

- from lib389.monitor import (Monitor, MonitorLDBM, MonitorSNMP)

+ import json

+ from lib389.monitor import (Monitor, MonitorLDBM, MonitorSNMP, MonitorDiskSpace)

  from lib389.chaining import (ChainingLinks)

  from lib389.backend import Backends

+ from lib389.utils import convert_bytes

  

  

  def _format_status(log, mtype, json=False):

@@ -63,9 +65,39 @@ 

          for link in links.list():

              link_monitor = link.get_monitor()

              _format_status(log, link_monitor, args.json)

-             # Inejct a new line for now ... see https://pagure.io/389-ds-base/issue/50189

+             # Inject a new line for now ... see https://pagure.io/389-ds-base/issue/50189

              log.info("")

  

+ def disk_monitor(inst, basedn, log, args):

+     disk_space_mon = MonitorDiskSpace(inst)

+     disks = disk_space_mon.get_disks()

+     disk_list = []

+     for disk in disks:

+         # partition="/" size="52576092160" used="25305038848" available="27271053312" use%="48"

+         parts = disk.split()

+         mount = parts[0].split('=')[1].strip('"')

+         disk_size = convert_bytes(parts[1].split('=')[1].strip('"'))

+         used = convert_bytes(parts[2].split('=')[1].strip('"'))

+         avail = convert_bytes(parts[3].split('=')[1].strip('"'))

+         percent = parts[4].split('=')[1].strip('"')

+         if args.json:

+             disk_list.append({

+                 'mount': mount,

+                 'size': disk_size,

+                 'used': used,

+                 'avail': avail,

+                 'percent': percent

+             })

+         else:

+             log.info("Partition: " + mount)

+             log.info("Size: " + disk_size)

+             log.info("Used Space: " + used)

+             log.info("Available Space: " + avail)

+             log.info("Percentage Used: " + percent + "%\n")

+ 

+     if args.json:

+         log.info(json.dumps({"type": "list", "items": disk_list}))

+ 

  

  def create_parser(subparsers):

      monitor_parser = subparsers.add_parser('monitor', help="Monitor the state of the instance")

@@ -87,3 +119,6 @@ 

      chaining_parser = subcommands.add_parser('chaining', help="Monitor database chaining statistics")

      chaining_parser.add_argument('backend', nargs='?', help="Optional name of the chaining backend to monitor")

      chaining_parser.set_defaults(func=chaining_monitor)

+ 

+     disk_parser = subcommands.add_parser('disk', help="Disk space statistics.  All values are in bytes")

+     disk_parser.set_defaults(func=disk_monitor)

@@ -39,6 +39,7 @@ 

  import shlex

  import operator

  import subprocess

+ import math

  from packaging.version import LegacyVersion

  from socket import getfqdn

  from ldapurl import LDAPUrl

@@ -1302,3 +1303,14 @@ 

  def display_log_data(data, hide_sensitive=True):

      # Take a dict and mask all the sensitive data

      return {a: display_log_value(a, v, hide_sensitive) for a, v in data.items()}

+ 

+ 

+ def convert_bytes(bytes):

+     bytes = int(bytes)

+     if bytes == 0:

+         return "0 B"

+     size_name = ["B", "KB", "MB", "GB", "TB", "PB"]

+     i = int(math.floor(math.log(bytes, 1024)))

+     pow = math.pow(1024, i)

+     siz = round(bytes / pow, 2)

+     return "{} {}".format(siz, size_name[i])

Description: Add the disk monitoring to the CLI and UI

relates: https://pagure.io/389-ds-base/issue/50699

I think it is simple and good enough for a minimal representation.

Ideally, I think, we better have a 'human-readable" format option for both UI and CLI.

What do you think?

Like we can add a setting for choosing units. Or just make a switch for representing in Mb (and for CLI it will be a true-false option)

I think it is simple and good enough for a minimal representation.
Ideally, I think, we better have a 'human-readable" format option for both UI and CLI.
What do you think?

The UI is human readable, how is it not? Now, the CLI could definitely be more readable

Like we can add a setting for choosing units. Or just make a switch for representing in Mb (and for CLI it will be a true-false option)

Yeah I can do that

rebased onto 012887a1972c9473abd9d05dfb391ee7935bf74f

2 months ago

@spichugi So to fix this I always convert bytes to a more friendly value, and I improved the CLI human readable format. Please review...

Minor thing - loadDiskSpace and reloadDisks have nearly the same code (I think only the log message is different). I think it would be better while encasulated...

The rest looks good. Ack.

Minor thing - loadDiskSpace and reloadDisks have nearly the same code (I think only the log message is different). I think it would be better while encasulated...
The rest looks good. Ack.

Sorry not sure what you mean. The big difference between the load and reload functions is that the load function calls other loading functions once its finishes setting the state. This is how it's done throughout the rest of the UI code.

I wonder if we can do something like this:

loadDiskSpace(firstLoad) {
    let cmd = [
        "dsconf", "-j", "ldapi://%2fvar%2frun%2fslapd-" + this.props.serverId + ".socket",
        "monitor", "disk"
    ];
    log_cmd("loadDiskSpace", "Load disk space info", cmd);
    cockpit
            .spawn(cmd, { superuser: true, err: "message" })
            .done(content => {
                let disks = JSON.parse(content);
                for (let disk of disks.items) {
                    disk.used = disk.used + " (" + disk.percent + "%)";
                }
                this.setState({
                    disks: disks.items
                });
                if (firstLoad) {
                    this.loadMonitorLDBM());
                }
             }
}

loadDisks() {
    this.loadDiskSpace(true);
}

reloadDisks() {
    this.loadDiskSpace(false);
}

I think it should work even if we put the this.loadMonitorLDBM()); callback inside of the callback from done().

That won't work correctly. The nested loading function must be "within" the setState call:

this.setState({
....
}, this.loadNextFunc);

I think it's best to leave the functions as separate functions. IMHO having similar code in two functions (that could be merged into a single function) is not as bad as it's made out to be. I feel ideology of avoiding duplicate code at all costs is antiquated. There are benefits to it, especially if the needs of the reload verses nested loading changes in the future. I feel it's more flexible and easier to read than adding a lot of wrappers and if/then/else conditions.

If you feel that strongly about it I will change it, but it also means we have to rewrite a lot of other code to be consistent with that model.

Okay, I got you. I think it makes sense to leave it like this here.

I think it's worth to have it in other places though... Like when we modify/add some modal window. It may have a lot of boilerplate react code and it can be nicely encapsulated for these kinds of similar functions.

You have my ack here! Thanks for the explanation!

Okay, I got you. I think it makes sense to leave it like this here.
I think it's worth to have it in other places though... Like when we modify/add some modal window. It may have a lot of boilerplate react code and it can be nicely encapsulated for these kinds of similar functions.

Please file an issue to address these areas.

You have my ack here! Thanks for the explanation!

Thanks!

rebased onto 0493b01

2 months ago

Pull-Request has been merged by mreynolds

2 months ago