Skip to content

Commit

Permalink
Minor fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
michaelawyu committed Jul 17, 2023
1 parent 32995fa commit c870d33
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 6 deletions.
2 changes: 1 addition & 1 deletion pkg/scheduler/framework/cyclestate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,6 @@ func TestPrepareScheduledOrBoundMap(t *testing.T) {

scheduleOrBoundMap := prepareScheduledOrBoundMap(scheduled, bound)
if diff := cmp.Diff(scheduleOrBoundMap, want); diff != "" {
t.Errorf("scheduledOrBoundMap diff (-got, +want): %s", diff)
t.Errorf("preparedScheduledOrBoundMap() scheduledOrBoundMap diff (-got, +want): %s", diff)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ const (
)

var (
doNotScheduleConstraintViolationReasonTemplate = "violated topology spread constraint %q (max skew %d)"
doNotScheduleConstraintViolationReasonTemplate = "violated doNotSchedule topology spread constraint %q (max skew %d)"
)

// Plugin is the scheduler plugin that enforces the
Expand Down
21 changes: 17 additions & 4 deletions pkg/scheduler/framework/plugins/topologyspreadconstraints/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@ func countByDomain(_ []fleetv1beta1.MemberCluster, _ framework.CycleStatePluginR
return &bindingCounterByDomain{}
}

// willViolatereturns whether producing one more binding in a domain would lead
// to violations; it will also return the skew change caused by the provisional placement.
// willViolate returns whether producing one more binding in a domain would lead
// to violations; it will also return the skew change (the delta between the max skew after setting
// up a placement and the one before) caused by the provisional placement.
func willViolate(_ bindingCounter, _ domainName, _ int) (violated bool, skewChange int, err error) {
// Not yet implemented.
return false, 0, nil
Expand Down Expand Up @@ -91,7 +92,13 @@ func evaluateAllConstraints(
// The cluster under inspection is part of the spread.

// Verify if the placement will violate the constraint.
violated, skewChange, err := willViolate(domainCounter, domainName(val), int(*constraint.MaxSkew))

// The default value for maxSkew is 1.
maxSkew := 1
if constraint.MaxSkew != nil {
maxSkew = int(*constraint.MaxSkew)
}
violated, skewChange, err := willViolate(domainCounter, domainName(val), maxSkew)
if err != nil {
return nil, nil, fmt.Errorf("failed to evaluate DoNotSchedule topology spread constraints: %w", err)
}
Expand Down Expand Up @@ -133,7 +140,13 @@ func evaluateAllConstraints(
// The cluster under inspection is part of the spread.

// Verify if the placement will violate the constraint.
violated, skewChange, err := willViolate(domainCounter, domainName(val), int(*constraint.MaxSkew))

// The default value for maxSkew is 1.
maxSkew := 1
if constraint.MaxSkew != nil {
maxSkew = int(*constraint.MaxSkew)
}
violated, skewChange, err := willViolate(domainCounter, domainName(val), maxSkew)
if err != nil {
return nil, nil, fmt.Errorf("failed to evaluate ScheduleAnyway topology spread constraints: %w", err)
}
Expand Down

0 comments on commit c870d33

Please sign in to comment.