Repository: ambari Updated Branches: refs/heads/branch-2.6 348061f9c -> 761600d29
AMBARI-22292. PU: Could not install version when only build changes (ncole) Project: http://git-wip-us.apache.org/repos/asf/ambari/repo Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/761600d2 Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/761600d2 Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/761600d2 Branch: refs/heads/branch-2.6 Commit: 761600d2940b8cabd8a20bc2dc252c05ac7cffee Parents: 348061f Author: Nate Cole <[email protected]> Authored: Mon Oct 23 14:00:35 2017 -0400 Committer: Nate Cole <[email protected]> Committed: Fri Nov 3 09:28:55 2017 -0400 ---------------------------------------------------------------------- .../ClusterStackVersionResourceProvider.java | 39 +--------------- .../state/repository/VersionDefinitionXml.java | 15 ++++-- .../ambari/server/utils/VersionUtils.java | 48 +++++++++++++++++++- .../state/repository/VersionDefinitionTest.java | 34 ++++++++++++++ .../ambari/server/utils/TestVersionUtils.java | 7 ++- 5 files changed, 99 insertions(+), 44 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ambari/blob/761600d2/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 a206583..44ef912 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 @@ -89,7 +89,6 @@ import org.apache.ambari.server.utils.StageUtils; import org.apache.ambari.server.utils.VersionUtils; import org.apache.commons.collections.CollectionUtils; import org.apache.commons.lang.StringUtils; -import org.apache.commons.lang.math.NumberUtils; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Lists; @@ -260,7 +259,7 @@ public class ClusterStackVersionResourceProvider extends AbstractControllerResou Collections.sort(entities, new Comparator<RepositoryVersionEntity>() { @Override public int compare(RepositoryVersionEntity o1, RepositoryVersionEntity o2) { - return compareVersions(o1.getVersion(), o2.getVersion()); + return VersionUtils.compareVersionsWithBuild(o1.getVersion(), o2.getVersion(), 4); } }); @@ -515,7 +514,7 @@ public class ClusterStackVersionResourceProvider extends AbstractControllerResou continue; } - int compare = compareVersions(hostRepoVersion.getVersion(), desiredRepoVersion); + int compare = VersionUtils.compareVersionsWithBuild(hostRepoVersion.getVersion(), desiredRepoVersion, 4); // ignore earlier versions if (compare <= 0) { @@ -824,40 +823,6 @@ public class ClusterStackVersionResourceProvider extends AbstractControllerResou } /** - * Additional check over {@link VersionUtils#compareVersions(String, String)} that - * compares build numbers - */ - private static int compareVersions(String version1, String version2) { - version1 = (null == version1) ? "0" : version1; - version2 = (null == version2) ? "0" : version2; - - // check _exact_ equality - if (StringUtils.equals(version1, version2)) { - return 0; - } - - int compare = VersionUtils.compareVersions(version1, version2); - if (0 != compare) { - return compare; - } - - int v1 = 0; - int v2 = 0; - if (version1.indexOf('-') > -1) { - v1 = NumberUtils.toInt(version1.substring(version1.indexOf('-')), 0); - } - - if (version2.indexOf('-') > -1) { - v2 = NumberUtils.toInt(version2.substring(version2.indexOf('-')), 0); - } - - compare = v2 - v1; - - return Integer.compare(compare, 0); - } - - - /** * Ensures that the stack tools and stack features are set on * {@link ConfigHelper#CLUSTER_ENV} for the stack of the repository being * distributed. This step ensures that the new repository can be distributed http://git-wip-us.apache.org/repos/asf/ambari/blob/761600d2/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 7944de8..d513815 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 @@ -308,12 +308,21 @@ public class VersionDefinitionXml { // !!! currently only one version is supported (unique service names) ManifestService manifest = manifests.get(serviceName); - summary.setVersions(manifest.version, StringUtils.isEmpty(manifest.releaseVersion) ? - release.version : manifest.releaseVersion); + final String versionToCompare; + final String summaryReleaseVersion; + if (StringUtils.isEmpty(manifest.releaseVersion)) { + versionToCompare = release.getFullVersion(); + summaryReleaseVersion = release.version; + } else { + versionToCompare = manifest.releaseVersion; + summaryReleaseVersion = manifest.releaseVersion; + } + + summary.setVersions(manifest.version, summaryReleaseVersion); // !!! installed service already meets the release version, then nothing to upgrade // !!! TODO should this be using the release compatible-with field? - if (VersionUtils.compareVersions(summary.getReleaseVersion(), serviceVersion, 4) > 0) { + if (VersionUtils.compareVersionsWithBuild(versionToCompare, serviceVersion, 4) > 0) { summary.setUpgrade(true); } } http://git-wip-us.apache.org/repos/asf/ambari/blob/761600d2/ambari-server/src/main/java/org/apache/ambari/server/utils/VersionUtils.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/utils/VersionUtils.java b/ambari-server/src/main/java/org/apache/ambari/server/utils/VersionUtils.java index d3d8592..35c43e2 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/utils/VersionUtils.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/utils/VersionUtils.java @@ -22,6 +22,7 @@ import java.util.List; import org.apache.ambari.server.bootstrap.BootStrapImpl; import org.apache.commons.lang.StringUtils; +import org.apache.commons.lang.math.NumberUtils; /** * Provides various utility functions to be used for version handling. @@ -86,8 +87,8 @@ public class VersionUtils { int length = Math.max(version1Parts.length, version2Parts.length); length = maxLengthToCompare == 0 || maxLengthToCompare > length ? length : maxLengthToCompare; - List<Integer> stack1Parts = new ArrayList<Integer>(); - List<Integer> stack2Parts = new ArrayList<Integer>(); + List<Integer> stack1Parts = new ArrayList<>(); + List<Integer> stack2Parts = new ArrayList<>(); for (int i = 0; i < length; i++) { // Robust enough to handle strings in the version @@ -210,4 +211,47 @@ public class VersionUtils { return versionParts[0] + "." + versionParts[1] + "." + versionParts[2]; } + + /** + * Compares versions, using a build number using a dash separator, if one exists. + * This is is useful when comparing repository versions with one another that include + * build number + * @param version1 + * the first version + * @param version2 + * the second version + * @param places + * the number of decimal-separated places to compare + * @return + */ + public static int compareVersionsWithBuild(String version1, String version2, int places) { + version1 = (null == version1) ? "0" : version1; + version2 = (null == version2) ? "0" : version2; + + // check _exact_ equality + if (StringUtils.equals(version1, version2)) { + return 0; + } + + int compare = VersionUtils.compareVersions(version1, version2, places); + if (0 != compare) { + return compare; + } + + int v1 = 0; + int v2 = 0; + if (version1.indexOf('-') > -1) { + v1 = NumberUtils.toInt(version1.substring(version1.indexOf('-')), 0); + } + + if (version2.indexOf('-') > -1) { + v2 = NumberUtils.toInt(version2.substring(version2.indexOf('-')), 0); + } + + compare = v2 - v1; + + return Integer.compare(compare, 0); + + } + } http://git-wip-us.apache.org/repos/asf/ambari/blob/761600d2/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 8433518..370a14f 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 @@ -429,7 +429,41 @@ public class VersionDefinitionTest { xml = VersionDefinitionXml.load(f.toURI().toURL()); summary = xml.getClusterSummary(cluster); assertEquals(1, summary.getAvailableServiceNames().size()); + } + + @Test + public void testAvailableBuildVersion() throws Exception { + + Cluster cluster = createNiceMock(Cluster.class); + RepositoryVersionEntity repositoryVersion = createNiceMock(RepositoryVersionEntity.class); + expect(repositoryVersion.getVersion()).andReturn("2.3.4.1-1").atLeastOnce(); + Service serviceHdfs = createNiceMock(Service.class); + expect(serviceHdfs.getName()).andReturn("HDFS").atLeastOnce(); + expect(serviceHdfs.getDisplayName()).andReturn("HDFS").atLeastOnce(); + expect(serviceHdfs.getDesiredRepositoryVersion()).andReturn(repositoryVersion).atLeastOnce(); + + Service serviceHBase = createNiceMock(Service.class); + expect(serviceHBase.getName()).andReturn("HBASE").atLeastOnce(); + expect(serviceHBase.getDisplayName()).andReturn("HBase").atLeastOnce(); + expect(serviceHBase.getDesiredRepositoryVersion()).andReturn(repositoryVersion).atLeastOnce(); + + // !!! should never be accessed as it's not in any VDF + Service serviceAMS = createNiceMock(Service.class); + + expect(cluster.getServices()).andReturn(ImmutableMap.<String, Service>builder() + .put("HDFS", serviceHdfs) + .put("HBASE", serviceHBase) + .put("AMBARI_METRICS", serviceAMS).build()).atLeastOnce(); + + replay(cluster, repositoryVersion, serviceHdfs, serviceHBase); + + File f = new File("src/test/resources/version_definition_test_maint_partial.xml"); + VersionDefinitionXml xml = VersionDefinitionXml.load(f.toURI().toURL()); + xml.release.version = "2.3.4.1"; + xml.release.build = "2"; + ClusterVersionSummary summary = xml.getClusterSummary(cluster); + assertEquals(1, summary.getAvailableServiceNames().size()); } http://git-wip-us.apache.org/repos/asf/ambari/blob/761600d2/ambari-server/src/test/java/org/apache/ambari/server/utils/TestVersionUtils.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/utils/TestVersionUtils.java b/ambari-server/src/test/java/org/apache/ambari/server/utils/TestVersionUtils.java index 5141dc0..698ea94 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/utils/TestVersionUtils.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/utils/TestVersionUtils.java @@ -17,13 +17,13 @@ */ package org.apache.ambari.server.utils; -import junit.framework.Assert; import org.apache.ambari.server.bootstrap.BootStrapImpl; import org.junit.Rule; import org.junit.Test; - import org.junit.rules.ExpectedException; +import junit.framework.Assert; + public class TestVersionUtils { @Rule @@ -48,6 +48,9 @@ public class TestVersionUtils { Assert.assertEquals(0, VersionUtils.compareVersions("2.2", "2.2.VER")); Assert.assertEquals(0, VersionUtils.compareVersions("2.2.VAR", "2.2.VER")); Assert.assertEquals(0, VersionUtils.compareVersions("2.2.3", "2.2.3.VER1.V")); + + Assert.assertEquals(0, VersionUtils.compareVersions("2.2.0.1-200", "2.2.0.1-100")); + Assert.assertEquals(1, VersionUtils.compareVersionsWithBuild("2.2.0.1-200", "2.2.0.1-100", 4)); } @Test
