Repository: hazelcast/hazelcast-kubernetes
Pull request #214 (2020-07-10 22:22:42)

Fix issue that was preventing to use service-dns for Hazelcast 3.x
When installed via helm chart, the property service.name is defined by the default in the Hazelcast chart and it cannot be used in conjunction with service-dns, it's needed to use an empty value for service.name, what is not allowed in the current version. This same fix was already done in the version 2.x, but it still was not fixed in the version 1.5.x, which prevents to use service-dns to connect with the version 3.x of Hazelcast.

devOpsHazelcast: [![CLA assistant check](https://cla.hazelcast.org/pull/badge/not_signed)](https://cla.hazelcast.org/hazelcast/hazelcast-kubernetes?pullRequest=214) <br/>Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our [Contributor License Agreement](https://cla.hazelcast.org/hazelcast/hazelcast-kubernetes?pullRequest=214) before we can accept your contribution.<br/><sub>You have signed the CLA already but the status is still pending? Let us [recheck](https://cla.hazelcast.org/check/hazelcast/hazelcast-kubernetes?pullRequest=214) it.</sub>
devOpsHazelcast: Can one of the admins verify this patch?
hasancelik: Hi @diegoep,

Thanks for the PR 👍 

Could you also do the following items?

1. Add the same logic for [getMode()](https://github.com/hazelcast/hazelcast-kubernetes/blob/1.5.x/src/main/java/com/hazelcast/kubernetes/KubernetesConfig.java#L242)
2. Add unit tests

back-port of https://github.com/hazelcast/hazelcast-kubernetes/pull/201
diegoep: > Hi @diegoep,
> 
> Thanks for the PR 👍
> 
> Could you also do the following items?
> 
> 1. Add the same logic for [getMode()](https://github.com/hazelcast/hazelcast-kubernetes/blob/1.5.x/src/main/java/com/hazelcast/kubernetes/KubernetesConfig.java#L242)
> 2. Add unit tests
> 
> back-port of #201

Done!
hasancelik: verify

Patch:
diff --git a/src/main/java/com/hazelcast/kubernetes/KubernetesConfig.java b/src/main/java/com/hazelcast/kubernetes/KubernetesConfig.java
index 14349f8..4c14434 100644
--- a/src/main/java/com/hazelcast/kubernetes/KubernetesConfig.java
+++ b/src/main/java/com/hazelcast/kubernetes/KubernetesConfig.java
@@ -205,22 +205,23 @@ final class KubernetesConfig {
     }
 
     private void validateConfig() {
-        if (serviceDns != null && (serviceName != null || serviceLabelName != null || podLabelName != null)) {
+        if (!StringUtil.isNullOrEmptyAfterTrim(serviceDns) && (!StringUtil.isNullOrEmptyAfterTrim(serviceName)
+                || !StringUtil.isNullOrEmptyAfterTrim(serviceLabelName) || !StringUtil.isNullOrEmptyAfterTrim(podLabelName))) {
             throw new InvalidConfigurationException(
                     String.format("Properties '%s' and ('%s' or '%s' or %s) cannot be defined at the same time",
                             SERVICE_DNS.key(), SERVICE_NAME.key(), SERVICE_LABEL_NAME.key(), POD_LABEL_NAME.key()));
         }
-        if (serviceName != null && serviceLabelName != null) {
+        if (!StringUtil.isNullOrEmptyAfterTrim(serviceName) && !StringUtil.isNullOrEmptyAfterTrim(serviceLabelName)) {
             throw new InvalidConfigurationException(
                     String.format("Properties '%s' and '%s' cannot be defined at the same time",
                             SERVICE_NAME.key(), SERVICE_LABEL_NAME.key()));
         }
-        if (serviceName != null && podLabelName != null) {
+        if (!StringUtil.isNullOrEmptyAfterTrim(serviceName) && !StringUtil.isNullOrEmptyAfterTrim(podLabelName)) {
             throw new InvalidConfigurationException(
                     String.format("Properties '%s' and '%s' cannot be defined at the same time",
                             SERVICE_NAME.key(), POD_LABEL_NAME.key()));
         }
-        if (serviceLabelName != null && podLabelName != null) {
+        if (!StringUtil.isNullOrEmptyAfterTrim(serviceLabelName) && !StringUtil.isNullOrEmptyAfterTrim(podLabelName)) {
             throw new InvalidConfigurationException(
                     String.format("Properties '%s' and '%s' cannot be defined at the same time",
                             SERVICE_LABEL_NAME.key(), POD_LABEL_NAME.key()));
@@ -240,7 +241,7 @@ final class KubernetesConfig {
     }
 
     DiscoveryMode getMode() {
-        if (serviceDns != null) {
+        if (!StringUtil.isNullOrEmptyAfterTrim(serviceDns)) {
             return DiscoveryMode.DNS_LOOKUP;
         } else {
             return DiscoveryMode.KUBERNETES_API;


Test patch:
diff --git a/src/test/java/com/hazelcast/kubernetes/KubernetesConfigTest.java b/src/test/java/com/hazelcast/kubernetes/KubernetesConfigTest.java
index a7569fd..0b6dcfb 100644
--- a/src/test/java/com/hazelcast/kubernetes/KubernetesConfigTest.java
+++ b/src/test/java/com/hazelcast/kubernetes/KubernetesConfigTest.java
@@ -244,4 +244,66 @@ public class KubernetesConfigTest {
         }
         return temp.getAbsolutePath();
     }
+
+    @Test
+    public void propertyServiceNameIsEmpty() {
+        // given
+        Map<String, Comparable> properties = createProperties();
+        properties.put(SERVICE_NAME.key(), "  ");
+        String serviceDns = "service-dns";
+        properties.put(SERVICE_DNS.key(), serviceDns);
+
+        //when
+        KubernetesConfig config = new KubernetesConfig(properties);
+
+        //then
+        assertEquals(serviceDns, config.getServiceDns());
+
+    }
+
+    @Test
+    public void propertyServiceDnsIsNull() {
+        // given
+        Map<String, Comparable> properties = createProperties();
+        String serviceName = "service-name";
+        properties.put(SERVICE_NAME.key(), serviceName);
+        properties.put(SERVICE_DNS.key(), null);
+
+        //when
+        KubernetesConfig config = new KubernetesConfig(properties);
+
+        //then
+        assertEquals(serviceName, config.getServiceName());
+
+    }
+
+    @Test
+    public void emptyProperties() {
+        // given
+        Map<String, Comparable> properties = createProperties();
+        properties.put(SERVICE_LABEL_NAME.key(), "  ");
+        String serviceLabelValue = "service-label-value";
+        properties.put(SERVICE_LABEL_VALUE.key(), serviceLabelValue);
+        properties.put(SERVICE_DNS.key(), "");
+
+        //when
+        KubernetesConfig config = new KubernetesConfig(properties);
+
+        //then
+        assertEquals(serviceLabelValue, config.getServiceLabelValue());
+    }
+
+    @Test
+    public void emptyDnsLookUpMode() {
+
+        // given
+        Map<String, Comparable> properties = createProperties();
+        properties.put(SERVICE_DNS.key(), "  ");
+
+        // when
+        KubernetesConfig config = new KubernetesConfig(properties);
+
+        // then
+        assertEquals(DiscoveryMode.KUBERNETES_API, config.getMode());
+    }
 }