Revert "AMBARI-19055 - Removing Tasks From host_role_command Causes Upgrades To Show As PENDING (jonathanhurley)"
This reverts commit 32840c1ed434ae4cc99e3ab6f7d3f2604fb69d06. Project: http://git-wip-us.apache.org/repos/asf/ambari/repo Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/5ec73bce Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/5ec73bce Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/5ec73bce Branch: refs/heads/branch-dev-patch-upgrade Commit: 5ec73bce006a659f33c09c57cb50f28037eca5d7 Parents: 3096c79 Author: Jonathan Hurley <[email protected]> Authored: Sat Dec 3 08:22:31 2016 -0500 Committer: Jonathan Hurley <[email protected]> Committed: Sat Dec 3 08:22:31 2016 -0500 ---------------------------------------------------------------------- .../controller/internal/CalculatedStatus.java | 52 +++++--------------- .../internal/RequestResourceProvider.java | 34 +++++-------- .../internal/StageResourceProvider.java | 2 +- .../internal/CalculatedStatusTest.java | 31 ------------ .../internal/RequestResourceProviderTest.java | 12 ++--- 5 files changed, 28 insertions(+), 103 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ambari/blob/5ec73bce/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/CalculatedStatus.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/CalculatedStatus.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/CalculatedStatus.java index 3c415df..3a86aef 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/CalculatedStatus.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/CalculatedStatus.java @@ -53,17 +53,6 @@ public class CalculatedStatus { */ private final double percent; - /** - * A status which represents a COMPLETED state at 0% - */ - public static final CalculatedStatus COMPLETED = new CalculatedStatus(HostRoleStatus.COMPLETED, - HostRoleStatus.COMPLETED, 100.0); - - /** - * A status which represents a PENDING state at 0% - */ - public static final CalculatedStatus PENDING = new CalculatedStatus(HostRoleStatus.PENDING, - HostRoleStatus.PENDING, 0.0); // ----- Constructors ------------------------------------------------------ @@ -90,6 +79,12 @@ public class CalculatedStatus { this.percent = percent; } + /** + * Static factory method to get Status that represents a Completed state + */ + public static CalculatedStatus getCompletedStatus() { + return new CalculatedStatus(HostRoleStatus.COMPLETED, HostRoleStatus.COMPLETED, 100.0); + } // ----- CalculatedStatus -------------------------------------------------- @@ -296,25 +291,14 @@ public class CalculatedStatus { } /** - * Calculates the overall status of an upgrade. If there are no tasks, then a - * status of {@link HostRoleStatus#COMPLETED} is returned. - * - * @param stageDto - * the map of stage-to-summary value objects - * @param stageIds - * the stage ids to consider from the value objects + * Calculates the overall status of an upgrade. + * @param stageDto the map of stage-to-summary value objects + * @param stageIds the stage ids to consider from the value objects * @return the calculated status */ public static CalculatedStatus statusFromStageSummary(Map<Long, HostRoleCommandStatusSummaryDTO> stageDto, Set<Long> stageIds) { - // if either are empty, then we have no tasks and therefore no status - we - // should return COMPLETED. This can happen if someone removes all tasks but - // leaves the stages and request - if (stageDto.isEmpty() || stageIds.isEmpty()) { - return COMPLETED; - } - Collection<HostRoleStatus> stageStatuses = new HashSet<>(); Collection<HostRoleStatus> stageDisplayStatuses = new HashSet<>(); Collection<HostRoleStatus> taskStatuses = new ArrayList<>(); @@ -394,28 +378,19 @@ public class CalculatedStatus { */ public static HostRoleStatus calculateSummaryStatusOfStage(Map<HostRoleStatus, Integer> counters, int total, boolean skippable) { - - // when there are 0 tasks, return COMPLETED - if (total == 0) { - return HostRoleStatus.COMPLETED; - } - if (counters.get(HostRoleStatus.PENDING) == total) { return HostRoleStatus.PENDING; } - // By definition, any tasks in a future stage must be held in a PENDING status. if (counters.get(HostRoleStatus.HOLDING) > 0 || counters.get(HostRoleStatus.HOLDING_FAILED) > 0 || counters.get(HostRoleStatus.HOLDING_TIMEDOUT) > 0) { return counters.get(HostRoleStatus.HOLDING) > 0 ? HostRoleStatus.HOLDING : counters.get(HostRoleStatus.HOLDING_FAILED) > 0 ? HostRoleStatus.HOLDING_FAILED : HostRoleStatus.HOLDING_TIMEDOUT; } - // Because tasks are not skippable, guaranteed to be FAILED if (counters.get(HostRoleStatus.FAILED) > 0 && !skippable) { return HostRoleStatus.FAILED; } - // Because tasks are not skippable, guaranteed to be TIMEDOUT if (counters.get(HostRoleStatus.TIMEDOUT) > 0 && !skippable) { return HostRoleStatus.TIMEDOUT; @@ -426,11 +401,9 @@ public class CalculatedStatus { if (counters.get(HostRoleStatus.ABORTED) > 0 && numActiveTasks == 0) { return HostRoleStatus.ABORTED; } - if (counters.get(HostRoleStatus.COMPLETED) == total) { return HostRoleStatus.COMPLETED; } - return HostRoleStatus.IN_PROGRESS; } @@ -442,8 +415,7 @@ public class CalculatedStatus { * * @return summary request status based on statuses of tasks in different states. */ - protected static HostRoleStatus calculateSummaryStatusOfUpgrade( - Map<HostRoleStatus, Integer> counters, int total) { + private static HostRoleStatus calculateSummaryStatusOfUpgrade(Map<HostRoleStatus, Integer> counters, int total) { return calculateSummaryStatusOfStage(counters, total, false); } @@ -456,8 +428,8 @@ public class CalculatedStatus { * * @return summary request status based on statuses of tasks in different states. */ - protected static HostRoleStatus calculateSummaryDisplayStatus( - Map<HostRoleStatus, Integer> counters, int total, boolean skippable) { + private static HostRoleStatus calculateSummaryDisplayStatus(Map<HostRoleStatus, Integer> counters, + int total, boolean skippable) { return counters.get(HostRoleStatus.SKIPPED_FAILED) > 0 ? HostRoleStatus.SKIPPED_FAILED : counters.get(HostRoleStatus.FAILED) > 0 ? HostRoleStatus.FAILED: calculateSummaryStatusOfStage(counters, total, skippable); http://git-wip-us.apache.org/repos/asf/ambari/blob/5ec73bce/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/RequestResourceProvider.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/RequestResourceProvider.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/RequestResourceProvider.java index 0690ee7..8c1bc57 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/RequestResourceProvider.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/RequestResourceProvider.java @@ -69,10 +69,10 @@ import org.apache.ambari.server.state.Cluster; import org.apache.ambari.server.state.Clusters; import org.apache.ambari.server.topology.LogicalRequest; import org.apache.ambari.server.topology.TopologyManager; -import org.apache.commons.lang.StringUtils; import com.google.common.collect.Sets; import com.google.inject.Inject; +import org.apache.commons.lang.StringUtils; /** * Resource provider for request resources. @@ -566,18 +566,13 @@ public class RequestResourceProvider extends AbstractControllerResourceProvider @Override public boolean equals(Object o) { - if (this == o) { - return true; - } - if (o == null || getClass() != o.getClass()) { - return false; - } + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; ServiceComponentTuple that = (ServiceComponentTuple) o; - if (serviceName != null ? !serviceName.equals(that.serviceName) : that.serviceName != null) { + if (serviceName != null ? !serviceName.equals(that.serviceName) : that.serviceName != null) return false; - } return !(componentName != null ? !componentName.equals(that.componentName) : that.componentName != null); } @@ -744,20 +739,15 @@ public class RequestResourceProvider extends AbstractControllerResourceProvider // get summaries from TopologyManager for logical requests summary.putAll(topologyManager.getStageSummaries(entity.getRequestId())); - // summary might be empty due to delete host have cleared all - // HostRoleCommands or due to hosts haven't registered yet with the cluster - // when the cluster is provisioned with a Blueprint - final CalculatedStatus status; LogicalRequest logicalRequest = topologyManager.getRequest(entity.getRequestId()); - if (summary.isEmpty() && null != logicalRequest) { - // in this case, it appears that there are no tasks but this is a logical - // topology request, so it's a matter of hosts simply not registering yet - // for tasks to be created - status = CalculatedStatus.PENDING; - } else { - // there are either tasks or this is not a logical request, so do normal - // status calculations - status = CalculatedStatus.statusFromStageSummary(summary, summary.keySet()); + + CalculatedStatus status = CalculatedStatus.statusFromStageSummary(summary, summary.keySet()); + if (summary.isEmpty() && logicalRequest == null) { + + // summary might be empty due to delete host have cleared all HostRoleCommands + // or due to hosts haven't registered yet with the cluster when the cluster is provisioned + // with a Blueprint + status = CalculatedStatus.getCompletedStatus(); } setResourceProperty(resource, REQUEST_STATUS_PROPERTY_ID, status.getStatus().toString(), requestedPropertyIds); http://git-wip-us.apache.org/repos/asf/ambari/blob/5ec73bce/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/StageResourceProvider.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/StageResourceProvider.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/StageResourceProvider.java index a778882..59dd9d9 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/StageResourceProvider.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/StageResourceProvider.java @@ -339,7 +339,7 @@ public class StageResourceProvider extends AbstractControllerResourceProvider im CalculatedStatus status; if (summary.isEmpty()) { // Delete host might have cleared all HostRoleCommands - status = CalculatedStatus.COMPLETED; + status = CalculatedStatus.getCompletedStatus(); } else { status = CalculatedStatus.statusFromStageSummary(summary, Collections.singleton(entity.getStageId())); } http://git-wip-us.apache.org/repos/asf/ambari/blob/5ec73bce/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/CalculatedStatusTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/CalculatedStatusTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/CalculatedStatusTest.java index a96f395..6f592cd 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/CalculatedStatusTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/CalculatedStatusTest.java @@ -657,37 +657,6 @@ public class CalculatedStatusTest { assertEquals(HostRoleStatus.IN_PROGRESS, calc.getStatus()); } - /** - * Tests that when there are no tasks and all counts are 0, that the returned - * status is {@link HostRoleStatus#COMPLETED}. - * - * @throws Exception - */ - @Test - public void testGetCompletedStatusForNoTasks() throws Exception { - // no status / no tasks - CalculatedStatus status = CalculatedStatus.statusFromTaskEntities( - new ArrayList<HostRoleCommandEntity>(), false); - - assertEquals(HostRoleStatus.COMPLETED, status.getStatus()); - - // empty summaries - status = CalculatedStatus.statusFromStageSummary( - new HashMap<Long, HostRoleCommandStatusSummaryDTO>(), new HashSet<Long>()); - - assertEquals(HostRoleStatus.COMPLETED, status.getStatus()); - - // generate a map of 0's - COMPLETED=0, IN_PROGRESS=0, etc - Map<HostRoleStatus, Integer> counts = CalculatedStatus.calculateStatusCounts(new ArrayList<HostRoleStatus>()); - Map<HostRoleStatus, Integer> displayCounts = CalculatedStatus.calculateStatusCounts(new ArrayList<HostRoleStatus>()); - - HostRoleStatus hostRoleStatus = CalculatedStatus.calculateSummaryStatusOfUpgrade(counts, 0); - HostRoleStatus hostRoleDisplayStatus = CalculatedStatus.calculateSummaryDisplayStatus(displayCounts, 0, false); - - assertEquals(HostRoleStatus.COMPLETED, hostRoleStatus); - assertEquals(HostRoleStatus.COMPLETED, hostRoleDisplayStatus); - } - private Collection<HostRoleCommandEntity> getTaskEntities(HostRoleStatus... statuses) { Collection<HostRoleCommandEntity> entities = new LinkedList<HostRoleCommandEntity>(); http://git-wip-us.apache.org/repos/asf/ambari/blob/5ec73bce/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/RequestResourceProviderTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/RequestResourceProviderTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/RequestResourceProviderTest.java index f7dff11..5dfc74d 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/RequestResourceProviderTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/RequestResourceProviderTest.java @@ -20,10 +20,11 @@ package org.apache.ambari.server.controller.internal; import static org.apache.ambari.server.controller.internal.HostComponentResourceProvider.HOST_COMPONENT_STALE_CONFIGS_PROPERTY_ID; +import org.apache.ambari.server.topology.Blueprint; import static org.easymock.EasyMock.anyObject; import static org.easymock.EasyMock.capture; -import static org.easymock.EasyMock.eq; import static org.easymock.EasyMock.expect; +import static org.easymock.EasyMock.eq; import static org.easymock.EasyMock.newCapture; import static org.powermock.api.easymock.PowerMock.createMock; import static org.powermock.api.easymock.PowerMock.createNiceMock; @@ -76,7 +77,6 @@ import org.apache.ambari.server.security.authorization.AuthorizationHelperInitia import org.apache.ambari.server.security.authorization.RoleAuthorization; import org.apache.ambari.server.state.Cluster; import org.apache.ambari.server.state.Clusters; -import org.apache.ambari.server.topology.Blueprint; import org.apache.ambari.server.topology.ClusterTopology; import org.apache.ambari.server.topology.HostGroupInfo; import org.apache.ambari.server.topology.LogicalRequest; @@ -1200,7 +1200,7 @@ public class RequestResourceProviderTest { } } Assert.assertNotNull(propertyIdToAssert); - Assert.assertEquals("true", propertyValueToAssert); + Assert.assertEquals("true", (String) propertyValueToAssert); } @Test @@ -1624,12 +1624,6 @@ public class RequestResourceProviderTest { verify(managementController, actionManager, clusters, requestMock, requestDAO, hrcDAO); } - /** - * Tests that topology requests return different status (PENDING) if there are - * no tasks. Normal requests should return COMPLETED. - * - * @throws Exception - */ @Test @PrepareForTest(AmbariServer.class) public void testGetLogicalRequestStatusWithNoTasks() throws Exception {
