You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by ji...@apache.org on 2018/01/19 15:54:46 UTC

[geode] branch develop updated: GEODE-4129: do not list coordinator as a different entry in list memb… (#1308)

This is an automated email from the ASF dual-hosted git repository.

jinmeiliao pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git


The following commit(s) were added to refs/heads/develop by this push:
     new 75f2401  GEODE-4129: do not list coordinator as a different entry in list memb… (#1308)
75f2401 is described below

commit 75f2401527b919aa08b36dc4db7f892b8a06be95
Author: jinmeiliao <ji...@pivotal.io>
AuthorDate: Fri Jan 19 07:54:43 2018 -0800

    GEODE-4129: do not list coordinator as a different entry in list memb… (#1308)
    
    * GEODE-4129: do not list coordinator as a different entry in list member command
---
 .../internal/cli/commands/GfshCommand.java         |   2 +-
 .../internal/cli/commands/ListMembersCommand.java  |  69 ++++++--------
 .../cli/commands/ListMembersCommandDUnitTest.java  |  15 +--
 .../cli/commands/ListMembersCommandTest.java       | 101 +++++++++++++++++++++
 .../internal/security/MultiGfshDUnitTest.java      |   3 +-
 .../test/junit/assertions/CommandResultAssert.java |   4 +-
 6 files changed, 134 insertions(+), 60 deletions(-)

diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/GfshCommand.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/GfshCommand.java
index eadc554..da7f39e 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/GfshCommand.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/GfshCommand.java
@@ -138,7 +138,7 @@ public interface GfshCommand extends CommandMarker {
   }
 
   /**
-   * if no members matches these names, an empty set would return
+   * if no members matches these names, an empty set would return, this does not include locators
    */
   default Set<DistributedMember> findMembers(String[] groups, String[] members) {
     return CliUtil.findMembers(groups, members);
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ListMembersCommand.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ListMembersCommand.java
index 6b25800..c676784 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ListMembersCommand.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ListMembersCommand.java
@@ -24,12 +24,9 @@ import org.springframework.shell.core.annotation.CliOption;
 import org.apache.geode.distributed.DistributedMember;
 import org.apache.geode.distributed.internal.InternalDistributedSystem;
 import org.apache.geode.distributed.internal.membership.MembershipManager;
-import org.apache.geode.internal.cache.InternalCache;
 import org.apache.geode.management.cli.CliMetaData;
 import org.apache.geode.management.cli.ConverterHint;
 import org.apache.geode.management.cli.Result;
-import org.apache.geode.management.internal.cli.CliUtil;
-import org.apache.geode.management.internal.cli.LogWrapper;
 import org.apache.geode.management.internal.cli.i18n.CliStrings;
 import org.apache.geode.management.internal.cli.result.ResultBuilder;
 import org.apache.geode.management.internal.cli.result.TabularResultData;
@@ -41,57 +38,43 @@ public class ListMembersCommand implements GfshCommand {
   @CliMetaData(relatedTopic = CliStrings.TOPIC_GEODE_SERVER)
   @ResourceOperation(resource = ResourcePermission.Resource.CLUSTER,
       operation = ResourcePermission.Operation.READ)
-  public Result listMember(@CliOption(key = {CliStrings.GROUP}, unspecifiedDefaultValue = "",
+  public Result listMember(@CliOption(key = {CliStrings.GROUP, CliStrings.GROUPS},
       optionContext = ConverterHint.MEMBERGROUP,
-      help = CliStrings.LIST_MEMBER__GROUP__HELP) String group) {
-    Result result;
+      help = CliStrings.ALTER_REGION__GROUP__HELP) String[] groups) {
 
-    // TODO: Add the code for identifying the system services
-    try {
-      Set<DistributedMember> memberSet = new TreeSet<>();
-      InternalCache cache = getCache();
+    Set<DistributedMember> memberSet = new TreeSet<>();
+    memberSet.addAll(this.findMembersIncludingLocators(groups, null));
 
-      // default get all the members in the DS
-      if (group.isEmpty()) {
-        memberSet.addAll(CliUtil.getAllMembers(cache));
-      } else {
-        memberSet.addAll(cache.getDistributedSystem().getGroupMembers(group));
-      }
+    if (memberSet.isEmpty()) {
+      return ResultBuilder.createInfoResult(CliStrings.LIST_MEMBER__MSG__NO_MEMBER_FOUND);
+    }
 
-      if (memberSet.isEmpty()) {
-        result = ResultBuilder.createInfoResult(CliStrings.LIST_MEMBER__MSG__NO_MEMBER_FOUND);
+    TabularResultData resultData = ResultBuilder.createTabularResultData();
+    final DistributedMember coordinatorMember = getCoordinator();
+    for (DistributedMember member : memberSet) {
+      resultData.accumulate("Name", member.getName());
+      if (member == coordinatorMember) {
+        resultData.accumulate("Id", member.getId() + " [Coordinator]");
       } else {
-
-        TabularResultData resultData = ResultBuilder.createTabularResultData();
-        final String coordinatorMember = getCoordinator();
-        resultData.accumulate("Name", "Coordinator:");
-        resultData.accumulate("Id", coordinatorMember);
-        for (DistributedMember member : memberSet) {
-          resultData.accumulate("Name", member.getName());
-          resultData.accumulate("Id", member.getId());
-        }
-
-        result = ResultBuilder.buildResult(resultData);
+        resultData.accumulate("Id", member.getId());
       }
-    } catch (Exception e) {
-      result = ResultBuilder
-          .createGemFireErrorResult("Could not fetch the list of members. " + e.getMessage());
-      LogWrapper.getInstance().warning(e.getMessage(), e);
     }
-    return result;
+
+    return ResultBuilder.buildResult(resultData);
   }
 
-  private String getCoordinator() {
-    String result = "unknown";
+  DistributedMember getCoordinator() {
     InternalDistributedSystem ids = InternalDistributedSystem.getConnectedInstance();
-    if ((ids != null) && (ids.isConnected())) {
-      MembershipManager mmgr = ids.getDistributionManager().getMembershipManager();
-      DistributedMember coord = mmgr.getCoordinator();
-      if (coord != null) {
-        result = coord.toString();
-      }
+    if (ids == null || !ids.isConnected()) {
+      return null;
     }
 
-    return result;
+    MembershipManager mmgr = ids.getDistributionManager().getMembershipManager();
+    if (mmgr == null) {
+      return null;
+    }
+
+    return mmgr.getCoordinator();
+
   }
 }
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ListMembersCommandDUnitTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ListMembersCommandDUnitTest.java
index e35ae43..034927c 100644
--- a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ListMembersCommandDUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ListMembersCommandDUnitTest.java
@@ -60,14 +60,10 @@ public class ListMembersCommandDUnitTest {
 
   @Test
   public void listAllMembers() throws Exception {
-    gfsh.executeAndAssertThat(LIST_MEMBER).statusIsSuccess();
-    String output = gfsh.getGfshOutput();
-
-    assertThat(output).contains("locator-0");
-    assertThat(output).contains("server-1");
-    assertThat(output).contains("server-2");
-    assertThat(output).contains("server-3");
-    assertThat(output).contains("Coordinator");
+    gfsh.executeAndAssertThat(LIST_MEMBER).statusIsSuccess().tableHasRowCount("Name", 4)
+        .tableHasColumnWithExactValuesInAnyOrder("Name", "locator-0", "server-1", "server-2",
+            "server-3")
+        .containsOutput("[Coordinator]");
   }
 
   @Test
@@ -85,8 +81,6 @@ public class ListMembersCommandDUnitTest {
   public void listMembersInServerGroupOne() throws Exception {
     gfsh.executeAndAssertThat(LIST_MEMBER + " --group=serverGroup1").statusIsSuccess();
     String output = gfsh.getGfshOutput();
-
-    assertThat(output).contains("Coordinator:");
     assertThat(output).contains("server-1");
     assertThat(output).contains("server-2");
     assertThat(output).doesNotContain("server-3");
@@ -97,7 +91,6 @@ public class ListMembersCommandDUnitTest {
     gfsh.executeAndAssertThat(LIST_MEMBER + " --group=serverGroup2").statusIsSuccess();
     String output = gfsh.getGfshOutput();
 
-    assertThat(output).contains("Coordinator:");
     assertThat(output).doesNotContain("server-1");
     assertThat(output).doesNotContain("server-2");
     assertThat(output).contains("server-3");
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ListMembersCommandTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ListMembersCommandTest.java
new file mode 100644
index 0000000..6ea66fa
--- /dev/null
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ListMembersCommandTest.java
@@ -0,0 +1,101 @@
+/*
+ * 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.
+ */
+
+package org.apache.geode.management.internal.cli.commands;
+
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.when;
+
+import java.util.HashSet;
+import java.util.Set;
+
+import org.junit.Before;
+import org.junit.ClassRule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.distributed.DistributedMember;
+import org.apache.geode.test.junit.categories.UnitTest;
+import org.apache.geode.test.junit.rules.GfshParserRule;
+
+
+@Category(UnitTest.class)
+public class ListMembersCommandTest {
+
+  @ClassRule
+  public static GfshParserRule gfsh = new GfshParserRule();
+
+  private ListMembersCommand command;
+  private Set<DistributedMember> members;
+  private DistributedMember member1;
+  private DistributedMember member2;
+
+  @Before
+  public void before() {
+    command = spy(ListMembersCommand.class);
+    members = new HashSet<>();
+    doReturn(members).when(command).findMembersIncludingLocators(any(), any());
+
+    member1 = mock(DistributedMember.class);
+    when(member1.getName()).thenReturn("name");
+    when(member1.getId()).thenReturn("id");
+    doReturn(member1).when(command).getCoordinator();
+
+    member2 = mock(DistributedMember.class);
+    when(member2.getName()).thenReturn("name2");
+    when(member2.getId()).thenReturn("id2");
+    // This will enforce the sort order in TreeSet used by ListMembersCommand.
+    when(member1.compareTo(member2)).thenReturn(-1);
+    when(member2.compareTo(member1)).thenReturn(1);
+  }
+
+  @Test
+  public void listMembersNoMemberFound() {
+    gfsh.executeAndAssertThat(command, "list members").containsOutput("No Members Found")
+        .statusIsSuccess();
+  }
+
+  @Test
+  public void basicListMembers() {
+    members.add(member1);
+
+    gfsh.executeAndAssertThat(command, "list members").tableHasRowCount("Name", 1)
+        .tableHasRowWithValues("Name", "Id", "name", "id [Coordinator]").statusIsSuccess();
+  }
+
+  @Test
+  public void noCoordinator() {
+    members.add(member1);
+    doReturn(null).when(command).getCoordinator();
+
+    gfsh.executeAndAssertThat(command, "list members").tableHasRowCount("Name", 1)
+        .tableHasRowWithValues("Name", "Id", "name", "id").statusIsSuccess();
+  }
+
+  @Test
+  public void listMembersMultipleItems() {
+    members.add(member1);
+    members.add(member2);
+
+    gfsh.executeAndAssertThat(command, "list members").tableHasRowCount("Name", 2)
+        .tableHasRowWithValues("Name", "Id", "name", "id [Coordinator]")
+        .tableHasRowWithValues("Name", "Id", "name2", "id2")
+        .tableHasColumnWithExactValuesInExactOrder("Name", member1.getName(), member2.getName())
+        .statusIsSuccess();
+  }
+}
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/security/MultiGfshDUnitTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/security/MultiGfshDUnitTest.java
index 9baaabf..47a6cc2 100644
--- a/geode-core/src/test/java/org/apache/geode/management/internal/security/MultiGfshDUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/security/MultiGfshDUnitTest.java
@@ -18,7 +18,6 @@ import static org.apache.geode.distributed.ConfigurationProperties.SECURITY_MANA
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotEquals;
 
-import java.io.IOException;
 import java.util.List;
 import java.util.Properties;
 import java.util.concurrent.TimeUnit;
@@ -64,7 +63,7 @@ public class MultiGfshDUnitTest {
 
   @Category(FlakyTest.class) // GEODE-1579
   @Test
-  public void testMultiUser() throws IOException, JSONException, InterruptedException {
+  public void testMultiUser() throws JSONException, InterruptedException {
 
     IgnoredException.addIgnoredException("java.util.zip.ZipException: zip file is empty");
     IgnoredException
diff --git a/geode-core/src/test/java/org/apache/geode/test/junit/assertions/CommandResultAssert.java b/geode-core/src/test/java/org/apache/geode/test/junit/assertions/CommandResultAssert.java
index 4336edf..d46f92c 100644
--- a/geode-core/src/test/java/org/apache/geode/test/junit/assertions/CommandResultAssert.java
+++ b/geode-core/src/test/java/org/apache/geode/test/junit/assertions/CommandResultAssert.java
@@ -26,7 +26,6 @@ import org.assertj.core.api.Assertions;
 import org.json.JSONArray;
 
 import org.apache.geode.management.cli.Result;
-import org.apache.geode.management.internal.cli.json.GfJsonException;
 import org.apache.geode.management.internal.cli.result.CommandResult;
 
 
@@ -184,8 +183,7 @@ public class CommandResultAssert
 
 
 
-  public CommandResultAssert tableHasRowWithValues(String... headersThenValues)
-      throws GfJsonException {
+  public CommandResultAssert tableHasRowWithValues(String... headersThenValues) {
     assertThat(headersThenValues.length % 2)
         .describedAs("You need to pass even number of parameters.").isEqualTo(0);
 

-- 
To stop receiving notification emails like this one, please contact
['"commits@geode.apache.org" <co...@geode.apache.org>'].