Move file-based discovery to core/server

[ywelsch]

Some of our integration tests as well as the REST tests currently use the unicast hosts list setting for node discovery. This list is statically defined at node startup, which makes it very inflexible. For the AbstractDisruptionTestCases, for example, we do port scanning to find free ports in order to fix the ports used by each node so that we can properly setup the unicast hosts list before starting the nodes. This port scanning does not always work reliably.. For our REST tests, we have to wait for the first node to be started up and bound to a port before we can start up the other nodes, as we need to put the address:port of the first node into the unicast hosts list of the other nodes. If we had something more flexible to achieve the same, this could really simplify our test infrastructure while also making it more resilient. Fortunately, such a more flexible node discovery already exists: it's the file-based discovery which is currently packaged as a plugin. The proposal here is to move the file-based discovery to the server project, so that it can be readily used in the testing framework for both AbstractDisruptionTestCase as well as all REST tests.

/cc: @franekrichardson

[elasticmachine]

Pinging @elastic/es-core-infra

[elasticmachine]

Pinging @elastic/es-distributed

[DaveCTurner]

The issue here is that we should not simply drop the file-based discovery plugin, because this would be a breaking change to any scripts that provision ES nodes with this plugin installed. I propose:

• in 6.x, move the implementation of FileBasedUnicastHostsProvider into server, but leave the discovery-file plugin in existence as an empty plugin that logs a deprecation warning if used.

• in 7.0, remove the now-empty discovery-file plugin.

This means that setting discovery.zen.hosts_provider: file will continue to work as it does now with the plugin installed; furthermore it will also start to work if the plugin is not installed. Installing the plugin will become a no-op in 6.x (except for the deprecation warning) so can be removed from provisioning scripting at leisure. @franekrichardson does this plan work for you?

[franekrichardson]

@DaveCTurner yes thats very manageable for us 👍

[rjernst]

@ywelsch What about adding file based discovery as a dependency of the test framework? It would slightly complicate the file based discovery unit tests, but we didn't add it to server in the first place for a reason right?

[jasontedor]

That’s not enough, it needs to be available as production code for external-to-us reasons.

[rjernst]

it needs to be available as production code for external-to-us reasons.

That was not in this issue description? I only see test uses.

[jasontedor]

@rjernst I will reach out to discuss.

[DaveCTurner]

I was not aware of a non-test-related reason for doing this. I am also not aware of the reasons why we kept it out of server. Moving it in seems benign - it's a small piece of code and does nothing unless configured - but that is not a convincing argument if there are other ways to achieve the same ends.

Are there other plugins that are available to integ and REST tests? I ask so I can look at the plumbing involved.