You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ambari.apache.org by ad...@apache.org on 2018/05/30 11:36:27 UTC

[ambari] branch branch-feature-AMBARI-14714 updated (4bef993 -> f8293cf)

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

adoroszlai pushed a change to branch branch-feature-AMBARI-14714
in repository https://gitbox.apache.org/repos/asf/ambari.git.


    from 4bef993  AMBARI-23972. Update code for : (1). 'mpack-recommendations' directory creation and (2). putting 'mpack_advisor_wrapper.py' in '/var/lib/ambari-server/resources/scripts/' folder.
     new 3490029  AMBARI-14714. Fix some unit tests
     new 938be76  AMBARI-14714. Fix ServiceResourceProviderTest
     new 0e9317e  AMBARI-23746. Cannot create same named service in different service groups in same request
     new 096dff5  AMBARI-23746. Cannot create same named component in different service groups in same request
     new 85c6c6e  AMBARI-22875. Service group name mismatch
     new 9120330  AMBARI-23746. Cannot query services with same name
     new f8293cf  AMBARI-23746. Use List for componentID duplicate check

The 7 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../actionmanager/ExecutionCommandWrapper.java     |  10 +-
 .../apache/ambari/server/agent/CommandReport.java  |   5 +
 .../api/query/render/ClusterBlueprintRenderer.java |   8 +-
 .../ambari/server/api/services/AmbariMetaInfo.java |   5 +-
 .../controller/AmbariManagementControllerImpl.java |   8 +-
 .../internal/ComponentResourceProvider.java        |  23 +-
 .../internal/ExportBlueprintRequest.java           |   3 +-
 .../ServiceDependencyResourceProvider.java         |  17 +-
 .../internal/ServiceResourceProvider.java          |  53 ++--
 .../org/apache/ambari/server/orm/dao/MpackDAO.java |   2 +-
 .../orm/entities/HostGroupComponentEntity.java     |   2 +-
 .../server/orm/entities/MpackInstanceEntity.java   |  21 +-
 .../apache/ambari/server/state/ConfigHelper.java   |   7 +-
 .../ambari/server/state/cluster/ClusterImpl.java   |  45 ++--
 .../stack/upgrade/RepositoryVersionHelper.java     |   2 +-
 .../BlueprintBasedClusterProvisionRequest.java     |  11 -
 .../ambari/server/topology/BlueprintFactory.java   |  36 +--
 .../ambari/server/topology/BlueprintImpl.java      |  16 +-
 .../apache/ambari/server/topology/Component.java   |  47 +---
 .../ambari/server/topology/HostGroupImpl.java      |   2 +-
 .../ambari/server/topology/MpackInstance.java      |  48 +---
 .../ambari/server/topology/ResolvedComponent.java  |   9 +-
 .../server/topology/ResolvedComponent_Builder.java |   2 +-
 .../server/topology/StackComponentResolver.java    |  56 ++---
 .../src/main/resources/Ambari-DDL-Derby-CREATE.sql |   1 +
 .../src/main/resources/Ambari-DDL-MySQL-CREATE.sql |   1 +
 .../main/resources/Ambari-DDL-Oracle-CREATE.sql    |   1 +
 .../main/resources/Ambari-DDL-Postgres-CREATE.sql  |   1 +
 .../resources/Ambari-DDL-SQLAnywhere-CREATE.sql    |   1 +
 .../main/resources/Ambari-DDL-SQLServer-CREATE.sql |   1 +
 .../actionmanager/ExecutionCommandWrapperTest.java |   8 +
 .../actionmanager/TestActionDBAccessorImpl.java    |   3 +
 .../server/actionmanager/TestActionScheduler.java  |  21 +-
 .../TestActionSchedulerThreading.java              |   4 +-
 .../ambari/server/agent/AgentResourceTest.java     |   2 +
 .../server/agent/HeartbeatProcessorTest.java       |   1 +
 .../ambari/server/agent/TestHeartbeatHandler.java  |   5 +
 .../ambari/server/agent/TestHeartbeatMonitor.java  |   1 +
 .../query/render/ClusterBlueprintRendererTest.java |   8 +-
 .../api/resources/BaseResourceDefinitionTest.java  |   7 +-
 .../api/services/HostComponentServiceTest.java     |   6 -
 .../AmbariCustomCommandExecutionHelperTest.java    |  27 +-
 .../AbstractControllerResourceProviderTest.java    |   7 +-
 .../internal/AbstractResourceProviderTest.java     |   7 +-
 .../internal/ComponentResourceProviderTest.java    |  13 +-
 .../HostComponentResourceProviderTest.java         |  10 +-
 .../internal/HostResourceProviderTest.java         |  17 +-
 .../controller/internal/JMXHostProviderTest.java   |  31 +--
 .../ServiceDependencyResourceProviderTest.java     |   2 +-
 .../internal/ServiceResourceProviderTest.java      | 178 ++++++++++---
 .../apache/ambari/server/orm/OrmTestHelper.java    |   2 +
 .../ambari/server/topology/AmbariContextTest.java  |   6 +-
 .../server/topology/BlueprintFactoryTest.java      |  42 ----
 .../server/topology/DownloadMpacksTaskTest.java    |   6 +-
 .../topology/StackComponentResolverTest.java       | 275 ++++++++++++++++++---
 .../server/topology/TopologyManagerTest.java       |   4 +-
 .../RejectUnknownComponentsTest.java}              |  10 +-
 .../HDP/2.0.6/services/HADOOP_CLIENTS/metainfo.xml |   5 +
 58 files changed, 687 insertions(+), 465 deletions(-)
 copy ambari-server/src/test/java/org/apache/ambari/server/topology/{StackComponentResolverTest.java => validators/RejectUnknownComponentsTest.java} (89%)

-- 
To stop receiving notification emails like this one, please contact
adoroszlai@apache.org.

[ambari] 05/07: AMBARI-22875. Service group name mismatch

Posted by ad...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

adoroszlai pushed a commit to branch branch-feature-AMBARI-14714
in repository https://gitbox.apache.org/repos/asf/ambari.git

commit 85c6c6e6cca7c052d51f2b2d50a4815720c6018f
Author: Doroszlai, Attila <ad...@apache.org>
AuthorDate: Wed May 30 00:28:58 2018 +0200

    AMBARI-22875. Service group name mismatch
---
 .../internal/ExportBlueprintRequest.java           |   3 +-
 .../orm/entities/HostGroupComponentEntity.java     |   2 +-
 .../server/orm/entities/MpackInstanceEntity.java   |  21 +-
 .../BlueprintBasedClusterProvisionRequest.java     |  11 -
 .../ambari/server/topology/BlueprintFactory.java   |  36 +--
 .../ambari/server/topology/BlueprintImpl.java      |  16 +-
 .../apache/ambari/server/topology/Component.java   |  47 +---
 .../ambari/server/topology/HostGroupImpl.java      |   2 +-
 .../ambari/server/topology/MpackInstance.java      |  48 +---
 .../ambari/server/topology/ResolvedComponent.java  |   9 +-
 .../server/topology/ResolvedComponent_Builder.java |   2 +-
 .../server/topology/StackComponentResolver.java    |  56 ++---
 .../src/main/resources/Ambari-DDL-Derby-CREATE.sql |   1 +
 .../src/main/resources/Ambari-DDL-MySQL-CREATE.sql |   1 +
 .../main/resources/Ambari-DDL-Oracle-CREATE.sql    |   1 +
 .../main/resources/Ambari-DDL-Postgres-CREATE.sql  |   1 +
 .../resources/Ambari-DDL-SQLAnywhere-CREATE.sql    |   1 +
 .../main/resources/Ambari-DDL-SQLServer-CREATE.sql |   1 +
 .../ambari/server/topology/AmbariContextTest.java  |   6 +-
 .../server/topology/BlueprintFactoryTest.java      |  42 ----
 .../server/topology/DownloadMpacksTaskTest.java    |   6 +-
 .../topology/StackComponentResolverTest.java       | 275 ++++++++++++++++++---
 .../server/topology/TopologyManagerTest.java       |   4 +-
 .../RejectUnknownComponentsTest.java}              |  10 +-
 24 files changed, 345 insertions(+), 257 deletions(-)

diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ExportBlueprintRequest.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ExportBlueprintRequest.java
index dc3f2d5..6e6fd69 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ExportBlueprintRequest.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ExportBlueprintRequest.java
@@ -272,8 +272,7 @@ public class ExportBlueprintRequest implements TopologyRequest {
           String.valueOf(resource.getPropertyValue(HostComponentResourceProvider.HOST_COMPONENT_SERVICE_NAME_PROPERTY_ID));
         String serviceGroupName =
           String.valueOf(resource.getPropertyValue(HostComponentResourceProvider.HOST_COMPONENT_SERVICE_GROUP_NAME_PROPERTY_ID));
-        StackId stackId = serviceGroupToMpack.get(serviceGroupName);
-        getComponents().add(new Component(componentName, stackId, serviceName, null));
+        getComponents().add(new Component(componentName, serviceGroupName, serviceName, null));
       }
       addAmbariComponentIfLocalhost((String) host.getObject().getPropertyValue(
           PropertyHelper.getPropertyId("Hosts", "host_name")));
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/HostGroupComponentEntity.java b/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/HostGroupComponentEntity.java
index f16ef40..708fb3f 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/HostGroupComponentEntity.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/HostGroupComponentEntity.java
@@ -211,7 +211,7 @@ public class HostGroupComponentEntity {
 
   /**
    * @return the name of the service instance defining this component
-   *         (only needs to be set if component resolution would be ambigous otherwise)
+   *         (only needs to be set if component resolution would be ambiguous otherwise)
    */
   public String getServiceName() {
     return serviceName;
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/MpackInstanceEntity.java b/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/MpackInstanceEntity.java
index 1956108..9063ed0 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/MpackInstanceEntity.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/MpackInstanceEntity.java
@@ -59,6 +59,9 @@ public abstract class MpackInstanceEntity {
   @Column(name = "mpack_name", nullable = false)
   private String mpackName;
 
+  @Column(name = "mpack_type", nullable = false)
+  private String mpackType;
+
   @Column(name = "mpack_version", nullable = false)
   private String mpackVersion;
 
@@ -90,20 +93,34 @@ public abstract class MpackInstanceEntity {
   }
 
   /**
-   * @return the name of the mpack
+   * @return the name of the mpack instance
    */
   public String getMpackName() {
     return mpackName;
   }
 
   /**
-   * @param mpackName the name of the mpack
+   * @param mpackName the name of the mpack instance
    */
   public void setMpackName(String mpackName) {
     this.mpackName = mpackName;
   }
 
   /**
+   * @return the name (type) of the mpack
+   */
+  public String getMpackType() {
+    return mpackType;
+  }
+
+  /**
+   * @param mpackType the name (type) of the mpack
+   */
+  public void setMpackType(String mpackType) {
+    this.mpackType = mpackType;
+  }
+
+  /**
    * @return the version of the mpack
    */
   public String getMpackVersion() {
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/topology/BlueprintBasedClusterProvisionRequest.java b/ambari-server/src/main/java/org/apache/ambari/server/topology/BlueprintBasedClusterProvisionRequest.java
index 22d4bab..58ce5d7 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/topology/BlueprintBasedClusterProvisionRequest.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/topology/BlueprintBasedClusterProvisionRequest.java
@@ -20,7 +20,6 @@ package org.apache.ambari.server.topology;
 import static java.util.stream.Collectors.toMap;
 
 import java.util.Collection;
-import java.util.HashMap;
 import java.util.Map;
 import java.util.Set;
 import java.util.function.Function;
@@ -162,16 +161,6 @@ public class BlueprintBasedClusterProvisionRequest implements Blueprint, Provisi
     return stack;
   }
 
-  public Map<String, Map<String, ServiceInstance>> getServicesByMpack() {
-    Map<String, Map<String, ServiceInstance>> result = new HashMap<>();
-    for (MpackInstance mpack : mpacks) {
-      Map<String, ServiceInstance> services = mpack.getServiceInstances().stream()
-        .collect(toMap(ServiceInstance::getName, Function.identity()));
-      result.put(mpack.getMpackName(), services);
-    }
-    return result;
-  }
-
   /**
    * @return service instances defined in the topology, mapped by service name,
    * whose name is unique across all mpacks.
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/topology/BlueprintFactory.java b/ambari-server/src/main/java/org/apache/ambari/server/topology/BlueprintFactory.java
index cc29551..0f13539 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/topology/BlueprintFactory.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/topology/BlueprintFactory.java
@@ -18,8 +18,6 @@
 
 package org.apache.ambari.server.topology;
 
-import static java.util.stream.Collectors.joining;
-import static java.util.stream.Collectors.toCollection;
 import static java.util.stream.Collectors.toSet;
 import static org.apache.ambari.server.controller.internal.BlueprintResourceProvider.BLUEPRINT_NAME_PROPERTY_ID;
 import static org.apache.ambari.server.controller.internal.BlueprintResourceProvider.COMPONENT_MPACK_INSTANCE_PROPERTY;
@@ -44,9 +42,6 @@ import java.util.Collections;
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
-import java.util.TreeSet;
-import java.util.function.Function;
-import java.util.stream.Stream;
 
 import javax.inject.Inject;
 import javax.inject.Provider;
@@ -56,14 +51,12 @@ import org.apache.ambari.server.orm.dao.BlueprintDAO;
 import org.apache.ambari.server.orm.entities.BlueprintEntity;
 import org.apache.ambari.server.stack.NoSuchStackException;
 import org.apache.ambari.server.state.StackId;
-import org.apache.commons.lang3.tuple.Pair;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import com.fasterxml.jackson.core.type.TypeReference;
 import com.fasterxml.jackson.databind.DeserializationFeature;
 import com.fasterxml.jackson.databind.ObjectMapper;
-import com.google.common.base.Joiner;
 
 /**
  * Create a Blueprint instance.
@@ -84,7 +77,7 @@ public class BlueprintFactory {
     BlueprintEntity entity = blueprintDAO.get().findByName(blueprintName);
     if (entity != null) {
       Set<StackId> stackIds = entity.getMpackInstances().stream()
-        .map(m -> new StackId(m.getMpackName(), m.getMpackVersion()))
+        .map(m -> new StackId(m.getMpackType(), m.getMpackVersion()))
         .collect(toSet());
       return new BlueprintImpl(entity, stackIds);
     }
@@ -113,7 +106,7 @@ public class BlueprintFactory {
       if (stackId.isPresent()) {
         String stackName = stackId.get().getStackName();
         String stackVersion = stackId.get().getStackVersion();
-        mpackInstances = Collections.singleton(new MpackInstance(stackName, stackVersion, null, null, Configuration.createEmpty()));
+        mpackInstances = Collections.singleton(new MpackInstance(stackName, stackName, stackVersion, null, Configuration.createEmpty()));
       }
     }
     Set<StackId> stackIds = mpackInstances.stream()
@@ -151,29 +144,6 @@ public class BlueprintFactory {
       : Optional.empty();
   }
 
-  /**
-   * Verify that each item in <code>items</code> is defined by only one stack.
-   *
-   * @param items the items to check
-   * @param type string description of the type of items (eg. "Service", or "Component")
-   * @param lookup a function to find the set of stacks that an item belongs to
-   * @throws IllegalArgumentException if some items are defined in multiple stacks
-   */
-  static void verifyStackDefinitionsAreDisjoint(Stream<String> items, String type, Function<String, Set<StackId>> lookup) {
-    Set<Pair<String, Set<StackId>>> definedInMultipleStacks = items
-      .map(s -> Pair.of(s, lookup.apply(s)))
-      .filter(p -> p.getRight().size() > 1)
-      .collect(toCollection(TreeSet::new));
-
-    if (!definedInMultipleStacks.isEmpty()) {
-      String msg = definedInMultipleStacks.stream()
-        .map(p -> String.format("%s %s is defined in multiple stacks: %s", type, p.getLeft(), Joiner.on(", ").join(p.getRight())))
-        .collect(joining("\n"));
-      LOG.error(msg);
-      throw new IllegalArgumentException(msg);
-    }
-  }
-
   //todo: Move logic to HostGroupImpl
   @SuppressWarnings("unchecked")
   private Collection<HostGroup> processHostGroups(Map<String, Object> properties) {
@@ -227,7 +197,7 @@ public class BlueprintFactory {
       //TODO, might want to add some validation here, to only accept value enum types, rwn
       ProvisionAction provisionAction = componentProperties.containsKey(COMPONENT_PROVISION_ACTION_PROPERTY_ID) ?
         ProvisionAction.valueOf(componentProperties.get(COMPONENT_PROVISION_ACTION_PROPERTY_ID)) : null;
-      components.add(new Component(componentName, new StackId(mpackInstance), serviceInstance, provisionAction));
+      components.add(new Component(componentName, mpackInstance, serviceInstance, provisionAction));
     }
 
     return components;
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/topology/BlueprintImpl.java b/ambari-server/src/main/java/org/apache/ambari/server/topology/BlueprintImpl.java
index 633bcb0..7e60cb9 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/topology/BlueprintImpl.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/topology/BlueprintImpl.java
@@ -29,6 +29,8 @@ import java.util.Map;
 import java.util.Set;
 import java.util.function.Supplier;
 
+import javax.annotation.Nonnull;
+
 import org.apache.ambari.server.orm.entities.BlueprintConfigEntity;
 import org.apache.ambari.server.orm.entities.BlueprintConfiguration;
 import org.apache.ambari.server.orm.entities.BlueprintEntity;
@@ -103,6 +105,7 @@ public class BlueprintImpl implements Blueprint {
     return stackIds;
   }
 
+  @Nonnull
   @Override
   public SecurityConfiguration getSecurity() {
     return security;
@@ -185,12 +188,8 @@ public class BlueprintImpl implements Blueprint {
 
   private Collection<MpackInstance> parseMpacks(BlueprintEntity blueprintEntity) throws NoSuchStackException {
     Collection<MpackInstance> mpackInstances = new ArrayList<>();
-    for (BlueprintMpackInstanceEntity mpack: blueprintEntity.getMpackInstances()) {
-      MpackInstance mpackInstance = new MpackInstance();
-      mpackInstance.setMpackName(mpack.getMpackName());
-      mpackInstance.setMpackVersion(mpack.getMpackVersion());
-      mpackInstance.setUrl(mpack.getMpackUri());
-      mpackInstance.setConfiguration(processConfiguration(mpack.getConfigurations()));
+    for (BlueprintMpackInstanceEntity mpack : blueprintEntity.getMpackInstances()) {
+      MpackInstance mpackInstance = new MpackInstance(mpack.getMpackName(), mpack.getMpackType(), mpack.getMpackVersion(), mpack.getMpackUri(), BlueprintImpl.fromConfigEntities(mpack.getConfigurations()));
       // TODO: come up with proper mpack -> stack resolution
       for(MpackInstanceServiceEntity serviceEntity: mpack.getServiceInstances()) {
         ServiceInstance serviceInstance = new ServiceInstance(
@@ -329,10 +328,7 @@ public class BlueprintImpl implements Blueprint {
       componentEntity.setHostGroupEntity(group);
       componentEntity.setHostGroupName(group.getName());
       componentEntity.setServiceName(component.getServiceInstance());
-      if (null != component.getStackId()) {
-        componentEntity.setMpackName(component.getStackId().getStackName());
-        componentEntity.setMpackVersion(component.getStackId().getStackVersion());
-      }
+      componentEntity.setMpackName(component.getMpackInstance());
 
       // add provision action (if specified) to entity type
       // otherwise, just leave this column null (provision_action)
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/topology/Component.java b/ambari-server/src/main/java/org/apache/ambari/server/topology/Component.java
index eac584f..99ab038 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/topology/Component.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/topology/Component.java
@@ -24,41 +24,25 @@ import java.util.Objects;
 import javax.annotation.Nullable;
 
 import org.apache.ambari.server.controller.internal.ProvisionAction;
-import org.apache.ambari.server.state.StackId;
+
+import com.google.common.base.MoreObjects;
 
 public class Component {
 
   private final String name;
-
-  @Nullable
   private final String mpackInstance;
-
-  @Nullable
-  private final StackId stackId;
-
-  @Nullable
   private final String serviceInstance;
   private final ProvisionAction provisionAction;
 
-  @Deprecated
   public Component(String name) {
     this(name, null, null, null);
   }
 
-  public Component(String name, @Nullable String mpackInstance, @Nullable String serviceInstance) {
+  public Component(String name, @Nullable String mpackInstance, @Nullable String serviceInstance, ProvisionAction provisionAction) {
     this.name = name;
-    this.stackId = null;
     this.mpackInstance = mpackInstance;
     this.serviceInstance = serviceInstance;
-    this.provisionAction = null;
-  }
-
-  public Component(String name, @Nullable StackId stackId, @Nullable String serviceInstance, ProvisionAction provisionAction) {
-    this.name = name;
-    this.stackId = stackId;
-    this.mpackInstance = null;
-    this.serviceInstance = serviceInstance;
-    this.provisionAction = provisionAction;
+    this.provisionAction = provisionAction != null ? provisionAction : ProvisionAction.INSTALL_AND_START;
   }
 
   /**
@@ -70,21 +54,7 @@ public class Component {
     return this.name;
   }
 
-  /**
-   * @return the mpack associated with this component as {@link String} (can be {@code null} if component -> mpack mapping is unambiguous)
-   */
-  public String getStackIdAsString() {
-    return stackId != null ? stackId.toString() : null;
-  }
-
-  /**
-   * @return the mpack associated with this component as {@link StackId} (can be {@code null} if component -> mpack mapping is unambiguous)
-   */
-  public StackId getStackId() {
-    return stackId;
-  }
-
-
+  @Nullable
   public String getMpackInstance() {
     return this.mpackInstance;
   }
@@ -93,6 +63,7 @@ public class Component {
    * @return the service instance this component belongs to. Can be {@code null} if component does not belong to a service
    * instance (there is a single service of the component's service type)
    */
+  @Nullable
   public String getServiceInstance() {
     return serviceInstance;
   }
@@ -109,9 +80,8 @@ public class Component {
 
   @Override
   public String toString() {
-    return com.google.common.base.Objects.toStringHelper(this)
+    return MoreObjects.toStringHelper(this)
       .add("name", name)
-      .add("stackId", stackId)
       .add("mpackInstance", mpackInstance)
       .add("serviceInstance", serviceInstance)
       .add("provisionAction", provisionAction)
@@ -125,7 +95,6 @@ public class Component {
     if (o == null || getClass() != o.getClass()) return false;
     Component component = (Component) o;
     return Objects.equals(name, component.name) &&
-      Objects.equals(stackId, component.stackId) &&
       Objects.equals(mpackInstance, component.mpackInstance) &&
       Objects.equals(serviceInstance, component.serviceInstance) &&
       provisionAction == component.provisionAction;
@@ -133,6 +102,6 @@ public class Component {
 
   @Override
   public int hashCode() {
-    return Objects.hash(name, stackId, mpackInstance, serviceInstance, provisionAction);
+    return Objects.hash(name, mpackInstance, serviceInstance, provisionAction);
   }
 }
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/topology/HostGroupImpl.java b/ambari-server/src/main/java/org/apache/ambari/server/topology/HostGroupImpl.java
index 83375d5..29e75a0 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/topology/HostGroupImpl.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/topology/HostGroupImpl.java
@@ -140,7 +140,7 @@ public class HostGroupImpl implements HostGroup {
 
       Component component = new Component(
         componentEntity.getName(),
-        stackId,
+        componentEntity.getMpackName(),
         componentEntity.getServiceName(),
         null == componentEntity.getProvisionAction() ? null : ProvisionAction.valueOf(componentEntity.getProvisionAction()));
 
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/topology/MpackInstance.java b/ambari-server/src/main/java/org/apache/ambari/server/topology/MpackInstance.java
index 882621b..5756941 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/topology/MpackInstance.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/topology/MpackInstance.java
@@ -21,7 +21,6 @@ package org.apache.ambari.server.topology;
 import java.util.ArrayList;
 import java.util.Collection;
 
-import org.apache.ambari.server.controller.internal.Stack;
 import org.apache.ambari.server.orm.entities.BlueprintEntity;
 import org.apache.ambari.server.orm.entities.BlueprintMpackInstanceEntity;
 import org.apache.ambari.server.orm.entities.MpackInstanceConfigEntity;
@@ -43,19 +42,23 @@ public class MpackInstance implements Configurable {
   @JsonProperty("url")
   private String url;
 
+  @JsonProperty("type")
   private String mpackType;
 
-  private Stack stack;
   private Configuration configuration = new Configuration();
 
   @JsonProperty("service_instances")
   private Collection<ServiceInstance> serviceInstances = new ArrayList<>();
 
-  public MpackInstance(String mpackName, String mpackVersion, String url, Stack stack, Configuration configuration) {
+  public MpackInstance(StackId stackId) {
+    this(null, stackId.getStackName(), stackId.getStackVersion(), null, Configuration.createEmpty());
+  }
+
+  public MpackInstance(String mpackName, String mpackType, String mpackVersion, String url, Configuration configuration) {
     this.mpackName = mpackName;
+    this.mpackType = mpackType;
     this.mpackVersion = mpackVersion;
     this.url = url;
-    this.stack = stack;
     this.configuration = configuration;
   }
 
@@ -69,42 +72,20 @@ public class MpackInstance implements Configurable {
   public MpackInstance() { }
 
   public String getMpackName() {
-    return mpackName;
-  }
-
-  public void setMpackName(String mpackName) {
-    this.mpackName = mpackName;
+    return mpackName != null ? mpackName : mpackType;
   }
 
   public String getMpackType() {
-    return mpackType;
-  }
-
-  public void setMpackType(String mpackType) {
-    this.mpackType = mpackType;
+    return mpackType != null ? mpackType : mpackName;
   }
 
-
   public String getMpackVersion() {
     return mpackVersion;
   }
 
-  public void setMpackVersion(String mpackVersion) {
-    this.mpackVersion = mpackVersion;
-  }
-
   @JsonIgnore
   public StackId getStackId() {
-    return new StackId(getMpackName(), getMpackVersion());
-  }
-
-  @JsonIgnore
-  public Stack getStack() {
-    return stack;
-  }
-
-  public void setStack(Stack stack) {
-    this.stack = stack;
+    return new StackId(getMpackType(), getMpackVersion());
   }
 
   @JsonIgnore
@@ -171,7 +152,8 @@ public class MpackInstance implements Configurable {
    */
   private void setCommonProperties(MpackInstanceEntity mpackInstanceEntity) {
     mpackInstanceEntity.setMpackUri(url);
-    mpackInstanceEntity.setMpackName(mpackName);
+    mpackInstanceEntity.setMpackName(getMpackName());
+    mpackInstanceEntity.setMpackType(getMpackType());
     mpackInstanceEntity.setMpackVersion(mpackVersion);
     Collection<MpackInstanceConfigEntity> mpackConfigEntities =
       BlueprintImpl.toConfigEntities(configuration, MpackInstanceConfigEntity::new);
@@ -192,11 +174,7 @@ public class MpackInstance implements Configurable {
   }
 
   public static MpackInstance fromEntity(MpackInstanceEntity entity) {
-    MpackInstance mpack = new MpackInstance();
-    mpack.setUrl(entity.getMpackUri());
-    mpack.setMpackName(entity.getMpackName());
-    mpack.setMpackVersion(entity.getMpackVersion());
-    mpack.setConfiguration(BlueprintImpl.fromConfigEntities(entity.getConfigurations()));
+    MpackInstance mpack = new MpackInstance(entity.getMpackName(), entity.getMpackType(), entity.getMpackVersion(), entity.getMpackUri(), BlueprintImpl.fromConfigEntities(entity.getConfigurations()));
     for (MpackInstanceServiceEntity serviceEntity: entity.getServiceInstances()) {
       ServiceInstance serviceInstance = new ServiceInstance();
       serviceInstance.setName(serviceEntity.getName());
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/topology/ResolvedComponent.java b/ambari-server/src/main/java/org/apache/ambari/server/topology/ResolvedComponent.java
index 2b9d478..b3849b4 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/topology/ResolvedComponent.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/topology/ResolvedComponent.java
@@ -61,7 +61,14 @@ public interface ResolvedComponent {
       .component(component)
       .componentName(component.getName())
       .serviceName(Optional.ofNullable(component.getServiceInstance()))
-      .serviceGroupName(Optional.ofNullable(component.getStackIdAsString()));
+      .serviceGroupName(Optional.ofNullable(component.getMpackInstance()));
+  }
+
+  /**
+   * Starts building a {@code ResolvedComponent}.
+   */
+  static Builder builder(String name) {
+    return new Builder().componentName(name);
   }
 
   class Builder extends ResolvedComponent_Builder {
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/topology/ResolvedComponent_Builder.java b/ambari-server/src/main/java/org/apache/ambari/server/topology/ResolvedComponent_Builder.java
index 7f3e805..082ed4b 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/topology/ResolvedComponent_Builder.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/topology/ResolvedComponent_Builder.java
@@ -34,7 +34,7 @@ import com.google.common.base.Preconditions;
  * Auto-generated superclass of {@link ResolvedComponent.Builder}, derived from the API of {@link
  * ResolvedComponent}.
  */
-abstract class ResolvedComponent_Builder {
+abstract class ResolvedComponent_Builder implements ResolvedComponent {
 
   /** Creates a new builder using {@code value} as a template. */
   public static ResolvedComponent.Builder from(ResolvedComponent value) {
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/topology/StackComponentResolver.java b/ambari-server/src/main/java/org/apache/ambari/server/topology/StackComponentResolver.java
index 783be40..4448feb 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/topology/StackComponentResolver.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/topology/StackComponentResolver.java
@@ -17,13 +17,16 @@
  */
 package org.apache.ambari.server.topology;
 
+import static java.util.stream.Collectors.toMap;
 import static java.util.stream.Collectors.toSet;
 
+import java.util.Collection;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
 import java.util.stream.Stream;
 
@@ -34,7 +37,6 @@ import org.apache.commons.lang3.tuple.Pair;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import com.google.common.base.Joiner;
 import com.google.common.base.Strings;
 
 public class StackComponentResolver implements ComponentResolver {
@@ -43,20 +45,21 @@ public class StackComponentResolver implements ComponentResolver {
 
   @Override
   public Map<String, Set<ResolvedComponent>> resolveComponents(BlueprintBasedClusterProvisionRequest request) {
-    Map<String, ServiceInstance> uniqueServices = request.getUniqueServices();
-    Map<String, Map<String, ServiceInstance>> mpackServices = request.getServicesByMpack();
+    Collection<MpackInstance> mpacks = request.getMpacks();
+    Map<String, StackId> stackIdByMpackName = getMpackStackIds(mpacks);
 
     Map<String, Set<ResolvedComponent>> result = new HashMap<>();
     List<String> problems = new LinkedList<>();
 
     StackDefinition stack = request.getStack();
     for (HostGroup hg : request.getHostGroups().values()) {
-      result.put(hg.getName(), new HashSet<>());
+      Set<ResolvedComponent> hostGroupComponents = new HashSet<>();
+      result.put(hg.getName(), hostGroupComponents);
 
       for (Component comp : hg.getComponents()) {
+        String mpackInstanceName = comp.getMpackInstance();
         Stream<Pair<StackId, ServiceInfo>> servicesForComponent = stack.getServicesForComponent(comp.getName());
-        servicesForComponent = filterByMpackName(comp, servicesForComponent);
-        servicesForComponent = filterByServiceName(comp, servicesForComponent, mpackServices, uniqueServices);
+        servicesForComponent = filterByMpackName(mpackInstanceName, servicesForComponent, stackIdByMpackName);
 
         Set<Pair<StackId, ServiceInfo>> serviceMatches = servicesForComponent.collect(toSet());
 
@@ -75,13 +78,13 @@ public class StackComponentResolver implements ComponentResolver {
             .build();
 
           LOG.debug("Component resolved: " + resolved);
-          result.get(hg.getName()).add(resolved);
+          hostGroupComponents.add(resolved);
         }
       }
     }
 
     if (!problems.isEmpty()) {
-      throw new IllegalArgumentException("Component resolution failure:\n" + Joiner.on("\n").join(problems));
+      throw new IllegalArgumentException("Component resolution failure:\n" + String.join("\n", problems));
     }
 
     return result;
@@ -95,8 +98,8 @@ public class StackComponentResolver implements ComponentResolver {
       .append(" found for component ").append(comp.getName())
       .append(" in host group " ).append(hg.getName());
 
-    if (!Strings.isNullOrEmpty(comp.getStackIdAsString())) {
-      sb.append(" mpack: ").append(comp.getStackIdAsString());
+    if (!Strings.isNullOrEmpty(comp.getMpackInstance())) {
+      sb.append(" mpack: ").append(comp.getMpackInstance());
     }
     if (!Strings.isNullOrEmpty(comp.getServiceInstance())) {
       sb.append(" service: ").append(comp.getServiceInstance());
@@ -108,33 +111,22 @@ public class StackComponentResolver implements ComponentResolver {
     return sb.toString();
   }
 
-  // if component references a specific service instance, filter the stream by the type of that service
-  private static Stream<Pair<StackId, ServiceInfo>> filterByServiceName(Component comp, Stream<Pair<StackId, ServiceInfo>> stream,
-    Map<String, Map<String, ServiceInstance>> mpackServices, Map<String, ServiceInstance> uniqueServices
+  // if component references a specific mpack instance, filter the stream by the name of that mpack
+  private static Stream<Pair<StackId, ServiceInfo>> filterByMpackName(
+    String mpackInstanceName,
+    Stream<Pair<StackId, ServiceInfo>> stream,
+    Map<String, StackId> stackIdByMpackInstanceName
   ) {
-    if (!Strings.isNullOrEmpty(comp.getServiceInstance())) {
-      String mpackName = comp.getStackIdAsString();
-      Map<String, ServiceInstance> services = !Strings.isNullOrEmpty(mpackName)
-        ? mpackServices.get(mpackName)
-        : uniqueServices;
-
-      ServiceInstance service = services.get(comp.getServiceInstance());
-      if (service != null) {
-        String serviceType = service.getType();
-
-        return stream.filter(pair -> pair.getRight().getName().equals(serviceType));
-      }
+    if (mpackInstanceName != null) {
+      StackId mpackStackId = stackIdByMpackInstanceName.get(mpackInstanceName);
+      return stream.filter(pair -> Objects.equals(pair.getLeft(), mpackStackId));
     }
-
     return stream;
   }
 
-  // if component references a specific mpack instance, filter the stream by the name of that mpack
-  private static Stream<Pair<StackId, ServiceInfo>> filterByMpackName(Component comp, Stream<Pair<StackId, ServiceInfo>> stream) {
-    if (comp.getStackId() != null) {
-      return stream.filter(pair -> pair.getLeft().equals(comp.getStackId()));
-    }
-    return stream;
+  private static Map<String, StackId> getMpackStackIds(Collection<MpackInstance> mpacks) {
+    return mpacks.stream()
+      .collect(toMap(MpackInstance::getMpackName, MpackInstance::getStackId));
   }
 
 }
diff --git a/ambari-server/src/main/resources/Ambari-DDL-Derby-CREATE.sql b/ambari-server/src/main/resources/Ambari-DDL-Derby-CREATE.sql
index 5c0e8a1..a4cfeb6 100644
--- a/ambari-server/src/main/resources/Ambari-DDL-Derby-CREATE.sql
+++ b/ambari-server/src/main/resources/Ambari-DDL-Derby-CREATE.sql
@@ -621,6 +621,7 @@ CREATE TABLE mpack_instance(
   blueprint_name VARCHAR(255),
   topology_request_id BIGINT,
   mpack_name VARCHAR(255) NOT NULL,
+  mpack_type VARCHAR(255) NOT NULL,
   mpack_version VARCHAR(255) NOT NULL,
   mpack_uri VARCHAR(255),
   mpack_id BIGINT,
diff --git a/ambari-server/src/main/resources/Ambari-DDL-MySQL-CREATE.sql b/ambari-server/src/main/resources/Ambari-DDL-MySQL-CREATE.sql
index d9f1472..4dce027 100644
--- a/ambari-server/src/main/resources/Ambari-DDL-MySQL-CREATE.sql
+++ b/ambari-server/src/main/resources/Ambari-DDL-MySQL-CREATE.sql
@@ -639,6 +639,7 @@ CREATE TABLE mpack_instance(
   blueprint_name VARCHAR(255),
   topology_request_id BIGINT,
   mpack_name VARCHAR(255) NOT NULL,
+  mpack_type VARCHAR(255) NOT NULL,
   mpack_version VARCHAR(255) NOT NULL,
   mpack_uri VARCHAR(255),
   mpack_id BIGINT,
diff --git a/ambari-server/src/main/resources/Ambari-DDL-Oracle-CREATE.sql b/ambari-server/src/main/resources/Ambari-DDL-Oracle-CREATE.sql
index a2b9dee..88bb0c3 100644
--- a/ambari-server/src/main/resources/Ambari-DDL-Oracle-CREATE.sql
+++ b/ambari-server/src/main/resources/Ambari-DDL-Oracle-CREATE.sql
@@ -619,6 +619,7 @@ CREATE TABLE mpack_instance(
   blueprint_name VARCHAR2(255),
   topology_request_id NUMBER(19),
   mpack_name VARCHAR2(255) NOT NULL,
+  mpack_type VARCHAR(255) NOT NULL,
   mpack_version VARCHAR2(255) NOT NULL,
   mpack_uri VARCHAR2(255),
   mpack_id NUMBER(19),
diff --git a/ambari-server/src/main/resources/Ambari-DDL-Postgres-CREATE.sql b/ambari-server/src/main/resources/Ambari-DDL-Postgres-CREATE.sql
index 505e8db..dc33e43 100644
--- a/ambari-server/src/main/resources/Ambari-DDL-Postgres-CREATE.sql
+++ b/ambari-server/src/main/resources/Ambari-DDL-Postgres-CREATE.sql
@@ -622,6 +622,7 @@ CREATE TABLE mpack_instance(
   blueprint_name VARCHAR(255),
   topology_request_id BIGINT,
   mpack_name VARCHAR(255) NOT NULL,
+  mpack_type VARCHAR(255) NOT NULL,
   mpack_version VARCHAR(255) NOT NULL,
   mpack_uri VARCHAR(255),
   mpack_id BIGINT,
diff --git a/ambari-server/src/main/resources/Ambari-DDL-SQLAnywhere-CREATE.sql b/ambari-server/src/main/resources/Ambari-DDL-SQLAnywhere-CREATE.sql
index 0033a69..3e847c6 100644
--- a/ambari-server/src/main/resources/Ambari-DDL-SQLAnywhere-CREATE.sql
+++ b/ambari-server/src/main/resources/Ambari-DDL-SQLAnywhere-CREATE.sql
@@ -617,6 +617,7 @@ CREATE TABLE mpack_instance(
   blueprint_name VARCHAR(255),
   topology_request_id NUMERIC(19),
   mpack_name VARCHAR(255) NOT NULL,
+  mpack_type VARCHAR(255) NOT NULL,
   mpack_version VARCHAR(255) NOT NULL,
   mpack_uri VARCHAR(255),
   mpack_id NUMERIC(19),
diff --git a/ambari-server/src/main/resources/Ambari-DDL-SQLServer-CREATE.sql b/ambari-server/src/main/resources/Ambari-DDL-SQLServer-CREATE.sql
index c6cca3c..1f1a24d 100644
--- a/ambari-server/src/main/resources/Ambari-DDL-SQLServer-CREATE.sql
+++ b/ambari-server/src/main/resources/Ambari-DDL-SQLServer-CREATE.sql
@@ -636,6 +636,7 @@ CREATE TABLE mpack_instance(
   blueprint_name VARCHAR(255),
   topology_request_id BIGINT,
   mpack_name VARCHAR(255) NOT NULL,
+  mpack_type VARCHAR(255) NOT NULL,
   mpack_version VARCHAR(255) NOT NULL,
   mpack_uri VARCHAR(255),
   mpack_id BIGINT,
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/topology/AmbariContextTest.java b/ambari-server/src/test/java/org/apache/ambari/server/topology/AmbariContextTest.java
index b53bc5e..5354088 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/topology/AmbariContextTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/topology/AmbariContextTest.java
@@ -407,9 +407,9 @@ public class AmbariContextTest {
 
     // test
     Stream<ResolvedComponent> components = Stream.of(
-      ResolvedComponent.builder(new Component("component1", new StackId("mpack", "1.0"), "service1", null)).buildPartial(),
-      ResolvedComponent.builder(new Component("component2", new StackId("mpack", "1.0"), "service1", null)).buildPartial(),
-      ResolvedComponent.builder(new Component("component3", new StackId("mpack", "1.0"), "service2", null)).buildPartial()
+      ResolvedComponent.builder(new Component("component1", "mpack", "service1", null)).buildPartial(),
+      ResolvedComponent.builder(new Component("component2", "mpack", "service1", null)).buildPartial(),
+      ResolvedComponent.builder(new Component("component3", "mpack", "service2", null)).buildPartial()
     );
     context.createAmbariHostResources(CLUSTER_ID, "host1", components);
 
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/topology/BlueprintFactoryTest.java b/ambari-server/src/test/java/org/apache/ambari/server/topology/BlueprintFactoryTest.java
index ff163d3..2b64390 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/topology/BlueprintFactoryTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/topology/BlueprintFactoryTest.java
@@ -35,7 +35,6 @@ import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
 import java.util.stream.Collectors;
-import java.util.stream.Stream;
 
 import org.apache.ambari.server.controller.internal.BlueprintResourceProvider;
 import org.apache.ambari.server.controller.internal.BlueprintResourceProviderTest;
@@ -250,45 +249,4 @@ public class BlueprintFactoryTest {
     testFactory.createBlueprint(props, null);
   }
 
-  @Test(expected = IllegalArgumentException.class) // THEN
-  public void verifyDefinitionsDisjointShouldRejectDuplication() {
-    // GIVEN
-    final String service1 = "unique service";
-    final String service2 = "duplicated service";
-    StackId stack1 = new StackId("a_stack", "1.0");
-    StackId stack2 = new StackId("another_stack", "0.9");
-    Stream<String> stream = ImmutableSet.of(service1, service2).stream();
-
-    // WHEN
-    BlueprintFactory.verifyStackDefinitionsAreDisjoint(stream, "Services", service -> {
-      switch (service) {
-        case service1: return ImmutableSet.of(stack1);
-        case service2: return ImmutableSet.of(stack1, stack2);
-        default: return null;
-      }
-    });
-  }
-
-  @Test
-  public void verifyStackDefinitionsAreDisjointShouldAllowDisjointStacks() {
-    // GIVEN
-    final String service1 = "unique service";
-    final String service2 = "another service";
-    StackId stack1 = new StackId("a_stack", "1.0");
-    StackId stack2 = new StackId("another_stack", "0.9");
-    Stream<String> stream = ImmutableSet.of(service1, service2).stream();
-
-    // WHEN
-    BlueprintFactory.verifyStackDefinitionsAreDisjoint(stream, "Services", service -> {
-      switch (service) {
-        case service1: return ImmutableSet.of(stack1);
-        case service2: return ImmutableSet.of(stack2);
-        default: return null;
-      }
-    });
-
-    // THEN
-    // no exception expected
-  }
-
 }
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/topology/DownloadMpacksTaskTest.java b/ambari-server/src/test/java/org/apache/ambari/server/topology/DownloadMpacksTaskTest.java
index 8a27be5..bce2a1e 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/topology/DownloadMpacksTaskTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/topology/DownloadMpacksTaskTest.java
@@ -124,11 +124,7 @@ public class DownloadMpacksTaskTest {
   }
 
   private static MpackInstance mpack(String stackName, String stackVersion) {
-    MpackInstance mpack = new MpackInstance();
-    mpack.setMpackName(stackName);
-    mpack.setMpackVersion(stackVersion);
-    mpack.setUrl(createUri(stackName, stackVersion));
-    return mpack;
+    return new MpackInstance(stackName, stackName, stackVersion, createUri(stackName, stackVersion), Configuration.createEmpty());
   }
 
   private static String createUri(String stackName, String stackVersion) {
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/topology/StackComponentResolverTest.java b/ambari-server/src/test/java/org/apache/ambari/server/topology/StackComponentResolverTest.java
index 6235aae..98179a6 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/topology/StackComponentResolverTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/topology/StackComponentResolverTest.java
@@ -17,71 +17,280 @@
  */
 package org.apache.ambari.server.topology;
 
-import static org.easymock.EasyMock.createNiceMock;
+import static java.util.Comparator.comparing;
+import static java.util.stream.Collectors.groupingBy;
+import static java.util.stream.Collectors.toList;
+import static java.util.stream.Collectors.toSet;
+import static org.easymock.EasyMock.anyString;
 import static org.easymock.EasyMock.expect;
-import static org.easymock.EasyMock.replay;
-import static org.easymock.EasyMock.reset;
+import static org.junit.Assert.assertEquals;
 
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Comparator;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
 import java.util.stream.Stream;
 
 import org.apache.ambari.server.controller.internal.StackDefinition;
-import org.apache.ambari.server.topology.validators.RejectUnknownComponents;
-import org.apache.ambari.server.topology.validators.TopologyValidator;
-import org.junit.After;
+import org.apache.ambari.server.state.ServiceInfo;
+import org.apache.ambari.server.state.StackId;
+import org.apache.commons.lang3.tuple.Pair;
+import org.easymock.EasyMockRule;
+import org.easymock.EasyMockSupport;
+import org.easymock.Mock;
+import org.easymock.MockType;
+import org.easymock.TestSubject;
 import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
 
+import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.ImmutableSortedSet;
+import com.google.common.collect.Sets;
 
-public class StackComponentResolverTest {
+public class StackComponentResolverTest extends EasyMockSupport {
 
-  private final TopologyValidator validator = new RejectUnknownComponents();
-  private final ClusterTopology topology = createNiceMock(ClusterTopology.class);
-  private final StackDefinition stack = createNiceMock(StackDefinition.class);
+  private static final StackId STACK_ID1 = new StackId("HDPCORE", "1.0.0-b123");
+  private static final StackId STACK_ID2 = new StackId("ODS", "1.0.0-b1111");
+  private static final MpackInstance MPACK_INSTANCE1 = new MpackInstance(STACK_ID1);
+  private static final MpackInstance MPACK_INSTANCE2 = new MpackInstance(STACK_ID2);
+
+  private static final Comparator<ResolvedComponent> RESOLVED_COMPONENT_COMPARATOR = comparing((ResolvedComponent comp) -> comp.stackId().toString())
+    .thenComparing(ResolvedComponent::effectiveServiceGroupName)
+    .thenComparing(ResolvedComponent::effectiveServiceName)
+    .thenComparing(ResolvedComponent::componentName);
+
+  private static final Set<String> CLIENT_COMPONENTS = Sets.union(hdpCoreComponents(), odsComponents()).stream()
+    .map(ResolvedComponent::componentName)
+    .filter(each -> each.contains("_CLIENT"))
+    .collect(toSet());
+
+  @Rule
+  public EasyMockRule rule = new EasyMockRule(this);
+
+  @TestSubject
+  private final ComponentResolver subject = new StackComponentResolver();
+
+  @Mock(type = MockType.NICE)
+  private BlueprintBasedClusterProvisionRequest request;
+
+  @Mock(type = MockType.NICE)
+  private StackDefinition stack;
 
   @Before
-  public void setUp() {
-    expect(topology.getStack()).andReturn(stack).anyTimes();
+  public void setup() {
+    expect(request.getStack()).andReturn(stack).anyTimes();
   }
 
-  @After
-  public void tearDown() {
-    reset(topology, stack);
+  @Test(expected = IllegalArgumentException.class) // THEN
+  public void ambiguousComponentName() {
+    // GIVEN
+    Set<ResolvedComponent> components = build(Sets.union(hdpCoreComponents(), odsComponents()));
+    aStackWith(components);
+    defineMpacksAs(MPACK_INSTANCE1, MPACK_INSTANCE2);
+    aHostGroupWith(components);
+    replayAll();
+
+    // WHEN
+    Map<String, Set<ResolvedComponent>> resolved = subject.resolveComponents(request);
+  }
+
+  @Test(expected = IllegalArgumentException.class) // THEN
+  public void unknownComponent() {
+    // GIVEN
+    Set<ResolvedComponent> components = build(Sets.union(hdpCoreComponents(), odsComponents()));
+    aStackWith(components);
+    defineMpacksAs(MPACK_INSTANCE1, MPACK_INSTANCE2);
+    aHostGroupWith(new Component("UNKNOWN_COMPONENT"));
+    replayAll();
+
+    // WHEN
+    Map<String, Set<ResolvedComponent>> resolved = subject.resolveComponents(request);
   }
 
   @Test
-  public void acceptsKnownComponents() throws Exception {
+  public void withExplicitNamesOnlyForClients() {
     // GIVEN
-    componentsInTopologyAre("VALID_COMPONENT", "ANOTHER_COMPONENT");
-    validComponentsAre("VALID_COMPONENT", "ANOTHER_COMPONENT", "ONE_MORE_COMPONENT");
+    Set<ResolvedComponent.Builder> builders = Sets.union(hdpCoreComponents(), odsComponents());
+    builders.stream().filter(each -> CLIENT_COMPONENTS.contains(each.componentName())).forEach(each -> each.serviceGroupName(each.stackId().getStackName()));
+    Set<ResolvedComponent> components = build(builders);
+    aStackWith(components);
+    defineMpacksAs(MPACK_INSTANCE1, MPACK_INSTANCE2);
+    aHostGroupWith(components);
+    replayAll();
 
     // WHEN
-    validator.validate(topology);
+    Map<String, Set<ResolvedComponent>> resolved = subject.resolveComponents(request);
 
     // THEN
-    // no exception expected
+    assertHostGroupEquals(ImmutableMap.of("node", components), resolved);
   }
 
-  @Test(expected = InvalidTopologyException.class)
-  public void rejectsUnknownComponents() throws Exception {
+  @Test
+  public void withExplicitDefaultNames() {
     // GIVEN
-    componentsInTopologyAre("VALID_COMPONENT", "UNKNOWN_COMPONENT");
-    validComponentsAre("VALID_COMPONENT", "ANOTHER_COMPONENT");
+    String mpackInstanceName1 = STACK_ID1.getStackName(), mpackInstanceName2 = STACK_ID2.getStackName();
+    Set<ResolvedComponent> components = build(Sets.union(
+      inServiceGroup(mpackInstanceName1, hdpCoreComponents()),
+      inServiceGroup(mpackInstanceName2, odsComponents())
+    ));
+
+    aStackWith(components);
+
+    defineMpacksAs(
+      withCustomName(MPACK_INSTANCE1, mpackInstanceName1),
+      withCustomName(MPACK_INSTANCE2, mpackInstanceName2)
+    );
+
+    aHostGroupWith(components);
+    replayAll();
+
+    // WHEN
+    Map<String, Set<ResolvedComponent>> resolved = subject.resolveComponents(request);
+
+    // THEN
+    assertHostGroupEquals(ImmutableMap.of("node", components), resolved);
+  }
+
+  @Test
+  public void withCustomNames() {
+    // GIVEN
+    String mpackInstanceName1 = "hdp-core", mpackInstanceName2 = "ods!";
+    Set<ResolvedComponent> components = build(Sets.union(
+      inServiceGroup(mpackInstanceName1, hdpCoreComponents()),
+      inServiceGroup(mpackInstanceName2, odsComponents())
+    ));
+
+    aStackWith(components);
+
+    defineMpacksAs(
+      withCustomName(MPACK_INSTANCE1, mpackInstanceName1),
+      withCustomName(MPACK_INSTANCE2, mpackInstanceName2)
+    );
+
+    aHostGroupWith(components);
+    replayAll();
 
     // WHEN
-    validator.validate(topology);
+    Map<String, Set<ResolvedComponent>> resolved = subject.resolveComponents(request);
+
+    // THEN
+    assertHostGroupEquals(ImmutableMap.of("node", components), resolved);
+  }
+
+  private void defineMpacksAs(MpackInstance... mpacks) {
+    expect(request.getMpacks()).andReturn(ImmutableSet.copyOf(mpacks)).anyTimes();
+  }
+
+  private void aStackWith(Set<ResolvedComponent> components) {
+    Map<Pair<String, String>, List<ResolvedComponent>> byServiceName = components.stream()
+      .collect(groupingBy(each -> Pair.of(each.effectiveServiceGroupName(), each.effectiveServiceName())));
+
+    Map<ResolvedComponent, ServiceInfo> services = new HashMap<>();
+    for (Map.Entry<Pair<String, String>, List<ResolvedComponent>> entry : byServiceName.entrySet()) {
+      ServiceInfo service = createNiceMock(ServiceInfo.class);
+      expect(service.getName()).andReturn(entry.getValue().iterator().next().serviceType()).anyTimes();
+      for (ResolvedComponent component : entry.getValue()) {
+        services.put(component, service);
+      }
+    }
+
+    Map<String, List<Pair<ResolvedComponent, ServiceInfo>>> byComponentName = components.stream()
+      .map(each -> Pair.of(each, services.get(each)))
+      .collect(groupingBy(each -> each.getLeft().componentName()));
+
+    for (Map.Entry<String, List<Pair<ResolvedComponent, ServiceInfo>>> entry : byComponentName.entrySet()) {
+      expect(stack.getServicesForComponent(entry.getKey()))
+        .andAnswer(() -> entry.getValue().stream().map(each -> Pair.of(each.getLeft().stackId(), each.getRight()))).anyTimes();
+    }
+    expect(stack.getServicesForComponent(anyString())).andAnswer(Stream::empty).anyTimes();
+  }
+
+  private void mpacksWith(String serviceName, String componentName, MpackInstance... mpacks) {
+    defineMpacksAs(mpacks);
+
+    Collection<Pair<StackId, ServiceInfo>> services = Arrays.stream(mpacks)
+      .map(mpack -> {
+        ServiceInfo service = createNiceMock(ServiceInfo.class);
+        expect(service.getName()).andReturn(serviceName).anyTimes();
+        return Pair.of(mpack.getStackId(), service);
+      })
+      .collect(toList());
+    expect(stack.getServicesForComponent(componentName)).andAnswer(services::stream).anyTimes();
+  }
+
+  private void aHostGroupWith(Component component) {
+    hostGroupWith(ImmutableSet.of(component));
+  }
+
+  private void aHostGroupWith(Collection<ResolvedComponent> components) {
+    hostGroupWith(components.stream()
+      .map(each -> new Component(each.componentName(), each.serviceGroupName().orElse(null), each.effectiveServiceName(), null))
+      .collect(toSet()));
+  }
+
+  private void hostGroupWith(Set<Component> components) {
+    HostGroup hostGroup = new HostGroupImpl("node", components, Configuration.createEmpty(), "1+");
+    Map<String, HostGroup> hostGroups = ImmutableMap.of(hostGroup.getName(), hostGroup);
+    expect(request.getHostGroups()).andReturn(hostGroups).anyTimes();
+  }
+
+  private static Set<ResolvedComponent> build(ResolvedComponent.Builder builder) {
+    return build(ImmutableSet.of(builder));
+  }
+
+  private static Set<ResolvedComponent> build(Set<ResolvedComponent.Builder> builders) {
+    builders.forEach(each ->
+      each.component(new Component(each.componentName(), each.effectiveServiceGroupName(), each.effectiveServiceName(), null)));
+
+    return builders.stream()
+      .map(ResolvedComponent.Builder::build)
+      .collect(toSet());
+  }
+
+  private static MpackInstance withCustomName(MpackInstance mpack, String name) {
+    return new MpackInstance(name, mpack.getMpackType(), mpack.getMpackVersion(), mpack.getUrl(), mpack.getConfiguration());
+  }
+
+  private static Set<ResolvedComponent.Builder> inServiceGroup(String name, Collection<ResolvedComponent.Builder> components) {
+    return components.stream().map(each -> each.serviceGroupName(name)).collect(toSet());
+  }
+
+  private static void assertHostGroupEquals(Map<String, Set<ResolvedComponent>> expected, Map<String, Set<ResolvedComponent>> actual) {
+    assertEquals(expected.keySet(), actual.keySet());
+    for (String hostGroupName : expected.keySet()) {
+      Set<ResolvedComponent> expectedComponents = ImmutableSortedSet.copyOf(RESOLVED_COMPONENT_COMPARATOR, expected.get(hostGroupName));
+      Set<ResolvedComponent> actualComponents = ImmutableSortedSet.copyOf(RESOLVED_COMPONENT_COMPARATOR, actual.get(hostGroupName));
+      assertEquals(expectedComponents, actualComponents);
+    }
   }
 
-  private void componentsInTopologyAre(String... components) {
-    expect(topology.getComponents()).andReturn(Stream.of(components)
-      .map(name -> ResolvedComponent.builder(new Component(name)).buildPartial())
-    ).anyTimes();
-    replay(topology);
+  private static Set<ResolvedComponent.Builder> hdpCoreComponents() {
+    Set<ResolvedComponent.Builder> builders = ImmutableSet.of(
+      ResolvedComponent.builder("HADOOP_CLIENT").serviceType("HADOOP_CLIENTS"),
+      ResolvedComponent.builder("DATANODE").serviceType("HDFS"),
+      ResolvedComponent.builder("NAMENODE").serviceType("HDFS"),
+      ResolvedComponent.builder("ZOOKEEPER_CLIENT").serviceType("ZOOKEEPER_CLIENTS"),
+      ResolvedComponent.builder("ZOOKEEPER_SERVER").serviceType("ZOOKEEPER")
+    );
+    builders.forEach(each -> each.stackId(STACK_ID1));
+    return builders;
   }
 
-  private void validComponentsAre(String... components) {
-    expect(stack.getComponents()).andReturn(ImmutableSet.<String>builder().add(components).build()).anyTimes();
-    replay(stack);
+  private static Set<ResolvedComponent.Builder> odsComponents() {
+    Set<ResolvedComponent.Builder> builders = ImmutableSet.of(
+      ResolvedComponent.builder("HADOOP_CLIENT").serviceType("HADOOP_CLIENTS"),
+      ResolvedComponent.builder("HBASE_MASTER").serviceType("HBASE"),
+      ResolvedComponent.builder("HBASE_REGIONSERVER").serviceType("HBASE"),
+      ResolvedComponent.builder("HBASE_CLIENT").serviceType("HBASE_CLIENTS"),
+      ResolvedComponent.builder("ZOOKEEPER_CLIENT").serviceType("ZOOKEEPER_CLIENTS")
+    );
+    builders.forEach(each -> each.stackId(STACK_ID2));
+    return builders;
   }
 
 }
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/topology/TopologyManagerTest.java b/ambari-server/src/test/java/org/apache/ambari/server/topology/TopologyManagerTest.java
index 39799de..5f7c854 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/topology/TopologyManagerTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/topology/TopologyManagerTest.java
@@ -220,8 +220,8 @@ public class TopologyManagerTest {
   private final Collection<Component> group2Components = Arrays.asList(new Component("component3"), new Component("component4"));
   private final Collection<String> group2ComponentNames = group2Components.stream().map(Component::getName).collect(toSet());
 
-  private final MpackInstance mpack1 = new MpackInstance("HDPCORE", "1.0.0.0", "http://mpacks.org/hdpcore", null, new Configuration());
-  private final MpackInstance mpack2 = new MpackInstance("HDF", "3.3.0", "http://mpacks.org/hdf", null, new Configuration());
+  private final MpackInstance mpack1 = new MpackInstance("HDPCORE", "HDPCORE", "1.0.0.0", "http://mpacks.org/hdpcore", new Configuration());
+  private final MpackInstance mpack2 = new MpackInstance("HDF", "HDF", "3.3.0", "http://mpacks.org/hdf", new Configuration());
 
   private Map<String, Collection<String>> group1ServiceComponents = new HashMap<>();
   private Map<String, Collection<String>> group2ServiceComponents = new HashMap<>();
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/topology/StackComponentResolverTest.java b/ambari-server/src/test/java/org/apache/ambari/server/topology/validators/RejectUnknownComponentsTest.java
similarity index 89%
copy from ambari-server/src/test/java/org/apache/ambari/server/topology/StackComponentResolverTest.java
copy to ambari-server/src/test/java/org/apache/ambari/server/topology/validators/RejectUnknownComponentsTest.java
index 6235aae..d71051b 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/topology/StackComponentResolverTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/topology/validators/RejectUnknownComponentsTest.java
@@ -15,7 +15,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package org.apache.ambari.server.topology;
+package org.apache.ambari.server.topology.validators;
 
 import static org.easymock.EasyMock.createNiceMock;
 import static org.easymock.EasyMock.expect;
@@ -25,15 +25,17 @@ import static org.easymock.EasyMock.reset;
 import java.util.stream.Stream;
 
 import org.apache.ambari.server.controller.internal.StackDefinition;
-import org.apache.ambari.server.topology.validators.RejectUnknownComponents;
-import org.apache.ambari.server.topology.validators.TopologyValidator;
+import org.apache.ambari.server.topology.ClusterTopology;
+import org.apache.ambari.server.topology.Component;
+import org.apache.ambari.server.topology.InvalidTopologyException;
+import org.apache.ambari.server.topology.ResolvedComponent;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 
 import com.google.common.collect.ImmutableSet;
 
-public class StackComponentResolverTest {
+public class RejectUnknownComponentsTest {
 
   private final TopologyValidator validator = new RejectUnknownComponents();
   private final ClusterTopology topology = createNiceMock(ClusterTopology.class);

-- 
To stop receiving notification emails like this one, please contact
adoroszlai@apache.org.

[ambari] 07/07: AMBARI-23746. Use List for componentID duplicate check

Posted by ad...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

adoroszlai pushed a commit to branch branch-feature-AMBARI-14714
in repository https://gitbox.apache.org/repos/asf/ambari.git

commit f8293cff7b260b9038fa9f2ba618929c65d82999
Author: Doroszlai, Attila <ad...@apache.org>
AuthorDate: Wed May 30 12:01:12 2018 +0200

    AMBARI-23746. Use List for componentID duplicate check
---
 .../server/controller/internal/ComponentResourceProvider.java       | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ComponentResourceProvider.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ComponentResourceProvider.java
index 64afd34..9e32f27 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ComponentResourceProvider.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ComponentResourceProvider.java
@@ -68,7 +68,7 @@ import org.apache.commons.lang.Validate;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Sets;
 import com.google.inject.Inject;
 import com.google.inject.assistedinject.Assisted;
@@ -394,7 +394,7 @@ public class ComponentResourceProvider extends AbstractControllerResourceProvide
     ServiceComponentFactory serviceComponentFactory = getManagementController().getServiceComponentFactory();
 
     // do all validation checks
-    Map<String, Set<Set<String>>> componentNames = new HashMap<>();
+    Map<String, Set<List<String>>> componentNames = new HashMap<>();
     Set<String> duplicates = new HashSet<>();
 
     for (ServiceComponentRequest request : requests) {
@@ -414,7 +414,7 @@ public class ComponentResourceProvider extends AbstractControllerResourceProvide
 
       debug("Received a createComponent request: {}", request);
 
-      Set<String> componentID = ImmutableSet.of(request.getServiceGroupName(), request.getServiceName(), request.getComponentName());
+      List<String> componentID = ImmutableList.of(request.getServiceGroupName(), request.getServiceName(), request.getComponentName());
       boolean added = componentNames
         .computeIfAbsent(request.getClusterName(), __ -> new HashSet<>())
         .add(componentID);

-- 
To stop receiving notification emails like this one, please contact
adoroszlai@apache.org.

[ambari] 06/07: AMBARI-23746. Cannot query services with same name

Posted by ad...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

adoroszlai pushed a commit to branch branch-feature-AMBARI-14714
in repository https://gitbox.apache.org/repos/asf/ambari.git

commit 9120330456c07dfbc2a5f34ddf1752e7675f6ff7
Author: Doroszlai, Attila <ad...@apache.org>
AuthorDate: Wed May 30 00:30:19 2018 +0200

    AMBARI-23746. Cannot query services with same name
---
 .../controller/AmbariManagementControllerImpl.java |  8 ++---
 .../internal/ComponentResourceProvider.java        |  2 +-
 .../ServiceDependencyResourceProvider.java         | 17 ++++------
 .../internal/ServiceResourceProvider.java          | 11 +++----
 .../ambari/server/state/cluster/ClusterImpl.java   | 38 +++++++++-------------
 .../internal/ComponentResourceProviderTest.java    | 13 +++++---
 .../ServiceDependencyResourceProviderTest.java     |  2 +-
 .../internal/ServiceResourceProviderTest.java      | 31 ++++++++----------
 8 files changed, 56 insertions(+), 66 deletions(-)

diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java
index 24b95e6..a036408 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java
@@ -761,7 +761,7 @@ public class AmbariManagementControllerImpl implements AmbariManagementControlle
 
     for (ServiceComponentHostRequest request : requests) {
       Cluster cluster = clusters.getCluster(request.getClusterName());
-      Service s = cluster.getService(request.getServiceName());
+      Service s = cluster.getService(request.getServiceGroupName(), request.getServiceName());
       ServiceComponent sc = s.getServiceComponent(
           request.getComponentName());
       serviceComponentNames.computeIfAbsent(sc.getClusterId(), c -> new HashMap<>())
@@ -1325,7 +1325,7 @@ public class AmbariManagementControllerImpl implements AmbariManagementControlle
 
     List<Service> services;
     if (!Strings.isNullOrEmpty(request.getServiceName())) {
-      services = ImmutableList.of(cluster.getService(request.getServiceName()));
+      services = ImmutableList.of(cluster.getService(request.getServiceGroupName(), request.getServiceName()));
     } else if (!Strings.isNullOrEmpty(request.getServiceGroupName())) {
       services = ImmutableList.copyOf(cluster.getServicesByServiceGroup(request.getServiceGroupName()));
     } else {
@@ -3300,7 +3300,7 @@ public class AmbariManagementControllerImpl implements AmbariManagementControlle
   }
 
   private boolean hostComponentAlreadyExists(Cluster cluster, ServiceComponentHost sch) throws AmbariException {
-    Service service = cluster.getService(sch.getServiceName());
+    Service service = cluster.getService(sch.getServiceGroupName(), sch.getServiceName());
     if (service != null) {
       ServiceComponent serviceComponent = service.getServiceComponent(sch.getServiceComponentName());
       if (serviceComponent != null) {
@@ -3319,7 +3319,7 @@ public class AmbariManagementControllerImpl implements AmbariManagementControlle
   private boolean skipInstallTaskForComponent(Map<String, String> requestProperties, Cluster cluster,
                                               ServiceComponentHost sch) throws AmbariException {
     boolean isClientComponent = false;
-    Service service = cluster.getService(sch.getServiceName());
+    Service service = cluster.getService(sch.getServiceGroupName(), sch.getServiceName());
     if (service != null) {
       ServiceComponent serviceComponent = service.getServiceComponent(sch.getServiceComponentName());
       if (serviceComponent != null) {
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ComponentResourceProvider.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ComponentResourceProvider.java
index c59e51d..64afd34 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ComponentResourceProvider.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ComponentResourceProvider.java
@@ -659,7 +659,7 @@ public class ComponentResourceProvider extends AbstractControllerResourceProvide
       }
       componentNames.get(clusterName).get(serviceName).add(componentName);
 
-      Service s = cluster.getService(serviceName);
+      Service s = cluster.getService(serviceGroupName, serviceName);
       ServiceComponent sc = s.getServiceComponent(componentName);
       State newState = getValidDesiredState(request);
 
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ServiceDependencyResourceProvider.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ServiceDependencyResourceProvider.java
index 3cb7d18..3b8c64a 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ServiceDependencyResourceProvider.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ServiceDependencyResourceProvider.java
@@ -20,11 +20,13 @@ package org.apache.ambari.server.controller.internal;
 
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
 
 import org.apache.ambari.server.AmbariException;
@@ -414,20 +416,15 @@ public class ServiceDependencyResourceProvider extends AbstractControllerResourc
 
     Set<ServiceDependencyResponse> responses = new HashSet<>();
     if (request.getServiceName() != null) {
-      Collection<Service> services = cluster.getServices().values();
-      Service currentService = null;
+      Collection<Service> services = cluster.getServicesById().values();
       for (Service service : services) {
-        if (service.getServiceGroupId() == serviceGroup.getServiceGroupId() &&
+        if (Objects.equals(service.getServiceGroupId(), serviceGroup.getServiceGroupId()) &&
                 service.getName().equals(request.getServiceName())) {
-          currentService = service;
-          break;
+          return new HashSet<>(service.getServiceDependencyResponses());
         }
       }
-
-      responses.addAll(currentService.getServiceDependencyResponses());
-      return responses;
     }
-    return responses;
+    return Collections.emptySet();
   }
 
 
@@ -509,7 +506,7 @@ public class ServiceDependencyResourceProvider extends AbstractControllerResourc
       //throws service group not found exception
       ServiceGroup serviceGroup = cluster.getServiceGroup(serviceGroupName);
       //throws service not found exception
-      Service service = cluster.getService(serviceName);
+      Service service = cluster.getService(serviceGroupName, serviceName);
 
       Cluster dependencyCluster = cluster;
       if (StringUtils.isNotEmpty(dependentClusterName)) {
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ServiceResourceProvider.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ServiceResourceProvider.java
index 98eff23..8931d8a 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ServiceResourceProvider.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ServiceResourceProvider.java
@@ -545,7 +545,7 @@ public class ServiceResourceProvider extends AbstractControllerResourceProvider
 
     Set<ServiceResponse> response = new HashSet<>();
     if (request.getServiceName() != null) {
-      Service s = cluster.getService(request.getServiceName());
+      Service s = cluster.getService(request.getServiceGroupName(), request.getServiceName());
       response.add(s.convertToResponse());
       return response;
     }
@@ -947,9 +947,8 @@ public class ServiceResourceProvider extends AbstractControllerResourceProvider
           throw new AuthorizationException("The user is not authorized to delete services");
         }
 
-        Service service = clusters.getCluster(
-            serviceRequest.getClusterName()).getService(
-          serviceRequest.getServiceName());
+        Service service = clusters.getCluster(serviceRequest.getClusterName())
+          .getService(serviceRequest.getServiceGroupName(), serviceRequest.getServiceName());
 
         //
         // Run through the list of service component hosts. If all host components are in removable state,
@@ -1114,8 +1113,8 @@ public class ServiceResourceProvider extends AbstractControllerResourceProvider
         throw new ParentObjectNotFoundException("Attempted to add a service to a cluster which doesn't exist", e);
       }
       try {
-        Service s = cluster.getService(serviceName);
-        if (s != null && (s.getServiceGroupName().equals(serviceGroupName))) {
+        Service s = cluster.getService(serviceGroupName, serviceName);
+        if (s != null) {
           // throw error later for dup
           duplicates.add(serviceID);
           continue;
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java b/ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java
index 598abe3..6c5678f 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java
@@ -159,6 +159,7 @@ import org.slf4j.LoggerFactory;
 
 import com.google.common.base.Functions;
 import com.google.common.base.Predicate;
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.ListMultimap;
@@ -439,9 +440,8 @@ public class ClusterImpl implements Cluster {
    * We need this for live status checks.
    */
   private void loadServiceHostComponents() {
-    for (Entry<String, Service> serviceKV : services.entrySet()) {
+    for (Service service : servicesById.values()) {
       /* get all the service component hosts **/
-      Service service = serviceKV.getValue();
       if (!serviceComponentHosts.containsKey(service.getName())) {
         serviceComponentHosts.put(service.getName(), new ConcurrentHashMap<>());
       }
@@ -486,11 +486,9 @@ public class ClusterImpl implements Cluster {
       ServiceGroupEntity serviceGroupEntity = serviceEntity.getClusterServiceGroupEntity();
       StackId stackId = new StackId(serviceGroupEntity.getStack());
       try {
-        if (ambariMetaInfo.getService(stackId.getStackName(),
-          stackId.getStackVersion(), serviceEntity.getServiceType()) != null) {
+        if (ambariMetaInfo.getService(stackId.getStackName(), stackId.getStackVersion(), serviceEntity.getServiceType()) != null) {
           Service service = serviceFactory.createExisting(this, getServiceGroup(serviceEntity.getServiceGroupId()), serviceEntity);
           services.put(serviceEntity.getServiceName(), service);
-          stackId = getService(serviceEntity.getServiceName()).getStackId();
           servicesById.put(serviceEntity.getServiceId(), service);
         }
 
@@ -705,7 +703,7 @@ public class ClusterImpl implements Cluster {
     Set<ServiceComponentHostResponse> createdSvcHostCmpnt = new HashSet<>();
     Cluster cluster = null;
     for (ServiceComponentHost serviceComponentHost : serviceComponentHosts) {
-      Service service = getService(serviceComponentHost.getServiceName());
+      Service service = getService(serviceComponentHost.getServiceGroupName(), serviceComponentHost.getServiceName());
       cluster = service.getCluster();
       ServiceComponent serviceComponent = service.getServiceComponent(serviceComponentHost.getServiceComponentName());
       serviceComponent.addServiceComponentHost(serviceComponentHost);
@@ -931,13 +929,13 @@ public class ClusterImpl implements Cluster {
     clusterGlobalLock.writeLock().lock();
     try {
       if (LOG.isDebugEnabled()) {
-        LOG.debug("Adding a new Service, clusterName={}, clusterId={}, serviceName={} serviceType={}",
-            getClusterName(), getClusterId(), service.getName(), service.getServiceType());
+        LOG.debug("Adding a new Service, clusterName={}, clusterId={} serviceGroup={} serviceName={} serviceType={}",
+            getClusterName(), getClusterId(), service.getServiceGroupName(), service.getName(), service.getServiceType());
       }
       //TODO get rid of services map
       services.put(service.getName(), service);
-
       servicesById.put(service.getServiceId(), service);
+
       try {
         loadServiceConfigTypes(service);
       } catch (AmbariException e) {
@@ -1210,7 +1208,7 @@ public class ClusterImpl implements Cluster {
 
   @Override
   public Service getServiceByComponentName(String componentName) throws AmbariException {
-    for (Service service : services.values()) {
+    for (Service service : servicesById.values()) {
       for (ServiceComponent component : service.getServiceComponents().values()) {
         if (component.getName().equals(componentName)) {
           return service;
@@ -1223,7 +1221,7 @@ public class ClusterImpl implements Cluster {
 
   @Override
   public Service getServiceByComponentId(Long componentId) throws AmbariException {
-    for (Service service : services.values()) {
+    for (Service service : servicesById.values()) {
       for (ServiceComponent component : service.getServiceComponents().values()) {
         if (component.getId().equals(componentId)) {
           return service;
@@ -1241,7 +1239,7 @@ public class ClusterImpl implements Cluster {
 
   @Override
   public String getComponentName(Long componentId) throws AmbariException {
-    for (Service service : services.values()) {
+    for (Service service : servicesById.values()) {
       for (ServiceComponent component : service.getServiceComponents().values()) {
         if (component.getId().equals(componentId)) {
           return component.getName();
@@ -1255,7 +1253,7 @@ public class ClusterImpl implements Cluster {
 
   @Override
   public String getComponentType(Long componentId) throws AmbariException {
-    for (Service service : services.values()) {
+    for (Service service : servicesById.values()) {
       for (ServiceComponent component : service.getServiceComponents().values()) {
         if (component.getId().equals(componentId)) {
           return component.getType();
@@ -1268,7 +1266,7 @@ public class ClusterImpl implements Cluster {
 
   @Override
   public Long getComponentId(String componentName) throws AmbariException {
-    for (Service service : services.values()) {
+    for (Service service : servicesById.values()) {
       for (ServiceComponent component : service.getServiceComponents().values()) {
         if (component.getName().equals(componentName)) {
           return component.getId();
@@ -1654,7 +1652,7 @@ public class ClusterImpl implements Cluster {
       getClusterId()).append(", desiredStackVersion=").append(
       desiredStackVersion.getStackId()).append(", services=[ ");
     boolean first = true;
-    for (Service s : services.values()) {
+    for (Service s : servicesById.values()) {
       if (!first) {
         sb.append(" , ");
       }
@@ -1686,7 +1684,7 @@ public class ClusterImpl implements Cluster {
     try {
       LOG.info("Deleting all services for cluster" + ", clusterName="
         + getClusterName());
-      for (Service service : services.values()) {
+      for (Service service : servicesById.values()) {
         if (!service.canBeRemoved()) {
           throw new AmbariException(
             "Found non removable service when trying to"
@@ -1696,7 +1694,7 @@ public class ClusterImpl implements Cluster {
       }
 
       DeleteHostComponentStatusMetaData deleteMetaData = new DeleteHostComponentStatusMetaData();
-      for (Service service : services.values()) {
+      for (Service service : ImmutableList.copyOf(servicesById.values())) {
         deleteService(service, deleteMetaData);
         topologyDeleteFormer.processDeleteMetaDataException(deleteMetaData);
       }
@@ -1887,7 +1885,7 @@ public class ClusterImpl implements Cluster {
     clusterGlobalLock.readLock().lock();
     try {
       boolean safeToRemove = true;
-      for (Service service : services.values()) {
+      for (Service service : servicesById.values()) {
         if (!service.canBeRemoved()) {
           safeToRemove = false;
           LOG.warn("Found non removable service" + ", clusterName="
@@ -2207,10 +2205,6 @@ public class ClusterImpl implements Cluster {
     return getServiceOrNull(resultingServiceIds.get(0));
   }
 
-  private boolean isServiceInstalled(String serviceName) {
-    return services.get(serviceName) != null;
-  }
-
   @Override
   public ServiceConfigVersionResponse setServiceConfigVersion(Long serviceId, Long version, String user, String note) throws AmbariException {
     if (null == user) {
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ComponentResourceProviderTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ComponentResourceProviderTest.java
index b49ae01..c4dc2fe 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ComponentResourceProviderTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ComponentResourceProviderTest.java
@@ -401,6 +401,8 @@ public class ComponentResourceProviderTest {
     expect(clusters.getCluster("Cluster100")).andReturn(cluster).anyTimes();
 
     expect(cluster.getService("Service100")).andReturn(service).anyTimes();
+    String serviceGroupName = "CORE";
+    expect(cluster.getService(serviceGroupName, "Service100")).andReturn(service).anyTimes();
     expect(service.getName()).andReturn("Service100").anyTimes();
     expect(service.getServiceType()).andReturn("Service100").anyTimes();
     expect(service.getServiceComponent("Component101")).andReturn(serviceComponent1).anyTimes();
@@ -427,13 +429,13 @@ public class ComponentResourceProviderTest {
     expect(component3Info.getCategory()).andReturn(null);
 
     expect(serviceComponent1.convertToResponse()).andReturn(
-      new ServiceComponentResponse(100L, "Cluster100", 1L, "", 1L, "Service100", "", 1L, "Component101", "Component101", stackId, "", serviceComponentStateCountMap,
+      new ServiceComponentResponse(100L, "Cluster100", 1L, serviceGroupName, 1L, "Service100", "", 1L, "Component101", "Component101", stackId, "", serviceComponentStateCountMap,
               false /* recovery not enabled */, "Component101 Client", null));
     expect(serviceComponent2.convertToResponse()).andReturn(
-      new ServiceComponentResponse(100L, "Cluster100", 1L, "", 1L, "Service100", "", 2L, "Component102", "Component102",stackId, "", serviceComponentStateCountMap,
+      new ServiceComponentResponse(100L, "Cluster100", 1L, serviceGroupName, 1L, "Service100", "", 2L, "Component102", "Component102",stackId, "", serviceComponentStateCountMap,
               false /* recovery not enabled */, "Component102 Client", null));
     expect(serviceComponent3.convertToResponse()).andReturn(
-      new ServiceComponentResponse(100L, "Cluster100", 1L, "", 1L, "Service100", "", 3L, "Component103", "Component103", stackId, "", serviceComponentStateCountMap,
+      new ServiceComponentResponse(100L, "Cluster100", 1L, serviceGroupName, 1L, "Service100", "", 3L, "Component103", "Component103", stackId, "", serviceComponentStateCountMap,
               false /* recovery not enabled */, "Component103 Client", null));
     expect(serviceComponent1.getDesiredState()).andReturn(State.INSTALLED).anyTimes();
     expect(serviceComponent2.getDesiredState()).andReturn(State.INSTALLED).anyTimes();
@@ -737,7 +739,8 @@ public class ComponentResourceProviderTest {
     expect(cluster.getServices()).andReturn(Collections.singletonMap("Service100", service)).anyTimes();
     expect(cluster.getClusterId()).andReturn(2L).anyTimes();
 
-    expect(cluster.getService("Service100")).andReturn(service).anyTimes();
+    String serviceGroupName = "CORE";
+    expect(cluster.getService(serviceGroupName, "Service100")).andReturn(service).anyTimes();
     expect(service.getName()).andReturn("Service100").anyTimes();
     expect(service.getServiceType()).andReturn("Service100").anyTimes();
     expect(service.getServiceComponent("Component101")).andReturn(serviceComponent1).anyTimes();
@@ -754,7 +757,7 @@ public class ComponentResourceProviderTest {
     expect(component1Info.getCategory()).andReturn(null);
 
     expect(serviceComponent1.convertToResponse()).andReturn(
-        new ServiceComponentResponse(100L, "Cluster100", 1L, "", 1L, "Service100", "", 1L, "Component101", "Component101", stackId, "", serviceComponentStateCountMap,
+        new ServiceComponentResponse(100L, "Cluster100", 1L, serviceGroupName, 1L, "Service100", "", 1L, "Component101", "Component101", stackId, "", serviceComponentStateCountMap,
             false /* recovery not enabled */, "Component101 Client", null));
     expect(serviceComponent1.getDesiredState()).andReturn(State.INSTALLED).anyTimes();
 
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ServiceDependencyResourceProviderTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ServiceDependencyResourceProviderTest.java
index 46b572a..4767f9b 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ServiceDependencyResourceProviderTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ServiceDependencyResourceProviderTest.java
@@ -241,7 +241,7 @@ public class ServiceDependencyResourceProviderTest {
             mapRequestProps, runSmokeTests, reconfigureClients, maintenanceStateHelper);
 
     Assert.assertEquals(State.INSTALLED,
-            clusters.getCluster(clusterName).getService(serviceName)
+            clusters.getCluster(clusterName).getService(serviceGroupName, serviceName)
                     .getDesiredState());
 
     if (resp != null) {
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ServiceResourceProviderTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ServiceResourceProviderTest.java
index e82ab51..1a344c6 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ServiceResourceProviderTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ServiceResourceProviderTest.java
@@ -143,11 +143,12 @@ public class ServiceResourceProviderTest {
     expect(clusters.getCluster("Cluster100")).andReturn(cluster).anyTimes();
 
     expect(cluster.getServicesById()).andReturn(Collections.emptyMap()).anyTimes();
-    expect(cluster.getService("Service100")).andReturn(null);
+    String serviceGroupName = "SERVICE_GROUP";
+    expect(cluster.getService(serviceGroupName, "Service100")).andReturn(null);
     expect(cluster.getDesiredStackVersion()).andReturn(stackId).anyTimes();
     expect(cluster.getClusterId()).andReturn(2L).anyTimes();
 
-    expect(cluster.getServiceGroup("SERVICE_GROUP")).andReturn(serviceGroup);
+    expect(cluster.getServiceGroup(serviceGroupName)).andReturn(serviceGroup);
     expect(cluster.getServiceGroup(1L)).andReturn(serviceGroup);
     expect(serviceGroup.getStackId()).andReturn(stackId).anyTimes();
     expect(service.getServiceGroupId()).andReturn(1L).anyTimes();
@@ -174,7 +175,7 @@ public class ServiceResourceProviderTest {
     // add properties to the request map
     properties.put(ServiceResourceProvider.SERVICE_CLUSTER_NAME_PROPERTY_ID, "Cluster100");
     properties.put(ServiceResourceProvider.SERVICE_SERVICE_NAME_PROPERTY_ID, "Service100");
-    properties.put(ServiceResourceProvider.SERVICE_SERVICE_GROUP_NAME_PROPERTY_ID, "SERVICE_GROUP");
+    properties.put(ServiceResourceProvider.SERVICE_SERVICE_GROUP_NAME_PROPERTY_ID, serviceGroupName);
     properties.put(ServiceResourceProvider.SERVICE_SERVICE_STATE_PROPERTY_ID, "INIT");
 
     propertySet.add(properties);
@@ -206,7 +207,7 @@ public class ServiceResourceProviderTest {
     expect(managementController.getAmbariMetaInfo()).andReturn(ambariMetaInfo).anyTimes();
 
     expect(clusters.getCluster(clusterName)).andReturn(cluster).anyTimes();
-    expect(cluster.getService(anyString())).andReturn(null);
+    expect(cluster.getService(anyString(), anyString())).andReturn(null);
     expect(cluster.getClusterId()).andReturn(2L).anyTimes();
 
     Set<Map<String, Object>> propertySet = new HashSet<>();
@@ -262,7 +263,7 @@ public class ServiceResourceProviderTest {
     expect(managementController.getAmbariMetaInfo()).andReturn(ambariMetaInfo).anyTimes();
 
     expect(clusters.getCluster(clusterName)).andReturn(cluster).anyTimes();
-    expect(cluster.getService(anyString())).andReturn(null);
+    expect(cluster.getService(stackId.getStackName(), serviceName)).andReturn(null);
     expect(cluster.getClusterId()).andReturn(2L).anyTimes();
 
     ServiceGroup serviceGroup = createNiceMock(ServiceGroup.class);
@@ -346,7 +347,6 @@ public class ServiceResourceProviderTest {
     Map<Long, Service> servicesById = ImmutableMap.of(1L, service0, 2L, service1, 3L, service2, 4L, service3, 5L, service4);
     expect(cluster.getServicesById()).andReturn(servicesById).anyTimes();
     expect(cluster.getServices()).andReturn(allResponseMap).anyTimes();
-    expect(cluster.getService("Service102")).andReturn(service2).anyTimes();
     expect(cluster.getService(null, "Service102")).andReturn(service2).anyTimes();
 
     expect(service0.convertToResponse()).andReturn(serviceResponse0).anyTimes();
@@ -465,7 +465,7 @@ public class ServiceResourceProviderTest {
     expect(clusters.getCluster("Cluster100")).andReturn(cluster).anyTimes();
 
     expect(cluster.getServices()).andReturn(allResponseMap).anyTimes();
-    expect(cluster.getService("KERBEROS")).andReturn(service0);
+    expect(cluster.getService(null, "KERBEROS")).andReturn(service0);
 
     expect(service0.convertToResponse()).andReturn(serviceResponse0).anyTimes();
 
@@ -533,7 +533,7 @@ public class ServiceResourceProviderTest {
     expect(clusters.getCluster("Cluster100")).andReturn(cluster).anyTimes();
 
     expect(cluster.getServices()).andReturn(allResponseMap).anyTimes();
-    expect(cluster.getService("KERBEROS")).andReturn(service0);
+    expect(cluster.getService(null, "KERBEROS")).andReturn(service0);
 
     expect(service0.convertToResponse()).andReturn(serviceResponse0).anyTimes();
 
@@ -600,7 +600,7 @@ public class ServiceResourceProviderTest {
     expect(clusters.getCluster("Cluster100")).andReturn(cluster).anyTimes();
 
     expect(cluster.getServices()).andReturn(allResponseMap).anyTimes();
-    expect(cluster.getService("KERBEROS")).andReturn(service0);
+    expect(cluster.getService(null, "KERBEROS")).andReturn(service0);
 
     expect(service0.convertToResponse()).andReturn(serviceResponse0).anyTimes();
 
@@ -669,7 +669,7 @@ public class ServiceResourceProviderTest {
     expect(clusters.getCluster("Cluster100")).andReturn(cluster).anyTimes();
 
     expect(cluster.getServices()).andReturn(allResponseMap).anyTimes();
-    expect(cluster.getService("KERBEROS")).andReturn(service0);
+    expect(cluster.getService(null, "KERBEROS")).andReturn(service0);
 
     expect(service0.convertToResponse()).andReturn(serviceResponse0).anyTimes();
 
@@ -754,7 +754,6 @@ public class ServiceResourceProviderTest {
 
     expect(cluster.getClusterId()).andReturn(2L).anyTimes();
     expect(cluster.getServicesById()).andReturn(ImmutableMap.of(1L, service0)).anyTimes();
-    expect(cluster.getService("Service102")).andReturn(service0).anyTimes();
     expect(cluster.getService(null, "Service102")).andReturn(service0).anyTimes();
 
     expect(service0.getDesiredState()).andReturn(State.INSTALLED).anyTimes();
@@ -876,7 +875,6 @@ public class ServiceResourceProviderTest {
     expect(managementController2.getAmbariMetaInfo()).andReturn(ambariMetaInfo).anyTimes();
 
     expect(cluster.getClusterId()).andReturn(2L).anyTimes();
-    expect(cluster.getService("Service102")).andReturn(service0).anyTimes();
     expect(cluster.getService(null, "Service102")).andReturn(service0).anyTimes();
 
     expect(stackId.getStackId()).andReturn("HDP-2.5").anyTimes();
@@ -999,8 +997,7 @@ public class ServiceResourceProviderTest {
     expect(managementController.getClusters()).andReturn(clusters).anyTimes();
     expect(clusters.getCluster("Cluster100")).andReturn(cluster).anyTimes();
     expect(cluster.getClusterId()).andReturn(2L).anyTimes();
-    expect(cluster.getService(serviceName)).andReturn(service).anyTimes();
-    expect(cluster.getService("SERVICE_GROUP", serviceName)).andReturn(service).anyTimes();
+    expect(cluster.getService(null, serviceName)).andReturn(service).anyTimes();
     expect(service.getDesiredState()).andReturn(State.INSTALLED).anyTimes();
     expect(service.getName()).andReturn(serviceName).anyTimes();
     expect(service.getServiceComponents()).andReturn(new HashMap<>());
@@ -1053,7 +1050,7 @@ public class ServiceResourceProviderTest {
     expect(managementController.getClusters()).andReturn(clusters).anyTimes();
     expect(clusters.getCluster("Cluster100")).andReturn(cluster).anyTimes();
     expect(cluster.getClusterId()).andReturn(2L).anyTimes();
-    expect(cluster.getService(serviceName)).andReturn(service).anyTimes();
+    expect(cluster.getService(null, serviceName)).andReturn(service).anyTimes();
     expect(service.getDesiredState()).andReturn(State.STARTED).anyTimes();
     expect(service.getName()).andReturn(serviceName).anyTimes();
     expect(service.getServiceComponents()).andReturn(new HashMap<>());
@@ -1108,7 +1105,7 @@ public class ServiceResourceProviderTest {
     // set expectations
     expect(managementController.getClusters()).andReturn(clusters).anyTimes();
     expect(clusters.getCluster("Cluster100")).andReturn(cluster).anyTimes();
-    expect(cluster.getService(serviceName)).andReturn(service).anyTimes();
+    expect(cluster.getService(null, serviceName)).andReturn(service).anyTimes();
     expect(service.getDesiredState()).andReturn(State.INSTALLED).anyTimes();
     expect(service.getName()).andReturn(serviceName).anyTimes();
     expect(service.getServiceComponents()).andReturn(scMap);
@@ -1197,7 +1194,7 @@ public class ServiceResourceProviderTest {
     // set expectations
     expect(managementController.getClusters()).andReturn(clusters).anyTimes();
     expect(clusters.getCluster(clusterName)).andReturn(cluster).anyTimes();
-    expect(cluster.getService(serviceName)).andReturn(service).anyTimes();
+    expect(cluster.getService(null, serviceName)).andReturn(service).anyTimes();
     expect(service.getDesiredState()).andReturn(State.STARTED).anyTimes();  // Service is in a non-removable state
     expect(service.getName()).andReturn(serviceName).anyTimes();
     expect(service.getServiceComponents()).andReturn(scMap).anyTimes();

-- 
To stop receiving notification emails like this one, please contact
adoroszlai@apache.org.

[ambari] 03/07: AMBARI-23746. Cannot create same named service in different service groups in same request

Posted by ad...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

adoroszlai pushed a commit to branch branch-feature-AMBARI-14714
in repository https://gitbox.apache.org/repos/asf/ambari.git

commit 0e9317e7e72addc4f5628f766afbec022fd1cbed
Author: Doroszlai, Attila <ad...@apache.org>
AuthorDate: Tue May 29 17:06:58 2018 +0200

    AMBARI-23746. Cannot create same named service in different service groups in same request
---
 .../internal/ServiceResourceProvider.java          |  38 +++----
 .../internal/ServiceResourceProviderTest.java      | 112 ++++++++++++++++++++-
 2 files changed, 129 insertions(+), 21 deletions(-)

diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ServiceResourceProvider.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ServiceResourceProvider.java
index ebaf14c..98eff23 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ServiceResourceProvider.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ServiceResourceProvider.java
@@ -76,6 +76,7 @@ import org.apache.ambari.server.state.State;
 import org.apache.ambari.server.topology.TopologyDeleteFormer;
 import org.apache.commons.lang.StringUtils;
 import org.apache.commons.lang.Validate;
+import org.apache.commons.lang3.tuple.Pair;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -178,8 +179,10 @@ public class ServiceResourceProvider extends AbstractControllerResourceProvider
    */
   @AssistedInject
   public ServiceResourceProvider(
-      @Assisted AmbariManagementController managementController,
-      MaintenanceStateHelper maintenanceStateHelper, TopologyDeleteFormer topologyDeleteFormer) {
+    @Assisted AmbariManagementController managementController,
+    MaintenanceStateHelper maintenanceStateHelper,
+    TopologyDeleteFormer topologyDeleteFormer
+  ) {
     super(Resource.Type.Service, PROPERTY_IDS, KEY_PROPERTY_IDS, managementController);
     this.maintenanceStateHelper = maintenanceStateHelper;
     this.topologyDeleteFormer = topologyDeleteFormer;
@@ -565,7 +568,7 @@ public class ServiceResourceProvider extends AbstractControllerResourceProvider
     if(request.getServiceGroupName() != null){
      clusterServices = cluster.getServicesByServiceGroup(serviceGroupName);
     }else{
-      clusterServices = cluster.getServicesById().values();
+      clusterServices = cluster.getServices().values();
     }
     for (Service s : clusterServices) {
       if (checkDesiredState
@@ -1067,8 +1070,8 @@ public class ServiceResourceProvider extends AbstractControllerResourceProvider
           throws AuthorizationException, AmbariException {
 
     AmbariMetaInfo ambariMetaInfo = getManagementController().getAmbariMetaInfo();
-    Map<String, Set<String>> serviceNames = new HashMap<>();
-    Set<String> duplicates = new HashSet<>();
+    Map<String, Set<Pair<String, String>>> serviceNames = new HashMap<>();
+    Set<Pair<String, String>> duplicates = new HashSet<>();
 
     for (ServiceRequest request : requests) {
       final String clusterName = request.getClusterName();
@@ -1079,24 +1082,21 @@ public class ServiceResourceProvider extends AbstractControllerResourceProvider
       Validate.notNull(serviceGroupName, "Service group name should be provided when creating a service");
       Validate.notEmpty(serviceName, "Service name should be provided when creating a service");
 
-      if (LOG.isDebugEnabled()) {
-        LOG.debug("Received a createService request, clusterId={}, serviceName={}, request={}", clusterName, serviceName, request);
-      }
+      LOG.debug("Received a createService request, clusterId={}, serviceGroupName={}, serviceName={}, request={}",
+        clusterName, serviceGroupName, serviceName, request);
 
       if (!AuthorizationHelper.isAuthorized(ResourceType.CLUSTER, getClusterResourceId(clusterName), RoleAuthorization.SERVICE_ADD_DELETE_SERVICES)) {
         throw new AuthorizationException("The user is not authorized to create services");
       }
 
-      if (!serviceNames.containsKey(clusterName)) {
-        serviceNames.put(clusterName, new HashSet<String>());
-      }
+      Pair<String, String> serviceID = Pair.of(serviceGroupName, serviceName);
+      Set<Pair<String, String>> services = serviceNames.computeIfAbsent(clusterName, __ -> new HashSet<>());
 
-      if (serviceNames.get(clusterName).contains(serviceName)) {
+      if (!services.add(serviceID)) {
         // throw error later for dup
-        duplicates.add(serviceName);
+        duplicates.add(serviceID);
         continue;
       }
-      serviceNames.get(clusterName).add(serviceName);
 
       if (StringUtils.isNotEmpty(request.getDesiredState())) {
         State state = State.valueOf(request.getDesiredState());
@@ -1117,7 +1117,7 @@ public class ServiceResourceProvider extends AbstractControllerResourceProvider
         Service s = cluster.getService(serviceName);
         if (s != null && (s.getServiceGroupName().equals(serviceGroupName))) {
           // throw error later for dup
-          duplicates.add(serviceName);
+          duplicates.add(serviceID);
           continue;
         }
       } catch (ServiceNotFoundException e) {
@@ -1139,10 +1139,10 @@ public class ServiceResourceProvider extends AbstractControllerResourceProvider
       }
 
       // validate the credential store input provided
-      ServiceInfo serviceInfo = ambariMetaInfo.getService(stackId.getStackName(),
+      if (StringUtils.isNotEmpty(request.getCredentialStoreEnabled())) {
+        ServiceInfo serviceInfo = ambariMetaInfo.getService(stackId.getStackName(),
           stackId.getStackVersion(), stackServiceName);
 
-      if (StringUtils.isNotEmpty(request.getCredentialStoreEnabled())) {
         boolean credentialStoreEnabled = Boolean.parseBoolean(request.getCredentialStoreEnabled());
         if (!serviceInfo.isCredentialStoreSupported() && credentialStoreEnabled) {
           throw new IllegalArgumentException("Invalid arguments, cannot enable credential store " +
@@ -1159,8 +1159,8 @@ public class ServiceResourceProvider extends AbstractControllerResourceProvider
     // Validate dups
     if (!duplicates.isEmpty()) {
       String clusterName = requests.iterator().next().getClusterName();
-      String msg = "Attempted to create a service which already exists: "
-              + ", clusterName=" + clusterName  + " serviceName=" + StringUtils.join(duplicates, ",");
+      String msg = "Attempted to create services which already exist: "
+              + ", clusterName=" + clusterName  + " " + StringUtils.join(duplicates, ", ");
 
       throw new DuplicateResourceException(msg);
     }
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ServiceResourceProviderTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ServiceResourceProviderTest.java
index de2152d..e82ab51 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ServiceResourceProviderTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ServiceResourceProviderTest.java
@@ -18,8 +18,10 @@
 
 package org.apache.ambari.server.controller.internal;
 
+import static java.util.Arrays.asList;
 import static org.easymock.EasyMock.anyBoolean;
 import static org.easymock.EasyMock.anyObject;
+import static org.easymock.EasyMock.anyString;
 import static org.easymock.EasyMock.capture;
 import static org.easymock.EasyMock.createMock;
 import static org.easymock.EasyMock.createNiceMock;
@@ -55,6 +57,7 @@ import org.apache.ambari.server.controller.ServiceResponse;
 import org.apache.ambari.server.controller.spi.Predicate;
 import org.apache.ambari.server.controller.spi.Request;
 import org.apache.ambari.server.controller.spi.Resource;
+import org.apache.ambari.server.controller.spi.ResourceAlreadyExistsException;
 import org.apache.ambari.server.controller.spi.ResourceProvider;
 import org.apache.ambari.server.controller.spi.SystemException;
 import org.apache.ambari.server.controller.utilities.PredicateBuilder;
@@ -86,6 +89,7 @@ import org.springframework.security.core.Authentication;
 import org.springframework.security.core.context.SecurityContextHolder;
 
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
 
 /**
  * ServiceResourceProvider tests.
@@ -186,6 +190,111 @@ public class ServiceResourceProviderTest {
   }
 
   @Test
+  public void createServicesInDifferentServiceGroups() throws Exception {
+    String clusterName = "cl1";
+    String serviceName = "HADOOP_CLIENTS";
+    StackId hdpcoreStackId = new StackId("HDPCORE", "1.0.0-b123");
+    StackId odsStackId = new StackId("ODS", "1.0.0-b111");
+
+    AmbariManagementController managementController = createNiceMock(AmbariManagementController.class);
+    Clusters clusters = createNiceMock(Clusters.class);
+    Cluster cluster = createNiceMock(Cluster.class);
+    AmbariMetaInfo ambariMetaInfo = createNiceMock(AmbariMetaInfo.class);
+    ServiceInfo serviceInfo = createNiceMock(ServiceInfo.class);
+
+    expect(managementController.getClusters()).andReturn(clusters).anyTimes();
+    expect(managementController.getAmbariMetaInfo()).andReturn(ambariMetaInfo).anyTimes();
+
+    expect(clusters.getCluster(clusterName)).andReturn(cluster).anyTimes();
+    expect(cluster.getService(anyString())).andReturn(null);
+    expect(cluster.getClusterId()).andReturn(2L).anyTimes();
+
+    Set<Map<String, Object>> propertySet = new HashSet<>();
+    long serviceGroupId = 1;
+    for (StackId stackId : asList(hdpcoreStackId, odsStackId)) {
+      ServiceGroup serviceGroup = createNiceMock(ServiceGroup.class);
+      Service service = createNiceMock(Service.class);
+      ServiceResponse response = createNiceMock(ServiceResponse.class);
+
+      expect(cluster.addService(eq(serviceGroup), eq(serviceName), eq(serviceName))).andReturn(service).anyTimes();
+      expect(cluster.getServiceGroup(stackId.getStackName())).andReturn(serviceGroup).anyTimes();
+      expect(cluster.getServiceGroup(serviceGroupId)).andReturn(serviceGroup).anyTimes();
+      expect(serviceGroup.getStackId()).andReturn(hdpcoreStackId).anyTimes();
+      expect(service.getServiceGroupId()).andReturn(serviceGroupId).anyTimes();
+      expect(service.convertToResponse()).andReturn(response).anyTimes();
+
+      replay(serviceGroup, service, response);
+      ++serviceGroupId;
+
+      propertySet.add(ImmutableMap.of(
+        ServiceResourceProvider.SERVICE_CLUSTER_NAME_PROPERTY_ID, clusterName,
+        ServiceResourceProvider.SERVICE_SERVICE_NAME_PROPERTY_ID, serviceName,
+        ServiceResourceProvider.SERVICE_SERVICE_GROUP_NAME_PROPERTY_ID, stackId.getStackName()
+      ));
+    }
+
+    expect(ambariMetaInfo.isValidService(anyString(), anyString(), anyString())).andReturn(true).anyTimes();
+    expect(ambariMetaInfo.getService(anyString(), anyString(), anyString())).andReturn(serviceInfo).anyTimes();
+
+    // replay
+    replay(managementController, clusters, cluster, ambariMetaInfo, serviceInfo);
+
+    SecurityContextHolder.getContext().setAuthentication(TestAuthenticationFactory.createAdministrator());
+
+    Request request = PropertyHelper.getCreateRequest(propertySet, null);
+    ResourceProvider provider = getServiceProvider(managementController);
+    provider.createResources(request);
+  }
+
+  @Test(expected = ResourceAlreadyExistsException.class)
+  public void createDuplicateServices() throws Exception {
+    String clusterName = "cl1";
+    String serviceName = "SOME_SERVICE";
+    StackId stackId = new StackId("HDPCORE", "1.0.0-b123");
+
+    AmbariManagementController managementController = createNiceMock(AmbariManagementController.class);
+    Clusters clusters = createNiceMock(Clusters.class);
+    Cluster cluster = createNiceMock(Cluster.class);
+    AmbariMetaInfo ambariMetaInfo = createNiceMock(AmbariMetaInfo.class);
+    ServiceInfo serviceInfo = createNiceMock(ServiceInfo.class);
+
+    expect(managementController.getClusters()).andReturn(clusters).anyTimes();
+    expect(managementController.getAmbariMetaInfo()).andReturn(ambariMetaInfo).anyTimes();
+
+    expect(clusters.getCluster(clusterName)).andReturn(cluster).anyTimes();
+    expect(cluster.getService(anyString())).andReturn(null);
+    expect(cluster.getClusterId()).andReturn(2L).anyTimes();
+
+    ServiceGroup serviceGroup = createNiceMock(ServiceGroup.class);
+    Service service = createNiceMock(Service.class);
+
+    expect(cluster.getServiceGroup(stackId.getStackName())).andReturn(serviceGroup).anyTimes();
+    expect(cluster.getServiceGroup(1L)).andReturn(serviceGroup).anyTimes();
+    expect(serviceGroup.getStackId()).andReturn(stackId).anyTimes();
+    expect(service.getServiceGroupId()).andReturn(1L).anyTimes();
+
+    Map<String, Object> serviceRequestProperties = ImmutableMap.of(
+      ServiceResourceProvider.SERVICE_CLUSTER_NAME_PROPERTY_ID, clusterName,
+      ServiceResourceProvider.SERVICE_SERVICE_NAME_PROPERTY_ID, serviceName,
+      ServiceResourceProvider.SERVICE_SERVICE_GROUP_NAME_PROPERTY_ID, stackId.getStackName()
+    );
+    Map<String, Object> almostIdenticalCopy = ImmutableMap.<String, Object>builder().putAll(serviceRequestProperties).put(ServiceResourceProvider.SERVICE_CREDENTIAL_STORE_ENABLED_PROPERTY_ID, "false").build();
+    Set<Map<String, Object>> propertySet = ImmutableSet.of(almostIdenticalCopy, serviceRequestProperties);
+
+    expect(ambariMetaInfo.isValidService(anyString(), anyString(), anyString())).andReturn(true).anyTimes();
+    expect(ambariMetaInfo.getService(anyString(), anyString(), anyString())).andReturn(serviceInfo).anyTimes();
+
+    // replay
+    replay(managementController, clusters, cluster, ambariMetaInfo, serviceInfo, serviceGroup, service);
+
+    SecurityContextHolder.getContext().setAuthentication(TestAuthenticationFactory.createAdministrator());
+
+    Request request = PropertyHelper.getCreateRequest(propertySet, null);
+    ResourceProvider provider = getServiceProvider(managementController);
+    provider.createResources(request);
+  }
+
+  @Test
   public void testGetResourcesAsAdministrator() throws Exception{
     testGetResources(TestAuthenticationFactory.createAdministrator());
   }
@@ -891,7 +1000,7 @@ public class ServiceResourceProviderTest {
     expect(clusters.getCluster("Cluster100")).andReturn(cluster).anyTimes();
     expect(cluster.getClusterId()).andReturn(2L).anyTimes();
     expect(cluster.getService(serviceName)).andReturn(service).anyTimes();
-    expect(cluster.getService(null, serviceName)).andReturn(service).anyTimes();
+    expect(cluster.getService("SERVICE_GROUP", serviceName)).andReturn(service).anyTimes();
     expect(service.getDesiredState()).andReturn(State.INSTALLED).anyTimes();
     expect(service.getName()).andReturn(serviceName).anyTimes();
     expect(service.getServiceComponents()).andReturn(new HashMap<>());
@@ -1152,7 +1261,6 @@ public class ServiceResourceProviderTest {
   @Test
   public void testCheckPropertyIds() throws Exception {
     AmbariManagementController managementController = createMock(AmbariManagementController.class);
-
     AbstractResourceProvider provider = getServiceProvider(managementController);
 
     Set<String> unsupported = provider.checkPropertyIds(

-- 
To stop receiving notification emails like this one, please contact
adoroszlai@apache.org.

[ambari] 04/07: AMBARI-23746. Cannot create same named component in different service groups in same request

Posted by ad...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

adoroszlai pushed a commit to branch branch-feature-AMBARI-14714
in repository https://gitbox.apache.org/repos/asf/ambari.git

commit 096dff5eeb74b7f2f640be293679b36fc0a3f787
Author: Doroszlai, Attila <ad...@apache.org>
AuthorDate: Tue May 29 19:32:03 2018 +0200

    AMBARI-23746. Cannot create same named component in different service groups in same request
---
 .../internal/ComponentResourceProvider.java         | 21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ComponentResourceProvider.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ComponentResourceProvider.java
index 2ffce9d..c59e51d 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ComponentResourceProvider.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ComponentResourceProvider.java
@@ -68,6 +68,7 @@ import org.apache.commons.lang.Validate;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Sets;
 import com.google.inject.Inject;
 import com.google.inject.assistedinject.Assisted;
@@ -393,7 +394,7 @@ public class ComponentResourceProvider extends AbstractControllerResourceProvide
     ServiceComponentFactory serviceComponentFactory = getManagementController().getServiceComponentFactory();
 
     // do all validation checks
-    Map<String, Map<String, Set<String>>> componentNames = new HashMap<>();
+    Map<String, Set<Set<String>>> componentNames = new HashMap<>();
     Set<String> duplicates = new HashSet<>();
 
     for (ServiceComponentRequest request : requests) {
@@ -413,21 +414,16 @@ public class ComponentResourceProvider extends AbstractControllerResourceProvide
 
       debug("Received a createComponent request: {}", request);
 
-      if (!componentNames.containsKey(request.getClusterName())) {
-        componentNames.put(request.getClusterName(), new HashMap<>());
-      }
-
-      Map<String, Set<String>> serviceComponents = componentNames.get(request.getClusterName());
-      if (!serviceComponents.containsKey(request.getServiceName())) {
-        serviceComponents.put(request.getServiceName(), new HashSet<String>());
-      }
+      Set<String> componentID = ImmutableSet.of(request.getServiceGroupName(), request.getServiceName(), request.getComponentName());
+      boolean added = componentNames
+        .computeIfAbsent(request.getClusterName(), __ -> new HashSet<>())
+        .add(componentID);
 
-      if (serviceComponents.get(request.getServiceName()).contains(request.getComponentName())) {
+      if (!added) {
         // throw error later for dup
         duplicates.add(request.toString());
         continue;
       }
-      serviceComponents.get(request.getServiceName()).add(request.getComponentName());
 
       if (StringUtils.isNotEmpty(request.getDesiredState())) {
         Validate.isTrue(State.INIT == State.valueOf(request.getDesiredState()),
@@ -462,9 +458,8 @@ public class ComponentResourceProvider extends AbstractControllerResourceProvide
 
     // Validate dups
     if (!duplicates.isEmpty()) {
-      //Java8 has StringJoiner library but ambari is not on Java8 yet.
       throw new DuplicateResourceException("Attempted to create one or more components which already exist:"
-                            + StringUtils.join(duplicates, ","));
+                            + String.join(", ", duplicates));
     }
 
     // now doing actual work

-- 
To stop receiving notification emails like this one, please contact
adoroszlai@apache.org.

[ambari] 02/07: AMBARI-14714. Fix ServiceResourceProviderTest

Posted by ad...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

adoroszlai pushed a commit to branch branch-feature-AMBARI-14714
in repository https://gitbox.apache.org/repos/asf/ambari.git

commit 938be763156e88c3183dfeb62a432cdfc906a9bb
Author: Doroszlai, Attila <ad...@apache.org>
AuthorDate: Fri May 18 22:02:15 2018 +0200

    AMBARI-14714. Fix ServiceResourceProviderTest
---
 .../internal/ServiceResourceProvider.java          |  6 +--
 .../api/resources/BaseResourceDefinitionTest.java  |  7 ++--
 .../AbstractControllerResourceProviderTest.java    |  7 ++--
 .../internal/AbstractResourceProviderTest.java     |  7 ++--
 .../controller/internal/JMXHostProviderTest.java   | 23 +++++------
 .../internal/ServiceResourceProviderTest.java      | 45 ++++++++++++----------
 6 files changed, 49 insertions(+), 46 deletions(-)

diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ServiceResourceProvider.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ServiceResourceProvider.java
index 72e43f7..ebaf14c 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ServiceResourceProvider.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ServiceResourceProvider.java
@@ -167,8 +167,7 @@ public class ServiceResourceProvider extends AbstractControllerResourceProvider
   @Inject
   private KerberosHelper kerberosHelper;
 
-  @Inject
-  private TopologyDeleteFormer topologyDeleteFormer;
+  private final TopologyDeleteFormer topologyDeleteFormer;
 
   // ----- Constructors ----------------------------------------------------
 
@@ -180,9 +179,10 @@ public class ServiceResourceProvider extends AbstractControllerResourceProvider
   @AssistedInject
   public ServiceResourceProvider(
       @Assisted AmbariManagementController managementController,
-      MaintenanceStateHelper maintenanceStateHelper) {
+      MaintenanceStateHelper maintenanceStateHelper, TopologyDeleteFormer topologyDeleteFormer) {
     super(Resource.Type.Service, PROPERTY_IDS, KEY_PROPERTY_IDS, managementController);
     this.maintenanceStateHelper = maintenanceStateHelper;
+    this.topologyDeleteFormer = topologyDeleteFormer;
 
     setRequiredCreateAuthorizations(EnumSet.of(RoleAuthorization.SERVICE_ADD_DELETE_SERVICES));
     setRequiredUpdateAuthorizations(RoleAuthorization.AUTHORIZATIONS_UPDATE_SERVICE);
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/api/resources/BaseResourceDefinitionTest.java b/ambari-server/src/test/java/org/apache/ambari/server/api/resources/BaseResourceDefinitionTest.java
index 22b55cd..a9a0555 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/api/resources/BaseResourceDefinitionTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/api/resources/BaseResourceDefinitionTest.java
@@ -51,6 +51,7 @@ import org.apache.ambari.server.controller.spi.Resource;
 import org.apache.ambari.server.controller.spi.ResourceProvider;
 import org.apache.ambari.server.events.publishers.AmbariEventPublisher;
 import org.apache.ambari.server.state.Service;
+import org.apache.ambari.server.topology.TopologyDeleteFormer;
 import org.apache.ambari.server.view.ViewRegistry;
 import org.junit.Assert;
 import org.junit.Before;
@@ -88,13 +89,13 @@ public class BaseResourceDefinitionTest {
 
     ResourceProviderFactory factory = createMock(ResourceProviderFactory.class);
     MaintenanceStateHelper maintenanceStateHelper = createNiceMock(MaintenanceStateHelper.class);
+    TopologyDeleteFormer topologyDeleteFormer = createNiceMock(TopologyDeleteFormer.class);
     AmbariManagementController managementController = createMock(AmbariManagementController.class);
 
     expect(maintenanceStateHelper.isOperationAllowed(anyObject(Resource.Type.class),
             anyObject(Service.class))).andReturn(true).anyTimes();
 
-    ResourceProvider serviceResourceProvider = new ServiceResourceProvider(managementController,
-        maintenanceStateHelper);
+    ResourceProvider serviceResourceProvider = new ServiceResourceProvider(managementController, maintenanceStateHelper, topologyDeleteFormer);
 
     expect(
         factory.getServiceResourceProvider(
@@ -102,7 +103,7 @@ public class BaseResourceDefinitionTest {
 
     AbstractControllerResourceProvider.init(factory);
 
-    replay(factory, managementController, maintenanceStateHelper);
+    replay(factory, managementController, maintenanceStateHelper, topologyDeleteFormer);
 
     processor.process(null, serviceNode, "http://c6401.ambari.apache.org:8080/api/v1/clusters/c1/services");
 
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AbstractControllerResourceProviderTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AbstractControllerResourceProviderTest.java
index daac174..5a76e2e 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AbstractControllerResourceProviderTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AbstractControllerResourceProviderTest.java
@@ -30,6 +30,7 @@ import org.apache.ambari.server.controller.MaintenanceStateHelper;
 import org.apache.ambari.server.controller.ResourceProviderFactory;
 import org.apache.ambari.server.controller.spi.Resource;
 import org.apache.ambari.server.controller.spi.ResourceProvider;
+import org.apache.ambari.server.topology.TopologyDeleteFormer;
 import org.junit.Test;
 
 import junit.framework.Assert;
@@ -45,16 +46,16 @@ public class AbstractControllerResourceProviderTest {
     ResourceProviderFactory factory = createMock(ResourceProviderFactory.class);
 
     MaintenanceStateHelper maintenanceStateHelper = createNiceMock(MaintenanceStateHelper.class);
+    TopologyDeleteFormer topologyDeleteFormer = createNiceMock(TopologyDeleteFormer.class);
 
-    ResourceProvider serviceResourceProvider = new ServiceResourceProvider(managementController,
-        maintenanceStateHelper);
+    ResourceProvider serviceResourceProvider = new ServiceResourceProvider(managementController, maintenanceStateHelper, topologyDeleteFormer);
 
     expect(factory.getServiceResourceProvider(managementController)).andReturn(
         serviceResourceProvider);
 
     AbstractControllerResourceProvider.init(factory);
 
-    replay(managementController, factory, maintenanceStateHelper);
+    replay(managementController, factory, maintenanceStateHelper, topologyDeleteFormer);
 
     AbstractResourceProvider provider =
         (AbstractResourceProvider) AbstractControllerResourceProvider.getResourceProvider(
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AbstractResourceProviderTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AbstractResourceProviderTest.java
index 5771337..bcf3279 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AbstractResourceProviderTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AbstractResourceProviderTest.java
@@ -52,6 +52,7 @@ import org.apache.ambari.server.controller.spi.SystemException;
 import org.apache.ambari.server.controller.spi.UnsupportedPropertyException;
 import org.apache.ambari.server.controller.utilities.PredicateBuilder;
 import org.apache.ambari.server.state.SecurityType;
+import org.apache.ambari.server.topology.TopologyDeleteFormer;
 import org.easymock.EasyMock;
 import org.easymock.IArgumentMatcher;
 import org.junit.Assert;
@@ -123,10 +124,10 @@ public class AbstractResourceProviderTest {
   public void testGetRequestStatus() {
     AmbariManagementController managementController = createMock(AmbariManagementController.class);
     MaintenanceStateHelper maintenanceStateHelper = createNiceMock(MaintenanceStateHelper.class);
-    replay(maintenanceStateHelper);
+    TopologyDeleteFormer topologyDeleteFormer = createNiceMock(TopologyDeleteFormer.class);
+    replay(maintenanceStateHelper, topologyDeleteFormer);
 
-    AbstractResourceProvider provider = new ServiceResourceProvider(managementController,
-        maintenanceStateHelper);
+    AbstractResourceProvider provider = new ServiceResourceProvider(managementController, maintenanceStateHelper, topologyDeleteFormer);
 
     RequestStatus status = provider.getRequestStatus(null);
 
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/JMXHostProviderTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/JMXHostProviderTest.java
index d1365e0..ca4a74f 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/JMXHostProviderTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/JMXHostProviderTest.java
@@ -51,6 +51,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.topology.TopologyDeleteFormer;
 import org.junit.After;
 import org.junit.Assert;
 import org.junit.Before;
@@ -554,26 +555,20 @@ public class JMXHostProviderTest {
 
   private static class JMXHostProviderModule extends AbstractProviderModule {
 
+    private final ResourceProvider clusterResourceProvider = new ClusterResourceProvider(controller);
 
-
-    ResourceProvider clusterResourceProvider = new ClusterResourceProvider(controller);
-
-    Injector injector = createNiceMock(Injector.class);
-    MaintenanceStateHelper maintenanceStateHelper = createNiceMock(MaintenanceStateHelper.class);
+    private final Injector injector = createNiceMock(Injector.class);
+    private final MaintenanceStateHelper maintenanceStateHelper = createNiceMock(MaintenanceStateHelper.class);
+    private final TopologyDeleteFormer topologyDeleteFormer = createNiceMock(TopologyDeleteFormer.class);
 
     {
       expect(injector.getInstance(Clusters.class)).andReturn(null);
-      replay(maintenanceStateHelper, injector);
+      replay(maintenanceStateHelper, injector, topologyDeleteFormer);
     }
 
-    ResourceProvider serviceResourceProvider = new ServiceResourceProvider(controller,
-        maintenanceStateHelper);
-
-    ResourceProvider hostCompResourceProvider = new
-      HostComponentResourceProvider(controller, injector);
-
-    ResourceProvider configResourceProvider = new ConfigurationResourceProvider(
-        controller);
+    private final ResourceProvider serviceResourceProvider = new ServiceResourceProvider(controller, maintenanceStateHelper, topologyDeleteFormer);
+    private final ResourceProvider hostCompResourceProvider = new HostComponentResourceProvider(controller, injector);
+    private final ResourceProvider configResourceProvider = new ConfigurationResourceProvider(controller);
 
     JMXHostProviderModule(AmbariManagementController ambariManagementController) {
       super();
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ServiceResourceProviderTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ServiceResourceProviderTest.java
index 718c477..de2152d 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ServiceResourceProviderTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ServiceResourceProviderTest.java
@@ -138,6 +138,7 @@ public class ServiceResourceProviderTest {
 
     expect(clusters.getCluster("Cluster100")).andReturn(cluster).anyTimes();
 
+    expect(cluster.getServicesById()).andReturn(Collections.emptyMap()).anyTimes();
     expect(cluster.getService("Service100")).andReturn(null);
     expect(cluster.getDesiredStackVersion()).andReturn(stackId).anyTimes();
     expect(cluster.getClusterId()).andReturn(2L).anyTimes();
@@ -233,8 +234,11 @@ public class ServiceResourceProviderTest {
 
     expect(clusters.getCluster("Cluster100")).andReturn(cluster).anyTimes();
 
+    Map<Long, Service> servicesById = ImmutableMap.of(1L, service0, 2L, service1, 3L, service2, 4L, service3, 5L, service4);
+    expect(cluster.getServicesById()).andReturn(servicesById).anyTimes();
     expect(cluster.getServices()).andReturn(allResponseMap).anyTimes();
-    expect(cluster.getService("Service102")).andReturn(service2);
+    expect(cluster.getService("Service102")).andReturn(service2).anyTimes();
+    expect(cluster.getService(null, "Service102")).andReturn(service2).anyTimes();
 
     expect(service0.convertToResponse()).andReturn(serviceResponse0).anyTimes();
     expect(service1.convertToResponse()).andReturn(serviceResponse1).anyTimes();
@@ -640,7 +644,9 @@ public class ServiceResourceProviderTest {
     expect(clusters.getCluster("Cluster100")).andReturn(cluster).anyTimes();
 
     expect(cluster.getClusterId()).andReturn(2L).anyTimes();
+    expect(cluster.getServicesById()).andReturn(ImmutableMap.of(1L, service0)).anyTimes();
     expect(cluster.getService("Service102")).andReturn(service0).anyTimes();
+    expect(cluster.getService(null, "Service102")).andReturn(service0).anyTimes();
 
     expect(service0.getDesiredState()).andReturn(State.INSTALLED).anyTimes();
     expect(service0.getServiceComponents()).andReturn(Collections.emptyMap()).anyTimes();
@@ -687,8 +693,7 @@ public class ServiceResourceProviderTest {
 
     SecurityContextHolder.getContext().setAuthentication(authentication);
 
-    ServiceResourceProvider provider = getServiceProvider(managementController,
-        maintenanceStateHelper);
+    ServiceResourceProvider provider = getServiceProvider(managementController);
 
     // add the property map to a set for the request.
     Map<String, Object> properties = new LinkedHashMap<>();
@@ -763,6 +768,7 @@ public class ServiceResourceProviderTest {
 
     expect(cluster.getClusterId()).andReturn(2L).anyTimes();
     expect(cluster.getService("Service102")).andReturn(service0).anyTimes();
+    expect(cluster.getService(null, "Service102")).andReturn(service0).anyTimes();
 
     expect(stackId.getStackId()).andReturn("HDP-2.5").anyTimes();
     expect(stackId.getStackName()).andReturn("HDP").anyTimes();
@@ -821,11 +827,9 @@ public class ServiceResourceProviderTest {
 
     SecurityContextHolder.getContext().setAuthentication(authentication);
 
-    ServiceResourceProvider provider1 = getServiceProvider(managementController1,
-        maintenanceStateHelper);
+    ServiceResourceProvider provider1 = getServiceProvider(managementController1, maintenanceStateHelper, null);
 
-    ServiceResourceProvider provider2 = getServiceProvider(managementController2,
-        maintenanceStateHelper);
+    ServiceResourceProvider provider2 = getServiceProvider(managementController2, maintenanceStateHelper, null);
 
     // add the property map to a set for the request.
     Map<String, Object> properties = new LinkedHashMap<>();
@@ -887,12 +891,13 @@ public class ServiceResourceProviderTest {
     expect(clusters.getCluster("Cluster100")).andReturn(cluster).anyTimes();
     expect(cluster.getClusterId()).andReturn(2L).anyTimes();
     expect(cluster.getService(serviceName)).andReturn(service).anyTimes();
+    expect(cluster.getService(null, serviceName)).andReturn(service).anyTimes();
     expect(service.getDesiredState()).andReturn(State.INSTALLED).anyTimes();
     expect(service.getName()).andReturn(serviceName).anyTimes();
     expect(service.getServiceComponents()).andReturn(new HashMap<>());
     expect(service.getCluster()).andReturn(cluster);
-    expect(cluster.getServiceGroup("SERVICE_GROUP")).andReturn(serviceGroup);
-    expect(cluster.getServiceGroup(1L)).andReturn(serviceGroup);
+    expect(cluster.getServiceGroup("SERVICE_GROUP")).andReturn(serviceGroup).anyTimes();
+    expect(cluster.getServiceGroup(1L)).andReturn(serviceGroup).anyTimes();
     //expect(serviceGroup.getStackId()).andReturn(stackId).anyTimes();
     expect(service.getServiceGroupId()).andReturn(1L).anyTimes();
 
@@ -1148,11 +1153,7 @@ public class ServiceResourceProviderTest {
   public void testCheckPropertyIds() throws Exception {
     AmbariManagementController managementController = createMock(AmbariManagementController.class);
 
-    MaintenanceStateHelper maintenanceStateHelperMock = createNiceMock(MaintenanceStateHelper.class);
-    replay(maintenanceStateHelperMock);
-
-    AbstractResourceProvider provider = new ServiceResourceProvider(managementController,
-        maintenanceStateHelperMock);
+    AbstractResourceProvider provider = getServiceProvider(managementController);
 
     Set<String> unsupported = provider.checkPropertyIds(
         Collections.singleton(ServiceResourceProvider.SERVICE_CLUSTER_NAME_PROPERTY_ID));
@@ -1322,8 +1323,11 @@ public class ServiceResourceProviderTest {
     expect(maintenanceStateHelperMock.isOperationAllowed(anyObject(Resource.Type.class), anyObject(Service.class))).andReturn(true).anyTimes();
     expect(maintenanceStateHelperMock.isOperationAllowed(anyObject(Resource.Type.class), anyObject(ServiceComponentHost.class))).andReturn(true).anyTimes();
 
-    replay(maintenanceStateHelperMock);
-    return getServiceProvider(managementController, maintenanceStateHelperMock);
+    TopologyDeleteFormer topologyDeleteFormer = createNiceMock(TopologyDeleteFormer.class);
+
+    replay(maintenanceStateHelperMock, topologyDeleteFormer);
+
+    return getServiceProvider(managementController, maintenanceStateHelperMock, topologyDeleteFormer);
   }
 
   /**
@@ -1331,15 +1335,16 @@ public class ServiceResourceProviderTest {
    */
   public static ServiceResourceProvider getServiceProvider(
       AmbariManagementController managementController,
-      MaintenanceStateHelper maintenanceStateHelper) {
-    return new ServiceResourceProvider(managementController, maintenanceStateHelper);
+      MaintenanceStateHelper maintenanceStateHelper, TopologyDeleteFormer topologyDeleteFormer) {
+    return new ServiceResourceProvider(managementController, maintenanceStateHelper, topologyDeleteFormer);
 
   }
 
   public static void createServices(AmbariManagementController controller,Set<ServiceRequest> requests)
       throws AmbariException, AuthorizationException, NoSuchFieldException, IllegalAccessException {
     MaintenanceStateHelper maintenanceStateHelperMock = createNiceMock(MaintenanceStateHelper.class);
-    ServiceResourceProvider provider = getServiceProvider(controller, maintenanceStateHelperMock);
+    TopologyDeleteFormer topologyDeleteFormer = createNiceMock(TopologyDeleteFormer.class);
+    ServiceResourceProvider provider = getServiceProvider(controller, maintenanceStateHelperMock, topologyDeleteFormer);
     provider.createServices(requests);
   }
 
@@ -1370,7 +1375,7 @@ public class ServiceResourceProviderTest {
       throws AmbariException, AuthorizationException, NoSuchFieldException, IllegalAccessException {
     ServiceResourceProvider provider;
     if (maintenanceStateHelper != null) {
-      provider = getServiceProvider(controller, maintenanceStateHelper);
+      provider = getServiceProvider(controller, maintenanceStateHelper, null);
     } else {
       provider = getServiceProvider(controller);
     }

-- 
To stop receiving notification emails like this one, please contact
adoroszlai@apache.org.

[ambari] 01/07: AMBARI-14714. Fix some unit tests

Posted by ad...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

adoroszlai pushed a commit to branch branch-feature-AMBARI-14714
in repository https://gitbox.apache.org/repos/asf/ambari.git

commit 34900292d43df9d273fdfae7075cb394428d5a80
Author: Doroszlai, Attila <ad...@apache.org>
AuthorDate: Fri May 18 22:01:55 2018 +0200

    AMBARI-14714. Fix some unit tests
---
 .../actionmanager/ExecutionCommandWrapper.java     | 10 ++++----
 .../apache/ambari/server/agent/CommandReport.java  |  5 ++++
 .../api/query/render/ClusterBlueprintRenderer.java |  8 +++++--
 .../ambari/server/api/services/AmbariMetaInfo.java |  5 ++--
 .../org/apache/ambari/server/orm/dao/MpackDAO.java |  2 +-
 .../apache/ambari/server/state/ConfigHelper.java   |  7 ++++--
 .../ambari/server/state/cluster/ClusterImpl.java   |  7 +++---
 .../stack/upgrade/RepositoryVersionHelper.java     |  2 +-
 .../actionmanager/ExecutionCommandWrapperTest.java |  8 +++++++
 .../actionmanager/TestActionDBAccessorImpl.java    |  3 +++
 .../server/actionmanager/TestActionScheduler.java  | 21 ++++++++---------
 .../TestActionSchedulerThreading.java              |  4 +---
 .../ambari/server/agent/AgentResourceTest.java     |  2 ++
 .../server/agent/HeartbeatProcessorTest.java       |  1 +
 .../ambari/server/agent/TestHeartbeatHandler.java  |  5 ++++
 .../ambari/server/agent/TestHeartbeatMonitor.java  |  1 +
 .../query/render/ClusterBlueprintRendererTest.java |  8 +++----
 .../api/services/HostComponentServiceTest.java     |  6 -----
 .../AmbariCustomCommandExecutionHelperTest.java    | 27 +++++++++-------------
 .../HostComponentResourceProviderTest.java         | 10 ++++----
 .../internal/HostResourceProviderTest.java         | 17 +++++++++++++-
 .../controller/internal/JMXHostProviderTest.java   |  8 +++----
 .../apache/ambari/server/orm/OrmTestHelper.java    |  2 ++
 .../HDP/2.0.6/services/HADOOP_CLIENTS/metainfo.xml |  5 ++++
 24 files changed, 106 insertions(+), 68 deletions(-)

diff --git a/ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ExecutionCommandWrapper.java b/ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ExecutionCommandWrapper.java
index cc021c3..4898db8 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ExecutionCommandWrapper.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ExecutionCommandWrapper.java
@@ -211,9 +211,9 @@ public class ExecutionCommandWrapper {
       ServiceGroupEntity serviceGroupEntity = serviceGroupDAO.find(clusterId,
           executionCommand.getServiceGroupName());
 
-      long mpackId = serviceGroupEntity.getStack().getMpackId();
+      StackEntity stack = serviceGroupEntity.getStack();
+      Long mpackId = stack.getMpackId();
       Mpack mpack = ambariMetaInfo.getMpack(mpackId);
-      MpackEntity mpackEntity = mpackDAO.findById(mpackId);
 
       if (null == executionCommand.getMpackId()) {
         executionCommand.setMpackId(mpackId);
@@ -221,10 +221,10 @@ public class ExecutionCommandWrapper {
 
       // setting repositoryFile
       final Host host = cluster.getHost(executionCommand.getHostname());  // can be null on internal commands
-      if (null == executionCommand.getRepositoryFile() && null != host) {
-        final CommandRepository commandRepository;
+      if (null == executionCommand.getRepositoryFile() && null != host && mpackId != null) {
+        MpackEntity mpackEntity = mpackDAO.findById(mpackId);
         RepoOsEntity osEntity = repoVersionHelper.getOSEntityForHost(mpackEntity, host);
-        commandRepository = repoVersionHelper.getCommandRepository(mpack, osEntity);
+        final CommandRepository commandRepository = repoVersionHelper.getCommandRepository(mpack, osEntity);
         executionCommand.setRepositoryFile(commandRepository);
       }
 
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/agent/CommandReport.java b/ambari-server/src/main/java/org/apache/ambari/server/agent/CommandReport.java
index 423c249..9fbcb81 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/agent/CommandReport.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/agent/CommandReport.java
@@ -193,6 +193,11 @@ public class CommandReport {
     return clusterId;
   }
 
+  @com.fasterxml.jackson.annotation.JsonProperty("clusterId")
+  public void setClusterId(String clusterId) {
+    this.clusterId = clusterId;
+  }
+
   /**
    * @param tags the config tags that match this command
    */
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRenderer.java b/ambari-server/src/main/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRenderer.java
index c7ad7e3..8ab41f4 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRenderer.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRenderer.java
@@ -53,6 +53,7 @@ import org.apache.ambari.server.controller.internal.ExportBlueprintRequest;
 import org.apache.ambari.server.controller.internal.HostComponentResourceProvider;
 import org.apache.ambari.server.controller.internal.RequestImpl;
 import org.apache.ambari.server.controller.internal.ResourceImpl;
+import org.apache.ambari.server.controller.internal.ServiceGroupResourceProvider;
 import org.apache.ambari.server.controller.spi.ClusterController;
 import org.apache.ambari.server.controller.spi.NoSuchParentResourceException;
 import org.apache.ambari.server.controller.spi.NoSuchResourceException;
@@ -115,7 +116,10 @@ public class ClusterBlueprintRenderer extends BaseRenderer implements Renderer {
 
     ensureChild(resultTree, Resource.Type.ClusterSetting);
 
-    TreeNode<Set<String>> serviceGroupNode = ensureChild(resultTree, Resource.Type.ServiceGroup);
+    TreeNode<Set<String>> serviceGroupNode = ensureChild(resultTree, Resource.Type.ServiceGroup,
+      ServiceGroupResourceProvider.SERVICE_GROUP_MPACK_NAME,
+      ServiceGroupResourceProvider.SERVICE_GROUP_MPACK_VERSION
+    );
     TreeNode<Set<String>> serviceNode = ensureChild(serviceGroupNode, Resource.Type.Service);
     ensureChild(serviceNode, Resource.Type.Component,
       ComponentResourceProvider.COMPONENT_CLUSTER_NAME_PROPERTY_ID,
@@ -247,7 +251,7 @@ public class ClusterBlueprintRenderer extends BaseRenderer implements Renderer {
     // TODO: find a way to add mpack uri
     List<Map<String, Object>> mpackInstances = clusterNode.getChild("servicegroups").getChildren().stream().map(
       child -> {
-        Map<String, Object> serviceGroupProps = child.getObject().getPropertiesMap().get("ServiceGroupInfo");
+        Map<String, Object> serviceGroupProps = child.getObject().getPropertiesMap().get(ServiceGroupResourceProvider.RESPONSE_KEY);
         return ImmutableMap.of(
           "name", serviceGroupProps.get("mpack_name"),
           "version", serviceGroupProps.get("mpack_version"));
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariMetaInfo.java b/ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariMetaInfo.java
index 89e6940..3729b64 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariMetaInfo.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariMetaInfo.java
@@ -1612,8 +1612,9 @@ public class AmbariMetaInfo {
    * @return a single mpack
    */
   public Mpack getMpack(Long mpackId) {
-    if (mpackManager.getMpackMap() != null && mpackManager.getMpackMap().containsKey(mpackId)) {
-      return mpackManager.getMpackMap().get(mpackId);
+    Map<Long, Mpack> mpacks = mpackManager.getMpackMap();
+    if (mpackId != null && mpacks != null) {
+      return mpacks.get(mpackId);
     }
     return null;
   }
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/orm/dao/MpackDAO.java b/ambari-server/src/main/java/org/apache/ambari/server/orm/dao/MpackDAO.java
index 8e12e93..b21e63c 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/orm/dao/MpackDAO.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/orm/dao/MpackDAO.java
@@ -65,7 +65,7 @@ public class MpackDAO extends CrudDAO<MpackEntity, Long> {
    * @return the mpack or {@code null} if none exists.
    */
   @RequiresSession
-  public MpackEntity findById(long id) {
+  public MpackEntity findById(Long id) {
     return m_entityManagerProvider.get().find(MpackEntity.class, id);
   }
 
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/state/ConfigHelper.java b/ambari-server/src/main/java/org/apache/ambari/server/state/ConfigHelper.java
index b69243d..f0b53c8 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/state/ConfigHelper.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/state/ConfigHelper.java
@@ -1701,12 +1701,15 @@ public class ConfigHelper {
     for (Map.Entry<String, String> currentConfig : currentConfigs.entrySet()) {
       String type = currentConfig.getKey();
       String tag = currentConfig.getValue();
-      Collection<String> changedKeys;
+      Collection<String> changedKeys = Collections.emptySet();
       if (previousConfigs.containsKey(type)) {
         changedKeys = findChangedKeys(cluster, type, Collections.singletonList(tag),
             Collections.singletonList(previousConfigs.get(type)));
       } else {
-        changedKeys = cluster.getConfig(type, tag).getProperties().keySet();
+        Config config = cluster.getConfig(type, tag);
+        if (config != null) {
+          changedKeys = config.getProperties().keySet();
+        }
       }
       if (CollectionUtils.isNotEmpty(changedKeys)) {
         changedConfigs.put(type, changedKeys);
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java b/ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java
index edd9fcf..598abe3 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java
@@ -2497,10 +2497,9 @@ public class ClusterImpl implements Cluster {
 
   //TODO this needs to be reworked to support multiple instance of same service
   @Transactional
-  ServiceConfigVersionResponse applyConfigs(Set<Config> configs, String user, String serviceConfigVersionNote) throws AmbariException{
-
-List<ClusterConfigEntity> appliedConfigs = new ArrayList<>();
-Long serviceName = getServiceForConfigTypes( configs.stream().map(Config::getType).collect(toList()));
+  ServiceConfigVersionResponse applyConfigs(Set<Config> configs, String user, String serviceConfigVersionNote) throws AmbariException {
+    List<ClusterConfigEntity> appliedConfigs = new ArrayList<>();
+    Long serviceName = getServiceForConfigTypes( configs.stream().map(Config::getType).collect(toList()));
     Long resultingServiceId = null;
     for (Config config : configs) {
       for (Long serviceId : serviceConfigTypes.keySet()) {
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/RepositoryVersionHelper.java b/ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/RepositoryVersionHelper.java
index 9630ef4..c19f9a5 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/RepositoryVersionHelper.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/RepositoryVersionHelper.java
@@ -329,7 +329,7 @@ public class RepositoryVersionHelper {
 
     long serviceGroupId = component.getServiceGroupId();
     ServiceGroup serviceGroup = cluster.getServiceGroup(serviceGroupId);
-    long mpackId = serviceGroup.getMpackId();
+    Long mpackId = serviceGroup.getMpackId();
     MpackEntity mpackEntity = mpackDAO.findById(mpackId);
     Mpack mpack = ambariMetaInfo.getMpack(mpackId);
 
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/actionmanager/ExecutionCommandWrapperTest.java b/ambari-server/src/test/java/org/apache/ambari/server/actionmanager/ExecutionCommandWrapperTest.java
index 0a7b81f..f9446bc 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/actionmanager/ExecutionCommandWrapperTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/actionmanager/ExecutionCommandWrapperTest.java
@@ -51,6 +51,7 @@ import org.codehaus.jettison.json.JSONException;
 import org.junit.AfterClass;
 import org.junit.Assert;
 import org.junit.BeforeClass;
+import org.junit.Ignore;
 import org.junit.Test;
 
 import com.google.common.collect.Lists;
@@ -204,6 +205,7 @@ public class ExecutionCommandWrapperTest {
     executionCommand.setConfigurations(confs);
     executionCommand.setConfigurationTags(confTags);
     executionCommand.setServiceName("HDFS");
+    executionCommand.setServiceGroupName("CORE");
     executionCommand.setCommandType(AgentCommandType.EXECUTION_COMMAND);
     executionCommand.setCommandParams(Collections.emptyMap());
 
@@ -280,6 +282,7 @@ public class ExecutionCommandWrapperTest {
    * @throws JSONException
    * @throws AmbariException
    */
+  @Ignore // need test mpack
   @Test
   public void testExecutionCommandHasVersionInfo()
       throws JSONException, AmbariException {
@@ -303,6 +306,7 @@ public class ExecutionCommandWrapperTest {
     executionCommand.setRoleParams(Collections.<String, String>emptyMap());
     executionCommand.setRoleCommand(RoleCommand.INSTALL);
     executionCommand.setServiceName("HDFS");
+    executionCommand.setServiceGroupName("CORE");
     executionCommand.setCommandType(AgentCommandType.EXECUTION_COMMAND);
     executionCommand.setCommandParams(commandParams);
 
@@ -326,6 +330,7 @@ public class ExecutionCommandWrapperTest {
     executionCommand.setRoleParams(Collections.<String, String> emptyMap());
     executionCommand.setRoleCommand(RoleCommand.START);
     executionCommand.setServiceName("HDFS");
+    executionCommand.setServiceGroupName("CORE");
     executionCommand.setCommandType(AgentCommandType.EXECUTION_COMMAND);
     executionCommand.setCommandParams(commandParams);
 
@@ -341,6 +346,7 @@ public class ExecutionCommandWrapperTest {
   /**
    * Test that the execution command wrapper ignores repository file when there are none to use.
    */
+  @Ignore // need test mpack
   @Test
   public void testExecutionCommandNoRepositoryFile() throws Exception {
     Cluster cluster = clusters.getCluster(CLUSTER1);
@@ -365,6 +371,7 @@ public class ExecutionCommandWrapperTest {
     executionCommand.setRoleParams(Collections.<String, String>emptyMap());
     executionCommand.setRoleCommand(RoleCommand.INSTALL);
     executionCommand.setServiceName("HDFS");
+    executionCommand.setServiceGroupName("CORE");
     executionCommand.setCommandType(AgentCommandType.EXECUTION_COMMAND);
     executionCommand.setCommandParams(commandParams);
 
@@ -389,6 +396,7 @@ public class ExecutionCommandWrapperTest {
     executionCommand.setRoleParams(Collections.<String, String> emptyMap());
     executionCommand.setRoleCommand(RoleCommand.START);
     executionCommand.setServiceName("HDFS");
+    executionCommand.setServiceGroupName("CORE");
     executionCommand.setCommandType(AgentCommandType.EXECUTION_COMMAND);
     executionCommand.setCommandParams(commandParams);
 
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/actionmanager/TestActionDBAccessorImpl.java b/ambari-server/src/test/java/org/apache/ambari/server/actionmanager/TestActionDBAccessorImpl.java
index 9e4ce97..fd4d8b0 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/actionmanager/TestActionDBAccessorImpl.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/actionmanager/TestActionDBAccessorImpl.java
@@ -48,6 +48,7 @@ import org.apache.ambari.server.orm.dao.ExecutionCommandDAO;
 import org.apache.ambari.server.orm.dao.HostRoleCommandDAO;
 import org.apache.ambari.server.orm.entities.HostRoleCommandEntity;
 import org.apache.ambari.server.serveraction.MockServerAction;
+import org.apache.ambari.server.state.Cluster;
 import org.apache.ambari.server.state.Clusters;
 import org.apache.ambari.server.state.StackId;
 import org.apache.ambari.server.state.svccomphost.ServiceComponentHostStartEvent;
@@ -117,6 +118,8 @@ public class TestActionDBAccessorImpl {
 
     StackId stackId = new StackId("HDP-0.1");
     clusters.addCluster(clusterName, stackId);
+    Cluster cluster = clusters.getCluster(clusterName);
+    cluster.addServiceGroup("core", stackId);
     db = injector.getInstance(ActionDBAccessorImpl.class);
 
     am = injector.getInstance(ActionManager.class);
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/actionmanager/TestActionScheduler.java b/ambari-server/src/test/java/org/apache/ambari/server/actionmanager/TestActionScheduler.java
index 1f1c102..330d651 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/actionmanager/TestActionScheduler.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/actionmanager/TestActionScheduler.java
@@ -480,7 +480,8 @@ public class TestActionScheduler {
 
     //Small action timeout to test rescheduling
     ActionScheduler scheduler = new ActionScheduler(100, 0, db, fsm, 3,
-        new HostsMap((String) null), unitOfWork, eventPublisher, conf, entityManagerProviderMock, hostRoleCommandDAOMock, null,null);
+      new HostsMap((String) null), unitOfWork, eventPublisher, conf, entityManagerProviderMock, hostRoleCommandDAOMock, null,
+      agentCommandsPublisher);
     scheduler.setTaskTimeoutAdjustment(false);
     // Start the thread
 
@@ -986,20 +987,16 @@ public class TestActionScheduler {
     Stage s = getStageWithServerAction(1, 977, null, "test", 2, false, false);
     s.setHostRoleStatus(null, Role.AMBARI_SERVER_ACTION.toString(), HostRoleStatus.IN_PROGRESS);
 
-    ActionScheduler scheduler = EasyMock.createMockBuilder(ActionScheduler.class)
-      .withConstructor(long.class, long.class, ActionDBAccessor.class, Clusters.class, int.class,
-            HostsMap.class, UnitOfWork.class, CommandReportEventPublisher.class, Configuration.class,
-            Provider.class, HostRoleCommandDAO.class, HostRoleCommandFactory.class)
-      .withArgs(100L, 50L, null, null, null, -1, null, null, eventPublisher, null, entityManagerProviderMock,
-            mock(HostRoleCommandDAO.class), mock(HostRoleCommandFactory.class))
-      .createNiceMock();
+    HostRoleCommandDAO hostRoleCommandDAO = createNiceMock(HostRoleCommandDAO.class);
+    AgentCommandsPublisher agentCommandsPublisher = createNiceMock(AgentCommandsPublisher.class);
+    ActionScheduler scheduler = new ActionScheduler(
+      100L, 50L, null, null, -1, null, null,
+      eventPublisher, null, entityManagerProviderMock, hostRoleCommandDAO, hostRoleCommandFactory, agentCommandsPublisher);
 
-    EasyMock.replay(scheduler);
+    EasyMock.replay(hostRoleCommandDAO, agentCommandsPublisher);
 
     // currentTime should be set to -1 and taskTimeout to 1 because it is needed for timeOutActionNeeded method will return false value
-    Assert.assertEquals(false, scheduler.timeOutActionNeeded(HostRoleStatus.IN_PROGRESS, s, null, Role.AMBARI_SERVER_ACTION.toString(), -1L, 1L));
-
-    EasyMock.verify(scheduler);
+    assertFalse(scheduler.timeOutActionNeeded(HostRoleStatus.IN_PROGRESS, s, null, Role.AMBARI_SERVER_ACTION.toString(), -1L, 1L));
   }
 
   @Test
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/actionmanager/TestActionSchedulerThreading.java b/ambari-server/src/test/java/org/apache/ambari/server/actionmanager/TestActionSchedulerThreading.java
index e25b380..07d7ebd 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/actionmanager/TestActionSchedulerThreading.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/actionmanager/TestActionSchedulerThreading.java
@@ -47,6 +47,7 @@ import org.apache.ambari.server.state.Service;
 import org.apache.ambari.server.state.ServiceGroup;
 import org.apache.ambari.server.state.StackId;
 import org.junit.After;
+import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 
@@ -56,8 +57,6 @@ import com.google.inject.Inject;
 import com.google.inject.Injector;
 import com.google.inject.persist.UnitOfWork;
 
-import junit.framework.Assert;
-
 /**
  * Tests {@link ActionScheduler}, focusing on multi-threaded concerns.
  */
@@ -144,7 +143,6 @@ public class TestActionSchedulerThreading {
     // check desired config
     Map<String, DesiredConfig> desiredConfigs = cluster.getDesiredConfigs();
     DesiredConfig desiredConfig = desiredConfigs.get(configType);
-    desiredConfig = desiredConfigs.get(configType);
     assertNotNull(desiredConfig);
     assertEquals(Long.valueOf(2), desiredConfig.getVersion());
     assertEquals("version-2", desiredConfig.getTag());
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/agent/AgentResourceTest.java b/ambari-server/src/test/java/org/apache/ambari/server/agent/AgentResourceTest.java
index 48a208d..54fc7ad 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/agent/AgentResourceTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/agent/AgentResourceTest.java
@@ -55,6 +55,7 @@ import org.apache.ambari.server.mpack.MpackManagerFactory;
 import org.apache.ambari.server.orm.DBAccessor;
 import org.apache.ambari.server.orm.dao.HostDAO;
 import org.apache.ambari.server.orm.dao.HostRoleCommandDAO;
+import org.apache.ambari.server.registry.RegistryManager;
 import org.apache.ambari.server.resources.RootLevelSettingsManagerFactory;
 import org.apache.ambari.server.scheduler.ExecutionScheduler;
 import org.apache.ambari.server.scheduler.ExecutionSchedulerImpl;
@@ -367,6 +368,7 @@ public class AgentResourceTest extends RandomPortJerseyTest {
       bind(ComponentResolver.class).to(StackComponentResolver.class);
       bind(BlueprintValidator.class).to(BasicBlueprintValidator.class);
       bind(StackFactory.class).to(DefaultStackFactory.class);
+      bind(RegistryManager.class).toInstance(createNiceMock(RegistryManager.class));
     }
 
     private void installDependencies() {
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/agent/HeartbeatProcessorTest.java b/ambari-server/src/test/java/org/apache/ambari/server/agent/HeartbeatProcessorTest.java
index 4a2536e..d738a42 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/agent/HeartbeatProcessorTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/agent/HeartbeatProcessorTest.java
@@ -1114,6 +1114,7 @@ public class HeartbeatProcessorTest {
     cmdReport.setRoleCommand(RoleCommand.ACTIONEXECUTE.name());
     cmdReport.setStatus(HostRoleStatus.COMPLETED.name());
     cmdReport.setRole("install_packages");
+    cmdReport.setClusterId("1");
 
     List<CommandReport> reports = new ArrayList<>();
     reports.add(cmdReport);
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/agent/TestHeartbeatHandler.java b/ambari-server/src/test/java/org/apache/ambari/server/agent/TestHeartbeatHandler.java
index 2b196b8..89b0d06 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/agent/TestHeartbeatHandler.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/agent/TestHeartbeatHandler.java
@@ -819,6 +819,7 @@ public class TestHeartbeatHandler {
     cr.setStdErr("none");
     cr.setStdOut("dummy output");
     cr.setExitCode(777);
+    cr.setClusterId("1");
     reports.add(cr);
     hb.setReports(reports);
     hb.setComponentStatus(new ArrayList<>());
@@ -885,6 +886,7 @@ public class TestHeartbeatHandler {
     cr.setStdErr("none");
     cr.setStdOut("dummy output");
     cr.setExitCode(777);
+    cr.setClusterId("1");
     reports.add(cr);
     hb.setReports(reports);
     hb.setComponentStatus(new ArrayList<>());
@@ -1232,6 +1234,7 @@ public class TestHeartbeatHandler {
     cr1.setStdErr("");
     cr1.setStdOut("");
     cr1.setExitCode(215);
+    cr1.setClusterId("1");
     cr1.setRoleCommand("STOP");
     //cr1.setClusterName(DummyCluster);
     ArrayList<CommandReport> reports = new ArrayList<>();
@@ -1257,6 +1260,7 @@ public class TestHeartbeatHandler {
     cr1.setStdErr("none");
     cr1.setStdOut("dummy output");
     cr1.setExitCode(0);
+    cr1.setClusterId("1");
     CommandReport cr2 = new CommandReport();
     cr2.setActionId(StageUtils.getActionId(requestId, stageId));
     cr2.setTaskId(2);
@@ -1268,6 +1272,7 @@ public class TestHeartbeatHandler {
     cr2.setStdErr("none");
     cr2.setStdOut("dummy output");
     cr2.setExitCode(0);
+    cr2.setClusterId("1");
 
     ArrayList<CommandReport> reports = new ArrayList<>();
     reports.add(cr1);
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/agent/TestHeartbeatMonitor.java b/ambari-server/src/test/java/org/apache/ambari/server/agent/TestHeartbeatMonitor.java
index 60b28ab..9ee133b 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/agent/TestHeartbeatMonitor.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/agent/TestHeartbeatMonitor.java
@@ -144,6 +144,7 @@ public class TestHeartbeatMonitor {
     hm.shutdown();
   }
 
+  @Ignore // need test mpack (a stack with mpackId)
   @Test
   public void testStateCommandsGeneration() throws AmbariException, InterruptedException,
           InvalidStateTransitionException {
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRendererTest.java b/ambari-server/src/test/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRendererTest.java
index 5298d1f..b881b9d 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRendererTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRendererTest.java
@@ -287,8 +287,8 @@ public class ClusterBlueprintRendererTest {
     serviceGroupResource.setProperty("ServiceGroupInfo/cluster_name", "c1");
     serviceGroupResource.setProperty("ServiceGroupInfo/service_group_id", "1");
     serviceGroupResource.setProperty("ServiceGroupInfo/service_group_name", "core");
-    serviceGroupResource.setProperty("ServiceGroupInfo/mpack_name", "HDP");
-    serviceGroupResource.setProperty("ServiceGroupInfo/mpack_version", "1.3.3");
+    serviceGroupResource.setProperty("ServiceGroupInfo/mpack_name", STACK_ID.getStackName());
+    serviceGroupResource.setProperty("ServiceGroupInfo/mpack_version", STACK_ID.getStackVersion());
     TreeNode<Resource> serviceGroup1Tree = serviceGroupsTree.addChild(serviceGroupResource, "ServiceGroup:1");
     clusterTree.addChild(serviceGroupsTree);
 
@@ -712,8 +712,8 @@ public class ClusterBlueprintRendererTest {
     serviceGroupResource.setProperty("ServiceGroupInfo/cluster_name", "c1");
     serviceGroupResource.setProperty("ServiceGroupInfo/service_group_id", "1");
     serviceGroupResource.setProperty("ServiceGroupInfo/service_group_name", "core");
-    serviceGroupResource.setProperty("ServiceGroupInfo/mpack_name", "HDP");
-    serviceGroupResource.setProperty("ServiceGroupInfo/mpack_version", "1.3.3");
+    serviceGroupResource.setProperty("ServiceGroupInfo/mpack_name", STACK_ID.getStackName());
+    serviceGroupResource.setProperty("ServiceGroupInfo/mpack_version", STACK_ID.getStackVersion());
     TreeNode<Resource> serviceGroup1Tree = serviceGroupsTree.addChild(serviceGroupResource, "ServiceGroup:1");
     clusterTree.addChild(serviceGroupsTree);
 
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/api/services/HostComponentServiceTest.java b/ambari-server/src/test/java/org/apache/ambari/server/api/services/HostComponentServiceTest.java
index ffe4f40..1ca5969 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/api/services/HostComponentServiceTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/api/services/HostComponentServiceTest.java
@@ -52,12 +52,6 @@ public class HostComponentServiceTest extends BaseServiceTest {
     args = new Object[] {null, getHttpHeaders(), getUriInfo(), null};
     listInvocations.add(new ServiceTestInvocation(Request.Type.GET, componentService, m, args, null));
 
-    //createHostComponent
-    componentService = new TestHostComponentService("clusterName", "serviceName", "componentName");
-    m = componentService.getClass().getMethod("createHostComponent", String.class, HttpHeaders.class, UriInfo.class, String.class);
-    args = new Object[] {"body", getHttpHeaders(), getUriInfo(), "componentName"};
-    listInvocations.add(new ServiceTestInvocation(Request.Type.POST, componentService, m, args, "body"));
-
     //createHostComponents
     componentService = new TestHostComponentService("clusterName", "serviceName", null);
     m = componentService.getClass().getMethod("createHostComponents", String.class, HttpHeaders.class, UriInfo.class);
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariCustomCommandExecutionHelperTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariCustomCommandExecutionHelperTest.java
index d1e0d21..ddcd951 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariCustomCommandExecutionHelperTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariCustomCommandExecutionHelperTest.java
@@ -23,7 +23,6 @@ import static org.apache.ambari.server.agent.ExecutionCommand.KeyNames.JAVA_VERS
 import static org.apache.ambari.server.agent.ExecutionCommand.KeyNames.NOT_MANAGED_HDFS_PATH_LIST;
 import static org.apache.ambari.server.agent.ExecutionCommand.KeyNames.STACK_NAME;
 import static org.apache.ambari.server.agent.ExecutionCommand.KeyNames.STACK_VERSION;
-import static org.easymock.EasyMock.createMockBuilder;
 import static org.easymock.EasyMock.createNiceMock;
 import static org.easymock.EasyMock.expect;
 import static org.easymock.EasyMock.replay;
@@ -152,10 +151,10 @@ public class AmbariCustomCommandExecutionHelperTest {
     ambariManagementController = injector.getInstance(AmbariManagementController.class);
     clusters = injector.getInstance(Clusters.class);
 
-    expect(configHelper.getPropertyValuesWithPropertyType(EasyMock.anyObject(StackId.class),
+    expect(configHelper.getPropertiesWithPropertyType(EasyMock.anyObject(StackId.class),
         EasyMock.anyObject(PropertyInfo.PropertyType.class),
         EasyMock.anyObject(Cluster.class),
-        EasyMock.anyObject(Map.class))).andReturn(Collections.EMPTY_SET);
+        EasyMock.anyObject(Map.class))).andReturn(Collections.emptyMap());
 
     replay(configHelper);
 
@@ -165,7 +164,6 @@ public class AmbariCustomCommandExecutionHelperTest {
     SecurityContextHolder.getContext().setAuthentication(TestAuthenticationFactory.createAdministrator());
     createClusterFixture("c1", new StackId("HDP-2.0.6"), "2.0.6-1234", "c1");
 
-    EasyMock.verify(configHelper);
     EasyMock.reset(configHelper);
 
     expect(hostRoleCommand.getTaskId()).andReturn(1L);
@@ -571,7 +569,6 @@ public class AmbariCustomCommandExecutionHelperTest {
    * This should cause Ambari to execute service check on dependent service client component.
    *
    * Also assures that service check doesn't works without dependency defined
-   * @throws Exception
    */
   @Test
   public void testServiceCheckRunsOnDependentClientService() throws Exception {
@@ -599,6 +596,7 @@ public class AmbariCustomCommandExecutionHelperTest {
 
     //add host with client only
     addHost("c1-c6403", "c1");
+    clusters.updateHostMappings(clusters.getHost("c1-c6403"));
 
     createService("c1", "CORE", "HADOOP_CLIENTS");
     createServiceComponent("c1", "CORE", "HADOOP_CLIENTS", "SOME_CLIENT_FOR_SERVICE_CHECK", "SOME_CLIENT_FOR_SERVICE_CHECK", State.INIT);
@@ -740,7 +738,7 @@ public class AmbariCustomCommandExecutionHelperTest {
     Gson gson = new Gson();
 
     ActionManager manager = createNiceMock(ActionManager.class);
-    StackId stackId = createNiceMock(StackId.class);
+    StackId stackId = new StackId(SOME_STACK_NAME, SOME_STACK_VERSION);
     Cluster cluster = createNiceMock(Cluster.class);
     Injector injector = createNiceMock(Injector.class);
     Configuration configuration = createNiceMock(Configuration.class);
@@ -752,9 +750,8 @@ public class AmbariCustomCommandExecutionHelperTest {
     expect(cluster.getClusterName()).andReturn(clusterName);
     expect(cluster.getDesiredStackVersion()).andReturn(stackId);
     expect(cluster.getDesiredConfigs()).andReturn(desiredConfigs);
-    expect(stackId.getStackName()).andReturn(SOME_STACK_NAME).anyTimes();
-    expect(stackId.getStackVersion()).andReturn(SOME_STACK_VERSION).anyTimes();
-    expect(configuration.getMySQLJarName()).andReturn(MYSQL_JAR);
+    expect(configuration.getApiSSLAuthentication()).andReturn(false);
+    expect(configuration.getMySQLJarName()).andReturn(MYSQL_JAR).anyTimes();
     expect(configuration.getJavaHome()).andReturn(JAVA_HOME);
     expect(configuration.getJDKName()).andReturn(JDK_NAME);
     expect(configuration.getJCEName()).andReturn(JCE_NAME);
@@ -770,16 +767,14 @@ public class AmbariCustomCommandExecutionHelperTest {
     expect(configHelper.filterInvalidPropertyValues(notManagedHdfsPathMap, NOT_MANAGED_HDFS_PATH_LIST))
       .andReturn(notManagedHdfsPathSet);
 
-    AmbariConfig ambariConfig = new AmbariConfig(configuration);
-    expect(metadataGenerator.getAmbariConfig()).andReturn(ambariConfig);
+    replay(configuration);
 
-    replay(manager, clusters, cluster, injector, stackId, configuration, configHelper, metadataGenerator);
+    AmbariConfig ambariConfig = new AmbariConfig(configuration);
+    expect(metadataGenerator.getAmbariConfig()).andReturn(ambariConfig).anyTimes();
 
-    AmbariManagementControllerImpl ambariManagementControllerImpl = createMockBuilder(AmbariManagementControllerImpl.class)
-      .withConstructor(manager, clusters, metadataGenerator, injector)
-      .createNiceMock();
+    replay(manager, cluster, injector, configHelper, metadataGenerator);
 
-    replay(ambariManagementControllerImpl);
+    AmbariManagementControllerImpl ambariManagementControllerImpl = new AmbariManagementControllerImpl(manager, clusters, metadataGenerator, injector);
 
     // Inject configuration manually
     Class<?> amciClass = AmbariManagementControllerImpl.class;
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/HostComponentResourceProviderTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/HostComponentResourceProviderTest.java
index 42bccdb..0592a68 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/HostComponentResourceProviderTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/HostComponentResourceProviderTest.java
@@ -239,7 +239,7 @@ public class HostComponentResourceProviderTest {
     hostsComponentResource1.setProperty(HostComponentResourceProvider.HOST_COMPONENT_STATE_PROPERTY_ID, State.INSTALLED.name());
     hostsComponentResource1.setProperty(HostComponentResourceProvider.HOST_COMPONENT_DESIRED_STATE_PROPERTY_ID, State.STARTED.name());
     hostsComponentResource1.setProperty(
-        HostComponentResourceProvider.HOST_COMPONENT_VERSION_PROPERTY_ID, stackId2.getStackVersion());
+        HostComponentResourceProvider.HOST_COMPONENT_VERSION_PROPERTY_ID, stackId.getStackVersion());
     hostsComponentResource1.setProperty(HostComponentResourceProvider.HOST_COMPONENT_DESIRED_STACK_ID_PROPERTY_ID, stackId2.getStackId());
     hostsComponentResource1.setProperty(HostComponentResourceProvider.HOST_COMPONENT_UPGRADE_STATE_PROPERTY_ID, UpgradeState.NONE.name());
 
@@ -252,7 +252,7 @@ public class HostComponentResourceProviderTest {
     hostsComponentResource2.setProperty(HostComponentResourceProvider.HOST_COMPONENT_STATE_PROPERTY_ID, State.INSTALLED.name());
     hostsComponentResource2.setProperty(HostComponentResourceProvider.HOST_COMPONENT_DESIRED_STATE_PROPERTY_ID, State.STARTED.name());
     hostsComponentResource2.setProperty(
-        HostComponentResourceProvider.HOST_COMPONENT_VERSION_PROPERTY_ID, stackId2.getStackVersion());
+        HostComponentResourceProvider.HOST_COMPONENT_VERSION_PROPERTY_ID, stackId.getStackVersion());
     hostsComponentResource2.setProperty(HostComponentResourceProvider.HOST_COMPONENT_DESIRED_STACK_ID_PROPERTY_ID, stackId2.getStackId());
     hostsComponentResource2.setProperty(HostComponentResourceProvider.HOST_COMPONENT_UPGRADE_STATE_PROPERTY_ID, UpgradeState.NONE.name());
 
@@ -265,7 +265,7 @@ public class HostComponentResourceProviderTest {
     hostsComponentResource3.setProperty(HostComponentResourceProvider.HOST_COMPONENT_STATE_PROPERTY_ID, State.INSTALLED.name());
     hostsComponentResource3.setProperty(HostComponentResourceProvider.HOST_COMPONENT_DESIRED_STATE_PROPERTY_ID, State.STARTED.name());
     hostsComponentResource3.setProperty(
-        HostComponentResourceProvider.HOST_COMPONENT_VERSION_PROPERTY_ID, stackId2.getStackVersion());
+        HostComponentResourceProvider.HOST_COMPONENT_VERSION_PROPERTY_ID, stackId.getStackVersion());
     hostsComponentResource3.setProperty(HostComponentResourceProvider.HOST_COMPONENT_DESIRED_STACK_ID_PROPERTY_ID, stackId2.getStackId());
     hostsComponentResource3.setProperty(HostComponentResourceProvider.HOST_COMPONENT_UPGRADE_STATE_PROPERTY_ID, UpgradeState.NONE.name());
 
@@ -291,7 +291,7 @@ public class HostComponentResourceProviderTest {
     Set<String> names = new HashSet<>();
     for (Resource resource : resources) {
       for (String key : expectedNameValues.keySet()) {
-        Assert.assertEquals(expectedNameValues.get(key), resource.getPropertyValue(key));
+        Assert.assertEquals(key, expectedNameValues.get(key), resource.getPropertyValue(key));
       }
       names.add((String) resource.getPropertyValue(
           HostComponentResourceProvider.HOST_COMPONENT_COMPONENT_NAME_PROPERTY_ID));
@@ -349,7 +349,7 @@ public class HostComponentResourceProviderTest {
     expect(managementController.findServiceName(cluster, "Component100")).andReturn("Service100").anyTimes();
     expect(clusters.getCluster("Cluster102")).andReturn(cluster).anyTimes();
     expect(cluster.getClusterId()).andReturn(2L).anyTimes();
-    expect(cluster.getService("Service100")).andReturn(service).anyTimes();
+    expect(cluster.getService("ServiceGroup100", "Service100")).andReturn(service).anyTimes();
     expect(service.getServiceComponent("Component100")).andReturn(component).anyTimes();
     expect(component.getServiceComponentHost("Host100")).andReturn(componentHost).anyTimes();
     expect(component.getName()).andReturn("Component100").anyTimes();
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/HostResourceProviderTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/HostResourceProviderTest.java
index 81694e3..6837b8d 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/HostResourceProviderTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/HostResourceProviderTest.java
@@ -183,6 +183,7 @@ public class HostResourceProviderTest extends EasyMockSupport {
 
     // replay
     replayAll();
+    replay(managementController);
 
     SecurityContextHolder.getContext().setAuthentication(authentication);
 
@@ -293,6 +294,7 @@ public class HostResourceProviderTest extends EasyMockSupport {
 
     // replay
     replayAll();
+    replay(managementController);
 
     SecurityContextHolder.getContext().setAuthentication(authentication);
 
@@ -391,6 +393,7 @@ public class HostResourceProviderTest extends EasyMockSupport {
 
     // replay
     replayAll();
+    replay(managementController);
 
     SecurityContextHolder.getContext().setAuthentication(TestAuthenticationFactory.createAdministrator());
 
@@ -483,6 +486,7 @@ public class HostResourceProviderTest extends EasyMockSupport {
 
     // replay
     replayAll();
+    replay(managementController);
 
     SecurityContextHolder.getContext().setAuthentication(TestAuthenticationFactory.createAdministrator());
 
@@ -577,6 +581,7 @@ public class HostResourceProviderTest extends EasyMockSupport {
 
     // replay
     replayAll();
+    replay(managementController);
 
     SecurityContextHolder.getContext().setAuthentication(TestAuthenticationFactory.createAdministrator());
 
@@ -648,6 +653,7 @@ public class HostResourceProviderTest extends EasyMockSupport {
 
     // replay
     replayAll();
+    replay(managementController);
 
     SecurityContextHolder.getContext().setAuthentication(TestAuthenticationFactory.createAdministrator());
 
@@ -751,6 +757,7 @@ public class HostResourceProviderTest extends EasyMockSupport {
 
     // replay
     replayAll();
+    replay(managementController);
 
     SecurityContextHolder.getContext().setAuthentication(authentication);
 
@@ -843,6 +850,7 @@ public class HostResourceProviderTest extends EasyMockSupport {
 
     // replay
     replayAll();
+    replay(managementController);
 
     SecurityContextHolder.getContext().setAuthentication(TestAuthenticationFactory.createAdministrator());
 
@@ -926,6 +934,7 @@ public class HostResourceProviderTest extends EasyMockSupport {
 
     // replay
     replayAll();
+    replay(managementController);
 
     SecurityContextHolder.getContext().setAuthentication(authentication);
 
@@ -1011,6 +1020,7 @@ public class HostResourceProviderTest extends EasyMockSupport {
 
     // replay
     replayAll();
+    replay(managementController);
 
     SecurityContextHolder.getContext().setAuthentication(authentication);
 
@@ -1080,6 +1090,7 @@ public class HostResourceProviderTest extends EasyMockSupport {
 
     // replay
     replayAll();
+    replay(managementController);
 
     AbstractResourceProviderTest.TestObserver observer = new AbstractResourceProviderTest.TestObserver();
     ((ObservableResourceProvider) provider).addObserver(observer);
@@ -1180,6 +1191,7 @@ public class HostResourceProviderTest extends EasyMockSupport {
 
     // replay mocks
     replayAll();
+    replay(managementController);
 
     SecurityContextHolder.getContext().setAuthentication(authentication);
 
@@ -1216,6 +1228,7 @@ public class HostResourceProviderTest extends EasyMockSupport {
 
     // replay mocks
     replayAll();
+    replay(managementController);
 
     SecurityContextHolder.getContext().setAuthentication(TestAuthenticationFactory.createAdministrator());
     getHosts(managementController, setRequests);
@@ -1251,6 +1264,7 @@ public class HostResourceProviderTest extends EasyMockSupport {
 
     // replay mocks
     replayAll();
+    replay(managementController);
 
     SecurityContextHolder.getContext().setAuthentication(TestAuthenticationFactory.createAdministrator());
     getHosts(managementController, setRequests);
@@ -1314,6 +1328,7 @@ public class HostResourceProviderTest extends EasyMockSupport {
 
     // replay mocks
     replayAll();
+    replay(managementController);
 
     SecurityContextHolder.getContext().setAuthentication(TestAuthenticationFactory.createAdministrator());
 
@@ -1384,7 +1399,7 @@ public class HostResourceProviderTest extends EasyMockSupport {
     provider.updateHosts(requests);
   }
 
-  private Injector createInjector() throws Exception {
+  private Injector createInjector() {
     Injector injector = Guice.createInjector(new AbstractModule() {
       @Override
       protected void configure() {
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/JMXHostProviderTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/JMXHostProviderTest.java
index 86511bf..d1365e0 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/JMXHostProviderTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/JMXHostProviderTest.java
@@ -133,7 +133,7 @@ public class JMXHostProviderTest {
     Cluster cluster = clusters.getCluster(clusterName);
     cluster.setDesiredStackVersion(new StackId("HDP-0.1"));
     String serviceGroupName = "CORE";
-    ServiceGroupResourceProviderTest.createServiceGroup(controller, clusterName, serviceGroupName, r.getStackVersion());
+    ServiceGroupResourceProviderTest.createServiceGroup(controller, clusterName, serviceGroupName, STACK_ID.getStackId());
     String serviceName = "HDFS";
     createService(clusterName, serviceGroupName, serviceName, null);
     String componentName1 = "NAMENODE";
@@ -148,7 +148,7 @@ public class JMXHostProviderTest {
     clusters.addHost(host1);
     Map<String, String> hostAttributes = new HashMap<>();
     hostAttributes.put("os_family", "redhat");
-    hostAttributes.put("os_release_version", "5.9");
+    hostAttributes.put("os_release_version", "6.4");
     clusters.getHost("h1").setHostAttributes(hostAttributes);
     String host2 = "h2";
     clusters.addHost(host2);
@@ -236,7 +236,7 @@ public class JMXHostProviderTest {
     clusters.addHost(host1);
     Map<String, String> hostAttributes = new HashMap<>();
     hostAttributes.put("os_family", "redhat");
-    hostAttributes.put("os_release_version", "5.9");
+    hostAttributes.put("os_release_version", "6.4");
     clusters.getHost("h1").setHostAttributes(hostAttributes);
     String host2 = "h2";
     clusters.addHost(host2);
@@ -338,7 +338,7 @@ public class JMXHostProviderTest {
     clusters.addHost(host1);
     Map<String, String> hostAttributes = new HashMap<>();
     hostAttributes.put("os_family", "redhat");
-    hostAttributes.put("os_release_version", "5.9");
+    hostAttributes.put("os_release_version", "6.4");
     clusters.getHost("h1").setHostAttributes(hostAttributes);
     String host2 = "h2";
     clusters.addHost(host2);
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/orm/OrmTestHelper.java b/ambari-server/src/test/java/org/apache/ambari/server/orm/OrmTestHelper.java
index 4218cb9..b1366bb 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/orm/OrmTestHelper.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/orm/OrmTestHelper.java
@@ -340,6 +340,8 @@ public class OrmTestHelper {
       mpackEntity.setMpackVersion(stackId.getStackVersion());
       mpackEntity.setMpackUri("http://mpacks.org/" + stackId.toString() + ".json");
       mpackDAO.create(mpackEntity);
+      stackEntity.setMpackId(mpackEntity.getId());
+      stackDAO.merge(stackEntity);
     }
 
     if (mpackEntity.getRepositoryOperatingSystems().isEmpty()) {
diff --git a/ambari-server/src/test/resources/stacks/HDP/2.0.6/services/HADOOP_CLIENTS/metainfo.xml b/ambari-server/src/test/resources/stacks/HDP/2.0.6/services/HADOOP_CLIENTS/metainfo.xml
index 385b9ed..b5085ae 100644
--- a/ambari-server/src/test/resources/stacks/HDP/2.0.6/services/HADOOP_CLIENTS/metainfo.xml
+++ b/ambari-server/src/test/resources/stacks/HDP/2.0.6/services/HADOOP_CLIENTS/metainfo.xml
@@ -29,6 +29,11 @@
           <category>CLIENT</category>
           <cardinality>0+</cardinality>
           <version>0.95.2.2.0.6.0-hbase-master</version>
+          <commandScript>
+            <script>scripts/some_client.py</script>
+            <scriptType>PYTHON</scriptType>
+            <timeout>300</timeout>
+          </commandScript>
         </component>
       </components>
       <commandScript>

-- 
To stop receiving notification emails like this one, please contact
adoroszlai@apache.org.