-
Notifications
You must be signed in to change notification settings - Fork 75
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
Add support for customizing terminationGracePeriodSeconds of the VTTablet pods #500
Add support for customizing terminationGracePeriodSeconds of the VTTablet pods #500
Conversation
…blet pods Signed-off-by: Yohei Yoshimuta <[email protected]>
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.
Thanks, @yoheimuta ! I just had one nit about the existing var name. Let me know what you think.
pkg/operator/vttablet/pod.go
Outdated
if spec.Vttablet.TerminationGracePeriodSeconds != nil { | ||
obj.Spec.TerminationGracePeriodSeconds = spec.Vttablet.TerminationGracePeriodSeconds | ||
} else { | ||
obj.Spec.TerminationGracePeriodSeconds = pointer.Int64Ptr(terminationGracePeriodSeconds) |
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.
Nit, but I feel like we should rename this variable to defaultTerminationGracePeriodSeconds
now.
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.
Signed-off-by: Yohei Yoshimuta <[email protected]>
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.
Looks good to me, thank you @yoheimuta
Closing and re-opening to re-trigger the CI tests that are in |
This PR introduces support for customizing the terminationGracePeriodSeconds of the VTTablet pods. Currently, this value is set to 30 minutes.
Having a fairly high terminationGracePeriodSeconds can lead to issues when adding sidecars that don't handle SIGTERM properly, resulting in long delays during the termination process. At least, this default constant value is not suitable for the use case of unmanaged tablets.
Similar to #406.