Add Support to ES 2.X

[falencastro]

It's compiling fine with maven (-Dmaven.test.skip=true).

Issues:

• Tests fail due to java.lang.IllegalArgumentException: Unknown Discovery type [srvtest].
• Exception java.lang.ClassNotFoundException: org.xbill.DNS.TextParseException happens only if you build the project with gradle. That happens because build.gradle is generating a zipfile with dnsjava-2.1.7.jar stored inside libs subfolder, instead of the root folder.

Details:

• ElasticsearchIntegrationTest migrated to ESIntegTestCase
• ImmutableSettings migrated to Settings
• Rollback to maven
• Include static plugin-descriptor.properties
• Instruct maven to pack plugin-descriptor.properties
• Correct ES plugin package name
• Added debug on SrvUnicastHostsProvider/lookupRecords
• Fixes build jar hell by replacing hamcrest-all with hamcrest-library

[chrismwendt]

Wonderful! Thank you, @falencastro ✨ I'm taking a look at it now 😃

[falencastro]

Hanging issue solved. I was running with a default java.policy file, just needed to replace it with:
grant { permission java.security.AllPermission; };
Everything is working fine now! 😃

[chrismwendt]

Elasticsearch itself recently switched from Maven to Gradle (which is why I had done so for this plugin as well). Why did you roll back to Maven?

[chrismwendt]

How about Starting SRV discovery?

[falencastro]

I saw that gradle was including unnecessary jars and storing them on a subfolder, while maven was generating proper zip files. Sure, the right thing to do is fix build.gradle, I'll take a look at this.

About the debug message, yeah, it will look better capitalized.

[chrismwendt]

This PR mixes tabs and spaces. Can you make it all 4-space indentation like it was before?

[chrismwendt]

Out of curiosity, is new ArrayList<DiscoveryNode>() generally preferable to Lists.newArrayList()?

[falencastro]

org.elasticsearch.common.collect.Lists which was being used, was removed from newer ES versions. I also took a look at consul-discovery.

I'll remove the tabs :)

[chrismwendt]

I believe that you can actually remove src/main/java/org/elasticsearch/discovery/srv/SrvDiscoveryModule.java. I did so in the other WIP: #22

[chrismwendt]

• Tests fail due to java.lang.IllegalArgumentException: Unknown Discovery type [srvtest].

I managed to solve this with https://github.com/github/elasticsearch-srv-discovery/pull/22/files#diff-c7aaa6daa650a88b917f442c2745dcabR59

It's pretty hacky because it pollutes the discovery namespace with something that should only be visible during testing, but I couldn't figure out another way around it. Let me know if you are able to get the test to work.

[falencastro]

Hi,

I implemented your suggestions. From your last PR:

• Tests fail due to java.nio.file.NoSuchFileException: .../build/resources/test

Still happening. Once you create this directory manually, test goes ahead but gets stuck at 85% after starting elasticsearch.

• The properties in plugin-descriptor.properties are hard-coded

In ES 5 theres a gradle plugin called esplugin, which will solve this, something like this will generate a new plugin-descriptor.properties:

esplugin {
  name 'srv-discovery'
  version '2.3.3'
  description 'SRV record discovery plugin'
  classname 'org.elasticsearch.plugin.discovery.srv.SrvDiscoveryPlugin'
}

• It throws an exception during runtime: java.lang.ClassNotFoundException: org.xbill.DNS.TextParseException

Fixed on build.gradle:

jar {
    manifest {
        attributes("Class-Path": configurations.compile.collect { "lib/$it.name" }.join(' '))
    }
}

[chrismwendt]

Were you able to at least build the tests? gradle build seems to hang on me:

> gradle build
:compileJava UP-TO-DATE
:processResources UP-TO-DATE
:classes UP-TO-DATE
:jar UP-TO-DATE
:distTar UP-TO-DATE
:distZip UP-TO-DATE
:assemble UP-TO-DATE
:compileTestJava UP-TO-DATE
:processTestResources UP-TO-DATE
:testClasses UP-TO-DATE
> Building 76% > :test^C

When I run the test in IntelliJ, it also hangs at what appears to be the end:

NOTE: All tests run in this JVM: [SrvDiscoveryIntegrationTest]

I'm not sure what's going on there.

• Tests fail due to java.nio.file.NoSuchFileException: .../build/resources/test

Still happening. Once you create this directory manually, test goes ahead but gets stuck at 85% after starting elasticsearch

Any idea why it's getting stuck during the test run?

Thanks for looking into the versioning - I think we can tolerate hard-coding it until ES 5.

And thanks for fixing that TextParseException - I wouldn't have thought of that fix.

[mfussenegger]

If I may chime in:

It's pretty hacky because it pollutes the discovery namespace with something that should only be visible during testing, but I couldn't figure out another way around it. Let me know if you are able to get the test to work.

I think you could use a stub implementation like we did in https://github.com/crate/crate/blob/master/dns-discovery/src/test/java/io/crate/discovery/SrvDiscoveryIntegrationTest.java#L45

I think that way the whole srvtest code could be removed.

Tests fail due to java.nio.file.NoSuchFileException: .../build/resources/test

I think this could be solved by adding

sourceSets {
    test {
        output.resourcesDir = null
    }
}

to the build.gradle

Tests hang for me as well at 85%

Didn't investigate but things I've noticed:

The package namespace isn't correct.

In SrvDiscoveryPlugin it is package org.elasticsearch.plugin.discovery.srv but should be package org.elasticsearch.plugin.discovery.

I think the tests need to define the plugin in order to load it:

    @Override
    protected Collection<Class<? extends Plugin>> nodePlugins() {
        return pluginList(SrvDiscoveryPlugin.class);
    }

And in SrvDiscoveryPlugin in onModule it should probably check the discovery.type setting and only load the components if it matches srv.