[AMBARI-23982] - 'Start All' services call fails post EU as the state of Timeline Reader is INIT (#1415)

5 files changed, 227 insertions, 11 deletions

diff --git a/ambari-server/src/main/java/org/apache/ambari/server/serveraction/AbstractServerAction.java b/ambari-server/src/main/java/org/apache/ambari/server/serveraction/AbstractServerAction.java
index 7d5a847867..3c38398d8c 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/serveraction/AbstractServerAction.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/serveraction/AbstractServerAction.java
@@ -199,5 +199,4 @@ public abstract class AbstractServerAction implements ServerAction {
   protected void auditLog(AuditEvent ae) {
     auditLogger.log(ae);
   }
-
 }
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/AbstractUpgradeServerAction.java b/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/AbstractUpgradeServerAction.java
index 1cffd41c20..abbf6b3175 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/AbstractUpgradeServerAction.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/AbstractUpgradeServerAction.java
@@ -27,6 +27,7 @@ import org.apache.ambari.server.state.UpgradeContext;
 import org.apache.ambari.server.state.UpgradeContextFactory;
 import org.apache.ambari.server.state.UpgradeHelper;
 
+import com.google.gson.Gson;
 import com.google.inject.Inject;
 
 /**
@@ -64,6 +65,15 @@ public abstract class AbstractUpgradeServerAction extends AbstractServerAction {
   protected AgentConfigsHolder agentConfigsHolder;
 
   /**
+   * Gets the injected instance of the {@link Gson} serializer/deserializer.
+   *
+   * @return the injected {@link Gson} instance.
+   */
+  protected Gson getGson() {
+    return gson;
+  }
+
+  /**
    * Gets an initialized {@link UpgradeContext} for the in-progress upgrade.
    */
   protected UpgradeContext getUpgradeContext(Cluster cluster) {
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/AddComponentAction.java b/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/AddComponentAction.java
index 9a339d24c7..c97c9ed1fb 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/AddComponentAction.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/AddComponentAction.java
@@ -27,6 +27,7 @@ import java.util.stream.Collectors;
 
 import org.apache.ambari.server.AmbariException;
 import org.apache.ambari.server.ServiceComponentNotFoundException;
+import org.apache.ambari.server.ServiceNotFoundException;
 import org.apache.ambari.server.actionmanager.HostRoleStatus;
 import org.apache.ambari.server.agent.CommandReport;
 import org.apache.ambari.server.events.listeners.upgrade.StackVersionListener;
@@ -40,6 +41,8 @@ import org.apache.ambari.server.state.State;
 import org.apache.ambari.server.state.UpgradeContext;
 import org.apache.ambari.server.state.stack.upgrade.AddComponentTask;
 
+import com.google.gson.Gson;
+
 /**
  * The {@link AddComponentAction} is used to add a component during an upgrade.
  */
@@ -72,8 +75,20 @@ public class AddComponentAction extends AbstractUpgradeServerAction {
     String serializedJson = commandParameters.get(
         AddComponentTask.PARAMETER_SERIALIZED_ADD_COMPONENT_TASK);
 
+    Gson gson = getGson();
     AddComponentTask task = gson.fromJson(serializedJson, AddComponentTask.class);
 
+    final Service service;
+    try {
+      service = cluster.getService(task.service);
+    } catch (ServiceNotFoundException snfe) {
+      return createCommandReport(0, HostRoleStatus.COMPLETED, "{}", "",
+          String.format(
+              "%s was not installed in this cluster since %s is not an installed service.",
+              task.component, task.service));
+    }
+
+
     // build the list of candidate hosts
     Collection<Host> candidates = MasterHostResolver.getCandidateHosts(cluster, task.hosts,
         task.hostService, task.hostComponent);
@@ -84,19 +99,13 @@ public class AddComponentAction extends AbstractUpgradeServerAction {
           task.hostService, task.hostComponent));
     }
 
-    Service service = cluster.getService(task.service);
-    if (null == service) {
-      return createCommandReport(0, HostRoleStatus.FAILED, "{}", "",
-          String.format("Unable to add %s since %s is not installed in this cluster.",
-              task.component, task.service));
-    }
-
     // create the component if it doesn't exist in the service yet
     ServiceComponent serviceComponent;
     try {
-       serviceComponent = service.getServiceComponent(task.component);
-    } catch( ServiceComponentNotFoundException scnfe ) {
+      serviceComponent = service.getServiceComponent(task.component);
+    } catch (ServiceComponentNotFoundException scnfe) {
       serviceComponent = service.addServiceComponent(task.component);
+      serviceComponent.setDesiredState(State.INSTALLED);
     }
 
     StringBuilder buffer = new StringBuilder(String.format(
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeHelper.java b/ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeHelper.java
index f3438ac639..a48d98c638 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeHelper.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeHelper.java
@@ -865,7 +865,7 @@ public class UpgradeHelper {
    * participating in the upgrade or downgrade. The following actions are
    * performed in order:
    * <ul>
-   * <li>The desired repository for every service and component is changed<
+   * <li>The desired repository for every service and component is changed
    * <li>The {@link UpgradeState} of every component host is moved to either
    * {@link UpgradeState#IN_PROGRESS} or {@link UpgradeState#NONE}.
    * <li>In the case of an upgrade, new configurations and service
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/AddComponentActionTest.java b/ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/AddComponentActionTest.java
new file mode 100644
index 0000000000..134791ead6
--- /dev/null
+++ b/ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/AddComponentActionTest.java
@@ -0,0 +1,198 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.ambari.server.serveraction.upgrades;
+
+import static org.easymock.EasyMock.expect;
+import static org.easymock.EasyMock.expectLastCall;
+
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.Map;
+
+import org.apache.ambari.server.ServiceComponentNotFoundException;
+import org.apache.ambari.server.ServiceNotFoundException;
+import org.apache.ambari.server.agent.ExecutionCommand;
+import org.apache.ambari.server.alerts.AmbariPerformanceRunnable;
+import org.apache.ambari.server.events.listeners.upgrade.StackVersionListener;
+import org.apache.ambari.server.stack.MasterHostResolver;
+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.Service;
+import org.apache.ambari.server.state.ServiceComponent;
+import org.apache.ambari.server.state.ServiceComponentHost;
+import org.apache.ambari.server.state.State;
+import org.apache.ambari.server.state.UpgradeContext;
+import org.apache.ambari.server.state.stack.upgrade.AddComponentTask;
+import org.apache.ambari.server.state.stack.upgrade.ExecuteHostType;
+import org.easymock.EasyMockSupport;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.powermock.api.easymock.PowerMock;
+import org.powermock.core.classloader.annotations.PrepareForTest;
+import org.powermock.modules.junit4.PowerMockRunner;
+
+import com.google.common.collect.Lists;
+import com.google.gson.Gson;
+
+/**
+ * Tests {@link AddComponentAction}.
+ */
+/**
+ * Tests {@link AmbariPerformanceRunnable}.
+ */
+@RunWith(PowerMockRunner.class)
+@PrepareForTest({ AddComponentAction.class, MasterHostResolver.class })
+public class AddComponentActionTest extends EasyMockSupport {
+
+  private static final String CANDIDATE_SERVICE = "FOO-SERVICE";
+  private static final String CANDIDATE_COMPONENT = "FOO-COMPONENT";
+  private static final String NEW_SERVICE = CANDIDATE_SERVICE;
+  private static final String NEW_COMPONENT = "FOO-NEW-COMPONENT";
+  private static final String CLUSTER_NAME = "c1";
+
+  private final Map<String, String> m_commandParams = new HashMap<>();
+
+  private final Clusters m_mockClusters = createNiceMock(Clusters.class);
+  private final Cluster m_mockCluster = createNiceMock(Cluster.class);
+  private final Service m_mockCandidateService = createNiceMock(Service.class);
+  private final ServiceComponent m_mockCandidateServiceComponent = createNiceMock(ServiceComponent.class);
+
+  private final UpgradeContext m_mockUpgradeContext = createNiceMock(UpgradeContext.class);
+
+  private final String CANDIDATE_HOST_NAME = "c6401.ambari.apache.org";
+  private final Host m_mockHost = createStrictMock(Host.class);
+  private final Collection<Host> m_candidateHosts = Lists.newArrayList(m_mockHost);
+
+  private AddComponentAction m_action;
+
+  @Before
+  public void before() throws Exception {
+    PowerMock.mockStatic(MasterHostResolver.class);
+    expect(MasterHostResolver.getCandidateHosts(m_mockCluster, ExecuteHostType.ALL,
+        CANDIDATE_SERVICE, CANDIDATE_COMPONENT)).andReturn(m_candidateHosts).once();
+    PowerMock.replay(MasterHostResolver.class);
+
+    m_action = PowerMock.createNicePartialMock(AddComponentAction.class, "getUpgradeContext",
+        "createCommandReport", "getClusters", "getGson");
+
+    ExecutionCommand executionCommand = createNiceMock(ExecutionCommand.class);
+    expect(executionCommand.getCommandParams()).andReturn(m_commandParams).once();
+    m_action.setExecutionCommand(executionCommand);
+
+    expect(m_action.getClusters()).andReturn(m_mockClusters).atLeastOnce();
+    expect(m_action.getUpgradeContext(m_mockCluster)).andReturn(m_mockUpgradeContext).once();
+    expect(m_action.getGson()).andReturn(new Gson()).once();
+
+    AddComponentTask addComponentTask = new AddComponentTask();
+    addComponentTask.service = NEW_SERVICE;
+    addComponentTask.component = NEW_COMPONENT;
+    addComponentTask.hostService = CANDIDATE_SERVICE;
+    addComponentTask.hostComponent = CANDIDATE_COMPONENT;
+    addComponentTask.hosts = ExecuteHostType.ALL;
+
+    String addComponentTaskJson = addComponentTask.toJson();
+    m_commandParams.put("clusterName", CLUSTER_NAME);
+    m_commandParams.put(AddComponentTask.PARAMETER_SERIALIZED_ADD_COMPONENT_TASK,
+        addComponentTaskJson);
+
+    expect(m_mockClusters.getCluster(CLUSTER_NAME)).andReturn(m_mockCluster).once();
+  }
+
+  @After
+  public void after() throws Exception {
+    PowerMock.verify(m_action);
+  }
+
+  /**
+   * Tests that adding a component during upgrade invokes the correct methods.
+   *
+   * @throws Exception
+   */
+  @Test
+  public void testAddComponentDuringUpgrade() throws Exception {
+    expect(m_mockCluster.getService(NEW_SERVICE)).andReturn(m_mockCandidateService).once();
+    expect(m_mockCandidateService.getServiceComponent(NEW_COMPONENT)).andThrow(new ServiceComponentNotFoundException(CLUSTER_NAME, NEW_SERVICE, NEW_COMPONENT));
+    expect(m_mockCandidateService.addServiceComponent(NEW_COMPONENT)).andReturn(m_mockCandidateServiceComponent).once();
+
+    expect(m_mockHost.getHostName()).andReturn(CANDIDATE_HOST_NAME).atLeastOnce();
+
+    m_mockCandidateServiceComponent.setDesiredState(State.INSTALLED);
+    expectLastCall().once();
+
+    Map<String, ServiceComponentHost> existingSCHs = new HashMap<>();
+    expect(m_mockCandidateServiceComponent.getServiceComponentHosts()).andReturn(existingSCHs).once();
+
+    ServiceComponentHost mockServiceComponentHost = createNiceMock(ServiceComponentHost.class);
+    expect(m_mockCandidateServiceComponent.addServiceComponentHost(CANDIDATE_HOST_NAME)).andReturn(mockServiceComponentHost).once();
+    mockServiceComponentHost.setState(State.INSTALLED);
+    expectLastCall().once();
+
+    mockServiceComponentHost.setDesiredState(State.INSTALLED);
+    expectLastCall().once();
+
+    mockServiceComponentHost.setVersion(StackVersionListener.UNKNOWN_VERSION);
+    expectLastCall().once();
+
+    PowerMock.replay(m_action);
+    replayAll();
+
+    m_action.execute(null);
+
+    verifyAll();
+  }
+
+  /**
+   * Tests that we fail without any candidates.
+   *
+   * @throws Exception
+   */
+  @Test
+  public void testAddComponentDuringUpgradeFailsWithNoCandidates() throws Exception {
+    PowerMock.replay(m_action);
+    replayAll();
+
+    m_candidateHosts.clear();
+
+    m_action.execute(null);
+
+    verifyAll();
+  }
+
+  /**
+   * Tests that we fail when the candidateg service isn't installed in the
+   * cluster.
+   *
+   * @throws Exception
+   */
+  @Test
+  public void testAddComponentWhereServiceIsNotInstalled() throws Exception {
+    expect(m_mockCluster.getService(NEW_SERVICE)).andThrow(
+        new ServiceNotFoundException(CLUSTER_NAME, CANDIDATE_SERVICE)).once();
+
+    PowerMock.replay(m_action);
+    replayAll();
+
+    m_action.execute(null);
+
+    verifyAll();
+  }
+
+}