You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ambari.apache.org by be...@apache.org on 2018/11/14 11:10:20 UTC

[ambari] branch trunk updated: AMBARI-24888 fix SingleHostTopologyUpdater (benyoka) (#2603)

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

benyoka pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/ambari.git


The following commit(s) were added to refs/heads/trunk by this push:
     new 6395167  AMBARI-24888 fix SingleHostTopologyUpdater (benyoka) (#2603)
6395167 is described below

commit 6395167a5a74d44b9d86dabeb2c0860df450ec67
Author: benyoka <be...@users.noreply.github.com>
AuthorDate: Wed Nov 14 12:10:15 2018 +0100

    AMBARI-24888 fix SingleHostTopologyUpdater (benyoka) (#2603)
    
    * AMBARI-24888 fix SingleHostTopologyUpdater (benyoka)
    
    * AMBARI-24888 review comment (benyoka)
---
 .../internal/BlueprintConfigurationProcessor.java  | 80 ++++++----------------
 .../BlueprintConfigurationProcessorTest.java       | 36 ++++++++++
 2 files changed, 56 insertions(+), 60 deletions(-)

diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java
index 7803e90..1daf1f5 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java
@@ -122,6 +122,7 @@ public class BlueprintConfigurationProcessor {
 
   private final static String HADOOP_ENV_CONFIG_TYPE_NAME = "hadoop-env";
   private final static String RANGER_TAGSYNC_SITE_CONFIG_TYPE_NAME = "ranger-tagsync-site";
+  private static final String LOCALHOST = "localhost";
 
 
   /**
@@ -1887,14 +1888,20 @@ public class BlueprintConfigurationProcessor {
                                          ClusterTopology topology) {
 
       String replacedValue = super.updateForClusterCreate(propertyName, origValue, properties, topology);
+      // %HOSTGROUP% token replacement happened
       if (!Objects.equals(origValue, replacedValue)) {
         return replacedValue;
-      } else {
+      }
+      // localhost typically means stack default values. If property is set to a concrete value such as an FQDN skip
+      // validation and update
+      else if (null != origValue && !origValue.contains(LOCALHOST)) {
+        return origValue;
+      }
+      else {
         int matchingGroupCount = topology.getHostGroupsForComponent(component).size();
         if (matchingGroupCount == 1) {
           //todo: warn if > 1 hosts
-          return replacePropertyValue(origValue,
-            topology.getHostAssignmentsForComponent(component).iterator().next(), properties);
+          return origValue.replace(LOCALHOST, topology.getHostAssignmentsForComponent(component).iterator().next() );
         } else {
           //todo: extract all hard coded HA logic
           Cardinality cardinality = topology.getBlueprint().getStack().getCardinality(component);
@@ -1924,12 +1931,6 @@ public class BlueprintConfigurationProcessor {
                 // reference must point to the logical nameservice, rather than an individual namenode
                 return origValue;
               }
-
-              if (!origValue.contains("localhost")) {
-                // if this NameNode HA property is a FDQN, then simply return it
-                return origValue;
-              }
-
             }
 
             if (topology.isNameNodeHAEnabled() && isComponentSecondaryNameNode() && (matchingGroupCount == 0)) {
@@ -1938,48 +1939,6 @@ public class BlueprintConfigurationProcessor {
               return origValue;
             }
 
-            if (topology.isYarnResourceManagerHAEnabled() && isComponentResourceManager() && (matchingGroupCount == 2)) {
-              if (!origValue.contains("localhost")) {
-                // if this Yarn property is a FQDN, then simply return it
-                return origValue;
-              }
-            }
-
-            if ((isOozieServerHAEnabled(properties)) && isComponentOozieServer() && (matchingGroupCount > 1)) {
-              if (!origValue.contains("localhost")) {
-                // if this Oozie property is a FQDN, then simply return it
-                return origValue;
-              }
-            }
-
-            if ((isHiveServerHAEnabled(properties)) && isComponentHiveServer() && (matchingGroupCount > 1)) {
-              if (!origValue.contains("localhost")) {
-                // if this Hive property is a FQDN, then simply return it
-                return origValue;
-              }
-            }
-
-            if ((isComponentHiveMetaStoreServer()) && matchingGroupCount > 1) {
-              if (!origValue.contains("localhost")) {
-                // if this Hive MetaStore property is a FQDN, then simply return it
-                return origValue;
-              }
-            }
-
-            if (isRangerAdmin() && matchingGroupCount > 1) {
-              if (origValue != null && !origValue.contains("localhost")) {
-                // if this Ranger admin property is a FQDN then simply return it
-                return origValue;
-              }
-            }
-
-            if ((isComponentAppTimelineServer() || isComponentHistoryServer()) &&
-              (matchingGroupCount > 1 && origValue != null && !origValue.contains("localhost"))) {
-              // in case of multiple component instances of AppTimelineServer or History Server leave custom value
-              // if set
-              return origValue;
-            }
-
             if (topology.isComponentHadoopCompatible(component)) {
               return origValue;
             }
@@ -1992,10 +1951,6 @@ public class BlueprintConfigurationProcessor {
       }
     }
 
-    public String replacePropertyValue(String origValue, String host, Map<String, Map<String, String>> properties) {
-      return origValue.replace("localhost", host);
-    }
-
     @Override
     public Collection<String> getRequiredHostGroups(String propertyName,
                                                     String origValue,
@@ -2147,7 +2102,12 @@ public class BlueprintConfigurationProcessor {
    * This updater detects the case when the specified component
    * is not found, and returns the original property value.
    *
+   * @deprecated {@link SingleHostTopologyUpdater} has been changed not to validate explicitly set (other than
+   *   the typically stack default {@code localhost}) values. The new semantics make this class obsolete. If you want to
+   *   submit a cluster with some intentionally missing components, set respective properties to a value other than the
+   *   stack default {@code localhost} (e.g it can be empty string or and FQDN).
    */
+  @Deprecated
   private static class OptionalSingleHostTopologyUpdater extends SingleHostTopologyUpdater {
 
     public OptionalSingleHostTopologyUpdater(String component) {
@@ -2333,7 +2293,7 @@ public class BlueprintConfigurationProcessor {
                                          Map<String, Map<String, String>> properties,
                                          ClusterTopology topology) {
 
-      if (!origValue.contains("%HOSTGROUP") && (!origValue.contains("localhost"))) {
+      if (!origValue.contains("%HOSTGROUP") && (!origValue.contains(LOCALHOST))) {
         // this property must contain FQDNs specified directly by the user
         // of the Blueprint, so the processor should not attempt to update them
         return origValue;
@@ -2422,7 +2382,7 @@ public class BlueprintConfigurationProcessor {
      */
     private Collection<String> getHostStringsFromLocalhost(String origValue, ClusterTopology topology) {
       Set<String> hostStrings = new HashSet<>();
-      if(origValue.contains("localhost")) {
+      if(origValue.contains(LOCALHOST)) {
         Matcher localhostMatcher = LOCALHOST_PORT_REGEX.matcher(origValue);
         String port = null;
         if(localhostMatcher.find()) {
@@ -2589,7 +2549,7 @@ public class BlueprintConfigurationProcessor {
      */
     public boolean isFQDNValue(String value) {
       return !value.contains("%HOSTGROUP") &&
-        !value.contains("localhost");
+        !value.contains(LOCALHOST);
     }
   }
 
@@ -2731,7 +2691,7 @@ public class BlueprintConfigurationProcessor {
 
       // short-circuit out any custom property values defined by the deployer
       if (!origValue.contains("%HOSTGROUP") &&
-        (!origValue.contains("localhost"))) {
+        (!origValue.contains(LOCALHOST))) {
         // this property must contain FQDNs specified directly by the user
         // of the Blueprint, so the processor should not attempt to update them
         return origValue;
@@ -2774,7 +2734,7 @@ public class BlueprintConfigurationProcessor {
 
       // short-circuit out any custom property values defined by the deployer
       if (!origValue.contains("%HOSTGROUP") &&
-        (!origValue.contains("localhost"))) {
+        (!origValue.contains(LOCALHOST))) {
         // this property must contain FQDNs specified directly by the user
         // of the Blueprint, so the processor should not attempt to update them
         return Collections.emptySet();
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java
index de70e36..835cda5 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java
@@ -2598,6 +2598,42 @@ public class BlueprintConfigurationProcessorTest extends EasyMockSupport {
   }
 
   @Test
+  public void testDoUpdateForClusterCreate_SingleHostProperty__MissingComponent_NoValidationForFqdn() throws Exception {
+    Map<String, Map<String, String>> properties = new HashMap<>();
+    Map<String, String> typeProps = new HashMap<>();
+    properties.put("mapred-site",
+      new HashMap<>(ImmutableMap.of("mapreduce.job.hdfs-servers", "www.externalnamenode.org")));
+    Configuration clusterConfig = new Configuration(properties, emptyMap());
+
+    Collection<String> group1Components = new HashSet<>();
+    group1Components.add("SECONDARY_NAMENODE");
+    group1Components.add("RESOURCEMANAGER");
+    TestHostGroup group1 = new TestHostGroup("group1", group1Components, Collections.singleton("testhost"));
+
+    Collection<String> group2Components = new HashSet<>();
+    group2Components.add("DATANODE");
+    group2Components.add("HDFS_CLIENT");
+    TestHostGroup group2 = new TestHostGroup("group2", group2Components, Collections.singleton("testhost2"));
+
+    Collection<TestHostGroup> hostGroups = new HashSet<>();
+    hostGroups.add(group1);
+    hostGroups.add(group2);
+
+    expect(stack.getCardinality("NAMENODE")).andReturn(new Cardinality("1")).anyTimes();
+
+    ClusterTopology topology = createClusterTopology(bp, clusterConfig, hostGroups);
+    BlueprintConfigurationProcessor updater = new BlueprintConfigurationProcessor(topology);
+
+    // No exception this time as SingleHostTopologyUpdater does no validation for fqdn's
+    updater.doUpdateForClusterCreate();
+    // No change as fqdn's are not updated
+    assertEquals(
+      "www.externalnamenode.org",
+      clusterConfig.getPropertyValue("mapred-site", "mapreduce.job.hdfs-servers"));
+  }
+
+
+  @Test
   public void testDoUpdateForClusterCreate_SingleHostProperty__MultipleMatchingHostGroupsError() throws Exception {
 
     Map<String, Map<String, String>> properties = new HashMap<>();