Repository: ambari
Updated Branches:
  refs/heads/branch-2.5 57588964c -> 883779d22


AMBARI-20835. Unable to proceed from manual prompt in EU wizard due to 
IllegalArgumentException (ncole)


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

Branch: refs/heads/branch-2.5
Commit: 883779d2209c7de7cec7190d91f380c517dc83f7
Parents: 5758896
Author: Nate Cole <nc...@hortonworks.com>
Authored: Mon Apr 24 13:18:55 2017 -0400
Committer: Nate Cole <nc...@hortonworks.com>
Committed: Mon Apr 24 17:09:22 2017 -0400

----------------------------------------------------------------------
 .../actionmanager/ActionDBAccessorImpl.java     | 18 ++++----
 .../server/orm/dao/HostRoleCommandDAO.java      |  2 +-
 .../FixCapacitySchedulerOrderingPolicy.java     |  5 ++
 .../server/state/cluster/ClustersImpl.java      |  4 --
 .../server/actionmanager/TestActionManager.java | 48 +++++++++++++++++---
 .../AmbariManagementControllerTest.java         | 17 +++++--
 6 files changed, 70 insertions(+), 24 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/ambari/blob/883779d2/ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ActionDBAccessorImpl.java
----------------------------------------------------------------------
diff --git 
a/ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ActionDBAccessorImpl.java
 
b/ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ActionDBAccessorImpl.java
index 27ddd62..ee18007 100644
--- 
a/ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ActionDBAccessorImpl.java
+++ 
b/ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ActionDBAccessorImpl.java
@@ -358,7 +358,7 @@ public class ActionDBAccessorImpl implements 
ActionDBAccessor {
     requestDAO.create(requestEntity);
 
     //TODO wire request to cluster
-    List<StageEntity> stageEntities = new 
ArrayList<StageEntity>(request.getStages().size());
+    List<StageEntity> stageEntities = new 
ArrayList<>(request.getStages().size());
 
     addRequestToAuditlogCache(request);
 
@@ -370,13 +370,14 @@ public class ActionDBAccessorImpl implements 
ActionDBAccessor {
       stageDAO.create(stageEntity);
 
       List<HostRoleCommand> orderedHostRoleCommands = 
stage.getOrderedHostRoleCommands();
+      List<HostRoleCommandEntity> hostRoleCommandEntities = new ArrayList<>();
 
       for (HostRoleCommand hostRoleCommand : orderedHostRoleCommands) {
         HostRoleCommandEntity hostRoleCommandEntity = 
hostRoleCommand.constructNewPersistenceEntity();
         hostRoleCommandEntity.setStage(stageEntity);
         hostRoleCommandDAO.create(hostRoleCommandEntity);
+        hostRoleCommandEntities.add(hostRoleCommandEntity);
 
-        assert hostRoleCommandEntity.getTaskId() != null;
         hostRoleCommand.setTaskId(hostRoleCommandEntity.getTaskId());
 
         String prefix = "";
@@ -432,6 +433,7 @@ public class ActionDBAccessorImpl implements 
ActionDBAccessor {
         roleSuccessCriteriaDAO.create(roleSuccessCriteriaEntity);
       }
 
+      stageEntity.setHostRoleCommands(hostRoleCommandEntities);
       stageEntity = stageDAO.merge(stageEntity);
     }
 
@@ -495,15 +497,15 @@ public class ActionDBAccessorImpl implements 
ActionDBAccessor {
 
   @Override
   public void updateHostRoleStates(Collection<CommandReport> reports) {
-    Map<Long, CommandReport> taskReports = new HashMap<Long, CommandReport>();
+    Map<Long, CommandReport> taskReports = new HashMap<>();
     for (CommandReport report : reports) {
       taskReports.put(report.getTaskId(), report);
     }
 
     long now = System.currentTimeMillis();
 
-    List<Long> requestsToCheck = new ArrayList<Long>();
-    List<Long> abortedCommandUpdates = new ArrayList<Long>();
+    List<Long> requestsToCheck = new ArrayList<>();
+    List<Long> abortedCommandUpdates = new ArrayList<>();
 
     List<HostRoleCommandEntity> commandEntities = 
hostRoleCommandDAO.findByPKs(taskReports.keySet());
     for (HostRoleCommandEntity commandEntity : commandEntities) {
@@ -707,12 +709,12 @@ public class ActionDBAccessorImpl implements 
ActionDBAccessor {
       return Collections.emptyList();
     }
 
-    List<HostRoleCommand> commands = new ArrayList<HostRoleCommand>();
+    List<HostRoleCommand> commands = new ArrayList<>();
 
     Map<Long, HostRoleCommand> cached = 
hostRoleCommandCache.getAllPresent(taskIds);
     commands.addAll(cached.values());
 
-    List<Long> absent = new ArrayList<Long>();
+    List<Long> absent = new ArrayList<>();
     absent.addAll(taskIds);
     absent.removeAll(cached.keySet());
 
@@ -801,7 +803,7 @@ public class ActionDBAccessorImpl implements 
ActionDBAccessor {
   @Override
   public List<Request> getRequests(Collection<Long> requestIds) {
     List<RequestEntity> requestEntities = requestDAO.findByPks(requestIds);
-    List<Request> requests = new ArrayList<Request>(requestEntities.size());
+    List<Request> requests = new ArrayList<>(requestEntities.size());
     for (RequestEntity requestEntity : requestEntities) {
       requests.add(requestFactory.createExisting(requestEntity));
     }

http://git-wip-us.apache.org/repos/asf/ambari/blob/883779d2/ambari-server/src/main/java/org/apache/ambari/server/orm/dao/HostRoleCommandDAO.java
----------------------------------------------------------------------
diff --git 
a/ambari-server/src/main/java/org/apache/ambari/server/orm/dao/HostRoleCommandDAO.java
 
b/ambari-server/src/main/java/org/apache/ambari/server/orm/dao/HostRoleCommandDAO.java
index 30b3100..7582957 100644
--- 
a/ambari-server/src/main/java/org/apache/ambari/server/orm/dao/HostRoleCommandDAO.java
+++ 
b/ambari-server/src/main/java/org/apache/ambari/server/orm/dao/HostRoleCommandDAO.java
@@ -676,7 +676,7 @@ public class HostRoleCommandDAO {
   @TransactionalLock(lockArea = LockArea.HRC_STATUS_CACHE, lockType = 
LockType.WRITE)
   public void remove(HostRoleCommandEntity entity) {
     EntityManager entityManager = entityManagerProvider.get();
-    entityManager.remove(merge(entity));
+    entityManager.remove(entity);
     invalidateHostRoleCommandStatusSummaryCache(entity);
   }
 

http://git-wip-us.apache.org/repos/asf/ambari/blob/883779d2/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/FixCapacitySchedulerOrderingPolicy.java
----------------------------------------------------------------------
diff --git 
a/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/FixCapacitySchedulerOrderingPolicy.java
 
b/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/FixCapacitySchedulerOrderingPolicy.java
index fbb88d8..92f6e50 100644
--- 
a/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/FixCapacitySchedulerOrderingPolicy.java
+++ 
b/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/FixCapacitySchedulerOrderingPolicy.java
@@ -66,6 +66,11 @@ public class FixCapacitySchedulerOrderingPolicy extends 
AbstractServerAction {
     Cluster cluster = clusters.getCluster(clusterName);
     Config config = cluster.getDesiredConfigByType(SOURCE_CONFIG_TYPE);
 
+    if (null == config) {
+      return createCommandReport(0, HostRoleStatus.COMPLETED, "{}",
+          String.format("The cluster does not have %s defined.", 
SOURCE_CONFIG_TYPE), "");
+    }
+
     Map<String, String> properties = config.getProperties();
 
     Set<String> parentQueueNames = new HashSet<>();

http://git-wip-us.apache.org/repos/asf/ambari/blob/883779d2/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 6c89b88..e0a4965 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
@@ -811,10 +811,6 @@ public class ClustersImpl implements Clusters {
       }
     }
 
-
-    entity.setHostRoleCommandEntities(null);
-    hostRoleCommandDAO.removeByHostId(entity.getHostId());
-
     entity.setHostStateEntity(null);
     hostStateDAO.removeByHostId(entity.getHostId());
     hostConfigMappingDAO.removeByHostId(entity.getHostId());

http://git-wip-us.apache.org/repos/asf/ambari/blob/883779d2/ambari-server/src/test/java/org/apache/ambari/server/actionmanager/TestActionManager.java
----------------------------------------------------------------------
diff --git 
a/ambari-server/src/test/java/org/apache/ambari/server/actionmanager/TestActionManager.java
 
b/ambari-server/src/test/java/org/apache/ambari/server/actionmanager/TestActionManager.java
index d5f2475..fbd7c4e 100644
--- 
a/ambari-server/src/test/java/org/apache/ambari/server/actionmanager/TestActionManager.java
+++ 
b/ambari-server/src/test/java/org/apache/ambari/server/actionmanager/TestActionManager.java
@@ -26,10 +26,12 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertSame;
+import static org.junit.Assert.assertTrue;
 
 import java.sql.SQLException;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.List;
 
 import org.apache.ambari.server.AmbariException;
@@ -42,11 +44,16 @@ import org.apache.ambari.server.audit.AuditLogger;
 import org.apache.ambari.server.events.publishers.JPAEventPublisher;
 import org.apache.ambari.server.orm.GuiceJpaInitializer;
 import org.apache.ambari.server.orm.InMemoryDefaultTestModule;
+import org.apache.ambari.server.orm.dao.StageDAO;
+import org.apache.ambari.server.orm.entities.HostRoleCommandEntity;
+import org.apache.ambari.server.orm.entities.StageEntity;
+import org.apache.ambari.server.orm.entities.StageEntityPK;
 import org.apache.ambari.server.state.Clusters;
 import org.apache.ambari.server.state.StackId;
 import 
org.apache.ambari.server.state.svccomphost.ServiceComponentHostStartEvent;
 import org.apache.ambari.server.utils.CommandUtils;
 import org.apache.ambari.server.utils.StageUtils;
+import org.apache.commons.collections.CollectionUtils;
 import org.easymock.EasyMock;
 import org.junit.After;
 import org.junit.Before;
@@ -102,7 +109,7 @@ public class TestActionManager {
     Assert.assertEquals(stageId, stage.getStageId());
     stage.setHostRoleStatus(hostname, "HBASE_MASTER", HostRoleStatus.QUEUED);
     db.hostRoleScheduled(stage, hostname, "HBASE_MASTER");
-    List<CommandReport> reports = new ArrayList<CommandReport>();
+    List<CommandReport> reports = new ArrayList<>();
     CommandReport cr = new CommandReport();
     cr.setTaskId(1);
     cr.setActionId(StageUtils.getActionId(requestId, stageId));
@@ -143,7 +150,7 @@ public class TestActionManager {
     Assert.assertEquals(stageId, stage.getStageId());
     stage.setHostRoleStatus(hostname, "HBASE_MASTER", HostRoleStatus.QUEUED);
     db.hostRoleScheduled(stage, hostname, "HBASE_MASTER");
-    List<CommandReport> reports = new ArrayList<CommandReport>();
+    List<CommandReport> reports = new ArrayList<>();
     CommandReport cr = new CommandReport();
     cr.setTaskId(2);
     cr.setActionId(StageUtils.getActionId(requestId, stageId));
@@ -180,7 +187,7 @@ public class TestActionManager {
     Assert.assertEquals(stageId, stage.getStageId());
     stage.setHostRoleStatus(hostname, "HBASE_MASTER", HostRoleStatus.QUEUED);
     db.hostRoleScheduled(stage, hostname, "HBASE_MASTER");
-    List<CommandReport> reports = new ArrayList<CommandReport>();
+    List<CommandReport> reports = new ArrayList<>();
     CommandReport cr = new CommandReport();
     cr.setTaskId(1);
     cr.setActionId(StageUtils.getActionId(requestId, stageId));
@@ -219,7 +226,7 @@ public class TestActionManager {
         RoleCommand.START,
         new ServiceComponentHostStartEvent(Role.HBASE_MASTER.toString(),
             hostname, System.currentTimeMillis()), "cluster1", "HBASE", false, 
false);
-    List<Stage> stages = new ArrayList<Stage>();
+    List<Stage> stages = new ArrayList<>();
     stages.add(s);
     Request request = new Request(stages, clusters);
     db.persistActions(request);
@@ -236,7 +243,7 @@ public class TestActionManager {
         RoleCommand.START,
         new ServiceComponentHostStartEvent(Role.HBASE_REGIONSERVER.toString(),
           hostname, System.currentTimeMillis()), "cluster1", "HBASE", false, 
false);
-    List<Stage> stages = new ArrayList<Stage>();
+    List<Stage> stages = new ArrayList<>();
     stages.add(s);
     Request request = new Request(stages, clusters);
     db.persistActions(request);
@@ -270,7 +277,7 @@ public class TestActionManager {
     Clusters clusters = createNiceMock(Clusters.class);
     Stage stage1 = createNiceMock(Stage.class);
     Stage stage2 = createNiceMock(Stage.class);
-    List<Stage> listStages = new ArrayList<Stage>();
+    List<Stage> listStages = new ArrayList<>();
     listStages.add(stage1);
     listStages.add(stage2);
 
@@ -286,4 +293,33 @@ public class TestActionManager {
 
     verify(queue, db, clusters);
   }
+
+  /**
+   * Tests whether {@link ActionDBAccessor#persistActions(Request)} associates 
tasks with their
+   * stages.  Improvements to {@code Stage} processing exposed the fact that 
the association wasn't
+   * being made, and JPA didn't know of the Stage-to-Tasks child relationship.
+   *
+   * @throws Exception
+   */
+  @Test
+  public void testPersistCommandsWithStages() throws Exception {
+    ActionDBAccessor db = injector.getInstance(ActionDBAccessorImpl.class);
+
+    populateActionDBWithTwoCommands(db, hostname);
+
+    List<Stage> stages = db.getAllStages(requestId);
+    assertEquals(1, stages.size());
+    Stage stage = stages.get(0);
+
+    StageEntityPK pk = new StageEntityPK();
+    pk.setRequestId(stage.getRequestId());
+    pk.setStageId(stage.getStageId());
+
+    StageDAO dao = injector.getInstance(StageDAO.class);
+    StageEntity stageEntity = dao.findByPK(pk);
+    assertNotNull(stageEntity);
+
+    Collection<HostRoleCommandEntity> commandEntities = 
stageEntity.getHostRoleCommands();
+    assertTrue(CollectionUtils.isNotEmpty(commandEntities));
+  }
 }

http://git-wip-us.apache.org/repos/asf/ambari/blob/883779d2/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java
----------------------------------------------------------------------
diff --git 
a/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java
 
b/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java
index a7b99bb..5ef754e 100644
--- 
a/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java
+++ 
b/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java
@@ -18,11 +18,6 @@
 
 package org.apache.ambari.server.controller;
 
-import javax.net.ssl.HostnameVerifier;
-import javax.net.ssl.HttpsURLConnection;
-import javax.net.ssl.SSLSession;
-import javax.persistence.EntityManager;
-import junit.framework.Assert;
 import static org.easymock.EasyMock.capture;
 import static org.easymock.EasyMock.createNiceMock;
 import static org.easymock.EasyMock.createStrictMock;
@@ -55,6 +50,11 @@ import java.util.Properties;
 import java.util.Set;
 import java.util.UUID;
 
+import javax.net.ssl.HostnameVerifier;
+import javax.net.ssl.HttpsURLConnection;
+import javax.net.ssl.SSLSession;
+import javax.persistence.EntityManager;
+
 import org.apache.ambari.server.AmbariException;
 import org.apache.ambari.server.ClusterNotFoundException;
 import org.apache.ambari.server.DuplicateResourceException;
@@ -158,6 +158,7 @@ import org.easymock.Capture;
 import org.easymock.EasyMock;
 import org.junit.After;
 import org.junit.AfterClass;
+import org.junit.Assert;
 import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Ignore;
@@ -9016,6 +9017,8 @@ public class AmbariManagementControllerTest {
 
     Assert.assertNull(topologyHostInfoDAO.findByHostname(host1));
 
+    Long firstHostId = clusters.getHost(host1).getHostId();
+
     // Deletion without specifying cluster should be successful
     requests.clear();
     requests.add(new HostRequest(host1, null, null));
@@ -9029,6 +9032,10 @@ public class AmbariManagementControllerTest {
     Assert.assertFalse(clusters.getClustersForHost(host1).contains(cluster));
     Assert.assertNull(topologyHostInfoDAO.findByHostname(host1));
 
+    // verify there are no host role commands for the host
+    List<HostRoleCommandEntity> tasks = 
hostRoleCommandDAO.findByHostId(firstHostId);
+    assertEquals(0, tasks.size());
+
     // Case 3: Delete host that is still part of the cluster, and specify the 
cluster_name in the request
     requests.clear();
     requests.add(new HostRequest(host2, cluster1, null));

Reply via email to