Change in vdsm[master]: vdsm-upgrade: adds wrapper to ovirt-node-upgrade

Alon Bar-Lev alonbl at redhat.com
Tue Jun 10 10:59:21 UTC 2014


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 <dougsland at redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alonbl at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland at redhat.com>
Gerrit-Reviewer: Joey Boggs <jboggs at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list