Fix fluentd lifecycle hook BackoffLimitExceeded on deploy_to_doks_release - #839
Conversation
…ness wait The post-upgrade hook job was spawning 100 pods (backoffLimit: 100, restartPolicy: Never) each failing in ~9s because MinIO isn't ready immediately after a Helm upgrade, totalling ~15 minutes before BackoffLimitExceeded and causing helm --atomic to roll back. Changes: - Add a wait loop in the hook container that polls MinIO (aws s3 ls) every 10s for up to 10 minutes before attempting put-bucket-lifecycle-configuration - Reduce backoffLimit from 100 to 3 since retries are now handled within the container - Add before-hook-creation to hook-delete-policy so any stale failed job is always cleaned up before the next upgrade attempt creates a new one AI-Generated: true
Up to standards ✅🟢 Issues
|
There was a problem hiding this comment.
Pull Request Overview
This PR effectively addresses the BackoffLimitExceeded issue by implementing a readiness wait loop and optimizing the Job configuration. While the implementation aligns with the core requirements (polling interval, backoff reduction, and cleanup policy), the readiness check itself introduces a potential race condition.
The current loop only checks for the availability of the S3 endpoint. Since this job configures a specific bucket created by a preceding hook, the script may proceed before the bucket is actually available, leading to failures despite the wait loop. Additionally, the embedded shell logic lacks automated verification, increasing the risk of regressions in the deployment environment.
About this PR
- The shell script logic added to the Helm template lacks automated tests. This makes the hook susceptible to regressions if CLI versions or shell environments change in the deployment pipeline.
Test suggestions
- Verify the shell script waits for the full 600 seconds and then exits with code 1 if MinIO remains unreachable.
- Verify the shell script proceeds to apply the lifecycle configuration immediately after 'aws s3 ls' succeeds.
- Verify the 'backoffLimit' and 'hook-delete-policy' are correctly rendered in the Kubernetes Job manifest.
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. Verify the shell script waits for the full 600 seconds and then exits with code 1 if MinIO remains unreachable.
2. Verify the shell script proceeds to apply the lifecycle configuration immediately after 'aws s3 ls' succeeds.
3. Verify the 'backoffLimit' and 'hook-delete-policy' are correctly rendered in the Kubernetes Job manifest.
Low confidence findings
- The wait loop relies on 'aws s3 ls' which might fail for reasons other than connectivity (e.g., IAM permission errors). This could cause the job to wait for the full 10-minute timeout before reporting a failure that was identifiable immediately.
TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback
| MAX_WAIT=600 | ||
| ELAPSED=0 | ||
| echo "Waiting for MinIO at ${ENDPOINT}..." | ||
| until aws s3 ls --endpoint-url "${ENDPOINT}" > /dev/null 2>&1; do |
There was a problem hiding this comment.
🟡 MEDIUM RISK
Suggestion: The current check only verifies that the MinIO endpoint is reachable. Since the script's primary action is to configure a specific bucket, the wait loop should verify the bucket's existence to avoid a race condition where the service is up but the target bucket has not yet been initialized.
| until aws s3 ls --endpoint-url "${ENDPOINT}" > /dev/null 2>&1; do | |
| until aws s3 ls "s3://${BUCKET}" --endpoint-url "${ENDPOINT}" > /dev/null 2>&1; do |
| # any positive value will ensure that the bucket is created before this | ||
| "helm.sh/hook-weight": "10" | ||
| "helm.sh/hook-delete-policy": hook-succeeded | ||
| "helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded |
There was a problem hiding this comment.
⚪ LOW RISK
Nitpick: It is safer to quote the annotation value to ensure consistent parsing, especially when it contains special characters like commas.
| "helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded | |
| "helm.sh/hook-delete-policy": "before-hook-creation,hook-succeeded" |
…IO (#841) The previous fix (PR #839) waited for MinIO to respond to `aws s3 ls` before calling put-bucket-lifecycle-configuration. However, MinIO can be accessible while the specific bucket doesn't exist yet (bucket creation is a separate hook). This caused rapid failures of the lifecycle config command, hitting BackoffLimitExceeded (with backoffLimit:3) long before Helm's 1500s timeout. Changes: - Wait for the specific bucket (`aws s3api head-bucket`) instead of just MinIO (`aws s3 ls`), so the lifecycle config only runs once the bucket actually exists - Extend MAX_WAIT to 1200s (fits within Helm's 1500s timeout as a single container, no Kubernetes backoff overhead) - Set backoffLimit:0 so Kubernetes doesn't add exponential backoff delays between retries — all retry logic is in-shell - Add an in-shell retry loop (up to 5 attempts) for put-bucket-lifecycle-configuration to handle transient API errors after the bucket is confirmed ready AI-Generated: true Co-authored-by: circleci-app[bot] <127350680+circleci-app[bot]@users.noreply.github.com> Co-authored-by: Ricardo Bernardino <ricardo.bernardino@codacy.com>
Problem
Job 30973 (
deploy_to_doks_release) failed in the Install Codacy step with:This happened on both the initial attempt and the automatic retry added in #838.
Root cause
codacy/templates/fluentd/lifecycle-police-job.yamlis apost-upgradeHelm hook that sets a MinIO bucket lifecycle policy. It was configured with:restartPolicy: Never+backoffLimit: 100→ Kubernetes creates a new pod for every failure (no reuse)aws s3api put-bucket-lifecycle-configurationimmediately after the upgrade, while MinIO is still restartingBackoffLimitExceededhook-delete-policy: hook-succeededonly → failed job remains in the cluster unless Helm's atomic rollback cleans it upBecause both the first attempt and the retry exhaust all 100 pods against an unavailable MinIO, neither succeeds.
Fix
codacy/templates/fluentd/lifecycle-police-job.yamlaws s3 lsevery 10 s for up to 10 minutes before attempting the lifecycle operation. This tolerates MinIO restarting during the Helm upgrade.backoffLimitfrom 100 → 3: retry logic is now inside the container, so only 3 pod-level retries are needed as a safety net.before-hook-creationtohook-delete-policy: ensures any stale failed hook job from a previous run is deleted before the next upgrade creates a new one, preventing conflicts if Helm's atomic rollback misses cleanup.https://app.circleci.com/agents/gh/codacy/chat/82879e75-8ec3-430b-bf8d-0bab85c5a7a4