Merge pull request #1178 from munnerz/fix-scheduler-bug

Fix bug in challenge scheduler causing invalid results
This commit is contained in:
jetstack-bot 2019-01-08 14:25:19 +00:00 committed by GitHub
commit 920b6ffcdb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 39 additions and 8 deletions

View File

@ -124,7 +124,7 @@ func (s *Scheduler) determineChallengeCandidates(allChallenges []*cmapi.Challeng
// If there are any already in-progress challenges for a domain and type,
// filter them out.
candidates := quickFilterChallenges(dedupedCandidates, func(ch *cmapi.Challenge) bool {
candidates := filterChallenges(dedupedCandidates, func(ch *cmapi.Challenge) bool {
for _, inPCh := range inProgress {
if compareChallenges(ch, inPCh) == 0 {
glog.V(6).Infof("There is already a challenge processing for domain %q (type %q)", ch.Spec.DNSName, ch.Spec.Type)
@ -149,7 +149,7 @@ func sortChallengesByTimestamp(chs []*cmapi.Challenge) {
// notProcessingChallenges will filter out challenges from the given slice
// that have status.processing set to true.
func notProcessingChallenges(chs []*cmapi.Challenge) []*cmapi.Challenge {
return quickFilterChallenges(chs, func(ch *cmapi.Challenge) bool {
return filterChallenges(chs, func(ch *cmapi.Challenge) bool {
return !ch.Status.Processing
})
}
@ -157,7 +157,7 @@ func notProcessingChallenges(chs []*cmapi.Challenge) []*cmapi.Challenge {
// processingChallenges will filter out challenges from the given slice
// that have status.processing set to false.
func processingChallenges(chs []*cmapi.Challenge) []*cmapi.Challenge {
return quickFilterChallenges(chs, func(ch *cmapi.Challenge) bool {
return filterChallenges(chs, func(ch *cmapi.Challenge) bool {
return ch.Status.Processing
})
}
@ -165,20 +165,18 @@ func processingChallenges(chs []*cmapi.Challenge) []*cmapi.Challenge {
// incompleteChallenges will filter out challenges from the given slice
// that are in a 'final' state
func incompleteChallenges(chs []*cmapi.Challenge) []*cmapi.Challenge {
return quickFilterChallenges(chs, func(ch *cmapi.Challenge) bool {
return filterChallenges(chs, func(ch *cmapi.Challenge) bool {
return !acme.IsFinalState(ch.Status.State)
})
}
// https://github.com/golang/go/wiki/SliceTricks#Filtering-without-allocating
func quickFilterChallenges(chs []*cmapi.Challenge, fn func(ch *cmapi.Challenge) bool) []*cmapi.Challenge {
ret := chs[:0]
func filterChallenges(chs []*cmapi.Challenge, fn func(ch *cmapi.Challenge) bool) []*cmapi.Challenge {
ret := []*cmapi.Challenge{}
for _, ch := range chs {
if fn(ch) {
ret = append(ret, ch)
}
}
return ret
}

View File

@ -227,6 +227,33 @@ func TestScheduleN(t *testing.T) {
gen.SetChallengeType("http01")),
},
},
// this test case replicates a failure seen in CI
{
name: "schedule a challenge when other challenges are already in progress",
n: 5,
challenges: []*cmapi.Challenge{
gen.Challenge("test1-0",
gen.SetChallengeDNSName("rvrko.certmanager.kubernetes.network"),
gen.SetChallengeType("dns-01"),
gen.SetChallengeWildcard(true)),
gen.Challenge("test1-1",
gen.SetChallengeDNSName("rvrko.certmanager.kubernetes.network"),
gen.SetChallengeType("dns-01"),
gen.SetChallengeWildcard(false),
// the non-wildcard version *is* processing
gen.SetChallengeProcessing(true)),
gen.Challenge("should-schedule",
gen.SetChallengeDNSName("aodob.certmanager.kubernetes.network"),
gen.SetChallengeType("dns-01"),
gen.SetChallengeWildcard(true)),
},
expected: []*cmapi.Challenge{
gen.Challenge("should-schedule",
gen.SetChallengeDNSName("aodob.certmanager.kubernetes.network"),
gen.SetChallengeType("dns-01"),
gen.SetChallengeWildcard(true)),
},
},
{
name: "don't schedule when total number of scheduled challenges exceeds global maximum",
n: 5,

View File

@ -64,6 +64,12 @@ func SetChallengePresented(p bool) ChallengeModifier {
}
}
func SetChallengeWildcard(p bool) ChallengeModifier {
return func(ch *v1alpha1.Challenge) {
ch.Spec.Wildcard = p
}
}
func SetChallengeState(s v1alpha1.State) ChallengeModifier {
return func(ch *v1alpha1.Challenge) {
ch.Status.State = s