-
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
Not moving alloc data when sticky is turned off #2017
Conversation
|
||
// stripAllocation strips the allocation of fields which can be re-assembled in | ||
// the server. | ||
func (c *Client) stripAllocation(alloc *structs.Allocation) *structs.Allocation { |
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 you have updateAllocStatus call this. Otherwise call sites may not do it.
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.
We will loose the information we need if we strip the allocation at the updateAllocStatus
site. So this needs to be stripped before we send them over the wire.
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.
Also the updateAllocStatus
pushes the alloc to a chan, and we are stripping the allocations when we receive them from the chan so it should be fine.
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.
Okay so allocSync
seems like the wrong place to be adding that logic. That method is used just to batch updates. Can you move the logic to updateAllocStatus
and revert back the stripAllocation changes because you will have the info you need there.
@@ -1103,7 +1109,10 @@ func (c *Client) allocSync() { | |||
if blockedAlloc, ok := c.blockedAllocations[alloc.ID]; ok && alloc.Terminated() { | |||
var prevAllocDir *allocdir.AllocDir | |||
if ar, ok := c.getAllocRunners()[alloc.ID]; ok { | |||
prevAllocDir = ar.GetAllocDir() | |||
tg := alloc.Job.LookupTaskGroup(alloc.TaskGroup) | |||
if tg != nil && tg.EphemeralDisk.Sticky { |
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.
Check that EphemeralDisk isn't nil
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.
@diptanu You missed this
@@ -1393,7 +1404,7 @@ func (c *Client) runAllocs(update *allocUpdates) { | |||
// previous allocation | |||
var prevAllocDir *allocdir.AllocDir | |||
tg := add.Job.LookupTaskGroup(add.TaskGroup) | |||
if tg != nil && tg.EphemeralDisk.Sticky == true && ar != nil { | |||
if tg != nil && tg.EphemeralDisk.Sticky && ar != nil { |
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.
Same here. You need to do a pull. There was a panic I fixed caused by not checking the EphemeralDisk block
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. |
No description provided.