ambari git commit: AMBARI-15942 - Upgrade Checks Fail After New Host Is Added To Cluster (jonathanhurley)

[jo...@apache.org]

Repository: ambari
Updated Branches:
  refs/heads/trunk 4ff426f9b -> f7f2ebe03


AMBARI-15942 - Upgrade Checks Fail After New Host Is Added To Cluster (jonathanhurley)


Project: http://git-wip-us.apache.org/repos/asf/ambari/repo
Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/f7f2ebe0
Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/f7f2ebe0
Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/f7f2ebe0

Branch: refs/heads/trunk
Commit: f7f2ebe036395f0c51e4114ab8ab59916635ce63
Parents: 4ff426f
Author: Jonathan Hurley <jh...@hortonworks.com>
Authored: Mon Apr 18 10:10:08 2016 -0400
Committer: Jonathan Hurley <jh...@hortonworks.com>
Committed: Mon Apr 18 10:10:30 2016 -0400

----------------------------------------------------------------------
 .../apache/ambari/server/state/Clusters.java    |  8 ++
 .../server/state/cluster/ClustersImpl.java      | 29 ++++---
 .../ambari/server/state/host/HostImpl.java      |  8 +-
 .../server/state/cluster/ClustersTest.java      | 88 ++++++++++++++------
 4 files changed, 97 insertions(+), 36 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/ambari/blob/f7f2ebe0/ambari-server/src/main/java/org/apache/ambari/server/state/Clusters.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/state/Clusters.java b/ambari-server/src/main/java/org/apache/ambari/server/state/Clusters.java
index 59dcaf8..e660d20 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/state/Clusters.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/state/Clusters.java
@@ -119,6 +119,14 @@ public interface Clusters {
   Host getHostById(Long hostId) throws AmbariException;
 
   /**
+   * Updates the internal mappings of hosts using the specified host.
+   *
+   * @param host
+   *          the host to update the internal mappings for.
+   */
+  void updateHostMappings(Host host);
+
+  /**
    * Add a Host object to be managed by this server
    * @param hostname Host to be added
    * @throws AmbariException

http://git-wip-us.apache.org/repos/asf/ambari/blob/f7f2ebe0/ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClustersImpl.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClustersImpl.java b/ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClustersImpl.java
index dcbf5a5..f78dd95 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClustersImpl.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClustersImpl.java
@@ -18,6 +18,7 @@
 
 package org.apache.ambari.server.state.cluster;
 
+import java.text.MessageFormat;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
@@ -38,7 +39,6 @@ import org.apache.ambari.server.DuplicateResourceException;
 import org.apache.ambari.server.HostNotFoundException;
 import org.apache.ambari.server.agent.DiskInfo;
 import org.apache.ambari.server.api.services.AmbariMetaInfo;
-import org.apache.ambari.server.configuration.Configuration;
 import org.apache.ambari.server.events.HostAddedEvent;
 import org.apache.ambari.server.events.HostRegisteredEvent;
 import org.apache.ambari.server.events.HostRemovedEvent;
@@ -143,8 +143,6 @@ public class ClustersImpl implements Clusters {
   @Inject
   private HostFactory hostFactory;
   @Inject
-  private Configuration configuration;
-  @Inject
   private AmbariMetaInfo ambariMetaInfo;
   @Inject
   private SecurityHelper securityHelper;
@@ -409,6 +407,18 @@ public class ClustersImpl implements Clusters {
   }
 
   /**
+   * {@inheritDoc}
+   */
+  @Override
+  public void updateHostMappings(Host host) {
+    Long hostId = host.getHostId();
+
+    if (null != hostId) {
+      hostsById.put(hostId, host);
+    }
+  }
+
+  /**
    * Register a host by creating a {@link HostEntity} object in the database and setting its state to
    * {@link HostState#INIT}. This does not add the host the cluster.
    * @param hostname Host to be added
@@ -418,11 +428,8 @@ public class ClustersImpl implements Clusters {
   public void addHost(String hostname) throws AmbariException {
     checkLoaded();
 
-    String duplicateMessage = "Duplicate entry for Host"
-        + ", hostName= " + hostname;
-
     if (hosts.containsKey(hostname)) {
-      throw new AmbariException(duplicateMessage);
+      throw new AmbariException(MessageFormat.format("Duplicate entry for Host {0}", hostname));
     }
 
     w.lock();
@@ -440,7 +447,11 @@ public class ClustersImpl implements Clusters {
       host.setHealthStatus(new HostHealthStatus(HealthStatus.UNKNOWN, ""));
       host.setHostAttributes(new HashMap<String, String>());
       host.setState(HostState.INIT);
+
+      // the hosts by ID map is updated separately since the host has not yet
+      // been persisted yet - the below event is what causes the persist
       hosts.put(hostname, host);
+
       hostClusterMap.put(hostname, Collections.newSetFromMap(new ConcurrentHashMap<Cluster, Boolean>()));
 
       if (LOG.isDebugEnabled()) {
@@ -752,9 +763,7 @@ public class ClustersImpl implements Clusters {
   @Override
   public void unmapHostFromCluster(String hostname, String clusterName) throws AmbariException {
     final Cluster cluster = getCluster(clusterName);
-    unmapHostFromClusters(hostname, new HashSet<Cluster>() {{
-      add(cluster);
-    }});
+    unmapHostFromClusters(hostname, Sets.newHashSet(cluster));
   }
 
   @Transactional

http://git-wip-us.apache.org/repos/asf/ambari/blob/f7f2ebe0/ambari-server/src/main/java/org/apache/ambari/server/state/host/HostImpl.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/state/host/HostImpl.java b/ambari-server/src/main/java/org/apache/ambari/server/state/host/HostImpl.java
index 186590e..0514720 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/state/host/HostImpl.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/state/host/HostImpl.java
@@ -295,7 +295,10 @@ public class HostImpl implements Host {
         + e.hostInfo.toString()
         + ", registrationTime=" + e.registrationTime
         + ", agentVersion=" + agentVersion);
+
       host.persist();
+      host.clusters.updateHostMappings(host);
+
       //todo: proper host joined notification
       boolean associatedWithCluster = false;
       try {
@@ -1360,11 +1363,12 @@ public class HostImpl implements Host {
             hostConfigMap.put(configType, hostConfig);
             if (cluster != null) {
               Config conf = cluster.getDesiredConfigByType(configType);
-              if(conf == null)
+              if(conf == null) {
                 LOG.error("Config inconsistency exists:"+
                     " unknown configType="+configType);
-              else
+              } else {
                 hostConfig.setDefaultVersionTag(conf.getTag());
+              }
             }
           }
           Config config = configEntry.getValue();

http://git-wip-us.apache.org/repos/asf/ambari/blob/f7f2ebe0/ambari-server/src/test/java/org/apache/ambari/server/state/cluster/ClustersTest.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/state/cluster/ClustersTest.java b/ambari-server/src/test/java/org/apache/ambari/server/state/cluster/ClustersTest.java
index b563268..43645b4 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/state/cluster/ClustersTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/state/cluster/ClustersTest.java
@@ -18,17 +18,31 @@
 
 package org.apache.ambari.server.state.cluster;
 
-import com.google.common.collect.Maps;
-import com.google.inject.Guice;
-import com.google.inject.Inject;
-import com.google.inject.Injector;
-import com.google.inject.persist.PersistService;
-import junit.framework.Assert;
+import static org.easymock.EasyMock.createNiceMock;
+import static org.easymock.EasyMock.expect;
+import static org.easymock.EasyMock.replay;
+import static org.junit.Assert.fail;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.UUID;
+
+import javax.persistence.EntityManager;
+
 import org.apache.ambari.server.AmbariException;
 import org.apache.ambari.server.ClusterNotFoundException;
 import org.apache.ambari.server.DuplicateResourceException;
 import org.apache.ambari.server.HostNotFoundException;
-import org.apache.ambari.server.api.services.AmbariMetaInfo;
+import org.apache.ambari.server.agent.AgentEnv;
+import org.apache.ambari.server.agent.HostInfo;
+import org.apache.ambari.server.events.HostRegisteredEvent;
 import org.apache.ambari.server.orm.GuiceJpaInitializer;
 import org.apache.ambari.server.orm.InMemoryDefaultTestModule;
 import org.apache.ambari.server.orm.OrmTestHelper;
@@ -41,6 +55,7 @@ import org.apache.ambari.server.orm.dao.TopologyRequestDAO;
 import org.apache.ambari.server.orm.entities.ClusterStateEntity;
 import org.apache.ambari.server.orm.entities.HostComponentDesiredStateEntityPK;
 import org.apache.ambari.server.orm.entities.HostEntity;
+import org.apache.ambari.server.state.AgentVersion;
 import org.apache.ambari.server.state.Cluster;
 import org.apache.ambari.server.state.Clusters;
 import org.apache.ambari.server.state.Config;
@@ -53,6 +68,7 @@ import org.apache.ambari.server.state.ServiceComponent;
 import org.apache.ambari.server.state.ServiceComponentHost;
 import org.apache.ambari.server.state.StackId;
 import org.apache.ambari.server.state.State;
+import org.apache.ambari.server.state.host.HostRegistrationRequestEvent;
 import org.apache.ambari.server.topology.Blueprint;
 import org.apache.ambari.server.topology.Configuration;
 import org.apache.ambari.server.topology.HostGroupInfo;
@@ -61,34 +77,24 @@ import org.apache.ambari.server.topology.LogicalRequest;
 import org.apache.ambari.server.topology.PersistedState;
 import org.apache.ambari.server.topology.TopologyRequest;
 import org.apache.ambari.server.topology.TopologyTask;
+import org.apache.ambari.server.utils.EventBusSynchronizer;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 
-import javax.persistence.EntityManager;
-
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
+import com.google.common.collect.Maps;
+import com.google.inject.Guice;
+import com.google.inject.Inject;
+import com.google.inject.Injector;
+import com.google.inject.persist.PersistService;
 
-import static org.easymock.EasyMock.createNiceMock;
-import static org.easymock.EasyMock.expect;
-import static org.easymock.EasyMock.replay;
-import static org.junit.Assert.fail;
+import junit.framework.Assert;
 
 public class ClustersTest {
 
   private Clusters clusters;
   private Injector injector;
   @Inject
-  private AmbariMetaInfo metaInfo;
-  @Inject
   private OrmTestHelper helper;
   @Inject
   private HostDAO hostDAO;
@@ -593,6 +599,40 @@ public class ClustersTest {
     }
   }
 
+  /**
+   * Tests that {@link HostRegisteredEvent} properly updates the
+   * {@link Clusters} in-memory mapping of hostIds to hosts.
+   *
+   * @throws AmbariException
+   */
+  @Test
+  public void testHostRegistrationPopulatesIdMapping() throws Exception {
+    String clusterName = UUID.randomUUID().toString();
+    String hostName = UUID.randomUUID().toString();
+
+    // required so that the event which does the work is executed synchornously
+    EventBusSynchronizer.synchronizeAmbariEventPublisher(injector);
+
+    Cluster cluster = createCluster(clusterName);
+    Assert.assertNotNull(cluster);
+
+    addHostToCluster(hostName, clusterName);
+    Host host = clusters.getHost(hostName);
+    Assert.assertNotNull(host);
+
+    HostRegistrationRequestEvent registrationEvent = new HostRegistrationRequestEvent(
+        host.getHostName(),
+        new AgentVersion(""), System.currentTimeMillis(), new HostInfo(), new AgentEnv());
+
+    host.handleEvent(registrationEvent);
+
+    Long hostId = host.getHostId();
+    Assert.assertNotNull(hostId);
+
+    host = clusters.getHostById(hostId);
+    Assert.assertNotNull(host);
+  }
+
   private void createTopologyRequest(Cluster cluster, String hostName) {
     final String groupName = "MyHostGroup";