Skip to content
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

eureka 1.3.1 failed parse JSON register #29

Open
jerome-laforge opened this issue Nov 10, 2015 · 14 comments
Open

eureka 1.3.1 failed parse JSON register #29

jerome-laforge opened this issue Nov 10, 2015 · 14 comments
Assignees

Comments

@jerome-laforge
Copy link

With eureka 1.3.1, JSON doesn't work anymore. It is rejected:
HTTP returned 400 registering Instance ... Body="{"error": "cannot parse request body"}"

The problem seems come from DataCenterInfo where:

@ryansb ryansb self-assigned this Nov 15, 2015
@ryansb
Copy link
Contributor

ryansb commented Nov 15, 2015

Hrm, so I've started working on this, but it turns out there's more to it than just the DataCenterInfo class. It looks like there are some inconsistencies with how Ports are represented introduced in 1.3.1.

I see, for example, some ports as {"$": 9090, "enabled": "true"} and others as {"$": "9090", "enabled": "true"} so somewhere along the way Eureka stopped forcing all port numbers to strings, and instead mixes them. This is a recipe for 😢 time.

Also, it looks like they require incoming ports in JSON to be represented as integers, which is incompatible with the 1.1.X series of APIs. Given this, I'm thinking about dropping the JSON API altogether and using only XML since it seems to be the "first class" way to interact with Eureka.

It shouldn't need to affect the public API since the UseJSON bool would become a noop, and hopefully the XML-only interface would let us keep compatibility with the 1.1.X APIs.

What do the rest of the contributors think about that? @tysonstewart @Camtendo @jcox92

@jerome-laforge
Copy link
Author

I have to use JSON API, since the InstanceMetadata with XML API seems not to work.
Do you have some tips about how to use InstanceMetadata with XML API ?

thx in adv.

@itsrainy
Copy link
Contributor

@ryansb That seems reasonable to me. I'm not at all familiar with the differences (if any) between the JSON and XML APIs so I'll defer to your judgement on that. All of our internal tools that use Fargo only use the XML API so we should be good on that front!

@Camtendo
Copy link
Member

I haven't contributed actual code to this (just the OSS badges) so I'll also defer my opinion. That proposition makes sense to me though.

@ryansb
Copy link
Contributor

ryansb commented Nov 16, 2015

@jcox92 what version of Eureka are you going to be using going forward? Will you be migrating to 1.3.x, or will you stay on the 1.1.x series?

@itsrainy
Copy link
Contributor

@ryansb We're planning on staying on 1.1.x going forward. As of right now we don't have any plans to upgrade that I'm aware of.

@cmitchell
Copy link

I'm using Eureka 1.3 Any chance you would consider modifying fargo.Instance so that we can pass in our own JSON, and it is just stored as json, and then that json is passed to Eureka during registration? That would put the burden on the user if they choose/need to use JSON, but you wouldn't have to keep modifying your code.

By the way, I can't get the xml to work either for Eureka 1.3, so this issue really unfortunately means I can't use your awesome package.

@cmitchell
Copy link

I stand corrected XML does work with Eureka 1.3 I've included the XML output, in case anyone else is having difficulty.

2016/09/29 15:47:12 Registering instance with url http://localhost:8761/eureka/apps/ES-Search
2016/09/29 15:47:12 Got eureka response from url=http://localhost:8761/eureka/apps/ES-Search/i-123456
2016/09/29 15:47:12 Instance=i-123456 not yet registered with App=ES-Search, registering.
2016/09/29 15:47:12 postBody: POST http://localhost:8761/eureka/apps/ES-Search : i-123456ES-SearchlocalhostUP600000MyOwn000000testValue
2016/09/29 15:47:12 Got eureka response from url=http://localhost:8761/eureka/apps/ES-Search
2016/09/29 15:47:12 Getting instance with url http://localhost:8761/eureka/apps/ES-Search/i-123456
2016/09/29 15:47:12 Got eureka response from url=http://localhost:8761/eureka/apps/ES-Search/i-123456

@seh
Copy link
Contributor

seh commented Sep 29, 2016

What isn't working correctly with the JSON that fargo is sending to Eureka? Is it the same problem noted above?
Do you have a server log file available?

@cmitchell
Copy link

Yes, it is the same problem as noted above - there isn't a way to add the "@ class": "com.netflix.appinfo.MyDataCenterInfo" to the Instance struct. Here was the output

2016/09/29 15:03:24 Registering instance with url http://localhost:8761/eureka/apps/ES-Search
2016/09/29 15:03:24 Got eureka response from url=http://localhost:8761/eureka/apps/ES-Search/i-123456
2016/09/29 15:03:24 Instance=i-123456 not yet registered with App=ES-Search, registering.
2016/09/29 15:03:24 postBody: POST http://localhost:8761/eureka/apps/ES-Search : {"instance":{"hostName":"i-123456","app":"ES-Search","ipAddr":"localhost","vipAddress":"","secureVipAddress":"","status":"UP","overriddenstatus":"","port":{"$":"6000","@enabled":""},"securePort":{"$":"0","@enabled":""},"homePageUrl":"","statusPageUrl":"","healthCheckUrl":"","countryId":0,"dataCenterInfo":{"name":"MyOwn","metadata":null},"leaseInfo":{"renewalIntervalInSecs":0,"durationInSecs":0,"registrationTimestamp":0,"lastRenewalTimestamp":0,"evictionTimestamp":0,"serviceUpTimestamp":0},"metadata":{"testkey":"testValue"}}}
2016/09/29 15:03:24 Got eureka response from url=http://localhost:8761/eureka/apps/ES-Search
2016/09/29 15:03:24 HTTP returned 400 registering Instance=i-123456 App=ES-Search Body="{"error": "cannot parse request body"}"

When I manually POSTed via poster the same json but with the "@ class": "com.netflix.appinfo.MyDataCenterInfo" added to the DataCenterInfo block, it worked.

@seh
Copy link
Contributor

seh commented Sep 29, 2016

That's odd; the Eureka server doesn't demand that that field be present for the XML document. After I've registered an instance and I ask Eureka to send it back, that field is present—populated by Eureka on its own.

I take it the proposal is that we send that field when registering an instance. Before doing that, I'd like to study the Eureka server code to see why it's complaining about this lack.

@seh
Copy link
Contributor

seh commented Nov 4, 2016

Eureka release 1.2.2 introduced the Jackson codec, which brings along encoding the ports as integers. I was about to go try to handle this when I read @ryansb's earlier comment about the rest of the changes.

What a mess.

@seh
Copy link
Contributor

seh commented Jan 10, 2017

As I've been working on resolving this problem, here's what I've learned:

  • Eureka post-1.2.2 will accept ports as strings, though it emits them as integers.
  • Eureka post-1.2.2 will not accept a "dataCenterInfo" object without a "@Class" field.
  • Eureka post-1.2.2 will accept a "dataCenterInfo" object with a "@Class" field with a corresponding value of any of the following:
    • com.netflix.appinfo.MyDataCenterInfo
      The sibling "name" field must be present with the value "MyOwn".
      The sibling "metadata" field can be absent, but must not be present but null.
    • com.netflix.appinfo.InstanceInfo$DefaultDataCenterInfo
      Alias for "com.netflix.appinfo.MyDataCenterInfo", subject to the same constraints.
    • com.netflix.appinfo.AmazonInfo
      The sibling "metadata" field must be present and nonnull.

@seh
Copy link
Contributor

seh commented Jan 10, 2017

See pull request #59 for a proposed solution to this problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants