-
Notifications
You must be signed in to change notification settings - Fork 24
Add --images mode to ibutsu-pod #796
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.
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
Conversation
Reviewer's GuideAdds a new --images mode to ibutsu-pod.sh that builds and runs locally tagged Podman images for backend, worker, scheduler, flower, and frontend instead of using runtime dev mounts, while keeping existing dev behavior as the default. Sequence diagram for ibutsu-pod.sh startup with and without --imagessequenceDiagram
actor Developer
participant Script as ibutsu_pod_sh
participant Git as git_repo
participant Podman as podman
rect rgb(230,230,230)
Note over Developer,Script: Default mode (runtime dev mounts)
Developer->>Script: run ./ibutsu-pod.sh
Script->>Script: parse_args (USE_IMAGES=false)
Script->>Podman: create pod and infra containers
Script->>Podman: run backend using python_base + bind mount
Script->>Podman: run worker using python_base + bind mount
Script->>Podman: run scheduler using python_base + bind mount
Script->>Podman: run flower using python_base + bind mount
Script->>Podman: run frontend using node_base + bind mount
Script->>Podman: wait for backend and frontend health
end
rect rgb(220,240,255)
Note over Developer,Script: New --images mode (locally built images)
Developer->>Script: run ./ibutsu-pod.sh --images
Script->>Script: parse_args (USE_IMAGES=true)
Script->>Git: git_rev_parse_current_branch
Git-->>Script: IMAGE_TAG
Script->>Podman: build ibutsu-backend:IMAGE_TAG
Script->>Podman: build ibutsu-worker:IMAGE_TAG
Script->>Podman: build ibutsu-scheduler:IMAGE_TAG
Script->>Podman: build ibutsu-flower:IMAGE_TAG
Script->>Podman: build ibutsu-frontend:IMAGE_TAG
Script->>Podman: create pod and infra containers
Script->>Podman: run ibutsu-backend:IMAGE_TAG
Script->>Podman: run ibutsu-worker:IMAGE_TAG
Script->>Podman: run ibutsu-scheduler:IMAGE_TAG
Script->>Podman: run ibutsu-flower:IMAGE_TAG
Script->>Podman: run ibutsu-frontend:IMAGE_TAG
Script->>Podman: wait for backend and frontend health
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
8f08a83 to
7c729d5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes - here's some feedback:
- The IMAGE_TAG calculation is duplicated in both build_images() and the main --images branch; consider computing it once (near option parsing) and reusing the same variable to keep tag selection consistent and easier to adjust.
- The podman run blocks for backend, worker, scheduler, flower, and frontend now have large duplicated sections between the USE_IMAGES=true/false branches; extracting small helper functions or shared argument arrays would reduce repetition and make future changes less error-prone.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The IMAGE_TAG calculation is duplicated in both build_images() and the main --images branch; consider computing it once (near option parsing) and reusing the same variable to keep tag selection consistent and easier to adjust.
- The podman run blocks for backend, worker, scheduler, flower, and frontend now have large duplicated sections between the USE_IMAGES=true/false branches; extracting small helper functions or shared argument arrays would reduce repetition and make future changes less error-prone.
## Individual Comments
### Comment 1
<location> `scripts/ibutsu-pod.sh:49-58` </location>
<code_context>
echo ""
}
+# Function to build container images locally
+function build_images() {
+ echo "================================="
+ echo "Building container images..."
+
+ # Get the current git branch name for image tagging
+ IMAGE_TAG=$(git rev-parse --abbrev-ref HEAD 2>/dev/null | tr '/' '-')
+ if [[ -z "$IMAGE_TAG" ]]; then
+ IMAGE_TAG="local"
+ fi
+ echo "Using image tag: ${IMAGE_TAG}"
+
+ # Build backend images from the backend directory
</code_context>
<issue_to_address>
**suggestion:** Avoid duplicating IMAGE_TAG resolution logic between build_images and the main flow
IMAGE_TAG is computed inside build_images and again near the `if [[ $USE_IMAGES = true ]]` block, leading to two slightly different implementations of the same logic. Centralize this into a helper or compute IMAGE_TAG once near the top of the script so both build_images and the run path share a single, consistent value.
Suggested implementation:
```
echo ""
}
# Resolve IMAGE_TAG once so it can be reused across the script
function resolve_image_tag() {
if [[ -n "${IMAGE_TAG:-}" ]]; then
# Respect pre-set IMAGE_TAG (from environment or caller)
return
fi
local git_branch
git_branch=$(git rev-parse --abbrev-ref HEAD 2>/dev/null | tr '/' '-') || git_branch=""
if [[ -n "$git_branch" ]]; then
IMAGE_TAG="$git_branch"
else
IMAGE_TAG="local"
fi
}
# Function to build container images locally
function build_images() {
```
```
echo "================================="
echo "Building container images..."
# Ensure IMAGE_TAG is resolved once and reused
resolve_image_tag
echo "Using image tag: ${IMAGE_TAG}"
```
To fully remove duplication, the other place in this script where `IMAGE_TAG` is computed (near the `if [[ $USE_IMAGES = true ]]` block) should be updated to **call `resolve_image_tag` instead of recomputing it inline**. For example, replace the inline `git rev-parse` / defaulting logic there with:
```bash
resolve_image_tag
# then use $IMAGE_TAG
```
This will ensure a single, consistent implementation of image tag resolution shared by both the build and run paths.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
ed30bd4 to
ae3351f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds a --images (or -i) flag to the ibutsu-pod.sh script that enables building and using locally-built container images instead of mounting source directories at runtime. This allows testing containerized builds in the local development environment.
Key changes:
- Added
build_images()function that builds all five container images (backend, worker, scheduler, flower, frontend) from local Dockerfiles - Modified all container run commands to conditionally use pre-built images or runtime dev mounts based on the new
USE_IMAGESflag - Added celery scheduler container to the pod setup (previously missing from the script)
Comments suppressed due to low confidence (1)
scripts/ibutsu-pod.sh:180
- The 'git' command is used to generate IMAGE_TAG but is not included in the list of required tools checked at line 163. While the command has error handling (2>/dev/null and fallback to "local"), users without git installed would silently fallback to "local" tag without knowing git is missing. Consider either adding 'git' to the required tools list, or adding a warning message when git is not available.
else
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project status has failed because the head coverage (64.81%) is below the target coverage (85.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #796 +/- ##
=======================================
Coverage 64.81% 64.81%
=======================================
Files 157 157
Lines 7633 7633
Branches 659 659
=======================================
Hits 4947 4947
Misses 2467 2467
Partials 219 219 Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
run all the containers with locally built images to be able to test the builds and built containers within the local test environment.
ae3351f to
e9b5061
Compare
run all the containers with locally built images to be able to test the builds and built containers within the local test environment.
Summary by Sourcery
Add a new option to ibutsu-pod.sh to run the local stack using pre-built container images instead of development mounts, including support for building and tagging those images from the current Git branch.
New Features:
Enhancements: