-
Notifications
You must be signed in to change notification settings - Fork 25.2k
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.
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
Conversation
Pinging @elastic/es-core-infra (:Core/Infra/Build) |
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' |
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.
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 { |
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.
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.
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. |
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.
This looks good to me, but I'd leave it to @atorok to give his final approval here build wise :)
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.
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.
plugins/repository-azure/qa/microsoft-azure-storage/build.gradle
Outdated
Show resolved
Hide resolved
plugins/repository-azure/qa/microsoft-azure-storage/build.gradle
Outdated
Show resolved
Hide resolved
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.
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') |
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.
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.
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.
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.
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 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
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'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.
@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 :) |
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. |
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! |
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.
LGTM
Thanks all! |
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.
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.
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.
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.
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.
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.
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.
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.
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.
This pull request adds a new
:test:fixtures:azure-fixture
project which provides a docker-compose based container that runs aAzureHttpFixture
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 aroundAzureHttpHandler
and is now executed within the docker container. The:plugins:repository-azure:qa:microsoft-azure
project uses the new test fixture and the existingAzureStorageFixture
has been removed.