Skip to content

[#3169] Made 'nginx' start after 'php' and corrected the SSH agent recovery advice in 'vortex-doctor'. - #3170

Merged
AlexSkrypnyk merged 8 commits into
mainfrom
feature/3169-nginx-php-ssh-doctor
Sep 30, 2026
Merged

AlexSkrypnyk merged 8 commits into
mainfrom
feature/3169-nginx-php-ssh-doctor

Conversation

@AlexSkrypnyk

@AlexSkrypnyk AlexSkrypnyk commented Sep 30, 2026 •

Copy link
Copy Markdown
Member

Closes #3169

Summary

nginx now lists php under depends_on in docker-compose.yml, so Compose starts php first, and vortex-doctor now recommends ahoy up --no-deps --force-recreate cli for an SSH key that's missing inside the CLI container, where it used to say pygmy restart (failed ssh-add -l check) or a bare ahoy up (key missing from Pygmy).

nginx and php both depended only on cli, so Compose started them in parallel. nginx resolves its FastCGI upstream when it loads its config, so when php hadn't joined the network yet it exited with [emerg] host not found in upstream "php", and with no restart policy on any service it stayed down. It's a race, so it only shows up now and then. The SSH advice had a separate gap: the CLI container reaches the Pygmy agent through volumes_from: container:amazeeio-ssh-agent, and /lagoon/entrypoints/10-ssh-agent.sh links SSH_AUTH_SOCK only if that socket exists when the container starts. pygmy restart creates a new amazeeio-ssh-agent and leaves a running CLI container attached to the previous agent, so following the advice reproduced the failure, and ahoy up alone does nothing because docker compose up --detach leaves a running container with an unchanged config alone.

After merge, docker compose up starts php before nginx, and ahoy up --no-deps --force-recreate cli reattaches the CLI container to the current agent so ssh-add -l passes again. This doesn't add a restart policy to nginx, doesn't touch Lagoon deployments (they ignore depends_on), and leaves the doctor's other checks alone, including Run 'pygmy up' or 'pygmy restart' to fix. for a stopped Pygmy service and After adding these lines, run 'ahoy up'. for a missing volumes_from entry, where the config change does recreate the container.

Before / After

Startup order

BEFORE - nginx.depends_on is [cli], so nginx and php start in parallel

                 ┌─────────┐
                 │   cli   │
                 └────┬────┘
            ┌─────────┴─────────┐
            ▼                   ▼
       ┌─────────┐         ┌─────────┐
       │  nginx  │         │   php   │
       └─────────┘         └─────────┘
   loads its config,       joins the network,
   looks up "php"          timing varies

   If "php" isn't on the network yet:
   [emerg] host not found in upstream "php"
   nginx exits and, with no restart policy, stays down


AFTER - nginx.depends_on is [cli, php], so php starts first

       ┌─────────┐
       │   cli   │
       └────┬────┘
            ▼
       ┌─────────┐
       │   php   │   joins the network
       └────┬────┘
            ▼
       ┌─────────┐
       │  nginx  │   loads its config, "php" resolves
       └─────────┘

Doctor advice for a missing SSH key

BEFORE - 'pygmy restart' leaves the running CLI container on the old agent

  ahoy doctor fails the SSH check:
  "SSH key was not added to the container.
   Run 'pygmy restart'."
       │
       ▼
  pygmy restart, with the stack still running
  removes every Pygmy container, then creates them again
       │
       ├───────────────────────┐
       ▼                       ▼
  amazeeio-ssh-agent           cli container
  new container, new socket    not recreated, still attached to the old agent
                               │
                               ▼
                               ssh-add -l fails, key still missing


AFTER - recreating the CLI container attaches it to the current agent

  ahoy doctor fails the SSH check:
  "SSH key was not added to the container.
   Run 'ahoy up --no-deps --force-recreate cli'."
       │
       ▼
  docker compose up --detach --no-deps --force-recreate cli
  recreates only the cli container
       │
       ▼
  /lagoon/entrypoints/10-ssh-agent.sh
  links SSH_AUTH_SOCK to the socket of the current amazeeio-ssh-agent
       │
       ▼
  ssh-add -l succeeds, the key is available in the CLI container

Changes

Startup order

  • docker-compose.yml: adds php to nginx.depends_on. The short list form resolves to condition: service_started, so nginx waits for the php container to start.
  • .vortex/tests/phpunit/Fixtures/docker-compose.{noenv,env,env_mod,env_local}.json: the 4 resolved-config fixtures gain php (condition service_started) under nginx.depends_on.
  • .vortex/installer/tests/Fixtures/handler_process/_baseline/docker-compose.yml: gains - php. The 20 scenario docker-compose.yml fixtures are diffs against the baseline, so they only move their @@ hunk offsets by 1 line.

Doctor advice

  • .vortex/tooling/src/vortex-doctor: the failure for an agent that's unreachable inside the CLI container now ends with Run 'ahoy up --no-deps --force-recreate cli'., and the warning for a key missing from Pygmy now ends with Run 'pygmy restart' and then 'ahoy up --no-deps --force-recreate cli'.
  • .vortex/tooling/tests/unit/doctor.bats (new, the script had no BATS file): 5 tests with strict steps_run mocks for pygmy and docker. A key available in the CLI container passes. An unreachable agent (ssh-add -l exits 2) fails with the recreate advice and not Run 'pygmy restart'.. A key missing from Pygmy warns with pygmy restart plus the recreate command and never reaches ssh-add -l. A volume that isn't mounted warns with the volumes_from instructions. VORTEX_DOCTOR_CHECK_SSH=0 makes 0 calls to pygmy or docker.

Docs

  • .vortex/docs/content/development/environment/pygmy.mdx: new ## SSH key in the CLI container section. It explains that the CLI container links to the agent socket only at start, that pygmy restart while the stack runs (or starting the stack before Pygmy) loses the key, that ahoy up --no-deps --force-recreate cli restores it, and that ahoy doctor prints that command.

Summary by CodeRabbit

  • Bug Fixes

    • Nginx now waits for both the CLI and PHP services to start, helping avoid startup issues.
    • SSH-key diagnostics provide a command to recreate the CLI container when it cannot access the SSH agent. Running ahoy up alone may not replace the existing container.
  • Documentation

    • Added guidance on how Pygmy restarts and startup order can affect SSH-key access, how to restore it, and how ahoy doctor checks key availability.

@coderabbitai

coderabbitai Bot commented Sep 30, 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: ASSERTIVE

Plan: Team

Run ID: 2217d934-c6c1-4110-ad20-c654d318a2c0

📥 Commits

Reviewing files that changed from the base of the PR and between e2939b7 and dd1d0ec.

📒 Files selected for processing (1)
  • .vortex/tooling/tests/unit/doctor.bats

Included review availability: This review used your included allowance. 2 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 4 reviews per hour.


Walkthrough

The nginx service now starts after php. The SSH-agent diagnostics, tests, and Pygmy documentation describe recreating the CLI container when SSH-key access is unavailable.

Changes

nginx startup ordering

Layer / File(s) Summary
Add the PHP startup dependency
docker-compose.yml, .vortex/tests/phpunit/Fixtures/docker-compose.*.json
The nginx service now declares a service_started dependency on php in the Compose configuration and four test fixtures. Its existing dependency on cli remains.

SSH-agent recovery guidance

Layer / File(s) Summary
Update SSH-agent recovery instructions
.vortex/tooling/src/vortex-doctor, .vortex/tooling/tests/unit/doctor.bats, .vortex/docs/content/development/environment/pygmy.mdx
The doctor now recommends recreating the CLI container in the relevant SSH-key failure cases. Unit tests cover available keys, agent failures, missing identities, missing SSH volume, and a disabled check. The Pygmy documentation describes stale-agent scenarios and the recreation command.

Priority: ➖ Normal

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

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to dd1d0

nginx starts after php, and the SSH-agent guidance provides a viable recovery path. No concrete merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely identifies both main changes: making nginx start after php and correcting the SSH agent recovery advice in vortex-doctor.
Linked Issues check ✅ Passed Issue #3169 is open and directly linked. The PR adds php to nginx.depends_on and updates the four Compose fixtures and installer baseline. It changes vortex-doctor to recommend `ahoy up --no-dep…
Out of Scope Changes check ✅ Passed The changed files support Issue #3169. Compose fixture updates support the dependency change. The doctor update, BATS tests, and Pygmy documentation support the SSH-agent recovery change. The PR does …
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files.
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


A rabbit checks the keys at dawn,
Then tells the CLI to start anew.
nginx waits for php to join,
The agent’s socket comes in view.
The stack starts up in order,
And bun hops home to chew.

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

@github-actions

This comment has been minimized.

@AlexSkrypnyk

This comment has been minimized.

2 similar comments
@AlexSkrypnyk

This comment has been minimized.

@AlexSkrypnyk

This comment has been minimized.

@codecov

codecov Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.65%. Comparing base (837439b) to head (dd1d0ec).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3170      +/-   ##
==========================================
- Coverage   87.02%   86.65%   -0.37%     
==========================================
  Files         114      106       -8     
  Lines        5255     5089     -166     
  Branches       49        3      -46     
==========================================
- Hits         4573     4410     -163     
+ Misses        682      679       -3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions

github-actions Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

📖 Documentation preview for this pull request has been deployed to Netlify:

https://6abd89fca000e2fc30a5a39a--vortex-docs.netlify.app

This preview is rebuilt on every commit and is not the production documentation site.

….bats' so an inherited preset can't turn off the SSH check.
…e 'PREFLIGHT' only defaults flags the fixture already sets.
@github-actions

This comment has been minimized.

1 similar comment
@github-actions

Copy link
Copy Markdown

Code coverage (threshold: 90%)

  Classes: 100.00% (1/1)
  Methods: 100.00% (2/2)
  Lines:   100.00% (230/230)
Per-class coverage
Drupal\ys_demo\Plugin\Block\CounterBlock
  Methods: 100.00% ( 2/ 2)   Lines: 100.00% ( 10/ 10)

@AlexSkrypnyk

This comment has been minimized.

2 similar comments
@AlexSkrypnyk

This comment has been minimized.

@AlexSkrypnyk

Copy link
Copy Markdown
Member Author

Code coverage (threshold: 90%)

  Classes: 100.00% (1/1)
  Methods: 100.00% (2/2)
  Lines:   100.00% (230/230)
Per-class coverage
Drupal\ys_demo\Plugin\Block\CounterBlock
  Methods: 100.00% ( 2/ 2)   Lines: 100.00% ( 10/ 10)

@AlexSkrypnyk AlexSkrypnyk added the Needs review Pull request needs a review from assigned developers label Sep 30, 2026
@AlexSkrypnyk
AlexSkrypnyk merged commit ec0f814 into main Sep 30, 2026
38 checks passed
@AlexSkrypnyk
AlexSkrypnyk deleted the feature/3169-nginx-php-ssh-doctor branch September 30, 2026 23:29
@AlexSkrypnyk AlexSkrypnyk added this to the 1.42.0 milestone Oct 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A1 Board worker 1 Needs review Pull request needs a review from assigned developers

Projects

Status: Release queue

Development

Successfully merging this pull request may close these issues.

Start nginx after php and correct the doctor's SSH agent recovery advice

1 participant