You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ambari.apache.org by am...@apache.org on 2017/10/30 10:35:00 UTC
ambari git commit: AMBARI-22319. Allow the same config type to belong
to multiple services (amagyar)
Repository: ambari
Updated Branches:
refs/heads/branch-feature-AMBARI-22008 b34818db3 -> 582d402bf
AMBARI-22319. Allow the same config type to belong to multiple services (amagyar)
Project: http://git-wip-us.apache.org/repos/asf/ambari/repo
Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/582d402b
Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/582d402b
Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/582d402b
Branch: refs/heads/branch-feature-AMBARI-22008
Commit: 582d402bfa8bd9247559a52e299afbfe47495466
Parents: b34818d
Author: Attila Magyar <am...@hortonworks.com>
Authored: Mon Oct 30 11:34:27 2017 +0100
Committer: Attila Magyar <am...@hortonworks.com>
Committed: Mon Oct 30 11:34:27 2017 +0100
----------------------------------------------------------------------
.../AmbariManagementControllerImpl.java | 8 +-
.../server/controller/internal/Stack.java | 14 ++++
.../ambari/server/state/ConfigHelper.java | 2 +-
.../server/state/cluster/ClusterImpl.java | 79 +++++++-------------
.../ambari/server/topology/AmbariContext.java | 7 +-
.../server/state/cluster/ClusterTest.java | 4 +
.../server/topology/AmbariContextTest.java | 5 +-
7 files changed, 58 insertions(+), 61 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/ambari/blob/582d402b/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java
----------------------------------------------------------------------
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 1b1f524..d90ccb8 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
@@ -795,14 +795,8 @@ public class AmbariManagementControllerImpl implements AmbariManagementControlle
// If the config type is for a service, then allow a user with SERVICE_MODIFY_CONFIGS to
// update, else ensure the user has CLUSTER_MODIFY_CONFIGS
- String service = null;
+ String service = cluster.getServiceByConfigType(configType);
- try {
- service = cluster.getServiceForConfigTypes(Collections.singleton(configType));
- } catch (IllegalArgumentException e) {
- // Ignore this since we may have hit a config type that spans multiple services. This may
- // happen in unit test cases but should not happen with later versions of stacks.
- }
// Get the changes so that the user's intention can be determined. For example, maybe
// the user wants to change the run-as user for a service or maybe the the cluster-wide
http://git-wip-us.apache.org/repos/asf/ambari/blob/582d402b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/Stack.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/Stack.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/Stack.java
index f8feef2..912d9be 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/Stack.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/Stack.java
@@ -18,10 +18,12 @@
package org.apache.ambari.server.controller.internal;
+import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
+import java.util.List;
import java.util.Map;
import java.util.Set;
@@ -494,6 +496,18 @@ public class Stack {
"Specified configuration type is not associated with any service: " + config);
}
+ public List<String> getServicesForConfigType(String config) {
+ List<String> serviceNames = new ArrayList<>();
+ for (Map.Entry<String, Map<String, Map<String, ConfigProperty>>> entry : serviceConfigurations.entrySet()) {
+ Map<String, Map<String, ConfigProperty>> typeMap = entry.getValue();
+ String serviceName = entry.getKey();
+ if (typeMap.containsKey(config) && !getExcludedConfigurationTypes(serviceName).contains(config)) {
+ serviceNames.add(serviceName);
+ }
+ }
+ return serviceNames;
+ }
+
/**
* Return the dependencies specified for the given component.
*
http://git-wip-us.apache.org/repos/asf/ambari/blob/582d402b/ambari-server/src/main/java/org/apache/ambari/server/state/ConfigHelper.java
----------------------------------------------------------------------
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 eade914..edeb4b7 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
@@ -1123,7 +1123,7 @@ public class ConfigHelper {
if (null != baseConfig) {
try {
- String service = cluster.getServiceForConfigTypes(Collections.singleton(type));
+ String service = cluster.getServiceByConfigType(type);
if (!serviceMapped.containsKey(service)) {
serviceMapped.put(service, new HashSet<>());
}
http://git-wip-us.apache.org/repos/asf/ambari/blob/582d402b/ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java
----------------------------------------------------------------------
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 9c0b0ca..4a86e00 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
@@ -18,6 +18,8 @@
package org.apache.ambari.server.state.cluster;
+import static java.util.stream.Collectors.toList;
+
import java.text.MessageFormat;
import java.util.ArrayList;
import java.util.Collection;
@@ -29,13 +31,13 @@ import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
+import java.util.Objects;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.ConcurrentSkipListMap;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.locks.ReadWriteLock;
-import java.util.stream.Collectors;
import javax.annotation.Nullable;
import javax.persistence.EntityManager;
@@ -1135,7 +1137,7 @@ public class ClusterImpl implements Cluster {
return clusterDAO.getLatestConfigurationsWithTypes(clusterId, getDesiredStackVersion(), types)
.stream()
.map(clusterConfigEntity -> configFactory.createExisting(this, clusterConfigEntity))
- .collect(Collectors.toList());
+ .collect(toList());
}
@Override
@@ -1571,37 +1573,35 @@ public class ClusterImpl implements Cluster {
@Override
public String getServiceForConfigTypes(Collection<String> configTypes) {
- //debug
- LOG.info("Looking for service for config types {}", configTypes);
- String serviceName = null;
- for (String configType : configTypes) {
- for (Entry<String, String> entry : serviceConfigTypes.entries()) {
- if (StringUtils.equals(entry.getValue(), configType)) {
- if (serviceName != null) {
- if (entry.getKey()!=null && !StringUtils.equals(serviceName, entry.getKey())) {
- throw new IllegalArgumentException(String.format("Config type %s belongs to %s service, " +
- "but also qualified for %s", configType, serviceName, entry.getKey()));
- }
- } else {
- serviceName = entry.getKey();
- }
- }
- }
+ List<String> serviceNames = configTypes.stream()
+ .map(this::getServiceByConfigType)
+ .filter(Objects::nonNull)
+ .collect(toList());
+ boolean allTheSame = new HashSet<>(serviceNames).size() <= 1;
+ if (!allTheSame) {
+ throw new IllegalArgumentException(String.format(
+ "Config types: %s should belong to a single installed service. But they belong to: %s", configTypes, serviceNames));
}
- LOG.info("Service {} returning", serviceName);
- return serviceName;
+ return serviceNames.isEmpty() ? null : serviceNames.get(0);
+ }
+
+ public List<String> serviceNameByConfigType(String configType) {
+ return serviceConfigTypes.entries().stream()
+ .filter(entry -> StringUtils.equals(entry.getValue(), configType))
+ .map(entry -> entry.getKey())
+ .collect(toList());
}
@Override
public String getServiceByConfigType(String configType) {
- for (Entry<String, String> entry : serviceConfigTypes.entries()) {
- String serviceName = entry.getKey();
- String type = entry.getValue();
- if (StringUtils.equals(type, configType)) {
- return serviceName;
- }
- }
- return null;
+ return serviceNameByConfigType(configType).stream()
+ .filter(this::isServiceInstalled)
+ .findFirst()
+ .orElse(null);
+ }
+
+ private boolean isServiceInstalled(String serviceName) {
+ return services.get(serviceName) != null;
}
@Override
@@ -1884,28 +1884,7 @@ public class ClusterImpl implements Cluster {
@Transactional
ServiceConfigVersionResponse applyConfigs(Set<Config> configs, String user, String serviceConfigVersionNote) {
-
- String serviceName = null;
- for (Config config : configs) {
- for (Entry<String, String> entry : serviceConfigTypes.entries()) {
- if (StringUtils.equals(entry.getValue(), config.getType())) {
- if (serviceName == null) {
- serviceName = entry.getKey();
- break;
- } else if (!serviceName.equals(entry.getKey())) {
- String error = String.format("Updating configs for multiple services by a " +
- "single API request isn't supported. Conflicting services %s and %s for %s",
- serviceName, entry.getKey(), config.getType());
- IllegalArgumentException exception = new IllegalArgumentException(error);
- LOG.error(error + ", config version not created for {}", serviceName);
- throw exception;
- } else {
- break;
- }
- }
- }
- }
-
+ String serviceName = getServiceForConfigTypes(configs.stream().map(Config::getType).collect(toList()));
// update the selected flag for every config type
ClusterEntity clusterEntity = getClusterEntity();
Collection<ClusterConfigEntity> clusterConfigs = clusterEntity.getClusterConfigEntities();
http://git-wip-us.apache.org/repos/asf/ambari/blob/582d402b/ambari-server/src/main/java/org/apache/ambari/server/topology/AmbariContext.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/topology/AmbariContext.java b/ambari-server/src/main/java/org/apache/ambari/server/topology/AmbariContext.java
index eb39562..b09c778 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/topology/AmbariContext.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/topology/AmbariContext.java
@@ -731,7 +731,12 @@ public class AmbariContext {
// iterate over topo host group configs which were defined in
for (Map.Entry<String, Map<String, String>> entry : userProvidedGroupProperties.entrySet()) {
String type = entry.getKey();
- String service = stack.getServiceForConfigType(type);
+ List<String> services = stack.getServicesForConfigType(type);
+ String service = services.stream()
+ .filter(each -> topology.getBlueprint().getServices().contains(each))
+ .findFirst()
+ .orElseThrow(() -> new IllegalArgumentException("Specified configuration type is not associated with any service: " + type));
+
Config config = configFactory.createReadOnly(type, groupName, entry.getValue(), null);
//todo: attributes
Map<String, Config> serviceConfigs = groupConfigs.get(service);
http://git-wip-us.apache.org/repos/asf/ambari/blob/582d402b/ambari-server/src/test/java/org/apache/ambari/server/state/cluster/ClusterTest.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/state/cluster/ClusterTest.java b/ambari-server/src/test/java/org/apache/ambari/server/state/cluster/ClusterTest.java
index 4b09c6d..8b37f72 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/state/cluster/ClusterTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/state/cluster/ClusterTest.java
@@ -1203,6 +1203,7 @@ public class ClusterTest {
@Test
public void testServiceConfigVersions() throws Exception {
createDefaultCluster();
+ c1.addService("HDFS", helper.getOrCreateRepositoryVersion(new StackId("HDP", "0.1"), "0.1"));
Config config1 = configFactory.createNew(c1, "hdfs-site", "version1",
new HashMap<String, String>() {{ put("a", "b"); }}, new HashMap<>());
@@ -1261,6 +1262,7 @@ public class ClusterTest {
@Test
public void testSingleServiceVersionForMultipleConfigs() throws Exception {
createDefaultCluster();
+ c1.addService("HDFS", helper.getOrCreateRepositoryVersion(new StackId("HDP", "0.1"), "0.1"));
Config config1 = configFactory.createNew(c1, "hdfs-site", "version1",
new HashMap<String, String>() {{ put("a", "b"); }}, new HashMap<>());
@@ -1383,6 +1385,7 @@ public class ClusterTest {
public void testAllServiceConfigVersionsWithConfigGroups() throws Exception {
// Given
createDefaultCluster();
+ c1.addService("HDFS", helper.getOrCreateRepositoryVersion(new StackId("HDP", "0.1"), "0.1"));
Config hdfsSiteConfigV1 = configFactory.createNew(c1, "hdfs-site", "version1",
ImmutableMap.of("p1", "v1"), ImmutableMap.of());
@@ -1442,6 +1445,7 @@ public class ClusterTest {
public void testAllServiceConfigVersionsWithDeletedConfigGroups() throws Exception {
// Given
createDefaultCluster();
+ c1.addService("HDFS", helper.getOrCreateRepositoryVersion(new StackId("HDP", "0.1"), "0.1"));
Config hdfsSiteConfigV1 = configFactory.createNew(c1, "hdfs-site", "version1",
ImmutableMap.of("p1", "v1"), ImmutableMap.of());
http://git-wip-us.apache.org/repos/asf/ambari/blob/582d402b/ambari-server/src/test/java/org/apache/ambari/server/topology/AmbariContextTest.java
----------------------------------------------------------------------
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 0deeae9..5128c65 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
@@ -18,6 +18,7 @@
package org.apache.ambari.server.topology;
+import static java.util.Collections.singletonList;
import static org.easymock.EasyMock.anyObject;
import static org.easymock.EasyMock.capture;
import static org.easymock.EasyMock.createMock;
@@ -215,7 +216,7 @@ public class AmbariContextTest {
expect(repositoryVersion.getType()).andReturn(RepositoryType.STANDARD).atLeastOnce();
expect(repositoryVersionDAO.findByStack(EasyMock.anyObject(StackId.class))).andReturn(
- Collections.singletonList(repositoryVersion)).atLeastOnce();
+ singletonList(repositoryVersion)).atLeastOnce();
replay(repositoryVersionDAO, repositoryVersion);
context.configFactory = configFactory;
@@ -240,7 +241,7 @@ public class AmbariContextTest {
expect(stack.getVersion()).andReturn(STACK_VERSION).anyTimes();
for (Map.Entry<String, String> entry : configTypeServiceMapping.entrySet()) {
- expect(stack.getServiceForConfigType(entry.getKey())).andReturn(entry.getValue()).anyTimes();
+ expect(stack.getServicesForConfigType(entry.getKey())).andReturn(singletonList(entry.getValue())).anyTimes();
}
expect(controller.getClusters()).andReturn(clusters).anyTimes();