capistrant commented on code in PR #19419:
URL: https://github.com/apache/druid/pull/19419#discussion_r3201937404
##########
extensions-core/kubernetes-overlord-extensions/src/main/java/org/apache/druid/k8s/overlord/taskadapter/DynamicConfigPodTemplateSelector.java:
##########
@@ -119,16 +119,18 @@ private void
validateTemplateSupplier(Supplier<PodTemplate> templateSupplier) th
@Override
public Optional<PodTemplateWithName> getPodTemplateForTask(Task task)
{
- String requested =
task.getContextValue(DruidK8sConstants.TASK_CONTEXT_POD_TEMPLATE_SELECTION_KEY);
- if (requested != null) {
- Supplier<PodTemplate> supplier = podTemplates.get(requested);
- if (supplier == null) {
- throw new IAE(
- "Task [%s] requested pod template [%s] via context key, but no
such template is configured.",
- task.getId(), requested
- );
+ if (effectiveConfig.isAllowTaskPodTemplateSelection()) {
+ String requested =
task.getContextValue(DruidK8sConstants.TASK_CONTEXT_POD_TEMPLATE_SELECTION_KEY);
+ if (requested != null) {
+ Supplier<PodTemplate> supplier = podTemplates.get(requested);
+ if (supplier == null) {
+ throw new IAE(
+ "Task [%s] requested pod template [%s] via context key, but no
such template is configured.",
+ task.getId(), requested
+ );
+ }
+ return Optional.of(new PodTemplateWithName(requested, supplier.get()));
Review Comment:
debug level log stating the task is overriding the pod spec could be helpful
down the road too
##########
extensions-core/kubernetes-overlord-extensions/src/main/java/org/apache/druid/k8s/overlord/taskadapter/DynamicConfigPodTemplateSelector.java:
##########
@@ -119,16 +119,18 @@ private void
validateTemplateSupplier(Supplier<PodTemplate> templateSupplier) th
@Override
public Optional<PodTemplateWithName> getPodTemplateForTask(Task task)
{
- String requested =
task.getContextValue(DruidK8sConstants.TASK_CONTEXT_POD_TEMPLATE_SELECTION_KEY);
- if (requested != null) {
- Supplier<PodTemplate> supplier = podTemplates.get(requested);
- if (supplier == null) {
- throw new IAE(
- "Task [%s] requested pod template [%s] via context key, but no
such template is configured.",
- task.getId(), requested
- );
+ if (effectiveConfig.isAllowTaskPodTemplateSelection()) {
+ String requested =
task.getContextValue(DruidK8sConstants.TASK_CONTEXT_POD_TEMPLATE_SELECTION_KEY);
Review Comment:
nit: for observability it might be nice to log a warn if this is not null +
`effectiveConfig.isAllowTaskPodTemplateSelection()` is false. that way the
owner of the spec has at least something to find if they are confused.
something like -
`task[<insert_task_id>] set
<DruidK8sConstants.TASK_CONTEXT_POD_TEMPLATE_SELECTION_KEY> in task context.
Not applying because cluster config does not allow overriding pod spec at
runtime.`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
