chore(ci): give the incus-ci memory gate 45 min of patience - #192
Merged
Conversation
…f 15 The pre-launch capacity check in molecule/shared/create.yml has been retrying 30x30s (15 min) since the runner pool was rebuilt after the lab outage. That was fine for one PR at a time; it stops working the moment more than a couple of heavy PRs are open, because heavy scenarios (beats_security, es_kibana, upgrade_multi_node, elasticsearch_diagnostics, elasticstack_default) each need 8–20 GB and lose the memory race against a swarm of 1–2 GB jobs coming through faster. A recent 5-PR merge sequence lit up five of those with matching "No capacity: X committed + Y needed > Z available" stdouts. Bump to 90x30s (45 min) and lift the standard molecule.yml default timeout from 45 to 60 min so the gate still fails loudly with the last-attempt stdout before GitHub Actions cancels the job. The existing per-caller overrides (elasticsearch=30, modules=20) stay where they are — those jobs are small and don't starve.
📝 WalkthroughWalkthroughThe Molecule workflow timeout increased from 45 to 60 minutes. The locked container launch task increased from 30 to 90 retries, extending its retry window from 15 to 45 minutes. ChangesMolecule execution windows
Estimated code review effort: 2 (Simple) | ~5 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@molecule/shared/create.yml`:
- Around line 133-142: Update the retry handling around the capacity check and
cleanup loop in the Incus launch flow so partial containers from a failed batch
are deleted or their committed capacity is rolled back while holding the lock
before recalculating capacity. Ensure the next retry excludes resources from
earlier successful launches, allowing the capacity gate to evaluate the
remaining batch correctly.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 68cfad12-e108-4c66-be64-1430ccf4f255
📒 Files selected for processing (2)
.github/workflows/molecule.ymlmolecule/shared/create.yml
Comment on lines
+133
to
+142
| # 90 retries × 30s = 45 min. Deliberately shorter than the workflow | ||
| # timeout (60 min for the standard molecule.yml call, 180 min for | ||
| # full_stack) so the task fails loudly with the last "No capacity: | ||
| # …" stdout instead of being silently cancelled by the workflow | ||
| # timeout — that made the queue-starvation cases very hard to | ||
| # diagnose (see PR investigating the elasticsearch_roles_calculation | ||
| # / elasticstack_default post-outage hangs). | ||
| retries: 30 | ||
| # / elasticstack_default post-outage hangs). Bumped from 30 after a | ||
| # sustained multi-PR CI storm evicted every heavy scenario on the | ||
| # first attempt. | ||
| retries: 90 |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Make retries safe after a partial launch.
If an earlier incus launch succeeds and a later launch fails, the next retry performs the capacity check before the cleanup loop at Lines 117-119. The partial containers remain counted in committed_mb, while _needed_mb still includes the full batch. The retry can therefore fail the capacity gate on every attempt and consume all 90 retries.
Delete the expected containers or roll back partial launches while holding the lock before recalculating capacity.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@molecule/shared/create.yml` around lines 133 - 142, Update the retry handling
around the capacity check and cleanup loop in the Incus launch flow so partial
containers from a failed batch are deleted or their committed capacity is rolled
back while holding the lock before recalculating capacity. Ensure the next retry
excludes resources from earlier successful launches, allowing the capacity gate
to evaluate the remaining batch correctly.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The pre-launch capacity check in
molecule/shared/create.ymlhas been retrying 30×30s (15 min) since the runner pool was rebuilt after the lab outage. Fine for one PR at a time, but the moment more than a couple of heavy PRs are open — beats_security, es_kibana, upgrade_multi_node, elasticsearch_diagnostics, elasticstack_default each want 8–20 GB — they lose the memory race against a swarm of 1–2 GB scenarios cycling through faster. A recent 5-PR merge sequence lit up five of those with matching "No capacity: X committed + Y needed > Z available" stdouts before we even looked at the actual test output.Bumps
retries: 30 → 90(45 min of patience) and lifts the standardmolecule.ymldefault timeout from 45 to 60 minutes so the gate still fails loudly with the last-attempt stdout before GitHub Actions silently cancels the job. Per-caller overrides (elasticsearch=30, modules=20) stay put — those scenarios are small and dont starve.Summary by CodeRabbit