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/abd64590
Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/abd64590
Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/abd64590

Branch: refs/heads/branch-2.6
Commit: abd645907367f7044399f1fce25613b7cee7cd6a
Parents: ecd1b20
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 09:50:23 2017 -0400

----------------------------------------------------------------------
 .../VersionDefinitionResourceProvider.java      |   8 +-
 .../ambari/server/state/RepositoryType.java     |  22 ++-
 .../ambari/server/state/UpgradeContext.java     |  15 +-
 .../ambari/server/state/UpgradeContextTest.java | 173 +++++++++++++++++++
 4 files changed, 209 insertions(+), 9 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/ambari/blob/abd64590/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 629f3cd..77c09d4 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
@@ -71,10 +71,10 @@ import org.codehaus.jackson.node.ArrayNode;
 import org.codehaus.jackson.node.JsonNodeFactory;
 import org.codehaus.jackson.node.ObjectNode;
 
+import com.google.common.collect.ListMultimap;
 import com.google.common.collect.Sets;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
-import com.google.common.collect.ListMultimap;
 
 /**
  * The {@link VersionDefinitionResourceProvider} class deals with managing 
Version Definition
@@ -347,7 +347,7 @@ public class VersionDefinitionResourceProvider extends 
AbstractAuthorizedResourc
       throws SystemException, UnsupportedPropertyException,
       NoSuchResourceException, NoSuchParentResourceException {
 
-    Set<Resource> results = new HashSet<Resource>();
+    Set<Resource> results = new HashSet<>();
     Set<String> requestPropertyIds = getRequestPropertyIds(request, predicate);
 
     Set<Map<String, Object>> propertyMaps = getPropertyMaps(predicate);
@@ -424,7 +424,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/abd64590/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 215cab8..2e95c59 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/abd64590/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 8bf0400..c5b8269 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
@@ -290,8 +290,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) {
@@ -324,7 +325,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
@@ -398,6 +399,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()) {
@@ -505,8 +507,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;
   }
 
   /**
@@ -781,9 +783,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;

http://git-wip-us.apache.org/repos/asf/ambari/blob/abd64590/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();
+  }
+
+}

Reply via email to