Skip to content
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.

Sign up for GitHub

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

Differentiate base paths in repository integration tests #47284

Merged
merged 7 commits into from
Sep 30, 2019
Merged

Differentiate base paths in repository integration tests #47284

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
8c0360e
Revert "Mute Snapshot/Restore with repository-azure (#47204)"
tlrx Sep 30, 2019
177782b
Revert "Mute azure third party tests (#47205)"
tlrx Sep 30, 2019
4db0299
Differentiate repository base paths in Azure tests
tlrx Sep 30, 2019
f06c1f8
clean up afterwards in 3rd party tests
tlrx Sep 30, 2019
05591d2
Adapt for HDFS
tlrx Sep 30, 2019
c4b8542
S3+GCS
tlrx Sep 30, 2019
022c08e
Merge branch 'master' into azure-it
elasticmachine Sep 30, 2019
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
2 changes: 1 addition & 1 deletion plugins/repository-azure/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ task thirdPartyTest(type: Test) {
systemProperty 'test.azure.key', azureKey ? azureKey : ""
systemProperty 'test.azure.sas_token', azureSasToken ? azureSasToken : ""
systemProperty 'test.azure.container', azureContainer ? azureContainer : ""
systemProperty 'test.azure.base', azureBasePath ? azureBasePath : ""
systemProperty 'test.azure.base', (azureBasePath ? azureBasePath : "") + "_third_party_tests_" + project.testSeed
Copy link
Member

Choose a reason for hiding this comment

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

@tlrx thanks for this one!

I think technically this is the perfect fix in terms of stability. One problem we're creating here though is that we are leaking a bunch of blobs for each test run now.
Should we add some logic to delete the leaked folders maybe? (otherwise we force infra to deal with this somehow which will be tricky for them I think)

Copy link
Member

Choose a reason for hiding this comment

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

To avoid this issue for now: Maybe we can simply add a constant path here and another constant in the other build so we don't get interference between the test runs? We don't have to worry about different builds interfering with each other, infra makes sure that doesn't happen so that should be fine?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I wrongly assumed that the container/buckets were recreated on demand by some Infra task before running the tests.

I think we can easily keep the base_path with test seed for the 3rd party tests, as we can easily clean up the files when tearing down the test (similarly to what the test set up is already doing doing)

For the repository integration test, I'll go with adding a constant.

}

if (azureAccount || azureKey || azureContainer || azureBasePath || azureSasToken) {
Expand Down
2 changes: 1 addition & 1 deletion plugins/repository-azure/qa/microsoft-azure-storage/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ task azureStorageFixture(type: AntFixture) {

Map<String, Object> expansions = [
'container': azureContainer,
'base_path': azureBasePath
'base_path': azureBasePath + "_integration_tests"
]

processTestResources {
Expand Down
8 changes: 2 additions & 6 deletions ...ft-azure-storage/src/test/resources/rest-api-spec/test/repository_azure/10_repository.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,8 @@ setup:
---
"Snapshot/Restore with repository-azure":
- skip:
version: "all"
reason: "AwaitsFix https://github.com/elastic/elasticsearch/issues/47201"
# BELOW 3 lines to be uncommented when un-muting
# - skip:
# version: " - 7.9.99"
# reason: "8.0 changes get snapshots response format"
version: " - 7.9.99"
reason: "8.0 changes get snapshots response format"

# Get repository
- do:
Expand Down
21 changes: 0 additions & 21 deletions ...rc/test/java/org/elasticsearch/repositories/azure/AzureStorageCleanupThirdPartyTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,25 +109,4 @@ private void ensureSasTokenPermissions() {
}));
future.actionGet();
}

// override here to mute only for Azure, please remove this overload when un-muting
@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/47202")
@Override
public void testCreateSnapshot() {
super.testCreateSnapshot();
}

// override here to mute only for Azure, please remove this overload when un-muting
@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/47202")
@Override
public void testCleanup() throws Exception {
super.testCleanup();
}

// override here to mute only for Azure, please remove this overload when un-muting
@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/47202")
@Override
public void testListChildren() throws Exception {
super.testListChildren();
}
}
2 changes: 0 additions & 2 deletions plugins/repository-gcs/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import java.nio.file.Files

/*
* Licensed to Elasticsearch under one or more contributor
* license agreements. See the NOTICE file distributed with
Expand Down
4 changes: 2 additions & 2 deletions plugins/repository-gcs/qa/google-cloud-storage/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ task thirdPartyTest (type: Test) {
include '**/GoogleCloudStorageThirdPartyTests.class'
systemProperty 'tests.security.manager', false
systemProperty 'test.google.bucket', gcsBucket
systemProperty 'test.google.base', gcsBasePath
systemProperty 'test.google.base', gcsBasePath + "_third_party_tests_" + project.testSeed
nonInputProperties.systemProperty 'test.google.account', "${ -> encodedCredentials.call() }"
}

Expand Down Expand Up @@ -128,7 +128,7 @@ check.dependsOn thirdPartyTest

Map<String, Object> expansions = [
'bucket': gcsBucket,
'base_path': gcsBasePath
'base_path': gcsBasePath + "_integration_tests"
]

processTestResources {
Expand Down
13 changes: 12 additions & 1 deletion ...epository-hdfs/src/test/java/org/elasticsearch/repositories/hdfs/HdfsRepositoryTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,16 @@ protected SecureSettings credentials() {
return new MockSecureSettings();
}

@Override
public void tearDown() throws Exception {
if (isJava11() == false) {
super.tearDown();
}
}

@Override
protected void createRepository(String repoName) {
assumeFalse("https://github.com/elastic/elasticsearch/issues/31498", JavaVersion.current().equals(JavaVersion.parse("11")));
assumeFalse("https://github.com/elastic/elasticsearch/issues/31498", isJava11());
AcknowledgedResponse putRepositoryResponse = client().admin().cluster().preparePutRepository(repoName)
.setType("hdfs")
.setSettings(Settings.builder()
Expand All @@ -70,4 +77,8 @@ protected void assertCleanupResponse(CleanupRepositoryResponse response, long by
assertThat(response.result().blobs(), equalTo(0L));
}
}

private static boolean isJava11() {
return JavaVersion.current().equals(JavaVersion.parse("11"));
}
}
6 changes: 3 additions & 3 deletions plugins/repository-s3/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ task thirdPartyTest(type: Test) {
systemProperty 'test.s3.account', s3PermanentAccessKey
systemProperty 'test.s3.key', s3PermanentSecretKey
systemProperty 'test.s3.bucket', s3PermanentBucket
systemProperty 'test.s3.base', s3PermanentBasePath
systemProperty 'test.s3.base', s3PermanentBasePath + "_third_party_tests_" + project.testSeed
}

if (useFixture) {
Expand Down Expand Up @@ -248,9 +248,9 @@ task s3Fixture(type: AntFixture) {
processTestResources {
Map<String, Object> expansions = [
'permanent_bucket': s3PermanentBucket,
'permanent_base_path': s3PermanentBasePath,
'permanent_base_path': s3PermanentBasePath + "_integration_tests",
'temporary_bucket': s3TemporaryBucket,
'temporary_base_path': s3TemporaryBasePath,
'temporary_base_path': s3TemporaryBasePath + "_integration_tests",
'ec2_bucket': s3EC2Bucket,
'ec2_base_path': s3EC2BasePath,
'ecs_bucket': s3ECSBucket,
Expand Down
6 changes: 6 additions & 0 deletions ...rk/src/main/java/org/elasticsearch/repositories/AbstractThirdPartyRepositoryTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,12 @@ public void setUp() throws Exception {
deleteAndAssertEmpty(getRepository().basePath());
}

@Override
public void tearDown() throws Exception {
deleteAndAssertEmpty(getRepository().basePath());
super.tearDown();
}

private void deleteAndAssertEmpty(BlobPath path) throws Exception {
final BlobStoreRepository repo = getRepository();
final PlainActionFuture<Void> future = PlainActionFuture.newFuture();
Expand Down