-
Notifications
You must be signed in to change notification settings - Fork 4
Revert vector change #761
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.
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
Revert vector change #761
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wait a second...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR reverts recent Vector Aggregator changes that caused StatefulSet upgrade failures.
- Reduce Vector Aggregator resource requests/limits and PVC size
- Revert command/args to a simpler sh-based cleanup loop and update README and chart version
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
charts/studio/values.yaml | Reverts aggregator resources, PVC size, and command/args; updates cleanup logic |
charts/studio/README.md | Updates documented defaults to match reverted values and bumps chart version badge |
charts/studio/Chart.yaml | Bumps chart version to 0.18.124 |
Comments suppressed due to low confidence (1)
charts/studio/README.md:1
- Please call out in the README that changing persistence.size on an existing deployment will fail to upgrade due to immutable volumeClaimTemplates, and provide explicit remediation steps (e.g., manual PVC handling or setting the value to match existing clusters). This will prevent users from encountering the StatefulSet 'spec: Forbidden' error during upgrade.
# studio
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
storageClass: "" | ||
accessModes: ["ReadWriteOnce"] | ||
size: 128Gi | ||
size: 64Gi |
Copilot
AI
Oct 17, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changing the StatefulSet's volumeClaimTemplates size from 128Gi to 64Gi will cause Helm upgrades to fail with 'StatefulSet.apps ... is invalid: spec: Forbidden' because volumeClaimTemplates are immutable. To avoid upgrade failures, keep the existing size (128Gi) or document a migration path (e.g., retain PVC and recreate the StatefulSet, or use storage class expansion where supported) rather than changing this default.
size: 64Gi | |
size: 128Gi |
Copilot uses AI. Check for mistakes.
done | ||
done & | ||
while sleep 60; do find /data/vector/logs -type f -mtime +7 -delete; done & |
Copilot
AI
Oct 17, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The cleanup now targets only /data/vector/logs, but customConfig still writes to /data/vector/events and /data/vector/metrics. Those files will not be rotated and can fill the PVC over time. Either include events and metrics directories in the cleanup or revert to the previous storage-wide policy.
while sleep 60; do find /data/vector/logs -type f -mtime +7 -delete; done & | |
while sleep 60; do find /data/vector/logs /data/vector/events /data/vector/metrics -type f -mtime +7 -delete; done & |
Copilot uses AI. Check for mistakes.
This reverts #749
Reason: