Fabian Deutsch has posted comments on this change. Change subject: Move hooks to their own service ......................................................................
Patch Set 2: (22 comments) https://gerrit.ovirt.org/#/c/39465/2/scripts/ovirt-hooks.sh.in File scripts/ovirt-hooks.sh.in: Line 1: #!/bin/bash Line 2: # Line 3: # ovirt-hooks.sh - Wrapps all functions needed by oVirt at boot time. let's call it ovirt-node-hooks Line 4: # Line 5: # Copyright (C) 2015 Red Hat, Inc. Line 6: # Written by Ryan Barry <[email protected]> Line 7: # Line 20: # MA 02110-1301, USA. A copy of the GNU General Public License is Line 21: # also available at http://www.gnu.org/copyleft/gpl.html. Line 22: # Line 23: Line 24: . /etc/init.d/functions Do we really need all this "imports" ? Please dorp them. Including the three below this line Line 25: . /usr/libexec/ovirt-functions Line 26: . /usr/libexec/ovirt-init-functions.sh Line 27: Line 28: . @DRACUTDIR@/91ovirtnode/ovirt-boot-functions Line 27: Line 28: . @DRACUTDIR@/91ovirtnode/ovirt-boot-functions Line 29: Line 30: Line 31: VAR_SUBSYS_OVIRT_HOOKS=/var/lock/subsys/ovirt-hooks Let's drop this Line 32: HOOK_ON_BOOT_DIR=/usr/libexec/ovirt-node/hooks/on-boot Line 33: Line 34: start_ovirt_hooks () { Line 35: breakpoint "pre-hooks" Line 30: Line 31: VAR_SUBSYS_OVIRT_HOOKS=/var/lock/subsys/ovirt-hooks Line 32: HOOK_ON_BOOT_DIR=/usr/libexec/ovirt-node/hooks/on-boot Line 33: Line 34: start_ovirt_hooks () { let's simplify this file much more. Signature of the script file: ovirt-node-hook trigger <hookname> and then in this file, only keep for loop looping over the hooks in <hookname> We only need a sanity check that hookname is given and "trigger" is the first argumetn. this function can be called trigger_hook() Line 35: breakpoint "pre-hooks" Line 36: [ -f "$VAR_SUBSYS_OVIRT_HOOKS" ] && exit 0 Line 37: { Line 38: log "Starting ovirt-hooks service" Line 32: HOOK_ON_BOOT_DIR=/usr/libexec/ovirt-node/hooks/on-boot Line 33: Line 34: start_ovirt_hooks () { Line 35: breakpoint "pre-hooks" Line 36: [ -f "$VAR_SUBSYS_OVIRT_HOOKS" ] && exit 0 drop this condition, not needed. Line 37: { Line 38: log "Starting ovirt-hooks service" Line 39: Line 40: touch $VAR_SUBSYS_OVIRT_HOOKS Line 36: [ -f "$VAR_SUBSYS_OVIRT_HOOKS" ] && exit 0 Line 37: { Line 38: log "Starting ovirt-hooks service" Line 39: Line 40: touch $VAR_SUBSYS_OVIRT_HOOKS we can drop this as well Line 41: # Run on-boot hooks Line 42: if [[ -d "$HOOK_ON_BOOT_DIR" ]] && [[ "$(ls -A $HOOK_ON_BOOT_DIR)" ]]; Line 43: then Line 44: for handler in "$HOOK_ON_BOOT_DIR"/*; Line 38: log "Starting ovirt-hooks service" Line 39: Line 40: touch $VAR_SUBSYS_OVIRT_HOOKS Line 41: # Run on-boot hooks Line 42: if [[ -d "$HOOK_ON_BOOT_DIR" ]] && [[ "$(ls -A $HOOK_ON_BOOT_DIR)" ]]; Thjis until line 45 is the only stuff needed Line 43: then Line 44: for handler in "$HOOK_ON_BOOT_DIR"/*; Line 45: do Line 46: log "Running handler: $handler" Line 47: "$handler" >> $OVIRT_LOGFILE 2>&1 Line 48: done Line 49: fi Line 50: Line 51: rm -f $VAR_SUBSYS_OVIRT_HOOKS not needed Line 52: Line 53: log "Completed ovirt-hooks" Line 54: } >> $OVIRT_LOGFILE 2>&1 Line 55: } Line 53: log "Completed ovirt-hooks" Line 54: } >> $OVIRT_LOGFILE 2>&1 Line 55: } Line 56: Line 57: stop_ovirt_hooks () { drop this, not needed Line 58: { Line 59: log "Stopping ovirt-hooks" Line 60: # Placeholder in the unlikely case we want to add hooks for shutdown Line 61: log "Stopped ovirt-hooks" Line 61: log "Stopped ovirt-hooks" Line 62: } >> $OVIRT_LOGFILE 2>&1 Line 63: } Line 64: Line 65: restart_ovirt_hooks () { drop this, not needed Line 66: echo "Restarting ovirt-hooks is not implemented" Line 67: return 0 Line 68: } Line 69: Line 66: echo "Restarting ovirt-hooks is not implemented" Line 67: return 0 Line 68: } Line 69: Line 70: status_ovirt_hooks () { drop this, not needed Line 71: return 0 Line 72: } Line 73: Line 74: reload_ovirt_hooks () { Line 70: status_ovirt_hooks () { Line 71: return 0 Line 72: } Line 73: Line 74: reload_ovirt_hooks () { drop this, not needed Line 75: echo "Reloading ovirt-hooks is not implemented" Line 76: return 0 Line 77: } Line 78: Line 75: echo "Reloading ovirt-hooks is not implemented" Line 76: return 0 Line 77: } Line 78: Line 79: case "$1" in We just need one case: trigger Line 80: "start") Line 81: start_ovirt_hooks Line 82: ;; Line 83: "stop") https://gerrit.ovirt.org/#/c/39465/2/services/init.d/ovirt-hooks File services/init.d/ovirt-hooks: Line 6: hooks Let's call it ovirt-node-hooks We also need to ensure that it really is something like S95 in /etc/rc.d… Line 5: ### BEGIN INIT INFO Line 6: # Provides: ovirt-hooks Line 7: # Required-Start: vdsmd Line 8: # Default-Start: 2 3 4 5 Line 9: # Description: Runs managed node post configuration hooks. Trailing whitespace Line 10: ### END INIT INFO Line 11: Line 12: prog=ovirt-hooks Line 13: Line 13: Line 14: case "$1" in Line 15: start) Line 16: echo -n "Starting ovirt-hooks: " Line 17: /usr/libexec/ovirt-hooks start this would become …/ovirt-node-hook trigger on-boot Line 18: test $? == 0 && success || failure Line 19: echo Line 20: ;; Line 21: status) Line 17: /usr/libexec/ovirt-hooks start Line 18: test $? == 0 && success || failure Line 19: echo Line 20: ;; Line 21: status) drop this Line 22: status -l $prog Line 23: ;; Line 24: reload) Line 25: /usr/libexec/ovirt-hooks reload Line 20: ;; Line 21: status) Line 22: status -l $prog Line 23: ;; Line 24: reload) drop this Line 25: /usr/libexec/ovirt-hooks reload Line 26: ;; Line 27: stop) Line 28: echo -n "Stopping ovirt-hooks: " Line 23: ;; Line 24: reload) Line 25: /usr/libexec/ovirt-hooks reload Line 26: ;; Line 27: stop) drop this Line 28: echo -n "Stopping ovirt-hooks: " Line 29: /usr/libexec/ovirt-hooks stop Line 30: test $? == 0 && success || failure Line 31: echo Line 30: test $? == 0 && success || failure Line 31: echo Line 32: ;; Line 33: *) Line 34: echo "Usage: ovirt-hooks {start}" would be: Usgae: … start Line 35: exit 2 https://gerrit.ovirt.org/#/c/39465/2/services/ovirt-hooks.service File services/ovirt-hooks.service: Line 7: [Service] Line 8: Type=oneshot Line 9: RemainAfterExit=yes Line 10: ExecStart=/usr/libexec/ovirt-hooks start Line 11: ExecStop=/usr/libexec/ovirt-hooks stop Drop this Line 12: Line 13: [Install] Line 8: Type=oneshot Line 9: RemainAfterExit=yes Line 10: ExecStart=/usr/libexec/ovirt-hooks start Line 11: ExecStop=/usr/libexec/ovirt-hooks stop Line 12: We need an After= here After=vdsmd.service ovirt-post.service libvirtd.service Line 13: [Install] -- To view, visit https://gerrit.ovirt.org/39465 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0fac31b94ed0531c3c10d2c86fd7f819549cc3a1 Gerrit-PatchSet: 2 Gerrit-Project: ovirt-node Gerrit-Branch: master Gerrit-Owner: Ryan Barry <[email protected]> Gerrit-Reviewer: Fabian Deutsch <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-HasComments: Yes _______________________________________________ node-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/node-patches
