Eureka url can not be set from xml config
bzaicsek-cxn opened this issue · 18 comments
Hi!
I have the following xml configuration:
<hazelcast>
<network>
<join>
<eureka enabled="true">
<self-registration>true</self-registration>
<namespace>hazelcast</namespace>
<use-metadata-for-host-and-port>false</use-metadata-for-host-and-port>
<use-classpath-eureka-client-props>false</use-classpath-eureka-client-props>
<shouldUseDns>false</shouldUseDns>
<name>hazelcast-test</name>
<serviceUrl.default>http://localhost:8761/eureka</serviceUrl.default>
</eureka>
</join>
</network>
</hazelcast>
This is how your README suggests it. However, when I try to run the program, I get an error:
Caused by: com.hazelcast.config.InvalidConfigurationException: Unknown properties: '[shouldusedns, serviceurl.default]' on discovery strategy
at com.hazelcast.spi.discovery.impl.DiscoveryServicePropertiesUtil.verifyNoUnknownProperties(DiscoveryServicePropertiesUtil.java:88)
I have managed to debug that the problem comes from the fact that somehow my keys from the XML get lower-cased and the properties in the class PropertyBasedEurekaClientConfigConstants are not all lower cased. (https://github.com/hazelcast/hazelcast-eureka/blob/v1.1.2/src/main/java/com/hazelcast/eureka/one/PropertyBasedEurekaClientConfigConstants.java) So when it tries to find my settings in the allowed properties DiscoveryServicePropertiesUtil it finds that "shouldusedns" and "serviceurl.default" are not valid. I am happy to send a pullrequest with the fix if you could direct me to the class that lower-cases my originally camelCased properties.
Please help!
Hi!
I have found that this XML can work, so this can be a good workaround:
<discovery-strategies>
<discovery-strategy class="com.hazelcast.eureka.one.EurekaOneDiscoveryStrategy" enabled="true">
<properties>
<property name="self-registration">true</property>
<property name="namespace">hazelcast</property>
<property name="use-metadata-for-host-and-port">false</property>
<property name="use-classpath-eureka-client-props">false</property>
<property name="skip-eureka-registration-verification">false</property>
<property name="appinfo.initial.replicate.time">60</property>
<property name="shouldUseDns">false</property>
<property name="name">hazelcast-test</property>
<property name="serviceUrl.default">http://[the-url-of-your-server]:8761/eureka</property>
</properties>
</discovery-strategy>
</discovery-strategies>
close the issue, if you feel so, but than, please add this to the README.
Hi,
I'd like to check and fix this issue.
Hi @omidp , glad to see you here. And the issue is yours!
Hi @bzaicsek-cxn,
I am not able to reproduce this error. with the given xml I am getting
Invalid content was found starting with element eureka
I created a simple project to reproduce the error here.
Please help me to reproduce the error thank you.
And I realized that you are using hazelcast-eureka-one version 1.1.2 is that correct ?
Hi @omidp,
It is great to see you working on this issue
About error that you encounter, it seems xsd version of your xml file is wrong. I could reproduce the issue with Hazelcast 4.0 and Eureka 2.0 which are latest stable versions, here is the my xml:
<hazelcast xmlns="http://www.hazelcast.com/schema/config"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://www.hazelcast.com/schema/config
http://www.hazelcast.com/schema/config/hazelcast-config-4.0.xsd">
<network>
<join>
<multicast enabled="false"/>
<eureka enabled="true">
<self-registration>true</self-registration>
<namespace>hazelcast</namespace>
<use-metadata-for-host-and-port>false</use-metadata-for-host-and-port>
<use-classpath-eureka-client-props>false</use-classpath-eureka-client-props>
<shouldUseDns>false</shouldUseDns>
<name>hazelcast-test</name>
<serviceUrl.default>http://localhost:8761/eureka</serviceUrl.default>
</eureka>
</join>
</network>
</hazelcast>
Hi!
I am here only to sign that I have got the message. I will need some days to find all the details since now I am not working on this.
Hi @bzaicsek-cxn ,
I think you do not need to provide any information about your setup or reproducer. As I mentioned above, I have already reproduced the issue with latest version, I will help @omidp about it
Hi,
Thank you for your help.
I realized here all xml node names are converted to lower case, however, in PropertyBasedEurekaClientConfigConstants.java shouldUseDns is being used as it is.
so which one I should change since I don't know the real reason why author of these files made this decision ?
@omidp , constants were added to make configuring Eureka discovery without properties file possible (related PR --> #13). Using same convention with main Eureka repo is good approach but when this feature were added, we have only one type of XML configuration style for discovery configs:
<discovery-strategies>
<discovery-strategy class="com.hazelcast.eureka.one.EurekaOneDiscoveryStrategy" enabled="true">
<properties>
<property name="self-registration">true</property>
....
</properties>
</discovery-strategy>
</discovery-strategies>
That's why we have used camelCased property constants at our discovery service.
Here is the our existing options to solve issue;
- At Hazelcast core side, we can prevent to convert Eureka properties to lower case:
https://github.com/hazelcast/hazelcast/blob/master/hazelcast/src/main/java/com/hazelcast/internal/config/MemberDomConfigProcessor.java#L1344 - At Eureka plugin side, we can use lower case constants but we do not follow Eureka convention anymore. And also we need to make some enhancements at plugin codebase like converting lower cased props to camel case before create
EurekaClientConfig:
https://github.com/hazelcast/hazelcast-eureka/blob/master/src/main/java/com/hazelcast/eureka/one/EurekaOneDiscoveryStrategy.java#L178
Personally, I prefer first option. But let's first hear what others have to say. WDYT @mesutcelik @alparslanavci?
I don't feel like we need to implement a Eureka specific solution in Hazelcast Core. I am fine if we introduce a generic solution. cc: @blazember
Otherwise, we need to handle it in Eureka Plugin even though the solution is not straightforward.
Hi @omidp glad to see you picked this up!
I agree with @hasancelik and @mesutcelik that it is better to solve this in Hazelcast Core. An idea is that you can pass down a boolean to this: https://github.com/hazelcast/hazelcast/blob/master/hazelcast/src/main/java/com/hazelcast/internal/config/MemberDomConfigProcessor.java#L1344 indicating whether or not we should lowercase. For deciding this you can use tag in handleAliasedDiscoveryStrategy().
You can test your solution with updating https://github.com/hazelcast/hazelcast/blob/master/hazelcast/src/test/java/com/hazelcast/config/XMLConfigBuilderTest.java#L401 and https://github.com/hazelcast/hazelcast/blob/master/hazelcast/src/test/java/com/hazelcast/config/YamlConfigBuilderTest.java#L362.
Feel free to ping me if you have further questions or you have a PR to review. Good luck with it
@omidp, the issue is related to all eureka config nodes not only shouldUseDns and serviceUrl.default so you need to provide more general solution. For example, you can start with adding one more argument into cleanNodeName method to decide which tag will not be lowercase.
I dont think changing cleanNodeName method is a good idea. it has been used in a lot of places, however maybe I can create an overload of this method with an extra boolean parameter.
I am also not fan of making a method configurable by sending parameters, it is against my OOP code :)
I will see what I can do to make it more general.
Ok I think this approach is better. can you plz check and give me feedback.
fixed by hazelcast/hazelcast#16679
Hi!
Should this work? Was this released? I can not find the changes in Hazelcast 4.0.2 or in HazelcastJet 4.2.
@bzaicsek-cxn Hi! This fix goes to 4.1, whose Beta we will be releasing in ~2 weeks and GA is expected around end of October :) Feel free to try it out with the Beta and let us know. I'm very interested in your feedback