[Tests] Fix copying files for test cluster (#124628)

In case when file with `.attach_pid` in name was stored in distribution
and then deleted, the exception could stop copying/linking files
without any sign of issue. The files were then missing in the cluster
used in the test causing them sometimes to fail (depending on which
files haven't been copied).

When using `Files.walk` it is impossible to catch the IOException and
continue walking through files conditionally. It has been replaced with
FileVisitor implementation to be able to continue if the exception is
caused by files left temporarily by JVM but no longer available.

build-tools/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java

@@ -61,11 +61,14 @@ import java.io.StringWriter;
 import java.io.UncheckedIOException;
 import java.net.URL;
 import java.nio.charset.StandardCharsets;
+import java.nio.file.FileVisitResult;
 import java.nio.file.Files;
 import java.nio.file.NoSuchFileException;
 import java.nio.file.Path;
+import java.nio.file.SimpleFileVisitor;
 import java.nio.file.StandardCopyOption;
 import java.nio.file.StandardOpenOption;
+import java.nio.file.attribute.BasicFileAttributes;
 import java.time.Instant;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -1295,40 +1298,47 @@ public class ElasticsearchNode implements TestClusterConfiguration {
 
     private void sync(Path sourceRoot, Path destinationRoot, BiConsumer<Path, Path> syncMethod) {
         assert Files.exists(destinationRoot) == false;
-        try (Stream<Path> stream = Files.walk(sourceRoot)) {
+        try {
-            stream.forEach(source -> {
+            Files.walkFileTree(sourceRoot, new SimpleFileVisitor<>() {
-                Path relativeDestination = sourceRoot.relativize(source);
+                @Override
-                if (relativeDestination.getNameCount() <= 1) {
+                public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) throws IOException {
-                    return;
+                    Path relativeDestination = sourceRoot.relativize(dir);
+                    if (relativeDestination.getNameCount() <= 1) {
+                        return FileVisitResult.CONTINUE;
+                    }
+                    // Throw away the first name as the archives have everything in a single top level folder we are not interested in
+                    relativeDestination = relativeDestination.subpath(1, relativeDestination.getNameCount());
+                    Path destination = destinationRoot.resolve(relativeDestination);
+                    Files.createDirectories(destination);
+                    return FileVisitResult.CONTINUE;
                 }
-                // Throw away the first name as the archives have everything in a single top level folder we are not interested in
-                relativeDestination = relativeDestination.subpath(1, relativeDestination.getNameCount());
 
-                Path destination = destinationRoot.resolve(relativeDestination);
+                @Override
-                if (Files.isDirectory(source)) {
+                public FileVisitResult visitFile(Path source, BasicFileAttributes attrs) throws IOException {
-                    try {
+                    Path relativeDestination = sourceRoot.relativize(source);
-                        Files.createDirectories(destination);
+                    if (relativeDestination.getNameCount() <= 1) {
-                    } catch (IOException e) {
+                        return FileVisitResult.CONTINUE;
-                        throw new UncheckedIOException("Can't create directory " + destination.getParent(), e);
-                    }
-                } else {
-                    try {
-                        Files.createDirectories(destination.getParent());
-                    } catch (IOException e) {
-                        throw new UncheckedIOException("Can't create directory " + destination.getParent(), e);
                     }
+                    // Throw away the first name as the archives have everything in a single top level folder we are not interested in
+                    relativeDestination = relativeDestination.subpath(1, relativeDestination.getNameCount());
+                    Path destination = destinationRoot.resolve(relativeDestination);
+                    Files.createDirectories(destination.getParent());
                     syncMethod.accept(destination, source);
+                    return FileVisitResult.CONTINUE;
+                }
+
+                @Override
+                public FileVisitResult visitFileFailed(Path file, IOException exc) throws IOException {
+                    if (exc instanceof NoSuchFileException noFileException) {
+                        // Ignore these files that are sometimes left behind by the JVM
+                        if (noFileException.getFile() != null && noFileException.getFile().contains(".attach_pid")) {
+                            LOGGER.info("Ignoring file left behind by JVM: {}", noFileException.getFile());
+                            return FileVisitResult.CONTINUE;
+                        }
+                    }
+                    throw exc;
                 }
             });
-        } catch (UncheckedIOException e) {
-            if (e.getCause() instanceof NoSuchFileException cause) {
-                // Ignore these files that are sometimes left behind by the JVM
-                if (cause.getFile() == null || cause.getFile().contains(".attach_pid") == false) {
-                    throw new UncheckedIOException(cause);
-                }
-            } else {
-                throw e;
-            }
         } catch (IOException e) {
             throw new UncheckedIOException("Can't walk source " + sourceRoot, e);
         }

test/test-clusters/build.gradle

@@ -9,6 +9,10 @@ dependencies {
   implementation "com.fasterxml.jackson.core:jackson-annotations:${versions.jackson}"
   implementation "com.fasterxml.jackson.core:jackson-databind:${versions.jackson}"
   implementation "org.elasticsearch.gradle:reaper"
+
+  testImplementation "junit:junit:${versions.junit}"
+  testImplementation "org.hamcrest:hamcrest:${versions.hamcrest}"
+  testImplementation "org.apache.logging.log4j:log4j-core:${versions.log4j}"
 }
 
 tasks.named("processResources").configure {

test/test-clusters/src/main/java/org/elasticsearch/test/cluster/util/IOUtils.java

@@ -15,9 +15,12 @@ import org.apache.logging.log4j.Logger;
 import java.io.File;
 import java.io.IOException;
 import java.io.UncheckedIOException;
+import java.nio.file.FileVisitResult;
 import java.nio.file.Files;
 import java.nio.file.NoSuchFileException;
 import java.nio.file.Path;
+import java.nio.file.SimpleFileVisitor;
+import java.nio.file.attribute.BasicFileAttributes;
 import java.util.Comparator;
 import java.util.function.BiConsumer;
 import java.util.stream.Stream;
@@ -118,35 +121,37 @@ public final class IOUtils {
 
     private static void sync(Path sourceRoot, Path destinationRoot, BiConsumer<Path, Path> syncMethod) {
         assert Files.exists(destinationRoot) == false;
-        try (Stream<Path> stream = Files.walk(sourceRoot)) {
+        try {
-            stream.forEach(source -> {
+            Files.walkFileTree(sourceRoot, new SimpleFileVisitor<>() {
-                Path relativeDestination = sourceRoot.relativize(source);
+                @Override
+                public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) throws IOException {
+                    Path relativeDestination = sourceRoot.relativize(dir);
+                    Path destination = destinationRoot.resolve(relativeDestination);
+                    Files.createDirectories(destination);
+                    return FileVisitResult.CONTINUE;
+                }
 
-                Path destination = destinationRoot.resolve(relativeDestination);
+                @Override
-                if (Files.isDirectory(source)) {
+                public FileVisitResult visitFile(Path source, BasicFileAttributes attrs) throws IOException {
-                    try {
+                    Path relativeDestination = sourceRoot.relativize(source);
-                        Files.createDirectories(destination);
+                    Path destination = destinationRoot.resolve(relativeDestination);
-                    } catch (IOException e) {
+                    Files.createDirectories(destination.getParent());
-                        throw new UncheckedIOException("Can't create directory " + destination.getParent(), e);
-                    }
-                } else {
-                    try {
-                        Files.createDirectories(destination.getParent());
-                    } catch (IOException e) {
-                        throw new UncheckedIOException("Can't create directory " + destination.getParent(), e);
-                    }
                     syncMethod.accept(destination, source);
+                    return FileVisitResult.CONTINUE;
+                }
+
+                @Override
+                public FileVisitResult visitFileFailed(Path file, IOException exc) throws IOException {
+                    if (exc instanceof NoSuchFileException noFileException) {
+                        // Ignore these files that are sometimes left behind by the JVM
+                        if (noFileException.getFile() != null && noFileException.getFile().contains(".attach_pid")) {
+                            LOGGER.info("Ignoring file left behind by JVM: {}", noFileException.getFile());
+                            return FileVisitResult.CONTINUE;
+                        }
+                    }
+                    throw exc;
                 }
             });
-        } catch (UncheckedIOException e) {
-            if (e.getCause() instanceof NoSuchFileException cause) {
-                // Ignore these files that are sometimes left behind by the JVM
-                if (cause.getFile() == null || cause.getFile().contains(".attach_pid") == false) {
-                    throw new UncheckedIOException(cause);
-                }
-            } else {
-                throw e;
-            }
         } catch (IOException e) {
             throw new UncheckedIOException("Can't walk source " + sourceRoot, e);
         }

test/test-clusters/src/test/java/org/elasticsearch/test/cluster/util/IOUtilsTests.java

@@ -0,0 +1,75 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the "Elastic License
+ * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
+ * Public License v 1"; you may not use this file except in compliance with, at
+ * your election, the "Elastic License 2.0", the "GNU Affero General Public
+ * License v3.0 only", or the "Server Side Public License, v 1".
+ */
+
+package org.elasticsearch.test.cluster.util;
+
+import org.junit.Test;
+
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.core.Is.is;
+import static org.hamcrest.core.Is.isA;
+import static org.junit.Assert.assertThrows;
+
+public class IOUtilsTests {
+
+    @Test
+    public void testSyncWithLinks() throws IOException {
+        // given
+        Path sourceDir = Files.createTempDirectory("sourceDir");
+        Files.createFile(sourceDir.resolve("file1.txt"));
+        Files.createFile(sourceDir.resolve("file2.txt"));
+        Files.createDirectory(sourceDir.resolve("nestedDir"));
+        Files.createFile(sourceDir.resolve("nestedDir").resolve("file3.txt"));
+
+        Path baseDestinationDir = Files.createTempDirectory("baseDestinationDir");
+        Path destinationDir = baseDestinationDir.resolve("destinationDir");
+
+        // when
+        IOUtils.syncWithLinks(sourceDir, destinationDir);
+
+        // then
+        assertFileExists(destinationDir.resolve("file1.txt"));
+        assertFileExists(destinationDir.resolve("file2.txt"));
+        assertFileExists(destinationDir.resolve("nestedDir").resolve("file3.txt"));
+    }
+
+    private void assertFileExists(Path path) throws IOException {
+        assertThat("File " + path + " doesn't exist", Files.exists(path), is(true));
+        assertThat("File " + path + " is not a regular file", Files.isRegularFile(path), is(true));
+        assertThat("File " + path + " is not readable", Files.isReadable(path), is(true));
+        if (OS.current() != OS.WINDOWS) {
+            assertThat("Expected 2 hard links", Files.getAttribute(path, "unix:nlink"), is(2));
+        }
+    }
+
+    @Test
+    public void testSyncWithLinksThrowExceptionWhenDestinationIsNotWritable() throws IOException {
+        // given
+        Path sourceDir = Files.createTempDirectory("sourceDir");
+        Files.createFile(sourceDir.resolve("file1.txt"));
+
+        Path baseDestinationDir = Files.createTempDirectory("baseDestinationDir");
+        Path destinationDir = baseDestinationDir.resolve("destinationDir");
+
+        baseDestinationDir.toFile().setWritable(false);
+
+        // when
+        UncheckedIOException ex = assertThrows(UncheckedIOException.class, () -> IOUtils.syncWithLinks(sourceDir, destinationDir));
+
+        // then
+        assertThat(ex.getCause(), isA(IOException.class));
+        assertThat(ex.getCause().getMessage(), containsString("destinationDir"));
+    }
+}