trinodb / charts

Apache License 2.0
151 stars 173 forks source link

accessControl properties are only added to the coordinator #189

Closed mgorbatenko closed 1 month ago

mgorbatenko commented 4 months ago

accessControl properties need to be on worker nodes to properly implement graceful shutdowns. Without that, the preStop api request will result in a 403. A work around is to add the access control properties via additionalConfigFiles. I believe it would be helpful to have the properties added to the worker configmap as well, so that the properties are present on both the coordinator and worker nodes.

Some of my learnings outlined on the Trino slack here.

sdaberdaku commented 1 month ago

According to the official Trino documentation:

Access control must be configured on the coordinator. Authorization for operations on specific worker nodes, such a triggering Graceful shutdown, must also be configured on all workers.

which means that the accessControl configuration should only be added to the worker when the graceful shutdown feature is required.

It would be nice if the worker configuration had a simple option to enable graceful shutdown, which would automatically add the required preStop hook and configuration options.

I could provide a PR for the latter. @nineinchnick what do you think?

nineinchnick commented 1 month ago

There's #213 but looks like it stalled. You can open another PR. We definitely need a test case for this too, to catch any potential regressions in the future.