Skip to content

Commit 371c545

Browse files
committed
fix race condition, now each instance uses its own tmp directory
1 parent 23cdf4b commit 371c545

1 file changed

Lines changed: 50 additions & 22 deletions

File tree

src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java

Lines changed: 50 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,12 @@ public class AnsibleRunnerContextBuilder {
4242
private final Map<String, Object> jobConf;
4343
private final Collection<INodeEntry> nodes;
4444
private final Collection<File> tempFiles;
45+
/**
46+
* Per-builder working directory (e.g. {@code ansible-exec-<executionId>-builder-<rand>/})
47+
* created atomically with
48+
* {@link Files#createTempDirectory(java.nio.file.Path, String, java.nio.file.attribute.FileAttribute[])}.
49+
* EXCLUSIVELY owned by this builder; safe to delete recursively.
50+
*/
4551
private File executionSpecificDir;
4652

4753
private AnsiblePluginGroup pluginGroup;
@@ -827,20 +833,25 @@ public void cleanupTempFiles() {
827833
}
828834
tempFiles.clear();
829835

830-
// Clean up execution-specific directory (including group_vars when inventory was generated/inline)
831-
// Note: group_vars created alongside user-provided inventory files is cleaned up by AnsibleRunner
836+
// Clean up the builder-specific working directory (including group_vars when inventory
837+
// was generated/inline). Note: group_vars created alongside user-provided inventory files
838+
// is cleaned up by AnsibleRunner.
839+
// executionSpecificDir is an EXCLUSIVE per-builder directory (ansible-exec-<execId>-builder-<rand>/),
840+
// so recursive deletion is always safe — no sibling builder shares files with it.
832841
if (executionSpecificDir != null && executionSpecificDir.exists()) {
833842
if (!getDebug()) {
834-
log.debug("Cleaning up execution-specific directory: {}", executionSpecificDir.getAbsolutePath());
843+
log.debug("Cleaning up builder-specific directory: {}", executionSpecificDir.getAbsolutePath());
835844
if (!deleteDirectoryRecursively(executionSpecificDir)) {
836-
log.warn("Failed to completely delete execution-specific directory: {}", executionSpecificDir.getAbsolutePath());
845+
log.warn("Failed to completely delete builder-specific directory: {}", executionSpecificDir.getAbsolutePath());
837846
} else {
838-
log.debug("Successfully deleted execution-specific directory: {}", executionSpecificDir.getAbsolutePath());
847+
log.debug("Successfully deleted builder-specific directory: {}", executionSpecificDir.getAbsolutePath());
839848
}
840849
} else {
841-
// In debug mode, the execution-specific directory is intentionally preserved for troubleshooting.
842-
// Note: These directories will accumulate over time and may require periodic manual cleanup.
843-
log.debug("Debug mode enabled; not cleaning up execution-specific directory: {}", executionSpecificDir.getAbsolutePath());
850+
// In debug mode the per-builder directory is intentionally preserved for troubleshooting.
851+
// Sibling directories of the same Rundeck execution share the ansible-exec-<execId>-* prefix
852+
// for easy filtering. These directories will accumulate over time and may require periodic
853+
// manual cleanup.
854+
log.debug("Debug mode enabled; not cleaning up builder-specific directory: {}", executionSpecificDir.getAbsolutePath());
844855
}
845856
}
846857
}
@@ -1145,16 +1156,25 @@ public Boolean generateInventoryNodesAuth() {
11451156
}
11461157

11471158
/**
1148-
* Creates and returns an execution-specific temporary directory path.
1149-
* This ensures that each execution has its own isolated directory for inventory and group_vars,
1150-
* preventing conflicts when multiple workflow step executions run in parallel.
1159+
* Creates and returns a builder-specific temporary directory path.
1160+
*
1161+
* <p>Each builder receives a unique flat directory directly under the configured base tmp
1162+
* dir, named {@code ansible-exec-<executionId>-builder-<rand>/}. The directory is created
1163+
* atomically with {@link Files#createTempDirectory(java.nio.file.Path, String, java.nio.file.attribute.FileAttribute[])},
1164+
* which guarantees uniqueness without any coordination between sibling threads sharing the
1165+
* same Rundeck executionId.</p>
11511166
*
1152-
* @return The path to the execution-specific directory
1167+
* <p>The {@code ansible-exec-<executionId>} prefix preserves the ability to filter all
1168+
* directories belonging to a given Rundeck execution (useful in debug mode), while keeping
1169+
* each directory exclusively owned by a single builder. Recursive cleanup of one builder's
1170+
* directory cannot affect siblings.</p>
1171+
*
1172+
* @return The absolute path to the builder-specific directory
11531173
*/
11541174
String getExecutionSpecificTmpDir() {
11551175
// Return cached directory if already created
11561176
if (executionSpecificDir != null) {
1157-
log.debug("Using cached execution-specific directory: {}", executionSpecificDir.getAbsolutePath());
1177+
log.debug("Using cached builder-specific directory: {}", executionSpecificDir.getAbsolutePath());
11581178
return executionSpecificDir.getAbsolutePath();
11591179
}
11601180

@@ -1169,18 +1189,26 @@ String getExecutionSpecificTmpDir() {
11691189
// Get base tmp directory
11701190
String baseTmpDir = AnsibleUtil.getCustomTmpPathDir(framework);
11711191

1172-
// Create execution-specific directory
11731192
if (executionId != null && !executionId.isEmpty()) {
1174-
executionSpecificDir = new File(baseTmpDir, "ansible-exec-" + executionId);
1175-
log.debug("Creating execution-specific directory: {}", executionSpecificDir.getAbsolutePath());
1193+
java.nio.file.Path basePath = Paths.get(baseTmpDir);
1194+
try {
1195+
// createDirectories is idempotent: safe under concurrent invocations.
1196+
Files.createDirectories(basePath);
1197+
} catch (IOException e) {
1198+
String errorMsg = "Failed to ensure base tmp directory exists: " + basePath;
1199+
log.error(errorMsg, e);
1200+
throw new IllegalStateException(errorMsg, e);
1201+
}
1202+
1203+
// Atomic + unique by construction; the executionId prefix lets operators filter all
1204+
// directories belonging to a given Rundeck execution with a single glob.
11761205
try {
1177-
// Use Files.createDirectories() which is idempotent (safe to call if directory exists)
1178-
// and handles race conditions properly. Unlike mkdirs(), it doesn't return false
1179-
// when the directory already exists - it only throws IOException on actual failure.
1180-
Files.createDirectories(executionSpecificDir.toPath());
1181-
log.debug("Successfully ensured execution-specific directory exists: {}", executionSpecificDir.getAbsolutePath());
1206+
executionSpecificDir = Files
1207+
.createTempDirectory(basePath, "ansible-exec-" + executionId + "-builder-")
1208+
.toFile();
1209+
log.debug("Created builder-specific directory: {}", executionSpecificDir.getAbsolutePath());
11821210
} catch (IOException e) {
1183-
String errorMsg = "Failed to create execution-specific directory: " + executionSpecificDir.getAbsolutePath();
1211+
String errorMsg = "Failed to create builder-specific directory under: " + basePath;
11841212
log.error(errorMsg, e);
11851213
throw new IllegalStateException(errorMsg, e);
11861214
}

0 commit comments

Comments
 (0)