[
https://issues.apache.org/jira/browse/GEODE-4129?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16332442#comment-16332442
]
ASF GitHub Bot commented on GEODE-4129:
---------------------------------------
jinmeiliao closed pull request #1308: GEODE-4129: do not list coordinator as a
different entry in list memb…
URL: https://github.com/apache/geode/pull/1308
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
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 eadc55446b..da7f39e762 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 @@ default Execution getMembersFunctionExecutor(final
Set<DistributedMember> member
}
/**
- * 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 6b25800082..c6767844be 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.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 @@
@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 e35ae43cf4..034927cc7d 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 void listMembersWithoutConnection() throws Exception
{
@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 void listMembersInLocatorGroup() throws Exception {
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 void listMembersInServerGroupTwo() throws Exception {
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 0000000000..6ea66fa7e5
--- /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 9baaabfecb..47a6cc2b45 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.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 void setup() throws Exception {
@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 fc10639f1f..29439c5f24 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.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 CommandResultAssert
tableHasColumnWithExactValuesInAnyOrder(String header
- 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);
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
> gfsh list members command feels inconsistent
> --------------------------------------------
>
> Key: GEODE-4129
> URL: https://issues.apache.org/jira/browse/GEODE-4129
> Project: Geode
> Issue Type: Bug
> Components: docs, gfsh
> Reporter: Jens Deppe
> Assignee: Jinmei Liao
> Priority: Major
> Labels: pull-request-available
>
> {{list members}} now also shows the 'coordinator':
> {noformat}
> gfsh>list members
> Name | Id
> ------------ | ------------------------------------------------
> Coordinator: | 10.118.19.26(locator1:7903:locator)<ec><v0>:1024
> locator1 | 10.118.19.26(locator1:7903:locator)<ec><v0>:1024
> server1 | 10.118.19.26(server1:7933)<v1>:1025
> server2 | 10.118.19.26(server2:8075)<v2>:1026
> {noformat}
> This just looks untidy.
> Here {{server-2}} is part of a group:
> {noformat}
> gfsh>list members --group=group1
> Name | Id
> ------------ | ------------------------------------------------
> Coordinator: | 10.118.19.26(locator1:7903:locator)<ec><v0>:1024
> server2 | 10.118.19.26(server2:8075)<v2>:1026
> {noformat}
> But the 'coordinator' is not part of that group. Again, inconsistent and
> untidy.
> I would prefer the relevant member to be highlight somehow as a coordinator.
> For example:
> {noformat}
> gfsh>list members
> Name | Id
> ------------ | ------------------------------------------------
> locator1 | 10.118.19.26(locator1:7903:locator)<ec><v0>:1024 [coordinator]
> server1 | 10.118.19.26(server1:7933)<v1>:1025
> server2 | 10.118.19.26(server2:8075)<v2>:1026
> {noformat}
> Or perhaps better yet would be to introduce a separate {{list coordinator}}
> command.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
