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:30 UTC
[ambari] 03/07: AMBARI-23746. Cannot create same named service in
different service groups in same request
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.