Mercurial > hg > release > thermostat-1.2

changeset 1658:3e61adfe186a

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
thermostat service fails silently if agent fails to connect. Reviewed-by: omajid Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2015-March/012987.html PR2263
author Severin Gehwolf <sgehwolf@redhat.com>
date Tue, 03 Mar 2015 16:32:21 +0100
parents 331e21088194
children 2418a81fcd27
files agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/Activator.java agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/AgentApplication.java agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/ServiceCommand.java agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/locale/LocaleResources.java agent/cli/src/main/resources/com/redhat/thermostat/agent/cli/impl/strings.properties agent/cli/src/test/java/com/redhat/thermostat/agent/cli/impl/ServiceCommandTest.java
diffstat 6 files changed, 100 insertions(+), 65 deletions(-) [+]
line wrap: on
line diff
--- a/agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/Activator.java	Mon Mar 02 17:10:50 2015 +0100
+++ b/agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/Activator.java	Tue Mar 03 16:32:21 2015 +0100
@@ -75,7 +75,7 @@
 
             @Override
             public void dependenciesUnavailable() {
-                agentApplication.shutdown();
+                agentApplication.shutdown(ExitStatus.EXIT_SUCCESS);
                 reg.unregisterCommands();
             }
         });
@@ -87,7 +87,7 @@
         if (agentApplication != null) {
             // Bundle may be shut down *before* deps become available and
             // app is set.
-            agentApplication.shutdown();
+            agentApplication.shutdown(ExitStatus.EXIT_SUCCESS);
         }
         reg.unregisterCommands();
         tracker.close();
--- a/agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/AgentApplication.java	Mon Mar 02 17:10:50 2015 +0100
+++ b/agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/AgentApplication.java	Tue Mar 03 16:32:21 2015 +0100
@@ -62,9 +62,9 @@
 import com.redhat.thermostat.common.MultipleServiceTracker.Action;
 import com.redhat.thermostat.common.cli.AbstractStateNotifyingCommand;
 import com.redhat.thermostat.common.cli.Arguments;
-import com.redhat.thermostat.common.cli.Command;
 import com.redhat.thermostat.common.cli.CommandContext;
 import com.redhat.thermostat.common.cli.CommandException;
+import com.redhat.thermostat.common.tools.ApplicationState;
 import com.redhat.thermostat.common.utils.LoggingUtils;
 import com.redhat.thermostat.shared.config.InvalidConfigurationException;
 import com.redhat.thermostat.storage.core.Connection.ConnectionListener;
@@ -173,12 +173,12 @@
                     logger.log(Level.SEVERE, e.getMessage());
                     // log stack trace as info only
                     logger.log(Level.INFO, e.getMessage(), e);
-                    shutdown();
+                    shutdown(ExitStatus.EXIT_ERROR);
                 } catch (ConnectionException e) {
                     logger.log(Level.SEVERE, "Could not connect to storage (" + e.getMessage() + ")");
                     // log stack trace as info only
                     logger.log(Level.INFO, "Could nto connect to storage", e);
-                    shutdown();
+                    shutdown(ExitStatus.EXIT_ERROR);
                 }
                 
                 return configServer;
@@ -214,7 +214,7 @@
         }
     }
     
-    public void shutdown() {
+    public void shutdown(int shutDownStatus) {
         // Exit application
         if (shutdownLatch != null) {
             shutdownLatch.countDown();
@@ -226,6 +226,12 @@
         if (configServerTracker != null) {
             configServerTracker.close();
         }
+        this.exitStatus.setExitStatus(shutDownStatus);
+        if (shutDownStatus == ExitStatus.EXIT_SUCCESS) {
+            getNotifier().fireAction(ApplicationState.STOP);
+        } else {
+            getNotifier().fireAction(ApplicationState.FAIL);
+        }
     }
     
     private class CustomSignalHandler implements SignalHandler {
@@ -254,7 +260,7 @@
             if (Boolean.getBoolean(VERBOSE_MODE_PROPERTY)) {
                 System.out.println(VERBOSE_MODE_AGENT_STOPPED_MSG);
             }
-            shutdown();
+            shutdown(ExitStatus.EXIT_SUCCESS);
         }
         
     }
@@ -266,8 +272,7 @@
             
         } catch (Exception e) {
             logger.log(Level.SEVERE, "Could not get BackendRegistry instance.", e);
-            exitStatus.setExitStatus(ExitStatus.EXIT_ERROR);
-            shutdown();
+            shutdown(ExitStatus.EXIT_ERROR);
             // Since this would throw NPE's down the line if we continue in this
             // method, let's fail right and early :)
             throw new RuntimeException(e);
@@ -284,8 +289,7 @@
             logger.log(Level.SEVERE,
                     "Agent could not start, probably because a configured backend could not be activated.",
                     le);
-            exitStatus.setExitStatus(ExitStatus.EXIT_ERROR);
-            shutdown();
+            shutdown(ExitStatus.EXIT_ERROR);
         }
         logger.fine("Agent started.");
         // Hook for integration tests. Print a well known message to stdout
@@ -295,6 +299,7 @@
         }
 
         logger.info("Agent id: " + agent.getId());
+        getNotifier().fireAction(ApplicationState.START, agent.getId());
         return agent;
     }
     
@@ -325,7 +330,7 @@
                 if (shutdownLatch.getCount() > 0) {
                     // In the rare case we lose one of our deps, gracefully shutdown
                     logger.severe("Storage unexpectedly became unavailable");
-                    shutdown();
+                    shutdown(ExitStatus.EXIT_ERROR);
                 }
             }
             
--- a/agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/ServiceCommand.java	Mon Mar 02 17:10:50 2015 +0100
+++ b/agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/ServiceCommand.java	Tue Mar 03 16:32:21 2015 +0100
@@ -52,6 +52,7 @@
 import com.redhat.thermostat.common.cli.AbstractStateNotifyingCommand;
 import com.redhat.thermostat.common.cli.CommandContext;
 import com.redhat.thermostat.common.cli.CommandException;
+import com.redhat.thermostat.common.cli.Console;
 import com.redhat.thermostat.common.tools.ApplicationState;
 import com.redhat.thermostat.common.utils.LoggingUtils;
 import com.redhat.thermostat.launcher.Launcher;
@@ -122,7 +123,9 @@
                     String dbUrl = (String) payload;
                     String[] agentArgs =  new String[] {"agent", "-d", dbUrl};
                     logger.fine("starting agent now...");
-                    launcher.run(agentArgs, false);
+                    listeners.clear();
+                    listeners.add(new AgentStartedListener(cmdCtx.getConsole()));
+                    launcher.run(agentArgs, listeners, false);
                     break;
                 case FAIL:
                     storageFailed = true;
@@ -148,6 +151,36 @@
     public boolean isStorageRequired() {
         return false;
     }
+    
+    private static class AgentStartedListener implements ActionListener<ApplicationState> {
+
+        private final Console console;
+
+        private AgentStartedListener(Console console) {
+            this.console = console;
+        }
+
+        @Override
+        public void actionPerformed(ActionEvent<ApplicationState> actionEvent) {
+            if (actionEvent.getSource() instanceof AbstractStateNotifyingCommand) {
+                AbstractStateNotifyingCommand agent = (AbstractStateNotifyingCommand) actionEvent.getSource();
+                // Implementation detail: there is a single AgentCommand instance registered
+                // as an OSGi service. We remove ourselves as listener so that we don't get
+                // notified in the case that the command is invoked by some other means later.
+                agent.getNotifier().removeActionListener(this);
+
+                switch (actionEvent.getActionId()) {
+                case START:
+                    logger.fine("Agent started via service. Agent ID was: " + actionEvent.getPayload());
+                    break;
+                case FAIL:
+                    console.getError().println(translator.localize(LocaleResources.STARTING_AGENT_FAILED).getContents());
+                    break;
+                }
+            }
+
+        }
+    }
 
 }
 
--- a/agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/locale/LocaleResources.java	Mon Mar 02 17:10:50 2015 +0100
+++ b/agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/locale/LocaleResources.java	Tue Mar 03 16:32:21 2015 +0100
@@ -43,6 +43,7 @@
     SERVICE_FAILED_TO_START_DB,
     LAUNCHER_UNAVAILABLE,
     UNEXPECTED_RESULT_STORAGE,
+    STARTING_AGENT_FAILED,
     ;
 
     static final String RESOURCE_BUNDLE = "com.redhat.thermostat.agent.cli.impl.strings";
--- a/agent/cli/src/main/resources/com/redhat/thermostat/agent/cli/impl/strings.properties	Mon Mar 02 17:10:50 2015 +0100
+++ b/agent/cli/src/main/resources/com/redhat/thermostat/agent/cli/impl/strings.properties	Tue Mar 03 16:32:21 2015 +0100
@@ -1,3 +1,4 @@
 SERVICE_FAILED_TO_START_DB = Service failed to start due to error starting storage.
 LAUNCHER_UNAVAILABLE = Launcher is not available
 UNEXPECTED_RESULT_STORAGE = Unexpected result from storage.
+STARTING_AGENT_FAILED = Thermostat agent failed to start. See logs for details.
--- a/agent/cli/src/test/java/com/redhat/thermostat/agent/cli/impl/ServiceCommandTest.java	Mon Mar 02 17:10:50 2015 +0100
+++ b/agent/cli/src/test/java/com/redhat/thermostat/agent/cli/impl/ServiceCommandTest.java	Tue Mar 03 16:32:21 2015 +0100
@@ -82,8 +82,6 @@
     private static final String[] STORAGE_STOP_ARGS = { "storage", "--stop" };
     private static final String[] AGENT_ARGS = {"agent", "-d", "Test String"};
     
-    private static int count = 0;
-
     @SuppressWarnings("unchecked")
     @Before
     public void setUp() {
@@ -109,7 +107,6 @@
 
     @After
     public void tearDown() {
-        count = 0;
         listeners = null;
         mockLauncher = null;
         serviceCommand = null;
@@ -144,58 +141,12 @@
         
         verify(mockLauncher, times(1)).run(eq(STORAGE_START_ARGS), isA(Collection.class), anyBoolean());
         verify(mockLauncher, times(1)).run(eq(STORAGE_STOP_ARGS), anyBoolean());
-        verify(mockLauncher, times(1)).run(eq(AGENT_ARGS), anyBoolean());
+        verify(mockLauncher, times(1)).run(eq(AGENT_ARGS), isA(Collection.class), anyBoolean());
         verify(mockActionEvent, times(1)).getActionId();
     }
 
     @SuppressWarnings("unchecked")
     @Test(timeout=1000)
-    public void testMultipleRun() throws CommandException {
-        doAnswer(new Answer<Void>() {
-            public Void answer(InvocationOnMock invocation) throws Throwable {
-                Object[] args = invocation.getArguments();
-                listeners = (Collection<ActionListener<ApplicationState>>)args[1];
-                
-                if (count == 0) {
-                    when(mockActionEvent.getActionId()).thenReturn(ApplicationState.START);
-                } else {
-                    when(mockActionEvent.getActionId()).thenReturn(ApplicationState.FAIL);
-                    when(mockActionEvent.getPayload()).thenReturn(new Exception("Test Exception"));
-                }
-                ++count;
-                
-                for(ActionListener<ApplicationState> listener : listeners) {
-                    listener.actionPerformed(mockActionEvent);
-                }
-                return null;
-            }
-        }).when(mockLauncher).run(eq(STORAGE_START_ARGS), isA(Collection.class), anyBoolean());
-        
-        boolean exTriggered = false;
-        try {
-            serviceCommand.run(mockCommandContext);
-        } catch (CommandException e) { 
-            exTriggered = true;
-        }
-        Assert.assertFalse(exTriggered);
-        
-        try {
-            serviceCommand.run(mockCommandContext);
-        } catch (CommandException e) {
-            exTriggered = true;
-            Assert.assertEquals(e.getLocalizedMessage(), "Service failed to start due to error starting storage.");
-        }
-        Assert.assertTrue(exTriggered);
-        Assert.assertEquals("Test Exception\n", stdErrOut.toString());
-        
-        verify(mockLauncher, times(2)).run(eq(STORAGE_START_ARGS), isA(Collection.class), anyBoolean());
-        verify(mockLauncher, times(1)).run(eq(STORAGE_STOP_ARGS), anyBoolean());
-        verify(mockLauncher, times(1)).run(eq(AGENT_ARGS), anyBoolean());
-        verify(mockActionEvent, times(2)).getActionId();
-    }
-
-    @SuppressWarnings("unchecked")
-    @Test(timeout=1000)
     public void testStorageFailStart()  throws CommandException {
         doAnswer(new Answer<Void>() {
             public Void answer(InvocationOnMock invocation) throws Throwable {
@@ -223,7 +174,7 @@
         
         verify(mockLauncher, times(1)).run(eq(STORAGE_START_ARGS), isA(Collection.class), anyBoolean());
         verify(mockLauncher, never()).run(eq(STORAGE_STOP_ARGS), anyBoolean());
-        verify(mockLauncher, never()).run(eq(AGENT_ARGS), anyBoolean());
+        verify(mockLauncher, never()).run(eq(AGENT_ARGS), isA(Collection.class), anyBoolean());
         verify(mockActionEvent, times(1)).getActionId();
     }
     
@@ -256,9 +207,53 @@
         
         verify(mockLauncher, times(1)).run(eq(STORAGE_START_ARGS), isA(Collection.class), anyBoolean());
         verify(mockLauncher, never()).run(eq(STORAGE_STOP_ARGS), anyBoolean());
-        verify(mockLauncher, never()).run(eq(AGENT_ARGS), anyBoolean());
+        verify(mockLauncher, never()).run(eq(AGENT_ARGS), isA(Collection.class), anyBoolean());
         verify(mockActionEvent, times(1)).getActionId();
     }
+    
+    @Test
+    public void testAgentStartFail()  throws CommandException {
+        doAnswer(new Answer<Void>() {
+            public Void answer(InvocationOnMock invocation) throws Throwable {
+                Object[] args = invocation.getArguments();
+                listeners = (Collection<ActionListener<ApplicationState>>)args[1];
+                
+                when(mockActionEvent.getActionId()).thenReturn(ApplicationState.START);
+                
+                for(ActionListener<ApplicationState> listener : listeners) {
+                    listener.actionPerformed(mockActionEvent);
+                }
+                return null;
+            }
+        }).when(mockLauncher).run(eq(STORAGE_START_ARGS), isA(Collection.class), anyBoolean());
+        doAnswer(new Answer<Void>() {
+            public Void answer(InvocationOnMock invocation) throws Throwable {
+                Object[] args = invocation.getArguments();
+                listeners = (Collection<ActionListener<ApplicationState>>)args[1];
+                
+                when(mockActionEvent.getActionId()).thenReturn(ApplicationState.FAIL);
+                
+                for(ActionListener<ApplicationState> listener : listeners) {
+                    listener.actionPerformed(mockActionEvent);
+                }
+                return null;
+            }
+        }).when(mockLauncher).run(eq(AGENT_ARGS), isA(Collection.class), anyBoolean());
+        
+        boolean exTriggered = false;
+        try {
+            serviceCommand.run(mockCommandContext);
+        } catch (CommandException e) {
+            exTriggered = true;
+        }
+        Assert.assertFalse(exTriggered);
+        Assert.assertEquals("Thermostat agent failed to start. See logs for details.\n", stdErrOut.toString());
+        
+        verify(mockLauncher, times(1)).run(eq(STORAGE_START_ARGS), isA(Collection.class), anyBoolean());
+        verify(mockLauncher, times(1)).run(eq(STORAGE_STOP_ARGS), anyBoolean());
+        verify(mockLauncher, times(1)).run(eq(AGENT_ARGS), isA(Collection.class), anyBoolean());
+        verify(mockActionEvent, times(2)).getActionId();
+    }
 
 }