Skip to content
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.

Sign up for GitHub

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

Jump to bottom

prevent predicatesOrdering from escaping from UT #77576

Merged
merged 1 commit into from
May 9, 2019
Merged

prevent predicatesOrdering from escaping from UT #77576

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 0 additions & 5 deletions pkg/scheduler/algorithm/predicates/predicates.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,11 +173,6 @@ func Ordering() []string {
return predicatesOrdering
}

// SetPredicatesOrdering sets the ordering of predicates.
func SetPredicatesOrdering(names []string) {
predicatesOrdering = names
}

// GetPersistentVolumeInfo returns a persistent volume object by PV ID.
func (c *CachedPersistentVolumeInfo) GetPersistentVolumeInfo(pvID string) (*v1.PersistentVolume, error) {
return c.Get(pvID)
Expand Down
10 changes: 10 additions & 0 deletions pkg/scheduler/algorithm/predicates/utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,3 +77,13 @@ func portsConflict(existingPorts schedulernodeinfo.HostPortInfo, wantPorts []*v1

return false
}

// SetPredicatesOrderingDuringTest sets the predicatesOrdering to the specified
// value, and returns a function that restores the original value.
func SetPredicatesOrderingDuringTest(value []string) func() {
origVal := predicatesOrdering
predicatesOrdering = value
return func() {
predicatesOrdering = origVal
}
}
10 changes: 6 additions & 4 deletions pkg/scheduler/core/generic_scheduler_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ func TestSelectHost(t *testing.T) {
}

func TestGenericScheduler(t *testing.T) {
algorithmpredicates.SetPredicatesOrdering(order)
defer algorithmpredicates.SetPredicatesOrderingDuringTest(order)()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this is clever, but a bit unreadable. One might think that the whole line is defered and executed after the function returns. Optionally, you can store and defer the returned function, but it is up to you.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Credit goes to @liggitt :)

defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.XYZ, true)()

tests := []struct {
name string
predicates map[string]algorithmpredicates.FitPredicate
Expand Down Expand Up @@ -479,7 +479,6 @@ func TestGenericScheduler(t *testing.T) {

// makeScheduler makes a simple genericScheduler for testing.
func makeScheduler(predicates map[string]algorithmpredicates.FitPredicate, nodes []*v1.Node) *genericScheduler {
algorithmpredicates.SetPredicatesOrdering(order)
cache := internalcache.New(time.Duration(0), wait.NeverStop)
fwk, _ := framework.NewFramework(EmptyPluginRegistry, nil)
for _, n := range nodes {
Expand All @@ -503,6 +502,7 @@ func makeScheduler(predicates map[string]algorithmpredicates.FitPredicate, nodes
}

func TestFindFitAllError(t *testing.T) {
defer algorithmpredicates.SetPredicatesOrderingDuringTest(order)()
predicates := map[string]algorithmpredicates.FitPredicate{"true": truePredicate, "matches": matchesPredicate}
nodes := makeNodeList([]string{"3", "2", "1"})
scheduler := makeScheduler(predicates, nodes)
Expand Down Expand Up @@ -531,6 +531,7 @@ func TestFindFitAllError(t *testing.T) {
}

func TestFindFitSomeError(t *testing.T) {
defer algorithmpredicates.SetPredicatesOrderingDuringTest(order)()
predicates := map[string]algorithmpredicates.FitPredicate{"true": truePredicate, "matches": matchesPredicate}
nodes := makeNodeList([]string{"3", "2", "1"})
scheduler := makeScheduler(predicates, nodes)
Expand Down Expand Up @@ -846,7 +847,7 @@ var startTime20190107 = metav1.Date(2019, 1, 7, 1, 1, 1, 0, time.UTC)
// TestSelectNodesForPreemption tests selectNodesForPreemption. This test assumes
// that podsFitsOnNode works correctly and is tested separately.
func TestSelectNodesForPreemption(t *testing.T) {
algorithmpredicates.SetPredicatesOrdering(order)
defer algorithmpredicates.SetPredicatesOrderingDuringTest(order)()
tests := []struct {
name string
predicates map[string]algorithmpredicates.FitPredicate
Expand Down Expand Up @@ -1005,7 +1006,7 @@ func TestSelectNodesForPreemption(t *testing.T) {

// TestPickOneNodeForPreemption tests pickOneNodeForPreemption.
func TestPickOneNodeForPreemption(t *testing.T) {
algorithmpredicates.SetPredicatesOrdering(order)
defer algorithmpredicates.SetPredicatesOrderingDuringTest(order)()
tests := []struct {
name string
predicates map[string]algorithmpredicates.FitPredicate
Expand Down Expand Up @@ -1321,6 +1322,7 @@ func TestNodesWherePreemptionMightHelp(t *testing.T) {
}

func TestPreempt(t *testing.T) {
defer algorithmpredicates.SetPredicatesOrderingDuringTest(order)()
failedPredMap := FailedPredicateMap{
"machine1": []algorithmpredicates.PredicateFailureReason{algorithmpredicates.NewInsufficientResourceError(v1.ResourceMemory, 1000, 500, 300)},
"machine2": []algorithmpredicates.PredicateFailureReason{algorithmpredicates.ErrDiskConflict},
Expand Down