This is an automated email from the ASF dual-hosted git repository.

jonathanhurley pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/ambari.git


The following commit(s) were added to refs/heads/trunk by this push:
     new c20e418  [AMBARI-25086] Upgrading Using a Modified Default Version 
Repository Fails on Some Services (#2754)
c20e418 is described below

commit c20e418a2c0d278db6dea9caa5fd45cf6c80723d
Author: Jonathan Hurley <jonathanhur...@apache.org>
AuthorDate: Fri Jan 4 08:37:00 2019 -0500

    [AMBARI-25086] Upgrading Using a Modified Default Version Repository Fails 
on Some Services (#2754)
---
 .../apache/ambari/server/state/ServiceInfo.java    | 38 +++++++++++---
 .../state/repository/VersionDefinitionXml.java     | 25 +++++++--
 .../state/repository/VersionDefinitionTest.java    | 59 ++++++++++++++++++++--
 3 files changed, 105 insertions(+), 17 deletions(-)

diff --git 
a/ambari-server/src/main/java/org/apache/ambari/server/state/ServiceInfo.java 
b/ambari-server/src/main/java/org/apache/ambari/server/state/ServiceInfo.java
index fa17dea..117b51f 100644
--- 
a/ambari-server/src/main/java/org/apache/ambari/server/state/ServiceInfo.java
+++ 
b/ambari-server/src/main/java/org/apache/ambari/server/state/ServiceInfo.java
@@ -245,7 +245,7 @@ public class ServiceInfo implements Validable {
 
   @Override
   public void addErrors(Collection<String> errors) {
-    this.errorSet.addAll(errors);
+    errorSet.addAll(errors);
   }
 
   /**
@@ -363,11 +363,11 @@ public class ServiceInfo implements Validable {
     }
     // If set to null and has a parent, then the value would have already been 
resolved and set.
     // Otherwise, return the default value (true).
-    return this.supportDeleteViaUIInternal;
+    return supportDeleteViaUIInternal;
   }
 
   public void setSupportDeleteViaUI(boolean supportDeleteViaUI) {
-    this.supportDeleteViaUIInternal = supportDeleteViaUI;
+    supportDeleteViaUIInternal = supportDeleteViaUI;
   }
 
   public String getName() {
@@ -395,7 +395,7 @@ public class ServiceInfo implements Validable {
   }
 
   public void setServiceAdvisorType(ServiceAdvisorType type) {
-    this.serviceAdvisorType = type;
+    serviceAdvisorType = type;
   }
 
   public ServiceAdvisorType getServiceAdvisorType() {
@@ -711,6 +711,7 @@ public class ServiceInfo implements Validable {
   /**
    * @deprecated Use {@link #getSingleSignOnEnabledTest()} instead
    */
+  @Deprecated
   public String getSingleSignOnEnabledConfiguration() {
     return singleSignOnInfo != null ? 
singleSignOnInfo.getEnabledConfiguration() : null;
   }
@@ -725,7 +726,7 @@ public class ServiceInfo implements Validable {
   public boolean isKerberosRequiredForSingleSignOnIntegration() {
     return singleSignOnInfo != null && singleSignOnInfo.isKerberosRequired();
   }
-  
+
   /**
    * Gets a new value for LDAP integration support
    */
@@ -735,7 +736,7 @@ public class ServiceInfo implements Validable {
 
   /**
    * Sets a new value for LDAP integration support
-   * 
+   *
    * @param ldapInfo
    *          a {@link ServiceLdapInfo}
    */
@@ -812,7 +813,7 @@ public class ServiceInfo implements Validable {
    * @param typeAttributes attributes associated with the type
    */
   public synchronized void setTypeAttributes(String type, Map<String, 
Map<String, String>> typeAttributes) {
-    if (this.configTypes == null) {
+    if (configTypes == null) {
       configTypes = new HashMap<>();
     }
     configTypes.put(type, typeAttributes);
@@ -1373,10 +1374,31 @@ public class ServiceInfo implements Validable {
 
     if (ldapInfo != null && ldapInfo.isSupported() && 
StringUtils.isBlank(ldapInfo.getLdapEnabledTest())) {
       setValid(false);
-      addError("LDAP support is indicated for service " + getName() + " but no 
test configuration has been set by ldapEnabledTest."); 
+      addError("LDAP support is indicated for service " + getName() + " but no 
test configuration has been set by ldapEnabledTest.");
     }
   }
 
+  /**
+   * Gets whether this service advertises a version based on whether at least
+   * one of its components advertises a version.
+   *
+   * @return {@code true} if at least 1 component of this service advertises a
+   *         version, {@code false} otherwise.
+   */
+  public boolean isVersionAdvertised() {
+    if (null == components) {
+      return false;
+    }
+
+    for (ComponentInfo componentInfo : components) {
+      if (componentInfo.isVersionAdvertised()) {
+        return true;
+      }
+    }
+
+    return false;
+  }
+
   public enum Selection {
     DEFAULT,
     TECH_PREVIEW,
diff --git 
a/ambari-server/src/main/java/org/apache/ambari/server/state/repository/VersionDefinitionXml.java
 
b/ambari-server/src/main/java/org/apache/ambari/server/state/repository/VersionDefinitionXml.java
index abca273..13419a9 100644
--- 
a/ambari-server/src/main/java/org/apache/ambari/server/state/repository/VersionDefinitionXml.java
+++ 
b/ambari-server/src/main/java/org/apache/ambari/server/state/repository/VersionDefinitionXml.java
@@ -200,9 +200,14 @@ public class VersionDefinitionXml {
   }
 
   /**
-   * Gets the list of stack services, applying information from the version 
definition.
-   * @param stack the stack for which to get the information
-   * @return the list of {@code ManifestServiceInfo} instances for each 
service in the stack
+   * Gets the list of stack services, applying information from the version
+   * definition. This will include both services which advertise a version and
+   * those which do not.
+   * 
+   * @param stack
+   *          the stack for which to get the information
+   * @return the list of {@code ManifestServiceInfo} instances for each service
+   *         in the stack
    */
   public synchronized List<ManifestServiceInfo> getStackServices(StackInfo 
stack) {
 
@@ -610,7 +615,12 @@ public class VersionDefinitionXml {
   }
 
   /**
-   * Builds a Version Definition that is the default for the stack
+   * Builds a Version Definition that is the default for the stack.
+   * <p>
+   * This will use all of the services defined on the stack, excluding those
+   * which do not advertise a version. If a service doesn't advertise a 
version,
+   * we cannot include it in a generated VDF.
+   *
    * @return the version definition
    */
   public static VersionDefinitionXml build(StackInfo stackInfo) {
@@ -630,6 +640,13 @@ public class VersionDefinitionXml {
     xml.release.display = stackId.toString();
 
     for (ServiceInfo si : stackInfo.getServices()) {
+      // do NOT build a manifest entry for services on the stack which cannot 
be
+      // a part of an upgrade - this is to prevent services like KERBEROS from
+      // preventing an upgrade if it's in Maintenance Mode
+      if (!si.isVersionAdvertised()) {
+        continue;
+      }
+
       ManifestService ms = new ManifestService();
       ms.serviceName = si.getName();
       ms.version = StringUtils.trimToEmpty(si.getVersion());
diff --git 
a/ambari-server/src/test/java/org/apache/ambari/server/state/repository/VersionDefinitionTest.java
 
b/ambari-server/src/test/java/org/apache/ambari/server/state/repository/VersionDefinitionTest.java
index 885cb82..133c685 100644
--- 
a/ambari-server/src/test/java/org/apache/ambari/server/state/repository/VersionDefinitionTest.java
+++ 
b/ambari-server/src/test/java/org/apache/ambari/server/state/repository/VersionDefinitionTest.java
@@ -17,9 +17,7 @@
  */
 package org.apache.ambari.server.state.repository;
 
-import static org.easymock.EasyMock.createNiceMock;
 import static org.easymock.EasyMock.expect;
-import static org.easymock.EasyMock.replay;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
@@ -49,6 +47,7 @@ import 
org.apache.ambari.server.state.stack.RepositoryXml.Repo;
 import org.apache.ambari.spi.RepositoryType;
 import org.apache.commons.io.FileUtils;
 import org.easymock.EasyMock;
+import org.easymock.EasyMockSupport;
 import org.junit.Test;
 
 import com.google.common.collect.ImmutableMap;
@@ -58,7 +57,7 @@ import com.google.common.collect.Sets;
 /**
  * Tests for repository definitions.
  */
-public class VersionDefinitionTest {
+public class VersionDefinitionTest extends EasyMockSupport {
 
   private static File file = new 
File("src/test/resources/version_definition_test.xml");
 
@@ -455,7 +454,7 @@ public class VersionDefinitionTest {
         .put("AMBARI_METRICS", serviceAMS).build()).atLeastOnce();
 
 
-    replay(cluster, repositoryVersion, serviceHdfs, serviceHBase, stackInfo, 
ami);
+    replayAll();
 
     File f = new 
File("src/test/resources/version_definition_test_all_services.xml");
     VersionDefinitionXml xml = VersionDefinitionXml.load(f.toURI().toURL());
@@ -511,7 +510,7 @@ public class VersionDefinitionTest {
         .put("HBASE", serviceHBase)
         .put("AMBARI_METRICS", serviceAMS).build()).atLeastOnce();
 
-    replay(cluster, repositoryVersion, serviceHdfs, serviceHBase, stackInfo, 
ami);
+    replayAll();
 
     File f = new 
File("src/test/resources/version_definition_test_maint_partial.xml");
     VersionDefinitionXml xml = VersionDefinitionXml.load(f.toURI().toURL());
@@ -574,6 +573,56 @@ public class VersionDefinitionTest {
     assertTrue(results.contains("E"));
   }
 
+  /**
+   * Tests that a VDF can be built from the cluster services correctly, taking
+   * into account things like whether a service has components which advertise 
a
+   * version. The first part of this will test that services which do not
+   * advertise a version are not returned when building a VDF from cluster
+   * services. The 2nd part will test to make sure that when combining a VDF
+   * with the stack, all services are returned regardless of whether they
+   * advertise a version.
+   *
+   * @throws Exception
+   */
+  @Test
+  public void testBuild() throws Exception {
+    ServiceInfo serviceWithVersionAdvertised = 
createNiceMock(ServiceInfo.class);
+    ServiceInfo serviceWithoutVersionAdvertised = 
createNiceMock(ServiceInfo.class);
+
+    List<ServiceInfo> stackServices = 
Lists.newArrayList(serviceWithVersionAdvertised,
+        serviceWithoutVersionAdvertised);
+
+    
expect(serviceWithVersionAdvertised.isVersionAdvertised()).andReturn(true).atLeastOnce();
+    
expect(serviceWithVersionAdvertised.getName()).andReturn("BAR").atLeastOnce();
+    
expect(serviceWithVersionAdvertised.getVersion()).andReturn("1.5.0").atLeastOnce();
+
+    
expect(serviceWithoutVersionAdvertised.isVersionAdvertised()).andReturn(false).atLeastOnce();
+    
expect(serviceWithoutVersionAdvertised.getName()).andReturn("BAZ").atLeastOnce();
+    
expect(serviceWithoutVersionAdvertised.getVersion()).andReturn("2.0.0").atLeastOnce();
+
+    File f = new 
File("src/test/resources/version_definition_test_all_services.xml");
+    VersionDefinitionXml xml1 = VersionDefinitionXml.load(f.toURI().toURL());
+
+    RepositoryXml repositoryXml = createNiceMock(RepositoryXml.class);
+    
expect(repositoryXml.getOses()).andReturn(xml1.repositoryInfo.getOses()).atLeastOnce();
+
+    StackInfo stackInfo = createNiceMock(StackInfo.class);
+    expect(stackInfo.getName()).andReturn("FOO").anyTimes();
+    expect(stackInfo.getVersion()).andReturn("1.0.0").anyTimes();
+    expect(stackInfo.getServices()).andReturn(stackServices).atLeastOnce();
+    
expect(stackInfo.getRepositoryXml()).andReturn(repositoryXml).atLeastOnce();
+
+    replayAll();
+
+    VersionDefinitionXml vdf = VersionDefinitionXml.build(stackInfo);
+    assertEquals(1, vdf.manifestServices.size());
+
+    List<ManifestServiceInfo> manifestServices = 
vdf.getStackServices(stackInfo);
+    assertEquals(2, manifestServices.size());
+
+    verifyAll();
+  }
+
   private static ServiceInfo makeService(final String name) {
     return new ServiceInfo() {
       @Override

Reply via email to