Optimize Dockerfile: slim base image, pinned binaries, reduced layers#1153
Optimize Dockerfile: slim base image, pinned binaries, reduced layers#1153Nesar976 wants to merge 10 commits intokrkn-chaos:mainfrom
Conversation
Signed-off-by: Nesar976 <kavrinesar@gmail.com>
ⓘ You are approaching your monthly quota for Qodo. Upgrade your plan Review Summary by QodoOptimize Dockerfile with slim base and pinned binaries
WalkthroughsDescription• Replace multi-stage build with slim Python base image • Replace source builds with pinned release binaries for oc, virtctl, yq • Consolidate RUN commands to reduce Docker layers • Update entrypoint to use Python 3 instead of Python 3.9 Diagramflowchart LR
A["Multi-stage Dockerfile<br/>golang + fedora"] -->|"Switch base image"| B["python:3.9-slim"]
C["Source builds<br/>oc, virtctl"] -->|"Replace with binaries"| D["Pinned releases<br/>v4.18.0, v1.7.0, v4.40.5"]
E["Multiple RUN layers<br/>dnf, pip, setup"] -->|"Consolidate"| F["Optimized layers<br/>apt, pip, chmod"]
G["Python 3.9 entrypoint"] -->|"Update"| H["Python 3 entrypoint"]
File Changes1. containers/Dockerfile.template
|
Code Review by Qodo
1. amd64-only binaries
|
| @@ -1,89 +1,78 @@ | |||
| # oc build | |||
| FROM golang:1.24.9 AS oc-build | |||
There was a problem hiding this comment.
thank you very much for the contribution, Unfortunately we're forced to keep the build of the binary tools in the Dockerfile in order to be able to update the tools dependencies manually whenever a CVE is discovered without the need to wait the release cycles of the teams behind them. So we need to leave this and building of virtctl
There was a problem hiding this comment.
Thanks for the clarification. I’ve restored the Dockerfile to build the tools from source, matching the current main implementation.
containers/Dockerfile.template
Outdated
|
|
||
| FROM fedora:40 | ||
| # krkn-chaos/krkn Dockerfile | ||
| FROM python:3.9-slim |
There was a problem hiding this comment.
we have moved to pyhon3.11, please update this version
There was a problem hiding this comment.
Updated the runtime to Python 3.11 to align with the current project version.
There was a problem hiding this comment.
did you push your changes? not seeing the updates here
There was a problem hiding this comment.
Thanks for pointing that out. The changes were already pushed earlier, but after restoring the Dockerfile to align with main (building tools from source and updating the runtime to Python 3.11), there’s no additional diff to display, which is why it may look unchanged now.
Please let me know if you’d like me to apply any further adjustments on top of this.
Signed-off-by: Nesar976 <kavrinesar@gmail.com>
63d79a8 to
140c94a
Compare
|
@paigerube14 , Thanks for the review. I’ve restored the Dockerfile to build all required tools from source as in main, and updated the runtime to Python 3.11. Happy to make any further adjustments if needed. |
|
cc: @ddjain |
Signed-off-by: Nesar976 <kavrinesar@gmail.com>
|
@paigerube14 Thanks for pointing that out. The changes were already pushed earlier, but after restoring the Dockerfile to align with main (building tools from source and updating the runtime to Python 3.11), there’s no additional diff to display, which is why it may look unchanged now. Please let me know if you’d like me to apply any further adjustments on top of this. |
|
Hi Nesar976
|
|
Hi @ddjain thanks for the suggestion. |
Signed-off-by: Nesar976 <kavrinesar@gmail.com>
|
Hi @ddjain , Addressed the feedback: moved to python:3.11-alpine, removed apt-get, and aligned everything with Alpine (apk) . Changes are limited to what was requested. Please let me know if anything else is needed. |
|
Hi @Nesar976, Thanks for addressing the feedback and switching to I noticed the attached screenshot shows the Dockerfile changes. What I was actually looking for was the console output of the docker build and docker run commands (for a simple scenario), just to confirm the container builds successfully and runs as expected after the update. Could you please share the terminal output for those commands in the PR? That would help us verify everything is working correctly. Thanks again for the update! |
Signed-off-by: Nesar976 <kavrinesar@gmail.com>
63facad to
c992872
Compare
Hi @ddjain,
I’ve made the requested changes and attached the terminal output for docker build and docker run below for verification.
Please let me know if any further changes are required.
Thanks!
|
|
Hi @Nesar976 , Thanks for sharing the build and run logs 👍 The image builds successfully, which is great. However, I noticed that the failure during docker run is due to running an invalid scenario. To properly validate the updated Dockerfile, could you please run a valid scenario using the documented krkn-hub Docker command? You can refer to the official example here: This will help us confirm that the container not only builds but also executes a supported scenario successfully with the new python:3.11-alpine base Thanks again for the updates! |
Signed-off-by: Nesar976 <kavrinesar@gmail.com>
Hi @ddjain Executed a valid Pod Disruption scenario using the documented krkn Docker command on Docker Desktop Kubernetes with the updated python:3.11-alpine base image . The nginx pod was successfully deleted and recovered.
The run completed with exit_status=0 and job_status=true.
Attaching execution logs/screenshots for verification.
|
|
Hi @ddjain Also, I’m currently contributing to the project through the LFX program this term, so I’m actively working on understanding the codebase and improving wherever possible. |
|
/LGTM |
|
Hi @ddjain, thanks for the approval 🙂 |
|
@Nesar976 yes, please update/rebase the branch. |
|
Hi @ddjain I’ve updated and rebased the branch on the latest main. |
config.yaml
Outdated
| telemetry: | ||
| enabled: false | ||
| archive_path: "/tmp" | ||
| events_backup: false |
There was a problem hiding this comment.
Let's remove this file, all configs live under the "config" folder
test-config.yaml
Outdated
| - context: | ||
| cluster: docker-desktop | ||
| user: docker-desktop | ||
| name: docker-desktop |
There was a problem hiding this comment.
Let's also remove this file
pod_scenario.yaml
Outdated
| - id: kill-pods | ||
| config: | ||
| namespace_pattern: "default" | ||
| label_selector: "app=nginx" |
There was a problem hiding this comment.
And this one too please, TIA
|
once extra files are removed, this PR looks good to me |
5eb9ad5 to
269668a
Compare
|
Hi @paigerube14 Removed the unintended config files from the PR as suggested. Please let me know if anything else needs adjustment. |
containers/Dockerfile.template
Outdated
| # or just clone directly. The original logic cloned into /home/krkn/kraken. | ||
| # We will preserve the logic of cloning the repo. | ||
| RUN git clone https://github.com/krkn-chaos/krkn.git /home/krkn/kraken | ||
| RUN sed -i 's/exec python3\.9/exec python3/' /home/krkn/kraken/containers/entrypoint.sh |
There was a problem hiding this comment.
do we need this line since you updated the entrypoint to be python3?
There was a problem hiding this comment.
Good catch — you’re right, that line is no longer needed since the entrypoint already uses python3. I’ll remove it.
containers/Dockerfile.template
Outdated
| dnf clean all | ||
|
|
||
| # copy oc client binary from oc-build image | ||
| ARG TARGETARCH |
There was a problem hiding this comment.
is this needed in this PR? I think we have separate PR's that work on adding targetarch
There was a problem hiding this comment.
Hi @paigerube14 Good catch — it’s not strictly required for this PR. I’ll remove ARG TARGETARCH to keep this change focused on the Dockerfile optimizations.
Signed-off-by: Nesar976 <kavrinesar@gmail.com>
269668a to
3872bdb
Compare
Signed-off-by: Nesar976 <kavrinesar@gmail.com>
|
Hi @paigerube14 Thanks for the feedback. I’ve removed ARG TARGETARCH to keep this PR strictly focused on the Dockerfile optimizations. |





Type of change
Description
This PR improves the Dockerfile by switching to a slimmer base image, replacing source builds with pinned release binaries, and consolidating layers to reduce image size and build time. The entrypoint was updated to match the new base image while preserving existing behavior.
Fix #1125