-
Notifications
You must be signed in to change notification settings - Fork 2k
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
service discovery: add initial MVP implementation #12368
Conversation
service discovery: add state store functionality
events: add state objects and logic for service registrations.
service discovery: add RPC endpoints and FSM logic
service discovery: add HTTP endpoints and sdk wrapper
jobspec: add service block provider parameter
config: add native service discovery admin boolean parameter.
service discovery: add config boolean parameter and fingerprinting
This commit performs refactoring to pull out common service registration objects into a new `client/serviceregistration` package. This new package will form the base point for all client specific service registration functionality. The Consul specific implementation is not moved as it also includes non-service registration implementations; this reduces the blast radius of the changes as well.
client: refactor common service registration objects from Consul.
client: add Nomad service registration implementation.
client: hookup service wrapper for use in clients
When a node fails its heart beating a number of actions are taken to ensure state is cleaned. Service registrations a loosely tied to nodes, therefore we should remove these from state when a node is considered terminally down.
cli: add service commands for list, info, and delete.
The service registration client name was used to provide a distinction between the service block and the service client. This however creates new wording to understand and does not match the CLI, therefore this change fixes that so we have a Services client. Consul specific objects within the service file have been moved to the consul location to create a clearer separation.
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.
LGTM! (last two commits)
7db3236
to
2d8b370
Compare
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
This PR contains the MVP work for Nomad service discovery, whereby Nomad services can specify a provider parameter and become registered within Nomad state. It includes state, RPC, HTTP API, CLI and client work. It does not currently include template functionality which is being worked on currently.
All commits have been reviewed previously except the final two. These two commits perform small refactoring to accommodate CI changes on main and a naming change discussed internally.
I was planning on merging this without squashing to keep some history of the development rather than a single commit which contains +10k changes. If there are thoughts on why we should squash then I am happy to alter the approach.
The failing Nomad RPC test is not a result of changes here, and is fixed within #12364