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.

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

Add docker-compose based test fixture for Azure #48636

Merged
merged 4 commits into from
Oct 31, 2019

Conversation

tlrx
Copy link
Member

@tlrx tlrx commented Oct 29, 2019

This pull request adds a new :test:fixtures:azure-fixture project which provides a docker-compose based container that runs a AzureHttpFixture Java class that emulates an Azure Storage service.

The logic to emulate the service is extracted from existing tests and placed in AzureHttpHandler into the new project so that it can be easily reused. The :plugins:repository-azure project is an example of such utilization.

The AzureHttpFixture fixture is just a wrapper around AzureHttpHandler and is now executed within the docker container. The :plugins:repository-azure:qa:microsoft-azure project uses the new test fixture and the existing AzureStorageFixture has been removed.

@tlrx tlrx added >test Issues or PRs that are addressing/adding tests :Delivery/Build Build or test infrastructure :Distributed/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs v8.0.0 v7.5.0 v7.6.0 labels Oct 29, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (:Core/Infra/Build)

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed (:Distributed/Snapshot/Restore)

@@ -38,20 +49,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.

* Minimal HTTP handler that acts as an Azure compliant server
*/
@SuppressForbidden(reason = "Uses a HttpServer to emulate an Azure endpoint")
public class AzureHttpHandler implements HttpHandler {
Copy link
Member Author

@tlrx tlrx Oct 29, 2019

Choose a reason for hiding this comment

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

The implementation is the same as InternalHttpHandler with the following changes:

  • container name is passed as a ctor parameter
  • some links to the Azure REST APIs are added as comments
  • Put Blob now also checks "If-None-Match" header as the AzureStorageFixture did
  • remove dependency on azure SDK as it was just used for header names and error code strings.

@tlrx
Copy link
Member Author

tlrx commented Oct 29, 2019

The main motivations around this change are to allow more parallelization when executing tests on CI and to allow new features to easily use Azure fixtures in tests.

Copy link
Member

@original-brownbear original-brownbear left a comment

Choose a reason for hiding this comment

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

This looks good to me, but I'd leave it to @atorok to give his final approval here build wise :)

Copy link
Contributor

@mark-vieira mark-vieira left a comment

Choose a reason for hiding this comment

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

Just a couple of comments, otherwise this LGTM.

Thanks for taking care of this. I'm very happy to see usages of AntFixture removed. We should definitely do the same for all the others.

test/fixtures/azure-fixture/build.gradle Outdated Show resolved Hide resolved
test/fixtures/azure-fixture/build.gradle Outdated Show resolved Hide resolved
test/fixtures/azure-fixture/build.gradle Outdated Show resolved Hide resolved
Copy link
Contributor

@alpar-t alpar-t left a comment

Choose a reason for hiding this comment

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

Do we have a compelling reason to roll out own ? It seems there is an implementation of azure called azurite pointed to in the azure docs.

@@ -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.

test/fixtures/azure-fixture/build.gradle Show resolved Hide resolved
@original-brownbear
Copy link
Member

@atorok last time I tried (little less than a year ago) Azurite wasn't compatible enough to work for us. We also recently tried using it in a Cloud project and couldn't get it to work with SDK v8 (which we're using in ES and newer SDKs are unstable sadly :(). We should keep an eye on it though, maybe we can make use of it eventually. It seems they are making progress in that project :)

@tlrx
Copy link
Member Author

tlrx commented Oct 30, 2019

Do we have a compelling reason to roll out own ? It seems there is an implementation of azure called azurite pointed to in the azure docs.

I think Armin already mentioned the reasons to not use azurite (yet) but it seems that they are now almost feature complete (Blob APIs) so I hope we'll use it in the future.

@tlrx
Copy link
Member Author

tlrx commented Oct 30, 2019

Thanks @mark-vieira @original-brownbear and @atorok for your feedback. I addressed most of the comments or I asked precision when needed. Please let me know what you think!

@tlrx tlrx requested a review from alpar-t October 30, 2019 10:10
Copy link
Contributor

@alpar-t alpar-t left a comment

Choose a reason for hiding this comment

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

LGTM

@tlrx tlrx merged commit 3e4791a into elastic:master Oct 31, 2019
@tlrx tlrx deleted the azure-storage-test-fixture branch October 31, 2019 08:28
@tlrx
Copy link
Member Author

tlrx commented Oct 31, 2019

Thanks all!

tlrx added a commit to tlrx/elasticsearch that referenced this pull request Oct 31, 2019
This commit adds a new :test:fixtures:azure-fixture project which 
provides a docker-compose based container that runs a AzureHttpFixture 
Java class that emulates an Azure Storage service.

The logic to emulate the service is extracted from existing tests and 
placed in AzureHttpHandler into the new project so that it can be 
easily reused. The :plugins:repository-azure project is an example 
of such utilization.

The AzureHttpFixture fixture is just a wrapper around AzureHttpHandler 
and is now executed within the docker container. 

The :plugins:repository-azure:qa:microsoft-azure project uses the new 
test fixture and the existing AzureStorageFixture has been removed.
tlrx added a commit that referenced this pull request Nov 7, 2019
Similarly to what has be done for Azure in #48636, this commit 
adds a new :test:fixtures:gcs-fixture project which provides two 
docker-compose based fixtures that emulate a Google Cloud 
Storage service.

Some code has been extracted from existing tests and placed 
into this new project so that it can be easily reused in other 
projects.
tlrx added a commit to tlrx/elasticsearch that referenced this pull request Nov 7, 2019
Similarly to what has be done for Azure in elastic#48636, this commit
adds a new :test:fixtures:gcs-fixture project which provides two
docker-compose based fixtures that emulate a Google Cloud
Storage service.

Some code has been extracted from existing tests and placed
into this new project so that it can be easily reused in other
projects.
tlrx added a commit that referenced this pull request Nov 7, 2019
Similarly to what has be done for Azure in #48636, this commit
adds a new :test:fixtures:gcs-fixture project which provides two
docker-compose based fixtures that emulate a Google Cloud
Storage service.

Some code has been extracted from existing tests and placed
into this new project so that it can be easily reused in other
projects.
tlrx added a commit that referenced this pull request Nov 7, 2019
Similarly to what has be done for Azure in #48636, this commit
adds a new :test:fixtures:gcs-fixture project which provides two
docker-compose based fixtures that emulate a Google Cloud
Storage service.

Some code has been extracted from existing tests and placed
into this new project so that it can be easily reused in other
projects.
tlrx added a commit that referenced this pull request Nov 18, 2019
Similarly to what has been done for Azure (#48636) and GCS (#48762), 
this committ removes the existing Ant fixture that emulates a S3 storage 
service in favor of multiple docker-compose based fixtures.

The goals here are multiple: be able to reuse a s3-fixture outside of the 
repository-s3 plugin; allow parallel execution of integration tests; removes 
the existing AmazonS3Fixture that has evolved in a weird beast in 
dedicated, more maintainable fixtures.

The server side logic that emulates S3 mostly comes from the latest 
HttpHandler made for S3 blob store repository tests, with additional 
features extracted from the (now removed) AmazonS3Fixture: 
authentication checks, session token checks and improved response 
errors. Chunked upload request support for S3 object has been added 
too. 

The server side logic of all tests now reside in a single S3HttpHandler class.

Whereas AmazonS3Fixture contained logic for basic tests, session token 
tests, EC2 tests or ECS tests, the S3 fixtures are now dedicated to each 
kind of test. Fixtures are inheriting from each other, making things easier 
to maintain.
tlrx added a commit to tlrx/elasticsearch that referenced this pull request Nov 18, 2019
Similarly to what has been done for Azure (elastic#48636) and GCS (elastic#48762),
this committ removes the existing Ant fixture that emulates a S3 storage
service in favor of multiple docker-compose based fixtures.

The goals here are multiple: be able to reuse a s3-fixture outside of the
repository-s3 plugin; allow parallel execution of integration tests; removes
the existing AmazonS3Fixture that has evolved in a weird beast in
dedicated, more maintainable fixtures.

The server side logic that emulates S3 mostly comes from the latest
HttpHandler made for S3 blob store repository tests, with additional
features extracted from the (now removed) AmazonS3Fixture:
authentication checks, session token checks and improved response
errors. Chunked upload request support for S3 object has been added
too.

The server side logic of all tests now reside in a single S3HttpHandler class.

Whereas AmazonS3Fixture contained logic for basic tests, session token
tests, EC2 tests or ECS tests, the S3 fixtures are now dedicated to each
kind of test. Fixtures are inheriting from each other, making things easier
to maintain.
tlrx added a commit that referenced this pull request Nov 18, 2019
Similarly to what has been done for Azure (#48636) and GCS (#48762),
this committ removes the existing Ant fixture that emulates a S3 storage
service in favor of multiple docker-compose based fixtures.

The goals here are multiple: be able to reuse a s3-fixture outside of the
repository-s3 plugin; allow parallel execution of integration tests; removes
the existing AmazonS3Fixture that has evolved in a weird beast in
dedicated, more maintainable fixtures.

The server side logic that emulates S3 mostly comes from the latest
HttpHandler made for S3 blob store repository tests, with additional
features extracted from the (now removed) AmazonS3Fixture:
authentication checks, session token checks and improved response
errors. Chunked upload request support for S3 object has been added
too.

The server side logic of all tests now reside in a single S3HttpHandler class.

Whereas AmazonS3Fixture contained logic for basic tests, session token
tests, EC2 tests or ECS tests, the S3 fixtures are now dedicated to each
kind of test. Fixtures are inheriting from each other, making things easier
to maintain.
tlrx added a commit that referenced this pull request Nov 18, 2019
Similarly to what has been done for Azure (#48636) and GCS (#48762),
this committ removes the existing Ant fixture that emulates a S3 storage
service in favor of multiple docker-compose based fixtures.

The goals here are multiple: be able to reuse a s3-fixture outside of the
repository-s3 plugin; allow parallel execution of integration tests; removes
the existing AmazonS3Fixture that has evolved in a weird beast in
dedicated, more maintainable fixtures.

The server side logic that emulates S3 mostly comes from the latest
HttpHandler made for S3 blob store repository tests, with additional
features extracted from the (now removed) AmazonS3Fixture:
authentication checks, session token checks and improved response
errors. Chunked upload request support for S3 object has been added
too.

The server side logic of all tests now reside in a single S3HttpHandler class.

Whereas AmazonS3Fixture contained logic for basic tests, session token
tests, EC2 tests or ECS tests, the S3 fixtures are now dedicated to each
kind of test. Fixtures are inheriting from each other, making things easier
to maintain.
@mark-vieira mark-vieira added the Team:Delivery Meta label for Delivery team label Nov 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Delivery/Build Build or test infrastructure :Distributed/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs Team:Delivery Meta label for Delivery team >test Issues or PRs that are addressing/adding tests v7.5.0 v7.6.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants