-
Notifications
You must be signed in to change notification settings - Fork 193
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
feat(helm): add lifecycle hooks to alloy container #1774
Conversation
f695de7
to
4fbc87b
Compare
This seems reasonable but looping in @petewall for any thoughts. |
@@ -183,6 +191,9 @@ controller: | |||
# -- Configures the DNS policy for the pod. https://kubernetes.io/docs/concepts/services-networking/dns-pod-service/#pod-s-dns-policy | |||
dnsPolicy: ClusterFirst | |||
|
|||
# -- Termination grace period in seconds for the Grafana Alloy pods. | |||
terminationGracePeriodSeconds: null |
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.
should this have a default value? 0?
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.
According to Kubernetes documentation, the default is 30s. https://kubernetes.io/docs/concepts/workloads/pods/pod-lifecycle/
I will add a note to the comment.
Left a few comments. All in all, looks good to me. |
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.
LGTM
@clayton-cornell for review of docs. |
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.
LGTM
Seeing some helm linting errors. |
I think the linting issues should be fixed now @mattdurham Thank you. |
* feat: add lifecycle hooks to alloy container * feat: add terminationGracePeriodSeconds to helm chart
PR Description
Which issue(s) this PR fixes
Fixes #1140
Fixes #1673
Adds lifecycle hooks to the alloy container to allow graceful target de-registration when using with AWS LoadBalancer Controller ingress.
Notes to the Reviewer
PR Checklist