Skip to content

[chore] Optimize k8s steps in GHA #123

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

Merged
marcorosa merged 3 commits into from
Oct 2, 2025
+61 −15
Merged

[chore] Optimize k8s steps in GHA #123

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6e54c17
Add missing kubelogin step
marcorosa Sep 30, 2025
7a31668
Do not force tracing
marcorosa Oct 2, 2025
7f91bc7
Optimize Dockerfile
marcorosa Oct 2, 2025
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
14 changes: 14 additions & 0 deletions .github/workflows/docker.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,13 @@ jobs:
with:
version: 'latest'

- name: Set up kubelogin
uses: azure/use-kubelogin@v1
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
with:
kubelogin-version: 'latest'
Comment on lines +220 to +225
Copy link
Copy Markdown
Contributor

@github-actions github-actions bot Oct 2, 2025

Choose a reason for hiding this comment

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

Consider pinning to a specific version instead of 'latest' for better reproducibility and security. Using 'latest' can lead to unexpected behavior when the action is updated.

- name: Set up kubelogin
  uses: azure/use-kubelogin@v1.0.0  # Use specific version
  with:
    kubelogin-version: 'v0.0.32'     # Pin kubelogin version too


- name: Configure kubectl for SAP BTP Kyma
run: |
mkdir -p ~/.kube
Comment on lines +220 to 229
Copy link
Copy Markdown
Contributor

@github-actions github-actions bot Oct 2, 2025

Choose a reason for hiding this comment

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

Code duplication detected: The kubelogin setup step is duplicated in the workflow. Consider extracting this into a reusable action or using workflow templates to avoid redundancy.

# Create a separate action or use anchors/templates
- name: Setup Kubelogin
  uses: ./.github/actions/setup-kubelogin
  with:
    version: 'latest'

Expand All @@ -241,6 +248,13 @@ jobs:
with:
version: 'latest'

- name: Set up kubelogin
uses: azure/use-kubelogin@v1
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
with:
kubelogin-version: 'latest'

- name: Configure kubectl for SAP BTP Kyma
run: |
mkdir -p ~/.kube
Expand Down
58 changes: 44 additions & 14 deletions backend-agent/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,19 @@
FROM astral/uv:python3.11-trixie-slim AS builder

# Install build dependencies including Rust for packages that need it
RUN apt-get update && apt-get install -y \
# Install build dependencies with minimal footprint
RUN apt-get update && apt-get install -y --no-install-recommends \
build-essential \
git \
curl \
pkg-config \
libssl-dev \
&& curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y \
&& rm -rf /var/lib/apt/lists/* \
&& apt-get autoremove -y

# Install Rust with minimal profile and immediate cleanup
RUN curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y --profile minimal \
&& . ~/.cargo/env \
&& rm -rf /var/lib/apt/lists/*
&& rustup component add rustfmt

# Add Rust to PATH
ENV PATH="/root/.cargo/bin:${PATH}"
Expand All @@ -23,24 +27,50 @@ COPY pyproject.toml uv.lock ./
ENV UV_EXTRA_INDEX_URL="https://download.pytorch.org/whl/cpu"
ENV TORCH_INDEX_URL="https://download.pytorch.org/whl/cpu"

# Install dependencies using uv with proper build environment
# Install dependencies with aggressive progressive cleanup
RUN . ~/.cargo/env && \
uv sync --frozen --no-dev --no-cache && \
# Clean up any temporary files to reduce layer size
rm -rf /root/.cache/uv /tmp/* /var/tmp/* && \
# Remove Rust toolchain after build to reduce image size
rustup self uninstall -y
# Install dependencies with bytecode compilation for better performance
uv sync --frozen --no-dev --no-cache --compile-bytecode && \
# Immediate cleanup of build artifacts during installation
find /app/.venv -type d -name "__pycache__" -exec rm -rf {} + 2>/dev/null || true && \
find /app/.venv -name "*.pyc" -delete 2>/dev/null || true && \
find /app/.venv -name "*.pyo" -delete 2>/dev/null || true && \
# Remove test files and documentation from packages (keeping runtime libs)
find /app/.venv -type d -name "tests" -exec rm -rf {} + 2>/dev/null || true && \
find /app/.venv -type d -name "test" -exec rm -rf {} + 2>/dev/null || true && \
find /app/.venv -type d -name "docs" -exec rm -rf {} + 2>/dev/null || true && \
# Strip debug symbols from shared libraries to reduce size
find /app/.venv -name "*.so" -exec strip {} + 2>/dev/null || true && \
# Aggressive cache and temporary file cleanup
rm -rf /root/.cache/uv \
/root/.cache/pip \
/root/.cache/* \
/tmp/* \
/var/tmp/* \
/root/.cargo/registry \
/root/.cargo/git \
/app/.venv/share \
&& \
# Remove Rust toolchain completely after build
rustup self uninstall -y && \
# Final build tools cleanup to free space
apt-get autoremove -y build-essential git curl pkg-config && \
apt-get autoclean
Comment on lines 31 to +58
Copy link
Copy Markdown
Contributor

@github-actions github-actions bot Oct 2, 2025

Choose a reason for hiding this comment

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

The aggressive cleanup strategy is good for reducing image size, but some operations might be redundant or risky. The find commands with 2>/dev/null || true could mask legitimate errors. Consider using more targeted cleanup and verify that stripping shared libraries doesn't break functionality.

# More targeted and safer cleanup
RUN . ~/.cargo/env && \
    uv sync --frozen --no-dev --no-cache --compile-bytecode && \
    # Remove only Python cache files
    find /app/.venv -name "*.pyc" -delete && \
    find /app/.venv -name "__pycache__" -type d -exec rm -rf {} + && \
    # Test that libraries still work before stripping
    python -c "import torch; print('Libraries OK')" && \
    find /app/.venv -name "*.so" -exec strip --strip-unneeded {} \; && \
    # Clean up build artifacts
    rm -rf /root/.cache /tmp/* /var/tmp/* && \
    rustup self uninstall -y


# ----------------------------------------

FROM python:3.11-slim-trixie AS runtime

# Install only runtime dependencies
RUN apt-get update && apt-get install -y \
# Install minimal runtime dependencies
RUN apt-get update && apt-get install -y --no-install-recommends \
libssl3 \
libffi8 \
# Add required libraries for ML packages
libgomp1 \
libglib2.0-0 \
&& rm -rf /var/lib/apt/lists/* \
&& apt-get clean
&& apt-get autoremove -y \
&& apt-get autoclean
Comment on lines +64 to +73
Copy link
Copy Markdown
Contributor

@github-actions github-actions bot Oct 2, 2025

Choose a reason for hiding this comment

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

Good optimization using --no-install-recommends and proper cleanup. Consider adding a verification step to ensure the image still functions correctly after aggressive cleanup.

# Add runtime verification
RUN apt-get update && apt-get install -y --no-install-recommends \
    libssl3 \
    libffi8 \
    libgomp1 \
    libglib2.0-0 \
    && rm -rf /var/lib/apt/lists/* \
    && apt-get autoremove -y \
    && apt-get autoclean \
    # Verify critical libraries are available
    && python3 -c "import ssl, ctypes; print('Runtime deps OK')"


WORKDIR /app

Expand All @@ -50,7 +80,7 @@ COPY --from=builder /app/.venv /app/.venv
# Copy dependency files
COPY pyproject.toml uv.lock ./

# Copy the rest of the application
# Copy the application
COPY . .

# Make sure we use the virtual environment
Expand Down
4 changes: 3 additions & 1 deletion backend-agent/status.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,9 @@ def trace_llm(self,
'prompt': prompt,
'response': response.to_dict()
}
self.trace['llm_messages'].append(message)
# Only trace if there's an active trace context
if hasattr(self, 'trace') and self.trace:
self.trace['llm_messages'].append(message)
Comment on lines +181 to +183
Copy link
Copy Markdown
Contributor

@github-actions github-actions bot Oct 2, 2025

Choose a reason for hiding this comment

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

Good defensive programming to check for trace context. However, the condition could be more explicit about what constitutes a valid trace. Consider using a more specific check.

# More explicit trace validation
if hasattr(self, 'trace') and self.trace is not None and isinstance(self.trace, dict) and 'llm_messages' in self.trace:
    self.trace['llm_messages'].append(message)


def finish_trace(self, completed: bool, output: str):
"""
Expand Down