Skip to content

Add docker-compose based test fixture for Azure #48636

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

Merged
merged 4 commits into from
Oct 31, 2019
Merged

Add docker-compose based test fixture for Azure #48636

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
69e70e4
azure storage test fixture
tlrx Oct 29, 2019
adafc3d
Remove log trace
tlrx Oct 29, 2019
6957bf3
add missing dependsOn
tlrx Oct 29, 2019
1b3db04
Mark's feedback
tlrx Oct 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
1 change: 1 addition & 0 deletions plugins/repository-azure/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ dependencies {
compile 'com.microsoft.azure:azure-keyvault-core:1.0.0'
compile 'com.google.guava:guava:20.0'
compile 'org.apache.commons:commons-lang3:3.4'
testCompile project(':test:fixtures:azure-fixture')
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be better not to have multiple types of dependencies on a project it makes things harder to change, we already depend on it as a fixture it would be nice to avoid depending on the jar as well.
From what I see it's because the tests also use it to start up some fixtures, could we change that so they talk to one externally ? It would be even better if we could verify the retries as a unit tests without talking to the actual service, e.x. by mocking the sdk ? I understand if that's more work that we don't want to take on right now, but it would be a nice to have.

Copy link
Member Author

Choose a reason for hiding this comment

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

It would be better not to have multiple types of dependencies on a project it makes things harder to change, we already depend on it as a fixture it would be nice to avoid depending on the jar as well.

Yes, I agree. I wanted to have the fixture plumbing and its logic located at the same place within the same project and this is the only way I know to do it. The AzureHttpHandler could be placed into the test framework (and the fixture depends on it) but I don't think this a better solution.

From what I see it's because the tests also use it to start up some fixtures, could we change that so they talk to one externally ?

For AzureBlobStoreRepositoryTests having the server logic running in the same JVM is really convenient when it comes to understand or debug things. I guess we could change this to run on external servers but we'll lost the ability to debug easily.

It would be even better if we could verify the retries as a unit tests without talking to the actual service, e.x. by mocking the sdk ?

This is what was done for a long time (and still done in some tests) but for ESBlobStoreRepositoryIntegTestCase and other *RetriesTests we want to test how the SDK behaves exactly in various scenarios of server side errors. The SDKs offer some way to mock the HTTP calls (either using Mockito or some kind of low-level execution mock) but it does not reflect the exact behavior of the SDK and issues like #46589 would have been hard to troubleshoot and fix.

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess we could change this to run on external servers but we'll lost the ability to debug easily.

Thinking about this twice, we could maybe just always run the fixture within the container in debug mode and connect the IDE to the ephemeral port

Copy link
Contributor

Choose a reason for hiding this comment

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

I'll leave this to your best judgment. I don't feel strongly about not having this dependency and you bring up good arguments for not doing it too.

}

dependencyLicenses {
Expand Down
23 changes: 11 additions & 12 deletions 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 @@ -26,6 +26,9 @@ import static org.elasticsearch.gradle.PropertyNormalization.IGNORE_VALUE

apply plugin: 'elasticsearch.standalone-rest-test'
apply plugin: 'elasticsearch.rest-test'
apply plugin: 'elasticsearch.test.fixtures'

testFixtures.useFixture ":test:fixtures:azure-fixture", "azure-fixture"

boolean useFixture = false

Expand All @@ -38,20 +41,12 @@ String azureSasToken = System.getenv("azure_storage_sas_token")
if (!azureAccount && !azureKey && !azureContainer && !azureBasePath && !azureSasToken) {
azureAccount = 'azure_integration_test_account'
azureKey = 'YXp1cmVfaW50ZWdyYXRpb25fdGVzdF9rZXk=' // The key is "azure_integration_test_key" encoded using base64
azureContainer = 'container_test'
azureBasePath = 'integration_test'
azureContainer = 'container'
Copy link
Member Author

Choose a reason for hiding this comment

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

This is changed to accommodate the default values used by the new test fixture.

azureBasePath = ''
azureSasToken = ''
useFixture = true
}

/** A task to start the fixture which emulates an Azure Storage service **/
task azureStorageFixture(type: AntFixture) {
dependsOn testClasses
env 'CLASSPATH', "${ -> project.sourceSets.test.runtimeClasspath.asPath }"
executable = new File(project.runtimeJavaHome, 'bin/java')
args 'org.elasticsearch.repositories.azure.AzureStorageFixture', baseDir, azureContainer
}

Map<String, Object> expansions = [
'container': azureContainer,
'base_path': azureBasePath + "_integration_tests"
Expand All @@ -77,11 +72,15 @@ testClusters.integTest {
}

if (useFixture) {
tasks.integTest.dependsOn azureStorageFixture
def azureAddress = {
int ephemeralPort = project(':test:fixtures:azure-fixture').postProcessFixture.ext."test.fixtures.azure-fixture.tcp.8091"
assert ephemeralPort > 0
'http://127.0.0.1:' + ephemeralPort
}
// Use a closure on the string to delay evaluation until tests are executed. The endpoint_suffix is used
// in a hacky way to change the protocol and endpoint. We must fix that.
setting 'azure.client.integration_test.endpoint_suffix',
{ "ignored;DefaultEndpointsProtocol=http;BlobEndpoint=http://${azureStorageFixture.addressAndPort }" }, IGNORE_VALUE
{ "ignored;DefaultEndpointsProtocol=http;BlobEndpoint=${ -> azureAddress() }" }, IGNORE_VALUE
String firstPartOfSeed = project.rootProject.testSeed.tokenize(':').get(0)
setting 'thread_pool.repository_azure.max', (Math.abs(Long.parseUnsignedLong(firstPartOfSeed, 16) % 10) + 1).toString(), System.getProperty('ignore.tests.seed') == null ? DEFAULT : IGNORE_VALUE
}
Expand Down
Loading