Skip to content

[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

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -546,9 +546,15 @@ class VagrantTestPlugin implements Plugin<Project> {
javaPackagingTest.command = 'ssh'
javaPackagingTest.args = ['--command', 'sudo bash "$PACKAGING_TESTS/run-tests.sh"']
} else {
// powershell sessions run over winrm always run as administrator, whether --elevated is passed or not. however
// remote sessions have some restrictions on what they can do, such as impersonating another user (or the same user
// without administrator elevation), which we need to do for these tests. passing --elevated runs the session
// as a scheduled job locally on the vm as a true administrator to get around this limitation
//
// https://github.com/hashicorp/vagrant/blob/9c299a2a357fcf87f356bb9d56e18a037a53d138/plugins/communicators/winrm/communicator.rb#L195-L225
// https://devops-collective-inc.gitbooks.io/secrets-of-powershell-remoting/content/manuscript/accessing-remote-computers.html
javaPackagingTest.command = 'winrm'
// winrm commands run as administrator
javaPackagingTest.args = ['--command', 'powershell -File "$Env:PACKAGING_TESTS/run-tests.ps1"']
javaPackagingTest.args = ['--elevated', '--command', 'powershell -File "$Env:PACKAGING_TESTS/run-tests.ps1"']
}

TaskExecutionAdapter javaPackagingReproListener = createReproListener(project, javaPackagingTest.path)
Expand Down
20 changes: 20 additions & 0 deletions qa/vagrant/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,12 @@ dependencies {
compile "org.hamcrest:hamcrest-core:${versions.hamcrest}"
compile "org.hamcrest:hamcrest-library:${versions.hamcrest}"

compile "org.apache.httpcomponents:httpcore:${versions.httpcore}"
compile "org.apache.httpcomponents:httpclient:${versions.httpclient}"
compile "org.apache.httpcomponents:fluent-hc:${versions.httpclient}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note to self to check on this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usage of httpcomponents' client in general, or the fluent api? I just added the latter to reduce the verbosity a little but don't mind removing it

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fluent API.

compile "commons-codec:commons-codec:${versions.commonscodec}"
compile "commons-logging:commons-logging:${versions.commonslogging}"

compile project(':libs:core')

// pulls in the jar built by this project and its dependencies
Expand Down Expand Up @@ -73,3 +79,17 @@ tasks.test.enabled = false
// this project doesn't get published
tasks.dependencyLicenses.enabled = false
tasks.dependenciesInfo.enabled = false

tasks.thirdPartyAudit.excludes = [
//commons-logging optional dependencies
'org.apache.avalon.framework.logger.Logger',
'org.apache.log.Hierarchy',
'org.apache.log.Logger',
'org.apache.log4j.Category',
'org.apache.log4j.Level',
'org.apache.log4j.Logger',
'org.apache.log4j.Priority',
//commons-logging provided dependencies
'javax.servlet.ServletContextEvent',
'javax.servlet.ServletContextListener'
]
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,12 @@

package org.elasticsearch.packaging.test;

import org.apache.http.client.fluent.Request;
import org.elasticsearch.packaging.util.Archives;
import org.elasticsearch.packaging.util.Platforms;
import org.elasticsearch.packaging.util.ServerUtils;
import org.elasticsearch.packaging.util.Shell;
import org.elasticsearch.packaging.util.Shell.Result;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.FixMethodOrder;
Expand All @@ -28,9 +34,32 @@
import org.elasticsearch.packaging.util.Distribution;
import org.elasticsearch.packaging.util.Installation;

import java.nio.file.Files;
import java.nio.file.Path;
import java.util.Arrays;
import java.util.stream.Stream;

import static java.util.stream.Collectors.joining;
import static org.elasticsearch.packaging.util.Archives.ARCHIVE_OWNER;
import static org.elasticsearch.packaging.util.Cleanup.cleanEverything;
import static org.elasticsearch.packaging.util.Archives.installArchive;
import static org.elasticsearch.packaging.util.Archives.verifyArchiveInstallation;
import static org.elasticsearch.packaging.util.FileMatcher.Fileness.File;
import static org.elasticsearch.packaging.util.FileMatcher.file;
import static org.elasticsearch.packaging.util.FileMatcher.p660;
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.rm;
import static org.elasticsearch.packaging.util.ServerUtils.makeRequest;
import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.CoreMatchers.notNullValue;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.isEmptyString;
import static org.junit.Assert.assertTrue;
import static org.junit.Assume.assumeThat;
import static org.junit.Assume.assumeTrue;

/**
Expand Down Expand Up @@ -61,4 +90,220 @@ public void test10Install() {
installation = installArchive(distribution());
verifyArchiveInstallation(installation, distribution());
}

@Test
public void test20PluginsListWithNoPlugins() {
assumeThat(installation, is(notNullValue()));

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");

assertThat(r.stdout, isEmptyString());
}

@Test
public void test30AbortWhenJavaMissing() {
assumeThat(installation, is(notNullValue()));

final Shell sh = new Shell();
if (Platforms.WINDOWS) {
// 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 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(
"$Env:PATH = '" + newPath + "'; " +
"Remove-Item Env:JAVA_HOME; " +
installation.bin("elasticsearch.bat")
);

assertThat(runResult.exitCode, is(1));
assertThat(runResult.stderr, containsString("could not find java; set JAVA_HOME or ensure java is in PATH"));

} else {
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 + "'");

assertThat(runResult.exitCode, is(1));
assertThat(runResult.stdout, containsString("could not find java; set JAVA_HOME or ensure java is in PATH"));
}
}

@Test
public void test40CreateKeystoreManually() {
assumeThat(installation, is(notNullValue()));

final Shell sh = new Shell();
if (Platforms.WINDOWS) {
// 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
sh.powershell(
installation.bin("elasticsearch-keystore.bat") + " create; " +
"$account = New-Object System.Security.Principal.NTAccount 'vagrant'; " +
"$acl = Get-Acl '" + installation.config("elasticsearch.keystore") + "'; " +
"$acl.SetOwner($account); " +
"Set-Acl '" + installation.config("elasticsearch.keystore") + "' $acl"
);
} else {
sh.bash("sudo -u " + ARCHIVE_OWNER + " " + installation.bin("elasticsearch-keystore") + " create");
}

assertThat(installation.config("elasticsearch.keystore"), file(File, ARCHIVE_OWNER, ARCHIVE_OWNER, p660));

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"));

// cleanup for next test
rm(installation.config("elasticsearch.keystore"));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May be better to do it in @After or move the cleanup to be the first thing, assuming that a previous test somehow left it there it would still work.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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).

}

@Test
public void test50StartAndStop() {
assumeThat(installation, is(notNullValue()));

Archives.runElasticsearch(installation);

final String gcLogName = Platforms.LINUX
? "gc.log.0.current"
: "gc.log";
assertTrue("gc logs exist", Files.exists(installation.logs.resolve(gcLogName)));
ServerUtils.runElasticsearchTests();

Archives.stopElasticsearch(installation);
}

@Test
public void test60AutoCreateKeystore() {
assumeThat(installation, is(notNullValue()));

assertThat(installation.config("elasticsearch.keystore"), file(File, ARCHIVE_OWNER, ARCHIVE_OWNER, p660));

final Shell sh = new Shell();
final Result result;
if (Platforms.WINDOWS) {
result = sh.powershell(installation.bin("elasticsearch-keystore.bat") + " list");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if it would make sense to have an abstraction on top of Shell to call bin scripts in a platform independent way. I think it would make tests more readable e.x. having something like ESShell().kestore().list() to achieve this. Or maybe EsShell().escalatePrivileges().keystore().list() to make it run with administrator/root

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, though I'd lean towards just doing it for the executable paths

} else {
result = sh.bash("sudo -u " + ARCHIVE_OWNER + " " + installation.bin("elasticsearch-keystore") + " list");
}
assertThat(result.stdout, containsString("keystore.seed"));
}

@Test
public void test70CustomPathConfAndJvmOptions() {
assumeThat(installation, is(notNullValue()));

final Path tempConf = getTempDir().resolve("esconf-alternate");

try {
mkdir(tempConf);
cp(installation.config("elasticsearch.yml"), tempConf.resolve("elasticsearch.yml"));
cp(installation.config("log4j2.properties"), tempConf.resolve("log4j2.properties"));

// we have to disable Log4j from using JMX lest it will hit a security
// manager exception before we have configured logging; this will fail
// startup since we detect usages of logging before it is configured
final String jvmOptions =
"-Xms512m\n" +
"-Xmx512m\n" +
"-Dlog4j2.disable.jmx=true\n";
append(tempConf.resolve("jvm.options"), jvmOptions);

final Shell sh = new Shell();
if (Platforms.WINDOWS) {
sh.powershell(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it'd be safe to use Java code for this regardless of OS. For some of these things it is great that we're shelling out because we're emulating a real person, but not I don't think a person is going to type this into powershell.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure we can change these permissions on windows in java because of how it exposes filesystem attributes for windows' fs. Or at least that was what I originally intended to do but couldn't get it working. Is there something you could point me towards?

But yeah definitely right about the user not doing it this way, they'll use the gui

"$account = New-Object System.Security.Principal.NTAccount 'vagrant'; " +
"$tempConf = Get-ChildItem '" + tempConf + "' -Recurse; " +
"$tempConf += Get-Item '" + tempConf + "'; " +
"$tempConf | ForEach-Object { " +
"$acl = Get-Acl $_.FullName; " +
"$acl.SetOwner($account); " +
"Set-Acl $_.FullName $acl " +
"}"
);
} else {
sh.bash("chown -R elasticsearch:elasticsearch " + tempConf);
}

final Shell serverShell = new Shell();
serverShell.getEnv().put("ES_PATH_CONF", tempConf.toString());
serverShell.getEnv().put("ES_JAVA_OPTS", "-XX:-UseCompressedOops");

Archives.runElasticsearch(installation, serverShell);

final String nodesResponse = makeRequest(Request.Get("http://localhost:9200/_nodes"));
assertThat(nodesResponse, containsString("\"heap_init_in_bytes\":536870912"));
assertThat(nodesResponse, containsString("\"using_compressed_ordinary_object_pointers\":\"false\""));

Archives.stopElasticsearch(installation);

} finally {
rm(tempConf);
}
}

@Test
public void test80RelativePathConf() {
assumeThat(installation, is(notNullValue()));

final Path temp = getTempDir().resolve("esconf-alternate");
final Path tempConf = temp.resolve("config");

try {
mkdir(tempConf);
Stream.of(
"elasticsearch.yml",
"log4j2.properties",
"jvm.options"
).forEach(file -> cp(installation.config(file), tempConf.resolve(file)));

append(tempConf.resolve("elasticsearch.yml"), "node.name: relative");

final Shell sh = new Shell();
if (Platforms.WINDOWS) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This conditional is repeated a few times and not very readable, I might consider extracting it to a utility method.
Would also consider adding Platform.onWindowsOnly(PlatformSpecificAction do, String description) where PlatformSpecificAction is just a functional interface, I would find that more readable, and besides the else condition is not technically correct java as not windows doesn't imply posix ( in theory at least ). It would also give us the opportunity to log that desciption was carried out, or it wasn't on thi platform.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've mostly tried to keep the control flow as simple as possible in cases like this where we're very unlikely to need to extend it any further - that is, even though the if (windows) {} else {} is gross, the effect is really clear and it's unlikely to ever need a third code path.

I like what you've suggested here though and I'll try it out.

the else condition is not technically correct java as not windows doesn't imply posix ( in theory at least )

Agree, this has consistently bothered me. Similarly though, making more of a distinction than that has seemed like overkill

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it would have been an overkill and useless distraction in a conditional, that's also why I suggested abstracting the platform specifics a bit more. Assuming that the platform abstractions are correct, most of the test code doesn't care about the platform, having these conditionals only when there is an actual difference in behavior between these platforms that we need to test for would make it a much easier read and imho help allot in maintaining these tests.

sh.powershell(
"$account = New-Object System.Security.Principal.NTAccount 'vagrant'; " +
"$tempConf = Get-ChildItem '" + temp + "' -Recurse; " +
"$tempConf += Get-Item '" + temp + "'; " +
"$tempConf | ForEach-Object { " +
"$acl = Get-Acl $_.FullName; " +
"$acl.SetOwner($account); " +
"Set-Acl $_.FullName $acl " +
"}"
);
} else {
sh.bash("chown -R elasticsearch:elasticsearch " + temp);
}

final Shell serverShell = new Shell(temp);
serverShell.getEnv().put("ES_PATH_CONF", "config");
Archives.runElasticsearch(installation, serverShell);

final String nodesResponse = makeRequest(Request.Get("http://localhost:9200/_nodes"));
assertThat(nodesResponse, containsString("\"name\":\"relative\""));

Archives.stopElasticsearch(installation);

} finally {
rm(tempConf);
}
}


}
Loading