AMBARI-21656 - Patch/Service Downgrades Are Not Correctly Scoped (jonathanhurley)
Project: http://git-wip-us.apache.org/repos/asf/ambari/repo Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/e89e9505 Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/e89e9505 Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/e89e9505 Branch: refs/heads/branch-feature-logsearch-ui Commit: e89e9505b1d509eda027cfe8bd7784976c6c0434 Parents: 4cd9657 Author: Jonathan Hurley <jhur...@hortonworks.com> Authored: Thu Aug 3 16:15:29 2017 -0400 Committer: Jonathan Hurley <jhur...@hortonworks.com> Committed: Fri Aug 4 10:01:04 2017 -0400 ---------------------------------------------------------------------- .../VersionDefinitionResourceProvider.java | 4 +- .../ambari/server/state/RepositoryType.java | 22 ++- .../ambari/server/state/UpgradeContext.java | 17 +- .../ambari/server/state/UpgradeContextTest.java | 173 +++++++++++++++++++ 4 files changed, 208 insertions(+), 8 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ambari/blob/e89e9505/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/VersionDefinitionResourceProvider.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/VersionDefinitionResourceProvider.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/VersionDefinitionResourceProvider.java index e41e3da..5d65f35 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/VersionDefinitionResourceProvider.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/VersionDefinitionResourceProvider.java @@ -426,7 +426,9 @@ public class VersionDefinitionResourceProvider extends AbstractAuthorizedResourc */ private void checkForParent(XmlHolder holder) throws AmbariException { RepositoryVersionEntity entity = holder.entity; - if (entity.getType() != RepositoryType.PATCH) { + + // only STANDARD types don't have a parent + if (entity.getType() == RepositoryType.STANDARD) { return; } http://git-wip-us.apache.org/repos/asf/ambari/blob/e89e9505/ambari-server/src/main/java/org/apache/ambari/server/state/RepositoryType.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/state/RepositoryType.java b/ambari-server/src/main/java/org/apache/ambari/server/state/RepositoryType.java index da99908..3f7d447 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/state/RepositoryType.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/state/RepositoryType.java @@ -43,6 +43,26 @@ public enum RepositoryType { /** * Repository is used to update services. */ - SERVICE + SERVICE; + + /** + * Gets whether applications of this repository are revertable after they have + * been finalized. + * + * @return {@code true} if the repository can be revert, {@code false} + * otherwise. + */ + public boolean isRevertable() { + switch (this) { + case MAINT: + case PATCH: + return true; + case SERVICE: + case STANDARD: + return false; + default: + return false; + } + } } http://git-wip-us.apache.org/repos/asf/ambari/blob/e89e9505/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 59a1b02..0e409a4 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 @@ -296,8 +296,9 @@ public class UpgradeContext { throw new AmbariException(String.format("Could not find Upgrade with id %s to revert.", revertUpgradeId)); } - if (revertUpgrade.getOrchestration() != RepositoryType.PATCH) { - throw new AmbariException("Can only revert upgrades that have been done as a patch."); + if (!revertUpgrade.getOrchestration().isRevertable()) { + throw new AmbariException(String.format("The %s repository type is not revertable", + revertUpgrade.getOrchestration())); } if (revertUpgrade.getDirection() != Direction.UPGRADE) { @@ -330,7 +331,7 @@ public class UpgradeContext { // !!! direction can ONLY be an downgrade on revert m_direction = Direction.DOWNGRADE; - m_orchestration = RepositoryType.PATCH; + m_orchestration = revertUpgrade.getOrchestration(); } else { // determine direction @@ -404,6 +405,7 @@ public class UpgradeContext { cluster.getClusterId(), Direction.UPGRADE); m_repositoryVersion = upgrade.getRepositoryVersion(); + m_orchestration = upgrade.getOrchestration(); // populate the repository maps for all services in the upgrade for (UpgradeHistoryEntity history : upgrade.getHistory()) { @@ -511,8 +513,8 @@ public class UpgradeContext { m_resolver = new MasterHostResolver(m_cluster, configHelper, this); m_orchestration = upgradeEntity.getOrchestration(); - m_isRevert = upgradeEntity.getOrchestration() == RepositoryType.PATCH && - upgradeEntity.getDirection() == Direction.DOWNGRADE; + m_isRevert = upgradeEntity.getOrchestration().isRevertable() + && upgradeEntity.getDirection() == Direction.DOWNGRADE; } /** @@ -787,9 +789,12 @@ public class UpgradeContext { switch (m_orchestration) { case PATCH: case SERVICE: + case MAINT: return scope == UpgradeScope.PARTIAL; case STANDARD: return scope == UpgradeScope.COMPLETE; + default: + break; } return false; @@ -1207,4 +1212,4 @@ public class UpgradeContext { return hostOrderItems; } } -} +} \ No newline at end of file http://git-wip-us.apache.org/repos/asf/ambari/blob/e89e9505/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 new file mode 100644 index 0000000..e2bb27e --- /dev/null +++ b/ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeContextTest.java @@ -0,0 +1,173 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.ambari.server.state; + +import static junit.framework.Assert.assertEquals; +import static junit.framework.Assert.assertFalse; +import static junit.framework.Assert.assertTrue; +import static org.easymock.EasyMock.eq; +import static org.easymock.EasyMock.expect; + +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import org.apache.ambari.server.controller.internal.UpgradeResourceProvider; +import org.apache.ambari.server.orm.dao.RepositoryVersionDAO; +import org.apache.ambari.server.orm.dao.UpgradeDAO; +import org.apache.ambari.server.orm.entities.RepositoryVersionEntity; +import org.apache.ambari.server.orm.entities.UpgradeEntity; +import org.apache.ambari.server.orm.entities.UpgradeHistoryEntity; +import org.apache.ambari.server.state.stack.UpgradePack; +import org.apache.ambari.server.state.stack.upgrade.Direction; +import org.apache.ambari.server.state.stack.upgrade.UpgradeType; +import org.apache.hadoop.metrics2.sink.relocated.google.common.collect.Lists; +import org.easymock.EasyMock; +import org.easymock.EasyMockSupport; +import org.easymock.Mock; +import org.junit.Before; +import org.junit.Test; + +/** + * Tests {@link UpgradeContext}. + */ +public class UpgradeContextTest extends EasyMockSupport { + + @Mock + private UpgradeEntity m_completedRevertableUpgrade; + + @Mock + private RepositoryVersionEntity m_completedUpgradeTargetRepositoryVersion; + + @Mock + private RepositoryVersionEntity m_completedUpgradeSourceRepositoryVersion; + + + @Mock + private UpgradeDAO m_upgradeDAO; + + @Before + public void setup() { + injectMocks(this); + + expect(m_completedUpgradeSourceRepositoryVersion.getId()).andReturn(1L).anyTimes(); + expect(m_completedUpgradeSourceRepositoryVersion.getStackId()).andReturn(new StackId("HDP", "2.6")).anyTimes(); + expect(m_completedUpgradeTargetRepositoryVersion.getId()).andReturn(1L).anyTimes(); + expect(m_completedUpgradeTargetRepositoryVersion.getStackId()).andReturn(new StackId("HDP", "2.6")).anyTimes(); + + UpgradeHistoryEntity upgradeHistoryEntity = createNiceMock(UpgradeHistoryEntity.class); + expect(upgradeHistoryEntity.getServiceName()).andReturn("HDFS").atLeastOnce(); + expect(upgradeHistoryEntity.getFromReposistoryVersion()).andReturn(m_completedUpgradeSourceRepositoryVersion).anyTimes(); + expect(upgradeHistoryEntity.getTargetRepositoryVersion()).andReturn(m_completedUpgradeTargetRepositoryVersion).anyTimes(); + List<UpgradeHistoryEntity> upgradeHistory = Lists.newArrayList(upgradeHistoryEntity); + + expect(m_upgradeDAO.findUpgrade(1L)).andReturn(m_completedRevertableUpgrade).anyTimes(); + + expect( + m_upgradeDAO.findLastUpgradeForCluster(EasyMock.anyLong(), + eq(Direction.UPGRADE))).andReturn(m_completedRevertableUpgrade).anyTimes(); + + expect(m_completedRevertableUpgrade.getDirection()).andReturn(Direction.UPGRADE).anyTimes(); + expect(m_completedRevertableUpgrade.getRepositoryVersion()).andReturn(m_completedUpgradeTargetRepositoryVersion).anyTimes(); + expect(m_completedRevertableUpgrade.getOrchestration()).andReturn(RepositoryType.PATCH).anyTimes(); + expect(m_completedRevertableUpgrade.getHistory()).andReturn(upgradeHistory).anyTimes(); + expect(m_completedRevertableUpgrade.getUpgradePackage()).andReturn(null).anyTimes(); + } + + /** + * Tests that the {@link UpgradeContext} for a reversion has the correct + * parameters set. + * + * @throws Exception + */ + @Test + public void testRevert() throws Exception { + Cluster cluster = createNiceMock(Cluster.class); + UpgradeHelper upgradeHelper = createNiceMock(UpgradeHelper.class); + ConfigHelper configHelper = createNiceMock(ConfigHelper.class); + RepositoryVersionDAO repositoryVersionDAO = createNiceMock(RepositoryVersionDAO.class); + RepositoryVersionEntity hdfsRepositoryVersion = createNiceMock(RepositoryVersionEntity.class); + + Service service = createNiceMock(Service.class); + UpgradePack upgradePack = createNiceMock(UpgradePack.class); + + 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(service.getDesiredRepositoryVersion()).andReturn(hdfsRepositoryVersion).once(); + expect(cluster.getService("HDFS")).andReturn(service).atLeastOnce(); + + 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(cluster, requestMap, null, upgradeHelper, + m_upgradeDAO, repositoryVersionDAO, configHelper); + + assertEquals(Direction.DOWNGRADE, context.getDirection()); + assertEquals(RepositoryType.PATCH, context.getOrchestrationType()); + assertEquals(1, context.getSupportedServices().size()); + assertTrue(context.isPatchRevert()); + + verifyAll(); + } + + /** + * Tests that the {@link UpgradeContext} for a patch downgrade has the + * correcting scope/orchestration set. + * + * @throws Exception + */ + @Test + public void testDowngradeScope() throws Exception { + Cluster cluster = createNiceMock(Cluster.class); + UpgradeHelper upgradeHelper = createNiceMock(UpgradeHelper.class); + ConfigHelper configHelper = createNiceMock(ConfigHelper.class); + RepositoryVersionDAO repositoryVersionDAO = createNiceMock(RepositoryVersionDAO.class); + RepositoryVersionEntity hdfsRepositoryVersion = createNiceMock(RepositoryVersionEntity.class); + Service service = createNiceMock(Service.class); + UpgradePack upgradePack = createNiceMock(UpgradePack.class); + + 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(service.getDesiredRepositoryVersion()).andReturn(hdfsRepositoryVersion).once(); + expect(cluster.getService("HDFS")).andReturn(service).atLeastOnce(); + + Map<String, Object> requestMap = new HashMap<>(); + requestMap.put(UpgradeResourceProvider.UPGRADE_TYPE, UpgradeType.NON_ROLLING.name()); + requestMap.put(UpgradeResourceProvider.UPGRADE_DIRECTION, Direction.DOWNGRADE.name()); + + replayAll(); + + UpgradeContext context = new UpgradeContext(cluster, requestMap, null, upgradeHelper, + m_upgradeDAO, repositoryVersionDAO, configHelper); + + assertEquals(Direction.DOWNGRADE, context.getDirection()); + assertEquals(RepositoryType.PATCH, context.getOrchestrationType()); + assertEquals(1, context.getSupportedServices().size()); + assertFalse(context.isPatchRevert()); + + verifyAll(); + } + +}
