You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@knox.apache.org by sm...@apache.org on 2023/02/03 10:40:53 UTC

[knox] branch master updated: KNOX-2869 - Handling the case when previously persisted CM discovery config file is empty (#722)

This is an automated email from the ASF dual-hosted git repository.

smolnar pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/knox.git


The following commit(s) were added to refs/heads/master by this push:
     new e9b3d9e50 KNOX-2869 - Handling the case when previously persisted CM discovery config file is empty (#722)
e9b3d9e50 is described below

commit e9b3d9e5005d2bd40f5013b508a84a14c8ea31f8
Author: Sandor Molnar <sm...@apache.org>
AuthorDate: Fri Feb 3 11:40:47 2023 +0100

    KNOX-2869 - Handling the case when previously persisted CM discovery config file is empty (#722)
---
 .../ClouderaManagerServiceDiscoveryMessages.java   |  6 +++
 .../monitor/DiscoveryConfigurationFileStore.java   | 50 +++++++++++++---------
 .../cm/monitor/PollingConfigurationAnalyzer.java   |  6 ++-
 .../DiscoveryConfigurationFileStoreTest.java       | 35 ++++++++++++---
 4 files changed, 70 insertions(+), 27 deletions(-)

diff --git a/gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/ClouderaManagerServiceDiscoveryMessages.java b/gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/ClouderaManagerServiceDiscoveryMessages.java
index 9a1d75883..cd4a1f2f3 100644
--- a/gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/ClouderaManagerServiceDiscoveryMessages.java
+++ b/gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/ClouderaManagerServiceDiscoveryMessages.java
@@ -142,6 +142,12 @@ public interface ClouderaManagerServiceDiscoveryMessages {
            text = "Terminating monitoring of {1} @ {0} for configuration changes because there are no referencing descriptors.")
   void stoppingConfigMonitoring(String discoverySource, String clusterName);
 
+  @Message(level = MessageLevel.WARN, text = "Missing property in previously saved service discovery configuration {0}")
+  void missingServiceDiscoveryConfigProperty(String propertyName);
+
+  @Message(level = MessageLevel.DEBUG, text = "There is no cluster configuration for {0} @ {1} to check yet.")
+  void noClusterConfiguration(String clusterName, String discoveryAddress);
+
   @Message(level = MessageLevel.DEBUG, text = "Checking {0} @ {1} for configuration changes...")
   void checkingClusterConfiguration(String clusterName, String discoveryAddress);
 
diff --git a/gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/monitor/DiscoveryConfigurationFileStore.java b/gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/monitor/DiscoveryConfigurationFileStore.java
index bb0d8c7b5..46c96c6b2 100644
--- a/gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/monitor/DiscoveryConfigurationFileStore.java
+++ b/gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/monitor/DiscoveryConfigurationFileStore.java
@@ -17,6 +17,7 @@
 package org.apache.knox.gateway.topology.discovery.cm.monitor;
 
 import org.apache.commons.io.FileUtils;
+import org.apache.commons.lang3.StringUtils;
 import org.apache.knox.gateway.config.GatewayConfig;
 import org.apache.knox.gateway.topology.discovery.ServiceDiscoveryConfig;
 
@@ -86,27 +87,34 @@ public class DiscoveryConfigurationFileStore extends AbstractConfigurationStore
         try (InputStream in = Files.newInputStream(persisted.toPath())) {
           props.load(in);
 
-          result.add(new ServiceDiscoveryConfig() {
-            @Override
-            public String getAddress() {
-              return props.getProperty(PROP_CLUSTER_SOURCE);
-            }
-
-            @Override
-            public String getCluster() {
-              return props.getProperty(PROP_CLUSTER_NAME);
-            }
-
-            @Override
-            public String getUser() {
-              return props.getProperty(PROP_CLUSTER_USER);
-            }
-
-            @Override
-            public String getPasswordAlias() {
-              return props.getProperty(PROP_CLUSTER_ALIAS);
-            }
-          });
+          if (StringUtils.isBlank(props.getProperty(PROP_CLUSTER_SOURCE))) {
+            log.missingServiceDiscoveryConfigProperty(PROP_CLUSTER_SOURCE);
+          } else if (StringUtils.isBlank(props.getProperty(PROP_CLUSTER_NAME))) {
+            log.missingServiceDiscoveryConfigProperty(PROP_CLUSTER_NAME);
+          } else {
+            result.add(new ServiceDiscoveryConfig() {
+              @Override
+              public String getAddress() {
+                return props.getProperty(PROP_CLUSTER_SOURCE);
+              }
+
+              @Override
+              public String getCluster() {
+                return props.getProperty(PROP_CLUSTER_NAME);
+              }
+
+              @Override
+              public String getUser() {
+                return props.getProperty(PROP_CLUSTER_USER);
+              }
+
+              @Override
+              public String getPasswordAlias() {
+                return props.getProperty(PROP_CLUSTER_ALIAS);
+              }
+            });
+          }
+
         } catch (IOException e) {
           log.failedToLoadClusterMonitorServiceDiscoveryConfig(getMonitorType(), e);
         }
diff --git a/gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/monitor/PollingConfigurationAnalyzer.java b/gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/monitor/PollingConfigurationAnalyzer.java
index 7460ea6e9..121b764e5 100644
--- a/gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/monitor/PollingConfigurationAnalyzer.java
+++ b/gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/monitor/PollingConfigurationAnalyzer.java
@@ -204,6 +204,10 @@ public class PollingConfigurationAnalyzer implements Runnable {
         for (Map.Entry<String, List<String>> entry : configCache.getClusterNames().entrySet()) {
           String address = entry.getKey();
           for (String clusterName : entry.getValue()) {
+            if (configCache.getDiscoveryConfig(address, clusterName) == null) {
+              log.noClusterConfiguration(clusterName, address);
+              continue;
+            }
             log.checkingClusterConfiguration(clusterName, address);
 
             // Check here for existing descriptor references, and add to the removal list if there are not any
@@ -234,10 +238,10 @@ public class PollingConfigurationAnalyzer implements Runnable {
         }
         clustersToStopMonitoring.clear(); // reset the removal list
 
-        waitFor(interval);
       } catch (Exception e) {
         log.clouderaManagerConfigurationChangesMonitoringError(e);
       }
+      waitFor(interval);
     }
 
     log.stoppedClouderaManagerConfigMonitor();
diff --git a/gateway-discovery-cm/src/test/java/org/apache/knox/gateway/topology/discovery/cm/monitor/DiscoveryConfigurationFileStoreTest.java b/gateway-discovery-cm/src/test/java/org/apache/knox/gateway/topology/discovery/cm/monitor/DiscoveryConfigurationFileStoreTest.java
index c5e93738b..fd4eeab71 100644
--- a/gateway-discovery-cm/src/test/java/org/apache/knox/gateway/topology/discovery/cm/monitor/DiscoveryConfigurationFileStoreTest.java
+++ b/gateway-discovery-cm/src/test/java/org/apache/knox/gateway/topology/discovery/cm/monitor/DiscoveryConfigurationFileStoreTest.java
@@ -16,14 +16,20 @@
  */
 package org.apache.knox.gateway.topology.discovery.cm.monitor;
 
-import org.apache.knox.gateway.topology.discovery.ServiceDiscoveryConfig;
-import org.junit.Test;
-
-import java.util.Set;
-
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.channels.FileChannel;
+import java.nio.file.Paths;
+import java.nio.file.StandardOpenOption;
+import java.util.Set;
+
+import org.apache.knox.gateway.topology.discovery.ServiceDiscoveryConfig;
+import org.junit.Test;
 
 
 public class DiscoveryConfigurationFileStoreTest extends AbstractConfigurationStoreTest {
@@ -61,6 +67,25 @@ public class DiscoveryConfigurationFileStoreTest extends AbstractConfigurationSt
     }
   }
 
+  @Test
+  public void testLoadingEmptyFile() throws IOException {
+    final DiscoveryConfigurationFileStore configStore = new DiscoveryConfigurationFileStore(createGatewayConfig());
+    final ServiceDiscoveryConfig serviceDiscoveryConfig = createConfig("http://myhost:1234", "myCluster", "iam", "pwd.alias");
+    try {
+      configStore.store(serviceDiscoveryConfig);
+      final File persistenceFile = configStore.getPersistenceFile(serviceDiscoveryConfig.getAddress(), serviceDiscoveryConfig.getCluster());
+      assertTrue(persistenceFile.length() > 0);
+      //truncate file content
+      FileChannel.open(Paths.get(persistenceFile.getAbsolutePath()), StandardOpenOption.WRITE).truncate(0).close();
+      assertEquals(0, persistenceFile.length());
+      final Set<ServiceDiscoveryConfig> persistedConfigs = configStore.getAll();
+      assertTrue(persistedConfigs.isEmpty());
+    } finally {
+      configStore.remove(serviceDiscoveryConfig.getAddress(), serviceDiscoveryConfig.getCluster());
+      assertEquals(0, listFiles(DATA_DIR).size());
+    }
+  }
+
   @Test
   public void testMultiple() {
     final DiscoveryConfigurationStore configStore = new DiscoveryConfigurationFileStore(createGatewayConfig());