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/193446d0 Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/193446d0 Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/193446d0 Branch: refs/heads/branch-feature-AMBARI-22008-isilon Commit: 193446d05987e5358d5d24debad1973ace93ab7e Parents: 45b6549 Author: Attila Magyar <amag...@hortonworks.com> Authored: Mon Oct 30 11:34:27 2017 +0100 Committer: Attila Magyar <amag...@hortonworks.com> Committed: Fri Jan 5 08:54:45 2018 +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/193446d0/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 198b617..a2d8494 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 @@ -802,14 +802,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/193446d0/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/193446d0/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 6813fc0..3b0ee0f 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/193446d0/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 2266c62..2f3afd6 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 @@ -1581,37 +1583,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 @@ -1894,28 +1894,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/193446d0/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 933afa2..9e7eb17 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 @@ -732,7 +732,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/193446d0/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/193446d0/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();
