From e0517fea3399946a20852cefff300eb3d4d7ece7 Mon Sep 17 00:00:00 2001 From: Szilard Nemeth Date: Mon, 12 Aug 2019 14:36:07 +0200 Subject: [PATCH] YARN-9134. No test coverage for redefining FPGA / GPU resource types in TestResourceUtils. Contributed by Peter Bacsko --- .../yarn/util/resource/TestResourceUtils.java | 164 +++++++++++------- 1 file changed, 98 insertions(+), 66 deletions(-) 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 7a701a4e90..9cc96b62e4 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 @@ -31,9 +31,15 @@ import org.junit.After; import org.junit.Assert; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import java.io.File; +import java.io.IOException; +import java.net.URL; import java.util.ArrayList; import java.util.HashMap; import java.util.List; @@ -43,6 +49,8 @@ * Test class to verify all resource utility methods. */ public class TestResourceUtils { + private static final Logger LOG = + LoggerFactory.getLogger(TestResourceUtils.class); private File nodeResourcesFile; private File resourceTypesFile; @@ -52,13 +60,16 @@ static class ResourceFileInformation { int resourceCount; Map resourceNameUnitsMap; - public ResourceFileInformation(String name, int count) { + ResourceFileInformation(String name, int count) { filename = name; resourceCount = count; resourceNameUnitsMap = new HashMap<>(); } } + @Rule + public ExpectedException expexted = ExpectedException.none(); + @Before public void setup() { ResourceUtils.resetResourceTypes(); @@ -66,14 +77,60 @@ public void setup() { @After public void teardown() { - if(nodeResourcesFile != null && nodeResourcesFile.exists()) { + if (nodeResourcesFile != null && nodeResourcesFile.exists()) { nodeResourcesFile.delete(); } - if(resourceTypesFile != null && resourceTypesFile.exists()) { + if (resourceTypesFile != null && resourceTypesFile.exists()) { resourceTypesFile.delete(); } } + public static String setupResourceTypes(Configuration conf, String filename) + throws Exception { + File source = new File( + conf.getClassLoader().getResource(filename).getFile()); + File dest = new File(source.getParent(), "resource-types.xml"); + FileUtils.copyFile(source, dest); + try { + ResourceUtils.getResourceTypes(); + } catch (Exception e) { + if (!dest.delete()) { + LOG.error("Could not delete {}", dest); + } + throw e; + } + return dest.getAbsolutePath(); + } + + private Map setupResourceTypesInternal( + Configuration conf, String srcFileName) throws IOException { + URL srcFileUrl = conf.getClassLoader().getResource(srcFileName); + if (srcFileUrl == null) { + throw new IllegalArgumentException( + "Source file does not exist: " + srcFileName); + } + File source = new File(srcFileUrl.getFile()); + File dest = new File(source.getParent(), "resource-types.xml"); + FileUtils.copyFile(source, dest); + this.resourceTypesFile = dest; + return ResourceUtils.getResourceTypes(); + } + + private Map setupNodeResources( + Configuration conf, String srcFileName) throws IOException { + URL srcFileUrl = conf.getClassLoader().getResource(srcFileName); + if (srcFileUrl == null) { + throw new IllegalArgumentException( + "Source file does not exist: " + srcFileName); + } + File source = new File(srcFileUrl.getFile()); + File dest = new File(source.getParent(), "node-resources.xml"); + FileUtils.copyFile(source, dest); + this.nodeResourcesFile = dest; + return ResourceUtils + .getNodeResourceInformation(conf); + } + private void testMemoryAndVcores(Map res) { String memory = ResourceInformation.MEMORY_MB.getName(); String vcores = ResourceInformation.VCORES.getName(); @@ -92,8 +149,7 @@ private void testMemoryAndVcores(Map res) { } @Test - public void testGetResourceTypes() throws Exception { - + public void testGetResourceTypes() { Map res = ResourceUtils.getResourceTypes(); Assert.assertEquals(2, res.size()); testMemoryAndVcores(res); @@ -101,7 +157,6 @@ public void testGetResourceTypes() throws Exception { @Test public void testGetResourceTypesConfigs() throws Exception { - Configuration conf = new YarnConfiguration(); ResourceFileInformation testFile1 = @@ -123,16 +178,11 @@ public void testGetResourceTypesConfigs() throws Exception { Map res; for (ResourceFileInformation testInformation : tests) { ResourceUtils.resetResourceTypes(); - File source = new File( - conf.getClassLoader().getResource(testInformation.filename) - .getFile()); - resourceTypesFile = new File(source.getParent(), "resource-types.xml"); - FileUtils.copyFile(source, resourceTypesFile); - res = ResourceUtils.getResourceTypes(); + res = setupResourceTypesInternal(conf, testInformation.filename); testMemoryAndVcores(res); Assert.assertEquals(testInformation.resourceCount, res.size()); - for (Map.Entry entry : testInformation.resourceNameUnitsMap - .entrySet()) { + for (Map.Entry entry : + testInformation.resourceNameUnitsMap.entrySet()) { String resourceName = entry.getKey(); Assert.assertTrue("Missing key " + resourceName, res.containsKey(resourceName)); @@ -142,7 +192,7 @@ public void testGetResourceTypesConfigs() throws Exception { } @Test - public void testGetResourceTypesConfigErrors() throws Exception { + public void testGetResourceTypesConfigErrors() throws IOException { Configuration conf = new YarnConfiguration(); String[] resourceFiles = {"resource-types-error-1.xml", @@ -151,22 +201,16 @@ public void testGetResourceTypesConfigErrors() throws Exception { for (String resourceFile : resourceFiles) { ResourceUtils.resetResourceTypes(); try { - File source = - new File(conf.getClassLoader().getResource(resourceFile).getFile()); - resourceTypesFile = new File(source.getParent(), "resource-types.xml"); - FileUtils.copyFile(source, resourceTypesFile); - ResourceUtils.getResourceTypes(); + setupResourceTypesInternal(conf, resourceFile); Assert.fail("Expected error with file " + resourceFile); - } catch (NullPointerException ne) { - throw ne; - } catch (Exception e) { + } catch (YarnRuntimeException | IllegalArgumentException e) { //Test passed } } } @Test - public void testInitializeResourcesMap() throws Exception { + public void testInitializeResourcesMap() { String[] empty = {"", ""}; String[] res1 = {"resource1", "m"}; String[] res2 = {"resource2", "G"}; @@ -227,8 +271,7 @@ public void testInitializeResourcesMap() throws Exception { } @Test - public void testInitializeResourcesMapErrors() throws Exception { - + public void testInitializeResourcesMapErrors() { String[] mem1 = {"memory-mb", ""}; String[] vcores1 = {"vcores", "M"}; @@ -268,11 +311,9 @@ public void testInitializeResourcesMapErrors() throws Exception { @Test public void testGetResourceInformation() throws Exception { - Configuration conf = new YarnConfiguration(); Map testRun = new HashMap<>(); - setupResourceTypes(conf, "resource-types-4.xml"); - // testRun.put("node-resources-1.xml", Resource.newInstance(1024, 1)); + setupResourceTypesInternal(conf, "resource-types-4.xml"); Resource test3Resources = Resource.newInstance(0, 0); test3Resources.setResourceInformation("resource1", ResourceInformation.newInstance("resource1", "Gi", 5L)); @@ -285,12 +326,8 @@ public void testGetResourceInformation() throws Exception { for (Map.Entry entry : testRun.entrySet()) { String resourceFile = entry.getKey(); ResourceUtils.resetNodeResources(); - File source = new File( - conf.getClassLoader().getResource(resourceFile).getFile()); - nodeResourcesFile = new File(source.getParent(), "node-resources.xml"); - FileUtils.copyFile(source, nodeResourcesFile); - Map actual = ResourceUtils - .getNodeResourceInformation(conf); + Map actual = setupNodeResources(conf, + resourceFile); Assert.assertEquals(actual.size(), entry.getValue().getResources().length); for (ResourceInformation resInfo : entry.getValue().getResources()) { @@ -302,28 +339,40 @@ public void testGetResourceInformation() throws Exception { @Test public void testGetNodeResourcesConfigErrors() throws Exception { Configuration conf = new YarnConfiguration(); - Map testRun = new HashMap<>(); - setupResourceTypes(conf, "resource-types-4.xml"); - String invalidNodeResFiles[] = { "node-resources-error-1.xml"}; + setupResourceTypesInternal(conf, "resource-types-4.xml"); + String[] invalidNodeResFiles = {"node-resources-error-1.xml"}; for (String resourceFile : invalidNodeResFiles) { ResourceUtils.resetNodeResources(); try { - File source = new File(conf.getClassLoader().getResource(resourceFile).getFile()); - nodeResourcesFile = new File(source.getParent(), "node-resources.xml"); - FileUtils.copyFile(source, nodeResourcesFile); - Map actual = ResourceUtils.getNodeResourceInformation(conf); + setupNodeResources(conf, resourceFile); Assert.fail("Expected error with file " + resourceFile); - } catch (NullPointerException ne) { - throw ne; - } catch (Exception e) { + } catch (YarnRuntimeException e) { //Test passed } } } @Test - public void testResourceNameFormatValidation() throws Exception { + public void testGetNodeResourcesRedefineFpgaErrors() throws Exception { + Configuration conf = new YarnConfiguration(); + expexted.expect(YarnRuntimeException.class); + expexted.expectMessage("Defined mandatory resource type=yarn.io/fpga"); + setupResourceTypesInternal(conf, + "resource-types-error-redefine-fpga-unit.xml"); + } + + @Test + public void testGetNodeResourcesRedefineGpuErrors() throws Exception { + Configuration conf = new YarnConfiguration(); + expexted.expect(YarnRuntimeException.class); + expexted.expectMessage("Defined mandatory resource type=yarn.io/gpu"); + setupResourceTypesInternal(conf, + "resource-types-error-redefine-gpu-unit.xml"); + } + + @Test + public void testResourceNameFormatValidation() { String[] validNames = new String[] { "yarn.io/gpu", "gpu", @@ -360,10 +409,9 @@ public void testResourceNameFormatValidation() throws Exception { @Test public void testGetResourceInformationWithDiffUnits() throws Exception { - Configuration conf = new YarnConfiguration(); Map testRun = new HashMap<>(); - setupResourceTypes(conf, "resource-types-4.xml"); + setupResourceTypesInternal(conf, "resource-types-4.xml"); Resource test3Resources = Resource.newInstance(0, 0); //Resource 'resource1' has been passed as 5T @@ -382,12 +430,8 @@ public void testGetResourceInformationWithDiffUnits() throws Exception { for (Map.Entry entry : testRun.entrySet()) { String resourceFile = entry.getKey(); ResourceUtils.resetNodeResources(); - File source = new File( - conf.getClassLoader().getResource(resourceFile).getFile()); - nodeResourcesFile = new File(source.getParent(), "node-resources.xml"); - FileUtils.copyFile(source, nodeResourcesFile); - Map actual = ResourceUtils - .getNodeResourceInformation(conf); + Map actual = setupNodeResources(conf, + resourceFile); Assert.assertEquals(actual.size(), entry.getValue().getResources().length); for (ResourceInformation resInfo : entry.getValue().getResources()) { @@ -460,18 +504,6 @@ public void testResourceUnitParsing() throws Exception { "memory=2G,vcores=3,yarn.io/gpu=3", resTypes); Assert.assertEquals(2 * 1024, res.getMemorySize()); Assert.assertEquals(3, res.getResourceValue(ResourceInformation.GPU_URI)); - - // TODO, add more negative tests. - } - - public static String setupResourceTypes(Configuration conf, String filename) - throws Exception { - File source = new File( - conf.getClassLoader().getResource(filename).getFile()); - File dest = new File(source.getParent(), "resource-types.xml"); - FileUtils.copyFile(source, dest); - ResourceUtils.getResourceTypes(); - return dest.getAbsolutePath(); } @Test