You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by jj...@apache.org on 2019/05/22 15:53:05 UTC
[geode] branch develop updated: GEODE-6769: Fix Coordinator tag in
list members (#3603)
This is an automated email from the ASF dual-hosted git repository.
jjramos 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 c15cf96 GEODE-6769: Fix Coordinator tag in list members (#3603)
c15cf96 is described below
commit c15cf96c2cdb982955c091974ab00bdc5598abdc
Author: Juan José Ramos <ju...@users.noreply.github.com>
AuthorDate: Wed May 22 16:52:53 2019 +0100
GEODE-6769: Fix Coordinator tag in list members (#3603)
The command was searching for the coordinator member and using the
actual object memory reference to execute the comparison, which could
return wrong results..
- Fixed minor warnings.
- When searching for the coordinator, execute the comparison using the
memberUniqueId instead of the memory reference.
---
.../cli/commands/ListMembersCommandDUnitTest.java | 83 ++++++++++++++++------
.../internal/cli/commands/ListMembersCommand.java | 14 ++--
.../cli/commands/ListMembersCommandTest.java | 12 ++--
3 files changed, 76 insertions(+), 33 deletions(-)
diff --git a/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/ListMembersCommandDUnitTest.java b/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/ListMembersCommandDUnitTest.java
index 33f75a5..6c8559b 100644
--- a/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/ListMembersCommandDUnitTest.java
+++ b/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/ListMembersCommandDUnitTest.java
@@ -19,34 +19,46 @@ import static org.apache.geode.management.internal.cli.i18n.CliStrings.LIST_MEMB
import static org.apache.geode.test.junit.rules.GfshCommandRule.PortType.jmxManager;
import static org.assertj.core.api.Assertions.assertThat;
+import java.util.List;
import java.util.Properties;
+import java.util.regex.Pattern;
+import junitparams.JUnitParamsRunner;
+import junitparams.Parameters;
+import junitparams.naming.TestCaseName;
import org.junit.BeforeClass;
import org.junit.ClassRule;
import org.junit.Rule;
import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.apache.geode.management.internal.cli.result.CommandResult;
import org.apache.geode.test.dunit.rules.ClusterStartupRule;
import org.apache.geode.test.dunit.rules.MemberVM;
import org.apache.geode.test.junit.rules.GfshCommandRule;
-
+@RunWith(JUnitParamsRunner.class)
public class ListMembersCommandDUnitTest {
- @ClassRule
- public static ClusterStartupRule lsRule = new ClusterStartupRule();
- private static MemberVM locator;
+ private static MemberVM locator1, locator2;
+ private Pattern pattern = Pattern.compile("(.*)locator-0(.*)\\[Coordinator]");
@Rule
- public GfshCommandRule gfsh = new GfshCommandRule(locator::getJmxPort, jmxManager);
+ public GfshCommandRule gfsh = new GfshCommandRule(locator1::getJmxPort, jmxManager);
+
+ @ClassRule
+ public static ClusterStartupRule lsRule = new ClusterStartupRule();
@BeforeClass
- public static void setup() throws Exception {
+ public static void setup() {
Properties properties = new Properties();
properties.setProperty(GROUPS, "locatorGroup");
- locator = lsRule.startLocatorVM(0, properties);
- lsRule.startServerVM(1, "serverGroup1", locator.getPort());
- lsRule.startServerVM(2, "serverGroup1", locator.getPort());
- lsRule.startServerVM(3, "serverGroup2", locator.getPort());
+ // First member, becomes the coordinator by default.
+ locator1 = lsRule.startLocatorVM(0, properties);
+ locator2 = lsRule.startLocatorVM(1, properties, locator1.getPort());
+
+ lsRule.startServerVM(2, "serverGroup1", locator1.getPort(), locator2.getPort());
+ lsRule.startServerVM(3, "serverGroup1", locator1.getPort(), locator2.getPort());
+ lsRule.startServerVM(4, "serverGroup2", locator1.getPort(), locator2.getPort());
}
@Test
@@ -57,14 +69,16 @@ public class ListMembersCommandDUnitTest {
}
@Test
- public void listAllMembers() throws Exception {
+ public void listAllMembers() {
gfsh.executeAndAssertThat(LIST_MEMBER).statusIsSuccess()
.hasTableSection(ListMembersCommand.MEMBERS_SECTION).hasColumn("Name")
- .containsExactlyInAnyOrder("locator-0", "server-1", "server-2", "server-3");
+ .containsExactlyInAnyOrder("locator-0", "locator-1", "server-2", "server-3", "server-4");
+
+ assertCoordinatorTag(gfsh.getCommandResult());
}
@Test
- public void listMembersInLocatorGroup() throws Exception {
+ public void listMembersInLocatorGroup() {
gfsh.executeAndAssertThat(LIST_MEMBER + " --group=locatorGroup").statusIsSuccess();
String output = gfsh.getGfshOutput();
@@ -72,32 +86,61 @@ public class ListMembersCommandDUnitTest {
assertThat(output).doesNotContain("server-1");
assertThat(output).doesNotContain("server-2");
assertThat(output).doesNotContain("server-3");
+ assertCoordinatorTag(gfsh.getCommandResult());
}
@Test
- public void listMembersInServerGroupOne() throws Exception {
+ public void listMembersInServerGroupOne() {
gfsh.executeAndAssertThat(LIST_MEMBER + " --group=serverGroup1").statusIsSuccess();
String output = gfsh.getGfshOutput();
- assertThat(output).contains("server-1");
assertThat(output).contains("server-2");
- assertThat(output).doesNotContain("server-3");
+ assertThat(output).contains("server-3");
+ assertThat(output).doesNotContain("server-4");
}
@Test
- public void listMembersInServerGroupTwo() throws Exception {
+ public void listMembersInServerGroupTwo() {
gfsh.executeAndAssertThat(LIST_MEMBER + " --group=serverGroup2").statusIsSuccess();
String output = gfsh.getGfshOutput();
- assertThat(output).doesNotContain("server-1");
assertThat(output).doesNotContain("server-2");
- assertThat(output).contains("server-3");
+ assertThat(output).doesNotContain("server-3");
+ assertThat(output).contains("server-4");
}
@Test
- public void listMembersInNonExistentGroup() throws Exception {
+ public void listMembersInNonExistentGroup() {
gfsh.executeAndAssertThat(LIST_MEMBER + " --group=foo")
.statusIsSuccess()
.containsOutput("No Members Found")
.doesNotContainOutput("locator-0", "server-1", "server-2", "server-3");
}
+
+ @Test
+ @Parameters(value = {"true", "false"})
+ @TestCaseName("{method} - Connected to Coordinator: {params}")
+ public void listMembersShouldAlwaysTagTheCoordinatorMember(boolean useCoordinator)
+ throws Exception {
+ int jmxPort = useCoordinator ? locator1.getJmxPort() : locator2.getJmxPort();
+ gfsh.disconnect();
+ gfsh.connectAndVerify(jmxPort, jmxManager);
+
+ gfsh.executeAndAssertThat(LIST_MEMBER).statusIsSuccess()
+ .hasTableSection(ListMembersCommand.MEMBERS_SECTION).hasColumn("Id")
+ .hasSize(5);
+
+ assertCoordinatorTag(gfsh.getCommandResult());
+ }
+
+ /**
+ * Assert that the locator-0 has been marked with the "Coordinator" tag.
+ *
+ * @param commandResult GFSH command result to test.
+ */
+ private void assertCoordinatorTag(CommandResult commandResult) {
+ List<String> ids = commandResult.getResultData()
+ .getTableSection("members")
+ .getValuesInColumn("Id");
+ assertThat(ids.stream().filter(string -> pattern.matcher(string).matches())).hasSize(1);
+ }
}
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 ee9b4ac..5ee455d 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
@@ -46,8 +46,8 @@ public class ListMembersCommand extends GfshCommand {
help = CliStrings.LIST_MEMBER__GROUP__HELP) String[] groups) {
ResultModel crm = new ResultModel();
- Set<DistributedMember> memberSet = new TreeSet<>();
- memberSet.addAll(this.findMembersIncludingLocators(groups, null));
+ Set<DistributedMember> memberSet = new TreeSet<>(
+ this.findMembersIncludingLocators(groups, null));
if (memberSet.isEmpty()) {
crm.addInfo().addLine(CliStrings.LIST_MEMBER__MSG__NO_MEMBER_FOUND);
@@ -55,10 +55,11 @@ public class ListMembersCommand extends GfshCommand {
}
TabularResultModel resultData = crm.addTable(MEMBERS_SECTION);
- final DistributedMember coordinatorMember = getCoordinator();
+ final String coordinatorMemberId = getCoordinatorId();
for (DistributedMember member : memberSet) {
resultData.accumulate("Name", member.getName());
- if (member == coordinatorMember) {
+
+ if (member.getUniqueId().equals(coordinatorMemberId)) {
resultData.accumulate("Id", member.getId() + " [Coordinator]");
} else {
resultData.accumulate("Id", member.getId());
@@ -68,7 +69,7 @@ public class ListMembersCommand extends GfshCommand {
return crm;
}
- DistributedMember getCoordinator() {
+ String getCoordinatorId() {
InternalDistributedSystem ids = InternalDistributedSystem.getConnectedInstance();
if (ids == null || !ids.isConnected()) {
return null;
@@ -79,7 +80,6 @@ public class ListMembersCommand extends GfshCommand {
return null;
}
- return mmgr.getCoordinator();
-
+ return mmgr.getCoordinator().getUniqueId();
}
}
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
index b09f4b8..ef5f843 100644
--- 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
@@ -12,7 +12,6 @@
* 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.assertj.core.api.Assertions.assertThat;
@@ -34,16 +33,14 @@ import org.junit.Test;
import org.apache.geode.distributed.DistributedMember;
import org.apache.geode.test.junit.rules.GfshParserRule;
-
public class ListMembersCommandTest {
-
@ClassRule
public static GfshParserRule gfsh = new GfshParserRule();
private ListMembersCommand command;
- private Set<DistributedMember> members;
private DistributedMember member1;
private DistributedMember member2;
+ private Set<DistributedMember> members;
@Before
public void before() {
@@ -54,11 +51,14 @@ public class ListMembersCommandTest {
member1 = mock(DistributedMember.class);
when(member1.getName()).thenReturn("name");
when(member1.getId()).thenReturn("id");
- doReturn(member1).when(command).getCoordinator();
+ when(member1.getUniqueId()).thenReturn("uniqueId1");
+ doReturn("uniqueId1").when(command).getCoordinatorId();
member2 = mock(DistributedMember.class);
when(member2.getName()).thenReturn("name2");
when(member2.getId()).thenReturn("id2");
+ when(member2.getUniqueId()).thenReturn("uniqueId2");
+
// This will enforce the sort order in TreeSet used by ListMembersCommand.
when(member1.compareTo(member2)).thenReturn(-1);
when(member2.compareTo(member1)).thenReturn(1);
@@ -84,7 +84,7 @@ public class ListMembersCommandTest {
@Test
public void noCoordinator() {
members.add(member1);
- doReturn(null).when(command).getCoordinator();
+ doReturn(null).when(command).getCoordinatorId();
Map<String, List<String>> table = gfsh.executeAndAssertThat(command, "list members")
.hasTableSection().getActual().getContent();