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>'].