You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by GitBox <gi...@apache.org> on 2022/02/17 19:35:37 UTC

[GitHub] [hadoop] szilard-nemeth commented on a change in pull request #3812: YARN-11052. Improve code quality in TestRMWebServicesNodeLabels

szilard-nemeth commented on a change in pull request #3812:
URL: https://github.com/apache/hadoop/pull/3812#discussion_r809403464



##########
File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/webapp/TestRMWebServicesNodeLabels.java
##########
@@ -119,527 +132,441 @@ public TestRMWebServicesNodeLabels() {
   }
 
   @Test
-  public void testNodeLabels() throws JSONException, Exception {
+  public void testNodeLabels() throws Exception {
     WebResource r = resource();
-
     ClientResponse response;
 
     // Add a label
-    NodeLabelsInfo nlsifo = new NodeLabelsInfo();
-    nlsifo.getNodeLabelsInfo().add(new NodeLabelInfo("a"));
-    response =
-        r.path("ws").path("v1").path("cluster")
-            .path("add-node-labels").queryParam("user.name", userName)
-            .accept(MediaType.APPLICATION_JSON)
-            .entity(toJson(nlsifo, NodeLabelsInfo.class),
-                MediaType.APPLICATION_JSON)
-            .post(ClientResponse.class);
+    response = addNodeLabels(r, Lists.newArrayList(Pair.of(LABEL_A, DEFAULT_NL_EXCLUSIVITY)));
+    assertHttp200(response);
 
     // Verify
-    response =
-        r.path("ws").path("v1").path("cluster")
-            .path("get-node-labels").queryParam("user.name", userName)
-            .accept(MediaType.APPLICATION_JSON).get(ClientResponse.class);
-    assertEquals(MediaType.APPLICATION_JSON_TYPE + "; " + JettyUtils.UTF_8,
-        response.getType().toString());
-    nlsifo = response.getEntity(NodeLabelsInfo.class);
-    assertEquals(1, nlsifo.getNodeLabels().size());
-    for (NodeLabelInfo nl : nlsifo.getNodeLabelsInfo()) {
-      assertEquals("a", nl.getName());
-      assertTrue(nl.getExclusivity());
-    }
-    
+    response = getNodeLabels(r);
+    assertApplicationJsonUtf8Response(response);
+    assertNodeLabelsInfo(response.getEntity(NodeLabelsInfo.class), Lists.newArrayList(
+        Pair.of(LABEL_A, true)));
+
     // Add another
-    nlsifo = new NodeLabelsInfo();
-    nlsifo.getNodeLabelsInfo().add(new NodeLabelInfo("b", false));
-    response =
-        r.path("ws").path("v1").path("cluster")
-            .path("add-node-labels").queryParam("user.name", userName)
-            .accept(MediaType.APPLICATION_JSON)
-            .entity(toJson(nlsifo, NodeLabelsInfo.class),
-                MediaType.APPLICATION_JSON)
-            .post(ClientResponse.class);
+    response = addNodeLabels(r, Lists.newArrayList(Pair.of(LABEL_B, false)));
+    assertHttp200(response);
 
     // Verify
-    response =
-        r.path("ws").path("v1").path("cluster")
-            .path("get-node-labels").queryParam("user.name", userName)
-            .accept(MediaType.APPLICATION_JSON).get(ClientResponse.class);
-    assertEquals(MediaType.APPLICATION_JSON_TYPE + "; " + JettyUtils.UTF_8,
-        response.getType().toString());
-    nlsifo = response.getEntity(NodeLabelsInfo.class);
-    assertEquals(2, nlsifo.getNodeLabels().size());
-    // Verify exclusivity for 'y' as false
-    for (NodeLabelInfo nl : nlsifo.getNodeLabelsInfo()) {
-      if (nl.getName().equals("b")) {
-        assertFalse(nl.getExclusivity());
-      }
-    }
-    
+    response = getNodeLabels(r);
+    assertApplicationJsonUtf8Response(response);
+    // Verify exclusivity for 'b' as false
+    assertNodeLabelsInfo(response.getEntity(NodeLabelsInfo.class),
+        Lists.newArrayList(
+            Pair.of(LABEL_A, true),
+            Pair.of(LABEL_B, false)));
+
     // Add labels to a node
-    MultivaluedMapImpl params = new MultivaluedMapImpl();
-    params.add("labels", "a");
-    response =
-        r.path("ws").path("v1").path("cluster")
-            .path("nodes").path("nid:0")
-            .path("replace-labels")
-            .queryParam("user.name", userName)
-            .queryParams(params)
-            .accept(MediaType.APPLICATION_JSON)
-            .post(ClientResponse.class);
-    LOG.info("posted node nodelabel");
+    response = replaceLabelsOnNode(r, NODE_0, LABEL_A);
+    assertHttp200(response);
 
     // Add labels to another node
-    params = new MultivaluedMapImpl();
-    params.add("labels", "b");
-    response =
-        r.path("ws").path("v1").path("cluster")
-            .path("nodes").path("nid1:0")
-            .path("replace-labels")
-            .queryParam("user.name", userName)
-            .queryParams(params)
-            .accept(MediaType.APPLICATION_JSON)
-            .post(ClientResponse.class);
-    LOG.info("posted node nodelabel");
+    response = replaceLabelsOnNode(r, NODE_1, LABEL_B);
+    assertHttp200(response);
 
     // Add labels to another node
-    params = new MultivaluedMapImpl();
-    params.add("labels", "b");
-    response =
-        r.path("ws").path("v1").path("cluster")
-            .path("nodes").path("nid2:0")
-            .path("replace-labels")
-            .queryParam("user.name", userName)
-            .queryParams(params)
-            .accept(MediaType.APPLICATION_JSON)
-            .post(ClientResponse.class);
-    LOG.info("posted node nodelabel");
-
-    // Verify, using get-labels-to-Nodes
-    response =
-        r.path("ws").path("v1").path("cluster")
-            .path("label-mappings").queryParam("user.name", userName)
-            .accept(MediaType.APPLICATION_JSON).get(ClientResponse.class);
-    assertEquals(MediaType.APPLICATION_JSON_TYPE + "; " + JettyUtils.UTF_8,
-        response.getType().toString());
-    LabelsToNodesInfo ltni = response.getEntity(LabelsToNodesInfo.class);
-    assertEquals(2, ltni.getLabelsToNodes().size());
-    NodeIDsInfo nodes = ltni.getLabelsToNodes().get(
-        new NodeLabelInfo("b", false));
-    assertTrue(nodes.getNodeIDs().contains("nid2:0"));
-    assertTrue(nodes.getNodeIDs().contains("nid1:0"));
-    nodes = ltni.getLabelsToNodes().get(new NodeLabelInfo("a"));
-    assertTrue(nodes.getNodeIDs().contains("nid:0"));
+    response = replaceLabelsOnNode(r, NODE_2, LABEL_B);
+    assertHttp200(response);
+
+    // Verify all, using get-labels-to-Nodes
+    response = getNodeLabelMappings(r);
+    assertApplicationJsonUtf8Response(response);
+    LabelsToNodesInfo labelsToNodesInfo = response.getEntity(LabelsToNodesInfo.class);
+    assertLabelsToNodesInfo(labelsToNodesInfo, 2, Lists.newArrayList(
+        Pair.of(Pair.of(LABEL_B, false), Lists.newArrayList(NODE_1, NODE_2)),
+        Pair.of(Pair.of(LABEL_A, DEFAULT_NL_EXCLUSIVITY), Lists.newArrayList(NODE_0))
+    ));
 
     // Verify, using get-labels-to-Nodes for specified set of labels
-    params = new MultivaluedMapImpl();
-    params.add("labels", "a");
-    response =
-        r.path("ws").path("v1").path("cluster")
-            .path("label-mappings").queryParam("user.name", userName)
-            .queryParams(params)
-            .accept(MediaType.APPLICATION_JSON)
-            .get(ClientResponse.class);
-    assertEquals(MediaType.APPLICATION_JSON_TYPE + "; " + JettyUtils.UTF_8,
-        response.getType().toString());
-    ltni = response.getEntity(LabelsToNodesInfo.class);
-    assertEquals(1, ltni.getLabelsToNodes().size());
-    nodes = ltni.getLabelsToNodes().get(new NodeLabelInfo("a"));
-    assertTrue(nodes.getNodeIDs().contains("nid:0"));
+    response = getNodeLabelMappingsByLabels(r, LABEL_A);
+    assertApplicationJsonUtf8Response(response);
+    labelsToNodesInfo = response.getEntity(LabelsToNodesInfo.class);
+    assertLabelsToNodesInfo(labelsToNodesInfo, 1, Lists.newArrayList(
+        Pair.of(Pair.of(LABEL_A, DEFAULT_NL_EXCLUSIVITY), Lists.newArrayList(NODE_0))
+    ));
 
     // Verify
-    response =
-        r.path("ws").path("v1").path("cluster")
-            .path("nodes").path("nid:0")
-            .path("get-labels").queryParam("user.name", userName)
-            .accept(MediaType.APPLICATION_JSON).get(ClientResponse.class);
-    assertEquals(MediaType.APPLICATION_JSON_TYPE + "; " + JettyUtils.UTF_8,
-        response.getType().toString());
-    nlsifo = response.getEntity(NodeLabelsInfo.class);
-    assertTrue(nlsifo.getNodeLabelsInfo().contains(new NodeLabelInfo("a")));
+    response = getLabelsOfNode(r, NODE_0);
+    assertApplicationJsonUtf8Response(response);
+    assertNodeLabelsInfoContains(response.getEntity(NodeLabelsInfo.class),
+        Pair.of(LABEL_A, DEFAULT_NL_EXCLUSIVITY));
 
-    
     // Replace
-    params = new MultivaluedMapImpl();
-    params.add("labels", "b");
-    response =
-        r.path("ws").path("v1").path("cluster")
-            .path("nodes").path("nid:0")
-            .path("replace-labels")
-            .queryParam("user.name", userName)
-            .queryParams(params)
-            .accept(MediaType.APPLICATION_JSON)
-            .post(ClientResponse.class);
-    LOG.info("posted node nodelabel");
+    response = replaceLabelsOnNode(r, NODE_0, LABEL_B);
+    assertHttp200(response);
 
     // Verify
-    response =
-        r.path("ws").path("v1").path("cluster")
-            .path("nodes").path("nid:0")
-            .path("get-labels").queryParam("user.name", userName)
-            .accept(MediaType.APPLICATION_JSON).get(ClientResponse.class);
-    assertEquals(MediaType.APPLICATION_JSON_TYPE + "; " + JettyUtils.UTF_8,
-        response.getType().toString());
-    nlsifo = response.getEntity(NodeLabelsInfo.class);
-    assertTrue(nlsifo.getNodeLabelsInfo().contains(
-        new NodeLabelInfo("b", false)));
-            
+    response = getLabelsOfNode(r, NODE_0);
+    assertApplicationJsonUtf8Response(response);
+    assertNodeLabelsInfoContains(response.getEntity(NodeLabelsInfo.class), Pair.of(LABEL_B, false));
+
     // Replace labels using node-to-labels
-    NodeToLabelsEntryList ntli = new NodeToLabelsEntryList();
-    ArrayList<String> labels = new ArrayList<String>();
-    labels.add("a");
-    NodeToLabelsEntry nli = new NodeToLabelsEntry("nid:0", labels);
-    ntli.getNodeToLabels().add(nli);
-    response =
-        r.path("ws").path("v1").path("cluster")
-            .path("replace-node-to-labels")
-            .queryParam("user.name", userName)
-            .accept(MediaType.APPLICATION_JSON)
-            .entity(toJson(ntli, NodeToLabelsEntryList.class),
-              MediaType.APPLICATION_JSON)
-            .post(ClientResponse.class);
-        
+    response = replaceNodeToLabels(r, Lists.newArrayList(Pair.of(NODE_0,
+        Lists.newArrayList(LABEL_A))));
+    assertHttp200(response);
+
     // Verify, using node-to-labels
-    response =
-        r.path("ws").path("v1").path("cluster")
-            .path("get-node-to-labels").queryParam("user.name", userName)
-            .accept(MediaType.APPLICATION_JSON).get(ClientResponse.class);
-    assertEquals(MediaType.APPLICATION_JSON_TYPE + "; " + JettyUtils.UTF_8,
-        response.getType().toString());
-    NodeToLabelsInfo ntlinfo = response.getEntity(NodeToLabelsInfo.class);
-    NodeLabelsInfo nlinfo = ntlinfo.getNodeToLabels().get("nid:0");
-    assertEquals(1, nlinfo.getNodeLabels().size());
-    assertTrue(nlinfo.getNodeLabelsInfo().contains(new NodeLabelInfo("a")));
-    
+    response = getNodeToLabels(r);
+    assertApplicationJsonUtf8Response(response);
+    NodeToLabelsInfo nodeToLabelsInfo = response.getEntity(NodeToLabelsInfo.class);
+    NodeLabelsInfo nodeLabelsInfo = nodeToLabelsInfo.getNodeToLabels().get(NODE_0);
+    assertNodeLabelsSize(nodeLabelsInfo, 1);
+    assertNodeLabelsInfoContains(nodeLabelsInfo, Pair.of(LABEL_A, DEFAULT_NL_EXCLUSIVITY));
+
     // Remove all
-    params = new MultivaluedMapImpl();
-    params.add("labels", "");
-    response =
-        r.path("ws").path("v1").path("cluster")
-            .path("nodes").path("nid:0")
-            .path("replace-labels")
-            .queryParam("user.name", userName)
-            .queryParams(params)
-            .accept(MediaType.APPLICATION_JSON)
-            .post(ClientResponse.class);
-    LOG.info("posted node nodelabel");
+    response = replaceLabelsOnNode(r, NODE_0, "");
+    assertHttp200(response);
     // Verify
-    response =
-        r.path("ws").path("v1").path("cluster")
-            .path("nodes").path("nid:0")
-            .path("get-labels").queryParam("user.name", userName)
-            .accept(MediaType.APPLICATION_JSON).get(ClientResponse.class);
-    assertEquals(MediaType.APPLICATION_JSON_TYPE + "; " + JettyUtils.UTF_8,
-        response.getType().toString());
-    nlsifo = response.getEntity(NodeLabelsInfo.class);
-    assertTrue(nlsifo.getNodeLabelsInfo().isEmpty());
-    
+    response = getLabelsOfNode(r, NODE_0);
+    assertApplicationJsonUtf8Response(response);
+    assertNodeLabelsSize(response.getEntity(NodeLabelsInfo.class), 0);
+
     // Add a label back for auth tests
-    params = new MultivaluedMapImpl();
-    params.add("labels", "a");
-    response =
-        r.path("ws").path("v1").path("cluster")
-            .path("nodes").path("nid:0")
-            .path("replace-labels")
-            .queryParam("user.name", userName)
-            .queryParams(params)
-            .accept(MediaType.APPLICATION_JSON)
-            .post(ClientResponse.class);
-    LOG.info("posted node nodelabel");
+    response = replaceLabelsOnNode(r, NODE_0, LABEL_A);
+    assertHttp200(response);
 
     // Verify
-    response =
-        r.path("ws").path("v1").path("cluster")
-            .path("nodes").path("nid:0")
-            .path("get-labels").queryParam("user.name", userName)
-            .accept(MediaType.APPLICATION_JSON).get(ClientResponse.class);
-    assertEquals(MediaType.APPLICATION_JSON_TYPE + "; " + JettyUtils.UTF_8,
-        response.getType().toString());
-    nlsifo = response.getEntity(NodeLabelsInfo.class);
-    assertTrue(nlsifo.getNodeLabelsInfo().contains(new NodeLabelInfo("a")));
-    
+    response = getLabelsOfNode(r, NODE_0);
+    assertApplicationJsonUtf8Response(response);
+    assertNodeLabelsInfoContains(response.getEntity(NodeLabelsInfo.class),
+        Pair.of(LABEL_A, DEFAULT_NL_EXCLUSIVITY));
+
     // Auth fail replace labels on node
-    params = new MultivaluedMapImpl();
-    params.add("labels", "b");
-    response =
-        r.path("ws").path("v1").path("cluster")
-            .path("nodes").path("nid:0")
-            .path("replace-labels")
-            .queryParam("user.name", notUserName)
-            .queryParams(params)
-            .accept(MediaType.APPLICATION_JSON)
-            .post(ClientResponse.class);
+    response = replaceLabelsOnNodeWithUserName(r, NODE_0, notUserName, LABEL_B);
+    assertHttp401(response);
     // Verify
-    response =
-        r.path("ws").path("v1").path("cluster")
-            .path("nodes").path("nid:0")
-            .path("get-labels").queryParam("user.name", userName)
-            .accept(MediaType.APPLICATION_JSON).get(ClientResponse.class);
-    assertEquals(MediaType.APPLICATION_JSON_TYPE + "; " + JettyUtils.UTF_8,
-        response.getType().toString());
-    nlsifo = response.getEntity(NodeLabelsInfo.class);
-    assertTrue(nlsifo.getNodeLabelsInfo().contains(new NodeLabelInfo("a")));
-    
-    // Fail to add a label with post
-    response =
-        r.path("ws").path("v1").path("cluster")
-            .path("add-node-labels").queryParam("user.name", notUserName)
-            .accept(MediaType.APPLICATION_JSON)
-            .entity("{\"nodeLabels\":\"c\"}", MediaType.APPLICATION_JSON)
-            .post(ClientResponse.class);
+    response = getLabelsOfNode(r, NODE_0);
+    assertApplicationJsonUtf8Response(response);
+    assertNodeLabelsInfoContains(response.getEntity(NodeLabelsInfo.class),
+        Pair.of(LABEL_A, DEFAULT_NL_EXCLUSIVITY));
+
+    // Fail to add a label with wrong user
+    response = addNodeLabelsWithUser(r, Lists.newArrayList(Pair.of("c", DEFAULT_NL_EXCLUSIVITY)),
+        notUserName);
+    assertHttp401(response);
 
     // Verify
-    response =
-        r.path("ws").path("v1").path("cluster")
-            .path("get-node-labels").queryParam("user.name", userName)
-            .accept(MediaType.APPLICATION_JSON).get(ClientResponse.class);
-    assertEquals(MediaType.APPLICATION_JSON_TYPE + "; " + JettyUtils.UTF_8,
-        response.getType().toString());
-    nlsifo = response.getEntity(NodeLabelsInfo.class);
-    assertEquals(2, nlsifo.getNodeLabels().size());
-    
+    response = getNodeLabels(r);
+    assertApplicationJsonUtf8Response(response);
+    assertNodeLabelsSize(response.getEntity(NodeLabelsInfo.class), 2);
+
     // Remove cluster label (succeed, we no longer need it)
-    params = new MultivaluedMapImpl();
-    params.add("labels", "b");
-    response =
-        r.path("ws").path("v1").path("cluster")
-            .path("remove-node-labels")
-            .queryParam("user.name", userName)
-            .queryParams(params)
-            .accept(MediaType.APPLICATION_JSON)
-            .post(ClientResponse.class);
+    response = removeNodeLabel(r, LABEL_B);
+    assertHttp200(response);
     // Verify
-    response =
-        r.path("ws").path("v1").path("cluster")
-            .path("get-node-labels").queryParam("user.name", userName)
-            .accept(MediaType.APPLICATION_JSON).get(ClientResponse.class);
-    assertEquals(MediaType.APPLICATION_JSON_TYPE + "; " + JettyUtils.UTF_8,
-        response.getType().toString());
-    nlsifo = response.getEntity(NodeLabelsInfo.class);
-    assertEquals(1, nlsifo.getNodeLabels().size());
-    for (NodeLabelInfo nl : nlsifo.getNodeLabelsInfo()) {
-      assertEquals("a", nl.getName());
-      assertTrue(nl.getExclusivity());
-    }
-    
+    response = getNodeLabels(r);
+    assertApplicationJsonUtf8Response(response);
+    assertNodeLabelsInfo(response.getEntity(NodeLabelsInfo.class),
+        Lists.newArrayList(Pair.of(LABEL_A, true)));
+
     // Remove cluster label with post
-    params = new MultivaluedMapImpl();
-    params.add("labels", "a");
-    response =
-        r.path("ws").path("v1").path("cluster")
-            .path("remove-node-labels")
-            .queryParam("user.name", userName)
-            .queryParams(params)
-            .accept(MediaType.APPLICATION_JSON)
-            .post(ClientResponse.class);
+    response = removeNodeLabel(r, LABEL_A);
+    assertHttp200(response);
     // Verify
-    response =
-        r.path("ws").path("v1").path("cluster")
-            .path("get-node-labels").queryParam("user.name", userName)
-            .accept(MediaType.APPLICATION_JSON).get(ClientResponse.class);
-    assertEquals(MediaType.APPLICATION_JSON_TYPE + "; " + JettyUtils.UTF_8,
-        response.getType().toString());
-    nlsifo = response.getEntity(NodeLabelsInfo.class);
-    assertEquals(0, nlsifo.getNodeLabels().size());
+    response = getNodeLabels(r);
+    assertApplicationJsonUtf8Response(response);
+    nodeLabelsInfo = response.getEntity(NodeLabelsInfo.class);
+    assertEquals(0, nodeLabelsInfo.getNodeLabels().size());
 
     // Following test cases are to test replace when distributed node label
     // configuration is on
     // Reset for testing : add cluster labels
-    nlsifo = new NodeLabelsInfo();
-    nlsifo.getNodeLabelsInfo().add(new NodeLabelInfo("x", false));
-    nlsifo.getNodeLabelsInfo().add(new NodeLabelInfo("y", false));
-    response =
-        r.path("ws")
-            .path("v1")
-            .path("cluster")
-            .path("add-node-labels")
-            .queryParam("user.name", userName)
-            .accept(MediaType.APPLICATION_JSON)
-            .entity(toJson(nlsifo, NodeLabelsInfo.class),
-                MediaType.APPLICATION_JSON).post(ClientResponse.class);
+    response = addNodeLabels(r, Lists.newArrayList(
+        Pair.of(LABEL_X, false), Pair.of(LABEL_Y, false)));
+    assertHttp200(response);
     // Reset for testing : Add labels to a node
-    params = new MultivaluedMapImpl();
-    params.add("labels", "y");
-    response =
-        r.path("ws").path("v1").path("cluster").path("nodes").path("nid:0")
-            .path("replace-labels").queryParam("user.name", userName)
-            .queryParams(params)
-            .accept(MediaType.APPLICATION_JSON)
-            .post(ClientResponse.class);
-    LOG.info("posted node nodelabel");
-
-    //setting rmWebService for non Centralized NodeLabel Configuration
+    response = replaceLabelsOnNode(r, NODE_0, LABEL_Y);
+    assertHttp200(response);
+
+    //setting rmWebService for non-centralized NodeLabel Configuration
     rmWebService.isCentralizedNodeLabelConfiguration = false;
 
     // Case1 : Replace labels using node-to-labels
-    ntli = new NodeToLabelsEntryList();
-    labels = new ArrayList<String>();
-    labels.add("x");
-    nli = new NodeToLabelsEntry("nid:0", labels);
-    ntli.getNodeToLabels().add(nli);
-    response =
-        r.path("ws")
-            .path("v1")
-            .path("cluster")
-            .path("replace-node-to-labels")
-            .queryParam("user.name", userName)
-            .accept(MediaType.APPLICATION_JSON)
-            .entity(toJson(ntli, NodeToLabelsEntryList.class),
-                MediaType.APPLICATION_JSON).post(ClientResponse.class);
+    response = replaceNodeToLabels(r, Lists.newArrayList(Pair.of(NODE_0,
+        Lists.newArrayList(LABEL_X))));
+    assertHttp404(response);
 
     // Verify, using node-to-labels that previous operation has failed
-    response =
-        r.path("ws").path("v1").path("cluster").path("get-node-to-labels")
-            .queryParam("user.name", userName)
-            .accept(MediaType.APPLICATION_JSON).get(ClientResponse.class);
-    assertEquals(MediaType.APPLICATION_JSON_TYPE + "; " + JettyUtils.UTF_8,
-        response.getType().toString());
-    ntlinfo = response.getEntity(NodeToLabelsInfo.class);
-    nlinfo = ntlinfo.getNodeToLabels().get("nid:0");
-    assertEquals(1, nlinfo.getNodeLabels().size());
-    assertFalse(nlinfo.getNodeLabelsInfo().contains(
-        new NodeLabelInfo("x", false)));
+    response = getNodeToLabels(r);
+    assertApplicationJsonUtf8Response(response);
+    nodeToLabelsInfo = response.getEntity(NodeToLabelsInfo.class);
+    nodeLabelsInfo = nodeToLabelsInfo.getNodeToLabels().get(NODE_0);
+    assertNodeLabelsSize(nodeLabelsInfo, 1);
+    assertNodeLabelsInfoDoesNotContain(nodeLabelsInfo, Pair.of(LABEL_X, false));
 
     // Case2 : failure to Replace labels using replace-labels
-    response =
-        r.path("ws").path("v1").path("cluster").path("nodes").path("nid:0")
-            .path("replace-labels").queryParam("user.name", userName)
-            .accept(MediaType.APPLICATION_JSON)
-            .entity("{\"nodeLabelName\": [\"x\"]}", MediaType.APPLICATION_JSON)
-            .post(ClientResponse.class);
-    LOG.info("posted node nodelabel");
+    response = replaceLabelsOnNode(r, NODE_0, LABEL_X);
+    assertHttp404(response);
 
     // Verify, using node-to-labels that previous operation has failed
-    response =
-        r.path("ws").path("v1").path("cluster").path("get-node-to-labels")
-            .queryParam("user.name", userName)
-            .accept(MediaType.APPLICATION_JSON).get(ClientResponse.class);
-    assertEquals(MediaType.APPLICATION_JSON_TYPE + "; " + JettyUtils.UTF_8,
-        response.getType().toString());
-    ntlinfo = response.getEntity(NodeToLabelsInfo.class);
-    nlinfo = ntlinfo.getNodeToLabels().get("nid:0");
-    assertEquals(1, nlinfo.getNodeLabels().size());
-    assertFalse(nlinfo.getNodeLabelsInfo().contains(
-        new NodeLabelInfo("x", false)));
+    response = getNodeToLabels(r);
+    assertApplicationJsonUtf8Response(response);
+    nodeToLabelsInfo = response.getEntity(NodeToLabelsInfo.class);
+    nodeLabelsInfo = nodeToLabelsInfo.getNodeToLabels().get(NODE_0);
+    assertNodeLabelsSize(nodeLabelsInfo, 1);
+    assertNodeLabelsInfoDoesNotContain(nodeLabelsInfo, Pair.of(LABEL_X, false));
 
     //  Case3 : Remove cluster label should be successful
-    params = new MultivaluedMapImpl();
-    params.add("labels", "x");
-    response =
-        r.path("ws").path("v1").path("cluster")
-            .path("remove-node-labels")
-            .queryParam("user.name", userName)
-            .queryParams(params)
-            .accept(MediaType.APPLICATION_JSON)
-            .post(ClientResponse.class);
+    response = removeNodeLabel(r, LABEL_X);
+    assertHttp200(response);
     // Verify
-    response =
-        r.path("ws").path("v1").path("cluster")
-            .path("get-node-labels").queryParam("user.name", userName)
-            .accept(MediaType.APPLICATION_JSON).get(ClientResponse.class);
-    assertEquals(MediaType.APPLICATION_JSON_TYPE + "; " + JettyUtils.UTF_8,
-        response.getType().toString());
-    nlsifo = response.getEntity(NodeLabelsInfo.class);
-    assertEquals(new NodeLabelInfo("y", false),
-        nlsifo.getNodeLabelsInfo().get(0));
-    assertEquals("y", nlsifo.getNodeLabelsInfo().get(0).getName());
-    assertFalse(nlsifo.getNodeLabelsInfo().get(0).getExclusivity());
+    response = getNodeLabels(r);
+    assertApplicationJsonUtf8Response(response);
+    assertNodeLabelsInfoAtPosition(response.getEntity(NodeLabelsInfo.class), Pair.of(LABEL_Y,
+        false), 0);
 
     // Remove y
-    params = new MultivaluedMapImpl();
-    params.add("labels", "y");
-    response =
-        r.path("ws").path("v1").path("cluster")
-            .path("remove-node-labels")
-            .queryParam("user.name", userName)
-            .queryParams(params)
-            .accept(MediaType.APPLICATION_JSON)
-            .post(ClientResponse.class);
+    response = removeNodeLabel(r, LABEL_Y);
+    assertHttp200(response);
 
     // Verify
-    response =
-        r.path("ws").path("v1").path("cluster")
-            .path("get-node-labels").queryParam("user.name", userName)
-            .accept(MediaType.APPLICATION_JSON).get(ClientResponse.class);
-    assertEquals(MediaType.APPLICATION_JSON_TYPE + "; " + JettyUtils.UTF_8,
-        response.getType().toString());
-    nlsifo = response.getEntity(NodeLabelsInfo.class);
-    assertTrue(nlsifo.getNodeLabelsInfo().isEmpty());
-
-    // add a new nodelabel with exclusity
-    nlsifo = new NodeLabelsInfo();
-    nlsifo.getNodeLabelsInfo().add(new NodeLabelInfo("z", false));
-    response =
-        r.path("ws")
-            .path("v1")
-            .path("cluster")
-            .path("add-node-labels")
-            .queryParam("user.name", userName)
-            .accept(MediaType.APPLICATION_JSON)
-            .entity(toJson(nlsifo, NodeLabelsInfo.class),
-                MediaType.APPLICATION_JSON).post(ClientResponse.class);
+    response = getNodeLabels(r);
+    assertApplicationJsonUtf8Response(response);
+    assertNodeLabelsSize(response.getEntity(NodeLabelsInfo.class), 0);
 
+    // add a new nodelabel with exclusivity=false
+    response = addNodeLabels(r, Lists.newArrayList(Pair.of(LABEL_Z, false)));
+    assertHttp200(response);
     // Verify
+    response = getNodeLabels(r);
+    assertApplicationJsonUtf8Response(response);
+    assertNodeLabelsInfoAtPosition(response.getEntity(NodeLabelsInfo.class),
+        Pair.of(LABEL_Z, false), 0);
+    assertNodeLabelsSize(nodeLabelsInfo, 1);
+  }
+
+  private void assertLabelsToNodesInfo(LabelsToNodesInfo labelsToNodesInfo, int size,
+      List<Pair<Pair<String, Boolean>, List<String>>> nodeLabelsToNodesList) {
+    Map<NodeLabelInfo, NodeIDsInfo> labelsToNodes = labelsToNodesInfo.getLabelsToNodes();
+    assertNotNull("Labels to nodes mapping should not be null.", labelsToNodes);
+    assertEquals("Size of label to nodes mapping is not the expected.", size, labelsToNodes.size());
+
+    for (Pair<Pair<String, Boolean>, List<String>> nodeLabelToNodes : nodeLabelsToNodesList) {
+      Pair<String, Boolean> expectedNLData = nodeLabelToNodes.getLeft();
+      List<String> expectedNodes = nodeLabelToNodes.getRight();
+      NodeLabelInfo expectedNLInfo = new NodeLabelInfo(expectedNLData.getLeft(),
+          expectedNLData.getRight());
+      NodeIDsInfo actualNodes = labelsToNodes.get(expectedNLInfo);
+      assertNotNull(String.format("Node info not found. Expected NodeLabel data: %s",
+          expectedNLData), actualNodes);
+      for (String expectedNode : expectedNodes) {
+        assertTrue(String.format("Can't find node ID in actual Node IDs list: %s",
+                actualNodes.getNodeIDs()), actualNodes.getNodeIDs().contains(expectedNode));
+      }
+    }
+  }
+
+  private void assertNodeLabelsInfo(NodeLabelsInfo nodeLabelsInfo,
+      List<Pair<String, Boolean>> nlInfos) {
+    assertEquals(nlInfos.size(), nodeLabelsInfo.getNodeLabels().size());
+
+    for (int i = 0; i < nodeLabelsInfo.getNodeLabelsInfo().size(); i++) {
+      Pair<String, Boolean> expected = nlInfos.get(i);
+      NodeLabelInfo actual = nodeLabelsInfo.getNodeLabelsInfo().get(i);
+      LOG.debug("Checking NodeLabelInfo: {}", actual);
+      assertEquals(expected.getLeft(), actual.getName());
+      assertEquals(expected.getRight(), actual.getExclusivity());
+    }
+  }
+
+  private void assertNodeLabelsInfoAtPosition(NodeLabelsInfo nodeLabelsInfo, Pair<String,
+      Boolean> nlInfo, int pos) {
+    NodeLabelInfo actual = nodeLabelsInfo.getNodeLabelsInfo().get(pos);
+    LOG.debug("Checking NodeLabelInfo: {}", actual);
+    assertEquals(nlInfo.getLeft(), actual.getName());
+    assertEquals(nlInfo.getRight(), actual.getExclusivity());
+  }
+
+  private void assertNodeLabelsInfoContains(NodeLabelsInfo nodeLabelsInfo,
+      Pair<String, Boolean> nlInfo) {
+    NodeLabelInfo nodeLabelInfo = new NodeLabelInfo(nlInfo.getLeft(), nlInfo.getRight());
+    assertTrue(String.format("Cannot find nodeLabelInfo '%s' among items of node label info list:" +
+            " %s", nodeLabelInfo, nodeLabelsInfo.getNodeLabelsInfo()),
+        nodeLabelsInfo.getNodeLabelsInfo().contains(nodeLabelInfo));
+  }
+
+  private void assertNodeLabelsInfoDoesNotContain(NodeLabelsInfo nodeLabelsInfo, Pair<String,
+      Boolean> nlInfo) {
+    NodeLabelInfo nodeLabelInfo = new NodeLabelInfo(nlInfo.getLeft(), nlInfo.getRight());
+    assertFalse(String.format("Should have not found nodeLabelInfo '%s' among " +
+        "items of node label info list: %s", nodeLabelInfo, nodeLabelsInfo.getNodeLabelsInfo()),
+        nodeLabelsInfo.getNodeLabelsInfo().contains(nodeLabelInfo));
+  }
+
+  private void assertNodeLabelsSize(NodeLabelsInfo nodeLabelsInfo, int expectedSize) {
+    assertEquals(expectedSize, nodeLabelsInfo.getNodeLabelsInfo().size());
+  }
+
+  private ClientResponse replaceNodeToLabels(WebResource r,
+      List<Pair<String, List<String>>> nodeToLabelInfos) throws Exception {
+    NodeToLabelsEntryList nodeToLabelsEntries = new NodeToLabelsEntryList();
+
+    for (Pair<String, List<String>> nodeToLabelInfo : nodeToLabelInfos) {
+      ArrayList<String> labelList = new ArrayList<>(nodeToLabelInfo.getRight());
+      String nodeId = nodeToLabelInfo.getLeft();
+      NodeToLabelsEntry nli = new NodeToLabelsEntry(nodeId, labelList);
+      nodeToLabelsEntries.getNodeToLabels().add(nli);
+    }
+    return r.path("ws").path("v1").path("cluster")
+        .path("replace-node-to-labels")
+        .queryParam("user.name", userName)
+        .accept(MediaType.APPLICATION_JSON)
+        .entity(toJson(nodeToLabelsEntries, NodeToLabelsEntryList.class),
+            MediaType.APPLICATION_JSON)
+        .post(ClientResponse.class);
+  }
+
+  private ClientResponse getNodeLabelMappings(WebResource r) {
+    ClientResponse response;
     response =
         r.path("ws").path("v1").path("cluster")
-            .path("get-node-labels").queryParam("user.name", userName)
+            .path("label-mappings").queryParam("user.name", userName)
             .accept(MediaType.APPLICATION_JSON).get(ClientResponse.class);
+    return response;
+  }
+
+  private ClientResponse getNodeLabelMappingsByLabels(WebResource r, String... labelNames) {
+    MultivaluedMapImpl params = createMultiValuedMap(labelNames);
+    return r.path("ws").path("v1").path("cluster")
+        .path("label-mappings").queryParam("user.name", userName)
+        .queryParams(params)
+        .accept(MediaType.APPLICATION_JSON).get(ClientResponse.class);
+  }
+
+  private ClientResponse replaceLabelsOnNode(WebResource r, String node, String... labelNames) {
+    return replaceLabelsOnNodeWithUserName(r, node, userName, labelNames);
+  }
+
+  private ClientResponse replaceLabelsOnNodeWithUserName(WebResource r, String node,
+      String userName, String... labelNames) {

Review comment:
       As "replaceLabelsOnNode" calls "replaceLabelsOnNodeWithUserName" with various parameters, I don't think it's worth to declare all parameters as static.
   For the checkstyle issue: I think it's not a useful check what checkstyle does in this case and I don't think I can come up with another name that is different but still descriptive. Are you okay with ignoring this checkstyle issue?
   I'm usually ignoring this kind of "issue" from checkstyle when committing. 
   Thanks.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org