From 11446494f28594a7a704982b90543d770ffd17a4 Mon Sep 17 00:00:00 2001 From: Christoph Deppisch Date: Thu, 19 Dec 2024 09:09:37 +0100 Subject: [PATCH] fix: Make sure to use current pid to verify Camel app status - Auto adjust the pid that identifies the running Camel JBang application - Camel JBang pid may change on Unix OS due to descendant process being launched - Racing conditions may lead to parent pid being stored in test context - Make sure to also search for descendant process ids when verifying the Camel integration app status --- .../jbang/ProcessAndOutput.java | 22 ++++++++++- .../jbang/actions/JBangAction.java | 6 ++- .../actions/CamelRunIntegrationAction.java | 38 ++++++------------- .../actions/CamelVerifyIntegrationAction.java | 34 +++++++++++++---- 4 files changed, 63 insertions(+), 37 deletions(-) diff --git a/connectors/citrus-jbang-connector/src/main/java/org/citrusframework/jbang/ProcessAndOutput.java b/connectors/citrus-jbang-connector/src/main/java/org/citrusframework/jbang/ProcessAndOutput.java index 636463930f..598d139630 100644 --- a/connectors/citrus-jbang-connector/src/main/java/org/citrusframework/jbang/ProcessAndOutput.java +++ b/connectors/citrus-jbang-connector/src/main/java/org/citrusframework/jbang/ProcessAndOutput.java @@ -44,6 +44,8 @@ public class ProcessAndOutput { private BufferedReader reader; + private String app; + ProcessAndOutput(Process process) { this(process, ""); } @@ -134,6 +136,16 @@ private void readChunk() { } } + /** + * Get the process id of first descendant or the parent process itself in case there is no descendant process. + * On Linux the shell command represents the parent process and the JBang command as descendant process. + * Typically, we need the JBang command process id. + * @return + */ + public Long getProcessId() { + return getProcessId(app); + } + /** * Get the process id of first descendant or the parent process itself in case there is no descendant process. * On Linux the shell command represents the parent process and the JBang command as descendant process. @@ -142,7 +154,7 @@ private void readChunk() { */ public Long getProcessId(String app) { try { - if (isUnix()) { + if (app != null && isUnix()) { // wait for descendant process to be available await().atMost(5000L, TimeUnit.MILLISECONDS) .until(() -> process.descendants().findAny().isPresent()); @@ -165,4 +177,12 @@ private static boolean isUnix() { String os = System.getProperty("os.name").toLowerCase(); return os.contains("nix") || os.contains("nux") || os.contains("aix"); } + + /** + * Sets the application name that identifies this process. + * @param app + */ + public void setApp(String app) { + this.app = app; + } } diff --git a/connectors/citrus-jbang-connector/src/main/java/org/citrusframework/jbang/actions/JBangAction.java b/connectors/citrus-jbang-connector/src/main/java/org/citrusframework/jbang/actions/JBangAction.java index 59d754d9b4..b7db11fdad 100644 --- a/connectors/citrus-jbang-connector/src/main/java/org/citrusframework/jbang/actions/JBangAction.java +++ b/connectors/citrus-jbang-connector/src/main/java/org/citrusframework/jbang/actions/JBangAction.java @@ -83,13 +83,15 @@ public void doExecute(TestContext context) { .withSystemProperties(systemProperties) .run(context.replaceDynamicContentInString(scriptOrFile), context.resolveDynamicValuesInList(args)); + result.setApp(Objects.requireNonNullElse(app, scriptName)); + if (printOutput) { logger.info("JBang script '%s' output:".formatted(scriptName)); logger.info(result.getOutput()); } if (pidVar != null) { - context.setVariable(pidVar, result.getProcessId(Objects.requireNonNullElse(app, scriptName))); + context.setVariable(pidVar, result.getProcessId()); } int exitValue = result.getProcess().exitValue(); @@ -105,7 +107,7 @@ public void doExecute(TestContext context) { if (validationProcessor != null) { validationProcessor.validate(new DefaultMessage(result.getOutput().trim()) - .setHeader("pid", result.getProcessId(Objects.requireNonNullElse(app, scriptName))) + .setHeader("pid", result.getProcessId()) .setHeader("exitCode", result.getProcess().exitValue()), context); } diff --git a/endpoints/citrus-camel/src/main/java/org/citrusframework/camel/actions/CamelRunIntegrationAction.java b/endpoints/citrus-camel/src/main/java/org/citrusframework/camel/actions/CamelRunIntegrationAction.java index b1ed1f00db..13c36aada2 100644 --- a/endpoints/citrus-camel/src/main/java/org/citrusframework/camel/actions/CamelRunIntegrationAction.java +++ b/endpoints/citrus-camel/src/main/java/org/citrusframework/camel/actions/CamelRunIntegrationAction.java @@ -16,11 +16,9 @@ package org.citrusframework.camel.actions; -import java.io.File; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; -import java.nio.file.Paths; import java.nio.file.StandardOpenOption; import java.util.ArrayList; import java.util.Arrays; @@ -29,7 +27,6 @@ import java.util.Map; import java.util.Properties; -import org.citrusframework.camel.CamelSettings; import org.citrusframework.camel.jbang.CamelJBangSettings; import org.citrusframework.context.TestContext; import org.citrusframework.exceptions.CitrusRuntimeException; @@ -124,30 +121,10 @@ public void doExecute(TestContext context) { ProcessAndOutput pao = camelJBang().run(name, integrationToRun, resourceFiles, args.toArray(String[]::new)); - if (!pao.getProcess().isAlive()) { - logger.info("Failed to start Camel integration '%s'".formatted(name)); - logger.info(pao.getOutput()); + verifyProcessIsAlive(pao, name); - throw new CitrusRuntimeException(String.format("Failed to start Camel integration - exit code %s", pao.getProcess().exitValue())); - } - - // Wait for first log to unsure the run process is correctly launched before setting the pid in context - int maxAttempts = CamelSettings.getMaxAttempts(); - long delayBetweenAttempts = CamelSettings.getDelayBetweenAttempts(); - for (int i = 0; i < maxAttempts; i++) { - String log = pao.getOutput(); - if (log.length() > 0) { - break; - } - - try { - Thread.sleep(delayBetweenAttempts); - } catch (InterruptedException e) { - logger.warn("Interrupted while waiting for Camel integration '%s' first log".formatted(name), e); - } - } - - Long pid = pao.getProcessId(integrationToRun.getFileName().toString()); + pao.setApp(integrationToRun.getFileName().toString()); + Long pid = pao.getProcessId(); context.setVariable(name + ":pid", pid); context.setVariable(name + ":process:" + pid, pao); @@ -174,6 +151,15 @@ public void doExecute(TestContext context) { } } + private static void verifyProcessIsAlive(ProcessAndOutput pao, String name) { + if (!pao.getProcess().isAlive()) { + logger.info("Failed to start Camel integration '%s'".formatted(name)); + logger.info(pao.getOutput()); + + throw new CitrusRuntimeException(String.format("Failed to start Camel integration - exit code %s", pao.getProcess().exitValue())); + } + } + private String getFileExt(String sourceCode) { if (IsXmlPredicate.getInstance().test(sourceCode)) { return "xml"; diff --git a/endpoints/citrus-camel/src/main/java/org/citrusframework/camel/actions/CamelVerifyIntegrationAction.java b/endpoints/citrus-camel/src/main/java/org/citrusframework/camel/actions/CamelVerifyIntegrationAction.java index 9ea5efe286..0304d7f61e 100644 --- a/endpoints/citrus-camel/src/main/java/org/citrusframework/camel/actions/CamelVerifyIntegrationAction.java +++ b/endpoints/citrus-camel/src/main/java/org/citrusframework/camel/actions/CamelVerifyIntegrationAction.java @@ -17,6 +17,7 @@ package org.citrusframework.camel.actions; import java.util.Map; +import java.util.Objects; import org.citrusframework.camel.CamelSettings; import org.citrusframework.context.TestContext; @@ -124,15 +125,8 @@ private Long verifyRouteStatus(String name, String phase, TestContext context) { for (int i = 0; i < maxAttempts; i++) { if (context.getVariables().containsKey(name + ":pid")) { Long pid = context.getVariable(name + ":pid", Long.class); - Map properties = camelJBang().get(pid); - if ((phase.equals("Stopped") && properties.isEmpty()) || (!properties.isEmpty() && properties.get("STATUS").equals(phase))) { - logger.info(String.format("Verified Camel integration '%s' state '%s' - All values OK!", name, phase)); + if (findProcessAndVerifyStatus(pid, name, phase)) { return pid; - } else if (phase.equals("Error")) { - logger.info(String.format("Camel integration '%s' is in state 'Error'", name)); - if (stopOnErrorStatus) { - throw new CitrusRuntimeException(String.format("Failed to verify Camel integration '%s' - is in state 'Error'", name)); - } } if (context.getVariables().containsKey(name + ":process:" + pid)) { @@ -144,6 +138,15 @@ private Long verifyRouteStatus(String name, String phase, TestContext context) { throw new CitrusRuntimeException(String.format("Failed to verify Camel integration '%s' - exit code %s", name, pao.getProcess().exitValue())); } + + // Verify that current processId is the same as the one saved in test context + Long appPid = pao.getProcessId(); + if (!Objects.equals(pid, appPid)) { + // seems like there is another pid (descendant process) that should be verified + if (findProcessAndVerifyStatus(appPid, name, phase)) { + return appPid; + } + } } } @@ -162,6 +165,21 @@ private Long verifyRouteStatus(String name, String phase, TestContext context) { } + private boolean findProcessAndVerifyStatus(Long pid, String name, String phase) { + Map properties = camelJBang().get(pid); + if ((phase.equals("Stopped") && properties.isEmpty()) || (!properties.isEmpty() && properties.get("STATUS").equals(phase))) { + logger.info(String.format("Verified Camel integration '%s' state '%s' - All values OK!", name, phase)); + return true; + } else if (properties.getOrDefault("STATUS", "").equals("Error")) { + logger.info(String.format("Camel integration '%s' is in state 'Error'", name)); + if (stopOnErrorStatus) { + throw new CitrusRuntimeException(String.format("Failed to verify Camel integration '%s' - is in state 'Error'", name)); + } + } + + return false; + } + public String getIntegrationName() { return integrationName; }