-
Notifications
You must be signed in to change notification settings - Fork 364
WIP Cloud Map Service Discovery #551
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently making a bunch of sequential HTTP requests, any suggested alternatives?
Is it reasonable to pull in reactor here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
potentially, there's also ReactiveDiscoveryClient
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a reactive client in fdffdf1.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we always be filtering for healthy instances only, or is that generally a configurable option?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could be an option or also could go in a filter for instance in ribbon or spring cloud loadbalancer
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ServiceInstance doesn't appear to have a health property, would that mean it would just need to look it up as a string via getMetadata?
Any particular implementations I should take a look at for testing inspiration?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks interesting, needs to provide ReactiveDiscoveryClient
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
potentially, there's also ReactiveDiscoveryClient
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could be an option or also could go in a filter for instance in ribbon or spring cloud loadbalancer
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this always the case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's more of a best guess at the moment:
$ aws servicediscovery list-instances --service-id=srv-blah
{
"Instances": [
{
"Id": "fitz-test-instance",
"Attributes": {
"AWS_INSTANCE_IPV4": "blah",
"AWS_INSTANCE_IPV6": "blah blah blah",
"AWS_INSTANCE_PORT": "8080"
}
}
]
}
Got some of the configuration boilerplate knocked out, not sure what's next/what kind of testing makes sense for this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd recommend using the DiscoverInstances API that is built for this use-case.
- No need for pagination
- Filter on the health status is included out of the box
https://docs.aws.amazon.com/cloud-map/latest/api/API_DiscoverInstances.html
Super janky minimal implementation of an AWS Cloud Map based Service Discovery client (#398).
Currently meant as more of a conversation starter than something ready for a full review.
Note that @darylrobbins has a more fleshed out version of this parked in a branch, but I didn't feel comfortable opening a PR with his code.