ldraney/palinks
1
0
Fork 0
Code Issues 12 Pull requests 1 Projects Releases Packages Wiki Activity Actions

Spike: Dockerfile runtime image alignment #22

Merged
ldraney merged 2 commits from 18-dockerfile-alignment-spike into main 2026-06-08 03:16:12 +00:00
Conversation 3 Commits 2 Files changed 1 +146
ldraney commented 2026-06-07 16:33:02 +00:00
Owner
Copy link

Summary

  • Analyzed the gap between palinks' manual runtime Dockerfile stage and the platform-standard ruby-rails-runtime image used by landscaping-assistant
  • Documented line-by-line comparison, port implications (80 vs 3000), and the full set of changes needed across palinks and pal-e-deployments repos
  • The swap is straightforward: runtime stage drops from ~27 lines to ~8 lines, with port alignment as the only decision point

Changes

  • docs/pipeline.md: Added "Runtime Image Alignment Spike" section covering all six investigation questions — what the runtime image provides, removable lines, port differences, pipeline impact, overlay adjustments, and ArgoCD Image Updater impact

Test Plan

  • No code changes — documentation-only spike
  • Findings verified by cross-referencing palinks Dockerfile, landscaping-assistant Dockerfile, pal-e-deployments overlays for both apps, base standard deployment, and both Woodpecker pipelines
  • Manual review of findings accuracy

Review Checklist

  • Passed automated review-fix loop
  • No secrets committed
  • No unnecessary file changes
  • Commit messages are descriptive
  • Feature flag needed? No — documentation-only spike, no code changes

Related Notes

  • ldraney/palinks #18 — Spike: Align Dockerfile and pipeline with ruby-rails-runtime pattern
  • palinks — the project this work belongs to

Closes #18

## Summary - Analyzed the gap between palinks' manual runtime Dockerfile stage and the platform-standard `ruby-rails-runtime` image used by landscaping-assistant - Documented line-by-line comparison, port implications (80 vs 3000), and the full set of changes needed across palinks and pal-e-deployments repos - The swap is straightforward: runtime stage drops from ~27 lines to ~8 lines, with port alignment as the only decision point ## Changes - `docs/pipeline.md`: Added "Runtime Image Alignment Spike" section covering all six investigation questions — what the runtime image provides, removable lines, port differences, pipeline impact, overlay adjustments, and ArgoCD Image Updater impact ## Test Plan - [x] No code changes — documentation-only spike - [x] Findings verified by cross-referencing palinks Dockerfile, landscaping-assistant Dockerfile, pal-e-deployments overlays for both apps, base standard deployment, and both Woodpecker pipelines - [ ] Manual review of findings accuracy ## Review Checklist - [x] Passed automated review-fix loop - [x] No secrets committed - [x] No unnecessary file changes - [x] Commit messages are descriptive - [x] Feature flag needed? No — documentation-only spike, no code changes ## Related Notes - `ldraney/palinks #18` — Spike: Align Dockerfile and pipeline with ruby-rails-runtime pattern - `palinks` — the project this work belongs to Closes #18
Analyze the gap between palinks' manual runtime stage and the
platform-standard ruby-rails-runtime image. Document line-by-line
comparison, port implications, and required changes across palinks
and pal-e-deployments repos.

Closes #18

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
ldraney commented 2026-06-07 16:33:38 +00:00
Author
Owner
Copy link

QA Review -- PR #22

Scope

Documentation-only spike. 146 lines added to docs/pipeline.md, zero deletions, no code changes. Single file changed.

Findings

Content quality: Strong. The spike answers all six questions from issue #18 with concrete evidence. Line-by-line Dockerfile comparison, port analysis, and cross-repo impact assessment are thorough.

Minor observations (non-blocking):

  1. "Assumed" language in the comparison table. Several cells say "Assumed pre-installed" or "Assumed set in image" for what ruby-rails-runtime provides. This is reasonable for a spike (the runtime image contents were inferred from landscaping-assistant's usage pattern rather than inspecting the image directly), but a follow-up implementation ticket should confirm these assumptions by running docker inspect or docker run against the runtime image.

  2. Mermaid diagram in "Rolling Update" section still says port 80. The existing diagram at line 78 says "Start Puma on port 80" -- this was not updated as part of the spike, which is fine since the spike is investigatory. The summary correctly notes this needs updating during implementation.

  3. No secrets or sensitive data committed. Clean.

  4. Branch naming and commit message follow conventions. 18-dockerfile-alignment-spike matches {issue-number}-{kebab-case-purpose}.

VERDICT: APPROVE

Documentation-only spike with well-structured findings. The "assumed" caveats are appropriate for investigation scope -- they should be resolved during the implementation phase. No blockers.

## QA Review -- PR #22 ### Scope Documentation-only spike. 146 lines added to `docs/pipeline.md`, zero deletions, no code changes. Single file changed. ### Findings **Content quality: Strong.** The spike answers all six questions from issue #18 with concrete evidence. Line-by-line Dockerfile comparison, port analysis, and cross-repo impact assessment are thorough. **Minor observations (non-blocking):** 1. **"Assumed" language in the comparison table.** Several cells say "Assumed pre-installed" or "Assumed set in image" for what `ruby-rails-runtime` provides. This is reasonable for a spike (the runtime image contents were inferred from landscaping-assistant's usage pattern rather than inspecting the image directly), but a follow-up implementation ticket should confirm these assumptions by running `docker inspect` or `docker run` against the runtime image. 2. **Mermaid diagram in "Rolling Update" section still says port 80.** The existing diagram at line 78 says "Start Puma on port 80" -- this was not updated as part of the spike, which is fine since the spike is investigatory. The summary correctly notes this needs updating during implementation. 3. **No secrets or sensitive data committed.** Clean. 4. **Branch naming and commit message follow conventions.** `18-dockerfile-alignment-spike` matches `{issue-number}-{kebab-case-purpose}`. ### VERDICT: APPROVE Documentation-only spike with well-structured findings. The "assumed" caveats are appropriate for investigation scope -- they should be resolved during the implementation phase. No blockers.
ldraney commented 2026-06-08 01:40:43 +00:00
Author
Owner
Copy link

PR #22 Review

DOMAIN REVIEW

Tech stack: Documentation-only change (Markdown). No code, no Dockerfile changes, no infra changes. This is a spike deliverable -- a docs/pipeline.md addition of 146 lines documenting the investigation into swapping ruby:3.4.9-slim for ruby-rails-runtime in the palinks Dockerfile runtime stage.

Content accuracy verified against source files:

  • Dockerfile (Dockerfile lines 26-52): The "palinks runtime stage (current -- 27 lines)" block in the spike is a faithful reproduction of the actual runtime stage. Line counts and content match.
  • config/puma.rb (line 32): The spike correctly states Puma reads ENV.fetch("PORT", 3000) -- verified at bind "tcp://0.0.0.0:#{ENV.fetch("PORT", 3000)}".
  • docker-compose.yml: The spike correctly states dev is unaffected. The compose file uses ruby-rails-build for dev and binds on port 9999, which is independent of the runtime image swap.
  • Mermaid diagram (docs/pipeline.md line 33): The spike correctly identifies that the note "ruby-slim final image" in the existing sequenceDiagram would need updating in the implementation ticket.
  • Port 80 usage: Confirmed PORT=80 is set in Dockerfile ENV (line 40) and EXPOSE 80 (line 51). The spike's analysis of the port 80 vs 3000 decision is sound.
  • Q5 (ArgoCD Image Updater): Correct -- Image Updater tracks palinks/app tags, not base image changes. The runtime base swap is invisible to it.
  • Q3 (Woodpecker): Correct -- .woodpecker.yaml only runs Kaniko against the Dockerfile. No pipeline changes needed.

Spike quality: The six questions are well-structured, the comparison table is clear, the line-by-line diff is useful for the implementation ticket, and the "Summary of Required Changes" section provides a concrete action list. The recommendation (Option 2: switch to port 3000) is well-reasoned.

BLOCKERS

None.

This is a documentation-only spike with no code changes. The "no test coverage" blocker does not apply -- there is nothing to test. The content is factually accurate against the source files in the repo.

NITS

  1. "Assumed" entries in comparison table: The table has five "Assumed pre-installed" / "Assumed set in image" entries for what ruby-rails-runtime provides. This is honest for a spike (you can't inspect a Harbor image without pulling it), but the implementation ticket should verify these assumptions before committing to the swap. Consider adding a note like "Verify by inspecting the runtime image: docker run --rm harbor.../ruby-rails-runtime env" to reduce risk during implementation.

  2. CMD difference could be stronger: Q6 notes the rails server vs bundle exec puma difference but frames it as "consider adopting." For a production deployment, explicit bundle exec puma -C config/puma.rb is the better choice (avoids Rails server wrapper overhead, gives direct control over Puma config). The recommendation could be more definitive.

  3. Minor wording: "Unknown Ruby version (Harbor-managed)" in the comparison table -- this could be verified with docker inspect or by checking the ruby-rails-runtime Dockerfile in whatever repo builds it. Worth linking to the source if known.

SOP COMPLIANCE

  • Branch named after issue: 18-dockerfile-alignment-spike follows {issue-number}-{kebab-case-purpose}
  • PR body follows template: Summary, Changes, Test Plan, Review Checklist, Related Notes all present
  • Related Notes references parent issue: ldraney/palinks #18
  • Closes #18 present in PR body
  • No secrets committed: no .env files, no credentials, no API keys in diff
  • No unnecessary file changes: single file changed (docs/pipeline.md), scoped to the spike
  • Commit messages: not visible in diff but PR title is descriptive

PROCESS OBSERVATIONS

  • Spike deliverable completeness: The spike produces a docs file (required output) and identifies the exact file list for the implementation ticket (required output). Both spike deliverables are met.
  • Cross-repo awareness: The spike correctly identifies that pal-e-deployments changes are needed alongside palinks changes, which means the implementation ticket should be scoped as a multi-repo change or broken into coordinated tickets.
  • Change failure risk: Low. This is documentation only. The implementation ticket (when created) will carry moderate risk due to the port change affecting k8s probes, Service definitions, and kustomization patches simultaneously.

VERDICT: APPROVED

## PR #22 Review ### DOMAIN REVIEW **Tech stack**: Documentation-only change (Markdown). No code, no Dockerfile changes, no infra changes. This is a spike deliverable -- a `docs/pipeline.md` addition of 146 lines documenting the investigation into swapping `ruby:3.4.9-slim` for `ruby-rails-runtime` in the palinks Dockerfile runtime stage. **Content accuracy verified against source files:** - **Dockerfile** (`Dockerfile` lines 26-52): The "palinks runtime stage (current -- 27 lines)" block in the spike is a faithful reproduction of the actual runtime stage. Line counts and content match. - **`config/puma.rb`** (line 32): The spike correctly states Puma reads `ENV.fetch("PORT", 3000)` -- verified at `bind "tcp://0.0.0.0:#{ENV.fetch("PORT", 3000)}"`. - **`docker-compose.yml`**: The spike correctly states dev is unaffected. The compose file uses `ruby-rails-build` for dev and binds on port 9999, which is independent of the runtime image swap. - **Mermaid diagram** (`docs/pipeline.md` line 33): The spike correctly identifies that the note "ruby-slim final image" in the existing sequenceDiagram would need updating in the implementation ticket. - **Port 80 usage**: Confirmed `PORT=80` is set in Dockerfile ENV (line 40) and `EXPOSE 80` (line 51). The spike's analysis of the port 80 vs 3000 decision is sound. - **Q5 (ArgoCD Image Updater)**: Correct -- Image Updater tracks `palinks/app` tags, not base image changes. The runtime base swap is invisible to it. - **Q3 (Woodpecker)**: Correct -- `.woodpecker.yaml` only runs Kaniko against the Dockerfile. No pipeline changes needed. **Spike quality**: The six questions are well-structured, the comparison table is clear, the line-by-line diff is useful for the implementation ticket, and the "Summary of Required Changes" section provides a concrete action list. The recommendation (Option 2: switch to port 3000) is well-reasoned. ### BLOCKERS None. This is a documentation-only spike with no code changes. The "no test coverage" blocker does not apply -- there is nothing to test. The content is factually accurate against the source files in the repo. ### NITS 1. **"Assumed" entries in comparison table**: The table has five "Assumed pre-installed" / "Assumed set in image" entries for what `ruby-rails-runtime` provides. This is honest for a spike (you can't inspect a Harbor image without pulling it), but the implementation ticket should verify these assumptions before committing to the swap. Consider adding a note like "Verify by inspecting the runtime image: `docker run --rm harbor.../ruby-rails-runtime env`" to reduce risk during implementation. 2. **CMD difference could be stronger**: Q6 notes the `rails server` vs `bundle exec puma` difference but frames it as "consider adopting." For a production deployment, explicit `bundle exec puma -C config/puma.rb` is the better choice (avoids Rails server wrapper overhead, gives direct control over Puma config). The recommendation could be more definitive. 3. **Minor wording**: "Unknown Ruby version (Harbor-managed)" in the comparison table -- this could be verified with `docker inspect` or by checking the `ruby-rails-runtime` Dockerfile in whatever repo builds it. Worth linking to the source if known. ### SOP COMPLIANCE - [x] Branch named after issue: `18-dockerfile-alignment-spike` follows `{issue-number}-{kebab-case-purpose}` - [x] PR body follows template: Summary, Changes, Test Plan, Review Checklist, Related Notes all present - [x] Related Notes references parent issue: `ldraney/palinks #18` - [x] `Closes #18` present in PR body - [x] No secrets committed: no `.env` files, no credentials, no API keys in diff - [x] No unnecessary file changes: single file changed (`docs/pipeline.md`), scoped to the spike - [x] Commit messages: not visible in diff but PR title is descriptive ### PROCESS OBSERVATIONS - **Spike deliverable completeness**: The spike produces a docs file (required output) and identifies the exact file list for the implementation ticket (required output). Both spike deliverables are met. - **Cross-repo awareness**: The spike correctly identifies that `pal-e-deployments` changes are needed alongside `palinks` changes, which means the implementation ticket should be scoped as a multi-repo change or broken into coordinated tickets. - **Change failure risk**: Low. This is documentation only. The implementation ticket (when created) will carry moderate risk due to the port change affecting k8s probes, Service definitions, and kustomization patches simultaneously. ### VERDICT: APPROVED
Replace "Assumed" and "Unknown" entries in the ruby-rails-runtime comparison
table with verified facts from the base-images Dockerfile. Make the CMD
recommendation definitive: switch to explicit puma invocation, matching both
the landscaping-assistant pattern and the base-images README template.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
ldraney commented 2026-06-08 02:14:37 +00:00
Author
Owner
Copy link

PR #22 Review

DOMAIN REVIEW

Tech stack: Documentation-only PR (Markdown). The spike content covers Docker, Kubernetes/Kustomize, ArgoCD, and Woodpecker CI -- so infrastructure domain expertise applies for factual verification.

Factual accuracy verified against the codebase:

  • The palinks Dockerfile runtime stage (lines 26-52) matches the spike's "current -- 27 lines" listing exactly. Port 80, rails server CMD, manual apt-get, jemalloc symlink, user creation -- all confirmed.
  • config/puma.rb line 32 confirms ENV.fetch("PORT", 3000) -- the spike's claim about Puma defaulting to 3000 is correct.
  • .woodpecker.yaml confirmed: only runs Kaniko build-and-push, does not reference the runtime image directly. The spike's Q3 answer ("no pipeline changes needed") is accurate.
  • docker-compose.yml confirmed: uses ruby-rails-build for dev, not the runtime image. The spike's "no changes needed" claim is correct.
  • The build stage already uses harbor.tail5b443a.ts.net/library/ruby-rails-build:latest (Dockerfile line 5), consistent with the spike's description.

Content quality:

The spike is thorough. It covers all six investigation questions with concrete line counts, file paths, and specific port numbers. The comparison table between palinks' manual setup and ruby-rails-runtime pre-baked content is clear and useful. The recommendation (Option 2: switch to port 3000) is well-reasoned.

The Q6 (CMD difference) section adds genuine value -- explaining that rails server wraps Puma with development middleware overhead in production is a worthwhile finding.

The "Summary of Required Changes" section at the bottom is a solid handoff for the implementation ticket, listing exact files and exact values to change in both repos.

One factual caveat (not a blocker): The spike states the base-images Dockerfile uses RUBY_VERSION ARG to set the Ruby version, and that the runtime image pre-bakes identical WORKDIR, user creation, and env vars. These claims reference external repos (base-images, landscaping-assistant, pal-e-deployments) that I cannot verify from this repo alone. The palinks-side claims are all verified.

BLOCKERS

None. This is a documentation-only spike with no code changes. No test coverage, input validation, or security concerns apply.

NITS

  1. The spike's mermaid diagram note in the existing pipeline.md (line 33) says "ruby-slim final image" -- the spike correctly identifies this as needing an update in the implementation ticket, but does not update it in this PR. That is appropriate for a spike (no code changes), but worth noting that the existing doc will be temporarily inconsistent if someone reads only the top section without scrolling to the spike.

  2. Minor: The spike references base-images with a Forgejo URL (https://forgejo.tail5b443a.ts.net/ldraney/base-images). If the repo is ever moved or renamed, this link will break. Not actionable for a spike doc, just noting.

SOP COMPLIANCE

  • Branch named after issue: 18-dockerfile-alignment-spike follows {issue-number}-{kebab-case-purpose} convention
  • PR body follows template: Summary, Changes, Test Plan, Review Checklist, Related Notes all present
  • Related section references parent issue (ldraney/palinks #18) and project (palinks)
  • Closes #18 present for auto-close
  • No secrets committed (documentation-only, verified no credentials in diff)
  • No unnecessary file changes (single file: docs/pipeline.md, 146 additions)
  • Commit messages: not individually visible in diff, but PR title is descriptive

PROCESS OBSERVATIONS

  • This spike demonstrates good discipline: documentation-only deliverable, no premature code changes, clear handoff to an implementation ticket.
  • The spike correctly scopes the cross-repo impact (palinks + pal-e-deployments) and identifies the port decision as the key judgment call for the implementer.
  • Change failure risk: zero -- no runtime behavior is modified by this PR.

VERDICT: APPROVED

## PR #22 Review ### DOMAIN REVIEW **Tech stack:** Documentation-only PR (Markdown). The spike content covers Docker, Kubernetes/Kustomize, ArgoCD, and Woodpecker CI -- so infrastructure domain expertise applies for factual verification. **Factual accuracy verified against the codebase:** - The palinks Dockerfile runtime stage (lines 26-52) matches the spike's "current -- 27 lines" listing exactly. Port 80, `rails server` CMD, manual `apt-get`, jemalloc symlink, user creation -- all confirmed. - `config/puma.rb` line 32 confirms `ENV.fetch("PORT", 3000)` -- the spike's claim about Puma defaulting to 3000 is correct. - `.woodpecker.yaml` confirmed: only runs Kaniko build-and-push, does not reference the runtime image directly. The spike's Q3 answer ("no pipeline changes needed") is accurate. - `docker-compose.yml` confirmed: uses `ruby-rails-build` for dev, not the runtime image. The spike's "no changes needed" claim is correct. - The build stage already uses `harbor.tail5b443a.ts.net/library/ruby-rails-build:latest` (Dockerfile line 5), consistent with the spike's description. **Content quality:** The spike is thorough. It covers all six investigation questions with concrete line counts, file paths, and specific port numbers. The comparison table between palinks' manual setup and `ruby-rails-runtime` pre-baked content is clear and useful. The recommendation (Option 2: switch to port 3000) is well-reasoned. The Q6 (CMD difference) section adds genuine value -- explaining that `rails server` wraps Puma with development middleware overhead in production is a worthwhile finding. The "Summary of Required Changes" section at the bottom is a solid handoff for the implementation ticket, listing exact files and exact values to change in both repos. **One factual caveat (not a blocker):** The spike states the `base-images` Dockerfile uses `RUBY_VERSION` ARG to set the Ruby version, and that the runtime image pre-bakes identical `WORKDIR`, user creation, and env vars. These claims reference external repos (`base-images`, `landscaping-assistant`, `pal-e-deployments`) that I cannot verify from this repo alone. The palinks-side claims are all verified. ### BLOCKERS None. This is a documentation-only spike with no code changes. No test coverage, input validation, or security concerns apply. ### NITS 1. The spike's mermaid diagram note in the existing `pipeline.md` (line 33) says "ruby-slim final image" -- the spike correctly identifies this as needing an update in the implementation ticket, but does not update it in this PR. That is appropriate for a spike (no code changes), but worth noting that the existing doc will be temporarily inconsistent if someone reads only the top section without scrolling to the spike. 2. Minor: The spike references `base-images` with a Forgejo URL (`https://forgejo.tail5b443a.ts.net/ldraney/base-images`). If the repo is ever moved or renamed, this link will break. Not actionable for a spike doc, just noting. ### SOP COMPLIANCE - [x] Branch named after issue: `18-dockerfile-alignment-spike` follows `{issue-number}-{kebab-case-purpose}` convention - [x] PR body follows template: Summary, Changes, Test Plan, Review Checklist, Related Notes all present - [x] Related section references parent issue (`ldraney/palinks #18`) and project (`palinks`) - [x] `Closes #18` present for auto-close - [x] No secrets committed (documentation-only, verified no credentials in diff) - [x] No unnecessary file changes (single file: `docs/pipeline.md`, 146 additions) - [x] Commit messages: not individually visible in diff, but PR title is descriptive ### PROCESS OBSERVATIONS - This spike demonstrates good discipline: documentation-only deliverable, no premature code changes, clear handoff to an implementation ticket. - The spike correctly scopes the cross-repo impact (palinks + pal-e-deployments) and identifies the port decision as the key judgment call for the implementer. - Change failure risk: zero -- no runtime behavior is modified by this PR. ### VERDICT: APPROVED
ldraney deleted branch 18-dockerfile-alignment-spike 2026-06-08 03:16:12 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
ldraney/palinks!22
No description provided.