-
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
Making the allocs hold service ids #583
Conversation
@@ -246,6 +246,10 @@ func (s *SystemScheduler) computePlacements(place []allocTuple) error { | |||
Metrics: s.ctx.Metrics(), | |||
} | |||
|
|||
// Generate the service ids for the tasks that this allocation is going | |||
// to run | |||
alloc.PopulateServiceIds() |
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.
Can probably move this inside if option != nil {
block
ae861af
to
d4d7572
Compare
c.trackedTskLock.Unlock() | ||
for _, service := range task.Services { | ||
c.logger.Printf("[INFO] consul: registering service %s with consul.", service.Name) | ||
if err := c.registerService(service, task, allocID); err != nil { | ||
if err := c.registerService(service, task, alloc); err != nil { | ||
fmt.Printf("DIPTANU ERR %v\n", err) |
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.
Debug message? :)
for _, task := range tg.Tasks { | ||
for _, service := range task.Services { | ||
// We add a prefix to the Service ID so that we can know that this service | ||
// is managed by Nomad Consul since Consul can also have service which are not |
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.
Nomad Consul
-> Nomad
…hey are removed from the Tasks
for _, service := range task.Services { | ||
// If the ID for a service name is already generated then we re-use | ||
// it | ||
if ID, ok := oldIDs[service.Name]; ok { |
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.
lowercase the ID field. camelCase is go style
LGTM |
Making the allocs hold service ids
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. |
When we moved to generating the service ids on the server side, it created a problem for jobs whose count > 1 since service ids needs to be unique for an agent as Task definitions are shared across allocations.
This change makes the allocations host service ids for the tasks that they are going to run. Since Allocations are always unique, the service ids will be generated uniquely per allocation.