This is an automated email from the ASF dual-hosted git repository. adoroszlai pushed a commit to branch trunk in repository https://gitbox.apache.org/repos/asf/ambari.git
The following commit(s) were added to refs/heads/trunk by this push: new 6231892 AMBARI-25030. Improve component-host association syntax in Add Service request (#2711) 6231892 is described below commit 6231892fdc052016eba183935d69277b56a73917 Author: Doroszlai, Attila <6454655+adorosz...@users.noreply.github.com> AuthorDate: Tue Dec 11 18:04:53 2018 +0100 AMBARI-25030. Improve component-host association syntax in Add Service request (#2711) --- .../server/controller/AddServiceRequest.java | 167 ++++++++++++++++----- .../ambari/server/topology/Configuration.java | 16 ++ .../topology/addservice/RequestValidator.java | 4 +- .../server/controller/AddServiceRequestTest.java | 87 ++--------- .../test/resources/add_service_api/request1.json | 14 +- .../test/resources/add_service_api/request2.json | 16 +- .../test/resources/add_service_api/request4.json | 16 +- 7 files changed, 188 insertions(+), 132 deletions(-) diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/AddServiceRequest.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/AddServiceRequest.java index 4a88a6d..04d63a1 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/controller/AddServiceRequest.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/AddServiceRequest.java @@ -20,6 +20,7 @@ package org.apache.ambari.server.controller; import static java.util.Collections.emptySet; import static java.util.stream.Collectors.toMap; +import static java.util.stream.Collectors.toSet; import static org.apache.ambari.server.controller.internal.BaseClusterRequest.PROVISION_ACTION_PROPERTY; import static org.apache.ambari.server.controller.internal.ClusterResourceProvider.CREDENTIALS; import static org.apache.ambari.server.controller.internal.ClusterResourceProvider.SECURITY; @@ -29,6 +30,7 @@ import static org.apache.ambari.server.topology.Configurable.CONFIGURATIONS; import java.io.IOException; import java.io.UncheckedIOException; +import java.util.Arrays; import java.util.Collection; import java.util.HashMap; import java.util.Map; @@ -51,6 +53,7 @@ import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.common.base.MoreObjects; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; @@ -64,10 +67,10 @@ import io.swagger.annotations.ApiModelProperty; @JsonInclude(JsonInclude.Include.NON_EMPTY) public class AddServiceRequest { - static final String STACK_NAME = "stack_name"; - static final String STACK_VERSION = "stack_version"; - static final String SERVICES = "services"; - static final String COMPONENTS = "components"; + private static final String STACK_NAME = "stack_name"; + private static final String STACK_VERSION = "stack_version"; + private static final String SERVICES = "services"; + private static final String COMPONENTS = "components"; public static final Set<String> TOP_LEVEL_PROPERTIES = ImmutableSet.of( OPERATION_TYPE, CONFIG_RECOMMENDATION_STRATEGY, PROVISION_ACTION_PROPERTY, @@ -104,7 +107,6 @@ public class AddServiceRequest { ); } - private AddServiceRequest( OperationType operationType, ConfigRecommendationStrategy recommendationStrategy, @@ -219,25 +221,74 @@ public class AddServiceRequest { return security; } + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (obj == null || getClass() != obj.getClass()) { + return false; + } + + AddServiceRequest other = (AddServiceRequest) obj; + + return Objects.equals(operationType, other.operationType) && + Objects.equals(recommendationStrategy, other.recommendationStrategy) && + Objects.equals(provisionAction, other.provisionAction) && + Objects.equals(stackName, other.stackName) && + Objects.equals(stackVersion, other.stackVersion) && + Objects.equals(services, other.services) && + Objects.equals(components, other.components) && + Objects.equals(security, other.security) && + Objects.equals(configuration, other.configuration); + // credentials is ignored for equality, since it's not serialized + } + + @Override + public int hashCode() { + return Objects.hash(operationType, recommendationStrategy, provisionAction, stackName, stackVersion, + services, components, configuration, security); + // credentials is ignored for hashcode, since it's not serialized + } -// ------- inner classes ------- + @Override + public String toString() { + return MoreObjects.toStringHelper(this) + .add(OPERATION_TYPE, operationType) + .add(CONFIG_RECOMMENDATION_STRATEGY, recommendationStrategy) + .add(PROVISION_ACTION_PROPERTY, provisionAction) + .add(STACK_NAME, stackName) + .add(STACK_VERSION, stackVersion) + .add(SERVICES, services) + .add(COMPONENTS, components) + .add(CONFIGURATIONS, configuration) + .add(SECURITY, security) + // credentials is not part of string output + .toString(); + } + + // ------- inner classes ------- public enum OperationType { ADD_SERVICE, DELETE_SERVICE, MOVE_SERVICE } public static final class Component { - static final String COMPONENT_NAME = "component_name"; - static final String FQDN = "fqdn"; - private String name; - private String fqdn; + static final String COMPONENT_NAME = "name"; + static final String HOSTS = "hosts"; - public static Component of(String name, String fqdn) { - Component component = new Component(); - component.setName(name); - component.setFqdn(fqdn); - return component; + private final String name; + private final Set<Host> hosts; + + @JsonCreator + public Component(@JsonProperty(COMPONENT_NAME) String name, @JsonProperty(HOSTS) Set<Host> hosts) { + this.name = name; + this.hosts = hosts != null ? ImmutableSet.copyOf(hosts) : ImmutableSet.of(); + } + + public static Component of(String name, String... hosts) { + return new Component(name, Arrays.stream(hosts).map(Host::new).collect(toSet())); } @JsonProperty(COMPONENT_NAME) @@ -246,9 +297,47 @@ public class AddServiceRequest { return name; } - @JsonProperty(COMPONENT_NAME) - public void setName(String name) { - this.name = name; + @JsonProperty(HOSTS) + @ApiModelProperty(name = HOSTS) + public Set<Host> getHosts() { + return hosts; + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + + Component other = (Component) o; + + return Objects.equals(name, other.name) && + Objects.equals(hosts, other.hosts); + } + + @Override + public int hashCode() { + return Objects.hash(name, hosts); + } + + @Override + public String toString() { + return name; + } + } + + public static final class Host { + + static final String FQDN = "fqdn"; + + private final String fqdn; + + @JsonCreator + public Host(@JsonProperty(FQDN) String fqdn) { + this.fqdn = fqdn; } @JsonProperty(FQDN) @@ -257,41 +346,46 @@ public class AddServiceRequest { return fqdn; } - @JsonProperty(FQDN) - public void setFqdn(String fqdn) { - this.fqdn = fqdn; - } - @Override public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - Component component = (Component) o; - return Objects.equals(name, component.name) && - Objects.equals(fqdn, component.fqdn); + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + + Host other = (Host) o; + + return Objects.equals(fqdn, other.fqdn); } @Override public int hashCode() { - return Objects.hash(name, fqdn); + return Objects.hashCode(fqdn); } @Override public String toString() { - return name; + return "host: " + fqdn; } + } @ApiModel public static final class Service { + static final String NAME = "name"; - private String name; + private final String name; + + @JsonCreator + public Service(@JsonProperty(NAME) String name) { + this.name = name; + } public static Service of(String name) { - Service service = new Service(); - service.setName(name); - return service; + return new Service(name); } @JsonProperty(NAME) @@ -300,11 +394,6 @@ public class AddServiceRequest { return name; } - @JsonProperty(NAME) - public void setName(String name) { - this.name = name; - } - @Override public boolean equals(Object o) { if (this == o) return true; diff --git a/ambari-server/src/main/java/org/apache/ambari/server/topology/Configuration.java b/ambari-server/src/main/java/org/apache/ambari/server/topology/Configuration.java index fae5232..fc02297 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/topology/Configuration.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/topology/Configuration.java @@ -489,4 +489,20 @@ public class Configuration { public Pair<Map<String, Map<String, String>>, Map<String, Map<String, Map<String, String>>>> asPair() { return Pair.of(properties, attributes); } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (obj == null || getClass() != obj.getClass()) { + return false; + } + + Configuration other = (Configuration) obj; + + return Objects.equals(properties, other.properties) && + Objects.equals(attributes, other.attributes) && + Objects.equals(parentConfiguration, other.parentConfiguration); + } } diff --git a/ambari-server/src/main/java/org/apache/ambari/server/topology/addservice/RequestValidator.java b/ambari-server/src/main/java/org/apache/ambari/server/topology/addservice/RequestValidator.java index 04dc084..f976f82 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/topology/addservice/RequestValidator.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/topology/addservice/RequestValidator.java @@ -21,7 +21,6 @@ import static java.util.stream.Collectors.toSet; import java.util.Collection; import java.util.HashMap; -import java.util.HashSet; import java.util.LinkedHashMap; import java.util.Map; import java.util.Optional; @@ -227,8 +226,7 @@ public class RequestValidator { "Service %s (for component %s) already exists in cluster %s", serviceName, componentName, cluster.getClusterName()); newServices.computeIfAbsent(serviceName, __ -> new HashMap<>()) - .computeIfAbsent(componentName, __ -> new HashSet<>()) - .add(requestedComponent.getFqdn()); + .put(componentName, requestedComponent.getHosts().stream().map(AddServiceRequest.Host::getFqdn).collect(toSet())); } CHECK.checkArgument(!newServices.isEmpty(), "Request should have at least one new service or component to be added"); diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/AddServiceRequestTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/AddServiceRequestTest.java index c0f4afe..35491d6 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/controller/AddServiceRequestTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/AddServiceRequestTest.java @@ -18,22 +18,14 @@ package org.apache.ambari.server.controller; -import static org.apache.ambari.server.controller.AddServiceRequest.COMPONENTS; import static org.apache.ambari.server.controller.AddServiceRequest.Component; import static org.apache.ambari.server.controller.AddServiceRequest.OperationType.ADD_SERVICE; -import static org.apache.ambari.server.controller.AddServiceRequest.SERVICES; -import static org.apache.ambari.server.controller.AddServiceRequest.STACK_NAME; -import static org.apache.ambari.server.controller.AddServiceRequest.STACK_VERSION; import static org.apache.ambari.server.controller.AddServiceRequest.Service; -import static org.apache.ambari.server.controller.internal.BaseClusterRequest.PROVISION_ACTION_PROPERTY; import static org.apache.ambari.server.controller.internal.ProvisionAction.INSTALL_AND_START; import static org.apache.ambari.server.controller.internal.ProvisionAction.INSTALL_ONLY; -import static org.apache.ambari.server.controller.internal.ProvisionClusterRequest.CONFIG_RECOMMENDATION_STRATEGY; -import static org.apache.ambari.server.controller.internal.ServiceResourceProvider.OPERATION_TYPE; import static org.apache.ambari.server.topology.ConfigRecommendationStrategy.ALWAYS_APPLY; import static org.apache.ambari.server.topology.ConfigRecommendationStrategy.NEVER_APPLY; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; @@ -44,21 +36,15 @@ import java.util.List; import java.util.Map; import java.util.Optional; -import org.apache.ambari.server.controller.internal.ClusterResourceProvider; -import org.apache.ambari.server.controller.internal.ProvisionAction; import org.apache.ambari.server.security.encryption.CredentialStoreType; import org.apache.ambari.server.state.SecurityType; -import org.apache.ambari.server.topology.ConfigRecommendationStrategy; -import org.apache.ambari.server.topology.Configurable; import org.apache.ambari.server.topology.Configuration; import org.apache.ambari.server.topology.Credential; import org.apache.ambari.server.topology.SecurityConfiguration; -import org.apache.ambari.server.topology.SecurityConfigurationFactory; import org.junit.BeforeClass; import org.junit.Test; import com.fasterxml.jackson.core.JsonProcessingException; -import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -112,7 +98,7 @@ public class AddServiceRequestTest { configuration.getProperties()); assertEquals( - ImmutableSet.of(Component.of("NIMBUS", "c7401.ambari.apache.org"), Component.of("BEACON_SERVER", "c7402.ambari.apache.org")), + ImmutableSet.of(Component.of("NIMBUS", "c7401.ambari.apache.org", "c7402.ambari.apache.org"), Component.of("BEACON_SERVER", "c7402.ambari.apache.org", "c7403.ambari.apache.org")), request.getComponents()); assertEquals( @@ -137,7 +123,7 @@ public class AddServiceRequestTest { // filled-out values assertEquals( - ImmutableSet.of(Component.of("NIMBUS", "c7401.ambari.apache.org"), Component.of("BEACON_SERVER", "c7402.ambari.apache.org")), + ImmutableSet.of(Component.of("NIMBUS", "c7401.ambari.apache.org", "c7402.ambari.apache.org"), Component.of("BEACON_SERVER", "c7402.ambari.apache.org", "c7403.ambari.apache.org")), request.getComponents()); assertEquals( @@ -187,7 +173,7 @@ public class AddServiceRequestTest { // filled-out values assertEquals( - ImmutableSet.of(Component.of("NIMBUS", "c7401.ambari.apache.org"), Component.of("BEACON_SERVER", "c7402.ambari.apache.org")), + ImmutableSet.of(Component.of("NIMBUS", "c7401.ambari.apache.org", "c7402.ambari.apache.org"), Component.of("BEACON_SERVER", "c7402.ambari.apache.org", "c7403.ambari.apache.org")), request.getComponents()); // default / empty values @@ -223,78 +209,27 @@ public class AddServiceRequestTest { @Test public void testSerialize_basic() throws Exception { AddServiceRequest request = mapper.readValue(REQUEST_ALL_FIELDS_SET, AddServiceRequest.class); - - Map<String, ?> serialized = serialize(request); - - assertEquals(AddServiceRequest.OperationType.ADD_SERVICE.name(), serialized.get(OPERATION_TYPE)); - assertEquals(ConfigRecommendationStrategy.ALWAYS_APPLY.name(), serialized.get(CONFIG_RECOMMENDATION_STRATEGY)); - assertEquals(ProvisionAction.INSTALL_ONLY.name(), serialized.get(PROVISION_ACTION_PROPERTY)); - assertEquals("HDP", serialized.get(STACK_NAME)); - assertEquals("3.0", serialized.get(STACK_VERSION)); - - assertEquals( - ImmutableSet.of(ImmutableMap.of(Service.NAME, "BEACON"), ImmutableMap.of(Service.NAME, "STORM")), - ImmutableSet.copyOf((List<String>) serialized.get(SERVICES)) ); - - assertEquals( - ImmutableSet.of( - ImmutableMap.of(Component.COMPONENT_NAME, "NIMBUS", Component.FQDN, "c7401.ambari.apache.org"), - ImmutableMap.of(Component.COMPONENT_NAME, "BEACON_SERVER", Component.FQDN, "c7402.ambari.apache.org")), - ImmutableSet.copyOf((List<String>) serialized.get(COMPONENTS)) ); - - assertEquals( - ImmutableList.of( - ImmutableMap.of( - "storm-site", - ImmutableMap.of( - "properties", ImmutableMap.of("ipc.client.connect.max.retries", "50"), - "properties_attributes", ImmutableMap.of("final", ImmutableMap.of("fs.defaultFS", "true")) - ) - ) - ), - serialized.get(Configurable.CONFIGURATIONS) - ); - - assertEquals( - ImmutableMap.of( - SecurityConfigurationFactory.TYPE_PROPERTY_ID, SecurityType.KERBEROS.name(), - SecurityConfigurationFactory.KERBEROS_DESCRIPTOR_PROPERTY_ID, KERBEROS_DESCRIPTOR1, - SecurityConfigurationFactory.KERBEROS_DESCRIPTOR_REFERENCE_PROPERTY_ID, "ref_to_kerb_desc" - ), - serialized.get(ClusterResourceProvider.SECURITY) - ); - - assertNull(serialized.get(ClusterResourceProvider.CREDENTIALS)); + AddServiceRequest serialized = serialize(request); + assertEquals(request, serialized); + assertEquals(ImmutableMap.of(), serialized.getCredentials()); } @Test public void testSerialize_EmptyOmitted() throws Exception { AddServiceRequest request = mapper.readValue(REQUEST_MINIMAL_SERVICES_ONLY, AddServiceRequest.class); - Map<String, ?> serialized = serialize(request); - - assertEquals(AddServiceRequest.OperationType.ADD_SERVICE.name(), serialized.get(OPERATION_TYPE)); - assertEquals(ProvisionAction.INSTALL_AND_START.name(), serialized.get(PROVISION_ACTION_PROPERTY)); - assertEquals( - ImmutableSet.of(ImmutableMap.of(Service.NAME, "BEACON"), ImmutableMap.of(Service.NAME, "STORM")), - ImmutableSet.copyOf((List<String>) serialized.get(SERVICES)) ); - - assertFalse(serialized.containsKey(STACK_NAME)); - assertFalse(serialized.containsKey(STACK_VERSION)); - assertFalse(serialized.containsKey(Configurable.CONFIGURATIONS)); - assertFalse(serialized.containsKey(COMPONENTS)); - + AddServiceRequest serialized = serialize(request); + assertEquals(request, serialized); } - private Map<String, ?> serialize(AddServiceRequest request) throws IOException { + private AddServiceRequest serialize(AddServiceRequest request) throws IOException { String serialized = mapper.writerWithDefaultPrettyPrinter().writeValueAsString(request); - return mapper.readValue(serialized, new TypeReference<Map<String, ?>>() {}); + return mapper.readValue(serialized, AddServiceRequest.class); } private static String read(String resourceName) { try { return Resources.toString(Resources.getResource(resourceName), StandardCharsets.UTF_8); - } - catch (IOException e) { + } catch (IOException e) { throw new UncheckedIOException(e); } } diff --git a/ambari-server/src/test/resources/add_service_api/request1.json b/ambari-server/src/test/resources/add_service_api/request1.json index 0b4d64b..73320bd 100644 --- a/ambari-server/src/test/resources/add_service_api/request1.json +++ b/ambari-server/src/test/resources/add_service_api/request1.json @@ -12,12 +12,18 @@ "components" : [ { - "component_name" : "NIMBUS", - "fqdn" : "c7401.ambari.apache.org" + "name" : "NIMBUS", + "hosts": [ + { "fqdn" : "c7401.ambari.apache.org" }, + { "fqdn" : "c7402.ambari.apache.org" } + ] }, { - "component_name" : "BEACON_SERVER", - "fqdn" : "c7402.ambari.apache.org" + "name" : "BEACON_SERVER", + "hosts": [ + { "fqdn" : "c7402.ambari.apache.org" }, + { "fqdn" : "c7403.ambari.apache.org" } + ] } ], diff --git a/ambari-server/src/test/resources/add_service_api/request2.json b/ambari-server/src/test/resources/add_service_api/request2.json index f0e540f..74b1428 100644 --- a/ambari-server/src/test/resources/add_service_api/request2.json +++ b/ambari-server/src/test/resources/add_service_api/request2.json @@ -6,13 +6,19 @@ "components" : [ { - "component_name" : "NIMBUS", - "fqdn" : "c7401.ambari.apache.org" + "name" : "NIMBUS", + "hosts": [ + { "fqdn" : "c7401.ambari.apache.org" }, + { "fqdn" : "c7402.ambari.apache.org" } + ] }, { - "component_name" : "BEACON_SERVER", - "fqdn" : "c7402.ambari.apache.org" + "name" : "BEACON_SERVER", + "hosts": [ + { "fqdn" : "c7402.ambari.apache.org" }, + { "fqdn" : "c7403.ambari.apache.org" } + ] } ] -} \ No newline at end of file +} diff --git a/ambari-server/src/test/resources/add_service_api/request4.json b/ambari-server/src/test/resources/add_service_api/request4.json index b6fd0ea..f37eaac 100644 --- a/ambari-server/src/test/resources/add_service_api/request4.json +++ b/ambari-server/src/test/resources/add_service_api/request4.json @@ -1,13 +1,19 @@ { "components" : [ { - "component_name" : "NIMBUS", - "fqdn" : "c7401.ambari.apache.org" + "name" : "NIMBUS", + "hosts": [ + { "fqdn" : "c7401.ambari.apache.org" }, + { "fqdn" : "c7402.ambari.apache.org" } + ] }, { - "component_name" : "BEACON_SERVER", - "fqdn" : "c7402.ambari.apache.org" + "name" : "BEACON_SERVER", + "hosts": [ + { "fqdn" : "c7402.ambari.apache.org" }, + { "fqdn" : "c7403.ambari.apache.org" } + ] } ] -} \ No newline at end of file +}