Skip to content

Commit

Permalink
consistently render requirements that we rebuild (#1731)
Browse files Browse the repository at this point in the history
Fixes #1729
  • Loading branch information
tzneal authored Apr 28, 2022
1 parent ee450bc commit e7a610b
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 2 deletions.
26 changes: 24 additions & 2 deletions pkg/apis/provisioning/v1alpha5/requirements.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,10 +121,10 @@ func (r *Requirements) rebuild() {
}
if values.IsComplement() {
req.Operator = v1.NodeSelectorOpNotIn
req.Values = values.ComplementValues().UnsortedList()
req.Values = values.ComplementValues().List()
} else {
req.Operator = v1.NodeSelectorOpIn
req.Values = values.Values().UnsortedList()
req.Values = values.Values().List()
}
r.Requirements = append(r.Requirements, req)
}
Expand All @@ -136,6 +136,28 @@ func (r *Requirements) rebuild() {
r.Requirements = append(r.Requirements, req)
}
}

sort.Slice(r.Requirements, func(a, b int) bool {
lhs := r.Requirements[a]
rhs := r.Requirements[b]
if lhs.Key != rhs.Key {
return lhs.Key < rhs.Key
}
if lhs.Operator != rhs.Operator {
return lhs.Operator < rhs.Operator
}
if len(lhs.Values) != len(rhs.Values) {
return len(lhs.Values) < len(rhs.Values)
}

// lengths are the same now
for i := 0; i < len(lhs.Values); i++ {
if lhs.Values[i] != rhs.Values[i] {
return lhs.Values[i] < rhs.Values[i]
}
}
return false
})
}

// Keys returns unique set of the label keys from the requirements
Expand Down
34 changes: 34 additions & 0 deletions pkg/apis/provisioning/v1alpha5/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package v1alpha5

import (
"context"
"github.com/aws/karpenter/pkg/utils/pretty"
"strings"
"testing"

Expand Down Expand Up @@ -359,3 +360,36 @@ var _ = Describe("Validation", func() {
})
})
})

var _ = Describe("Consistency", func() {
It("should return requirements in a consistent manner", func() {
nodeReqs := []v1.NodeSelectorRequirement{
{
Key: v1.LabelHostname,
Operator: v1.NodeSelectorOpIn,
Values: []string{"a", "b", "c", "d", "e", "f", "g"},
},
{
Key: v1.LabelTopologyZone,
Operator: v1.NodeSelectorOpIn,
Values: []string{"zone-1", "zone-2", "zone-3", "zone-4"},
},
{
Key: v1.LabelArchStable,
Operator: v1.NodeSelectorOpIn,
Values: []string{"amd64", "arm64"},
},
}

// generate an initial rendering of the requirements
reqs := Requirements{}.Add(nodeReqs...)
expected := pretty.Concise(reqs)

// all other renderings should be identical
for i := 0; i < 50; i++ {
newRequirements := Requirements{}.Add(nodeReqs...)
got := pretty.Concise(newRequirements)
Expect(got).To(Equal(expected))
}
})
})

0 comments on commit e7a610b

Please sign in to comment.