AMBARI-8687. Fix ignored unit tests for AMBARI-8661 and few production code fixes/improvements (Dmitri Lisnichenko via ncole)

ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterStackVersionResourceProvider.java

@@ -37,6 +37,7 @@ import org.apache.ambari.server.actionmanager.RequestFactory;
 import org.apache.ambari.server.actionmanager.Stage;
 import org.apache.ambari.server.actionmanager.StageFactory;
 import org.apache.ambari.server.api.services.AmbariMetaInfo;
+import org.apache.ambari.server.configuration.Configuration;
 import org.apache.ambari.server.controller.ActionExecutionContext;
 import org.apache.ambari.server.controller.AmbariActionExecutionHelper;
 import org.apache.ambari.server.controller.AmbariManagementController;
@@ -146,6 +147,9 @@ public class ClusterStackVersionResourceProvider extends AbstractControllerResou
   @Inject
   private static RequestFactory requestFactory;
 
+  @Inject
+  private static Configuration configuration;
+
   /**
    * Constructor.
    */
@@ -355,7 +359,7 @@ public class ClusterStackVersionResourceProvider extends AbstractControllerResou
               cluster.getClusterName(), INSTALL_PACKAGES_ACTION,
               Collections.singletonList(filter),
               params);
-      actionContext.setTimeout((short) 600);
+      actionContext.setTimeout(Short.valueOf(configuration.getDefaultAgentTaskTimeout()));
 
       try {
         actionExecutionHelper.get().addExecutionCommandsToStage(actionContext, stage);

ambari-server/src/main/java/org/apache/ambari/server/controller/internal/HostStackVersionResourceProvider.java

@@ -35,6 +35,7 @@ import org.apache.ambari.server.actionmanager.Stage;
 import org.apache.ambari.server.actionmanager.StageFactory;
 import org.apache.ambari.server.actionmanager.RequestFactory;
 import org.apache.ambari.server.api.services.AmbariMetaInfo;
+import org.apache.ambari.server.configuration.Configuration;
 import org.apache.ambari.server.controller.ActionExecutionContext;
 import org.apache.ambari.server.controller.AmbariActionExecutionHelper;
 import org.apache.ambari.server.controller.AmbariManagementController;
@@ -58,6 +59,7 @@ import com.google.inject.Inject;
 import org.apache.ambari.server.orm.entities.RepositoryVersionEntity;
 import org.apache.ambari.server.state.Cluster;
 import org.apache.ambari.server.state.Host;
+import org.apache.ambari.server.state.RepositoryVersionState;
 import org.apache.ambari.server.state.ServiceComponentHost;
 import org.apache.ambari.server.state.ServiceInfo;
 import org.apache.ambari.server.state.ServiceOsSpecific;
@@ -142,6 +144,9 @@ public class HostStackVersionResourceProvider extends AbstractControllerResource
   @Inject
   private static Provider<AmbariActionExecutionHelper> actionExecutionHelper;
 
+  @Inject
+  private static Configuration configuration;
+
 
   /**
    * Constructor.
@@ -318,6 +323,20 @@ public class HostStackVersionResourceProvider extends AbstractControllerResource
               desiredRepoVersion, stackId));
     }
 
+    HostVersionEntity hostVersEntity = hostVersionDAO.findByClusterStackVersionAndHost(clName, stackId,
+            desiredRepoVersion, hostName);
+    if (hostVersEntity == null) {
+      throw new IllegalArgumentException(String.format(
+        "Repo version %s for stack %s is not available for host %s",
+        desiredRepoVersion, stackId, hostName));
+    }
+    if (hostVersEntity.getState() != RepositoryVersionState.INSTALLED &&
+            hostVersEntity.getState() != RepositoryVersionState.INSTALL_FAILED) {
+      throw new UnsupportedOperationException(String.format("Repo version %s for stack %s " +
+        "for host %s is in %s state. Can not transition to INSTALLING state",
+              desiredRepoVersion, stackId, hostName, hostVersEntity.getState().toString()));
+    }
+
     List<OperatingSystemEntity> operatingSystems = repoVersionEnt.getOperatingSystems();
     Map<String, List<RepositoryEntity>> perOsRepos = new HashMap<String, List<RepositoryEntity>>();
     for (OperatingSystemEntity operatingSystem : operatingSystems) {
@@ -368,7 +387,7 @@ public class HostStackVersionResourceProvider extends AbstractControllerResource
             cluster.getClusterName(), INSTALL_PACKAGES_ACTION,
             Collections.singletonList(filter),
             params);
-    actionContext.setTimeout((short) 600);
+    actionContext.setTimeout(Short.valueOf(configuration.getDefaultAgentTaskTimeout()));
 
     String caption = String.format(INSTALL_PACKAGES_FULL_NAME + " on host %s", hostName);
     RequestStageContainer req = createRequest(caption);
@@ -399,6 +418,8 @@ public class HostStackVersionResourceProvider extends AbstractControllerResource
     }
 
     try {
+      hostVersEntity.setState(RepositoryVersionState.INSTALLING);
+      hostVersionDAO.merge(hostVersEntity);
       req.persist();
     } catch (AmbariException e) {
       throw new SystemException("Can not persist request", e);

ambari-server/src/main/java/org/apache/ambari/server/state/RepositoryVersionState.java

@@ -71,6 +71,8 @@ package org.apache.ambari.server.state;
  * UPGRADE_FAILED -> UPGRADING
  * UPGRADED -> CURRENT
  * INSTALLING -> INSTALLED | INSTALL_FAILED
+ * INSTALLED -> INSTALLED
+ * INSTALLED -> INSTALLING
  * INSTALL_FAILED -> INSTALLING
  * CURRENT -> INSTALLED
  * INSTALLED -> UPGRADING

ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java

@@ -1237,12 +1237,14 @@ public class ClusterImpl implements Cluster {
                 stackId, repositoryVersion);
 
         if (clusterVersion == null) {
-          throw new AmbariException("Repository version is null");
+          throw new AmbariException(String.format("Repository version %s not found for cluster %s",
+                  repositoryVersion, getClusterName()));
         }
 
         RepositoryVersionState worstState;
         if (clusterVersion.getState() != RepositoryVersionState.INSTALL_FAILED &&
-                clusterVersion.getState() != RepositoryVersionState.INSTALLING) {
+                clusterVersion.getState() != RepositoryVersionState.INSTALLING &&
+                clusterVersion.getState() != RepositoryVersionState.INSTALLED) {
           // anything else is not supported as of now
           return;
         }
@@ -1356,6 +1358,8 @@ public class ClusterImpl implements Cluster {
             case INSTALL_FAILED:
               allowedStates.add(RepositoryVersionState.INSTALLING);
             case INSTALLED:
+              allowedStates.add(RepositoryVersionState.INSTALLED);  // To allow reinstall
+              allowedStates.add(RepositoryVersionState.INSTALLING); // To allow reinstall
               allowedStates.add(RepositoryVersionState.UPGRADING);
             case UPGRADING:
               allowedStates.add(RepositoryVersionState.CURRENT);

ambari-server/src/test/java/org/apache/ambari/server/agent/TestHeartbeatHandler.java

@@ -98,7 +98,6 @@ import org.apache.ambari.server.utils.StageUtils;
 import org.codehaus.jackson.JsonGenerationException;
 import org.junit.After;
 import org.junit.Before;
-import org.junit.Ignore;
 import org.junit.Test;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -1058,7 +1057,6 @@ public class TestHeartbeatHandler {
     assertTrue(registrationResponse.getStatusCommands().get(0).equals(statusCmd1));
   }
 
-  @Ignore
   @Test
   @SuppressWarnings("unchecked")
   public void testTaskInProgressHandling() throws AmbariException, InvalidStateTransitionException {
@@ -1097,6 +1095,7 @@ public class TestHeartbeatHandler {
     cr.setClusterName(DummyCluster);
     cr.setServiceName(HDFS);
     cr.setRole(DATANODE);
+    cr.setRoleCommand("INSTALL");
     cr.setStatus("IN_PROGRESS");
     cr.setStdErr("none");
     cr.setStdOut("dummy output");
@@ -1106,7 +1105,7 @@ public class TestHeartbeatHandler {
     hb.setComponentStatus(new ArrayList<ComponentStatus>());
 
     final HostRoleCommand command = new HostRoleCommand(DummyHostname1,
-            Role.DATANODE, null, null);
+            Role.DATANODE, null, RoleCommand.INSTALL);
 
     ActionManager am = getMockActionManager();
     expect(am.getTasks(anyObject(List.class))).andReturn(
@@ -1121,7 +1120,6 @@ public class TestHeartbeatHandler {
     assertEquals("Host state should still be installing", State.INSTALLING, componentState1);
   }
 
-  @Ignore
   @Test
   @SuppressWarnings("unchecked")
   public void testOPFailedEventForAbortedTask() throws AmbariException, InvalidStateTransitionException {
@@ -1173,6 +1171,7 @@ public class TestHeartbeatHandler {
     cr.setClusterName(DummyCluster);
     cr.setServiceName(HDFS);
     cr.setRole(DATANODE);
+    cr.setRoleCommand("INSTALL");
     cr.setStatus("FAILED");
     cr.setStdErr("none");
     cr.setStdOut("dummy output");
@@ -1330,7 +1329,6 @@ public class TestHeartbeatHandler {
         State.INSTALLED, serviceComponentHost1.getState());
   }
 
-  @Ignore
   @Test
   @SuppressWarnings("unchecked")
   public void testUpgradeSpecificHandling() throws AmbariException, InvalidStateTransitionException {
@@ -1365,6 +1363,7 @@ public class TestHeartbeatHandler {
     cr.setClusterName(DummyCluster);
     cr.setServiceName(HDFS);
     cr.setRole(DATANODE);
+    cr.setRoleCommand("INSTALL");
     cr.setStatus(HostRoleStatus.IN_PROGRESS.toString());
     cr.setStdErr("none");
     cr.setStdOut("dummy output");
@@ -1595,7 +1594,6 @@ public class TestHeartbeatHandler {
             stack122, serviceComponentHost2.getStackVersion());
   }
 
-  @Ignore
   @Test
   @SuppressWarnings("unchecked")
   public void testComponentUpgradeInProgressReport() throws AmbariException, InvalidStateTransitionException {
@@ -1643,6 +1641,7 @@ public class TestHeartbeatHandler {
     cr1.setClusterName(DummyCluster);
     cr1.setServiceName(HDFS);
     cr1.setRole(DATANODE);
+    cr1.setRoleCommand("INSTALL");
     cr1.setStatus(HostRoleStatus.IN_PROGRESS.toString());
     cr1.setStdErr("none");
     cr1.setStdOut("dummy output");
@@ -1654,6 +1653,7 @@ public class TestHeartbeatHandler {
     cr2.setClusterName(DummyCluster);
     cr2.setServiceName(HDFS);
     cr2.setRole(NAMENODE);
+    cr2.setRoleCommand("INSTALL");
     cr2.setStatus(HostRoleStatus.IN_PROGRESS.toString());
     cr2.setStdErr("none");
     cr2.setStdOut("dummy output");
@@ -1686,7 +1686,6 @@ public class TestHeartbeatHandler {
   }
 
 
-  @Ignore
   @Test
   @SuppressWarnings("unchecked")
   public void testComponentUpgradeFailReport() throws AmbariException, InvalidStateTransitionException {
@@ -1765,6 +1764,7 @@ public class TestHeartbeatHandler {
     cr1.setClusterName(DummyCluster);
     cr1.setServiceName(HDFS);
     cr1.setRole(DATANODE);
+    cr1.setRoleCommand("INSTALL");
     cr1.setStatus(HostRoleStatus.FAILED.toString());
     cr1.setStdErr("none");
     cr1.setStdOut("dummy output");
@@ -1776,6 +1776,7 @@ public class TestHeartbeatHandler {
     cr2.setClusterName(DummyCluster);
     cr2.setServiceName(HDFS);
     cr2.setRole(NAMENODE);
+    cr2.setRoleCommand("INSTALL");
     cr2.setStatus(HostRoleStatus.FAILED.toString());
     cr2.setStdErr("none");
     cr2.setStdOut("dummy output");
@@ -2004,7 +2005,6 @@ public class TestHeartbeatHandler {
     assertEquals(HostHealthStatus.HealthStatus.ALERT.name(), hostObject.getStatus());
   }
 
-  @Ignore
   @Test
   @SuppressWarnings("unchecked")
   public void testIgnoreCustomActionReport() throws AmbariException, InvalidStateTransitionException {

ambari-server/src/test/java/org/apache/ambari/server/controller/internal/HostStackVersionResourceProviderTest.java

@@ -36,11 +36,14 @@ import org.apache.ambari.server.controller.spi.ResourceProvider;
 import org.apache.ambari.server.controller.utilities.PropertyHelper;
 import org.apache.ambari.server.orm.GuiceJpaInitializer;
 import org.apache.ambari.server.orm.InMemoryDefaultTestModule;
+import org.apache.ambari.server.orm.dao.HostVersionDAO;
 import org.apache.ambari.server.orm.dao.RepositoryVersionDAO;
+import org.apache.ambari.server.orm.entities.HostVersionEntity;
 import org.apache.ambari.server.orm.entities.RepositoryVersionEntity;
 import org.apache.ambari.server.state.Cluster;
 import org.apache.ambari.server.state.Clusters;
 import org.apache.ambari.server.state.Host;
+import org.apache.ambari.server.state.RepositoryVersionState;
 import org.apache.ambari.server.state.ServiceComponentHost;
 import org.apache.ambari.server.state.ServiceInfo;
 import org.apache.ambari.server.state.ServiceOsSpecific;
@@ -77,6 +80,7 @@ public class HostStackVersionResourceProviderTest {
   private Injector injector;
   private AmbariMetaInfo ambariMetaInfo;
   private RepositoryVersionDAO repositoryVersionDAOMock;
+  private HostVersionDAO hostVersionDAOMock;
 
   private String operatingSystemsJson = "[\n" +
           "   {\n" +
@@ -100,6 +104,7 @@ public class HostStackVersionResourceProviderTest {
   public void setup() throws Exception {
     // Create instances of mocks
     repositoryVersionDAOMock = createNiceMock(RepositoryVersionDAO.class);
+    hostVersionDAOMock = createNiceMock(HostVersionDAO.class);
     // Initialize injector
     InMemoryDefaultTestModule module = new InMemoryDefaultTestModule();
     injector = Guice.createInjector(Modules.override(module).with(new MockModule()));
@@ -147,6 +152,8 @@ public class HostStackVersionResourceProviderTest {
     ResourceProviderFactory resourceProviderFactory = createNiceMock(ResourceProviderFactory.class);
     ResourceProvider csvResourceProvider = createNiceMock(ClusterStackVersionResourceProvider.class);
 
+    HostVersionEntity hostVersionEntityMock = createNiceMock(HostVersionEntity.class);
+
     AbstractControllerResourceProvider.init(resourceProviderFactory);
 
     expect(managementController.getClusters()).andReturn(clusters).anyTimes();
@@ -170,11 +177,16 @@ public class HostStackVersionResourceProviderTest {
     expect(repositoryVersionDAOMock.findByStackAndVersion(anyObject(String.class),
             anyObject(String.class))).andReturn(repoVersion);
 
+
+    expect(hostVersionDAOMock.findByClusterStackVersionAndHost(anyObject(String.class),
+            anyObject(String.class), anyObject(String.class), anyObject(String.class))).andReturn(hostVersionEntityMock);
+    expect(hostVersionEntityMock.getState()).andReturn(RepositoryVersionState.INSTALL_FAILED).anyTimes();
+
     expect(actionManager.getRequestTasks(anyLong())).andReturn(Collections.<HostRoleCommand>emptyList()).anyTimes();
 
     // replay
     replay(managementController, response, clusters, resourceProviderFactory, csvResourceProvider,
-            cluster, repositoryVersionDAOMock, sch, actionManager);
+            cluster, repositoryVersionDAOMock, sch, actionManager, hostVersionEntityMock, hostVersionDAOMock);
 
     ResourceProvider provider = AbstractControllerResourceProvider.getResourceProvider(
         type,
@@ -211,6 +223,7 @@ public class HostStackVersionResourceProviderTest {
     @Override
     protected void configure() {
       bind(RepositoryVersionDAO.class).toInstance(repositoryVersionDAOMock);
+      bind(HostVersionDAO.class).toInstance(hostVersionDAOMock);
     }
   }
 }

ambari-server/src/test/python/custom_actions/TestCheckHost.py

@@ -231,54 +231,54 @@ class TestCheckHost(TestCase):
     structured_out_mock.assert_called_with({})
 
 
-  # @patch.object(Script, 'get_config')
-  # @patch.object(Script, 'get_tmp_dir')
-  # @patch('resource_management.libraries.script.Script.put_structured_out')
-  # @patch('ambari_agent.HostInfo.HostInfo.javaProcs')
-  # @patch('ambari_agent.HostInfo.HostInfo.checkLiveServices')
-  # @patch('ambari_agent.HostInfo.HostInfo.getUMask')
-  # @patch('ambari_agent.HostInfo.HostInfo.getTransparentHugePage')
-  # @patch('ambari_agent.HostInfo.HostInfo.checkIptables')
-  # @patch('ambari_agent.HostInfo.HostInfo.checkReverseLookup')
-  # @patch('time.time')
-  # def testLastAgentEnv(self, time_mock, checkReverseLookup_mock, checkIptables_mock, getTransparentHugePage_mock,
-  #                      getUMask_mock, checkLiveServices_mock, javaProcs_mock, put_structured_out_mock,
-  #                      get_tmp_dir_mock, get_config_mock):
-  #   jsonFilePath = os.path.join("../resources/custom_actions", "check_last_agent_env.json")
-  #   with open(jsonFilePath, "r") as jsonFile:
-  #     jsonPayload = json.load(jsonFile)
-  #
-  #   get_config_mock.return_value = ConfigDictionary(jsonPayload)
-  #   get_tmp_dir_mock.return_value = "/tmp"
-  #
-  #   checkHost = CheckHost()
-  #   checkHost.actionexecute(None)
-  #
-  #   # ensure the correct function was called
-  #   self.assertTrue(time_mock.called)
-  #   self.assertTrue(checkReverseLookup_mock.called)
-  #   self.assertTrue(checkIptables_mock.called)
-  #   self.assertTrue(getTransparentHugePage_mock.called)
-  #   self.assertTrue(getUMask_mock.called)
-  #   self.assertTrue(checkLiveServices_mock.called)
-  #   self.assertTrue(javaProcs_mock.called)
-  #   self.assertTrue(put_structured_out_mock.called)
-  #   # ensure the correct keys are in the result map
-  #   last_agent_env_check_result = put_structured_out_mock.call_args[0][0]
-  #   self.assertTrue('last_agent_env_check' in last_agent_env_check_result)
-  #   self.assertTrue('hostHealth' in last_agent_env_check_result['last_agent_env_check'])
-  #   self.assertTrue('iptablesIsRunning' in last_agent_env_check_result['last_agent_env_check'])
-  #   self.assertTrue('reverseLookup' in last_agent_env_check_result['last_agent_env_check'])
-  #   self.assertTrue('alternatives' in last_agent_env_check_result['last_agent_env_check'])
-  #   self.assertTrue('umask' in last_agent_env_check_result['last_agent_env_check'])
-  #   self.assertTrue('stackFoldersAndFiles' in last_agent_env_check_result['last_agent_env_check'])
-  #   self.assertTrue('existingRepos' in last_agent_env_check_result['last_agent_env_check'])
-  #   self.assertTrue('installedPackages' in last_agent_env_check_result['last_agent_env_check'])
-  #   self.assertTrue('existingUsers' in last_agent_env_check_result['last_agent_env_check'])
-  #
-  #   # try it now with errors
-  #   javaProcs_mock.side_effect = Exception("test exception")
-  #   checkHost.actionexecute(None)
-  #
-  #   #ensure the correct response is returned
-  #   put_structured_out_mock.assert_called_with({'last_agent_env_check': {'message': 'test exception', 'exit_code': 1}})
+  @patch.object(Script, 'get_config')
+  @patch.object(Script, 'get_tmp_dir')
+  @patch('resource_management.libraries.script.Script.put_structured_out')
+  @patch('ambari_agent.HostInfo.HostInfo.javaProcs')
+  @patch('ambari_agent.HostInfo.HostInfo.checkLiveServices')
+  @patch('ambari_agent.HostInfo.HostInfo.getUMask')
+  @patch('ambari_agent.HostInfo.HostInfo.getTransparentHugePage')
+  @patch('ambari_agent.HostInfo.HostInfo.checkIptables')
+  @patch('ambari_agent.HostInfo.HostInfo.checkReverseLookup')
+  @patch('time.time')
+  def testLastAgentEnv(self, time_mock, checkReverseLookup_mock, checkIptables_mock, getTransparentHugePage_mock,
+                       getUMask_mock, checkLiveServices_mock, javaProcs_mock, put_structured_out_mock,
+                       get_tmp_dir_mock, get_config_mock):
+    jsonFilePath = os.path.join("../resources/custom_actions", "check_last_agent_env.json")
+    with open(jsonFilePath, "r") as jsonFile:
+      jsonPayload = json.load(jsonFile)
+
+    get_config_mock.return_value = ConfigDictionary(jsonPayload)
+    get_tmp_dir_mock.return_value = "/tmp"
+
+    checkHost = CheckHost()
+    checkHost.actionexecute(None)
+
+    # ensure the correct function was called
+    self.assertTrue(time_mock.called)
+    self.assertTrue(checkReverseLookup_mock.called)
+    self.assertTrue(checkIptables_mock.called)
+    self.assertTrue(getTransparentHugePage_mock.called)
+    self.assertTrue(getUMask_mock.called)
+    self.assertTrue(checkLiveServices_mock.called)
+    self.assertTrue(javaProcs_mock.called)
+    self.assertTrue(put_structured_out_mock.called)
+    # ensure the correct keys are in the result map
+    last_agent_env_check_result = put_structured_out_mock.call_args[0][0]
+    self.assertTrue('last_agent_env_check' in last_agent_env_check_result)
+    self.assertTrue('hostHealth' in last_agent_env_check_result['last_agent_env_check'])
+    self.assertTrue('iptablesIsRunning' in last_agent_env_check_result['last_agent_env_check'])
+    self.assertTrue('reverseLookup' in last_agent_env_check_result['last_agent_env_check'])
+    self.assertTrue('alternatives' in last_agent_env_check_result['last_agent_env_check'])
+    self.assertTrue('umask' in last_agent_env_check_result['last_agent_env_check'])
+    self.assertTrue('stackFoldersAndFiles' in last_agent_env_check_result['last_agent_env_check'])
+    self.assertTrue('existingRepos' in last_agent_env_check_result['last_agent_env_check'])
+    self.assertTrue('installedPackages' in last_agent_env_check_result['last_agent_env_check'])
+    self.assertTrue('existingUsers' in last_agent_env_check_result['last_agent_env_check'])
+
+    # try it now with errors
+    javaProcs_mock.side_effect = Exception("test exception")
+    checkHost.actionexecute(None)
+
+    #ensure the correct response is returned
+    put_structured_out_mock.assert_called_with({'last_agent_env_check': {'message': 'test exception', 'exit_code': 1}})