From 989d3e2a7a912b979131acfcafeddf07dd92278b Mon Sep 17 00:00:00 2001 From: Kiran Gadhave Date: Fri, 18 Oct 2024 15:17:58 -0600 Subject: [PATCH 1/3] handling empty sets for dataSourceCondition and taskTypeCondition --- .../k8s/overlord/execution/Selector.java | 9 ++-- .../k8s/overlord/execution/SelectorTest.java | 53 +++++++++++++++++++ 2 files changed, 59 insertions(+), 3 deletions(-) diff --git a/extensions-contrib/kubernetes-overlord-extensions/src/main/java/org/apache/druid/k8s/overlord/execution/Selector.java b/extensions-contrib/kubernetes-overlord-extensions/src/main/java/org/apache/druid/k8s/overlord/execution/Selector.java index a314a69b3811..3bc0b2d57fca 100644 --- a/extensions-contrib/kubernetes-overlord-extensions/src/main/java/org/apache/druid/k8s/overlord/execution/Selector.java +++ b/extensions-contrib/kubernetes-overlord-extensions/src/main/java/org/apache/druid/k8s/overlord/execution/Selector.java @@ -23,7 +23,9 @@ import com.fasterxml.jackson.annotation.JsonProperty; import org.apache.druid.indexing.common.task.Task; import org.apache.druid.query.DruidMetrics; +import org.apache.druid.utils.CollectionUtils; +import java.util.Collections; import java.util.Map; import java.util.Objects; import java.util.Set; @@ -70,6 +72,7 @@ public Selector( public boolean evaluate(Task task) { boolean isMatch = true; + if (cxtTagsConditions != null) { isMatch = cxtTagsConditions.entrySet().stream().allMatch(entry -> { String tagKey = entry.getKey(); @@ -80,15 +83,15 @@ public boolean evaluate(Task task) } Object tagValue = tags.get(tagKey); - return tagValue == null ? false : tagValues.contains((String) tagValue); + return tagValue != null && tagValues.contains((String) tagValue); }); } - if (isMatch && taskTypeCondition != null) { + if (isMatch && !CollectionUtils.isNullOrEmpty(taskTypeCondition)) { isMatch = taskTypeCondition.contains(task.getType()); } - if (isMatch && dataSourceCondition != null) { + if (isMatch && !CollectionUtils.isNullOrEmpty(dataSourceCondition)) { isMatch = dataSourceCondition.contains(task.getDataSource()); } diff --git a/extensions-contrib/kubernetes-overlord-extensions/src/test/java/org/apache/druid/k8s/overlord/execution/SelectorTest.java b/extensions-contrib/kubernetes-overlord-extensions/src/test/java/org/apache/druid/k8s/overlord/execution/SelectorTest.java index 0ecff67408e3..3a086d1e453d 100644 --- a/extensions-contrib/kubernetes-overlord-extensions/src/test/java/org/apache/druid/k8s/overlord/execution/SelectorTest.java +++ b/extensions-contrib/kubernetes-overlord-extensions/src/test/java/org/apache/druid/k8s/overlord/execution/SelectorTest.java @@ -35,6 +35,59 @@ public class SelectorTest { + @Test + public void shouldReturnTrueWhenMatchTasksTagsAndEmptyDataSource() { + Map> cxtTagsConditions = new HashMap<>(); + cxtTagsConditions.put("tag1", Sets.newHashSet("tag1Value")); + + Task task = NoopTask.create(); + task.addToContext(DruidMetrics.TAGS, ImmutableMap.of("tag1", "tag1Value")); + + Selector selector = new Selector( + "TestSelector", + cxtTagsConditions, + Sets.newHashSet(NoopTask.TYPE), + Sets.newHashSet() + ); + + Assert.assertTrue(selector.evaluate(task)); + } + + @Test + public void shouldReturnTrueWhenMatchDataSourceTagsAndEmptyTasks() { + String datasource = "table"; + Map> cxtTagsConditions = new HashMap<>(); + cxtTagsConditions.put("tag1", Sets.newHashSet("tag1Value")); + + Task task = NoopTask.forDatasource(datasource); + task.addToContext(DruidMetrics.TAGS, ImmutableMap.of("tag1", "tag1Value")); + + Selector selector = new Selector( + "TestSelector", + cxtTagsConditions, + Sets.newHashSet(), + Sets.newHashSet(datasource) + ); + + Assert.assertTrue(selector.evaluate(task)); + } + + @Test + public void shouldReturnTrueWhenMatchDataSourceTasksAndEmptyTags() { + String datasource = "table"; + Map> cxtTagsConditions = new HashMap<>(); + + Task task = NoopTask.forDatasource(datasource); + + Selector selector = new Selector( + "TestSelector", + cxtTagsConditions, + Sets.newHashSet(NoopTask.TYPE), + Sets.newHashSet(datasource) + ); + + Assert.assertTrue(selector.evaluate(task)); + } @Test public void shouldReturnTrueWhenAllTagsAndTasksMatch() From 3074609be528bd4fdec76e4c176f85c69da3cfab Mon Sep 17 00:00:00 2001 From: Kiran Gadhave Date: Tue, 22 Oct 2024 15:08:03 -0700 Subject: [PATCH 2/3] using new HashSet<>() to fix forbidden api error in testCheck --- .../apache/druid/k8s/overlord/execution/SelectorTest.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/extensions-contrib/kubernetes-overlord-extensions/src/test/java/org/apache/druid/k8s/overlord/execution/SelectorTest.java b/extensions-contrib/kubernetes-overlord-extensions/src/test/java/org/apache/druid/k8s/overlord/execution/SelectorTest.java index 3a086d1e453d..b9857c177fb1 100644 --- a/extensions-contrib/kubernetes-overlord-extensions/src/test/java/org/apache/druid/k8s/overlord/execution/SelectorTest.java +++ b/extensions-contrib/kubernetes-overlord-extensions/src/test/java/org/apache/druid/k8s/overlord/execution/SelectorTest.java @@ -30,6 +30,7 @@ import org.junit.Test; import java.util.HashMap; +import java.util.HashSet; import java.util.Map; import java.util.Set; @@ -47,7 +48,7 @@ public void shouldReturnTrueWhenMatchTasksTagsAndEmptyDataSource() { "TestSelector", cxtTagsConditions, Sets.newHashSet(NoopTask.TYPE), - Sets.newHashSet() + new HashSet<>() ); Assert.assertTrue(selector.evaluate(task)); @@ -65,7 +66,7 @@ public void shouldReturnTrueWhenMatchDataSourceTagsAndEmptyTasks() { Selector selector = new Selector( "TestSelector", cxtTagsConditions, - Sets.newHashSet(), + new HashSet<>(), Sets.newHashSet(datasource) ); From f1a6876aefdcebaebb2f07078e5898f7d573c6ab Mon Sep 17 00:00:00 2001 From: Kiran Gadhave Date: Mon, 28 Oct 2024 13:20:09 -0600 Subject: [PATCH 3/3] fixing style issues --- .../apache/druid/k8s/overlord/execution/Selector.java | 1 - .../druid/k8s/overlord/execution/SelectorTest.java | 9 ++++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/extensions-contrib/kubernetes-overlord-extensions/src/main/java/org/apache/druid/k8s/overlord/execution/Selector.java b/extensions-contrib/kubernetes-overlord-extensions/src/main/java/org/apache/druid/k8s/overlord/execution/Selector.java index 3bc0b2d57fca..29e31218ad01 100644 --- a/extensions-contrib/kubernetes-overlord-extensions/src/main/java/org/apache/druid/k8s/overlord/execution/Selector.java +++ b/extensions-contrib/kubernetes-overlord-extensions/src/main/java/org/apache/druid/k8s/overlord/execution/Selector.java @@ -25,7 +25,6 @@ import org.apache.druid.query.DruidMetrics; import org.apache.druid.utils.CollectionUtils; -import java.util.Collections; import java.util.Map; import java.util.Objects; import java.util.Set; diff --git a/extensions-contrib/kubernetes-overlord-extensions/src/test/java/org/apache/druid/k8s/overlord/execution/SelectorTest.java b/extensions-contrib/kubernetes-overlord-extensions/src/test/java/org/apache/druid/k8s/overlord/execution/SelectorTest.java index b9857c177fb1..14e2ffb21439 100644 --- a/extensions-contrib/kubernetes-overlord-extensions/src/test/java/org/apache/druid/k8s/overlord/execution/SelectorTest.java +++ b/extensions-contrib/kubernetes-overlord-extensions/src/test/java/org/apache/druid/k8s/overlord/execution/SelectorTest.java @@ -37,7 +37,8 @@ public class SelectorTest { @Test - public void shouldReturnTrueWhenMatchTasksTagsAndEmptyDataSource() { + public void shouldReturnTrueWhenMatchTasksTagsAndEmptyDataSource() + { Map> cxtTagsConditions = new HashMap<>(); cxtTagsConditions.put("tag1", Sets.newHashSet("tag1Value")); @@ -55,7 +56,8 @@ public void shouldReturnTrueWhenMatchTasksTagsAndEmptyDataSource() { } @Test - public void shouldReturnTrueWhenMatchDataSourceTagsAndEmptyTasks() { + public void shouldReturnTrueWhenMatchDataSourceTagsAndEmptyTasks() + { String datasource = "table"; Map> cxtTagsConditions = new HashMap<>(); cxtTagsConditions.put("tag1", Sets.newHashSet("tag1Value")); @@ -74,7 +76,8 @@ public void shouldReturnTrueWhenMatchDataSourceTagsAndEmptyTasks() { } @Test - public void shouldReturnTrueWhenMatchDataSourceTasksAndEmptyTags() { + public void shouldReturnTrueWhenMatchDataSourceTasksAndEmptyTags() + { String datasource = "table"; Map> cxtTagsConditions = new HashMap<>();