Skip to content

Correcting for install failures in chef-upgrader cookbooks - #431

Draft
johnmccrae wants to merge 3 commits into
mainfrom
jfm/windows_install_issue
Draft

Correcting for install failures in chef-upgrader cookbooks#431
johnmccrae wants to merge 3 commits into
mainfrom
jfm/windows_install_issue

Conversation

@johnmccrae

Copy link
Copy Markdown

Summary

This PR addresses a race condition between CDN cache propagation and Chef package
availability that could leave Windows nodes without a working Chef client after a
failed upgrade.

Two independent fixes are included, both working together to harden the upgrade path.


Problem

When Expeditor promotes a new Chef package from current to stable:

  1. mixlib-install calls the packages API and receives the new version metadata
  2. It constructs a download URL for the new package
  3. The CDN has not yet propagated the file to all edge nodes
  4. The download fails or retrieves a stale/incorrect package
  5. On Windows, because the upgrade process must move the C:\opscode\chef directory
    before installing, a failed download leaves the node with no Chef directory and
    no working client — the node becomes unreachable

Fix 1: Download URL Validation with Exponential Backoff

File: lib/mixlib/install/backend/package_router.rb

Before returning artifact information to any caller, mixlib-install now validates
that the resolved download URL is actually reachable. If the URL is not yet available,
it retries with exponential backoff before giving up.

  • Overrides info to call validate_artifact_url when platform filters are present
  • Performs up to MAX_DOWNLOAD_VALIDATE_RETRIES (3) HTTP HEAD requests against the
    download URL
  • Sleeps 2^n seconds between attempts (2s, then 4s) to allow CDN propagation
  • Raises ArtifactsNotFound with a clear message if the URL never becomes reachable,
    preventing the upgrade from starting at all
  • Non-HTTP errors (DNS failure, SSL error, connection refused) propagate as real
    errors rather than being masked as CDN issues
  • Nil or empty artifact URLs raise ArtifactsNotFound immediately

This fix benefits all mixlib-install consumers on all platforms.


Fix 2: Windows Backup and Restore Recovery

File: lib/mixlib/install/generator/powershell/scripts/install_project.ps1.erb

Three new PowerShell helper functions wrap the Windows install loop to ensure that a
failed upgrade always leaves the node with a working Chef installation.

Backup-ChefInstallation

  • Copies the existing install directory to <install_dir>.upgrade-backup before any
    destructive operation begins
  • Returns $null on fresh installs (no existing directory), which causes all
    downstream restore logic to no-op safely
  • If the backup itself fails, the upgrade is aborted before the original directory
    is touched

Restore-ChefInstallation

  • Called automatically when any exception is raised inside the install loop
  • Verifies the backup exists before attempting a restore
  • Removes the (potentially partial) install directory and moves the backup back into
    place
  • Guards Move-Item with a post-remove Test-Path check: if the directory could
    not be fully removed (e.g., locked files), it throws explicitly rather than moving
    the backup inside the partial directory

Remove-ChefBackup

  • Removes the backup directory after a confirmed successful install
  • Intentionally isolated in its own try/catch block outside the install
    try/catch, so a cleanup failure after a successful upgrade never triggers a rollback

Manual Recovery

If the restore itself fails, the operator receives:

  • The path to the still-intact backup directory
  • A two-step manual recovery command to clear any partial install and restore the
    backup, avoiding the need to re-image the node

Tests Added

spec/unit/mixlib/install/backend/package_router_spec.rb

  • URL accessible on first attempt: returns artifact, checks URL exactly once
  • URL never accessible: raises ArtifactsNotFound, retries correct number of times,
    sleeps with exponential backoff, includes product info in error message
  • URL accessible on retry: returns artifact, performs correct number of checks,
    sleeps once
  • Platform filters unavailable: skips validation entirely
  • Nil URL: raises immediately, makes no HTTP requests
  • Empty URL: raises immediately, makes no HTTP requests
  • Non-HTTP error (SocketError): propagates rather than being masked as a CDN issue

spec/unit/mixlib/install/generator_spec.rb

  • Generated PS1 script includes all three helper functions
  • Install loop calls backup before install and restore on failure
  • Restore failure prints a two-step manual recovery warning
  • Backup cleanup is in a separate try/catch from the install block
  • Original install error is always re-thrown after a restore attempt

Related Issue

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Chore (non-breaking change that does not add functionality or fix an issue)

Checklist:

  • I have read the CONTRIBUTING document.
  • I have run the pre-merge tests locally and they pass.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • If Gemfile.lock has changed, I have used --conservative to do it and included the full output in the Description above.
  • All new and existing tests passed.
  • All commits have been signed-off for the Developer Certificate of Origin.

Signed-off-by: John McCrae <john.mccrae@progress.com>
@johnmccrae
johnmccrae requested review from a team and jaymzh as code owners July 22, 2026 00:16
Signed-off-by: John McCrae <john.mccrae@progress.com>
Signed-off-by: John McCrae <john.mccrae@progress.com>
@johnmccrae
johnmccrae force-pushed the jfm/windows_install_issue branch from 0ed9660 to 0e00da1 Compare July 22, 2026 21:00

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes break things as the mixlib-install is not generating the scripts to do the install for chef_client_updater cookbook. These generator scripts are for the install.sh/ps1 scripts generated for the downloads api.

Comment on lines +317 to +340
# CAUTION: chef_client_updater cookbook compatibility
#
# The chef_client_updater cookbook calls Install-Project indirectly on Windows.
# Its upgrade flow:
# 1. Calls mixlib_install.install_command to get the install script.
# 2. Calls prepare_windows, which copies C:\opscode\chef -> C:\opscode\chef.upgrade
# and creates a scheduled task named chef_upgrade (or <product>_upgrade).
# 3. The scheduled task script runs Remove-Item "C:\opscode\chef" -Recurse -Force,
# then invokes the install script from step 1, which calls Install-Project.
# 4. On Install-Project failure the scheduled task catch block runs:
# Move-Item "C:\opscode\chef.upgrade" "C:\opscode\chef" to restore from backup.
#
# Because the install directory is removed before Install-Project is called,
# Backup-ChefInstallation finds no directory at that path and returns $null.
# Restore-ChefInstallation and Remove-ChefBackup both guard on $null and return
# early. All three functions are no-ops in the chef_client_updater flow.
#
# There is no directory naming conflict: chef_client_updater uses the suffix
# .upgrade (e.g. C:\opscode\chef.upgrade) while Install-Project uses
# .upgrade-backup (e.g. C:\opscode\chef.upgrade-backup).

# Copies the existing product installation directory to a timestamped backup path
# before a destructive upgrade begins. Returns the backup path, or $null if there
# was no existing installation to back up.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assumes this is only used for the chef_client_updater CB and not for omnitruck, commercial downloads api, and test-kitchen install.sh/ps1 script generation for installing chef-client on a new system.

@tpowell-progress
tpowell-progress marked this pull request as draft July 28, 2026 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants