Repository: ambari Updated Branches: refs/heads/branch-2.6 e3df8806f -> ef0973a1d
AMBARI-22166 - Not able to perform revert after deleting the upgraded service (jonathanhurley) Project: http://git-wip-us.apache.org/repos/asf/ambari/repo Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/ef0973a1 Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/ef0973a1 Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/ef0973a1 Branch: refs/heads/branch-2.6 Commit: ef0973a1d1e12cafc9aa3e8f88c1af27683d947c Parents: e3df880 Author: Jonathan Hurley <[email protected]> Authored: Mon Oct 9 13:21:40 2017 -0400 Committer: Jonathan Hurley <[email protected]> Committed: Mon Oct 9 16:05:54 2017 -0400 ---------------------------------------------------------------------- .../ambari/server/state/UpgradeContext.java | 20 +++++-- .../ambari/server/state/UpgradeContextTest.java | 60 +++++++++++++++++++- 2 files changed, 74 insertions(+), 6 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ambari/blob/ef0973a1/ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeContext.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeContext.java b/ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeContext.java index 9f31ab8..ae45cbd 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeContext.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeContext.java @@ -323,11 +323,23 @@ public class UpgradeContext { revertableUpgrade.getRepositoryVersion().getVersion())); } + // !!! build all service-specific reversions + Map<String,Service> clusterServices = cluster.getServices(); for (UpgradeHistoryEntity history : revertUpgrade.getHistory()) { - // !!! build all service-specific - m_services.add(history.getServiceName()); - m_sourceRepositoryMap.put(history.getServiceName(), history.getTargetRepositoryVersion()); - m_targetRepositoryMap.put(history.getServiceName(), history.getSourceRepositoryVersion()); + String serviceName = history.getServiceName(); + String componentName = history.getComponentName(); + + // if the service is no longer installed, do nothing + if (!clusterServices.containsKey(serviceName)) { + LOG.warn("{}/{} will not be reverted since it is no longer installed in the cluster", + serviceName, componentName); + + continue; + } + + m_services.add(serviceName); + m_sourceRepositoryMap.put(serviceName, history.getTargetRepositoryVersion()); + m_targetRepositoryMap.put(serviceName, history.getSourceRepositoryVersion()); } // downgrades (which is what a revert really is) have the same associated http://git-wip-us.apache.org/repos/asf/ambari/blob/ef0973a1/ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeContextTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeContextTest.java b/ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeContextTest.java index 5cf9a4c..f5dcbf8 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeContextTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeContextTest.java @@ -23,6 +23,7 @@ import static junit.framework.Assert.assertTrue; import static org.easymock.EasyMock.eq; import static org.easymock.EasyMock.expect; +import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -111,6 +112,11 @@ public class UpgradeContextTest extends EasyMockSupport { private VersionDefinitionXml m_vdfXml; /** + * The upgrade history to return for the completed upgrade. + */ + private List<UpgradeHistoryEntity> m_upgradeHistory = new ArrayList<>(); + + /** * The cluster services. */ private Map<String, Service> m_services = new HashMap<>(); @@ -128,7 +134,7 @@ public class UpgradeContextTest extends EasyMockSupport { expect(upgradeHistoryEntity.getServiceName()).andReturn(HDFS_SERVICE_NAME).anyTimes(); expect(upgradeHistoryEntity.getSourceRepositoryVersion()).andReturn(m_sourceRepositoryVersion).anyTimes(); expect(upgradeHistoryEntity.getTargetRepositoryVersion()).andReturn(m_targetRepositoryVersion).anyTimes(); - List<UpgradeHistoryEntity> upgradeHistory = Lists.newArrayList(upgradeHistoryEntity); + m_upgradeHistory = Lists.newArrayList(upgradeHistoryEntity); expect(m_repositoryVersionDAO.findByPK(1L)).andReturn(m_sourceRepositoryVersion).anyTimes(); expect(m_repositoryVersionDAO.findByPK(99L)).andReturn(m_targetRepositoryVersion).anyTimes(); @@ -143,12 +149,13 @@ public class UpgradeContextTest extends EasyMockSupport { expect(m_completedRevertableUpgrade.getDirection()).andReturn(Direction.UPGRADE).anyTimes(); expect(m_completedRevertableUpgrade.getRepositoryVersion()).andReturn(m_targetRepositoryVersion).anyTimes(); expect(m_completedRevertableUpgrade.getOrchestration()).andReturn(RepositoryType.PATCH).anyTimes(); - expect(m_completedRevertableUpgrade.getHistory()).andReturn(upgradeHistory).anyTimes(); + expect(m_completedRevertableUpgrade.getHistory()).andReturn(m_upgradeHistory).anyTimes(); expect(m_completedRevertableUpgrade.getUpgradePackage()).andReturn(null).anyTimes(); RepositoryVersionEntity hdfsRepositoryVersion = createNiceMock(RepositoryVersionEntity.class); expect(m_hdfsService.getDesiredRepositoryVersion()).andReturn(hdfsRepositoryVersion).anyTimes(); + expect(m_zookeeperService.getDesiredRepositoryVersion()).andReturn(hdfsRepositoryVersion).anyTimes(); expect(m_cluster.getService(HDFS_SERVICE_NAME)).andReturn(m_hdfsService).anyTimes(); m_services.put(HDFS_SERVICE_NAME, m_hdfsService); @@ -331,6 +338,55 @@ public class UpgradeContextTest extends EasyMockSupport { } /** + * Tests that the {@link UpgradeContext} for a reversion has the correct + * services included in the reversion if one of the services in the original + * upgrade has since been deleted. + * + * @throws Exception + */ + @Test + public void testRevertWithDeletedService() throws Exception { + UpgradeHelper upgradeHelper = createNiceMock(UpgradeHelper.class); + ConfigHelper configHelper = createNiceMock(ConfigHelper.class); + UpgradePack upgradePack = createNiceMock(UpgradePack.class); + + // give the completed upgrade 2 services which can be reverted + UpgradeHistoryEntity upgradeHistoryEntity = createNiceMock(UpgradeHistoryEntity.class); + expect(upgradeHistoryEntity.getServiceName()).andReturn(ZOOKEEPER_SERVICE_NAME).anyTimes(); + expect(upgradeHistoryEntity.getSourceRepositoryVersion()).andReturn(m_sourceRepositoryVersion).anyTimes(); + expect(upgradeHistoryEntity.getTargetRepositoryVersion()).andReturn(m_targetRepositoryVersion).anyTimes(); + m_upgradeHistory.add(upgradeHistoryEntity); + + expect(upgradeHelper.suggestUpgradePack(EasyMock.anyString(), EasyMock.anyObject(StackId.class), + EasyMock.anyObject(StackId.class), EasyMock.anyObject(Direction.class), + EasyMock.anyObject(UpgradeType.class), EasyMock.anyString())).andReturn(upgradePack).once(); + + expect(m_upgradeDAO.findRevertable(1L)).andReturn(m_completedRevertableUpgrade).once(); + + // remove HDFS, add ZK + m_services.remove(HDFS_SERVICE_NAME); + expect(m_cluster.getService(ZOOKEEPER_SERVICE_NAME)).andReturn(m_zookeeperService).anyTimes(); + m_services.put(ZOOKEEPER_SERVICE_NAME, m_zookeeperService); + assertEquals(1, m_services.size()); + + Map<String, Object> requestMap = new HashMap<>(); + requestMap.put(UpgradeResourceProvider.UPGRADE_TYPE, UpgradeType.ROLLING.name()); + requestMap.put(UpgradeResourceProvider.UPGRADE_REVERT_UPGRADE_ID, "1"); + + replayAll(); + + UpgradeContext context = new UpgradeContext(m_cluster, requestMap, null, upgradeHelper, + m_upgradeDAO, m_repositoryVersionDAO, configHelper); + + assertEquals(Direction.DOWNGRADE, context.getDirection()); + assertEquals(RepositoryType.PATCH, context.getOrchestrationType()); + assertEquals(1, context.getSupportedServices().size()); + assertTrue(context.isPatchRevert()); + + verifyAll(); + } + + /** * Tests that if a different {@link UpgradeEntity} is returned instead of the one * specified by the *
