Browse Source

Consolidate temp dir handling in packaging tests (#58292)

The packaging tests currently have a couple different ways of deciding
where temp files should be placed, and then sometimes used fixed file
or directory names within that dir. This commit conslidates some of that
temp dir handling by making it more compatible with the handling that
exists within the bats tests, where /tmp is not always appropriate due
to how systemd interacts with it. This commit also adds a utility
methhod for creating temp dirs, so as to ensure the new directory is
created as if a umask of 022 were used, which is not the case when using
Files.createTempDirectory without a set of permissions (it assumes 077).
Ryan Ernst 5 years ago
parent
commit
9310e5a440

+ 4 - 5
qa/os/src/test/java/org/elasticsearch/packaging/test/ArchiveTests.java

@@ -41,7 +41,6 @@ import static org.elasticsearch.packaging.util.FileExistenceMatchers.fileDoesNot
 import static org.elasticsearch.packaging.util.FileExistenceMatchers.fileExists;
 import static org.elasticsearch.packaging.util.FileUtils.append;
 import static org.elasticsearch.packaging.util.FileUtils.cp;
-import static org.elasticsearch.packaging.util.FileUtils.getTempDir;
 import static org.elasticsearch.packaging.util.FileUtils.mkdir;
 import static org.elasticsearch.packaging.util.FileUtils.mv;
 import static org.elasticsearch.packaging.util.FileUtils.rm;
@@ -250,7 +249,7 @@ public class ArchiveTests extends PackagingTestCase {
 
     public void test70CustomPathConfAndJvmOptions() throws Exception {
 
-        final Path tempConf = getTempDir().resolve("esconf-alternate");
+        final Path tempConf = createTempDir("esconf-alternate");
 
         try {
             mkdir(tempConf);
@@ -339,7 +338,7 @@ public class ArchiveTests extends PackagingTestCase {
 
     public void test80RelativePathConf() throws Exception {
 
-        final Path temp = getTempDir().resolve("esconf-alternate");
+        final Path temp = createTempDir("esconf-alternate");
         final Path tempConf = temp.resolve("config");
 
         try {
@@ -420,7 +419,7 @@ public class ArchiveTests extends PackagingTestCase {
         Path relativeDataPath = installation.data.relativize(installation.home);
         append(installation.config("elasticsearch.yml"), "path.data: " + relativeDataPath);
 
-        sh.setWorkingDirectory(getTempDir());
+        sh.setWorkingDirectory(getRootTempDir());
 
         startElasticsearch();
         stopElasticsearch();
@@ -432,7 +431,7 @@ public class ArchiveTests extends PackagingTestCase {
     public void test94ElasticsearchNodeExecuteCliNotEsHomeWorkDir() throws Exception {
         final Installation.Executables bin = installation.executables();
         // Run the cli tools from the tmp dir
-        sh.setWorkingDirectory(getTempDir());
+        sh.setWorkingDirectory(getRootTempDir());
 
         Platforms.PlatformAction action = () -> {
             Result result = sh.run(bin.certutilTool + " -h");

+ 2 - 3
qa/os/src/test/java/org/elasticsearch/packaging/test/CertGenCliTests.java

@@ -40,13 +40,12 @@ import static org.elasticsearch.packaging.util.FileMatcher.Fileness.File;
 import static org.elasticsearch.packaging.util.FileMatcher.file;
 import static org.elasticsearch.packaging.util.FileMatcher.p600;
 import static org.elasticsearch.packaging.util.FileUtils.escapePath;
-import static org.elasticsearch.packaging.util.FileUtils.getTempDir;
 import static org.hamcrest.CoreMatchers.containsString;
 import static org.junit.Assume.assumeTrue;
 
 public class CertGenCliTests extends PackagingTestCase {
-    private static final Path instancesFile = getTempDir().resolve("instances.yml");
-    private static final Path certificatesFile = getTempDir().resolve("certificates.zip");
+    private static final Path instancesFile = getRootTempDir().resolve("instances.yml");
+    private static final Path certificatesFile = getRootTempDir().resolve("certificates.zip");
 
     @Before
     public void filterDistros() {

+ 1 - 2
qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java

@@ -55,7 +55,6 @@ import static org.elasticsearch.packaging.util.FileMatcher.p600;
 import static org.elasticsearch.packaging.util.FileMatcher.p660;
 import static org.elasticsearch.packaging.util.FileMatcher.p775;
 import static org.elasticsearch.packaging.util.FileUtils.append;
-import static org.elasticsearch.packaging.util.FileUtils.getTempDir;
 import static org.elasticsearch.packaging.util.FileUtils.rm;
 import static org.hamcrest.Matchers.arrayWithSize;
 import static org.hamcrest.Matchers.containsString;
@@ -81,7 +80,7 @@ public class DockerTests extends PackagingTestCase {
     @Before
     public void setupTest() throws IOException {
         installation = runContainer(distribution());
-        tempDir = Files.createTempDirectory(getTempDir(), DockerTests.class.getSimpleName());
+        tempDir = createTempDir(DockerTests.class.getSimpleName());
     }
 
     @After

+ 2 - 3
qa/os/src/test/java/org/elasticsearch/packaging/test/KeystoreManagementTests.java

@@ -47,7 +47,6 @@ import static org.elasticsearch.packaging.util.FileMatcher.Fileness.File;
 import static org.elasticsearch.packaging.util.FileMatcher.file;
 import static org.elasticsearch.packaging.util.FileMatcher.p600;
 import static org.elasticsearch.packaging.util.FileMatcher.p660;
-import static org.elasticsearch.packaging.util.FileUtils.getTempDir;
 import static org.elasticsearch.packaging.util.FileUtils.rm;
 import static org.elasticsearch.packaging.util.Packages.assertInstalled;
 import static org.elasticsearch.packaging.util.Packages.assertRemoved;
@@ -295,7 +294,7 @@ public class KeystoreManagementTests extends PackagingTestCase {
 
         Path tempDir = null;
         try {
-            tempDir = Files.createTempDirectory(getTempDir(), DockerTests.class.getSimpleName());
+            tempDir = createTempDir(DockerTests.class.getSimpleName());
 
             String password = "password";
             String passwordFilename = "password.txt";
@@ -349,7 +348,7 @@ public class KeystoreManagementTests extends PackagingTestCase {
     private Path getKeystoreFileFromDockerContainer(String password, Path dockerKeystore) throws IOException {
         // Mount a temporary directory for copying the keystore
         Path dockerTemp = Path.of("/usr/tmp/keystore-tmp");
-        Path tempDirectory = Files.createTempDirectory(getTempDir(), KeystoreManagementTests.class.getSimpleName());
+        Path tempDirectory = createTempDir(KeystoreManagementTests.class.getSimpleName());
         Map<Path, Path> volumes = Map.of(tempDirectory, dockerTemp);
 
         // It's very tricky to properly quote a pipeline that you're passing to

+ 42 - 9
qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java

@@ -47,9 +47,12 @@ import org.junit.rules.TestWatcher;
 import org.junit.runner.Description;
 import org.junit.runner.RunWith;
 
+import java.io.IOException;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
+import java.nio.file.attribute.FileAttribute;
+import java.nio.file.attribute.PosixFilePermissions;
 import java.util.Collections;
 import java.util.List;
 
@@ -112,18 +115,18 @@ public abstract class PackagingTestCase extends Assert {
     public final TestName testNameRule = new TestName();
 
     @BeforeClass
-    public static void filterCompatible() {
+    public static void init() throws Exception {
         assumeTrue("only compatible distributions", distribution.packaging.compatible);
-    }
 
-    @BeforeClass
-    public static void cleanup() throws Exception {
-        installation = null;
-        cleanEverything();
-    }
+        // make sure temp dir exists
+        if (Files.exists(getRootTempDir()) == false) {
+            Files.createDirectories(getRootTempDir());
+        }
 
-    @BeforeClass
-    public static void createShell() throws Exception {
+        // cleanup from previous test
+        cleanup();
+
+        // create shell
         if (distribution().isDocker()) {
             ensureImageIsLoaded(distribution);
             sh = new Docker.DockerShell();
@@ -201,6 +204,11 @@ public abstract class PackagingTestCase extends Assert {
         }
     }
 
+    protected static void cleanup() throws Exception {
+        installation = null;
+        cleanEverything();
+    }
+
     /**
      * Starts and stops elasticsearch, and performs assertions while it is running.
      */
@@ -359,4 +367,29 @@ public abstract class PackagingTestCase extends Assert {
         }
     }
 
+    public static Path getRootTempDir() {
+        if (distribution().isPackage()) {
+            // The custom config directory is not under /tmp or /var/tmp because
+            // systemd's private temp directory functionally means different
+            // processes can have different views of what's in these directories
+            return Paths.get("/var/test-tmp").toAbsolutePath();
+        } else {
+            // vagrant creates /tmp for us in windows so we use that to avoid long paths
+            return Paths.get("/tmp").toAbsolutePath();
+        }
+    }
+
+    private static final FileAttribute<?>[] NEW_DIR_PERMS;
+    static {
+        if (Platforms.WINDOWS) {
+            NEW_DIR_PERMS = new FileAttribute<?>[0];
+        } else {
+            NEW_DIR_PERMS = new FileAttribute<?>[] { PosixFilePermissions.asFileAttribute(PosixFilePermissions.fromString("rwxr-xr-x")) };
+        }
+    }
+
+    public static Path createTempDir(String prefix) throws IOException {
+        return Files.createTempDirectory(getRootTempDir(), prefix, NEW_DIR_PERMS);
+    }
+
 }

+ 2 - 2
qa/os/src/test/java/org/elasticsearch/packaging/util/Cleanup.java

@@ -27,7 +27,7 @@ import java.util.Collections;
 import java.util.List;
 import java.util.function.Consumer;
 
-import static org.elasticsearch.packaging.util.FileUtils.getTempDir;
+import static org.elasticsearch.packaging.test.PackagingTestCase.getRootTempDir;
 import static org.elasticsearch.packaging.util.FileUtils.lsGlob;
 import static org.elasticsearch.packaging.util.Platforms.isDPKG;
 import static org.elasticsearch.packaging.util.Platforms.isRPM;
@@ -82,7 +82,7 @@ public class Cleanup {
         // when we run es as a role user on windows, add the equivalent here
 
         // delete files that may still exist
-        lsGlob(getTempDir(), "elasticsearch*").forEach(FileUtils::rm);
+        lsGlob(getRootTempDir(), "elasticsearch*").forEach(FileUtils::rm);
         final List<String> filesToDelete = Platforms.WINDOWS ? ELASTICSEARCH_FILES_WINDOWS : ELASTICSEARCH_FILES_LINUX;
         // windows needs leniency due to asinine releasing of file locking async from a process exiting
         Consumer<? super Path> rm = Platforms.WINDOWS ? FileUtils::rmWithRetries : FileUtils::rm;

+ 5 - 7
qa/os/src/test/java/org/elasticsearch/packaging/util/FileUtils.java

@@ -36,7 +36,6 @@ import java.nio.file.Files;
 import java.nio.file.LinkOption;
 import java.nio.file.OpenOption;
 import java.nio.file.Path;
-import java.nio.file.Paths;
 import java.nio.file.StandardOpenOption;
 import java.nio.file.attribute.BasicFileAttributes;
 import java.nio.file.attribute.FileOwnerAttributeView;
@@ -53,6 +52,7 @@ import java.util.stream.Stream;
 import java.util.zip.GZIPInputStream;
 import java.util.zip.ZipException;
 
+import static org.elasticsearch.packaging.test.PackagingTestCase.getRootTempDir;
 import static org.elasticsearch.packaging.util.FileExistenceMatchers.fileDoesNotExist;
 import static org.elasticsearch.packaging.util.FileExistenceMatchers.fileExists;
 import static org.hamcrest.Matchers.emptyIterable;
@@ -66,6 +66,9 @@ public class FileUtils {
 
     public static List<Path> lsGlob(Path directory, String glob) {
         List<Path> paths = new ArrayList<>();
+        if (Files.exists(directory) == false) {
+            return List.of();
+        }
         try (DirectoryStream<Path> stream = Files.newDirectoryStream(directory, glob)) {
 
             for (Path path : stream) {
@@ -297,13 +300,8 @@ public class FileUtils {
         return numericPathOwnership;
     }
 
-    // vagrant creates /tmp for us in windows so we use that to avoid long paths
-    public static Path getTempDir() {
-        return Paths.get("/tmp").toAbsolutePath();
-    }
-
     public static Path getDefaultArchiveInstallPath() {
-        return getTempDir().resolve("elasticsearch");
+        return getRootTempDir().resolve("elasticsearch");
     }
 
     private static final Pattern VERSION_REGEX = Pattern.compile("(\\d+\\.\\d+\\.\\d+(-SNAPSHOT)?)");