Skip to content

LCORE-1869: Add graceful teardown and automatic cleanup for llama-stack container #1802

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
anik120 wants to merge 1 commit into
base: main
Choose a base branch
from
+20 −3
Open

LCORE-1869: Add graceful teardown and automatic cleanup for llama-stack container #1802

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 20 additions & 3 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,14 @@ LLAMA_STACK_IMAGE ?= lightspeed-llama-stack:local
LLAMA_STACK_PORT ?= 8321
CONTAINER_RUNTIME ?= $(shell command -v podman 2>/dev/null || command -v docker 2>/dev/null)

.PHONY: run run-stack build-llama-stack-image remove-llama-stack-container start-llama-stack-container wait-for-llama-stack-health clean-llama-stack
.PHONY: run run-stack build-llama-stack-image remove-llama-stack-container stop-llama-stack-container start-llama-stack-container wait-for-llama-stack-health clean-llama-stack

run-stack: ## Run lightspeed-stack directly, without building dependent service/s
uv run src/lightspeed_stack.py -c $(CONFIG)

run: start-llama-stack-container ## Run the service locally with dependent services
@echo "Starting Lightspeed Core Stack..."
@trap 'echo ""; echo "Stopping services..."; $(MAKE) stop-llama-stack-container' EXIT INT TERM; \
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Jun 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

make run still leaves the container behind on the common exit path.

Line 27 only invokes stop-llama-stack-container, but the actual log-preserving cleanup lives in remove-llama-stack-container. After a normal Ctrl+C/EXIT you keep a stopped lightspeed-llama-stack container around and never write /tmp/llama-stack-last-run.log, which misses the stated automatic cleanup behavior.

Also applies to: 51-57

🤖 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 `@Makefile` at line 27, The trap in the Makefile's run path only calls
stop-llama-stack-container which leaves the stopped lightspeed-llama-stack
container and skips the log-preserving cleanup in remove-llama-stack-container;
update the trap to call remove-llama-stack-container (or both
stop-llama-stack-container then remove-llama-stack-container) so
/tmp/llama-stack-last-run.log is always written on EXIT/INT/TERM, and make the
same change for the other trap(s) referenced around lines 51-57 to ensure
consistent cleanup across all exit paths.

$(MAKE) run-stack
Comment on lines 25 to 28
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Jun 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail
# Inspect Makefile around the cited line ranges with line numbers
if [ -f Makefile ]; then
  wc -l Makefile
  sed -n '1,220p' Makefile | nl -ba | sed -n '1,220p'
else
  echo "Makefile not found at repo root"
fi

Repository: lightspeed-core/lightspeed-stack

Length of output: 150

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Makefile line 15-40 =="
awk 'NR>=15 && NR<=40 {printf "%5d\t%s\n", NR, $0}' Makefile

echo "== Makefile line 55-130 =="
awk 'NR>=55 && NR<=130 {printf "%5d\t%s\n", NR, $0}' Makefile

echo "== Makefile line 130-200 =="
awk 'NR>=130 && NR<=200 {printf "%5d\t%s\n", NR, $0}' Makefile

Repository: lightspeed-core/lightspeed-stack

Length of output: 9588

🏁 Script executed:

#!/bin/bash
set -euo pipefail

awk 'NR>=30 && NR<=90 {printf "%5d\t%s\n", NR, $0}' Makefile

Repository: lightspeed-core/lightspeed-stack

Length of output: 3415

Fix Makefile cleanup: trap doesn’t cover prerequisite startup failures, and shutdown doesn’t remove/save logs

  • run installs the trap only inside the run recipe, but start-llama-stack-container runs first as a prerequisite; it starts the container (run -d) and calls wait-for-llama-stack-health, which exit 1s after the timeout. When this fails, the run recipe (and trap) never executes, so the container can be left running.
  • The trap calls stop-llama-stack-container (stop/kill only), while log saving + removal (/tmp/llama-stack-last-run.log) happens only in remove-llama-stack-container. On normal make run shutdown you’ll stop but not remove the container or preserve the “last run” logs.
🤖 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 `@Makefile` around lines 25 - 28, The run recipe's trap is never installed
before Makefile prerequisites run (so start-llama-stack-container can fail and
leave containers running) and the trap only calls stop-llama-stack-container
(not remove-llama-stack-container which saves/removes logs). Fix by moving the
trap into a wrapper recipe that executes start-llama-stack-container and
run-stack inside the same shell (so the trap is active for startup failures),
change the trap handler to call remove-llama-stack-container (ensuring
/tmp/llama-stack-last-run.log is saved/removed), and update
start-llama-stack-container or wait-for-llama-stack-health to perform cleanup on
timeout/failure (call the same removal path) so no container or logs are left
orphaned; reference targets: run, start-llama-stack-container, run-stack,
wait-for-llama-stack-health, stop-llama-stack-container,
remove-llama-stack-container and the /tmp/llama-stack-last-run.log artifact.


build-llama-stack-image: remove-llama-stack-container ## Build llama-stack container image
Expand All @@ -34,10 +35,26 @@ build-llama-stack-image: remove-llama-stack-container ## Build llama-stack conta
fi
$(CONTAINER_RUNTIME) build -f deploy/llama-stack/test.containerfile -t $(LLAMA_STACK_IMAGE) .

remove-llama-stack-container: ## Remove existing llama-stack container
stop-llama-stack-container: ## Gracefully stop llama-stack container
@if [ -n "$(CONTAINER_RUNTIME)" ] && $(CONTAINER_RUNTIME) inspect $(LLAMA_STACK_CONTAINER_NAME) >/dev/null 2>&1; then \
echo "Removing existing llama-stack container..."; \
echo "Stopping llama-stack container (timeout: 10s)..."; \
if $(CONTAINER_RUNTIME) stop -t 10 $(LLAMA_STACK_CONTAINER_NAME) 2>/dev/null; then \
echo "✓ Container stopped gracefully"; \
else \
echo "⚠ Container did not stop gracefully, capturing logs..."; \
$(CONTAINER_RUNTIME) logs $(LLAMA_STACK_CONTAINER_NAME) > /tmp/llama-stack-failure.log 2>&1 || true; \
echo "Logs saved to /tmp/llama-stack-failure.log"; \
$(CONTAINER_RUNTIME) kill $(LLAMA_STACK_CONTAINER_NAME) 2>/dev/null || true; \
fi; \
fi

remove-llama-stack-container: ## Remove llama-stack container (saves logs first)
@if [ -n "$(CONTAINER_RUNTIME)" ] && $(CONTAINER_RUNTIME) inspect $(LLAMA_STACK_CONTAINER_NAME) >/dev/null 2>&1; then \
echo "Saving container logs before removal..."; \
$(CONTAINER_RUNTIME) logs $(LLAMA_STACK_CONTAINER_NAME) > /tmp/llama-stack-last-run.log 2>&1 || true; \
echo "Removing llama-stack container..."; \
$(CONTAINER_RUNTIME) rm -f $(LLAMA_STACK_CONTAINER_NAME); \
echo "✓ Container removed (logs saved to /tmp/llama-stack-last-run.log)"; \
fi

start-llama-stack-container: build-llama-stack-image ## Start llama-stack container
Expand Down
Loading