Mercurial > hg > thermostat-ng > agent
changeset 2427:bc83ef2d7047
Allow re-using of old attached Byteman agent.
Reviewed-by: neugens
Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2016-August/020642.html
| author | Severin Gehwolf <sgehwolf@redhat.com> |
|---|---|
| date | Tue, 23 Aug 2016 18:47:09 +0200 |
| parents | ec9be9806a79 |
| children | 88f534ba35ba |
| files | vm-byteman/agent/src/main/java/com/redhat/thermostat/vm/byteman/agent/internal/BytemanAgentAttachManager.java vm-byteman/agent/src/main/java/com/redhat/thermostat/vm/byteman/agent/internal/BytemanAgentInfo.java vm-byteman/agent/src/main/java/com/redhat/thermostat/vm/byteman/agent/internal/BytemanAttacher.java vm-byteman/agent/src/test/java/com/redhat/thermostat/vm/byteman/agent/internal/BytemanAgentAttachManagerTest.java vm-byteman/agent/src/test/java/com/redhat/thermostat/vm/byteman/agent/internal/BytemanAgentInfoTest.java vm-byteman/agent/src/test/java/com/redhat/thermostat/vm/byteman/agent/internal/BytemanAttacherTest.java |
| diffstat | 6 files changed, 221 insertions(+), 29 deletions(-) [+] |
line wrap: on
line diff
--- a/vm-byteman/agent/src/main/java/com/redhat/thermostat/vm/byteman/agent/internal/BytemanAgentAttachManager.java Wed Aug 24 12:06:35 2016 +0200 +++ b/vm-byteman/agent/src/main/java/com/redhat/thermostat/vm/byteman/agent/internal/BytemanAgentAttachManager.java Tue Aug 23 18:47:09 2016 +0200 @@ -39,6 +39,7 @@ import java.io.File; import java.util.ArrayList; import java.util.List; +import java.util.Properties; import java.util.logging.Level; import java.util.logging.Logger; @@ -65,7 +66,9 @@ private static final String BYTEMAN_HELPER_DIR = BYTEMAN_PLUGIN_LIBS_DIR + File.separator + "thermostat-helper"; private static final String BYTEMAN_HOME_PROPERTY = "org.jboss.byteman.home"; private static final Logger logger = LoggingUtils.getLogger(BytemanAgentAttachManager.class); + // package-private for testing + static final String THERMOSTAT_HELPER_SOCKET_NAME_PROPERTY = "org.jboss.byteman.thermostat.socketName"; static List<String> helperJars; private final SubmitHelper submit; @@ -94,16 +97,11 @@ logger.warning("Failed to attach byteman agent for VM '" + vmPid + "'. Skipping rule updater and IPC channel."); return null; } - if (info.isAttachFailedNoSuchProcess()) { - logger.finest("Process with pid " + vmPid + " went away before we could attach the byteman agent to it."); + VmSocketIdentifier socketId = new VmSocketIdentifier(vmId.get(), vmPid, writerId.getWriterID()); + boolean postAttachGood = performPostAttachSteps(info, socketId); + if (!postAttachGood) { return null; } - logger.fine("Attached byteman agent to VM '" + vmPid + "' at port: '" + info.getAgentListenPort()); - if (!addThermostatHelperJarsToClasspath(info)) { - logger.warning("VM '" + vmPid + "': Failed to add helper jars to target VM's classpath."); - return null; - } - VmSocketIdentifier socketId = new VmSocketIdentifier(vmId.get(), vmPid, writerId.getWriterID()); ThermostatIPCCallbacks callback = new BytemanMetricsReceiver(vmBytemanDao, socketId); ipcManager.startIPCEndpoint(socketId, callback); // Add a status record to storage @@ -115,6 +113,26 @@ return status; } + private boolean performPostAttachSteps(BytemanAgentInfo info, VmSocketIdentifier socketId) { + if (info.isAttachFailedNoSuchProcess()) { + logger.finest("Process with pid " + info.getVmPid() + " went away before we could attach the byteman agent to it."); + return false; + } + if (info.isOldAttach()) { + logger.finest("Proceeding with post-attach steps for already attached byteman agent"); + Properties properties = new Properties(); + properties.setProperty(THERMOSTAT_HELPER_SOCKET_NAME_PROPERTY, socketId.getName()); + return submit.setSystemProperties(properties, info); + } else { + logger.fine("Attached byteman agent to VM '" + info.getVmPid() + "' at port: '" + info.getAgentListenPort()); + boolean addJarsSuccess = addThermostatHelperJarsToClasspath(info); + if (!addJarsSuccess) { + logger.warning("VM '" + info.getVmPid() + "': Failed to add helper jars to target VM's classpath."); + } + return addJarsSuccess; + } + } + private boolean addThermostatHelperJarsToClasspath(BytemanAgentInfo info) { return submit.addJarsToSystemClassLoader(helperJars, info); } @@ -181,5 +199,17 @@ } } + boolean setSystemProperties(Properties properties, BytemanAgentInfo info) { + Submit submit = new Submit(null /* localhost */, info.getAgentListenPort()); + try { + String result = submit.setSystemProperties(properties); + logger.fine("Re-set system properties with result: " + result); + return true; + } catch (Exception e) { + logger.log(Level.INFO, e.getMessage(), e); + return false; + } + } + } }
--- a/vm-byteman/agent/src/main/java/com/redhat/thermostat/vm/byteman/agent/internal/BytemanAgentInfo.java Wed Aug 24 12:06:35 2016 +0200 +++ b/vm-byteman/agent/src/main/java/com/redhat/thermostat/vm/byteman/agent/internal/BytemanAgentInfo.java Tue Aug 23 18:47:09 2016 +0200 @@ -46,14 +46,16 @@ private final String vmId; private final String writerId; private final boolean isAttachFailedNoProc; + private final boolean isOldAttach; - BytemanAgentInfo(int vmPid, int agentListenPort, String listenHost, String vmId, String writerId, boolean isAttachFailedNoProc) { + BytemanAgentInfo(int vmPid, int agentListenPort, String listenHost, String vmId, String writerId, boolean isAttachFailedNoProc, boolean isOldAttach) { this.agentListenPort = agentListenPort; this.listenHost = listenHost; this.vmPid = vmPid; this.vmId = vmId; this.writerId = writerId; this.isAttachFailedNoProc = isAttachFailedNoProc; + this.isOldAttach = isOldAttach; } int getVmPid() { @@ -80,6 +82,10 @@ return isAttachFailedNoProc; } + boolean isOldAttach() { + return isOldAttach; + } + @Override public boolean equals(Object other) { if (other == null) { @@ -94,11 +100,12 @@ Objects.equals(vmPid, o.vmPid) && Objects.equals(vmId, o.vmId) && Objects.equals(writerId, o.writerId) && - Objects.equals(isAttachFailedNoProc, o.isAttachFailedNoProc); + Objects.equals(isAttachFailedNoProc, o.isAttachFailedNoProc) && + Objects.equals(isOldAttach, o.isOldAttach); } @Override public int hashCode() { - return Objects.hash(agentListenPort, listenHost, vmPid, vmId, writerId, isAttachFailedNoProc); + return Objects.hash(agentListenPort, listenHost, vmPid, vmId, writerId, isAttachFailedNoProc, isOldAttach); } }
--- a/vm-byteman/agent/src/main/java/com/redhat/thermostat/vm/byteman/agent/internal/BytemanAttacher.java Wed Aug 24 12:06:35 2016 +0200 +++ b/vm-byteman/agent/src/main/java/com/redhat/thermostat/vm/byteman/agent/internal/BytemanAttacher.java Tue Aug 23 18:47:09 2016 +0200 @@ -92,11 +92,12 @@ VmSocketIdentifier sockIdentifier = new VmSocketIdentifier(vmId, pid, agentId); String[] btmInstallProps = buildBytemanInstallProps(sockIdentifier, port); boolean agentJarToBootClassPath = true; - int actualPort = installer.install(Integer.toString(pid), agentJarToBootClassPath, false, null /* localhost */, port, btmInstallProps); + InstallResult result = installer.install(Integer.toString(pid), agentJarToBootClassPath, false, null /* localhost */, port, btmInstallProps); + int actualPort = result.getPort(); // Port might have changed here if agent rebooted and targed jvm // stayed alive if (actualPort > 0) { - return new BytemanAgentInfo(pid, actualPort, null, vmId, agentId, false); + return new BytemanAgentInfo(pid, actualPort, null, vmId, agentId, false, result.isOldAttach()); } else { return null; } @@ -107,7 +108,7 @@ return handleAttachFailure(e, vmId, port, pid); } catch (IOException e) { if (!processChecker.exists(pid)) { - return new BytemanAgentInfo(pid, port, null, vmId, agentId, true); + return new BytemanAgentInfo(pid, port, null, vmId, agentId, true, false); } return handleAttachFailure(e, vmId, port, pid); } @@ -151,7 +152,7 @@ private static final int UNKNOWN_PORT = -1; - int install(String vmPid, boolean addToBoot, boolean setPolicy, String hostname, int port, String[] properties) + InstallResult install(String vmPid, boolean addToBoot, boolean setPolicy, String hostname, int port, String[] properties) throws IllegalArgumentException, FileNotFoundException, IOException, AttachNotSupportedException, AgentLoadException, AgentInitializationException { @@ -159,17 +160,36 @@ boolean loaded = Boolean.parseBoolean(propVal); if (!loaded) { Install.install(vmPid, addToBoot, setPolicy, hostname, port, properties); - return port; + return new InstallResult(port, false); } else { try { int oldPort = Integer.parseInt(Install.getSystemProperty(vmPid, BYTEMAN_AGENT_PORT_PROPERTY)); logger.finest("VM (pid: " + vmPid + "): Not installing byteman agent since one is already attached on port " + oldPort); - return oldPort; + return new InstallResult(oldPort, true); } catch (NumberFormatException e) { logger.info("VM (pid: " + vmPid + "): Has a byteman agent already attached, but it wasn't thermostat that attached it"); - return UNKNOWN_PORT; + return new InstallResult(UNKNOWN_PORT, true); } } } } + + static class InstallResult { + + private final int port; + private final boolean isOldAttach; + + InstallResult(int port, boolean isOldAttach) { + this.port = port; + this.isOldAttach = isOldAttach; + } + + boolean isOldAttach() { + return isOldAttach; + } + + int getPort() { + return port; + } + } } \ No newline at end of file
--- a/vm-byteman/agent/src/test/java/com/redhat/thermostat/vm/byteman/agent/internal/BytemanAgentAttachManagerTest.java Wed Aug 24 12:06:35 2016 +0200 +++ b/vm-byteman/agent/src/test/java/com/redhat/thermostat/vm/byteman/agent/internal/BytemanAgentAttachManagerTest.java Tue Aug 23 18:47:09 2016 +0200 @@ -45,12 +45,14 @@ import static org.mockito.Matchers.isA; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import java.io.File; import java.io.IOException; import java.util.List; +import java.util.Properties; import org.junit.After; import org.junit.Before; @@ -72,7 +74,7 @@ private static final VmId SOME_VM_ID = new VmId("some-vm-id"); private static final int SOME_VM_PID = 99910; private static final String SOME_AGENT_ID = "some-agent-id"; - private static final BytemanAgentInfo SOME_SUCCESS_BYTEMAN_INFO = new BytemanAgentInfo(SOME_VM_PID, 3344, null, SOME_VM_ID.get(), SOME_AGENT_ID, false); + private static final BytemanAgentInfo SOME_SUCCESS_BYTEMAN_INFO = new BytemanAgentInfo(SOME_VM_PID, 3344, null, SOME_VM_ID.get(), SOME_AGENT_ID, false, false); private BytemanAgentAttachManager manager; private IPCEndpointsManager ipcManager; @@ -103,7 +105,7 @@ int vmPid = 1001; String agentId = "working-agent-id"; int listenPort = 9881; - BytemanAgentInfo bytemanAgentInfo = new BytemanAgentInfo(vmPid, listenPort, null, workingVmId, agentId, false); + BytemanAgentInfo bytemanAgentInfo = new BytemanAgentInfo(vmPid, listenPort, null, workingVmId, agentId, false, false); when(bytemanAttacher.attach(workingVmId, vmPid, agentId)).thenReturn(bytemanAgentInfo); // mock that installing of helper jars works @@ -136,6 +138,53 @@ assertEquals(listenPort, bytemanStatus.getListenPort()); } + @SuppressWarnings("unchecked") + @Test + public void canUseOldAttachedAgentStartIPCandAddsStatus() { + String workingVmId = "working-vm-id"; + int vmPid = 1001; + String agentId = "working-agent-id"; + int listenPort = 9881; + BytemanAgentInfo bytemanAgentInfo = new BytemanAgentInfo(vmPid, listenPort, null, workingVmId, agentId, false, true); + when(bytemanAttacher.attach(workingVmId, vmPid, agentId)).thenReturn(bytemanAgentInfo); + + // mock setting system properties' success + when(submit.setSystemProperties(any(Properties.class), any(BytemanAgentInfo.class))).thenReturn(true); + + VmId vmId = new VmId(workingVmId); + WriterID writerId = mock(WriterID.class); + when(writerId.getWriterID()).thenReturn(agentId); + manager.setWriterId(writerId); + VmBytemanStatus bytemanStatus = manager.attachBytemanToVm(vmId, vmPid); + VmSocketIdentifier socketId = new VmSocketIdentifier(workingVmId, vmPid, agentId); + + // IPC endpoint must be started + verify(ipcManager).startIPCEndpoint(eq(socketId), isA(BytemanMetricsReceiver.class)); + + // Status should have been updated/inserted + ArgumentCaptor<VmBytemanStatus> statusCaptor = ArgumentCaptor.forClass(VmBytemanStatus.class); + verify(vmBytemanDao).addOrReplaceBytemanStatus(statusCaptor.capture()); + VmBytemanStatus capturedStatus = statusCaptor.getValue(); + assertNotNull(capturedStatus); + assertEquals(workingVmId, capturedStatus.getVmId()); + assertEquals(agentId, capturedStatus.getAgentId()); + assertEquals(listenPort, capturedStatus.getListenPort()); + assertNull(capturedStatus.getRule()); + assertTrue(capturedStatus.getTimeStamp() > 0); + + // Helper jars must not have been added again to classpath + verify(submit, times(0)).addJarsToSystemClassLoader(any(List.class), any(BytemanAgentInfo.class)); + + // Verify properties got set appropriately + ArgumentCaptor<Properties> propsCaptor = ArgumentCaptor.forClass(Properties.class); + verify(submit).setSystemProperties(propsCaptor.capture(), eq(bytemanAgentInfo)); + Properties props = propsCaptor.getValue(); + String propVal = props.getProperty(BytemanAgentAttachManager.THERMOSTAT_HELPER_SOCKET_NAME_PROPERTY); + assertEquals(socketId.getName(), propVal); + + assertEquals(listenPort, bytemanStatus.getListenPort()); + } + @Test public void failureToAttachDoesNotStartIPC() throws Exception { BytemanAttacher failAttacher = getFailureAttacher();
--- a/vm-byteman/agent/src/test/java/com/redhat/thermostat/vm/byteman/agent/internal/BytemanAgentInfoTest.java Wed Aug 24 12:06:35 2016 +0200 +++ b/vm-byteman/agent/src/test/java/com/redhat/thermostat/vm/byteman/agent/internal/BytemanAgentInfoTest.java Tue Aug 23 18:47:09 2016 +0200 @@ -45,6 +45,7 @@ public class BytemanAgentInfoTest { private static final boolean SOME_ATTACH_FAILED = false; + private static final boolean SOME_IS_OLD_ATTACH = false; private static final int SOME_VM_PID = 99292; private static final int SOME_LISTEN_PORT = 3321; private static final String SOME_LISTEN_HOST = "foo-bar"; @@ -53,10 +54,22 @@ @Test public void testEqualsHashCode() { - BytemanAgentInfo bytemanInfo = new BytemanAgentInfo(SOME_VM_PID, SOME_LISTEN_PORT, SOME_LISTEN_HOST, SOME_VM_ID, SOME_AGENT_ID, SOME_ATTACH_FAILED); + BytemanAgentInfo bytemanInfo = new BytemanAgentInfo(SOME_VM_PID, + SOME_LISTEN_PORT, + SOME_LISTEN_HOST, + SOME_VM_ID, + SOME_AGENT_ID, + SOME_ATTACH_FAILED, + SOME_IS_OLD_ATTACH); assertFalse(bytemanInfo.equals(null)); assertFalse(bytemanInfo.equals(null)); // multiple calls - BytemanAgentInfo info2 = new BytemanAgentInfo(SOME_VM_PID, SOME_LISTEN_PORT, SOME_LISTEN_HOST, SOME_VM_ID, SOME_AGENT_ID, SOME_ATTACH_FAILED); + BytemanAgentInfo info2 = new BytemanAgentInfo(SOME_VM_PID, + SOME_LISTEN_PORT, + SOME_LISTEN_HOST, + SOME_VM_ID, + SOME_AGENT_ID, + SOME_ATTACH_FAILED, + SOME_IS_OLD_ATTACH); assertTrue(bytemanInfo.equals(info2)); assertTrue(info2.equals(bytemanInfo)); // reflexive property assertTrue(bytemanInfo.equals(info2)); // multiple calls @@ -67,35 +80,88 @@ @Test public void testNotEquals() { - BytemanAgentInfo bytemanInfo = new BytemanAgentInfo(SOME_VM_PID, SOME_LISTEN_PORT, SOME_LISTEN_HOST, SOME_VM_ID, SOME_AGENT_ID, SOME_ATTACH_FAILED); + BytemanAgentInfo bytemanInfo = new BytemanAgentInfo(SOME_VM_PID, + SOME_LISTEN_PORT, + SOME_LISTEN_HOST, + SOME_VM_ID, + SOME_AGENT_ID, + SOME_ATTACH_FAILED, + SOME_IS_OLD_ATTACH); // vmPid different - BytemanAgentInfo other = new BytemanAgentInfo(992, SOME_LISTEN_PORT, SOME_LISTEN_HOST, SOME_VM_ID, SOME_AGENT_ID, SOME_ATTACH_FAILED); + BytemanAgentInfo other = new BytemanAgentInfo(992, + SOME_LISTEN_PORT, + SOME_LISTEN_HOST, + SOME_VM_ID, + SOME_AGENT_ID, + SOME_ATTACH_FAILED, + SOME_IS_OLD_ATTACH); assertFalse(bytemanInfo.equals(other)); assertFalse(other.equals(bytemanInfo)); // listen port different - other = new BytemanAgentInfo(SOME_VM_PID, 5554, SOME_LISTEN_HOST, SOME_VM_ID, SOME_AGENT_ID, SOME_ATTACH_FAILED); + other = new BytemanAgentInfo(SOME_VM_PID, + 5554, + SOME_LISTEN_HOST, + SOME_VM_ID, + SOME_AGENT_ID, + SOME_ATTACH_FAILED, + SOME_IS_OLD_ATTACH); assertFalse(bytemanInfo.equals(other)); assertFalse(other.equals(bytemanInfo)); // listen host different - other = new BytemanAgentInfo(SOME_VM_PID, SOME_LISTEN_PORT, null, SOME_VM_ID, SOME_AGENT_ID, SOME_ATTACH_FAILED); + other = new BytemanAgentInfo(SOME_VM_PID, + SOME_LISTEN_PORT, + null, + SOME_VM_ID, + SOME_AGENT_ID, + SOME_ATTACH_FAILED, + SOME_IS_OLD_ATTACH); assertFalse(bytemanInfo.equals(other)); assertFalse(other.equals(bytemanInfo)); // vmId different - other = new BytemanAgentInfo(SOME_VM_PID, SOME_LISTEN_PORT, SOME_LISTEN_HOST, "other", SOME_AGENT_ID, SOME_ATTACH_FAILED); + other = new BytemanAgentInfo(SOME_VM_PID, + SOME_LISTEN_PORT, + SOME_LISTEN_HOST, + "other", + SOME_AGENT_ID, + SOME_ATTACH_FAILED, + SOME_IS_OLD_ATTACH); assertFalse(bytemanInfo.equals(other)); assertFalse(other.equals(bytemanInfo)); // agentId different - other = new BytemanAgentInfo(SOME_VM_PID, SOME_LISTEN_PORT, SOME_LISTEN_HOST, SOME_VM_ID, "foobar", SOME_ATTACH_FAILED); + other = new BytemanAgentInfo(SOME_VM_PID, + SOME_LISTEN_PORT, + SOME_LISTEN_HOST, + SOME_VM_ID, + "foobar", + SOME_ATTACH_FAILED, + SOME_IS_OLD_ATTACH); assertFalse(bytemanInfo.equals(other)); assertFalse(other.equals(bytemanInfo)); // attachFailed different - other = new BytemanAgentInfo(SOME_VM_PID, SOME_LISTEN_PORT, SOME_LISTEN_HOST, SOME_VM_ID, SOME_AGENT_ID, true); + other = new BytemanAgentInfo(SOME_VM_PID, + SOME_LISTEN_PORT, + SOME_LISTEN_HOST, + SOME_VM_ID, + SOME_AGENT_ID, + true, + SOME_IS_OLD_ATTACH); + assertFalse(bytemanInfo.equals(other)); + assertFalse(other.equals(bytemanInfo)); + + // old attach different + other = new BytemanAgentInfo(SOME_VM_PID, + SOME_LISTEN_PORT, + SOME_LISTEN_HOST, + SOME_VM_ID, + SOME_AGENT_ID, + SOME_ATTACH_FAILED, + true); assertFalse(bytemanInfo.equals(other)); assertFalse(other.equals(bytemanInfo)); }
--- a/vm-byteman/agent/src/test/java/com/redhat/thermostat/vm/byteman/agent/internal/BytemanAttacherTest.java Wed Aug 24 12:06:35 2016 +0200 +++ b/vm-byteman/agent/src/test/java/com/redhat/thermostat/vm/byteman/agent/internal/BytemanAttacherTest.java Tue Aug 23 18:47:09 2016 +0200 @@ -58,6 +58,7 @@ import com.redhat.thermostat.agent.utils.ProcessChecker; import com.redhat.thermostat.shared.config.CommonPaths; import com.redhat.thermostat.vm.byteman.agent.internal.BytemanAttacher.BtmInstallHelper; +import com.redhat.thermostat.vm.byteman.agent.internal.BytemanAttacher.InstallResult; public class BytemanAttacherTest { @@ -102,6 +103,8 @@ @Test public void attachSetsAppropriateProperties() throws Exception { BtmInstallHelper installer = mock(BtmInstallHelper.class); + when(installer.install(any(String.class), any(boolean.class), any(boolean.class), any(String.class), any(int.class), any(String[].class))) + .thenReturn(new InstallResult(8888, true)); ArgumentCaptor<String[]> propsCaptor = ArgumentCaptor.forClass(String[].class); BytemanAttacher attacher = new BytemanAttacher(installer, processChecker, paths); attacher.attach("testVmId", 9999, "fooAgent"); @@ -122,6 +125,23 @@ } /* + * Tests whether an old byteman agent attached by a previous thermostat + * agent run can be re-used when the JVM persists + */ + @Test + public void canReuseOldAttach() throws Exception { + int port = 39203; + BtmInstallHelper installer = mock(BtmInstallHelper.class); + when(installer.install(any(String.class), any(boolean.class), any(boolean.class), any(String.class), any(int.class), any(String[].class))) + .thenReturn(new InstallResult(port, true)); + BytemanAttacher attacher = new BytemanAttacher(installer, mock(ProcessChecker.class), paths); + + int somePid = 3222; + BytemanAgentInfo info = attacher.attach("someVmId", somePid, "someAgent"); + assertTrue(info.isOldAttach()); + } + + /* * English locale: "No such process" exception message. It shouldn't matter * though. Verified by next test. */