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 2ae3987 AMBARI-25000. START_ONLY provision action may be applied to the wrong component (#2708) 2ae3987 is described below commit 2ae3987d2b2b443036f4ee5119ff6152340cf468 Author: Doroszlai, Attila <6454655+adorosz...@users.noreply.github.com> AuthorDate: Tue Dec 11 20:06:26 2018 +0100 AMBARI-25000. START_ONLY provision action may be applied to the wrong component (#2708) --- .../controller/AmbariManagementControllerImpl.java | 13 +--- .../internal/HostComponentResourceProvider.java | 38 ++++++++-- .../server/topology/ClusterTopologyImpl.java | 3 +- .../addservice/ResourceProviderAdapter.java | 10 +-- .../HostComponentResourceProviderTest.java | 84 ++++++++++++++++++++++ 5 files changed, 127 insertions(+), 21 deletions(-) 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 642960d..36e2e14 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 @@ -124,6 +124,7 @@ import org.apache.ambari.server.configuration.Configuration; import org.apache.ambari.server.configuration.Configuration.DatabaseType; import org.apache.ambari.server.controller.internal.DeleteHostComponentStatusMetaData; import org.apache.ambari.server.controller.internal.DeleteStatusMetaData; +import org.apache.ambari.server.controller.internal.HostComponentResourceProvider; import org.apache.ambari.server.controller.internal.RequestOperationLevel; import org.apache.ambari.server.controller.internal.RequestResourceFilter; import org.apache.ambari.server.controller.internal.RequestStageContainer; @@ -294,8 +295,6 @@ public class AmbariManagementControllerImpl implements AmbariManagementControlle private static final String PASSWORD = "password"; - public static final String SKIP_INSTALL_FOR_COMPONENTS = "skipInstallForComponents"; - public static final String DONT_SKIP_INSTALL_FOR_COMPONENTS = "dontSkipInstallForComponents"; public static final String CLUSTER_NAME_VALIDATION_REGEXP = "^[a-zA-Z0-9_-]{1,100}$"; public static final Pattern CLUSTER_NAME_PTRN = Pattern.compile(CLUSTER_NAME_VALIDATION_REGEXP); @@ -3355,15 +3354,7 @@ public class AmbariManagementControllerImpl implements AmbariManagementControlle isClientComponent = serviceComponent.isClientComponent(); } } - // Skip INSTALL for service components if START_ONLY is set for component, or if START_ONLY is set on cluster - // level and no other provsion action is specified for component - String skipInstallForComponents = requestProperties.get(SKIP_INSTALL_FOR_COMPONENTS); - return !isClientComponent && - CLUSTER_PHASE_INITIAL_INSTALL.equals(requestProperties.get(CLUSTER_PHASE_PROPERTY)) && - skipInstallForComponents != null && - (skipInstallForComponents.contains(componentName) || - (skipInstallForComponents.equals("ALL") && !requestProperties.get(DONT_SKIP_INSTALL_FOR_COMPONENTS).contains(componentName)) - ); + return HostComponentResourceProvider.shouldSkipInstallTaskForComponent(componentName, isClientComponent, requestProperties); } diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/HostComponentResourceProvider.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/HostComponentResourceProvider.java index d24034d..7cb532a 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/HostComponentResourceProvider.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/HostComponentResourceProvider.java @@ -34,7 +34,6 @@ import java.util.Set; import org.apache.ambari.server.AmbariException; import org.apache.ambari.server.controller.AmbariManagementController; -import org.apache.ambari.server.controller.AmbariManagementControllerImpl; import org.apache.ambari.server.controller.MaintenanceStateHelper; import org.apache.ambari.server.controller.RequestStatusResponse; import org.apache.ambari.server.controller.ServiceComponentHostRequest; @@ -72,6 +71,7 @@ import org.apache.commons.lang.StringUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.inject.Inject; @@ -172,6 +172,12 @@ public class HostComponentResourceProvider extends AbstractControllerResourcePro UPGRADE_STATE, QUERY_PARAMETERS_RUN_SMOKE_TEST_ID); + public static final String SKIP_INSTALL_FOR_COMPONENTS = "skipInstallForComponents"; + public static final String DO_NOT_SKIP_INSTALL_FOR_COMPONENTS = "dontSkipInstallForComponents"; + public static final String ALL_COMPONENTS = "ALL"; + public static final String SKIP_INSTALL_FOR_ALL_COMPONENTS = joinComponentList(ImmutableSet.of(ALL_COMPONENTS)); + public static final String DO_NOT_SKIP_INSTALL_FOR_ANY_COMPONENTS = joinComponentList(ImmutableSet.of()); + /** * maintenance state helper */ @@ -390,10 +396,7 @@ public class HostComponentResourceProvider extends AbstractControllerResourcePro Map<String, String> requestInfo = new HashMap<>(); requestInfo.put("context", String.format("Install components on host %s", hostname)); requestInfo.put(CLUSTER_PHASE_PROPERTY, CLUSTER_PHASE_INITIAL_INSTALL); - requestInfo.put(AmbariManagementControllerImpl.SKIP_INSTALL_FOR_COMPONENTS, StringUtils.join - (skipInstallForComponents, ";")); - requestInfo.put(AmbariManagementControllerImpl.DONT_SKIP_INSTALL_FOR_COMPONENTS, StringUtils.join - (dontSkipInstallForComponents, ";")); + addProvisionActionProperties(skipInstallForComponents, dontSkipInstallForComponents, requestInfo); Request installRequest = PropertyHelper.getUpdateRequest(installProperties, requestInfo); @@ -425,6 +428,31 @@ public class HostComponentResourceProvider extends AbstractControllerResourcePro return requestStages.getRequestStatusResponse(); } + @VisibleForTesting + static void addProvisionActionProperties(Collection<String> skipInstallForComponents, Collection<String> dontSkipInstallForComponents, Map<String, String> requestInfo) { + requestInfo.put(SKIP_INSTALL_FOR_COMPONENTS, joinComponentList(skipInstallForComponents)); + requestInfo.put(DO_NOT_SKIP_INSTALL_FOR_COMPONENTS, joinComponentList(dontSkipInstallForComponents)); + } + + public static String joinComponentList(Collection<String> components) { + return components != null + ? ";" + String.join(";", components) + ";" + : ""; + } + + public static boolean shouldSkipInstallTaskForComponent(String componentName, boolean isClientComponent, Map<String, String> requestProperties) { + // Skip INSTALL for service components if START_ONLY is set for component, or if START_ONLY is set on cluster + // level and no other provision action is specified for component + String skipInstallForComponents = requestProperties.get(SKIP_INSTALL_FOR_COMPONENTS); + String searchString = joinComponentList(ImmutableSet.of(componentName)); + return !isClientComponent && + CLUSTER_PHASE_INITIAL_INSTALL.equals(requestProperties.get(CLUSTER_PHASE_PROPERTY)) && + skipInstallForComponents != null && + (skipInstallForComponents.contains(searchString) || + (skipInstallForComponents.equals(SKIP_INSTALL_FOR_ALL_COMPONENTS) && + !requestProperties.get(DO_NOT_SKIP_INSTALL_FOR_COMPONENTS).contains(searchString)) + ); + } // TODO, revisit this extra method, that appears to be used during Add Hosts // TODO, How do we determine the component list for INSTALL_ONLY during an Add Hosts operation? rwn diff --git a/ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterTopologyImpl.java b/ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterTopologyImpl.java index 54abae2..c1c5790 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterTopologyImpl.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterTopologyImpl.java @@ -19,6 +19,7 @@ package org.apache.ambari.server.topology; import static java.util.stream.Collectors.toSet; +import static org.apache.ambari.server.controller.internal.HostComponentResourceProvider.ALL_COMPONENTS; 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.state.ServiceInfo.HADOOP_COMPATIBLE_FS; @@ -221,7 +222,7 @@ public class ClusterTopologyImpl implements ClusterTopology { Collection<String> skipInstallForComponents = new ArrayList<>(); if (skipInstallTaskCreate) { - skipInstallForComponents.add("ALL"); + skipInstallForComponents.add(ALL_COMPONENTS); } else { // get the set of components that are marked as START_ONLY for this hostgroup skipInstallForComponents.addAll(hostGroup.getComponentNames(ProvisionAction.START_ONLY)); diff --git a/ambari-server/src/main/java/org/apache/ambari/server/topology/addservice/ResourceProviderAdapter.java b/ambari-server/src/main/java/org/apache/ambari/server/topology/addservice/ResourceProviderAdapter.java index a7fe1f0..a33d617 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/topology/addservice/ResourceProviderAdapter.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/topology/addservice/ResourceProviderAdapter.java @@ -20,8 +20,10 @@ package org.apache.ambari.server.topology.addservice; import static java.util.stream.Collectors.toList; import static java.util.stream.Collectors.toSet; import static org.apache.ambari.server.controller.AmbariManagementControllerImpl.CLUSTER_PHASE_PROPERTY; -import static org.apache.ambari.server.controller.AmbariManagementControllerImpl.DONT_SKIP_INSTALL_FOR_COMPONENTS; -import static org.apache.ambari.server.controller.AmbariManagementControllerImpl.SKIP_INSTALL_FOR_COMPONENTS; +import static org.apache.ambari.server.controller.internal.HostComponentResourceProvider.DO_NOT_SKIP_INSTALL_FOR_ANY_COMPONENTS; +import static org.apache.ambari.server.controller.internal.HostComponentResourceProvider.DO_NOT_SKIP_INSTALL_FOR_COMPONENTS; +import static org.apache.ambari.server.controller.internal.HostComponentResourceProvider.SKIP_INSTALL_FOR_ALL_COMPONENTS; +import static org.apache.ambari.server.controller.internal.HostComponentResourceProvider.SKIP_INSTALL_FOR_COMPONENTS; import java.util.List; import java.util.Map; @@ -278,8 +280,8 @@ public class ResourceProviderAdapter { private static void addProvisionProperties(ImmutableMap.Builder<String, String> requestInfo, State desiredState, ProvisionAction requestAction) { if (desiredState == State.INSTALLED && requestAction.skipInstall()) { - requestInfo.put(SKIP_INSTALL_FOR_COMPONENTS, "ALL"); - requestInfo.put(DONT_SKIP_INSTALL_FOR_COMPONENTS, ""); + requestInfo.put(SKIP_INSTALL_FOR_COMPONENTS, SKIP_INSTALL_FOR_ALL_COMPONENTS); + requestInfo.put(DO_NOT_SKIP_INSTALL_FOR_COMPONENTS, DO_NOT_SKIP_INSTALL_FOR_ANY_COMPONENTS); requestInfo.put(CLUSTER_PHASE_PROPERTY, AmbariManagementControllerImpl.CLUSTER_PHASE_INITIAL_INSTALL); } } diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/HostComponentResourceProviderTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/HostComponentResourceProviderTest.java index 24dd8ce..26fd36e 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/HostComponentResourceProviderTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/HostComponentResourceProviderTest.java @@ -18,6 +18,10 @@ package org.apache.ambari.server.controller.internal; +import static org.apache.ambari.server.controller.AmbariManagementControllerImpl.CLUSTER_PHASE_INITIAL_INSTALL; +import static org.apache.ambari.server.controller.AmbariManagementControllerImpl.CLUSTER_PHASE_PROPERTY; +import static org.apache.ambari.server.controller.internal.HostComponentResourceProvider.ALL_COMPONENTS; +import static org.apache.ambari.server.controller.internal.HostComponentResourceProvider.shouldSkipInstallTaskForComponent; import static org.easymock.EasyMock.createMock; import static org.easymock.EasyMock.createNiceMock; import static org.easymock.EasyMock.eq; @@ -25,16 +29,20 @@ import static org.easymock.EasyMock.expect; import static org.easymock.EasyMock.replay; import static org.easymock.EasyMock.verify; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import java.lang.reflect.Field; import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.LinkedHashMap; import java.util.LinkedHashSet; +import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Set; @@ -599,6 +607,82 @@ public class HostComponentResourceProviderTest { verify(managementController, resourceProviderFactory, stageContainer); } + @Test + public void doesNotSkipInstallTaskForClient() { + String component = "SOME_COMPONENT"; + assertFalse(shouldSkipInstallTaskForComponent(component, true, new RequestInfoBuilder().skipInstall(component).build())); + assertFalse(shouldSkipInstallTaskForComponent(component, true, new RequestInfoBuilder().skipInstall(ALL_COMPONENTS).build())); + } + + @Test + public void doesNotSkipInstallTaskForOtherPhase() { + String component = "SOME_COMPONENT"; + RequestInfoBuilder requestInfoBuilder = new RequestInfoBuilder().phase("INSTALL"); + assertFalse(shouldSkipInstallTaskForComponent(component, false, requestInfoBuilder.skipInstall(component).build())); + assertFalse(shouldSkipInstallTaskForComponent(component, false, requestInfoBuilder.skipInstall(ALL_COMPONENTS).build())); + } + + @Test + public void doesNotSkipInstallTaskForExplicitException() { + String component = "SOME_COMPONENT"; + RequestInfoBuilder requestInfoBuilder = new RequestInfoBuilder().skipInstall(ALL_COMPONENTS).doNotSkipInstall(component); + assertFalse(shouldSkipInstallTaskForComponent(component, false, requestInfoBuilder.build())); + } + + @Test + public void skipsInstallTaskIfRequested() { + String component = "SOME_COMPONENT"; + RequestInfoBuilder requestInfoBuilder = new RequestInfoBuilder().skipInstall(component); + assertTrue(shouldSkipInstallTaskForComponent(component, false, requestInfoBuilder.build())); + } + + @Test + public void skipsInstallTaskForAll() { + RequestInfoBuilder requestInfoBuilder = new RequestInfoBuilder().skipInstall(ALL_COMPONENTS); + assertTrue(shouldSkipInstallTaskForComponent("ANY_COMPONENT", false, requestInfoBuilder.build())); + } + + @Test + public void doesNotSkipInstallOfPrefixedComponent() { + String prefix = "HIVE_SERVER", component = prefix + "_INTERACTIVE"; + Map<String, String> requestInfo = new RequestInfoBuilder().skipInstall(component).build(); + assertTrue(shouldSkipInstallTaskForComponent(component, false, requestInfo)); + assertFalse(shouldSkipInstallTaskForComponent(prefix, false, requestInfo)); + } + + private static class RequestInfoBuilder { + + private String phase = CLUSTER_PHASE_INITIAL_INSTALL; + private final Collection<String> skipInstall = new LinkedList<>(); + private final Collection<String> doNotSkipInstall = new LinkedList<>(); + + public RequestInfoBuilder skipInstall(String... components) { + skipInstall.clear(); + skipInstall.addAll(Arrays.asList(components)); + return this; + } + + public RequestInfoBuilder doNotSkipInstall(String... components) { + doNotSkipInstall.clear(); + doNotSkipInstall.addAll(Arrays.asList(components)); + return this; + } + + public RequestInfoBuilder phase(String phase) { + this.phase = phase; + return this; + } + + public Map<String, String> build() { + Map<String, String> info = new HashMap<>(); + if (phase != null) { + info.put(CLUSTER_PHASE_PROPERTY, phase); + } + HostComponentResourceProvider.addProvisionActionProperties(skipInstall, doNotSkipInstall, info); + return info; + } + } + // Used to directly call updateHostComponents on the resource provider. // This exists as a temporary solution as a result of moving updateHostComponents from // AmbariManagentControllerImpl to HostComponentResourceProvider.