AMBARI-14965: Ambari server lists service even though service creation fails (Ajit Kumar via jluniya)
Project: http://git-wip-us.apache.org/repos/asf/ambari/repo Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/73fbe14c Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/73fbe14c Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/73fbe14c Branch: refs/heads/branch-dev-patch-upgrade Commit: 73fbe14c2a14619a5023ea0698cda72858c05fbe Parents: 37122a6 Author: Jayush Luniya <jlun...@hortonworks.com> Authored: Tue Feb 9 15:08:42 2016 -0800 Committer: Jayush Luniya <jlun...@hortonworks.com> Committed: Tue Feb 9 15:08:42 2016 -0800 ---------------------------------------------------------------------- .../internal/ServiceResourceProvider.java | 197 +++++++++---------- 1 file changed, 88 insertions(+), 109 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ambari/blob/73fbe14c/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ServiceResourceProvider.java ---------------------------------------------------------------------- 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 a2aca70..ed7659f 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 @@ -64,6 +64,8 @@ import org.apache.ambari.server.state.ServiceFactory; import org.apache.ambari.server.state.StackId; import org.apache.ambari.server.state.State; import org.apache.commons.lang.StringUtils; +import org.apache.commons.lang.Validate; + import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -148,7 +150,7 @@ public class ServiceResourceProvider extends AbstractControllerResourceProvider ResourceAlreadyExistsException, NoSuchParentResourceException { - final Set<ServiceRequest> requests = new HashSet<ServiceRequest>(); + final Set<ServiceRequest> requests = new HashSet<>(); for (Map<String, Object> propertyMap : request.getProperties()) { requests.add(getRequest(propertyMap)); } @@ -338,115 +340,11 @@ public class ServiceResourceProvider extends AbstractControllerResourceProvider LOG.warn("Received an empty requests set"); return; } - - Clusters clusters = getManagementController().getClusters(); - AmbariMetaInfo ambariMetaInfo = getManagementController().getAmbariMetaInfo(); - + Clusters clusters = getManagementController().getClusters(); // do all validation checks - Map<String, Set<String>> serviceNames = new HashMap<String, Set<String>>(); - Set<String> duplicates = new HashSet<String>(); - for (ServiceRequest request : requests) { - if (request.getClusterName() == null - || request.getClusterName().isEmpty() - || request.getServiceName() == null - || request.getServiceName().isEmpty()) { - throw new IllegalArgumentException("Cluster name and service name" - + " should be provided when creating a service"); - } - - if (LOG.isDebugEnabled()) { - LOG.debug("Received a createService request" - + ", clusterName=" + request.getClusterName() - + ", serviceName=" + request.getServiceName() - + ", request=" + request); - } - - if(!AuthorizationHelper.isAuthorized(ResourceType.CLUSTER, getClusterResourceId(request.getClusterName()), RoleAuthorization.SERVICE_ADD_DELETE_SERVICES)) { - throw new AuthorizationException("The user is not authorized to create services"); - } - - if (!serviceNames.containsKey(request.getClusterName())) { - serviceNames.put(request.getClusterName(), new HashSet<String>()); - } - if (serviceNames.get(request.getClusterName()) - .contains(request.getServiceName())) { - // throw error later for dup - duplicates.add(request.getServiceName()); - continue; - } - serviceNames.get(request.getClusterName()).add(request.getServiceName()); - - if (request.getDesiredState() != null - && !request.getDesiredState().isEmpty()) { - State state = State.valueOf(request.getDesiredState()); - if (!state.isValidDesiredState() - || state != State.INIT) { - throw new IllegalArgumentException("Invalid desired state" - + " only INIT state allowed during creation" - + ", providedDesiredState=" + request.getDesiredState()); - } - } - - Cluster cluster; - try { - cluster = clusters.getCluster(request.getClusterName()); - } catch (ClusterNotFoundException e) { - throw new ParentObjectNotFoundException("Attempted to add a service to a cluster which doesn't exist", e); - } - try { - Service s = cluster.getService(request.getServiceName()); - if (s != null) { - // throw error later for dup - duplicates.add(request.getServiceName()); - continue; - } - } catch (ServiceNotFoundException e) { - // Expected - } - - StackId stackId = cluster.getDesiredStackVersion(); - if (!ambariMetaInfo.isValidService(stackId.getStackName(), - stackId.getStackVersion(), request.getServiceName())) { - throw new IllegalArgumentException("Unsupported or invalid service" - + " in stack" - + ", clusterName=" + request.getClusterName() - + ", serviceName=" + request.getServiceName() - + ", stackInfo=" + stackId.getStackId()); - } - } - - // ensure only a single cluster update - if (serviceNames.size() != 1) { - throw new IllegalArgumentException("Invalid arguments, updates allowed" - + "on only one cluster at a time"); - } - - // Validate dups - if (!duplicates.isEmpty()) { - StringBuilder svcNames = new StringBuilder(); - boolean first = true; - for (String svcName : duplicates) { - if (!first) { - svcNames.append(","); - } - first = false; - svcNames.append(svcName); - } - String clusterName = requests.iterator().next().getClusterName(); - String msg; - if (duplicates.size() == 1) { - msg = "Attempted to create a service which already exists: " - + ", clusterName=" + clusterName + " serviceName=" + svcNames.toString(); - } else { - msg = "Attempted to create services which already exist: " - + ", clusterName=" + clusterName + " serviceNames=" + svcNames.toString(); - } - throw new DuplicateResourceException(msg); - } + validateCreateRequests(requests, clusters); ServiceFactory serviceFactory = getManagementController().getServiceFactory(); - - // now to the real work for (ServiceRequest request : requests) { Cluster cluster = clusters.getCluster(request.getClusterName()); @@ -457,11 +355,10 @@ public class ServiceResourceProvider extends AbstractControllerResourceProvider s.setDesiredState(state); s.setDesiredStackVersion(cluster.getDesiredStackVersion()); + s.persist(); cluster.addService(s); // Initialize service widgets getManagementController().initializeWidgetsAndLayouts(cluster, s); - - s.persist(); } } @@ -974,4 +871,86 @@ public class ServiceResourceProvider extends AbstractControllerResourceProvider return serviceSpecificProperties; } + + private void validateCreateRequests(Set<ServiceRequest> requests, Clusters clusters) + throws AuthorizationException, AmbariException { + + AmbariMetaInfo ambariMetaInfo = getManagementController().getAmbariMetaInfo(); + Map<String, Set<String>> serviceNames = new HashMap<>(); + Set<String> duplicates = new HashSet<>(); + for (ServiceRequest request : requests) { + final String clusterName = request.getClusterName(); + final String serviceName = request.getServiceName(); + Validate.notEmpty(clusterName, "Cluster 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" + + ", clusterName=" + clusterName + ", serviceName=" + serviceName + ", request=" + 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>()); + } + + if (serviceNames.get(clusterName).contains(serviceName)) { + // throw error later for dup + duplicates.add(serviceName); + continue; + } + serviceNames.get(clusterName).add(serviceName); + + if (StringUtils.isNotEmpty(request.getDesiredState())) { + State state = State.valueOf(request.getDesiredState()); + if (!state.isValidDesiredState() || state != State.INIT) { + throw new IllegalArgumentException("Invalid desired state" + + " only INIT state allowed during creation" + + ", providedDesiredState=" + request.getDesiredState()); + } + } + + Cluster cluster; + try { + cluster = clusters.getCluster(clusterName); + } catch (ClusterNotFoundException e) { + 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) { + // throw error later for dup + duplicates.add(serviceName); + continue; + } + } catch (ServiceNotFoundException e) { + // Expected + } + + StackId stackId = cluster.getDesiredStackVersion(); + if (!ambariMetaInfo.isValidService(stackId.getStackName(), + stackId.getStackVersion(), request.getServiceName())) { + throw new IllegalArgumentException("Unsupported or invalid service in stack, clusterName=" + clusterName + + ", serviceName=" + serviceName + ", stackInfo=" + stackId.getStackId()); + } + } + // ensure only a single cluster update + if (serviceNames.size() != 1) { + throw new IllegalArgumentException("Invalid arguments, updates allowed" + + "on only one cluster at a time"); + } + + // 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, ","); + + throw new DuplicateResourceException(msg); + } + + } }