From 7c089195fe28c83877a04aab8451634003811569 Mon Sep 17 00:00:00 2001 From: Simon Pichugin Date: Jul 23 2020 22:18:02 +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 691a6f2..1963c39 100644 --- a/src/cockpit/389-console/src/ds.jsx +++ b/src/cockpit/389-console/src/ds.jsx @@ -62,7 +62,7 @@ export class DSInstance extends React.Component { backupRows: [], notifications: [], activeKey: 1, - wasActiveList: [1], + wasActiveList: [], progressValue: 0, loadingOperate: false, @@ -140,7 +140,8 @@ export class DSInstance extends React.Component { this.updateProgress(25); this.setState( { - serverId: serverId + serverId: serverId, + wasActiveList: [this.state.activeKey] }, () => { this.loadBackups(); @@ -149,11 +150,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] }); } ); @@ -171,7 +174,8 @@ export class DSInstance extends React.Component { }, () => { this.setState({ - serverId: serverId + serverId: serverId, + wasActiveList: [] }); } ); @@ -186,7 +190,8 @@ export class DSInstance extends React.Component { }, () => { this.setState({ - serverId: serverId + serverId: serverId, + wasActiveList: [] }); } ); @@ -204,7 +209,8 @@ export class DSInstance extends React.Component { }, () => { this.setState({ - serverId: serverId + serverId: serverId, + wasActiveList: [] }); } ); @@ -220,51 +226,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() { @@ -322,8 +336,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