YARN-10082. FS-CS converter: disable terminal placement rule checking. Contributed by Peter Bacsko

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/FairScheduler.java

@@ -215,6 +215,7 @@ public class FairScheduler extends
   Resource reservationThreshold;
 
   private boolean migration;
+  private boolean noTerminalRuleCheck;
 
   public FairScheduler() {
     super(FairScheduler.class.getName());
@@ -1502,6 +1503,10 @@ public class FairScheduler extends
   public void serviceInit(Configuration conf) throws Exception {
     migration =
         conf.getBoolean(FairSchedulerConfiguration.MIGRATION_MODE, false);
+    noTerminalRuleCheck = migration &&
+        conf.getBoolean(FairSchedulerConfiguration.NO_TERMINAL_RULE_CHECK,
+            false);
+
     initScheduler(conf);
     super.serviceInit(conf);
 
@@ -2001,4 +2006,8 @@ public class FairScheduler extends
     throw new YarnException(
         "Update application priority is not supported in Fair Scheduler");
   }
+
+  public boolean isNoTerminalRuleCheck() {
+    return noTerminalRuleCheck;
+  }
 }

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/FairSchedulerConfiguration.java

@@ -91,6 +91,13 @@ public class FairSchedulerConfiguration extends Configuration {
    */
   public static final String MIGRATION_MODE = CONF_PREFIX + "migration.mode";
 
+  /**
+   * Disables checking whether a placement rule is terminal or not. Only
+   * used during migration mode. This property should NOT be used by end users!
+   */
+  public static final String NO_TERMINAL_RULE_CHECK = CONF_PREFIX +
+      "no-terminal-rule.check";
+
   public static final String ALLOCATION_FILE = CONF_PREFIX + "allocation.file";
   protected static final String DEFAULT_ALLOCATION_FILE = "fair-scheduler.xml";
   

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/QueuePlacementPolicy.java

@@ -107,8 +107,13 @@ final class QueuePlacementPolicy {
     LOG.debug("Placement rule order check");
     for (int i = 0; i < newTerminalState.size()-1; i++) {
       if (newTerminalState.get(i)) {
-        throw new AllocationConfigurationException("Rules after rule "
+        String errorMsg = "Rules after rule "
-            + (i+1) + " in queue placement policy can never be reached");
+            + (i+1) + " in queue placement policy can never be reached";
+        if (fs.isNoTerminalRuleCheck()) {
+          LOG.warn(errorMsg);
+        } else {
+          throw new AllocationConfigurationException(errorMsg);
+        }
       }
     }
     if (!newTerminalState.get(newTerminalState.size()-1)) {

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/converter/ConversionOptions.java

@@ -21,6 +21,7 @@ import org.slf4j.Logger;
 public class ConversionOptions {
   private DryRunResultHolder dryRunResultHolder;
   private boolean dryRun;
+  private boolean noTerminalRuleCheck;
 
   public ConversionOptions(DryRunResultHolder dryRunResultHolder,
       boolean dryRun) {
@@ -32,6 +33,14 @@ public class ConversionOptions {
     this.dryRun = dryRun;
   }
 
+  public void setNoTerminalRuleCheck(boolean ruleTerminalCheck) {
+    this.noTerminalRuleCheck = ruleTerminalCheck;
+  }
+
+  public boolean isNoRuleTerminalCheck() {
+    return noTerminalRuleCheck;
+  }
+
   public void handleWarning(String msg, Logger log) {
     if (dryRun) {
       dryRunResultHolder.addDryRunWarning(msg);

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/converter/FSConfigToCSConfigArgumentHandler.java

@@ -85,6 +85,11 @@ public class FSConfigToCSConfigArgumentHandler {
             true),
     DRY_RUN("dry run", "d", "dry-run", "Performs a dry-run of the conversion." +
             "Outputs whether the conversion is possible or not.", false),
+    NO_TERMINAL_RULE_CHECK("no terminal rule check", "t",
+        "no-terminal-rule-check",
+        "Disables checking whether a placement rule is terminal to maintain" +
+        " backward compatibility with configs that were made before YARN-8967.",
+        false),
     HELP("help", "h", "help", "Displays the list of options", false);
 
     private final String name;
@@ -180,6 +185,8 @@ public class FSConfigToCSConfigArgumentHandler {
       CommandLine cliParser) {
     conversionOptions.setDryRun(
         cliParser.hasOption(CliOption.DRY_RUN.shortSwitch));
+    conversionOptions.setNoTerminalRuleCheck(
+        cliParser.hasOption(CliOption.NO_TERMINAL_RULE_CHECK.shortSwitch));
 
     checkOptionPresent(cliParser, CliOption.YARN_SITE);
     checkOutputDefined(cliParser);

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/converter/FSConfigToCSConfigConverter.java

@@ -164,6 +164,8 @@ public class FSConfigToCSConfigConverter {
     Configuration conf = new YarnConfiguration();
     conf.addResource(new Path(params.getYarnSiteXmlConfig()));
     conf.setBoolean(FairSchedulerConfiguration.MIGRATION_MODE, true);
+    conf.setBoolean(FairSchedulerConfiguration.NO_TERMINAL_RULE_CHECK,
+        conversionOptions.isNoRuleTerminalCheck());
     return conf;
   }
 

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/converter/TestFSConfigToCSConfigArgumentHandler.java

@@ -444,4 +444,41 @@ public class TestFSConfigToCSConfigArgumentHandler {
     assertTrue("Unexpected error message",
         error.contains(expectedErrorMessage));
   }
+
+  @Test
+  public void testDisabledTerminalRuleCheck() throws Exception {
+    setupFSConfigConversionFiles(true);
+
+    String[] args = getArgumentsAsArrayWithDefaults("-f",
+        FSConfigConverterTestCommons.FS_ALLOC_FILE,
+        "-r", FSConfigConverterTestCommons.CONVERSION_RULES_FILE, "-p",
+        "-t");
+
+    FSConfigToCSConfigArgumentHandler argumentHandler =
+        new FSConfigToCSConfigArgumentHandler(conversionOptions);
+    argumentHandler.setConverterSupplier(this::getMockConverter);
+
+    argumentHandler.parseAndConvert(args);
+
+    assertTrue("-t switch had no effect",
+        conversionOptions.isNoRuleTerminalCheck());
+  }
+
+  @Test
+  public void testEnabledTerminalRuleCheck() throws Exception {
+    setupFSConfigConversionFiles(true);
+
+    String[] args = getArgumentsAsArrayWithDefaults("-f",
+        FSConfigConverterTestCommons.FS_ALLOC_FILE,
+        "-r", FSConfigConverterTestCommons.CONVERSION_RULES_FILE, "-p");
+
+    FSConfigToCSConfigArgumentHandler argumentHandler =
+        new FSConfigToCSConfigArgumentHandler(conversionOptions);
+    argumentHandler.setConverterSupplier(this::getMockConverter);
+
+    argumentHandler.parseAndConvert(args);
+
+    assertFalse("No terminal rule check was enabled",
+        conversionOptions.isNoRuleTerminalCheck());
+  }
 }

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/converter/TestFSConfigToCSConfigConverter.java

@@ -38,6 +38,7 @@ import java.util.Map;
 
 import org.apache.commons.io.FileUtils;
 import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.service.ServiceStateException;
 import org.apache.hadoop.yarn.api.records.Resource;
 import org.apache.hadoop.yarn.conf.YarnConfiguration;
 import org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.CapacitySchedulerConfiguration;
@@ -64,6 +65,8 @@ public class TestFSConfigToCSConfigConverter {
   private static final String FILE_PREFIX = "file:";
   private static final String FAIR_SCHEDULER_XML =
       prepareFileName("fair-scheduler-conversion.xml");
+  private static final String FS_INVALID_PLACEMENT_RULES_XML =
+      prepareFileName("fair-scheduler-invalidplacementrules.xml");
 
   @Mock
   private FSConfigToCSConfigRuleHandler ruleHandler;
@@ -446,6 +449,36 @@ public class TestFSConfigToCSConfigConverter {
     converter.convert(params);
   }
 
+  @Test
+  public void testConversionWithInvalidPlacementRules() throws Exception {
+    config = new Configuration(false);
+    config.set(FairSchedulerConfiguration.ALLOCATION_FILE,
+        FS_INVALID_PLACEMENT_RULES_XML);
+    config.setBoolean(FairSchedulerConfiguration.MIGRATION_MODE, true);
+    expectedException.expect(ServiceStateException.class);
+
+    converter.convert(config);
+  }
+
+  @Test
+  public void testConversionWhenInvalidPlacementRulesIgnored()
+      throws Exception {
+    FSConfigToCSConfigConverterParams params = createDefaultParamsBuilder()
+        .withClusterResource("vcores=20, memory-mb=240")
+        .withFairSchedulerXmlConfig(FS_INVALID_PLACEMENT_RULES_XML)
+        .build();
+
+    ConversionOptions conversionOptions = createDefaultConversionOptions();
+    conversionOptions.setNoTerminalRuleCheck(true);
+
+    converter = new FSConfigToCSConfigConverter(ruleHandler,
+        conversionOptions);
+
+    converter.convert(params);
+
+    // expected: no exception
+  }
+
   private Configuration getConvertedCSConfig() {
     ByteArrayInputStream input =
         new ByteArrayInputStream(csConfigOut.toByteArray());

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/resources/fair-scheduler-invalidplacementrules.xml

@@ -0,0 +1,56 @@
+<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
+<!--
+  Licensed to the Apache Software Foundation (ASF) under one
+  or more contributor license agreements.  See the NOTICE file
+  distributed with this work for additional information
+  regarding copyright ownership.  The ASF licenses this file
+  to you 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.
+-->
+<allocations>
+    <queue name="root">
+        <weight>1.0</weight>
+        <schedulingPolicy>drf</schedulingPolicy>
+        <aclSubmitApps>*</aclSubmitApps>
+        <aclAdministerApps>*</aclAdministerApps>
+        <queue name="default">
+            <weight>1.0</weight>
+            <schedulingPolicy>drf</schedulingPolicy>
+        </queue>
+        <queue name="users" type="parent">
+            <weight>1.0</weight>
+            <schedulingPolicy>drf</schedulingPolicy>
+            <queue name="joe">
+                <maxResources>8192 mb, 4 vcores</maxResources>
+                <weight>2.0</weight>
+                <schedulingPolicy>drf</schedulingPolicy>
+            </queue>
+            <queue name="alice">
+                <maxResources>50.0%</maxResources>
+                <maxRunningApps>20</maxRunningApps>
+                <weight>1.0</weight>
+                <schedulingPolicy>drf</schedulingPolicy>
+                <maxAMShare>0.2</maxAMShare>
+            </queue>
+        </queue>
+    </queue>
+    <defaultQueueSchedulingPolicy>fair</defaultQueueSchedulingPolicy>
+
+    <!-- this placement was accepted before YARN-8967 -->
+    <queuePlacementPolicy>
+        <rule name="specified" create="true"/>
+        <rule name="nestedUserQueue" create="true">
+            <rule name="default" create="true" queue="users"/>
+        </rule>
+        <rule name="default"/>
+    </queuePlacementPolicy>
+</allocations>