rackerlabs / auter

Automatic updates for RHEL, Debian, and their derivatives, with the ability to run pre/post hooks & reboot afterwards.
Apache License 2.0
67 stars 18 forks source link

check_package_manager_lock should run inside SLEEP_DELAY #206

Closed mark-hyde closed 5 years ago

mark-hyde commented 5 years ago

Auter currently checks for a package manager lock shortly after entering the apply_updates function, but since the actual yum command may not run until several hours later, should this test instead run after the SLEEP_DELAY has completed as currently it will not detect a concurrent yum update at the time that yum might actually be running.

rhodesn commented 5 years ago

Resolved in #210, thanks!

reaperzn commented 5 years ago

Hey @nrhodes91 Thanks for the timely fix.

While #210 does fix the specific case that @racker-markh mentioned, after reviewing the PR, maybe we should also look at moving the check_package_manager_lock call to after the pre-apply scripts loop. I was thinking along the same lines as Mark's issue where there may be a case where pre-apply scripts may take time to run or worse still, may perform some yum function which may cause a yum lock. The proposal here is to change: https://github.com/rackerlabs/auter/blob/develop/auter.yumdnfModule#L148...L162

   if [[ "${RC}" -eq 100 ]]; then
    # Sleep before continuing with pre-scripts and updates
    SLEEP_DELAY=$((RANDOM % MAXDELAY))
    [[ ${SLEEP_DELAY} -gt 1 ]] && logit "INFO: Sleeping for ${SLEEP_DELAY} seconds"
    sleep ${SLEEP_DELAY}

    # Check for yum.lock file
    check_package_manager_lock

    for SCRIPT in "${PREAPPLYSCRIPTDIR}"/*; do
      run_script "${SCRIPT}" "Pre-Apply"
    done

    logit "INFO: Applying updates"
    HISTORY_BEFORE=$(${PACKAGE_MANAGER} history list)

to:

  if [[ "${RC}" -eq 100 ]]; then
    # Sleep before continuing with pre-scripts and updates
    SLEEP_DELAY=$((RANDOM % MAXDELAY))
    [[ ${SLEEP_DELAY} -gt 1 ]] && logit "INFO: Sleeping for ${SLEEP_DELAY} seconds"
    sleep ${SLEEP_DELAY}

    for SCRIPT in "${PREAPPLYSCRIPTDIR}"/*; do
      run_script "${SCRIPT}" "Pre-Apply"
    done

    # Check for yum.lock file
    check_package_manager_lock

    logit "INFO: Applying updates"
    HISTORY_BEFORE=$(${PACKAGE_MANAGER} history list)
rhodesn commented 5 years ago

I understand where you are coming from, however if for example a pre-script stops a service in prep for patching, and then auter exits due to the yum lock, the system is in an unknown state. If we exit due to a yum lock before running any scripts, at least we know the state of the system has not changed.

reaperzn commented 5 years ago

Point taken. Then I'll re-close this issue.