AMBARI-22479 After removing force_delete_components option hosts are not deleted (dsen)
Project: http://git-wip-us.apache.org/repos/asf/ambari/repo Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/1f7bd75e Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/1f7bd75e Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/1f7bd75e Branch: refs/heads/branch-3.0-perf Commit: 1f7bd75e0e8514539ca27fe53b7a39a01a11c285 Parents: 19e6518 Author: Dmytro Sen <[email protected]> Authored: Thu Nov 30 10:45:18 2017 +0200 Committer: Dmytro Sen <[email protected]> Committed: Thu Nov 30 10:45:18 2017 +0200 ---------------------------------------------------------------------- .../AmbariManagementControllerImpl.java | 9 ---- .../internal/HostResourceProvider.java | 54 ++++++-------------- .../AmbariManagementControllerTest.java | 42 +++++---------- .../internal/HostResourceProviderTest.java | 6 +-- 4 files changed, 30 insertions(+), 81 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ambari/blob/1f7bd75e/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java ---------------------------------------------------------------------- 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 3d09154..455814a 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 @@ -3477,15 +3477,6 @@ public class AmbariManagementControllerImpl implements AmbariManagementControlle + ", hostname=" + request.getHostname() + ", request=" + request); } - - // Only allow removing master/slave components in DISABLED/UNKNOWN/INSTALL_FAILED/INIT state without stages - // generation. - // Clients may be removed without a state check. - if (!component.isClientComponent() && - !componentHost.getState().isRemovableState()) { - throw new AmbariException("To remove master or slave components they must be in " + - "DISABLED/INIT/INSTALLED/INSTALL_FAILED/UNKNOWN state. Current=" + componentHost.getState() + "."); - } } @Override http://git-wip-us.apache.org/repos/asf/ambari/blob/1f7bd75e/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/HostResourceProvider.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/HostResourceProvider.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/HostResourceProvider.java index 2b18eb2..5c740f1 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/HostResourceProvider.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/HostResourceProvider.java @@ -62,7 +62,6 @@ import org.apache.ambari.server.state.DesiredConfig; import org.apache.ambari.server.state.Host; import org.apache.ambari.server.state.MaintenanceState; import org.apache.ambari.server.state.ServiceComponentHost; -import org.apache.ambari.server.state.State; import org.apache.ambari.server.state.stack.OsFamily; import org.apache.ambari.server.topology.ClusterTopology; import org.apache.ambari.server.topology.InvalidTopologyException; @@ -146,9 +145,6 @@ public class HostResourceProvider extends AbstractControllerResourceProvider { //todo use the same json structure for cluster host addition (cluster template and upscale) - protected static final String FORCE_DELETE_COMPONENTS = "force_delete_components"; - - private static final Set<String> PK_PROPERTY_IDS = ImmutableSet.of(HOST_HOST_NAME_PROPERTY_ID); @Inject @@ -320,8 +316,6 @@ public class HostResourceProvider extends AbstractControllerResourceProvider { final Set<HostRequest> requests = new HashSet<>(); Map<String, String> requestInfoProperties = request.getRequestInfoProperties(); - final boolean forceDelete = requestInfoProperties.containsKey(FORCE_DELETE_COMPONENTS) && - requestInfoProperties.get(FORCE_DELETE_COMPONENTS).equals("true"); for (Map<String, Object> propertyMap : getPropertyMaps(predicate)) { requests.add(getRequest(propertyMap)); @@ -330,7 +324,7 @@ public class HostResourceProvider extends AbstractControllerResourceProvider { DeleteStatusMetaData deleteStatusMetaData = modifyResources(new Command<DeleteStatusMetaData>() { @Override public DeleteStatusMetaData invoke() throws AmbariException { - return deleteHosts(requests, request.isDryRunRequest(), forceDelete); + return deleteHosts(requests, request.isDryRunRequest()); } }); @@ -848,7 +842,7 @@ public class HostResourceProvider extends AbstractControllerResourceProvider { } @Transactional - protected DeleteStatusMetaData deleteHosts(Set<HostRequest> requests, boolean dryRun, boolean forceDelete) + protected DeleteStatusMetaData deleteHosts(Set<HostRequest> requests, boolean dryRun) throws AmbariException { AmbariManagementController controller = getManagementController(); @@ -863,7 +857,7 @@ public class HostResourceProvider extends AbstractControllerResourceProvider { } try { - validateHostInDeleteFriendlyState(hostRequest, clusters, forceDelete); + validateHostInDeleteFriendlyState(hostRequest, clusters); okToRemove.add(hostRequest); } catch (Exception ex) { deleteStatusMetaData.addException(hostName, ex); @@ -964,7 +958,7 @@ public class HostResourceProvider extends AbstractControllerResourceProvider { clusters.publishHostsDeletion(allClustersWithHosts, hostNames); } - private void validateHostInDeleteFriendlyState(HostRequest hostRequest, Clusters clusters, boolean forceDelete) throws AmbariException { + private void validateHostInDeleteFriendlyState(HostRequest hostRequest, Clusters clusters) throws AmbariException { Set<String> clusterNamesForHost = new HashSet<>(); String hostName = hostRequest.getHostname(); if (null != hostRequest.getClusterName()) { @@ -984,43 +978,25 @@ public class HostResourceProvider extends AbstractControllerResourceProvider { List<ServiceComponentHost> list = cluster.getServiceComponentHosts(hostName); if (!list.isEmpty()) { - List<String> componentsToRemove = new ArrayList<>(); List<String> componentsStarted = new ArrayList<>(); for (ServiceComponentHost sch : list) { - componentsToRemove.add(sch.getServiceComponentName()); - if (sch.getState() == State.STARTED) { + if (!sch.canBeRemoved()) { componentsStarted.add(sch.getServiceComponentName()); } } - if (forceDelete) { - // error if components are running - if (!componentsStarted.isEmpty()) { - StringBuilder reason = new StringBuilder("Cannot remove host ") - .append(hostName) - .append(" from ") - .append(hostRequest.getClusterName()) - .append( - ". The following roles exist, and these components must be stopped: "); + // error if components are running + if (!componentsStarted.isEmpty()) { + StringBuilder reason = new StringBuilder("Cannot remove host ") + .append(hostName) + .append(" from ") + .append(hostRequest.getClusterName()) + .append( + ". The following roles exist, and these components are not in the removable state: "); - reason.append(StringUtils.join(componentsToRemove, ", ")); + reason.append(StringUtils.join(componentsStarted, ", ")); - throw new AmbariException(reason.toString()); - } - } else { -// TODO why host with all components stopped can't be deleted? This functional is implemented and only this validation stops the request. - if (!componentsToRemove.isEmpty()) { - StringBuilder reason = new StringBuilder("Cannot remove host ") - .append(hostName) - .append(" from ") - .append(hostRequest.getClusterName()) - .append( - ". The following roles exist, and these components must be stopped if running, and then deleted: "); - - reason.append(StringUtils.join(componentsToRemove, ", ")); - - throw new AmbariException(reason.toString()); - } + throw new AmbariException(reason.toString()); } } } http://git-wip-us.apache.org/repos/asf/ambari/blob/1f7bd75e/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java index f8b6709..e76ecb9 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java @@ -8001,7 +8001,7 @@ public class AmbariManagementControllerTest { } @Test - public void testDeleteHostComponentWithForce() throws Exception { + public void testDeleteHostWithComponent() throws Exception { String cluster1 = getUniqueName(); createCluster(cluster1); @@ -8045,51 +8045,39 @@ public class AmbariManagementControllerTest { cHost.handleEvent(new ServiceComponentHostOpSucceededEvent(cHost.getServiceComponentName(), cHost.getHostName(), System.currentTimeMillis())); } - // Case 1: Attempt delete when components still exist + // Case 1: Attempt delete when some components are STARTED Set<HostRequest> requests = new HashSet<>(); requests.clear(); requests.add(new HostRequest(host1, cluster1)); - try { - HostResourceProviderTest.deleteHosts(controller, requests, false, false); - fail("Expect failure deleting hosts when components exist and have not been deleted."); - } catch (Exception e) { - LOG.info("Exception is - " + e.getMessage()); - Assert.assertTrue(e.getMessage().contains("these components must be stopped if running, and then deleted")); - } Service s = cluster.getService(serviceName); s.getServiceComponent("DATANODE").getServiceComponentHost(host1).setState(State.STARTED); try { - HostResourceProviderTest.deleteHosts(controller, requests, false, true); + HostResourceProviderTest.deleteHosts(controller, requests, false); fail("Expect failure deleting hosts when components exist and have not been stopped."); } catch (Exception e) { LOG.info("Exception is - " + e.getMessage()); - Assert.assertTrue(e.getMessage().contains("these components must be stopped:")); + Assert.assertTrue(e.getMessage().contains("these components are not in the removable state:")); } + // Case 2: Attempt delete dryRun = true DeleteStatusMetaData data = null; - try { - data = HostResourceProviderTest.deleteHosts(controller, requests, true, true); - Assert.assertTrue(data.getDeletedKeys().size() == 0); - } catch (Exception e) { - LOG.info("Exception is - " + e.getMessage()); - fail("Do not expect failure deleting hosts when components exist and are stopped."); - } LOG.info("Test dry run of delete with all host components"); s.getServiceComponent("DATANODE").getServiceComponentHost(host1).setState(State.INSTALLED); try { - data = HostResourceProviderTest.deleteHosts(controller, requests, true, true); + data = HostResourceProviderTest.deleteHosts(controller, requests, true); Assert.assertTrue(data.getDeletedKeys().size() == 1); } catch (Exception e) { LOG.info("Exception is - " + e.getMessage()); fail("Do not expect failure deleting hosts when components exist and are stopped."); } + // Case 3: Attempt delete dryRun = false LOG.info("Test successful delete with all host components"); s.getServiceComponent("DATANODE").getServiceComponentHost(host1).setState(State.INSTALLED); try { - data = HostResourceProviderTest.deleteHosts(controller, requests, false, true); + data = HostResourceProviderTest.deleteHosts(controller, requests, false); Assert.assertNotNull(data); Assert.assertTrue(4 == data.getDeletedKeys().size()); Assert.assertTrue(0 == data.getExceptionForKeys().size()); @@ -8154,17 +8142,11 @@ public class AmbariManagementControllerTest { cHost.handleEvent(new ServiceComponentHostOpSucceededEvent(cHost.getServiceComponentName(), cHost.getHostName(), System.currentTimeMillis())); } - // Case 1: Attempt delete when components still exist Set<HostRequest> requests = new HashSet<>(); requests.clear(); requests.add(new HostRequest(host1, cluster1)); - try { - HostResourceProviderTest.deleteHosts(controller, requests); - fail("Expect failure deleting hosts when components exist and have not been deleted."); - } catch (Exception e) { - } - // Case 2: Delete host that is still part of cluster, but do not specify the cluster_name in the request + // Case 1: Delete host that is still part of cluster, but do not specify the cluster_name in the request Set<ServiceComponentHostRequest> schRequests = new HashSet<>(); // Disable HC for non-clients schRequests.add(new ServiceComponentHostRequest(cluster1, serviceName, componentName1, host1, "DISABLED")); @@ -8201,7 +8183,7 @@ public class AmbariManagementControllerTest { List<HostRoleCommandEntity> tasks = hostRoleCommandDAO.findByHostId(firstHostId); assertEquals(0, tasks.size()); - // Case 3: Delete host that is still part of the cluster, and specify the cluster_name in the request + // Case 2: Delete host that is still part of the cluster, and specify the cluster_name in the request requests.clear(); requests.add(new HostRequest(host2, cluster1)); try { @@ -8214,7 +8196,7 @@ public class AmbariManagementControllerTest { Assert.assertFalse(clusters.getClustersForHost(host2).contains(cluster)); Assert.assertNull(topologyHostInfoDAO.findByHostname(host2)); - // Case 4: Attempt to delete a host that has already been deleted + // Case 3: Attempt to delete a host that has already been deleted requests.clear(); requests.add(new HostRequest(host1, null)); try { @@ -8232,7 +8214,7 @@ public class AmbariManagementControllerTest { // expected } - // Case 5: Attempt to delete a host that was never added to the cluster + // Case 4: Attempt to delete a host that was never added to the cluster requests.clear(); requests.add(new HostRequest(host3, null)); try { http://git-wip-us.apache.org/repos/asf/ambari/blob/1f7bd75e/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/HostResourceProviderTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/HostResourceProviderTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/HostResourceProviderTest.java index fd28081..5e6201b 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/HostResourceProviderTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/HostResourceProviderTest.java @@ -1346,11 +1346,11 @@ public class HostResourceProviderTest extends EasyMockSupport { HostResourceProvider provider = getHostProvider(controller); HostResourceProvider.setTopologyManager(topologyManager); - provider.deleteHosts(requests, false, false); + provider.deleteHosts(requests, false); } public static DeleteStatusMetaData deleteHosts(AmbariManagementController controller, - Set<HostRequest> requests, boolean dryRun, boolean forceDelete) + Set<HostRequest> requests, boolean dryRun) throws AmbariException { TopologyManager topologyManager = EasyMock.createNiceMock(TopologyManager.class); expect(topologyManager.getRequests(Collections.emptyList())).andReturn(Collections.emptyList()).anyTimes(); @@ -1359,7 +1359,7 @@ public class HostResourceProviderTest extends EasyMockSupport { HostResourceProvider provider = getHostProvider(controller); HostResourceProvider.setTopologyManager(topologyManager); - return provider.deleteHosts(requests, dryRun, forceDelete); + return provider.deleteHosts(requests, dryRun); } public static void updateHosts(AmbariManagementController controller, Set<HostRequest> requests)
