Fix fluentd lifecycle hook: wait for bucket existence, not just MinIO - #841
Conversation
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
Up to standards ✅🟢 Issues
|
![codacy-production[bot]](https://avatars.githubusercontent.com/in/56611?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the reliability of the fluentd lifecycle hook by ensuring it waits for the specific MinIO bucket to be created rather than merely checking if the service is ready. The shift to a 1200-second timeout and internal shell retries for S3 operations is consistent with the goal of handling asynchronous bucket provisioning.
While the implementation meets Codacy quality standards, the requirement to set backoffLimit: 0 creates a risk of fragile deployments. Disabling Kubernetes-level retries entirely makes the Job vulnerable to transient infrastructure events such as pod eviction or node preemption, which could lead to unnecessary deployment failures. Additionally, the suppression of stderr in the polling loop may obscure root causes like authentication failures during the wait period.
About this PR
- The decision to disable Kubernetes-level retries (backoffLimit: 0) creates a single point of failure at the infrastructure level. Even though the script handles application-level retries for S3, the Pod itself cannot recover from cluster-level interruptions. A backoffLimit of 1 is generally preferred for Helm hooks to ensure resilience against non-application failures.
Test suggestions
- Verify the hook polls and waits successfully if the bucket is created within the 1200s timeout.\n- [ ] Verify the hook exits with an error if the bucket readiness timeout (1200s) is exceeded.\n- [ ] Verify the lifecycle configuration command retries up to 5 times before failing on transient API errors.\n- [ ] Verify the Job specification ensures no Kubernetes-level pod restarts occur on failure.
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. Verify the hook polls and waits successfully if the bucket is created within the 1200s timeout.\n- [ ] Verify the hook exits with an error if the bucket readiness timeout (1200s) is exceeded.\n- [ ] Verify the lifecycle configuration command retries up to 5 times before failing on transient API errors.\n- [ ] Verify the Job specification ensures no Kubernetes-level pod restarts occur on failure.
TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback
| "helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded | ||
| spec: | ||
| backoffLimit: 3 | ||
| backoffLimit: 0 |
There was a problem hiding this comment.
🟡 MEDIUM RISK
Suggestion: Setting backoffLimit to 0 prevents the Job from recovering from transient Pod-level failures (such as node pressure or eviction). While the script contains internal retries for the S3 operations, a value of 0 means the entire Helm hook fails immediately if the container is interrupted for environment-level reasons. Consider allowing at least one retry (backoffLimit: 1) to ensure the hook is resilient to infrastructure glitches.
| echo "Waiting for MinIO at ${ENDPOINT}..." | ||
| until aws s3 ls --endpoint-url "${ENDPOINT}" > /dev/null 2>&1; do | ||
| echo "Waiting for MinIO bucket ${BUCKET} at ${ENDPOINT}..." | ||
| until aws s3api head-bucket --bucket "${BUCKET}" --endpoint-url "${ENDPOINT}" > /dev/null 2>&1; do |
There was a problem hiding this comment.
⚪ LOW RISK
Suggestion: Redirecting stderr to /dev/null hides potential configuration, network, or authentication errors, making it difficult to debug why the bucket check is failing during the 20-minute wait window. Removing '2>&1' would allow error messages to appear in the pod logs for easier troubleshooting.
PR #841 accidentally removed the `"Filter":{}` field that PR #840 had added to the S3 lifecycle rule JSON. Newer MinIO versions strictly follow the AWS S3 spec and require a Filter element in each lifecycle rule (even when empty). Without it, put-bucket-lifecycle-configuration returns MalformedXML immediately, causing the in-shell retry loop to exhaust its 5 attempts and exit with failure, which Kubernetes surfaces as BackoffLimitExceeded on the post-upgrade hook. AI-Generated: true Co-authored-by: circleci-app[bot] <127350680+circleci-app[bot]@users.noreply.github.com>
1 participant
Problem
Job
deploy_to_doks_release(31035) fails at "Install Codacy" withBackoffLimitExceededon thefluentd-lifecycle-policy-creatorpost-upgrade hook — both the initial attempt and the retry introduced in PR #838.Root cause: PR #839 added a MinIO readiness wait using
aws s3 ls, which checks if the MinIO service responds. However, MinIO can be up and responding while the specific bucket (fluentdoperator.bucketName) hasn't been created yet — bucket creation is handled by a separate hook with a lower weight. When MinIO is accessible but the bucket doesn't exist,put-bucket-lifecycle-configurationfails immediately. WithbackoffLimit: 3, Kubernetes retries it 4 times in rapid succession, hittingBackoffLimitExceededwithin a few minutes — well before Helm's 1500s timeout.Fix
aws s3api head-bucket --bucket "${BUCKET}") instead of just MinIO (aws s3 ls). The lifecycle config only runs once the bucket is confirmed to exist.MAX_WAITto 1200s — stays within Helm's 1500s timeout as a single container with no Kubernetes backoff overhead.backoffLimit: 0— all retry logic lives in the shell script, so no Kubernetes exponential backoff eats into the available 1500s window.put-bucket-lifecycle-configurationto handle transient API errors once the bucket is ready.https://app.circleci.com/agents/gh/codacy/chat/3f864ea6-b904-4dd6-a4fd-88d380b792cb