Added ReregisterInstance to allow registering over an existing registrat... #4
Conversation
…ration Added DeregisterInstance to all deregistration Added UpdateInstanceStatus to allow an instance to be taken out of service Other rpc and struct changes to match.
| @@ -135,6 +143,47 @@ func (e *EurekaConnection) RegisterInstance(ins *Instance) error { | |||
| return nil | |||
| } | |||
|
|
|||
| // DeregisterInstance will register the given Instance with eureka but DOES | |||
| // NOT automatically send heartbeats. See HeartBeatInstance for that | |||
There was a problem hiding this comment.
You'll need to update these godocs before I can merge this in.
There was a problem hiding this comment.
Oops, sure thing. Missed that one.
|
I'd definitely like to merge these back in, it'd be great if you could write tests for your additions. Thanks! |
|
I have written tests, but am having trouble getting the vagrant env with eureka working. The gradle build failed, so I just pulled a binary from the jenkins job linked from the eureka github. But it's giving me 404's on all requests, so that's not good. I'll keep working on that, but let me know if you have any hints.. |
|
Figured out the 404 problem: the vagrant nodes didn't get their eurekas. Now it seems that the eurekas are running, but don't see each other, and then a bunch of the tests fail because of that. Could you add a little more info on the steps to setup and run the tests? I'm guessing that I need to have the eurekas know about their node*.localdomain names, so I'll try that next... (I had been doing development against a cluster we have internally, and hadn't tried setting up a vagrant harness like this, so please excuse my dumb questions.) |
|
I'll add more instructions to the tests. So far they've only really been run on my local setup, so I appreciate the feedback. |
|
@cquinn I've added testing instructions at https://github.com/hudl/fargo#testing and I've updated the vagrant provisioning script to use the eureka build from Netflix's cloudbees page. Let me know if the tests work on your vagrant installation. |
|
Thanks Ryan, that helped a lot, and my Vagrant Eurekas are running well now. The tests all work except the TestMetadataReading which fails with: (my line numbers are different) It looks like the call to GetString() after the AddMetadataString call ends up doing a parse() which wipes out the parsed map since Raw was never set. Does this test work for you? |
|
It does work for me, you may have to run in twice (separated by 20 seconds or so) for it to pass. I believe the syncing between eureka servers occasionally causes that test to fail. |
|
Ah, it now works after a second run. I still don't see how the instance metadata is read back: probably via readAppInto() but I don't see where that is enabled for the tests. Not a big deal. I'll clean up my files and prepare another PR later today. |
… TestDeregistration, TestUpdateStatus. Although I had to turn off TestDeregistration since that triggers flakiness in other tests. Also removed first So() in TestMetadataReading since that was causing Convey errors for me and since that path is already tested in TestConnectionCreation which I moved to earlier in the file.
|
OK, all done, ready for merge I hope. |
Added ReregisterInstance courtesy of @cquinn
|
Looks great, thanks! |
...ion
Added DeregisterInstance to all deregistration
Added UpdateInstanceStatus to allow an instance to be taken out of service
Other rpc and struct changes to match.
Let me know if you think you'd like these changes back and I'll write the tests to go with them. (I'll probably write the test anyway :)