From f878ddb067c6547f07fcde0c198a37c132a0b214 Mon Sep 17 00:00:00 2001
From: Wangda Tan <wangda@apache.org>
Date: Sat, 17 Feb 2018 12:28:08 +0800
Subject: [PATCH] YARN-7328. ResourceUtils allows
 yarn.nodemanager.resource-types.memory-mb and .vcores to override
 yarn.nodemanager.resource.memory-mb and .cpu-vcores. (lovekesh bansal via
 wangda)

Change-Id: Ibb1faf0beefec079dae2208986976b2f64650672
(cherry picked from commit ca1043ab9030339d7cdd3275c3f8f4713b8bff59)
---
 .../yarn/util/resource/ResourceUtils.java     | 20 +++++++------
 .../yarn/util/resource/TestResourceUtils.java | 28 +++++++++++++++++-
 .../resource-types/node-resources-2.xml       | 11 -------
 .../resource-types/node-resources-error-1.xml | 29 +++++++++++++++++++
 .../resource-types/resource-types-error-2.xml |  4 +--
 5 files changed, 69 insertions(+), 23 deletions(-)
 create mode 100644 hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/resources/resource-types/node-resources-error-1.xml

diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/util/resource/ResourceUtils.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/util/resource/ResourceUtils.java
index 5a660cea8a7..f211f49ff05 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/util/resource/ResourceUtils.java
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/util/resource/ResourceUtils.java
@@ -84,20 +84,22 @@ public class ResourceUtils {
       Map<String, ResourceInformation> resourceInformationMap)
       throws YarnRuntimeException {
     /*
-     * Supporting 'memory' also as invalid resource name, in addition to
+     * Supporting 'memory', 'memory-mb', 'vcores' also as invalid resource names, in addition to
      * 'MEMORY' for historical reasons
      */
-    String key = "memory";
-    if (resourceInformationMap.containsKey(key)) {
-      LOG.warn(
-          "Attempt to define resource '" + key + "', but it is not allowed.");
-      throw new YarnRuntimeException(
-          "Attempt to re-define mandatory resource '" + key + "'.");
+    String keys[] = { "memory", ResourceInformation.MEMORY_URI,
+        ResourceInformation.VCORES_URI };
+    for(String key : keys) {
+      if (resourceInformationMap.containsKey(key)) {
+        LOG.warn("Attempt to define resource '" + key + "', but it is not allowed.");
+        throw new YarnRuntimeException(
+            "Attempt to re-define mandatory resource '" + key + "'.");
+      }
     }
 
     for (Map.Entry<String, ResourceInformation> mandatoryResourceEntry :
         ResourceInformation.MANDATORY_RESOURCES.entrySet()) {
-      key = mandatoryResourceEntry.getKey();
+      String key = mandatoryResourceEntry.getKey();
       ResourceInformation mandatoryRI = mandatoryResourceEntry.getValue();
 
       ResourceInformation newDefinedRI = resourceInformationMap.get(key);
@@ -485,8 +487,8 @@ public class ResourceUtils {
         if (!initializedNodeResources) {
           Map<String, ResourceInformation> nodeResources = initializeNodeResourceInformation(
               conf);
-          addMandatoryResources(nodeResources);
           checkMandatoryResources(nodeResources);
+          addMandatoryResources(nodeResources);
           setAllocationForMandatoryResources(nodeResources, conf);
           readOnlyNodeResources = Collections.unmodifiableMap(nodeResources);
           initializedNodeResources = true;
diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/util/resource/TestResourceUtils.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/util/resource/TestResourceUtils.java
index 1111ca6d843..2671de85057 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/util/resource/TestResourceUtils.java
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/util/resource/TestResourceUtils.java
@@ -287,7 +287,7 @@ public class TestResourceUtils {
     Map<String, Resource> testRun = new HashMap<>();
     setupResourceTypes(conf, "resource-types-4.xml");
     // testRun.put("node-resources-1.xml", Resource.newInstance(1024, 1));
-    Resource test3Resources = Resource.newInstance(1024, 1);
+    Resource test3Resources = Resource.newInstance(0, 0);
     test3Resources.setResourceInformation("resource1",
         ResourceInformation.newInstance("resource1", "Gi", 5L));
     test3Resources.setResourceInformation("resource2",
@@ -315,6 +315,32 @@ public class TestResourceUtils {
     }
   }
 
+  @Test
+  public void testGetNodeResourcesConfigErrors() throws Exception {
+    Configuration conf = new YarnConfiguration();
+    Map<String, Resource> testRun = new HashMap<>();
+    setupResourceTypes(conf, "resource-types-4.xml");
+    String invalidNodeResFiles[] = { "node-resources-error-1.xml"};
+
+    for (String resourceFile : invalidNodeResFiles) {
+      ResourceUtils.resetNodeResources();
+      File dest = null;
+      try {
+        File source = new File(conf.getClassLoader().getResource(resourceFile).getFile());
+        dest = new File(source.getParent(), "node-resources.xml");
+        FileUtils.copyFile(source, dest);
+        Map<String, ResourceInformation> actual = ResourceUtils.getNodeResourceInformation(conf);
+        Assert.fail("Expected error with file " + resourceFile);
+      } catch (NullPointerException ne) {
+        throw ne;
+      } catch (Exception e) {
+        if (dest != null) {
+          dest.delete();
+        }
+      }
+    }
+  }
+
   @Test
   public void testResourceNameFormatValidation() throws Exception {
     String[] validNames = new String[] {
diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/resources/resource-types/node-resources-2.xml b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/resources/resource-types/node-resources-2.xml
index 382d5dd2ca2..3323294252f 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/resources/resource-types/node-resources-2.xml
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/resources/resource-types/node-resources-2.xml
@@ -15,17 +15,6 @@ limitations under the License. See accompanying LICENSE file.
 -->
 
 <configuration>
-
- <property>
-   <name>yarn.nodemanager.resource-type.memory-mb</name>
-   <value>1024Mi</value>
- </property>
-
- <property>
-   <name>yarn.nodemanager.resource-type.vcores</name>
-   <value>1</value>
- </property>
-
  <property>
    <name>yarn.nodemanager.resource-type.resource1</name>
    <value>5Gi</value>
diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/resources/resource-types/node-resources-error-1.xml b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/resources/resource-types/node-resources-error-1.xml
new file mode 100644
index 00000000000..4ea3c7445a7
--- /dev/null
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/resources/resource-types/node-resources-error-1.xml
@@ -0,0 +1,29 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<?xml-stylesheet type="text/xsl" href="configuration.xsl"?>
+<!--
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License. See accompanying LICENSE file.
+-->
+
+<configuration>
+
+    <property>
+        <name>yarn.nodemanager.resource-type.vcores</name>
+        <value>1024</value>
+    </property>
+
+    <property>
+        <name>yarn.nodemanager.resource.resource1</name>
+        <value>1</value>
+    </property>
+
+</configuration>
diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/resources/resource-types/resource-types-error-2.xml b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/resources/resource-types/resource-types-error-2.xml
index fa43b6c14ef..a34db385451 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/resources/resource-types/resource-types-error-2.xml
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/resources/resource-types/resource-types-error-2.xml
@@ -18,7 +18,7 @@ limitations under the License. See accompanying LICENSE file.
 
  <property>
    <name>yarn.resource-types</name>
-   <value>vcores,resource1</value>
+   <value>resource1,resource2</value>
  </property>
 
  <property>
@@ -27,7 +27,7 @@ limitations under the License. See accompanying LICENSE file.
  </property>
 
  <property>
-   <name>yarn.resource-types.vcores.units</name>
+   <name>yarn.resource-types.resource2.units</name>
    <value>Az</value>
  </property>
 </configuration>