Skip to content
This repository has been archived by the owner on Apr 2, 2023. It is now read-only.

Sla restart instances #12

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

lenhattan86
Copy link
Collaborator

SlaRestartInstances api

How it works:

  • pull the existing task config
  • restart instances using jobUpdate(taskConfig, targetInstances)

Tests:

  • local integration tests.
  • Unit tests to be added.

@lenhattan86
Copy link
Collaborator Author

@ridv can you take a look if it works?
then I will add unit tests later.

Copy link
Contributor

@ridv ridv left a 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.

realis.go Show resolved Hide resolved
@lenhattan86
Copy link
Collaborator Author

@ridv added the unit test for this abandoned PR :)

Copy link
Contributor

@ridv ridv left a 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)
Copy link
Contributor

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)
Copy link
Contributor

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 :/

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.

2 participants