-
Notifications
You must be signed in to change notification settings - Fork 25.2k
[test] port archive distribution packaging tests #31314
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
1e1bc67
67c73e2
17291fa
2686d60
45dff8a
8012b78
25ce2ef
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -96,11 +96,9 @@ public void test10Install() { | |
public void test20PluginsListWithNoPlugins() { | ||
assumeThat(installation, is(notNullValue())); | ||
|
||
// todo here | ||
final Installation.Executables bin = installation.executables(); | ||
final Shell sh = new Shell(); | ||
final Result r = Platforms.WINDOWS | ||
? sh.powershell(installation.bin("elasticsearch-plugin.bat") + " list") | ||
: sh.bash(installation.bin("elasticsearch-plugin") + " list"); | ||
final Result r = sh.run(bin.elasticsearchPlugin + " list"); | ||
|
||
assertThat(r.stdout, isEmptyString()); | ||
} | ||
|
@@ -109,37 +107,36 @@ public void test20PluginsListWithNoPlugins() { | |
public void test30AbortWhenJavaMissing() { | ||
assumeThat(installation, is(notNullValue())); | ||
|
||
final Installation.Executables bin = installation.executables(); | ||
final Shell sh = new Shell(); | ||
|
||
Platforms.onWindows(() -> { | ||
// on windows, removing java from PATH and removing JAVA_HOME is less involved than changing the permissions of the java | ||
// executable. we also don't check permissions in the windows scripts anyway | ||
final String originalPath = sh.powershell("$Env:PATH").stdout.trim(); | ||
final String originalPath = sh.run("$Env:PATH").stdout.trim(); | ||
final String newPath = Arrays.stream(originalPath.split(";")) | ||
.filter(path -> path.contains("Java") == false) | ||
.collect(joining(";")); | ||
|
||
final String javaHome = sh.powershell("$Env:JAVA_HOME").stdout.trim(); | ||
|
||
// note the lack of a $ when clearing the JAVA_HOME env variable - with a $ it deletes the java home directory | ||
// https://docs.microsoft.com/en-us/powershell/module/microsoft.powershell.core/providers/environment-provider?view=powershell-6 | ||
// | ||
// this won't persist to another session so we don't have to reset anything | ||
final Result runResult = sh.powershellIgnoreExitCode( | ||
final Result runResult = sh.runIgnoreExitCode( | ||
"$Env:PATH = '" + newPath + "'; " + | ||
"Remove-Item Env:JAVA_HOME; " + | ||
installation.bin("elasticsearch.bat") | ||
bin.elasticsearch | ||
); | ||
|
||
assertThat(runResult.exitCode, is(1)); | ||
assertThat(runResult.stderr, containsString("could not find java; set JAVA_HOME or ensure java is in PATH")); | ||
}); | ||
|
||
Platforms.onLinux(() -> { | ||
final String javaPath = sh.bash("which java").stdout.trim(); | ||
sh.bash("chmod -x '" + javaPath + "'"); | ||
final Result runResult = sh.bashIgnoreExitCode(installation.bin("elasticsearch").toString()); | ||
sh.bash("chmod +x '" + javaPath + "'"); | ||
final String javaPath = sh.run("which java").stdout.trim(); | ||
sh.run("chmod -x '" + javaPath + "'"); | ||
final Result runResult = sh.runIgnoreExitCode(bin.elasticsearch.toString()); | ||
sh.run("chmod +x '" + javaPath + "'"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe have this in a |
||
|
||
assertThat(runResult.exitCode, is(1)); | ||
assertThat(runResult.stdout, containsString("could not find java; set JAVA_HOME or ensure java is in PATH")); | ||
|
@@ -150,16 +147,17 @@ public void test30AbortWhenJavaMissing() { | |
public void test40CreateKeystoreManually() { | ||
assumeThat(installation, is(notNullValue())); | ||
|
||
final Installation.Executables bin = installation.executables(); | ||
final Shell sh = new Shell(); | ||
|
||
Platforms.onLinux(() -> sh.bash("sudo -u " + ARCHIVE_OWNER + " " + installation.bin("elasticsearch-keystore") + " create")); | ||
Platforms.onLinux(() -> sh.run("sudo -u " + ARCHIVE_OWNER + " " + bin.elasticsearchKeystore + " create")); | ||
|
||
// this is a hack around the fact that we can't run a command in the same session as the same user but not as administrator. | ||
// the keystore ends up being owned by the Administrators group, so we manually set it to be owned by the vagrant user here. | ||
// from the server's perspective the permissions aren't really different, this is just to reflect what we'd expect in the tests. | ||
// when we run these commands as a role user we won't have to do this | ||
Platforms.onWindows(() -> sh.powershell( | ||
installation.bin("elasticsearch-keystore.bat") + " create; " + | ||
Platforms.onWindows(() -> sh.run( | ||
bin.elasticsearchKeystore + " create; " + | ||
"$account = New-Object System.Security.Principal.NTAccount 'vagrant'; " + | ||
"$acl = Get-Acl '" + installation.config("elasticsearch.keystore") + "'; " + | ||
"$acl.SetOwner($account); " + | ||
|
@@ -168,11 +166,15 @@ public void test40CreateKeystoreManually() { | |
|
||
assertThat(installation.config("elasticsearch.keystore"), file(File, ARCHIVE_OWNER, ARCHIVE_OWNER, p660)); | ||
|
||
// todo here | ||
final Result r = Platforms.WINDOWS | ||
? sh.powershell(installation.bin("elasticsearch-keystore.bat") + " list") | ||
: sh.bash("sudo -u " + ARCHIVE_OWNER + " " + installation.bin("elasticsearch-keystore") + " list"); | ||
assertThat(r.stdout, containsString("keystore.seed")); | ||
Platforms.onLinux(() -> { | ||
final Result r = sh.run("sudo -u " + ARCHIVE_OWNER + " " + bin.elasticsearchKeystore + " list"); | ||
assertThat(r.stdout, containsString("keystore.seed")); | ||
}); | ||
|
||
Platforms.onWindows(() -> { | ||
final Result r = sh.run(bin.elasticsearchKeystore + " list"); | ||
assertThat(r.stdout, containsString("keystore.seed")); | ||
}); | ||
|
||
// cleanup for next test | ||
rm(installation.config("elasticsearch.keystore")); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. May be better to do it in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll move it to the next test, that reflects the way we do cleanup in general (at the beginning rather than at the end). |
||
|
@@ -199,15 +201,16 @@ public void test60AutoCreateKeystore() { | |
|
||
assertThat(installation.config("elasticsearch.keystore"), file(File, ARCHIVE_OWNER, ARCHIVE_OWNER, p660)); | ||
|
||
final Installation.Executables bin = installation.executables(); | ||
final Shell sh = new Shell(); | ||
|
||
Platforms.onLinux(() -> { | ||
final Result result = sh.bash("sudo -u " + ARCHIVE_OWNER + " " + installation.bin("elasticsearch-keystore") + " list"); | ||
final Result result = sh.run("sudo -u " + ARCHIVE_OWNER + " " + bin.elasticsearchKeystore + " list"); | ||
assertThat(result.stdout, containsString("keystore.seed")); | ||
}); | ||
|
||
Platforms.onWindows(() -> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could have methods such as
But we really don't need to go that far. This is fine as it is for now. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah I like the way that reads and did that before, but took it out because it didn't seem necessary |
||
final Result result = sh.powershell(installation.bin("elasticsearch-keystore.bat") + " list"); | ||
final Result result = sh.run(bin.elasticsearchKeystore + " list"); | ||
assertThat(result.stdout, containsString("keystore.seed")); | ||
}); | ||
} | ||
|
@@ -233,8 +236,8 @@ public void test70CustomPathConfAndJvmOptions() throws IOException { | |
append(tempConf.resolve("jvm.options"), jvmOptions); | ||
|
||
final Shell sh = new Shell(); | ||
Platforms.onLinux(() -> sh.bash("chown -R elasticsearch:elasticsearch " + tempConf)); | ||
Platforms.onWindows(() -> sh.powershell( | ||
Platforms.onLinux(() -> sh.run("chown -R elasticsearch:elasticsearch " + tempConf)); | ||
Platforms.onWindows(() -> sh.run( | ||
"$account = New-Object System.Security.Principal.NTAccount 'vagrant'; " + | ||
"$tempConf = Get-ChildItem '" + tempConf + "' -Recurse; " + | ||
"$tempConf += Get-Item '" + tempConf + "'; " + | ||
|
@@ -280,8 +283,8 @@ public void test80RelativePathConf() throws IOException { | |
append(tempConf.resolve("elasticsearch.yml"), "node.name: relative"); | ||
|
||
final Shell sh = new Shell(); | ||
Platforms.onLinux(() -> sh.bash("chown -R elasticsearch:elasticsearch " + temp)); | ||
Platforms.onWindows(() -> sh.powershell( | ||
Platforms.onLinux(() -> sh.run("chown -R elasticsearch:elasticsearch " + temp)); | ||
Platforms.onWindows(() -> sh.run( | ||
"$account = New-Object System.Security.Principal.NTAccount 'vagrant'; " + | ||
"$tempConf = Get-ChildItem '" + temp + "' -Recurse; " + | ||
"$tempConf += Get-Item '" + temp + "'; " + | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -75,20 +75,19 @@ public static Installation installArchive(Distribution distribution, Path fullIn | |
|
||
if (distribution.packaging == Distribution.Packaging.TAR) { | ||
|
||
Platforms.onLinux(() -> sh.bash("tar -C " + baseInstallPath + " -xzpf " + distributionFile)); | ||
Platforms.onLinux(() -> sh.run("tar -C " + baseInstallPath + " -xzpf " + distributionFile)); | ||
|
||
if (Platforms.WINDOWS) { | ||
throw new RuntimeException("Distribution " + distribution + " is not supported on windows"); | ||
} | ||
|
||
} else if (distribution.packaging == Distribution.Packaging.ZIP) { | ||
|
||
Platforms.onLinux(() -> sh.bash("unzip " + distributionFile + " -d " + baseInstallPath)); | ||
Platforms.onLinux(() -> sh.run("unzip " + distributionFile + " -d " + baseInstallPath)); | ||
|
||
Platforms.onWindows(() -> | ||
sh.powershell( | ||
"Add-Type -AssemblyName 'System.IO.Compression.Filesystem'; " + | ||
"[IO.Compression.ZipFile]::ExtractToDirectory('" + distributionFile + "', '" + baseInstallPath + "')" | ||
Platforms.onWindows(() -> sh.run( | ||
"Add-Type -AssemblyName 'System.IO.Compression.Filesystem'; " + | ||
"[IO.Compression.ZipFile]::ExtractToDirectory('" + distributionFile + "', '" + baseInstallPath + "')" | ||
)); | ||
|
||
} else { | ||
|
@@ -113,17 +112,17 @@ public static Installation installArchive(Distribution distribution, Path fullIn | |
private static void setupArchiveUsersLinux(Path installPath) { | ||
final Shell sh = new Shell(); | ||
|
||
if (sh.bashIgnoreExitCode("getent group elasticsearch").isSuccess() == false) { | ||
if (sh.runIgnoreExitCode("getent group elasticsearch").isSuccess() == false) { | ||
if (isDPKG()) { | ||
sh.bash("addgroup --system elasticsearch"); | ||
sh.run("addgroup --system elasticsearch"); | ||
} else { | ||
sh.bash("groupadd -r elasticsearch"); | ||
sh.run("groupadd -r elasticsearch"); | ||
} | ||
} | ||
|
||
if (sh.bashIgnoreExitCode("id elasticsearch").isSuccess() == false) { | ||
if (sh.runIgnoreExitCode("id elasticsearch").isSuccess() == false) { | ||
if (isDPKG()) { | ||
sh.bash("adduser " + | ||
sh.run("adduser " + | ||
"--quiet " + | ||
"--system " + | ||
"--no-create-home " + | ||
|
@@ -132,7 +131,7 @@ private static void setupArchiveUsersLinux(Path installPath) { | |
"--shell /bin/false " + | ||
"elasticsearch"); | ||
} else { | ||
sh.bash("useradd " + | ||
sh.run("useradd " + | ||
"--system " + | ||
"-M " + | ||
"--gid elasticsearch " + | ||
|
@@ -141,14 +140,14 @@ private static void setupArchiveUsersLinux(Path installPath) { | |
"elasticsearch"); | ||
} | ||
} | ||
sh.bash("chown -R elasticsearch:elasticsearch " + installPath); | ||
sh.run("chown -R elasticsearch:elasticsearch " + installPath); | ||
} | ||
|
||
private static void setupArchiveUsersWindows(Path installPath) { | ||
// we want the installation to be owned as the vagrant user rather than the Administrators group | ||
|
||
final Shell sh = new Shell(); | ||
sh.powershell( | ||
sh.run( | ||
"$account = New-Object System.Security.Principal.NTAccount 'vagrant'; " + | ||
"$install = Get-ChildItem -Path '" + installPath + "' -Recurse; " + | ||
"$install += Get-Item -Path '" + installPath + "'; " + | ||
|
@@ -263,25 +262,27 @@ public static void runElasticsearch(Installation installation) throws IOExceptio | |
public static void runElasticsearch(Installation installation, Shell sh) throws IOException { | ||
final Path pidFile = installation.home.resolve("elasticsearch.pid"); | ||
|
||
final Installation.Executables bin = installation.executables(); | ||
|
||
Platforms.onLinux(() -> { | ||
// If jayatana is installed then we try to use it. Elasticsearch should ignore it even when we try. | ||
// If it doesn't ignore it then Elasticsearch will fail to start because of security errors. | ||
// This line is attempting to emulate the on login behavior of /usr/share/upstart/sessions/jayatana.conf | ||
if (Files.exists(Paths.get("/usr/share/java/jayatanaag.jar"))) { | ||
sh.getEnv().put("JAVA_TOOL_OPTIONS", "-javaagent:/usr/share/java/jayatanaag.jar"); | ||
} | ||
sh.bash("sudo -E -u " + ARCHIVE_OWNER + " " + | ||
installation.bin("elasticsearch") + " -d -p " + installation.home.resolve("elasticsearch.pid")); | ||
sh.run("sudo -E -u " + ARCHIVE_OWNER + " " + | ||
bin.elasticsearch + " -d -p " + installation.home.resolve("elasticsearch.pid")); | ||
}); | ||
|
||
Platforms.onWindows(() -> { | ||
// this starts the server in the background. the -d flag is unsupported on windows | ||
// these tests run as Administrator. we don't want to run the server as Administrator, so we provide the current user's | ||
// username and password to the process which has the effect of starting it not as Administrator. | ||
sh.powershell( | ||
sh.run( | ||
"$password = ConvertTo-SecureString 'vagrant' -AsPlainText -Force; " + | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wow. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My experience has been that one line of bash converts roughly to ten lines of powershell |
||
"$processInfo = New-Object System.Diagnostics.ProcessStartInfo; " + | ||
"$processInfo.FileName = '" + installation.bin("elasticsearch.bat") + "'; " + | ||
"$processInfo.FileName = '" + bin.elasticsearch + "'; " + | ||
"$processInfo.Arguments = '-p " + installation.home.resolve("elasticsearch.pid") + "'; " + | ||
"$processInfo.Username = 'vagrant'; " + | ||
"$processInfo.Password = $password; " + | ||
|
@@ -304,8 +305,8 @@ public static void runElasticsearch(Installation installation, Shell sh) throws | |
String pid = slurp(pidFile).trim(); | ||
assertThat(pid, not(isEmptyOrNullString())); | ||
|
||
Platforms.onLinux(() -> sh.bash("ps " + pid)); | ||
Platforms.onWindows(() -> sh.powershell("Get-Process -Id " + pid)); | ||
Platforms.onLinux(() -> sh.run("ps " + pid)); | ||
Platforms.onWindows(() -> sh.run("Get-Process -Id " + pid)); | ||
} | ||
|
||
public static void stopElasticsearch(Installation installation) { | ||
|
@@ -315,8 +316,8 @@ public static void stopElasticsearch(Installation installation) { | |
assertThat(pid, not(isEmptyOrNullString())); | ||
|
||
final Shell sh = new Shell(); | ||
Platforms.onLinux(() -> sh.bash("kill -SIGTERM " + pid)); | ||
Platforms.onWindows(() -> sh.powershell("Get-Process -Id " + pid + " | Stop-Process -Force")); | ||
Platforms.onLinux(() -> sh.run("kill -SIGTERM " + pid)); | ||
Platforms.onWindows(() -> sh.run("Get-Process -Id " + pid + " | Stop-Process -Force")); | ||
} | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
++ I think this reads much better.
This is probably a matter of preference, but I think making
sh
a field and just having something like this would read even better:Admittedly I personally hate variables used only once.
This will also need a
sh.run(String executable, String... args)
that joins args with a space.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I considered doing this and decided against it because all the commands are run using
"bash", "-c", "[everything passed to Shell.run]"
, and making it look like it handles arguments separately would be trappy (i.e. this makes it clear that Shell does no quoting/escaping for you)