luxolus
/
open-nomad
2
0
Fork 0
Code Issues 1 Pull Requests Packages Releases Wiki Activity

Wrap reschedule events in another struct and other review feedback

This commit is contained in:
Preetha Appan 2018-01-17 11:05:22 -06:00
parent 0a39f213e3
commit 8408835af8
No known key found for this signature in database
GPG Key ID: 9F7C19990A50EAFC
4 changed files with 205 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
nomad/structs/diff.go
View File

@ -234,6 +234,12 @@ func (tg *TaskGroup) Diff(other *TaskGroup, contextual bool) (*TaskGroupDiff, er
diff.Objects = append(diff.Objects, rDiff)
}
// Reschedule policy diff
reschedDiff := primitiveObjectDiff(tg.ReschedulePolicy, other.ReschedulePolicy, nil, "ReschedulePolicy", contextual)
if reschedDiff != nil {
diff.Objects = append(diff.Objects, reschedDiff)
}
// EphemeralDisk diff
diskDiff := primitiveObjectDiff(tg.EphemeralDisk, other.EphemeralDisk, nil, "EphemeralDisk", contextual)
if diskDiff != nil {

142
nomad/structs/diff_test.go
View File

@ -1494,6 +1494,148 @@ func TestTaskGroupDiff(t *testing.T) {
},
},
},
{
// ReschedulePolicy added
Old: &TaskGroup{},
New: &TaskGroup{
ReschedulePolicy: &ReschedulePolicy{
Attempts: 1,
Interval: 15 * time.Second,
},
},
Expected: &TaskGroupDiff{
Type: DiffTypeEdited,
Objects: []*ObjectDiff{
{
Type: DiffTypeAdded,
Name: "ReschedulePolicy",
Fields: []*FieldDiff{
{
Type: DiffTypeAdded,
Name: "Attempts",
Old: "",
New: "1",
},
{
Type: DiffTypeAdded,
Name: "Interval",
Old: "",
New: "15000000000",
},
},
},
},
},
},
{
// ReschedulePolicy deleted
Old: &TaskGroup{
ReschedulePolicy: &ReschedulePolicy{
Attempts: 1,
Interval: 15 * time.Second,
},
},
New: &TaskGroup{},
Expected: &TaskGroupDiff{
Type: DiffTypeEdited,
Objects: []*ObjectDiff{
{
Type: DiffTypeDeleted,
Name: "ReschedulePolicy",
Fields: []*FieldDiff{
{
Type: DiffTypeDeleted,
Name: "Attempts",
Old: "1",
New: "",
},
{
Type: DiffTypeDeleted,
Name: "Interval",
Old: "15000000000",
New: "",
},
},
},
},
},
},
{
// ReschedulePolicy edited
Old: &TaskGroup{
ReschedulePolicy: &ReschedulePolicy{
Attempts: 1,
Interval: 1 * time.Second,
},
},
New: &TaskGroup{
ReschedulePolicy: &ReschedulePolicy{
Attempts: 2,
Interval: 2 * time.Second,
},
},
Expected: &TaskGroupDiff{
Type: DiffTypeEdited,
Objects: []*ObjectDiff{
{
Type: DiffTypeEdited,
Name: "ReschedulePolicy",
Fields: []*FieldDiff{
{
Type: DiffTypeEdited,
Name: "Attempts",
Old: "1",
New: "2",
},
{
Type: DiffTypeEdited,
Name: "Interval",
Old: "1000000000",
New: "2000000000",
},
},
},
},
},
}, {
// ReschedulePolicy edited with context
Contextual: true,
Old: &TaskGroup{
ReschedulePolicy: &ReschedulePolicy{
Attempts: 1,
Interval: 1 * time.Second,
},
},
New: &TaskGroup{
ReschedulePolicy: &ReschedulePolicy{
Attempts: 1,
Interval: 2 * time.Second,
},
},
Expected: &TaskGroupDiff{
Type: DiffTypeEdited,
Objects: []*ObjectDiff{
{
Type: DiffTypeEdited,
Name: "ReschedulePolicy",
Fields: []*FieldDiff{
{
Type: DiffTypeNone,
Name: "Attempts",
Old: "1",
New: "1",
},
{
Type: DiffTypeEdited,
Name: "Interval",
Old: "1000000000",
New: "2000000000",
},
},
},
},
},
},
{
// Update strategy deleted
Old: &TaskGroup{

63
nomad/structs/structs.go
View File

@ -2521,11 +2521,11 @@ var (
)
var (
defaultServiceJobReschedulePolicy = ReschedulePolicy{
DefaultServiceJobReschedulePolicy = ReschedulePolicy{
Attempts: 2,
Interval: 1 * time.Hour,
}
defaultBatchJobReschedulePolicy = ReschedulePolicy{
DefaultBatchJobReschedulePolicy = ReschedulePolicy{
Attempts: 1,
Interval: 24 * time.Hour,
}
@ -2632,26 +2632,25 @@ func (r *ReschedulePolicy) Copy() *ReschedulePolicy {
}
func (r *ReschedulePolicy) Validate() error {
var mErr multierror.Error
// Check for ambiguous/confusing settings
if r.Attempts < 0 {
multierror.Append(&mErr, fmt.Errorf("Attempts must be >= 0 (got %v)", r.Attempts))
}
if r != nil && r.Attempts > 0 {
var mErr multierror.Error
// Check for ambiguous/confusing settings
if r.Interval.Nanoseconds() < ReschedulePolicyMinInterval.Nanoseconds() {
multierror.Append(&mErr, fmt.Errorf("Interval cannot be less than %v (got %v)", RestartPolicyMinInterval, r.Interval))
}
if r.Interval.Nanoseconds() < ReschedulePolicyMinInterval.Nanoseconds() {
multierror.Append(&mErr, fmt.Errorf("Interval cannot be less than %v (got %v)", RestartPolicyMinInterval, r.Interval))
return mErr.ErrorOrNil()
}
return mErr.ErrorOrNil()
return nil
}
func NewReshedulePolicy(jobType string) *ReschedulePolicy {
switch jobType {
case JobTypeService, JobTypeSystem:
rp := defaultServiceJobReschedulePolicy
case JobTypeService:
rp := DefaultServiceJobReschedulePolicy
return &rp
case JobTypeBatch:
rp := defaultBatchJobReschedulePolicy
rp := DefaultBatchJobReschedulePolicy
return &rp
}
return nil
@ -4918,7 +4917,13 @@ type DeploymentStatusUpdate struct {
StatusDescription string
}
// RescheduleTracker encapsulates previous reschedule events
type RescheduleTracker struct {
Events []*RescheduleEvent
}
// RescheduleEvent is used to keep track of previous attempts at rescheduling an allocation
type RescheduleEvent struct {
// RescheduleTime is the timestamp of a reschedule attempt
RescheduleTime int64
@ -4929,11 +4934,11 @@ type RescheduleTracker struct {
PrevNodeID string
}
func (rt *RescheduleTracker) Copy() *RescheduleTracker {
func (rt *RescheduleEvent) Copy() *RescheduleEvent {
if rt == nil {
return nil
}
copy := new(RescheduleTracker)
copy := new(RescheduleEvent)
*copy = *rt
return copy
}
@ -5038,7 +5043,7 @@ type Allocation struct {
ModifyTime int64
// RescheduleTrackers captures details of previous reschedule attempts of the allocation
RescheduleTrackers []*RescheduleTracker
RescheduleTracker *RescheduleTracker
}
// Index returns the index of the allocation. If the allocation is from a task
@ -5097,9 +5102,9 @@ func (a *Allocation) copyImpl(job bool) *Allocation {
na.TaskStates = ts
}
if a.RescheduleTrackers != nil {
var rescheduleTrackers []*RescheduleTracker
for _, tracker := range a.RescheduleTrackers {
if a.RescheduleTracker != nil {
var rescheduleTrackers []*RescheduleEvent
for _, tracker := range a.RescheduleTracker.Events {
rescheduleTrackers = append(rescheduleTrackers, tracker.Copy())
}
}
@ -5126,8 +5131,8 @@ func (a *Allocation) TerminalStatus() bool {
}
// ShouldReschedule returns if the allocation is eligible to be rescheduled according
// to its status and ReschedulePolicy
func (a *Allocation) ShouldReschedule(reschedulePolicy *ReschedulePolicy) bool {
// to its status and ReschedulePolicy given its failure time
func (a *Allocation) ShouldReschedule(reschedulePolicy *ReschedulePolicy, failTime time.Time) bool {
// First check the desired state
switch a.DesiredStatus {
case AllocDesiredStatusStop, AllocDesiredStatusEvict:
@ -5136,7 +5141,7 @@ func (a *Allocation) ShouldReschedule(reschedulePolicy *ReschedulePolicy) bool {
}
switch a.ClientStatus {
case AllocClientStatusFailed:
return a.RescheduleEligible(reschedulePolicy)
return a.RescheduleEligible(reschedulePolicy, failTime)
default:
return false
}
@ -5144,7 +5149,7 @@ func (a *Allocation) ShouldReschedule(reschedulePolicy *ReschedulePolicy) bool {
// RescheduleEligible returns if the allocation is eligible to be rescheduled according
// to its ReschedulePolicy and the current state of its reschedule trackers
func (a *Allocation) RescheduleEligible(reschedulePolicy *ReschedulePolicy) bool {
func (a *Allocation) RescheduleEligible(reschedulePolicy *ReschedulePolicy, failTime time.Time) bool {
if reschedulePolicy == nil {
return false
}
@ -5154,13 +5159,13 @@ func (a *Allocation) RescheduleEligible(reschedulePolicy *ReschedulePolicy) bool
if attempts == 0 {
return false
}
if a.RescheduleTrackers == nil && attempts > 0 {
if (a.RescheduleTracker == nil || len(a.RescheduleTracker.Events) == 0) && attempts > 0 {
return true
}
attempted := 0
for j := len(a.RescheduleTrackers) - 1; j >= 0; j-- {
lastAttempt := a.RescheduleTrackers[j].RescheduleTime
timeDiff := time.Now().UTC().UnixNano() - lastAttempt
for j := len(a.RescheduleTracker.Events) - 1; j >= 0; j-- {
lastAttempt := a.RescheduleTracker.Events[j].RescheduleTime
timeDiff := failTime.UTC().UnixNano() - lastAttempt
if timeDiff < interval.Nanoseconds() {
attempted += 1
}
@ -5477,7 +5482,7 @@ const (
EvalTriggerDeploymentWatcher = "deployment-watcher"
EvalTriggerFailedFollowUp = "failed-follow-up"
EvalTriggerMaxPlans = "max-plan-attempts"
EvalTriggerRetryFailedAlloc = "replacing-after-failure"
EvalTriggerRetryFailedAlloc = "alloc-failure"
)
const (

34
nomad/structs/structs_test.go
View File

@ -2434,7 +2434,7 @@ func TestReschedulePolicy_Validate(t *testing.T) {
},
{
ReschedulePolicy: &ReschedulePolicy{-1, 5 * time.Minute},
err: fmt.Errorf("Attempts must be >= 0 (got -1)"),
err: nil,
},
{
ReschedulePolicy: &ReschedulePolicy{1, 1 * time.Second},
@ -2682,18 +2682,22 @@ func TestAllocation_Terminated(t *testing.T) {
func TestAllocation_ShouldReschedule(t *testing.T) {
type testCase struct {
Desc string
FailTime time.Time
ClientStatus string
DesiredStatus string
ReschedulePolicy *ReschedulePolicy
RescheduleTrackers []*RescheduleTracker
RescheduleTrackers []*RescheduleEvent
ShouldReschedule bool
}
fail := time.Now()
harness := []testCase{
{
Desc: "Reschedule when desired state is stop",
ClientStatus: AllocClientStatusPending,
DesiredStatus: AllocDesiredStatusStop,
FailTime: fail,
ReschedulePolicy: nil,
ShouldReschedule: false,
},
@ -2701,6 +2705,7 @@ func TestAllocation_ShouldReschedule(t *testing.T) {
Desc: "Reschedule when client status is complete",
ClientStatus: AllocClientStatusComplete,
DesiredStatus: AllocDesiredStatusRun,
FailTime: fail,
ReschedulePolicy: nil,
ShouldReschedule: false,
},
@ -2708,6 +2713,7 @@ func TestAllocation_ShouldReschedule(t *testing.T) {
Desc: "Reschedule with nil reschedule policy",
ClientStatus: AllocClientStatusFailed,
DesiredStatus: AllocDesiredStatusRun,
FailTime: fail,
ReschedulePolicy: nil,
ShouldReschedule: false,
},
@ -2715,6 +2721,7 @@ func TestAllocation_ShouldReschedule(t *testing.T) {
Desc: "Reschedule when client status is complete",
ClientStatus: AllocClientStatusComplete,
DesiredStatus: AllocDesiredStatusRun,
FailTime: fail,
ReschedulePolicy: nil,
ShouldReschedule: false,
},
@ -2722,6 +2729,7 @@ func TestAllocation_ShouldReschedule(t *testing.T) {
Desc: "Reschedule with policy when client status complete",
ClientStatus: AllocClientStatusComplete,
DesiredStatus: AllocDesiredStatusRun,
FailTime: fail,
ReschedulePolicy: &ReschedulePolicy{1, 1 * time.Minute},
ShouldReschedule: false,
},
@ -2729,6 +2737,7 @@ func TestAllocation_ShouldReschedule(t *testing.T) {
Desc: "Reschedule with no previous attempts",
ClientStatus: AllocClientStatusFailed,
DesiredStatus: AllocDesiredStatusRun,
FailTime: fail,
ReschedulePolicy: &ReschedulePolicy{1, 1 * time.Minute},
ShouldReschedule: true,
},
@ -2737,9 +2746,10 @@ func TestAllocation_ShouldReschedule(t *testing.T) {
ClientStatus: AllocClientStatusFailed,
DesiredStatus: AllocDesiredStatusRun,
ReschedulePolicy: &ReschedulePolicy{2, 5 * time.Minute},
RescheduleTrackers: []*RescheduleTracker{
FailTime: fail,
RescheduleTrackers: []*RescheduleEvent{
{
RescheduleTime: time.Now().Add(-1 * time.Minute).UTC().UnixNano(),
RescheduleTime: fail.Add(-1 * time.Minute).UTC().UnixNano(),
},
},
ShouldReschedule: true,
@ -2748,10 +2758,11 @@ func TestAllocation_ShouldReschedule(t *testing.T) {
Desc: "Reschedule with too old previous attempts",
ClientStatus: AllocClientStatusFailed,
DesiredStatus: AllocDesiredStatusRun,
FailTime: fail,
ReschedulePolicy: &ReschedulePolicy{1, 5 * time.Minute},
RescheduleTrackers: []*RescheduleTracker{
RescheduleTrackers: []*RescheduleEvent{
{
RescheduleTime: time.Now().Add(-6 * time.Minute).UTC().UnixNano(),
RescheduleTime: fail.Add(-6 * time.Minute).UTC().UnixNano(),
},
},
ShouldReschedule: true,
@ -2760,13 +2771,14 @@ func TestAllocation_ShouldReschedule(t *testing.T) {
Desc: "Reschedule with no leftover attempts",
ClientStatus: AllocClientStatusFailed,
DesiredStatus: AllocDesiredStatusRun,
FailTime: fail,
ReschedulePolicy: &ReschedulePolicy{2, 5 * time.Minute},
RescheduleTrackers: []*RescheduleTracker{
RescheduleTrackers: []*RescheduleEvent{
{
RescheduleTime: time.Now().Add(-3 * time.Minute).UTC().UnixNano(),
RescheduleTime: fail.Add(-3 * time.Minute).UTC().UnixNano(),
},
{
RescheduleTime: time.Now().Add(-4 * time.Minute).UTC().UnixNano(),
RescheduleTime: fail.Add(-4 * time.Minute).UTC().UnixNano(),
},
},
ShouldReschedule: false,
@ -2777,10 +2789,10 @@ func TestAllocation_ShouldReschedule(t *testing.T) {
alloc := Allocation{}
alloc.DesiredStatus = state.DesiredStatus
alloc.ClientStatus = state.ClientStatus
alloc.RescheduleTrackers = state.RescheduleTrackers
alloc.RescheduleTracker = &RescheduleTracker{state.RescheduleTrackers}
t.Run(state.Desc, func(t *testing.T) {
if got := alloc.ShouldReschedule(state.ReschedulePolicy); got != state.ShouldReschedule {
if got := alloc.ShouldReschedule(state.ReschedulePolicy, state.FailTime); got != state.ShouldReschedule {
t.Fatalf("expected %v but got %v", state.ShouldReschedule, got)
}
})