JENKINS-39498 Fix invalid data

jenkinsci · Nov 20, 2017 · 8478576 · 8478576
1 parent 0abd387
commit 8478576
Show file tree

Hide file tree

Showing 5 changed files with 231 additions and 19 deletions.
diff --git a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/GerritServer.java b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/GerritServer.java
@@ -425,6 +425,7 @@ public void start() {
 
         missedEventsPlaybackManager.checkIfEventsLogPluginSupported();
         addListener((GerritEventListener)missedEventsPlaybackManager);
+        missedEventsPlaybackManager.start();
 
         logger.info(name + " started");
         started = true;

diff --git a/...onyericsson/hudson/plugins/gerrit/trigger/playback/GerritMissedEventsPlaybackManager.java b/...onyericsson/hudson/plugins/gerrit/trigger/playback/GerritMissedEventsPlaybackManager.java
@@ -80,13 +80,14 @@
  *
  * @author scott.hebert@ericsson.com
  */
-public class GerritMissedEventsPlaybackManager implements ConnectionListener, NamedGerritEventListener {
+public class GerritMissedEventsPlaybackManager extends Thread implements ConnectionListener, NamedGerritEventListener {
 
     private static final String GERRIT_SERVER_EVENT_DATA_FOLDER = "/gerrit-server-event-data/";
     private static final Logger logger = LoggerFactory.getLogger(GerritMissedEventsPlaybackManager.class);
     static final String EVENTS_LOG_PLUGIN_NAME = "events-log";
     private static final String EVENTS_LOG_PLUGIN_URL = "a/plugins/" + EVENTS_LOG_PLUGIN_NAME + "/events/";
     private static final String GERRIT_TRIGGER_SERVER_TIMESTAMPS_XML = "gerrit-trigger-server-timestamps.xml";
+    private static final long SLEEPTIME = 2000;
 
     private String serverName;
     /**
@@ -126,7 +127,7 @@ public void checkIfEventsLogPluginSupported() {
         GerritServer server = PluginImpl.getServer_(serverName);
         if (server != null && server.getConfig() != null) {
             isSupported = GerritPluginChecker.isPluginEnabled(
-                    server.getConfig(), EVENTS_LOG_PLUGIN_NAME);
+                    server.getConfig(), EVENTS_LOG_PLUGIN_NAME, true);
         }
     }
 
@@ -138,6 +139,8 @@ protected void load() throws IOException {
         XmlFile xml = getConfigXml(serverName);
         if (xml.exists()) {
             serverTimestamp  = (EventTimeSlice)xml.unmarshal(serverTimestamp);
+        } else {
+            serverTimestamp = null;
         }
     }
 
@@ -155,6 +158,38 @@ protected synchronized Date getDateFromTimestamp() {
         return new Date();
     }
 
+    @Override
+    public void run() {
+        while (true) {
+            if (playBackComplete) {
+                boolean previousIsSupported = isSupported;
+                checkIfEventsLogPluginSupported();
+                boolean currentIsSupported = isSupported;
+                if (previousIsSupported && !currentIsSupported) {
+                    logger.warn("Missed Events Playback used to be supported. now it is not!");
+                    // we could be missing events here that we should be persisting...
+                    // so let's remove the data file so we are ready if it comes back
+                    try {
+                        XmlFile config = getConfigXml(serverName);
+                        config.delete();
+                        logger.warn("Deleting " + config.getFile().getAbsolutePath());
+                    } catch (IOException e) {
+                        logger.error(e.getMessage(), e);
+                    }
+                }
+                if (!previousIsSupported && currentIsSupported) {
+                    logger.warn("Missed Events Playback used to be NOT supported. now it IS!");
+                }
+            }
+            //CS IGNORE EmptyBlock FOR NEXT 5 LINES. REASON: ignore.
+            try {
+                Thread.sleep(SLEEPTIME);
+            } catch (InterruptedException e) {
+                //e.printStackTrace();
+            }
+        }
+    }
+
     /**
      * When the connection is established, we load in the last-alive
      * timestamp for this server and try to determine if a time range

diff --git a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/utils/GerritPluginChecker.java b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/utils/GerritPluginChecker.java
@@ -77,37 +77,76 @@ private static String buildURL(IGerritHudsonTriggerConfig config) {
      * Given a status code, decode its status.
      * @param statusCode HTTP code
      * @param pluginName plugin that was checked.
+     * @param quiet Should we log messages.
      * @return true/false if installed or not.
      */
-    private static boolean decodeStatus(int statusCode, String pluginName) {
+    private static boolean decodeStatus(int statusCode, String pluginName, boolean quiet) {
+        String message = "";
         switch (statusCode) {
             case HttpURLConnection.HTTP_OK:
-                logger.info(Messages.PluginInstalled(pluginName));
+                message = Messages.PluginInstalled(pluginName);
+                if (quiet) {
+                    logger.debug(message);
+                } else {
+                    logger.info(message);
+                }
                 return true;
             case HttpURLConnection.HTTP_NOT_FOUND:
-                logger.info(Messages.PluginNotInstalled(pluginName));
+                message = Messages.PluginNotInstalled(pluginName);
+                if (quiet) {
+                    logger.debug(message);
+                } else {
+                    logger.warn(message);
+                }
                 return false;
             case HttpURLConnection.HTTP_UNAUTHORIZED:
-                logger.warn(Messages.PluginHttpConnectionUnauthorized(pluginName,
-                        Messages.HttpConnectionUnauthorized()));
+                message = Messages.PluginHttpConnectionUnauthorized(pluginName,
+                        Messages.HttpConnectionUnauthorized());
+                if (quiet) {
+                    logger.debug(message);
+                } else {
+                    logger.warn(message);
+                }
                 return false;
             case HttpURLConnection.HTTP_FORBIDDEN:
-                logger.warn(Messages.PluginHttpConnectionForbidden(pluginName,
-                        Messages.HttpConnectionUnauthorized()));
+                message = Messages.PluginHttpConnectionForbidden(pluginName,
+                        Messages.HttpConnectionUnauthorized());
+                if (quiet) {
+                    logger.debug(message);
+                } else {
+                    logger.warn(message);
+                }
                 return false;
             default:
-                logger.warn(Messages.PluginHttpConnectionGeneralError(pluginName,
-                        Messages.HttpConnectionError(statusCode)));
+                message = Messages.PluginHttpConnectionGeneralError(pluginName,
+                        Messages.HttpConnectionError(statusCode));
+                if (quiet) {
+                    logger.debug(message);
+                } else {
+                    logger.warn(message);
+                }
                 return false;
         }
     }
+
     /**
      * Query Gerrit to determine if plugin is enabled.
      * @param config Gerrit Server Config
      * @param pluginName The Gerrit Plugin name.
      * @return true if enabled.
      */
     public static boolean isPluginEnabled(IGerritHudsonTriggerConfig config, String pluginName) {
+        return isPluginEnabled(config, pluginName, false);
+    }
+
+    /**
+     * Query Gerrit to determine if plugin is enabled.
+     * @param config Gerrit Server Config
+     * @param pluginName The Gerrit Plugin name.
+     * @param quiet Whether we want to log a message.
+     * @return true if enabled.
+     */
+    public static boolean isPluginEnabled(IGerritHudsonTriggerConfig config, String pluginName, boolean quiet) {
 
         String restUrl = buildURL(config);
         if (restUrl == null) {
@@ -121,7 +160,7 @@ public static boolean isPluginEnabled(IGerritHudsonTriggerConfig config, String
             execute = HttpUtils.performHTTPGet(config, restUrl + "plugins/" + pluginName + "/");
             int statusCode = execute.getStatusLine().getStatusCode();
             logger.debug("status code: {}", statusCode);
-            return decodeStatus(statusCode, pluginName);
+            return decodeStatus(statusCode, pluginName, quiet);
         } catch (IOException e) {
             logger.warn(Messages.PluginHttpConnectionGeneralError(pluginName,
                     e.getMessage()), e);

diff --git a/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/mock/Setup.java b/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/mock/Setup.java
@@ -175,6 +175,7 @@ public static RefUpdated createRefUpdated(String serverName, String project, Str
 
         return event;
     }
+
     /**
      * Create a new patchset created event with the given data.
      * @param serverName The server name
@@ -183,6 +184,21 @@ public static RefUpdated createRefUpdated(String serverName, String project, Str
      * @return a pactchsetCreated event
      */
     public static PatchsetCreated createPatchsetCreated(String serverName, String project, String ref) {
+        return createPatchsetCreated(serverName, project, ref, "1418133772");
+    }
+
+    /**
+     * Create a new patchset created event with the given data.
+     * @param serverName The server name
+     * @param project The project
+     * @param ref The ref
+     * @param eventCreateOn Timestamp for eventcreateon.
+     * @return a patchsetCreated event
+     */
+    public static PatchsetCreated createPatchsetCreated(String serverName,
+                                                        String project,
+                                                        String ref,
+                                                        String eventCreateOn) {
         PatchsetCreated event = new PatchsetCreated();
         Change change = new Change();
         change.setBranch("branch");
@@ -202,7 +218,7 @@ public static PatchsetCreated createPatchsetCreated(String serverName, String pr
         patch.setRef(ref);
         event.setPatchset(patch);
         event.setProvider(new Provider(serverName, "gerrit", "29418", "ssh", "http://gerrit/", "1"));
-        event.setEventCreatedOn("1418133772");
+        event.setEventCreatedOn(eventCreateOn);
         return event;
     }
 

diff --git a/...sonyericsson/hudson/plugins/gerrit/trigger/playback/GerritMissedEventsFunctionalTest.java b/...sonyericsson/hudson/plugins/gerrit/trigger/playback/GerritMissedEventsFunctionalTest.java
@@ -53,6 +53,7 @@
 import static com.github.tomakehurst.wiremock.client.WireMock.get;
 import static com.github.tomakehurst.wiremock.client.WireMock.urlMatching;
 import static com.sonymobile.tools.gerrit.gerritevents.mock.SshdServerMock.GERRIT_STREAM_EVENTS;
+import static junit.framework.TestCase.assertFalse;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertSame;
@@ -81,7 +82,12 @@ public class GerritMissedEventsFunctionalTest {
     public final WireMockRule wireMockRule = new WireMockRule(0); // No-args constructor defaults to port 8089
 
     private static final int HTTPOK = 200;
+    private static final int HTTPERROR = 500;
     private static final int SLEEPTIME = 1000;
+    // 15 seconds
+    private static final long LONGSLEEPTIME = 15000;
+    private static final long WAITTIMEFOREVENTLOGDISABLE = 6000;
+    private static final long TIMESTAMPDELTA = 1000;
 
     private final int port = 29418;
 
@@ -156,6 +162,108 @@ public void testRestartWithMissedEvents() throws Exception {
 
     }
 
+    /**
+     * Test the scenario whereby events-log plugin becomes not supported during operation,
+     * events are received but NOT persisted and Jenkins is restarted.
+     * @throws Exception Error creating job.
+     */
+    @Test
+    public void testLosePluginSupportedWithEventsAndRestart() throws Exception {
+
+        int buildNum = 0;
+
+        GerritServer gerritServer = new GerritServer("ABCDEF");
+        PluginImpl.getInstance().addServer(gerritServer);
+        gerritServer.start();
+
+        Config config = (Config)gerritServer.getConfig();
+        config.setGerritFrontEndURL("http://localhost:" + wireMockRule.port());
+        config.setGerritProxy("");
+        config.setGerritAuthKeyFile(sshKey.getPrivateKey());
+        SshdServerMock.configureFor(sshd, gerritServer);
+        gerritServer.startConnection();
+
+        while (!gerritServer.isConnected()) {
+            Thread.sleep(SLEEPTIME);
+        }
+
+        Config config2 = (Config)gerritServer.getConfig();
+        config2.setUseRestApi(true);
+        config2.setGerritHttpUserName("scott");
+        config2.setGerritHttpPassword("scott");
+
+        //simulate a save of config...it calls doConfigSubmit()
+        gerritServer.getMissedEventsPlaybackManager().checkIfEventsLogPluginSupported();
+
+        FreeStyleProject project = DuplicatesUtil.createGerritTriggeredJob(j, "Test ABCDEF", gerritServer.getName());
+        createAndWaitforPatchset(gerritServer, project, ++buildNum);
+        createAndWaitforPatchset(gerritServer, project, ++buildNum);
+
+        EventTimeSlice lastTimeStamp = gerritServer.getMissedEventsPlaybackManager().getServerTimestamp();
+
+        // now we force the plugin is supported to false...
+        stubFor(get(urlEqualTo("/plugins/" + GerritMissedEventsPlaybackManager.EVENTS_LOG_PLUGIN_NAME + "/"))
+                .willReturn(aResponse()
+                        .withStatus(HTTPERROR)
+                        .withHeader("Content-Type", "text/html")
+                        .withBody("error")));
+
+        // Wait for it to be picked up
+        Thread.sleep(WAITTIMEFOREVENTLOGDISABLE);
+
+        assertFalse(gerritServer.getMissedEventsPlaybackManager().isSupported());
+
+        createAndWaitforPatchset(gerritServer, project, ++buildNum);
+        createAndWaitforPatchset(gerritServer, project, ++buildNum);
+
+        Long newTimestamp = lastTimeStamp.getTimeSlice() + TIMESTAMPDELTA;
+
+        gerritServer.stopConnection();
+        Thread.sleep(SLEEPTIME);
+
+        // now we re-enable feature:
+        stubFor(get(urlEqualTo("/plugins/" + GerritMissedEventsPlaybackManager.EVENTS_LOG_PLUGIN_NAME + "/"))
+                .willReturn(aResponse()
+                        .withStatus(HTTPOK)
+                        .withHeader("Content-Type", "text/html")
+                        .withBody("ok")));
+
+        String json = "{\"type\":\"patchset-created\",\"change\":{\"project\":\""
+                + project.getName()
+                + "\",\"branch\":\"develop\","
+                + "\"id\":\"Icae2322236e0e521950a0232effda08d6ffcdab7\",\"number\":\"392335\",\"subject\":\""
+                + "IPSEC: Small test code fixes due to Sonar warnings\",\"owner\":{\"name\":\"Szymon L\","
+                + "\"email\":\"szymon.l@abc.com\",\"username\":\"eszyabc\"},\"url\":"
+                + "\"https://abc.aaa.se/392335\","
+                + "\"commitMessage\":\"IPSEC: Small test code fixes due to Sonar warnings\\n\\nChange-Id: "
+                + "Icae2322236e0e521950a0232effda08d6ffcdab7\\nSigned-off-by:Szymon L \\u003cszymo"
+                + "n.l@abc.com\\u003e\\n\",\"status\":\"NEW\"},\"patchSet\":{\"number\":\"2\",\"revision"
+                + "\":\"607eea8f472235b3ee47483b630003250764dab2\",\"parents\":"
+                + "[\"87c0e57d2497ab334584ec9d1a7953ebcf016e10\"],"
+                + "\"ref\":\"refs/changes/35/392335/2\",\"uploader\":{\"name\":\"Szymon L\",\"email\":"
+                + "\"szymon"
+                + "@abc.com\",\"username\":\"eszyabc\"},\"createdOn\":1413448337,\"author\":{\"name\":\"Szy"
+                + "mon L\",\"email\":\"szymon.l@abc.com\",\"username\":\"eszyabc\"},\"isDraft\""
+                + ":false,\""
+                + "sizeInsertions\":6,\"sizeDeletions\":-7},\"author\":{\"name\":\"Build user for \","
+                + "\"email\":\"tnbuilder@"
+                + "abc.se\",\"username\":\"tnabc\"},\"approvals\":[{\"type\":\"Verified\""
+                + ",\"description\":"
+                + "\"Verified\",\"value\":\"-1\"}],\"comment\":\"Patch Set 2: Verified-1\\n\\nBuild Failed \\n\\nhttp:"
+                + "//jenkins/tn/job/tn-review/22579/ : FAILURE\",\"eventCreatedOn\":" + newTimestamp.toString() + "}\n";
+
+        stubFor(get(urlMatching(GerritMissedEventsPlaybackManagerTest.EVENTS_LOG_CHANGE_EVENTS_URL_REGEXP))
+                .willReturn(aResponse()
+                        .withStatus(HTTPOK)
+                        .withHeader("Content-Type", "text/html")
+                        .withBody(json)));
+
+        gerritServer.restartConnection();
+
+        Thread.sleep(LONGSLEEPTIME);
+        assertEquals(buildNum, project.getLastCompletedBuild().getNumber());
+
+    }
     /**
      * Test the scenario whereby connection is restarted and events are missed
      * but replayed. This simulates when REST API is enabled and connection is restarted.
@@ -190,14 +298,15 @@ public void testRestartWithRESTApiChangeMissedEvents() throws Exception {
     }
 
     /**
-     * Helper method to test the scenarios whereby connection is restarted and events are missed
-     * but replayed.
+     * Helper method to create patchset and wait for associated build.
      * @param gServer configured Gerrit Server.
-     * @param projectName Project name.
+     * @param project Project.
+     * @param buildNumberToWaitFor build number to wait for.
      * @throws Exception Error creating job.
      */
-    private void restartWithMissedEvents(GerritServer gServer, String projectName) throws Exception {
-        FreeStyleProject project = DuplicatesUtil.createGerritTriggeredJob(j, projectName, gServer.getName());
+    private void createAndWaitforPatchset(GerritServer gServer,
+                                          FreeStyleProject project,
+                                          int buildNumberToWaitFor) throws Exception {
 
         GerritTriggerApi api = new GerritTriggerApi();
         Handler handler = null;
@@ -208,7 +317,19 @@ private void restartWithMissedEvents(GerritServer gServer, String projectName) t
         }
         assertNotNull(handler);
         handler.post(Setup.createPatchsetCreated(gServer.getName()));
-        TestUtils.waitForBuilds(project, 1);
+        TestUtils.waitForBuilds(project, buildNumberToWaitFor);
+
+    }
+    /**
+     * Helper method to test the scenarios whereby connection is restarted
+     * and events are missed but replayed.
+     * @param gServer configured Gerrit Server.
+     * @param projectName Project name.
+     * @throws Exception Error creating job.
+     */
+    private void restartWithMissedEvents(GerritServer gServer, String projectName) throws Exception {
+        FreeStyleProject project = DuplicatesUtil.createGerritTriggeredJob(j, projectName, gServer.getName());
+        createAndWaitforPatchset(gServer, project, 1);
 
         assertNotNull(gServer.getMissedEventsPlaybackManager().getServerTimestamp());
         FreeStyleBuild buildOne = project.getLastCompletedBuild();