YARN-8456. Fix a configuration handling bug when user leave FPGA discover executable path configuration default but set OpenCL SDK path environment variable. (Zhankun Tang via wangda)

Change-Id: Iff150ea98ba0c60d448474fd940eb121afce6965

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/resourceplugin/fpga/IntelFpgaOpenclPlugin.java

@@ -124,7 +124,9 @@ public boolean initPlugin(Configuration conf) {
       } else {
         binaryPath = new File(pluginDefaultPreferredPath + "/bin", pluginDefaultBinaryName);
         if (binaryPath.exists()) {
-          pathToExecutable = pluginDefaultPreferredPath;
+          pathToExecutable = binaryPath.getAbsolutePath();
+          LOG.info("Succeed in finding FPGA discoverer executable: " +
+              pathToExecutable);
         } else {
           pathToExecutable = pluginDefaultBinaryName;
           LOG.warn("Failed to find FPGA discoverer executable in " +

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/resourceplugin/fpga/TestFpgaDiscoverer.java

@@ -32,9 +32,8 @@
 import java.io.File;
 import java.io.FileOutputStream;
 import java.io.IOException;
-import java.util.LinkedList;
+import java.lang.reflect.Field;
-import java.util.List;
+import java.util.*;
-import java.util.Map;
 
 import static org.mockito.Matchers.anyInt;
 import static org.mockito.Matchers.anyString;
@@ -60,8 +59,41 @@ public void before() throws IOException {
     f.mkdirs();
   }
 
+  // A dirty hack to modify the env of the current JVM itself - Dirty, but
+  // should be okay for testing.
+  @SuppressWarnings({ "rawtypes", "unchecked" })
+  private static void setNewEnvironmentHack(Map<String, String> newenv)
+      throws Exception {
+    try {
+      Class<?> cl = Class.forName("java.lang.ProcessEnvironment");
+      Field field = cl.getDeclaredField("theEnvironment");
+      field.setAccessible(true);
+      Map<String, String> env = (Map<String, String>) field.get(null);
+      env.clear();
+      env.putAll(newenv);
+      Field ciField = cl.getDeclaredField("theCaseInsensitiveEnvironment");
+      ciField.setAccessible(true);
+      Map<String, String> cienv = (Map<String, String>) ciField.get(null);
+      cienv.clear();
+      cienv.putAll(newenv);
+    } catch (NoSuchFieldException e) {
+      Class[] classes = Collections.class.getDeclaredClasses();
+      Map<String, String> env = System.getenv();
+      for (Class cl : classes) {
+        if ("java.util.Collections$UnmodifiableMap".equals(cl.getName())) {
+          Field field = cl.getDeclaredField("m");
+          field.setAccessible(true);
+          Object obj = field.get(env);
+          Map<String, String> map = (Map<String, String>) obj;
+          map.clear();
+          map.putAll(newenv);
+        }
+      }
+    }
+  }
+
   @Test
-  public void testLinuxFpgaResourceDiscoverPluginConfig() throws YarnException, IOException {
+  public void testLinuxFpgaResourceDiscoverPluginConfig() throws Exception {
     Configuration conf = new Configuration(false);
     FpgaDiscoverer discoverer = FpgaDiscoverer.getInstance();
 
@@ -73,8 +105,9 @@ public void testLinuxFpgaResourceDiscoverPluginConfig() throws YarnException, IO
     openclPlugin.setShell(mockPuginShell());
 
     discoverer.initialize(conf);
-    // Case 1. No configuration set for binary
+    // Case 1. No configuration set for binary(no environment "ALTERAOCLSDKROOT" set)
-    Assert.assertEquals("No configuration should return just a single binary name",
+    Assert.assertEquals("No configuration(no environment ALTERAOCLSDKROOT set)" +
+            "should return just a single binary name",
         "aocl", openclPlugin.getPathToExecutable());
 
     // Case 2. With correct configuration and file exists
@@ -91,6 +124,25 @@ public void testLinuxFpgaResourceDiscoverPluginConfig() throws YarnException, IO
     Assert.assertEquals("Correct configuration but file doesn't exists should return just a single binary name",
         "aocl", openclPlugin.getPathToExecutable());
 
+    // Case 4. Set a empty value
+    conf.set(YarnConfiguration.NM_FPGA_PATH_TO_EXEC, "");
+    discoverer.initialize(conf);
+    Assert.assertEquals("configuration with empty string value, should use aocl",
+        "aocl", openclPlugin.getPathToExecutable());
+
+    // Case 5. No configuration set for binary, but set environment "ALTERAOCLSDKROOT"
+    // we load the default configuration to start with
+    conf = new Configuration(true);
+    fakeBinary = new File(getTestParentFolder() + "/bin/aocl");
+    fakeBinary.getParentFile().mkdirs();
+    touchFile(fakeBinary);
+    Map<String, String> newEnv = new HashMap<String, String>();
+    newEnv.put("ALTERAOCLSDKROOT", getTestParentFolder());
+    setNewEnvironmentHack(newEnv);
+    discoverer.initialize(conf);
+    Assert.assertEquals("No configuration but with environment ALTERAOCLSDKROOT set",
+        getTestParentFolder() + "/bin/aocl", openclPlugin.getPathToExecutable());
+
   }
 
   @Test