Skip to content

Retry transient Vagrant Cloud 502 errors - #886

Open
jakduch wants to merge 1 commit into
theforeman:masterfrom
jakduch:fix/vagrant-cloud-retry
Open

jakduch wants to merge 1 commit into
theforeman:masterfrom
jakduch:fix/vagrant-cloud-retry

Conversation

@jakduch

@jakduch jakduch commented Sep 26, 2026

Copy link
Copy Markdown
Contributor

Why are you introducing these changes? (Problem description, related links)

Vagrant Cloud returned HTTP 502 while the test workflow fetched CentOS box metadata. This caused 14 Start VMs failures in run 36267392496 before deployment or tests began. The same pattern affected four earlier PR runs.

Closes #885.

What are the changes introduced in this pull request?

  • Retry vagrant up up to three attempts with a 15-second delay when its output contains both a Vagrant box metadata/file download error and HTTP 502.
  • Keep unrelated Vagrant failures fail-fast. A persistent 502 still fails after the third attempt.

How to test this pull request

Steps to reproduce:

  • Run ansible-playbook --syntax-check -i localhost, development/playbooks/vms/vms.yaml -e vm_action=start -e inventory_dir=/tmp.
  • Run ansible-lint development/playbooks/vms/vms.yaml.
  • With a fake vagrant command, verify that two matching 502 failures followed by success produce three attempts, a non-matching error produces one attempt, and a persistent matching 502 fails after three attempts.

Checklist

  • Tests added/updated (if applicable)
  • Documentation updated (not applicable; test infrastructure only)

@coderabbitai

coderabbitai Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 46210b03-5de5-438d-9b85-20a5bd3abc3d

📥 Commits

Reviewing files that changed from the base of the PR and between 6c79648 and 1f904a2.

📒 Files selected for processing (1)
  • development/playbooks/vms/vms.yaml

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The vagrant up task now retries up to two times, with a 15-second delay, when it fails and its combined output contains HTTP 502 and a recognized box metadata or file-download error.

Changes

Vagrant VM startup

Layer / File(s) Summary
Retry qualifying startup failures
development/playbooks/vms/vms.yaml
The task registers the command result and retries up to two times when the failure output includes HTTP 502 and a recognized download error. It waits 15 seconds between attempts. Success or output that does not meet these conditions stops retries.

Priority: ⬆️ High

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 1f904

The Vagrant startup task allows three attempts for qualifying HTTP 502 failures. No merge-blocking issue remains from this review.

Architecture Summary

Architecture risk: 🔵 Low · up to 1f904

The change affects 1 system.

Changed systems: development

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — development (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

  • observed — Modified behavior in development/playbooks/vms/vms.yaml: The vagrant up task now registers its result and retries up to two times, waiting 15 seconds between attempts. Retries continue only when the command fails and combined stdout/stderr contains HTTP 502 plus at least one of the specified box metadata or file-download errors; success, absence of HTTP 502, or absence of both download errors ends retries.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: retrying transient Vagrant Cloud HTTP 502 errors.
Description check ✅ Passed The description directly explains the transient HTTP 502 problem, the bounded retry behavior, fail-fast conditions, and testing approach.
Linked Issues check ✅ Passed The change implements the coding requirements in [#885]. The vagrant up task registers command output and retries only when the command fails, the combined stdout and stderr contain HTTP 502, and ei…
Out of Scope Changes check ✅ Passed The whole-PR diff changes only the vagrant up task in development/playbooks/vms/vms.yaml. The register, condition, retry limit, and delay directly support [#885]. No unrelated implementation or re…
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Retry HTTP 502 box metadata and download failures up to three times while keeping unrelated Vagrant errors fail-fast.

Refs theforeman#885
@jakduch
jakduch force-pushed the fix/vagrant-cloud-retry branch from a5d6152 to 1f904a2 Compare September 26, 2026 20:21
@jakduch

jakduch commented Sep 26, 2026

Copy link
Copy Markdown
Contributor Author

Updated in 1f904a2: retries is now 2, which gives one initial invocation plus two retries (three total attempts).

An exact Ansible 2.21 harness verified:

  • transient matching 502: success on invocation 3;
  • persistent matching 502: failure after exactly 3 invocations;
  • non-matching Vagrant error: immediate failure after 1 invocation.

The playbook syntax check and targeted ansible-lint both pass. Fresh CI is running in workflow run 36269253283.

@jakduch

jakduch commented Sep 26, 2026

Copy link
Copy Markdown
Contributor Author

The updated CI run is fully green: all 25 jobs passed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Retry transient Vagrant Cloud 502 errors when starting test VMs

1 participant