trinodb / charts

Apache License 2.0
151 stars 173 forks source link

Add option to enable worker graceful shutdown #226

Closed sdaberdaku closed 1 month ago

sdaberdaku commented 1 month ago

In this PR I have automated the process of enabling the Worker Graceful Shutdown feature.

Enabling the option will:

I also added a test to check if the graceful shutdown endpoint can be successfully called on the worker Pods.

Fixes #189

sdaberdaku commented 1 month ago

Hello @nineinchnick,

Thank you so much for finding the time to review this PR! I will try to implement all the required changes, hopefully during this week and I will let you know then they are ready.

nineinchnick commented 1 month ago

Thanks! No rush and we can always chat on the Trino slack if anything is not clear enough

sdaberdaku commented 1 month ago

Dear @nineinchnick,

I have addressed all your comments.

Regarding the test implementation, I considered using a long-running query; however, I was uncertain about its appropriateness as it seemed more like a Trino test than a Helm Chart test. Ultimately, my testing approach involves deleting a worker pod using kubectl and verifying that the logs of the affected pod contain the "Shutdown requested" string. This method effectively tests both the worker's authorization and the lifecycle configuration.

Looking forward to your feedback!

sdaberdaku commented 1 month ago

I rebased and addressed your last comment. Should be ready now!