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.
