Repository navigation
fix(decdn_node): create the fs cache-origin directory for kind: fs - #31
Conversation
When `decdn_cache_origin_kind: fs`, the role rendered
`[cache.origin] path = {{ decdn_cache_origin_path }}` into node.toml but
never created that directory. The daemon's `FilesystemOrigin::new` fails
fast if the base path is missing or is not a directory, and the unit is
`Restart=always`/`RestartSec=5`, so a `kind: fs` deploy that didn't
pre-create the dir out-of-band crash-looped on every (re)start.
Add a `when: kind == fs` task that creates `decdn_cache_origin_path`
(owner decdn:decdn, mode 0755) right after the existing data/cache/config
dir task, making a `kind: fs` deploy self-contained. The sharded blob
files under it ({path}/{hex[0..2]}/{hex}) are content, not config, and
stay an operator concern — only the base dir is role-managed.
Test coverage: switch the molecule `default` scenario from an http origin
to an fs origin so the new task actually runs, and assert in verify.yml
that the dir exists as decdn:decdn/0755 plus a node.toml [cache.origin]
content check. http-origin template coverage is retained by the
generate-keystore and slow-readiness scenarios.
Closes #28
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 52 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe role now provisions a filesystem cache-origin directory when configured for ChangesFilesystem cache origin
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
🚥 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request configures and tests a filesystem pull-through origin (kind: fs) for the decdn_node role, ensuring that the base directory is automatically created with the correct ownership and permissions. Feedback on the changes suggests combining the Ansible assertion conditions in verify.yml into a single short-circuiting expression to prevent AnsibleUndefinedVariable errors if the directory does not exist.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
Pull request overview
This PR fixes a deployment gap in the decdn_node Ansible role: when decdn_cache_origin_kind: fs, the role rendered an on-disk cache origin path into node.toml but didn’t create the corresponding directory, leading to daemon crash-loops on startup. It also updates the default Molecule scenario to exercise and verify this behavior in CI.
Changes:
- Add a conditional Ansible task to create
decdn_cache_origin_pathwhendecdn_cache_origin_kind == "fs". - Switch Molecule
defaultconverge to use anfsorigin and set a concrete origin path. - Extend Molecule verification to assert
[cache.origin]content innode.tomland that the origin directory exists with correct ownership/mode.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| ansible/roles/decdn_node/tasks/main.yml | Create the filesystem cache-origin base directory when kind: fs before install/service start. |
| ansible/molecule/default/converge.yml | Configure the default Molecule scenario to use an fs origin so the new task runs in CI. |
| ansible/molecule/default/verify.yml | Verify [cache.origin] TOML output and assert the origin directory exists with expected owner/group/mode. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…t 0755 rationale - verify.yml: combine the fs cache-origin dir assertions into one short-circuiting expression (file convention) so a missing dir routes to fail_msg instead of raising AnsibleUndefinedVariable on the absent pw_name/gr_name/mode — the regression this assert is meant to catch. - tasks/main.yml: reword the 0755 rationale — 0755 grants no write, so drop the misleading "populate as a different user" claim; state that the blobs are public content and 0755 only widens read access if the path is relocated outside the 0700 parent. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Summary
Fixes #28. When
decdn_cache_origin_kind: fs, thedecdn_noderole rendered[cache.origin] path = …intonode.tomlbut never created that directory. The daemon'sFilesystemOrigin::newfails fast on a missing/non-directory base path, and the unit isRestart=always/RestartSec=5— so akind: fsdeploy that didn't pre-create the dir out-of-band crash-looped on every (re)start until an operator made it by hand.Changes
roles/decdn_node/tasks/main.yml— newwhen: decdn_cache_origin_kind == "fs"task creatingdecdn_cache_origin_path(ownerdecdn:decdn, mode0755, per the issue), placed after the existing data/cache/config dir task so the dir exists before install +systemd … started. Idempotent and check-mode safe. Only the base dir is role-managed; the sharded blobs ({path}/{hex[0..2]}/{hex}) remain an operator concern.molecule/default/converge.yml— switched thedefaultscenario's origin fromhttptofs(/var/lib/decdn/origin) so the new task actually runs in CI.prepare.ymldeliberately does not pre-create that dir, so the assertion is non-vacuous. http-origin template coverage is retained by thegenerate-keystoreandslow-readinessscenarios.molecule/default/verify.yml— assert the dir exists asdecdn:decdn/0755/directory, and extend the node.toml content check to the nested[cache.origin]table.Verification
yamllint -c .yamllintandansible-lint(production profile): clean.molecule test --all: exit 0 — all three scenarios pass converge → idempotence → verify. The new task shows changed on converge and ok on idempotence (idempotent); the verify assertions pass.Note: the molecule daemon is a stub, so this verifies the role's dir-creation contract (dir created with correct owner/group/mode), not the daemon crash-loop itself (that behavior lives in
decdn/decdn).🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes