core: fix lastEventId tracking

due to singleton usage we may be reading wrong lastEventId, but it is not really needed so let's siplify the logic and use the existing AnsibleReturnValue for keeping track of it.
oVirt · Jul 21, 2022 · 39908a8 · 39908a8
1 parent 596d12d
commit 39908a8
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 19 deletions.
diff --git a/...modules/bll/src/main/java/org/ovirt/engine/core/common/utils/ansible/AnsibleExecutor.java b/...modules/bll/src/main/java/org/ovirt/engine/core/common/utils/ansible/AnsibleExecutor.java
@@ -147,7 +147,6 @@ public AnsibleReturnValue runCommand(AnsibleCommandConfig commandConfig, int tim
         }
 
         AnsibleReturnValue ret = new AnsibleReturnValue(AnsibleReturnCode.ERROR);
-        int lastEventId = 0;
 
         String playUuid = null;
         String msg = "";
@@ -158,6 +157,7 @@ public AnsibleReturnValue runCommand(AnsibleCommandConfig commandConfig, int tim
             ret.setLogFile(runnerClient.getLogger().getLogFile());
             ret.setPlayUuid(playUuid);
             ret.setStdout(String.format("%1$s/%2$s/artifacts/%2$s/stdout", AnsibleConstants.ANSIBLE_RUNNER_PATH, playUuid));
+            ret.setLastEventId(0);
             List<String> command = commandConfig.build();
 
             // Run the playbook:
@@ -168,7 +168,7 @@ public AnsibleReturnValue runCommand(AnsibleCommandConfig commandConfig, int tim
                 return ret;
             }
 
-            ret = runnerClient.artifactHandler(commandConfig.getUuid(), lastEventId, timeout, fn);
+            ret = runnerClient.artifactHandler(commandConfig.getUuid(), ret.getLastEventId(), timeout, fn);
         } catch (InventoryException ex) {
             String message = ex.getMessage();
             log.error("Error executing playbook: {}", message);
@@ -182,8 +182,7 @@ public AnsibleReturnValue runCommand(AnsibleCommandConfig commandConfig, int tim
         } finally {
             // Make sure all events are proccessed even in case of failure:
             if (playUuid != null && runnerClient != null && !async) {
-                lastEventId = runnerClient.getLastEventId();
-                runnerClient.processEvents(playUuid, lastEventId, fn, msg, ret.getLogFile());
+                runnerClient.processEvents(playUuid, ret.getLastEventId(), fn, msg, ret.getLogFile());
             }
         }
 

diff --git a/...les/bll/src/main/java/org/ovirt/engine/core/common/utils/ansible/AnsibleRunnerClient.java b/...les/bll/src/main/java/org/ovirt/engine/core/common/utils/ansible/AnsibleRunnerClient.java
@@ -40,7 +40,6 @@ public class AnsibleRunnerClient {
     private static Logger log = LoggerFactory.getLogger(AnsibleRunnerClient.class);
     private ObjectMapper mapper;
     private AnsibleRunnerLogger runnerLogger;
-    private String lastEvent = "";
     private static final int POLL_INTERVAL = 3000;
     private AnsibleReturnValue returnValue;
 
@@ -53,8 +52,12 @@ public AnsibleRunnerClient() {
         this.returnValue = new AnsibleReturnValue(AnsibleReturnCode.ERROR);
     }
 
-    public Boolean playHasEnded(UUID uuid) {
-        String jobEvents = getJobEventsDir(uuid.toString());
+    public Boolean playHasEnded(String uuid, int lastEventId) {
+        String lastEvent = getEventFileName(uuid, lastEventId);
+        if (lastEvent == null) {
+            return false;
+        }
+        String jobEvents = getJobEventsDir(uuid);
         File lastEventFile = new File(jobEvents + lastEvent);
         String res = "";
         try {
@@ -69,7 +72,7 @@ public AnsibleReturnValue artifactHandler(UUID uuid, int lastEventID, int timeou
             throws Exception {
         int executionTime = 0;
         setReturnValue(uuid);
-        while (!playHasEnded(uuid)) {
+        while (!playHasEnded(uuid.toString(), lastEventID)) {
             lastEventID = processEvents(uuid.toString(), lastEventID, fn, "", Paths.get(""));
             if (lastEventID == -1) {
                 return returnValue;
@@ -93,7 +96,7 @@ public void setReturnValue(UUID uuid) {
         returnValue.setStdout(Paths.get(this.getJobEventsDir(uuid.toString()), "../stdout").toString());
     }
 
-    public String getNextEvent(String playUuid, int lastEventId) {
+    public String getEventFileName(String playUuid, int eventId) {
         String jobEvents = getJobEventsDir(playUuid);
         if (!Files.exists(Paths.get(jobEvents))) {
             return null;
@@ -103,15 +106,11 @@ public String getNextEvent(String playUuid, int lastEventId) {
                 .map(File::getName)
                 .filter(item -> !item.contains("partial"))
                 .filter(item -> !item.endsWith(".tmp"))
-                .filter(item -> item.startsWith((lastEventId + 1) + "-"))
+                .filter(item -> item.startsWith(eventId + "-"))
                 .findFirst()
                 .orElse(null);
     }
 
-    public int getLastEventId() {
-        return Integer.valueOf(lastEvent.split("-")[0]);
-    }
-
     public String getJobEventsDir(String playUuid) {
         return String.format("%1$s/%2$s/artifacts/%2$s/job_events/", AnsibleConstants.ANSIBLE_RUNNER_PATH, playUuid);
     }
@@ -123,7 +122,8 @@ public int processEvents(String playUuid,
             Path logFile) {
         String jobEvents = getJobEventsDir(playUuid);
         while(true){
-            String event = getNextEvent(playUuid, lastEventId);
+            // get next event
+            String event = getEventFileName(playUuid, lastEventId + 1);
             if (event == null) {
                 break;
             }
@@ -187,11 +187,10 @@ public int processEvents(String playUuid,
                     }
                 }
             }
-            lastEvent = event;
-            returnValue.setLastEventId(getLastEventId());
-            lastEventId++;
+            lastEventId = Integer.valueOf(event.split("-")[0]);
+            returnValue.setLastEventId(lastEventId);
         }
-        return lastEvent.isEmpty() ? lastEventId : getLastEventId();
+        return lastEventId;
     }
 
     private Boolean jsonIsValid(String content) {