Re: [PR] feat: Kubernetes executionconfig API should return effective config (druid)

["FrankChen021 (via GitHub)" <gi...@apache.org>]

FrankChen021 commented on code in PR #19014:
URL: https://github.com/apache/druid/pull/19014#discussion_r3934180542


##########
extensions-core/kubernetes-overlord-extensions/src/main/java/org/apache/druid/k8s/overlord/execution/KubernetesTaskExecutionConfigResource.java:
##########
@@ -57,16 +57,18 @@ public class KubernetesTaskExecutionConfigResource
   private static final Logger log = new Logger(KubernetesTaskExecutionConfigResource.class);
   private final JacksonConfigManager configManager;
   private final AuditManager auditManager;
-  private AtomicReference<KubernetesTaskRunnerDynamicConfig> dynamicConfigRef = null;
+  private final KubernetesTaskRunnerEffectiveConfig effectiveConfig;
 
   @Inject
   public KubernetesTaskExecutionConfigResource(
       final JacksonConfigManager configManager,
-      final AuditManager auditManager
+      final AuditManager auditManager,
+      final KubernetesTaskRunnerEffectiveConfig effectiveConfig

Review Comment:
   [P2] Avoid requiring single-cluster config for multik8s
   
   Injecting `KubernetesTaskRunnerEffectiveConfig` makes this resource resolve the module's `KubernetesTaskRunnerStaticConfig`, whose `namespace` field is `@NotNull`. The `multik8s` path instead reads `MultipleKubernetesTaskRunnerConfig` and explicitly allows top-level `druid.indexer.runner.namespace` to be absent when cluster entries provide `taskNamespace` (for example with the custom-template adapter). On that supported configuration, provisioning this resource fails validation before GET/POST can run. Keep the resource dependency runner-agnostic or provide the active multik8s configuration.



##########
extensions-core/kubernetes-overlord-extensions/src/main/java/org/apache/druid/k8s/overlord/execution/DefaultKubernetesTaskRunnerDynamicConfig.java:
##########
@@ -35,14 +35,17 @@ public class DefaultKubernetesTaskRunnerDynamicConfig implements KubernetesTaskR
 
   @JsonCreator
   public DefaultKubernetesTaskRunnerDynamicConfig(
+      @Nullable
       @JsonProperty("podTemplateSelectStrategy") PodTemplateSelectStrategy podTemplateSelectStrategy,
+      @Nullable
       @JsonProperty("capacity") Integer capacity
   )
   {
     this.podTemplateSelectStrategy = podTemplateSelectStrategy;
     this.capacity = capacity;
   }
 
+  @Nullable

Review Comment:
   [P1] Duplicate @Nullable annotations prevent compilation
   
   The merge result places one `@Nullable` before `@Override` and another after `@JsonProperty` on both getters. `javax.annotation.Nullable` is not a repeatable annotation, so javac rejects this class with `Nullable is not a repeatable annotation type` before the module can build or test. Keep only one annotation on each getter (the second duplicate is at line 57).



-- 
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: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org