-
Notifications
You must be signed in to change notification settings - Fork 6
Sla restart instances #12
base: master
Are you sure you want to change the base?
Sla restart instances #12
Conversation
@ridv can you take a look if it works? |
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. I think there's an opportunity to save some network IO.
@ridv added the unit test for this abandoned PR :) |
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.
This may not be feasible with the approach we thought of due to canary deployments :/
jobUpdate := JobUpdateFromConfig(jobConfig.TaskConfig) | ||
jobUpdate. | ||
SlaAware(true). | ||
InstanceCount(jobConfig.InstanceCount) |
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 have to add a small change here to make the scheduler change the config. Let's add a label called `io.github.aurora.restart" with value +1 of the previous value.
jobUpdate.SlaPolicy(slaPolicy) | ||
} | ||
for _, v := range instances { | ||
jobUpdate.AddInstanceRange(v, v) |
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 don't think we have to set this to anything if we're going to operate on all instances. However, once case here that's pretty interesting is a canary deployment. In fact, that may break this entire idea since each task is tied to a different config in that case :/
SlaRestartInstances api
How it works:
Tests: