Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
use strongly instead of stringly typed IP addresses in ClusterMemberInfo 
Change-Id: Ib61b034f193df68bdac8dc7433e2d14ae176b815
Signed-off-by: Michael Vorburger <vorburger@redhat.com>
  • Loading branch information
@vorburger
vorburger committed Oct 9, 2018
1 parent 74d8cb3 commit 187e2d8
Show file tree
Hide file tree
Showing 6 changed files with 68 additions and 67 deletions.
9 changes: 4 additions & 5 deletions diagstatus/api/src/main/java/org/opendaylight/infrautils/diagstatus/ClusterMemberInfo.java
View file
Expand Up @@ -7,6 +7,7 @@
*/
package org.opendaylight.infrautils.diagstatus;

import java.net.InetAddress;
import java.util.List;

/**
Expand All @@ -16,11 +17,9 @@
*/
public interface ClusterMemberInfo {

// TODO change String to InetAddress in both getSelfAddress() and getClusterMembers()
InetAddress getSelfAddress();

String getSelfAddress();
List<InetAddress> getClusterMembers();

List<String> getClusterMembers();

boolean isLocalAddress(String ipAddress);
boolean isLocalAddress(InetAddress ipAddress);
}
29 changes: 14 additions & 15 deletions diagstatus/api/src/main/java/org/opendaylight/infrautils/diagstatus/MBeanUtils.java
View file
Expand Up @@ -9,8 +9,6 @@

import static java.util.Objects.requireNonNull;

import com.google.common.net.InetAddresses;
import com.google.errorprone.annotations.Var;
import java.io.IOException;
import java.lang.management.ManagementFactory;
import java.net.Inet6Address;
Expand Down Expand Up @@ -63,21 +61,24 @@ public final class MBeanUtils {
private MBeanUtils() {
}

public static JMXServiceURL getJMXUrl(String targetHost) throws MalformedURLException {
public static JMXServiceURL getJMXUrl(InetAddress targetHost) throws MalformedURLException {
String jmxUrl = constructJmxUrl(targetHost, RMI_REGISTRY_PORT);
return new JMXServiceURL(jmxUrl);
}

public static String constructJmxUrl(@Var String targetHost, int rmiRegistryPort) {
if (isIpv6Address(targetHost)) {
targetHost = '[' + targetHost + ']';
public static String constructJmxUrl(InetAddress targetHost, int rmiRegistryPort) {
String targetHostAsString;
if (targetHost instanceof Inet6Address) {
targetHostAsString = '[' + targetHost.getHostAddress() + ']';
} else {
targetHostAsString = targetHost.getHostAddress();
}
return JMX_HOST_PREFIX + targetHost + JMX_TARGET_PREFIX
+ targetHost + JMX_URL_SEPARATOR + rmiRegistryPort + JMX_URL_SUFFIX;
return JMX_HOST_PREFIX + targetHostAsString + JMX_TARGET_PREFIX
+ targetHostAsString + JMX_URL_SEPARATOR + rmiRegistryPort + JMX_URL_SUFFIX;
}

public static Pair<JMXConnectorServer,Registry> startRMIConnectorServer(MBeanServer mbeanServer, String selfAddress)
throws IOException {
public static Pair<JMXConnectorServer, Registry> startRMIConnectorServer(MBeanServer mbeanServer,
InetAddress selfAddress) throws IOException {
JMXServiceURL url = getJMXUrl(requireNonNull(selfAddress, "selfAddress"));
Registry registry = LocateRegistry.createRegistry(RMI_REGISTRY_PORT);
JMXConnectorServer cs;
Expand Down Expand Up @@ -160,11 +161,9 @@ public static <T, R> R invokeRemoteMBeanOperation(String remoteURL, String jmxNa
MBeanServerConnection mbsc = jmxc.getMBeanServerConnection();
T remoteMBean = getMBean(jmxName, klass, mbsc);
return function.apply(remoteMBean);
} catch (MalformedURLException e) {
LOG.error("MalformedURLException: {} (path={})", jmxURL, jmxURL.getURLPath(), e);
throw e;
}
}

private static Boolean isIpv6Address(String ipAddress) {
InetAddress address = InetAddresses.forString(ipAddress);
return address instanceof Inet6Address;
}
}
15 changes: 8 additions & 7 deletions .../src/main/java/org/opendaylight/infrautils/diagstatus/internal/ClusterMemberInfoImpl.java
View file
Expand Up @@ -9,6 +9,7 @@

import static java.util.Objects.requireNonNull;

import com.google.common.net.InetAddresses;
import java.net.InetAddress;
import java.util.ArrayList;
import java.util.Collections;
Expand Down Expand Up @@ -36,7 +37,7 @@ public class ClusterMemberInfoImpl implements ClusterMemberInfo {
private static final Logger LOG = LoggerFactory.getLogger(ClusterMemberInfoImpl.class);

@Override
public String getSelfAddress() {
public InetAddress getSelfAddress() {
Object clusterStatusMBeanValue;
try {
clusterStatusMBeanValue = MBeanUtils.getMBeanAttribute("akka:type=Cluster", "ClusterStatus");
Expand All @@ -47,14 +48,14 @@ public String getSelfAddress() {
String clusterStatusText = clusterStatusMBeanValue.toString();
String selfAddressMbean = requireNonNull(StringUtils.substringBetween(clusterStatusText,
"\"self-address\": ", ","), "null substringBetween() for: " + clusterStatusText);
return extractAddressFromAkka(selfAddressMbean);
return InetAddresses.forString(extractAddressFromAkka(selfAddressMbean));
} else {
throw new IllegalStateException("getMBeanAttribute(\"akka:type=Cluster\", \"ClusterStatus\") == null?!");
}
}

@Override
public List<String> getClusterMembers() {
public List<InetAddress> getClusterMembers() {
Object clusterMemberMBeanValue;
try {
clusterMemberMBeanValue = MBeanUtils.getMBeanAttribute("akka:type=Cluster", "Members");
Expand All @@ -63,21 +64,21 @@ public List<String> getClusterMembers() {
return Collections.emptyList();
}

List<String> clusterIPAddresses = new ArrayList<>();
List<InetAddress> clusterIPAddresses = new ArrayList<>();
if (clusterMemberMBeanValue != null) {
String[] clusterMembers = ((String)clusterMemberMBeanValue).split(",", -1);
for (String clusterMember : clusterMembers) {
String nodeIp = extractAddressFromAkka(clusterMember);
clusterIPAddresses.add(nodeIp);
clusterIPAddresses.add(InetAddresses.forString(nodeIp));
}
}
return clusterIPAddresses;
}

@Override
public boolean isLocalAddress(String ipAddress) {
public boolean isLocalAddress(InetAddress ipAddress) {
// TODO also checking if ipAddress === getSelfAddress() would seem to make sense here?
return ipAddress.equals(InetAddress.getLoopbackAddress().getHostAddress());
return ipAddress.equals(InetAddress.getLoopbackAddress());
}

private static String extractAddressFromAkka(String clusterMember) {
Expand Down
3 changes: 2 additions & 1 deletion ...main/java/org/opendaylight/infrautils/diagstatus/internal/DiagStatusServiceMBeanImpl.java
View file
Expand Up @@ -15,6 +15,7 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableMap.Builder;
import java.io.IOException;
import java.net.InetAddress;
import java.rmi.registry.Registry;
import java.util.Map;
import javax.annotation.PreDestroy;
Expand Down Expand Up @@ -72,7 +73,7 @@ public DiagStatusServiceMBeanImpl(DiagStatusService diagStatusService,

@Override
public void onSystemBootReady() {
String host = clusterMemberInfo.getSelfAddress();
InetAddress host = clusterMemberInfo.getSelfAddress();
try {
jmxConnector = MBeanUtils.startRMIConnectorServer(mbeanServer, host);
} catch (IOException e) {
Expand Down
58 changes: 28 additions & 30 deletions ...s/shell/src/main/java/org/opendaylight/infrautils/diagstatus/shell/DiagStatusCommand.java
View file
Expand Up @@ -8,8 +8,9 @@
package org.opendaylight.infrautils.diagstatus.shell;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Strings;
import com.google.common.net.InetAddresses;
import java.io.PrintStream;
import java.net.InetAddress;
import java.util.List;
import org.apache.karaf.shell.api.action.Command;
import org.apache.karaf.shell.api.action.Option;
Expand Down Expand Up @@ -49,12 +50,12 @@ protected void run(PrintStream ps) throws Exception {
strBuilder.append("Timestamp: ").append(new java.util.Date().toString()).append("\n");

if (null != nip) {
strBuilder.append(getNodeSpecificStatus(nip));
strBuilder.append(getNodeSpecificStatus(InetAddresses.forString(nip)));
} else {
List<String> clusterIPAddresses = clusterMemberInfoProvider.getClusterMembers();
List<InetAddress> clusterIPAddresses = clusterMemberInfoProvider.getClusterMembers();
if (!clusterIPAddresses.isEmpty()) {
String selfAddress = clusterMemberInfoProvider.getSelfAddress();
for (String memberAddress : clusterIPAddresses) {
InetAddress selfAddress = clusterMemberInfoProvider.getSelfAddress();
for (InetAddress memberAddress : clusterIPAddresses) {
try {
if (memberAddress.equals(selfAddress)) {
strBuilder.append(getLocalStatusSummary(memberAddress));
Expand All @@ -70,59 +71,56 @@ protected void run(PrintStream ps) throws Exception {
}
} else {
LOG.info("Could not obtain cluster members or the cluster-command is being executed locally\n");
strBuilder.append(getLocalStatusSummary("localhost"));
strBuilder.append(getLocalStatusSummary(InetAddress.getLoopbackAddress()));
}
}

ps.println(strBuilder.toString());
}

private String getLocalStatusSummary(String localIPAddress) {
return "Node IP Address: " + localIPAddress + "\n" + diagStatusServiceMBean.acquireServiceStatusDetailed();
private String getLocalStatusSummary(InetAddress memberAddress) {
return "Node IP Address: " + memberAddress.toString() + "\n"
+ diagStatusServiceMBean.acquireServiceStatusDetailed();
}

@VisibleForTesting
static String getRemoteStatusSummary(String ipAddress) throws Exception {
String url = MBeanUtils.constructJmxUrl(ipAddress, MBeanUtils.RMI_REGISTRY_PORT);
static String getRemoteStatusSummary(InetAddress memberAddress) throws Exception {
String url = MBeanUtils.constructJmxUrl(memberAddress, MBeanUtils.RMI_REGISTRY_PORT);
LOG.info("invokeRemoteJMXOperation() JMX service URL: {}", url);

String remoteJMXOperationResult = MBeanUtils.invokeRemoteMBeanOperation(url, MBeanUtils.JMX_OBJECT_NAME,
DiagStatusServiceMBean.class, remoteMBean -> remoteMBean.acquireServiceStatusDetailed());

StringBuilder strBuilder = new StringBuilder();
strBuilder.append("Node IP Address: ").append(ipAddress).append("\n");
strBuilder.append("Node IP Address: ").append(memberAddress).append("\n");
strBuilder.append(remoteJMXOperationResult);
return strBuilder.toString();
}

@SuppressWarnings("checkstyle:IllegalCatch")
private String getNodeSpecificStatus(String ipAddress) throws Exception {
private String getNodeSpecificStatus(InetAddress ipAddress) throws Exception {
StringBuilder strBuilder = new StringBuilder();
if (!Strings.isNullOrEmpty(ipAddress)) {
if (isIPAddressInCluster(ipAddress)) {
if (clusterMemberInfoProvider.isLocalAddress(ipAddress)) {
// Local IP Address
strBuilder.append(getLocalStatusSummary(ipAddress));
} else {
// Remote IP
try {
strBuilder.append(getRemoteStatusSummary(ipAddress));
} catch (Exception e) {
strBuilder.append("Remote Status retrieval JMX Operation failed for node: ").append(ipAddress);
LOG.error("Exception while reaching Host: {}", ipAddress, e);
}
}
if (isIPAddressInCluster(ipAddress)) {
if (clusterMemberInfoProvider.isLocalAddress(ipAddress)) {
// Local IP Address
strBuilder.append(getLocalStatusSummary(ipAddress));
} else {
strBuilder.append("Invalid IP Address or Not a cluster member IP Address: ").append(ipAddress);
// Remote IP
try {
strBuilder.append(getRemoteStatusSummary(ipAddress));
} catch (Exception e) {
strBuilder.append("Remote Status retrieval JMX Operation failed for node: ").append(ipAddress);
LOG.error("Exception while reaching Host: {}", ipAddress, e);
}
}
} else {
strBuilder.append("Invalid or Empty IP Address");
strBuilder.append("Invalid IP Address or Not a cluster member IP Address: ").append(ipAddress);
}
return strBuilder.toString();
}

private boolean isIPAddressInCluster(String ipAddress) {
List<String> clusterIPAddresses = clusterMemberInfoProvider.getClusterMembers();
private boolean isIPAddressInCluster(InetAddress ipAddress) {
List<InetAddress> clusterIPAddresses = clusterMemberInfoProvider.getClusterMembers();
if (!clusterIPAddresses.contains(ipAddress)) {
LOG.error("specified ip {} is not present in cluster", ipAddress);
return false;
Expand Down
21 changes: 12 additions & 9 deletions ...ell/src/test/java/org/opendaylight/infrautils/diagstatus/shell/DiagStatusCommandTest.java
View file
Expand Up @@ -7,9 +7,12 @@
*/
package org.opendaylight.infrautils.diagstatus.shell;

import static com.google.common.truth.Truth.assertThat;
import static java.util.Collections.emptyList;
import static org.mockito.Mockito.mock;

import com.google.common.net.InetAddresses;
import java.net.InetAddress;
import java.util.List;
import org.junit.Test;
import org.opendaylight.infrautils.diagstatus.ClusterMemberInfo;
Expand All @@ -28,36 +31,36 @@ public class DiagStatusCommandTest {

@Test
public void testGetRemoteStatusSummary_IPv4() throws Exception {
checkGetRemoteStatusSummary("127.0.0.1");
checkGetRemoteStatusSummary(InetAddresses.forString("127.0.0.1"));
}

@Test
public void testGetRemoteStatusSummary_IPv6() throws Exception {
checkGetRemoteStatusSummary("::1");
checkGetRemoteStatusSummary(InetAddresses.forString("::1"));
}

private static void checkGetRemoteStatusSummary(String ipAddress) throws Exception {
private static void checkGetRemoteStatusSummary(InetAddress inetAddress) throws Exception {
DiagStatusService diagStatusService = mock(DiagStatusService.class);
SystemReadyMonitor systemReadyMonitor = new TestSystemReadyMonitor(Behaviour.IMMEDIATE);
ClusterMemberInfo clusterMemberInfo = new ClusterMemberInfo() {
@Override
public String getSelfAddress() {
return ipAddress;
public InetAddress getSelfAddress() {
return inetAddress;
}

@Override
public List<String> getClusterMembers() {
public List<InetAddress> getClusterMembers() {
return emptyList();
}

@Override
public boolean isLocalAddress(String isLocalIpAddress) {
return ipAddress.equals(isLocalIpAddress);
public boolean isLocalAddress(InetAddress isLocalIpAddress) {
return inetAddress.equals(isLocalIpAddress);
}
};
try (DiagStatusServiceMBeanImpl diagStatusServiceMBeanImpl =
new DiagStatusServiceMBeanImpl(diagStatusService, systemReadyMonitor, clusterMemberInfo)) {
DiagStatusCommand.getRemoteStatusSummary(ipAddress);
assertThat(DiagStatusCommand.getRemoteStatusSummary(inetAddress)).contains(inetAddress.toString());
}
}
}

0 comments on commit 187e2d8

Please sign in to comment.