AMBARI-2267. During registrations, ambari server does not properly handle empty version numbers from agent. (smohanty)

git-svn-id: https://svn.apache.org/repos/asf/incubator/ambari/trunk@1489681 13f79535-47bb-0310-9956-ffa450edef68

ambari-server/src/main/java/org/apache/ambari/server/agent/HeartBeatHandler.java

@@ -392,7 +392,7 @@ public class HeartBeatHandler {
 
     String agentVersion = register.getAgentVersion();
     String serverVersion = ambariMetaInfo.getServerVersion();
-    if (!VersionUtils.areVersionsCompatible(serverVersion, agentVersion)) {
+    if (!VersionUtils.areVersionsEqual(serverVersion, agentVersion, true)) {
       LOG.warn("Received registration request from host with non compatible"
           + " agent version"
           + ", hostname=" + hostname

ambari-server/src/main/java/org/apache/ambari/server/utils/VersionUtils.java

@@ -32,7 +32,8 @@ public class VersionUtils {
    *                           0 to compare the whole version strings
    * @return 0 if both are equal up to the length compared, -1 if first one is lower, +1 otherwise
    */
-  public static int compareVersions(String version1, String version2, int maxLengthToCompare) {
+  public static int compareVersions(String version1, String version2, int maxLengthToCompare)
+    throws IllegalArgumentException {
     if (version1 == null || version1.isEmpty()) {
       throw new IllegalArgumentException("version1 cannot be null or empty");
     }
@@ -69,33 +70,57 @@ public class VersionUtils {
    *
    * @param version1
    * @param version2
-   * @return 0 if both are equal, -1 if first one is lower, +1 otherwise
+   * @param allowEmptyVersions Allow one or both version values to be null or empty string
+   * @return 0 if both are equal up to the length compared, -1 if first one is lower, +1 otherwise
    */
-  public static int compareVersions(String version1, String version2) {
+  public static int compareVersions(String version1, String version2, boolean allowEmptyVersions) {
+    if (allowEmptyVersions) {
+      if (version1 == null && version2 == null) {
+        return 0;
+      } else {
+        if (version1 == null) {
+          return -1;
+        }
+        if (version2 == null) {
+          return 1;
+        }
+      }
+
+      if (version1.isEmpty() && version2.isEmpty()) {
+        return 0;
+      } else {
+        if (version1.isEmpty()) {
+          return -1;
+        }
+        if (version2.isEmpty()) {
+          return 1;
+        }
+      }
+    }
+
     return compareVersions(version1, version2, 0);
   }
 
   /**
-   * Compares two version for equality
+   * Compares two versions strings of the form N.N.N.N
    *
    * @param version1
    * @param version2
-   * @return true if versions are equal; false otherwise
+   * @return 0 if both are equal, -1 if first one is lower, +1 otherwise
    */
-  public static boolean areVersionsEqual(String version1, String version2) {
-    return 0 == compareVersions(version1, version2, 0);
+  public static int compareVersions(String version1, String version2) {
+    return compareVersions(version1, version2, 0);
   }
 
   /**
-   * Checks if the two versions are compatible.
-   * TODO A relaxed check can be implemented where a server version is compatible to
-   * TODO more than one versions of agent and store.
+   * Compares two version for equality, allows empty versions
    *
-   * @param serverVersion The version of the server
-   * @param versionToCheck The version of the agent or the store
-   * @return true if the versions are compatible
+   * @param version1
+   * @param version2
+   * @param allowEmptyVersions Allow one or both version values to be null or empty string
+   * @return true if versions are equal; false otherwise
    */
-  public static boolean areVersionsCompatible(String serverVersion, String versionToCheck) {
-    return areVersionsEqual(serverVersion, versionToCheck);
+  public static boolean areVersionsEqual(String version1, String version2, boolean allowEmptyVersions) {
+    return 0 == compareVersions(version1, version2, allowEmptyVersions);
   }
 }

ambari-server/src/test/java/org/apache/ambari/server/agent/TestHeartbeatHandler.java

@@ -437,7 +437,47 @@ public class TestHeartbeatHandler {
     assertEquals(hostObject.getLastHeartbeatTime(),
         hostObject.getLastRegistrationTime());
   }
-  
+
+  @Test
+  public void testRegistrationWithBadVersion() throws AmbariException,
+      InvalidStateTransitionException {
+
+    ActionManager am = getMockActionManager();
+    Clusters fsm = clusters;
+    HeartBeatHandler handler = new HeartBeatHandler(fsm, new ActionQueue(), am,
+        injector);
+    clusters.addHost(DummyHostname1);
+    Host hostObject = clusters.getHost(DummyHostname1);
+    hostObject.setIPv4("ipv4");
+    hostObject.setIPv6("ipv6");
+
+    Register reg = new Register();
+    HostInfo hi = new HostInfo();
+    hi.setHostName(DummyHostname1);
+    hi.setOS(DummyOsType);
+    reg.setHostname(DummyHostname1);
+    reg.setHardwareProfile(hi);
+    reg.setAgentVersion(""); // Invalid agent version
+    try {
+      handler.handleRegistration(reg);
+      fail ("Expected failure for non compatible agent version");
+    } catch (AmbariException e) {
+      log.debug("Error:" + e.getMessage());
+      Assert.assertTrue(e.getMessage().contains(
+          "Cannot register host with non compatible agent version"));
+    }
+
+    reg.setAgentVersion(null); // Invalid agent version
+    try {
+      handler.handleRegistration(reg);
+      fail ("Expected failure for non compatible agent version");
+    } catch (AmbariException e) {
+      log.debug("Error:" + e.getMessage());
+      Assert.assertTrue(e.getMessage().contains(
+          "Cannot register host with non compatible agent version"));
+    }
+  }
+
   @Test
   public void testRegistrationPublicHostname() throws AmbariException, InvalidStateTransitionException {
     ActionManager am = getMockActionManager();
@@ -524,7 +564,6 @@ public class TestHeartbeatHandler {
     }
   }
 
-
   @Test
   public void testRegisterNewNode()
       throws AmbariException, InvalidStateTransitionException {