Alon Bar-Lev has posted comments on this change. Change subject: vdsm-upgrade: adds wrapper to ovirt-node-upgrade ......................................................................
Patch Set 3: (4 comments) http://gerrit.ovirt.org/#/c/28244/3/vdsm_reg/vdsm-upgrade File vdsm_reg/vdsm-upgrade: Line 42: # To get the messages async we should read stderr when triggering Line 43: # the upgrade since the tool uses subprocess.Popen() that waits Line 44: # the command terminates. Line 45: while True: Line 46: line = ' '.join(upgrade_tool.stdout.readline().strip("\n").split()[2:]) why do you split and select fields? I am not sure the format is guaranteed... just take the entire line and put it in. Line 47: if not line: Line 48: break Line 49: Line 50: msg = "<BSTRAP component=\'ovirt-node-upgrade\' " \ Line 47: if not line: Line 48: break Line 49: Line 50: msg = "<BSTRAP component=\'ovirt-node-upgrade\' " \ Line 51: "status=\'OK\' message=\'%s\'/>" % line you must escape line Line 52: Line 53: print(msg) Line 54: logging.debug(msg) Line 55: Line 52: Line 53: print(msg) Line 54: logging.debug(msg) Line 55: Line 56: error_upgrade = upgrade_tool.stdout.read().strip().split("\n")[-1] you should wait and examine the exit status. only per exit status you know if success or failure, never by parsing. Line 57: if not error_upgrade: Line 58: msg = "<BSTRAP component=\'ovirt-node-upgrade\' " \ Line 59: "status=\'OK\' message=\'Upgraded Succeeded\'/>\n" Line 60: msg += "<BSTRAP component='RHEV_INSTALL' status='OK'/>" Line 64: "status=\'OK\' message=\'Upgraded Failed\'/>\n" Line 65: msg += "<BSTRAP component=\'ovirt-node-upgrade\' " \ Line 66: "status=\'FAIL\' message=\'%s\'/>\n" % error_upgrade Line 67: msg += "<BSTRAP component='RHEV_INSTALL' status='FAIL'/>" Line 68: rc = -1 there is no such thing as -1 exit code Line 69: Line 70: print(msg) Line 71: logging.debug(msg) Line 72: -- To view, visit http://gerrit.ovirt.org/28244 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7b997d70a440545497246d1a19d9671b054a56a5 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Douglas Schilling Landgraf <[email protected]> Gerrit-Reviewer: Alon Bar-Lev <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Douglas Schilling Landgraf <[email protected]> Gerrit-Reviewer: Joey Boggs <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
