Optimize docker builds and auto-deploy#103
Conversation
Restart k8s deployments running new images
|
The changes primarily involve modifying Docker workflows and ignore files to improve build efficiency and deployment reliability. The Docker cache storage method has been altered to prevent exceeding GitHub storage limits, and new deployment processes have been set up to ensure seamless backend and frontend updates after pull request merges. Additionally, file exclusions in the Walkthrough
Model: gpt-4o | Prompt Tokens: 1248 | Completion Tokens: 178 |
There was a problem hiding this comment.
Here's a thoughtful code review enhanced by AI assistance. These insights offer suggestions and observations that may help guide your development process. Please consider them as recommendations rather than requirements—your expertise and judgment remain the most important factors in your coding decisions. Use whatever feels valuable and relevant to your specific project needs.
Always critique what AI says. Do not let AI replace YOUR I.
Model: anthropic--claude-4-sonnet | Prompt Tokens: 2471 | Completion Tokens: 1725
| cache-from: type=registry,ref=${{ secrets.DOCKER_REGISTRY_URL }}/stars-backend:cache | ||
| cache-to: type=registry,ref=${{ secrets.DOCKER_REGISTRY_URL }}/stars-backend:cache,mode=max | ||
| # Use docker registry cache not to exceed GitHub Actions storage limits | ||
| # Builds will be slower but won't fail due to storage limits |
There was a problem hiding this comment.
The cache configuration change from GitHub Actions cache to registry cache is good for avoiding storage limits. However, consider adding error handling and fallback mechanisms to ensure builds don't fail if the registry cache is unavailable.
cache-from: |
type=registry,ref=${{ secrets.DOCKER_REGISTRY_URL }}/stars-backend:cache
type=gha
cache-to: |
type=registry,ref=${{ secrets.DOCKER_REGISTRY_URL }}/stars-backend:cache,mode=max
type=gha,mode=max| deploy-backend: | ||
| name: Restart Backend Deployment | ||
| if: github.event.pull_request.merged && needs.build-backend.result == 'success' | ||
| needs: [check_version_update, build-backend] | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Set up kubectl | ||
| uses: azure/setup-kubectl@v3 | ||
| with: | ||
| version: 'latest' | ||
|
|
||
| - name: Configure kubectl for SAP BTP Kyma | ||
| run: | | ||
| mkdir -p ~/.kube | ||
| echo "${{ secrets.KUBECONFIG }}" | base64 -d > ~/.kube/config | ||
| chmod 600 ~/.kube/config | ||
|
|
||
| - name: Restart Backend Deployment | ||
| run: | | ||
| echo "🔄 Restarting backend deployment to pull latest image..." | ||
| kubectl rollout restart deployment/stars-backend -n stars | ||
| kubectl rollout status deployment/stars-backend -n stars --timeout=10m | ||
| echo "✅ Backend deployment restarted successfully" | ||
|
|
||
| deploy-frontend: | ||
| name: Restart Frontend Deployment | ||
| if: github.event.pull_request.merged && needs.build-frontend.result == 'success' | ||
| needs: [check_version_update, build-frontend] | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Set up kubectl | ||
| uses: azure/setup-kubectl@v3 | ||
| with: | ||
| version: 'latest' | ||
|
|
||
| - name: Configure kubectl for SAP BTP Kyma | ||
| run: | | ||
| mkdir -p ~/.kube | ||
| echo "${{ secrets.KUBECONFIG }}" | base64 -d > ~/.kube/config | ||
| chmod 600 ~/.kube/config | ||
|
|
||
| - name: Restart Frontend Deployment | ||
| run: | | ||
| echo "🔄 Restarting frontend deployment to pull latest image..." | ||
| kubectl rollout restart deployment/stars-frontend -n stars | ||
| kubectl rollout status deployment/stars-frontend -n stars --timeout=10m | ||
| echo "✅ Frontend deployment restarted successfully" |
There was a problem hiding this comment.
The deployment jobs have significant code duplication. Consider extracting the common kubectl configuration into a reusable composite action or using a matrix strategy to reduce maintenance overhead.
deploy:
name: Restart Deployments
if: github.event.pull_request.merged && (needs.build-backend.result == 'success' || needs.build-frontend.result == 'success')
needs: [check_version_update, build-backend, build-frontend]
runs-on: ubuntu-latest
strategy:
matrix:
service:
- { name: backend, condition: "needs.build-backend.result == 'success'" }
- { name: frontend, condition: "needs.build-frontend.result == 'success'" }
steps:
- name: Set up kubectl
uses: azure/setup-kubectl@v3
with:
version: 'latest'
- name: Configure kubectl for SAP BTP Kyma
run: |
mkdir -p ~/.kube
echo "${{ secrets.KUBECONFIG }}" | base64 -d > ~/.kube/config
chmod 600 ~/.kube/config
- name: Restart ${{ matrix.service.name }} Deployment
if: ${{ matrix.service.condition }}
run: |
echo "🔄 Restarting ${{ matrix.service.name }} deployment to pull latest image..."
kubectl rollout restart deployment/stars-${{ matrix.service.name }} -n stars
kubectl rollout status deployment/stars-${{ matrix.service.name }} -n stars --timeout=10m
echo "✅ ${{ matrix.service.name }} deployment restarted successfully"| - name: Configure kubectl for SAP BTP Kyma | ||
| run: | | ||
| mkdir -p ~/.kube | ||
| echo "${{ secrets.KUBECONFIG }}" | base64 -d > ~/.kube/config | ||
| chmod 600 ~/.kube/config |
There was a problem hiding this comment.
The kubectl configuration step has potential security improvements. Consider using a temporary file with proper cleanup and more restrictive permissions.
- name: Configure kubectl for SAP BTP Kyma
run: |
KUBECONFIG_FILE=$(mktemp)
echo "${{ secrets.KUBECONFIG }}" | base64 -d > "$KUBECONFIG_FILE"
chmod 400 "$KUBECONFIG_FILE"
export KUBECONFIG="$KUBECONFIG_FILE"
# Your kubectl commands here
rm -f "$KUBECONFIG_FILE"| **/*.pyo | ||
| **/*.pyd | ||
| **/.pytest_cache | ||
| **/test* | ||
| **/Test* | ||
| **/.coverage | ||
| **/htmlcov | ||
| **/.tox |
There was a problem hiding this comment.
Good improvements to the .dockerignore file! However, the test exclusion patterns might be too broad and could exclude legitimate files. Consider being more specific with test patterns.
# Test files (more specific patterns)
**/tests/
**/*_test.py
**/test_*.py
**/*Test*.py
**/.pytest_cache
**/.coverage
**/htmlcov
**/.tox
|
|
||
| # Sensitive data | ||
| .env | ||
| .env* |
There was a problem hiding this comment.
The .env* pattern is good for excluding environment files, but consider being more explicit about which environment files to exclude to avoid accidentally excluding legitimate files.
# Environment files
.env
.env.local
.env.development
.env.test
.env.production
.env.*.local
Optimize builds space in GitHub action (previously, we exceeded the space limit with the backend).
The workflow may be slightly slower now, as we use docker registry as a cache, but space limit errors should be resolved.
In addition, after new images have been pushed, k8s deployments are restarted (and the new image is pulled)