From 632a8e085287ed75361f3b168ec81ec2f5470497 Mon Sep 17 00:00:00 2001 From: Simon Pichugin Date: Jul 23 2020 10:00:39 +0000 Subject: Issue 51136 - JSON Error output has redundant messages Bug Description: When we try to start an instance for which 'systemctl start' command has failed, it produces excessive output which is not a clear JSON. Fix Description: Redirect stderr to stdout as we don't need the info in CLI. User needs to check logs if something went wrong. Add a new-line character in the end of DS CLI tool's stderr. Clean up React state processing for setServerID callback. https://pagure.io/389-ds-base/issue/51136 Reviewed by: mreynolds (Thanks!) --- diff --git a/src/cockpit/389-console/src/ds.jsx b/src/cockpit/389-console/src/ds.jsx index 6657259..5022113 100644 --- a/src/cockpit/389-console/src/ds.jsx +++ b/src/cockpit/389-console/src/ds.jsx @@ -59,7 +59,7 @@ export class DSInstance extends React.Component { backupRows: [], notifications: [], activeKey: 1, - wasActiveList: [1], + wasActiveList: [], progressValue: 0, loadingOperate: false, @@ -137,7 +137,8 @@ export class DSInstance extends React.Component { this.updateProgress(25); this.setState( { - serverId: serverId + serverId: serverId, + wasActiveList: [this.state.activeKey] }, () => { this.loadBackups(); @@ -146,11 +147,13 @@ export class DSInstance extends React.Component { if (action === "restart") { this.setState( { - serverId: "" + serverId: "", + wasActiveList: [] }, () => { this.setState({ - serverId: serverId + serverId: serverId, + wasActiveList: [this.state.activeKey] }); } ); @@ -168,7 +171,8 @@ export class DSInstance extends React.Component { }, () => { this.setState({ - serverId: serverId + serverId: serverId, + wasActiveList: [] }); } ); @@ -183,7 +187,8 @@ export class DSInstance extends React.Component { }, () => { this.setState({ - serverId: serverId + serverId: serverId, + wasActiveList: [] }); } ); @@ -201,7 +206,8 @@ export class DSInstance extends React.Component { }, () => { this.setState({ - serverId: serverId + serverId: serverId, + wasActiveList: [] }); } ); @@ -217,51 +223,59 @@ export class DSInstance extends React.Component { } })); } - let cmd = ["dsctl", "-l", "-j"]; - log_cmd("loadInstanceList", "Load the instance list select", cmd); - cockpit - .spawn(cmd, { superuser: true }) - .done(data => { - this.updateProgress(25); - let myObject = JSON.parse(data); - this.setState({ - instList: myObject.insts, - loadingOperate: false - }); - // Set default value for the inst select - if (serverId !== undefined && serverId !== "") { - this.setState({ - wasActiveList: [this.state.activeKey] - }); - this.setServerId(serverId, action); - } else { - if (myObject.insts.length > 0) { + this.setState( + { + wasActiveList: [] + }, + () => { + let cmd = ["dsctl", "-l", "-j"]; + log_cmd( + "loadInstanceList", + "Load the instance list select", + cmd + ); + cockpit + .spawn(cmd, { superuser: true }) + .done(data => { + this.updateProgress(25); + let myObject = JSON.parse(data); this.setState({ - wasActiveList: [this.state.activeKey] + instList: myObject.insts, + loadingOperate: false }); - this.setServerId(myObject.insts[0].replace("slapd-", ""), action); - } else { + // Set default value for the inst select + if (serverId !== undefined && serverId !== "") { + this.setServerId(serverId, action); + } else { + if (myObject.insts.length > 0) { + this.setServerId( + myObject.insts[0].replace("slapd-", ""), + action + ); + } else { + this.setState({ + serverId: "", + pageLoadingState: { + state: "noInsts", + jsx: staticStates["noInsts"] + } + }); + } + } + }) + .fail(_ => { this.setState({ + instList: [], serverId: "", + loadingOperate: false, pageLoadingState: { state: "noInsts", jsx: staticStates["noInsts"] } }); - } - } - }) - .fail(_ => { - this.setState({ - instList: [], - serverId: "", - loadingOperate: false, - pageLoadingState: { - state: "noInsts", - jsx: staticStates["noInsts"] - } - }); - }); + }); + } + ); } loadBackups() { @@ -319,8 +333,7 @@ export class DSInstance extends React.Component { handleServerIdChange(e) { this.setState({ pageLoadingState: { state: "loading", jsx: "" }, - progressValue: 25, - serverId: e.target.value + progressValue: 25 }); this.loadInstanceList(e.target.value); } diff --git a/src/lib389/cli/dsconf b/src/lib389/cli/dsconf index 71fc2b6..befeaee 100755 --- a/src/lib389/cli/dsconf +++ b/src/lib389/cli/dsconf @@ -139,7 +139,7 @@ if __name__ == '__main__': msg = format_error_to_dict(e) if args and args.json: - sys.stderr.write(json.dumps(msg, indent=4)) + sys.stderr.write(f"{json.dumps(msg, indent=4)}\n") else: log.error("Error: %s" % " - ".join(str(val) for val in msg.values())) result = False diff --git a/src/lib389/cli/dscreate b/src/lib389/cli/dscreate index dc4c706..d6edcbd 100755 --- a/src/lib389/cli/dscreate +++ b/src/lib389/cli/dscreate @@ -80,7 +80,7 @@ if __name__ == '__main__': log.debug(e, exc_info=True) msg = format_error_to_dict(e) if args and args.json: - sys.stderr.write(json.dumps(msg, indent=4)) + sys.stderr.write(f"{json.dumps(msg, indent=4)}\n") else: log.error("Error: %s" % " - ".join(str(val) for val in msg.values())) result = False diff --git a/src/lib389/cli/dsctl b/src/lib389/cli/dsctl index 21aabd1..fe9bc10 100755 --- a/src/lib389/cli/dsctl +++ b/src/lib389/cli/dsctl @@ -141,7 +141,7 @@ if __name__ == '__main__': log.debug(e, exc_info=True) msg = format_error_to_dict(e) if args.json: - sys.stderr.write(json.dumps(msg, indent=4)) + sys.stderr.write(f"{json.dumps(msg, indent=4)}\n") else: log.error("Error: %s" % " - ".join(str(val) for val in msg.values())) result = False diff --git a/src/lib389/cli/dsidm b/src/lib389/cli/dsidm index ae38d14..bac02c3 100755 --- a/src/lib389/cli/dsidm +++ b/src/lib389/cli/dsidm @@ -134,7 +134,7 @@ if __name__ == '__main__': log.debug(e, exc_info=True) msg = format_error_to_dict(e) if args.json: - sys.stderr.write(json.dumps(msg, indent=4)) + sys.stderr.write(f"{json.dumps(msg, indent=4)}\n") else: log.error("Error: %s" % " - ".join(str(val) for val in msg.values())) result = False diff --git a/src/lib389/lib389/__init__.py b/src/lib389/lib389/__init__.py index c3dc6f2..99ea9cc 100644 --- a/src/lib389/lib389/__init__.py +++ b/src/lib389/lib389/__init__.py @@ -1130,9 +1130,7 @@ class DirSrv(SimpleLDAPObject, object): if self.with_systemd(): self.log.debug("systemd status -> True") # Do systemd things here ... - subprocess.check_call(["systemctl", - "start", - "dirsrv@%s" % self.serverid]) + subprocess.check_output(["systemctl", "start", "dirsrv@%s" % self.serverid], stderr=subprocess.STDOUT) else: self.log.debug("systemd status -> False") # Start the process. @@ -1201,9 +1199,7 @@ class DirSrv(SimpleLDAPObject, object): if self.with_systemd(): self.log.debug("systemd status -> True") # Do systemd things here ... - subprocess.check_call(["systemctl", - "stop", - "dirsrv@%s" % self.serverid]) + subprocess.check_output(["systemctl", "stop", "dirsrv@%s" % self.serverid], stderr=subprocess.STDOUT) else: self.log.debug("systemd status -> False") # TODO: Make the pid path in the files things