Alon Bar-Lev has posted comments on this change. Change subject: hooks: Adding hooks for ovirt-node-upgrade ......................................................................
Patch Set 1: (6 comments) http://gerrit.ovirt.org/#/c/28251/1/hooks/README File hooks/README: Line 1: This hooks dir contains the hooks to ovirt-node-upgrade tool. refer to ovirt-node-upgade documentation instead of duplicate? Line 2: Line 3: Notes Line 4: ======== Line 5: pre-upgrade: Line 9: This directory should contain any POST-upgrade script needed for upgrade Line 10: Line 11: Naming and Permissions Line 12: ======================= Line 13: The naming of script should follow: '01-name-of-script', '02-name-of-script', the numbering - Line 14: is the order of running. Permission should be 755. Line 15: Line 16: Format Line 17: =========== http://gerrit.ovirt.org/#/c/28251/1/hooks/pre-upgrade/01-vdsm File hooks/pre-upgrade/01-vdsm: Line 18: Line 19: Line 20: def main(): Line 21: rc, out, err = utils.execCmd( Line 22: shlex.split("vdsm-tool service-status vdsmd"), please do not use shlex, use explicit arguments. Line 23: sudo=False, Line 24: raw=True Line 25: ) Line 26: if rc == 0: Line 27: rc, out, err = utils.execCmd( Line 28: shlex.split("vdsm-tool service-stop vdsmd"), Line 29: sudo=True, Line 30: raw=True Line 31: ) I think that you can stop unconditionally. stop of stopped should return success. Line 32: if rc != 0: Line 33: msg = "<BSTRAP component='RHEL_INSTALL' status='FAIL'" \ Line 34: " message='Cannot stop vdsm daemon before we" \ Line 35: " start the upgrade, please verify!'/>" Line 38: " message='vdsm daemon stopped for upgrade process!'/>" Line 39: else: Line 40: msg = "<BSTRAP component='RHEL_INSTALL' status='WARN'" \ Line 41: " message='vdsm daemon is already down before we" \ Line 42: " stop it for upgrade.'/>" this format does not belongs to new code, it belongs to the wrapper. Line 43: print(msg) Line 44: return rc Line 45: Line 46: if __name__ == "__main__": http://gerrit.ovirt.org/#/c/28251/1/ovirt-node-plugin-vdsm.spec.in File ovirt-node-plugin-vdsm.spec.in: Line 55: Line 56: Line 57: %post Line 58: # persist hook to ovirt-node-upgrade Line 59: persist {_libexecdir}/ovirt-node/hooks/pre-upgrade/01-vdsm >/dev/null 2>&1 || : this should be installed at rootfs, it is not configuration that requires persist. Line 60: Line 61: # reserve vdsm port 54321 Line 62: augtool << \EOF_sysctl Line 63: set /files/etc/sysctl.conf/net.ipv4.ip_local_reserved_ports 54321 -- To view, visit http://gerrit.ovirt.org/28251 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I986fe6edb8f5619dcbca33cccab080245654fd02 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-node-plugin-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: Fabian Deutsch <[email protected]> Gerrit-Reviewer: Joey Boggs <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
