-
Notifications
You must be signed in to change notification settings - Fork 25k
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
Make Minio Setup more Reliable #37747
Conversation
original-brownbear
commented
Jan 23, 2019
- Retry starting Minio five times in case we run into a race between finding the free port and starting it up
- Closes [CI] minio fails due to port clashes #37680
* Retry starting Minio five times in case we run into a race between finding the free port and starting it up * Closes elastic#37680
Pinging @elastic/es-distributed |
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.
Are there any downside to creating a minio-fixture
using elasticsearch.test.fixtures
plugin ? I think it would be a cleaner more consistent solution, than having a special case with custom retries, finding free port, parsing output lines etc.
AFAIK minio has a docker image that we could readily use. The plugin takes care of providing the ports used for the mapping as system properties. You can check existing uses of the plugin.
@original-brownbear I saw your comment on the ticket just now. There's a bit of a learning curve to the plugin for sure, but past that it doesn't seem like a difficult change to go that route directly and have this fixed for good. Let me know what you think, I'm not opposed to merging this in either, it's still better to have the test passing, but wouldn't like to have this complexity in the build for long. |
I agree with everything you say :). That said: My reasoning here would be I can't fix this using the plugin now/today I'm afraid but tests are still kind of horrible so I tried dealing with this one real quick. |
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'm not opposed to merging this to stop the failures.
I do think that moving this to docker is easier than it looks.
@atorok thanks, will add an issue for that move in a sec :) |
…ead-de-duplication * elastic/master: Use explicit version for build-tools in example plugin integ tests (elastic#37792) Change `rational` to `saturation` in script_score (elastic#37766) Deprecate types in get field mapping API (elastic#37667) Add ability to listen to group of affix settings (elastic#37679) Ensure changes requests return the latest mapping version (elastic#37633) Make Minio Setup more Reliable (elastic#37747)
* elastic/master: (85 commits) Use explicit version for build-tools in example plugin integ tests (elastic#37792) Change `rational` to `saturation` in script_score (elastic#37766) Deprecate types in get field mapping API (elastic#37667) Add ability to listen to group of affix settings (elastic#37679) Ensure changes requests return the latest mapping version (elastic#37633) Make Minio Setup more Reliable (elastic#37747) Liberalize StreamOutput#writeStringList (elastic#37768) Add PersistentTasksClusterService::unassignPersistentTask method (elastic#37576) Tests: disable testRandomGeoCollectionQuery on tiny polygons (elastic#37579) Use ILM for Watcher history deletion (elastic#37443) Make sure PutMappingRequest accepts content types other than JSON. (elastic#37720) Retry ILM steps that fail due to SnapshotInProgressException (elastic#37624) Use disassociate in preference to deassociate (elastic#37704) Delete Redundant RoutingServiceTests (elastic#37750) Always return metadata version if metadata is requested (elastic#37674) [TEST] Mute MlMappingsUpgradeIT testMappingsUpgrade Streamline skip_unavailable handling (elastic#37672) Only bootstrap and elect node in current voting configuration (elastic#37712) Ensure either success or failure path for SearchOperationListener is called (elastic#37467) Target only specific index in update settings test ...