Skip to content

YARN-11645. Order the queues by queuePath in json assert tests. #6432

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
brumi1024 merged 11 commits into from
Jan 24, 2024
Merged

YARN-11645. Order the queues by queuePath in json assert tests. #6432

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
78565a1
YARN-11645. Order the queues by queuePath in json assert tests.
tomicooler Jan 10, 2024
f0e2538
fix missing else
tomicooler Jan 11, 2024
9112e9c
remove unused method
tomicooler Jan 11, 2024
236d087
TestRMWebServicesConfigurationMutation polluted the test target dir with
tomicooler Jan 11, 2024
a5f9e4e
jettison.json -> jackson
tomicooler Jan 11, 2024
d1ec7f8
ensure there is no cs.xml
tomicooler Jan 11, 2024
2a33f37
make the objectmapper static final
tomicooler Jan 11, 2024
b574033
just int-float changes (jettison converted the floats to ints before)
tomicooler Jan 11, 2024
08b33a1
backup/restore config file in temp
tomicooler Jan 15, 2024
27fa42f
checkstyle fixes
tomicooler Jan 15, 2024
542efb5
no longer needed, afterclass cleanup is enough
tomicooler Jan 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 3 additions & 15 deletions ...che/hadoop/yarn/server/resourcemanager/webapp/TestRMWebServicesConfigurationMutation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@
import static org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.CapacitySchedulerConfiguration.ACCESSIBLE_NODE_LABELS;
import static org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.CapacitySchedulerConfiguration.CAPACITY;
import static org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.CapacitySchedulerConfiguration.MAXIMUM_CAPACITY;
import static org.apache.hadoop.yarn.server.resourcemanager.webapp.TestWebServiceUtil.getCapacitySchedulerConfigFileInTarget;
import static org.apache.hadoop.yarn.webapp.util.YarnWebServiceUtils.toJson;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;
Expand All @@ -80,11 +81,6 @@
public class TestRMWebServicesConfigurationMutation extends JerseyTestBase {
private static final Logger LOG = LoggerFactory
.getLogger(TestRMWebServicesConfigurationMutation.class);

private static final File CONF_FILE = new File(new File("target",
"test-classes"), YarnConfiguration.CS_CONFIGURATION_FILE);
private static final File OLD_CONF_FILE = new File(new File("target",
"test-classes"), YarnConfiguration.CS_CONFIGURATION_FILE + ".tmp");
private static final String LABEL_1 = "label1";
public static final QueuePath ROOT = new QueuePath("root");
public static final QueuePath ROOT_A = new QueuePath("root", "a");
Expand Down Expand Up @@ -117,12 +113,7 @@ protected void configureServlets() {
YarnConfiguration.MEMORY_CONFIGURATION_STORE);
conf.set(YarnConfiguration.YARN_ADMIN_ACL, userName);
try {
if (CONF_FILE.exists()) {
if (!CONF_FILE.renameTo(OLD_CONF_FILE)) {
throw new RuntimeException("Failed to rename conf file");
}
}
FileOutputStream out = new FileOutputStream(CONF_FILE);
FileOutputStream out = new FileOutputStream(getCapacitySchedulerConfigFileInTarget());
csConf.writeXml(out);
out.close();
} catch (IOException e) {
Expand Down Expand Up @@ -1070,10 +1061,7 @@ public void tearDown() throws Exception {
if (rm != null) {
rm.stop();
}
CONF_FILE.delete();
if (!OLD_CONF_FILE.renameTo(CONF_FILE)) {
throw new RuntimeException("Failed to re-copy old configuration file");
}
getCapacitySchedulerConfigFileInTarget().delete();
super.tearDown();
}
}
91 changes: 73 additions & 18 deletions ...rc/test/java/org/apache/hadoop/yarn/server/resourcemanager/webapp/TestWebServiceUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,18 @@
package org.apache.hadoop.yarn.server.resourcemanager.webapp;

import java.io.BufferedReader;
import java.io.File;
import java.io.FileWriter;
import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
import java.io.StringReader;
import java.io.StringWriter;
import java.net.URISyntaxException;
import java.util.ArrayList;
import java.util.Comparator;
import java.util.Iterator;
import java.util.List;
import java.util.Objects;
import java.util.stream.Collectors;
import javax.ws.rs.core.MediaType;
Expand All @@ -40,12 +45,15 @@

import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.SerializationFeature;
import com.google.inject.Guice;
import com.google.inject.servlet.ServletModule;
import com.sun.jersey.api.client.ClientResponse;
import com.sun.jersey.api.client.WebResource;
import com.sun.jersey.guice.spi.container.servlet.GuiceContainer;
import com.sun.jersey.test.framework.WebAppDescriptor;

import org.codehaus.jettison.json.JSONArray;
import org.codehaus.jettison.json.JSONException;
import org.codehaus.jettison.json.JSONObject;
import org.junit.Assert;
Expand All @@ -65,8 +73,6 @@
import org.apache.hadoop.yarn.webapp.GenericExceptionHandler;
import org.apache.hadoop.yarn.webapp.GuiceServletConfig;

import static org.apache.hadoop.yarn.conf.YarnConfiguration.MEMORY_CONFIGURATION_STORE;
import static org.apache.hadoop.yarn.conf.YarnConfiguration.SCHEDULER_CONFIGURATION_STORE_CLASS;
import static org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.CapacitySchedulerTestUtilities.GB;
import static org.junit.Assert.assertEquals;

Expand Down Expand Up @@ -94,7 +100,6 @@ protected void configureServlets() {
public static void runTest(String template, String name,
MockRM rm,
WebResource resource) throws Exception {
final boolean reinitAfterNodeChane = isMutableConfig(rm.getConfig());
try {
boolean legacyQueueMode = ((CapacityScheduler) rm.getResourceScheduler())
.getConfiguration().isLegacyQueueMode();
Expand All @@ -105,16 +110,10 @@ public static void runTest(String template, String name,

MockNM nm1 = rm.registerNode("h1:1234", 8 * GB, 8);
rm.registerNode("h2:1234", 8 * GB, 8);
if (reinitAfterNodeChane) {
reinitialize(rm, rm.getConfig());
}
assertJsonResponse(sendRequest(resource),
getExpectedResourceFile(template, name, "16", legacyQueueMode));
rm.registerNode("h3:1234", 8 * GB, 8);
MockNM nm4 = rm.registerNode("h4:1234", 8 * GB, 8);
if (reinitAfterNodeChane) {
reinitialize(rm, rm.getConfig());
}

assertJsonResponse(sendRequest(resource),
getExpectedResourceFile(template, name, "32", legacyQueueMode));
Expand Down Expand Up @@ -163,11 +162,6 @@ public static String legacySuffix(boolean legacyQueueMode, String text) {
return text;
}

public static boolean isMutableConfig(Configuration config) {
return Objects.equals(config.get(SCHEDULER_CONFIGURATION_STORE_CLASS),
MEMORY_CONFIGURATION_STORE);
}

public static ClientResponse sendRequest(WebResource resource) {
return resource.path("ws").path("v1").path("cluster")
.path("scheduler").accept(MediaType.APPLICATION_JSON)
Expand Down Expand Up @@ -213,15 +207,74 @@ public static void assertJsonResponse(ClientResponse response,
IOException {
assertJsonType(response);
JSONObject json = response.getEntity(JSONObject.class);
sortQueuesLexically(json);
Copy link
Member

@brumi1024 brumi1024 Jan 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was meant to ask this before: is there a reason we're using two separate Json libraries to do (in the end) a string comparison? Couldn't we move completely to Jackson?

Copy link
Contributor Author

@tomicooler tomicooler Jan 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved to jackson. There was a change in the test data, because jettison converted the floats (e.g. capacity=100.0) to an int (capacity=100).

String actual = prettyPrintJson(json.toString(2));
updateTestDataAutomatically(expectedResourceFilename, actual);
assertEquals(
prettyPrintJson(getResourceAsString(expectedResourceFilename)),
actual);
}

/**
* Sorts the "queue": [ {}, {}, {} ] parts recursively by the queuePath key.
*
* <p>
* There was a marshalling error described in YARN-4785 in CapacitySchedulerInfo.getQueues().
* If that issue still present, we can't sort the queues there, but only sort the leaf queues
* then the non-leaf queues which would make a consistent output, but hard to document.
* Instead we make sure the test data is at least ordered by queue names.
* </p>
*
* @param object the json object to sort.
* @throws JSONException when
*/
private static void sortQueuesLexically(JSONObject object) throws JSONException {
Iterator<?> keys = object.keys();
while (keys.hasNext()) {
String key = (String) keys.next();
Object o = object.get(key);
if (key.equals("queue") && (o instanceof JSONArray)) {
JSONArray original = (JSONArray) o;

List<JSONObject> queues = new ArrayList<>(original.length());
for (int i = 0; i < original.length(); i++) {
if (original.get(i) instanceof JSONObject) {
queues.add((JSONObject) original.get(i));
}
}
queues.sort(new Comparator<JSONObject>() {
private static final String SORT_BY_KEY = "queuePath";

@Override
public int compare(JSONObject a, JSONObject b) {
String vA = "";
String vB = "";

try {
vA = (String) a.get(SORT_BY_KEY);
vB = (String) b.get(SORT_BY_KEY);
} catch (JSONException ignored) {
}

return vA.compareTo(vB);
}
});

JSONArray sortedArray = new JSONArray(queues.size());
for (JSONObject queue : queues) {
sortedArray.put(queue);
}

object.put("queue", sortedArray);
} else if (o instanceof JSONObject) {
sortQueuesLexically((JSONObject) o);
}
}
}

private static String prettyPrintJson(String in) throws JsonProcessingException {
ObjectMapper objectMapper = new ObjectMapper();
objectMapper.configure(SerializationFeature.ORDER_MAP_ENTRIES_BY_KEYS, true);
return objectMapper
.writerWithDefaultPrettyPrinter()
.writeValueAsString(objectMapper.readTree(in));
Expand Down Expand Up @@ -293,12 +346,10 @@ public static MockRM createRM(Configuration config) {
return rm;
}

public static MockRM createMutableRM(Configuration conf) throws IOException {
public static MockRM createMutableRM(Configuration conf) {
conf.set(YarnConfiguration.SCHEDULER_CONFIGURATION_STORE_CLASS,
YarnConfiguration.MEMORY_CONFIGURATION_STORE);
MockRM rm = createRM(new CapacitySchedulerConfiguration(conf));
reinitialize(rm, conf);
return rm;
return createRM(new CapacitySchedulerConfiguration(conf));
}

public static void reinitialize(MockRM rm, Configuration conf) throws IOException {
Expand All @@ -309,4 +360,8 @@ public static void reinitialize(MockRM rm, Configuration conf) throws IOExceptio
CapacityScheduler cs = (CapacityScheduler) rm.getResourceScheduler();
cs.reinitialize(conf, rm.getRMContext(), true);
}

public static File getCapacitySchedulerConfigFileInTarget() {
return new File("target/test-classes", YarnConfiguration.CS_CONFIGURATION_FILE);
}
}
Loading