AMBARI-22590 - Messages for some services during PU package installation indicate circular dependency (jonathanhurley)
Project: http://git-wip-us.apache.org/repos/asf/ambari/repo Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/620543c6 Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/620543c6 Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/620543c6 Branch: refs/heads/branch-feature-AMBARI-20859 Commit: 620543c6c20307b35bd3ff433edf5b5dfdc33599 Parents: a7ac445 Author: Jonathan Hurley <[email protected]> Authored: Mon Dec 4 17:09:24 2017 -0500 Committer: Jonathan Hurley <[email protected]> Committed: Tue Dec 5 11:51:05 2017 -0500 ---------------------------------------------------------------------- .../RequiredServicesInRepositoryCheck.java | 18 ++---- .../ClusterStackVersionResourceProvider.java | 14 ++--- .../state/repository/VersionDefinitionXml.java | 59 ++++++++++++++++++-- .../RequiredServicesInRepositoryCheckTest.java | 6 +- .../state/repository/VersionDefinitionTest.java | 55 ++++++++++++++++++ 5 files changed, 120 insertions(+), 32 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ambari/blob/620543c6/ambari-server/src/main/java/org/apache/ambari/server/checks/RequiredServicesInRepositoryCheck.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/checks/RequiredServicesInRepositoryCheck.java b/ambari-server/src/main/java/org/apache/ambari/server/checks/RequiredServicesInRepositoryCheck.java index d911411..ceed53c 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/checks/RequiredServicesInRepositoryCheck.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/checks/RequiredServicesInRepositoryCheck.java @@ -18,7 +18,6 @@ package org.apache.ambari.server.checks; import java.util.LinkedHashSet; -import java.util.Map; import java.util.Set; import org.apache.ambari.server.AmbariException; @@ -66,23 +65,18 @@ public class RequiredServicesInRepositoryCheck extends AbstractCheckDescriptor { Cluster cluster = clustersProvider.get().getCluster(clusterName); VersionDefinitionXml xml = getVersionDefinitionXml(request); - Map<String, Set<String>> missingDependencies = xml.getMissingDependencies(cluster); + Set<String> missingDependencies = xml.getMissingDependencies(cluster); if (!missingDependencies.isEmpty()) { String failReasonTemplate = getFailReason(prerequisiteCheck, request); - StringBuilder message = new StringBuilder(); - for (String failedService : missingDependencies.keySet()) { - Set<String> servicesRequired = missingDependencies.get(failedService); + String message = String.format( + "The following services are also required to be included in this upgrade: %s", + StringUtils.join(missingDependencies, ", ")); - message.append(String.format( - "%s requires the following services which are not included: %s", - failedService, StringUtils.join(servicesRequired, ','))).append(System.lineSeparator()); - } - - prerequisiteCheck.setFailedOn(new LinkedHashSet<>(missingDependencies.keySet())); + prerequisiteCheck.setFailedOn(new LinkedHashSet<>(missingDependencies)); prerequisiteCheck.setStatus(PrereqCheckStatus.FAIL); - prerequisiteCheck.setFailReason(String.format(failReasonTemplate, message.toString())); + prerequisiteCheck.setFailReason(String.format(failReasonTemplate, message)); return; } http://git-wip-us.apache.org/repos/asf/ambari/blob/620543c6/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterStackVersionResourceProvider.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterStackVersionResourceProvider.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterStackVersionResourceProvider.java index fa13116..b590ee5 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterStackVersionResourceProvider.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterStackVersionResourceProvider.java @@ -449,18 +449,12 @@ public class ClusterStackVersionResourceProvider extends AbstractControllerResou // dependencies try { if (repoVersionEntity.getType().isPartial()) { - Map<String, Set<String>> missingDependencies = desiredVersionDefinition.getMissingDependencies(cluster); + Set<String> missingDependencies = desiredVersionDefinition.getMissingDependencies(cluster); if (!missingDependencies.isEmpty()) { - StringBuilder message = new StringBuilder( - "The following services are included in this repository, but the repository is missing their dependencies: ").append( - System.lineSeparator()); - - for (String failedService : missingDependencies.keySet()) { - message.append(String.format("%s requires the following services: %s", failedService, - StringUtils.join(missingDependencies.get(failedService), ','))).append( - System.lineSeparator()); - } + String message = String.format( + "The following services are also required to be included in this upgrade: %s", + StringUtils.join(missingDependencies, ", ")); throw new SystemException(message.toString()); } http://git-wip-us.apache.org/repos/asf/ambari/blob/620543c6/ambari-server/src/main/java/org/apache/ambari/server/state/repository/VersionDefinitionXml.java ---------------------------------------------------------------------- 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 a519d00..5f2e8fc 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 @@ -30,7 +30,6 @@ import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; -import java.util.TreeMap; import java.util.TreeSet; import javax.xml.XMLConstants; @@ -346,9 +345,9 @@ public class VersionDefinitionXml { * an empty map if there are none (never {@code null}). * @throws AmbariException */ - public Map<String, Set<String>> getMissingDependencies(Cluster cluster) + public Set<String> getMissingDependencies(Cluster cluster) throws AmbariException { - Map<String, Set<String>> missingDependencies = new TreeMap<>(); + Set<String> missingDependencies = Sets.newTreeSet(); String stackPackagesJson = cluster.getClusterProperty( ConfigHelper.CLUSTER_ENV_STACK_PACKAGES_PROPERTY, null); @@ -388,6 +387,9 @@ public class VersionDefinitionXml { return missingDependencies; } + // the installed services in the cluster + Map<String,Service> installedServices = cluster.getServices(); + ClusterVersionSummary clusterVersionSummary = getClusterSummary(cluster); Set<String> servicesInUpgrade = clusterVersionSummary.getAvailableServiceNames(); Set<String> servicesInRepository = getAvailableServiceNames(); @@ -399,17 +401,62 @@ public class VersionDefinitionXml { } for (String serviceRequired : servicesRequired) { - if (!servicesInRepository.contains(serviceRequired)) { - missingDependencies.put(serviceInUpgrade, Sets.newTreeSet(servicesRequired)); - break; + if (!servicesInRepository.contains(serviceRequired) && installedServices.containsKey(serviceRequired)) { + missingDependencies.add(serviceRequired); } } } + // now that we have built the list of missing dependencies based solely on + // the services participating in the upgrade, recursively see if any of + // those services have dependencies as well + missingDependencies = getRecursiveDependencies(missingDependencies, dependencies, + servicesInUpgrade, installedServices.keySet()); + return missingDependencies; } /** + * Gets all dependencies required to perform an upgrade, considering that the + * original set's depenencies may have dependencies of their own. + * + * @param missingDependencies + * the set of missing dependencies so far. + * @param dependencies + * the master list of dependency associations + * @param servicesInUpgrade + * the services which the VDF indicates are going to be in the + * upgrade * + * @param installedServices + * the services installed in the cluster + * @return a new set including any dependencies of those which were already + * found + */ + Set<String> getRecursiveDependencies(Set<String> missingDependencies, + Map<String, List<String>> dependencies, Set<String> servicesInUpgrade, + Set<String> installedServices) { + Set<String> results = Sets.newHashSet(); + results.addAll(missingDependencies); + + for (String missingDependency : missingDependencies) { + if (dependencies.containsKey(missingDependency)) { + List<String> subDependencies = dependencies.get(missingDependency); + for (String subDependency : subDependencies) { + if (!missingDependencies.contains(subDependency) + && installedServices.contains(subDependency) + && !servicesInUpgrade.contains(subDependency)) { + results.add(subDependency); + results.addAll(getRecursiveDependencies(results, dependencies, servicesInUpgrade, + installedServices)); + } + } + } + } + + return results; + } + + /** * Structures the manifest by service name. * <p/> * !!! WARNING. This is currently based on the assumption that there is one and only http://git-wip-us.apache.org/repos/asf/ambari/blob/620543c6/ambari-server/src/test/java/org/apache/ambari/server/checks/RequiredServicesInRepositoryCheckTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/checks/RequiredServicesInRepositoryCheckTest.java b/ambari-server/src/test/java/org/apache/ambari/server/checks/RequiredServicesInRepositoryCheckTest.java index 5984871..cc3798e 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/checks/RequiredServicesInRepositoryCheckTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/checks/RequiredServicesInRepositoryCheckTest.java @@ -19,8 +19,6 @@ package org.apache.ambari.server.checks; import static org.mockito.Mockito.mock; -import java.util.HashMap; -import java.util.Map; import java.util.Set; import org.apache.ambari.server.controller.PrereqCheckRequest; @@ -60,7 +58,7 @@ public class RequiredServicesInRepositoryCheckTest { /** * Used to return the missing dependencies for the test. */ - private Map<String, Set<String>> m_missingDependencies = new HashMap<>(); + private Set<String> m_missingDependencies = Sets.newTreeSet(); @Before public void setUp() throws Exception { @@ -106,7 +104,7 @@ public class RequiredServicesInRepositoryCheckTest { PrereqCheckRequest request = new PrereqCheckRequest(CLUSTER_NAME); request.setTargetRepositoryVersion(m_repositoryVersion); - m_missingDependencies.put("FOO", Sets.newHashSet("BAR")); + m_missingDependencies.add("BAR"); PrerequisiteCheck check = new PrerequisiteCheck(null, CLUSTER_NAME); m_requiredServicesCheck.perform(check, request); http://git-wip-us.apache.org/repos/asf/ambari/blob/620543c6/ambari-server/src/test/java/org/apache/ambari/server/state/repository/VersionDefinitionTest.java ---------------------------------------------------------------------- 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 9fe6146..0cd2e0f 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 @@ -32,6 +32,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Set; import org.apache.ambari.server.orm.entities.RepositoryVersionEntity; import org.apache.ambari.server.state.Cluster; @@ -45,9 +46,11 @@ import org.apache.ambari.server.state.stack.RepositoryXml; import org.apache.ambari.server.state.stack.RepositoryXml.Os; import org.apache.ambari.server.state.stack.RepositoryXml.Repo; import org.apache.commons.io.FileUtils; +import org.apache.hadoop.metrics2.sink.relocated.google.common.collect.Sets; import org.junit.Test; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Lists; /** * Tests for repository definitions. @@ -503,6 +506,58 @@ public class VersionDefinitionTest { assertEquals(1, summary.getAvailableServiceNames().size()); } + /** + * Tests that patch upgrade dependencies can be calculated recursively. + * + * @throws Exception + */ + @Test + public void testRecursiveDependencyDetection() throws Exception { + File f = new File("src/test/resources/version_definition_test_all_services.xml"); + VersionDefinitionXml xml = VersionDefinitionXml.load(f.toURI().toURL()); + + Map<String, List<String>> dependencies = new HashMap<>(); + dependencies.put("A", Lists.newArrayList("B", "X")); + dependencies.put("B", Lists.newArrayList("C", "D", "E")); + dependencies.put("E", Lists.newArrayList("A", "F")); + dependencies.put("F", Lists.newArrayList("B", "E")); + + // services not installed + dependencies.put("X", Lists.newArrayList("Y", "Z", "A")); + dependencies.put("Z", Lists.newArrayList("B")); + + Set<String> installedServices = Sets.newHashSet("A", "B", "C", "D", "E", "F", "G", "H"); + + Set<String> servicesInUpgrade = Sets.newHashSet("A"); + + Set<String> results = xml.getRecursiveDependencies(Sets.newHashSet("B"), dependencies, + servicesInUpgrade, installedServices); + + assertEquals(5, results.size()); + assertTrue(results.contains("B")); + assertTrue(results.contains("C")); + assertTrue(results.contains("D")); + assertTrue(results.contains("E")); + assertTrue(results.contains("F")); + + servicesInUpgrade = Sets.newHashSet("A", "B", "C", "E", "F"); + results = xml.getRecursiveDependencies(Sets.newHashSet("D"), dependencies, servicesInUpgrade, + installedServices); + + assertEquals(1, results.size()); + assertTrue(results.contains("D")); + + servicesInUpgrade = Sets.newHashSet("A", "F"); + results = xml.getRecursiveDependencies(Sets.newHashSet("B", "E"), dependencies, + servicesInUpgrade, + installedServices); + + assertEquals(4, results.size()); + assertTrue(results.contains("B")); + assertTrue(results.contains("C")); + assertTrue(results.contains("D")); + assertTrue(results.contains("E")); + } private static ServiceInfo makeService(final String name) { return new ServiceInfo() {
