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

Service Discovery Integration #219

Closed
wants to merge 21 commits into from
Closed

Service Discovery Integration #219

wants to merge 21 commits into from

Conversation

ryanuber
Copy link
Member

@ryanuber ryanuber commented Oct 6, 2015

This implements an additional layer on Nomad clients, which can manage registering and deregistering tasks in a discovery back-end like Consul. This includes Consul integration as well.

I think we may be able to make some adjustments here and in other areas to make this experience nicer.

What currently works:

  • Register/deregister when tasks start/end.
  • Registration of the task itself (no port information)
  • Registration of labeled dynamic ports (port info also registered)

One slightly strange detail is that by default, service registration entries have a long name, like job1-group1-task1. I've added a field to override this in the jobspec at the task level so that you can still get names like db, web, redis, etc., however, this opens up the possibility of overlapping names.

// can be taken in this back-end. Specifically, the IP address of the Nomad
// agent does not need to be used, because Consul has this information
// already and may even be configured to expose services on an alternate
// advertise address.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I may be misreading this but I'm not sure this is a safe assumption to make. Since nomad may be managing tasks across multiple IPs and those services will be bound to a specific interface, the IP is actually important. Even if nomad or consul is listening on a particular IP, tasks are not necessarily registered on the same IP.

// context can be used from this function for internal node information
// such as IP address. The allocID can be used to uniquely identify the
// same task running multiple instances on the same node.
Register(allocID, name string, port int) error
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we may want a richer struct as well here, so that the entire allocation and task can be pushed in.

@ryanuber
Copy link
Member Author

Closing this out for now - we are thinking of an alternate implementation that will improve on what this PR provides.

@ryanuber ryanuber closed this Oct 12, 2015
@F21
Copy link

F21 commented Oct 14, 2015

Is there any eta for when consul integration will make it into master?

@adrianlop
Copy link
Contributor

Hey there @ryanuber @dadgar @armon, any news on Nomad integration with Consul?
Thanks in advance!

@dadgar
Copy link
Contributor

dadgar commented Oct 29, 2015

@poll0rz: We will be bringing Consul integration in Nomad 0.2 which is due within the next couple of weeks. We want to get the experience right because it is a critical feature! Thanks for the patience!

@F21
Copy link

F21 commented Oct 29, 2015

@dadgar Is the plan to run consul on top of nomad (essentially managed by nomad) or to run consul along side?

@cbednarski
Copy link
Contributor

@F21 Nomad will support either workflow but we're aiming to manage it for you by default.

@dadgar dadgar deleted the f-consul branch March 11, 2016 00:50
@github-actions
Copy link

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.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants