
Automatic merge from submit-queue Do not list CronJob unmet starting times beyond deadline **What this PR does / why we need it**: See #36311. `getRecentUnmetScheduleTimes` gives up after 100 unmet times to avoid wasting too much CPU or memory generating all the times, as it generates them sequentially. When concurrency is forbidden, this is conceptually un-necessary: we only need the last unmet start time. This suggests that when concurrency is forbidden, we could generate times by going backward in time from now. This is not very practical as CronJob currently relies on a package that only provides `Next` and no `Prev`. Hand-cooking a `Prev` does not seem like a good idea. I could submit a PR to the cron library to add a `Prev` method, and use that when concurrency is forbidden through something like `getLastUnmetScheduleTime`. This would be `O(1)` and there would be no limit involved. (edit: actually, even for the other concurrency settings, we only start the last unmet start times -- there is a `TODO` in the controller to actually start all of them, but that is not implemented at the moment. This means the solution would apply, at least temporarily, to all concurrency settings). cc @soltysh what do you think? In the meantime, I would suggest to do something simple. Currently, the user has no way to configure anything to ensure that his CronJob will not get stuck if one job takes more that 100 unmet times. `getRecentUnmetScheduleTimes` starts with an initial time corresponding to the last start (or to the creation of the CronJob, if nothing has started yet). However, when `StartingDeadlineSeconds` is set, the controller will not start anything that is older than the deadline, so if the last start is way beyond the deadline, we are generating potentially lots of unmet start times that will not be considered by the scheduler for scheduling anyway. Consider a job running every minute, where the last instance has taken 120 minutes. This means there are more than 100 unmet times when we start counting from the last start time. **The PR makes `getRecentUnmetScheduleTimes` only consider times that do not fall beyond the deadline.** Here, the CronJob can be configured with a `StartingDeadlineSeconds` of, say, 10 minutes. After the 120min job has run, `getRecentUnmetScheduleTimes` will only consider the times in the last 10 minutes from now, and will not get stuck. As a side note on the max. number of unmet times to use as limits in terms of CPU used by the controller: I have run a quick benchmark on my i7 mac. Schedules corresponding to "once a week" tend to be more expensive to generate unmet times for. Just FYI. ``` +--------------+---------------+--------------+ | SCHEDULE | MISSED STARTS | TIMING | +--------------+---------------+--------------+ | */1 * * * ? | 100 | 383.645µs | | */30 * * * ? | 100 | 354.765µs | | 30 1 * * ? | 100 | 1.065124ms | | 30 1 * * 0 | 100 | 1.80034ms | | */1 * * * ? | 500 | 1.341365ms | | */30 * * * ? | 500 | 1.814441ms | | 30 1 * * ? | 500 | 8.475012ms | | 30 1 * * 0 | 500 | 10.020613ms | | */1 * * * ? | 1000 | 2.551697ms | | */30 * * * ? | 1000 | 4.075813ms | | 30 1 * * ? | 1000 | 17.674945ms | | 30 1 * * 0 | 1000 | 19.149324ms | | */1 * * * ? | 10000 | 25.725531ms | | */30 * * * ? | 10000 | 87.520022ms | | 30 1 * * ? | 10000 | 174.29216ms | | 30 1 * * 0 | 10000 | 196.565748ms | +--------------+---------------+--------------+ ``` using ```.go package main import ( "fmt" "time" "os" "strconv" "github.com/robfig/cron" "github.com/olekukonko/tablewriter" ) func timeSchedule(schedule string, iterations int) (time.Duration) { sched, err := cron.ParseStandard(schedule) if err != nil { panic(fmt.Sprintf("Unparseable schedule: %s", err)) } start := time.Now() t := time.Now() for i := 1; i <= iterations; i++ { t = sched.Next(t) } return time.Since(start) } func main() { table := tablewriter.NewWriter(os.Stdout) table.SetHeader([]string{"Schedule", "Missed starts", "Timing"}) schedules := []string{"*/1 * * * ?", "*/30 * * * ?", "30 1 * * ?", "30 1 * * 0"} iteration_nums := []int{100, 500, 1000, 10000} for _, iterations := range iteration_nums { for _, schedule := range schedules { table.Append([]string{schedule, strconv.Itoa(iterations), timeSchedule(schedule, iterations).String()}) } } table.Render() } ``` **Which issue this PR fixes**: fixes #36311 **Special notes for your reviewer**: **Release note**: ```release-note ```
391 lines
12 KiB
Go
391 lines
12 KiB
Go
/*
|
|
Copyright 2016 The Kubernetes Authors.
|
|
|
|
Licensed under the Apache License, Version 2.0 (the "License");
|
|
you may not use this file except in compliance with the License.
|
|
You may obtain a copy of the License at
|
|
|
|
http://www.apache.org/licenses/LICENSE-2.0
|
|
|
|
Unless required by applicable law or agreed to in writing, software
|
|
distributed under the License is distributed on an "AS IS" BASIS,
|
|
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
|
See the License for the specific language governing permissions and
|
|
limitations under the License.
|
|
*/
|
|
|
|
package cronjob
|
|
|
|
import (
|
|
"strings"
|
|
"testing"
|
|
"time"
|
|
|
|
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
|
|
"k8s.io/apimachinery/pkg/types"
|
|
"k8s.io/kubernetes/pkg/api/v1"
|
|
batch "k8s.io/kubernetes/pkg/apis/batch/v2alpha1"
|
|
)
|
|
|
|
func TestGetJobFromTemplate(t *testing.T) {
|
|
// getJobFromTemplate() needs to take the job template and copy the labels and annotations
|
|
// and other fields, and add a created-by reference.
|
|
|
|
var one int64 = 1
|
|
var no bool = false
|
|
|
|
sj := batch.CronJob{
|
|
ObjectMeta: v1.ObjectMeta{
|
|
Name: "mycronjob",
|
|
Namespace: "snazzycats",
|
|
UID: types.UID("1a2b3c"),
|
|
SelfLink: "/apis/batch/v1/namespaces/snazzycats/jobs/mycronjob",
|
|
},
|
|
Spec: batch.CronJobSpec{
|
|
Schedule: "* * * * ?",
|
|
ConcurrencyPolicy: batch.AllowConcurrent,
|
|
JobTemplate: batch.JobTemplateSpec{
|
|
ObjectMeta: v1.ObjectMeta{
|
|
Labels: map[string]string{"a": "b"},
|
|
Annotations: map[string]string{"x": "y"},
|
|
},
|
|
Spec: batch.JobSpec{
|
|
ActiveDeadlineSeconds: &one,
|
|
ManualSelector: &no,
|
|
Template: v1.PodTemplateSpec{
|
|
ObjectMeta: v1.ObjectMeta{
|
|
Labels: map[string]string{
|
|
"foo": "bar",
|
|
},
|
|
},
|
|
Spec: v1.PodSpec{
|
|
Containers: []v1.Container{
|
|
{Image: "foo/bar"},
|
|
},
|
|
},
|
|
},
|
|
},
|
|
},
|
|
},
|
|
}
|
|
|
|
var job *batch.Job
|
|
job, err := getJobFromTemplate(&sj, time.Time{})
|
|
if err != nil {
|
|
t.Errorf("Did not expect error: %s", err)
|
|
}
|
|
if !strings.HasPrefix(job.ObjectMeta.Name, "mycronjob-") {
|
|
t.Errorf("Wrong Name")
|
|
}
|
|
if len(job.ObjectMeta.Labels) != 1 {
|
|
t.Errorf("Wrong number of labels")
|
|
}
|
|
if len(job.ObjectMeta.Annotations) != 2 {
|
|
t.Errorf("Wrong number of annotations")
|
|
}
|
|
v, ok := job.ObjectMeta.Annotations[v1.CreatedByAnnotation]
|
|
if !ok {
|
|
t.Errorf("Missing created-by annotation")
|
|
}
|
|
expectedCreatedBy := `{"kind":"SerializedReference","apiVersion":"v1","reference":{"kind":"CronJob","namespace":"snazzycats","name":"mycronjob","uid":"1a2b3c","apiVersion":"batch"}}
|
|
`
|
|
if len(v) != len(expectedCreatedBy) {
|
|
t.Errorf("Wrong length for created-by annotation, expected %v got %v", len(expectedCreatedBy), len(v))
|
|
}
|
|
if v != expectedCreatedBy {
|
|
t.Errorf("Wrong value for created-by annotation, expected %v got %v", expectedCreatedBy, v)
|
|
}
|
|
}
|
|
|
|
func TestGetParentUIDFromJob(t *testing.T) {
|
|
j := &batch.Job{
|
|
ObjectMeta: v1.ObjectMeta{
|
|
Name: "foobar",
|
|
Namespace: v1.NamespaceDefault,
|
|
},
|
|
Spec: batch.JobSpec{
|
|
Selector: &metav1.LabelSelector{
|
|
MatchLabels: map[string]string{"foo": "bar"},
|
|
},
|
|
Template: v1.PodTemplateSpec{
|
|
ObjectMeta: v1.ObjectMeta{
|
|
Labels: map[string]string{
|
|
"foo": "bar",
|
|
},
|
|
},
|
|
Spec: v1.PodSpec{
|
|
Containers: []v1.Container{
|
|
{Image: "foo/bar"},
|
|
},
|
|
},
|
|
},
|
|
},
|
|
Status: batch.JobStatus{
|
|
Conditions: []batch.JobCondition{{
|
|
Type: batch.JobComplete,
|
|
Status: v1.ConditionTrue,
|
|
}},
|
|
},
|
|
}
|
|
{
|
|
// Case 1: No UID annotation
|
|
_, found := getParentUIDFromJob(*j)
|
|
|
|
if found {
|
|
t.Errorf("Unexpectedly found uid")
|
|
}
|
|
}
|
|
{
|
|
// Case 2: Has UID annotation
|
|
j.ObjectMeta.Annotations = map[string]string{v1.CreatedByAnnotation: `{"kind":"SerializedReference","apiVersion":"v1","reference":{"kind":"CronJob","namespace":"default","name":"pi","uid":"5ef034e0-1890-11e6-8935-42010af0003e","apiVersion":"extensions","resourceVersion":"427339"}}`}
|
|
|
|
expectedUID := types.UID("5ef034e0-1890-11e6-8935-42010af0003e")
|
|
|
|
uid, found := getParentUIDFromJob(*j)
|
|
if !found {
|
|
t.Errorf("Unexpectedly did not find uid")
|
|
} else if uid != expectedUID {
|
|
t.Errorf("Wrong UID: %v", uid)
|
|
}
|
|
}
|
|
|
|
}
|
|
|
|
func TestGroupJobsByParent(t *testing.T) {
|
|
uid1 := types.UID("11111111-1111-1111-1111-111111111111")
|
|
uid2 := types.UID("22222222-2222-2222-2222-222222222222")
|
|
uid3 := types.UID("33333333-3333-3333-3333-333333333333")
|
|
createdBy1 := map[string]string{v1.CreatedByAnnotation: `{"kind":"SerializedReference","apiVersion":"v1","reference":{"kind":"CronJob","namespace":"x","name":"pi","uid":"11111111-1111-1111-1111-111111111111","apiVersion":"extensions","resourceVersion":"111111"}}`}
|
|
createdBy2 := map[string]string{v1.CreatedByAnnotation: `{"kind":"SerializedReference","apiVersion":"v1","reference":{"kind":"CronJob","namespace":"x","name":"pi","uid":"22222222-2222-2222-2222-222222222222","apiVersion":"extensions","resourceVersion":"222222"}}`}
|
|
createdBy3 := map[string]string{v1.CreatedByAnnotation: `{"kind":"SerializedReference","apiVersion":"v1","reference":{"kind":"CronJob","namespace":"y","name":"pi","uid":"33333333-3333-3333-3333-333333333333","apiVersion":"extensions","resourceVersion":"333333"}}`}
|
|
noCreatedBy := map[string]string{}
|
|
|
|
{
|
|
// Case 1: There are no jobs and scheduledJobs
|
|
sjs := []batch.CronJob{}
|
|
js := []batch.Job{}
|
|
jobsBySj := groupJobsByParent(sjs, js)
|
|
if len(jobsBySj) != 0 {
|
|
t.Errorf("Wrong number of items in map")
|
|
}
|
|
}
|
|
|
|
{
|
|
// Case 2: there is one controller with no job.
|
|
sjs := []batch.CronJob{
|
|
{ObjectMeta: v1.ObjectMeta{Name: "e", Namespace: "x", UID: uid1}},
|
|
}
|
|
js := []batch.Job{}
|
|
jobsBySj := groupJobsByParent(sjs, js)
|
|
if len(jobsBySj) != 0 {
|
|
t.Errorf("Wrong number of items in map")
|
|
}
|
|
}
|
|
|
|
{
|
|
// Case 3: there is one controller with one job it created.
|
|
sjs := []batch.CronJob{
|
|
{ObjectMeta: v1.ObjectMeta{Name: "e", Namespace: "x", UID: uid1}},
|
|
}
|
|
js := []batch.Job{
|
|
{ObjectMeta: v1.ObjectMeta{Name: "a", Namespace: "x", Annotations: createdBy1}},
|
|
}
|
|
jobsBySj := groupJobsByParent(sjs, js)
|
|
|
|
if len(jobsBySj) != 1 {
|
|
t.Errorf("Wrong number of items in map")
|
|
}
|
|
jobList1, found := jobsBySj[uid1]
|
|
if !found {
|
|
t.Errorf("Key not found")
|
|
}
|
|
if len(jobList1) != 1 {
|
|
t.Errorf("Wrong number of items in map")
|
|
}
|
|
}
|
|
|
|
{
|
|
// Case 4: Two namespaces, one has two jobs from one controller, other has 3 jobs from two controllers.
|
|
// There are also two jobs with no created-by annotation.
|
|
js := []batch.Job{
|
|
{ObjectMeta: v1.ObjectMeta{Name: "a", Namespace: "x", Annotations: createdBy1}},
|
|
{ObjectMeta: v1.ObjectMeta{Name: "b", Namespace: "x", Annotations: createdBy2}},
|
|
{ObjectMeta: v1.ObjectMeta{Name: "c", Namespace: "x", Annotations: createdBy1}},
|
|
{ObjectMeta: v1.ObjectMeta{Name: "d", Namespace: "x", Annotations: noCreatedBy}},
|
|
{ObjectMeta: v1.ObjectMeta{Name: "a", Namespace: "y", Annotations: createdBy3}},
|
|
{ObjectMeta: v1.ObjectMeta{Name: "b", Namespace: "y", Annotations: createdBy3}},
|
|
{ObjectMeta: v1.ObjectMeta{Name: "d", Namespace: "y", Annotations: noCreatedBy}},
|
|
}
|
|
sjs := []batch.CronJob{
|
|
{ObjectMeta: v1.ObjectMeta{Name: "e", Namespace: "x", UID: uid1}},
|
|
{ObjectMeta: v1.ObjectMeta{Name: "f", Namespace: "x", UID: uid2}},
|
|
{ObjectMeta: v1.ObjectMeta{Name: "g", Namespace: "y", UID: uid3}},
|
|
}
|
|
|
|
jobsBySj := groupJobsByParent(sjs, js)
|
|
|
|
if len(jobsBySj) != 3 {
|
|
t.Errorf("Wrong number of items in map")
|
|
}
|
|
jobList1, found := jobsBySj[uid1]
|
|
if !found {
|
|
t.Errorf("Key not found")
|
|
}
|
|
if len(jobList1) != 2 {
|
|
t.Errorf("Wrong number of items in map")
|
|
}
|
|
jobList2, found := jobsBySj[uid2]
|
|
if !found {
|
|
t.Errorf("Key not found")
|
|
}
|
|
if len(jobList2) != 1 {
|
|
t.Errorf("Wrong number of items in map")
|
|
}
|
|
jobList3, found := jobsBySj[uid3]
|
|
if !found {
|
|
t.Errorf("Key not found")
|
|
}
|
|
if len(jobList3) != 2 {
|
|
t.Errorf("Wrong number of items in map")
|
|
}
|
|
}
|
|
|
|
}
|
|
|
|
func TestGetRecentUnmetScheduleTimes(t *testing.T) {
|
|
// schedule is hourly on the hour
|
|
schedule := "0 * * * ?"
|
|
// T1 is a scheduled start time of that schedule
|
|
T1, err := time.Parse(time.RFC3339, "2016-05-19T10:00:00Z")
|
|
if err != nil {
|
|
t.Errorf("test setup error: %v", err)
|
|
}
|
|
// T2 is a scheduled start time of that schedule after T1
|
|
T2, err := time.Parse(time.RFC3339, "2016-05-19T11:00:00Z")
|
|
if err != nil {
|
|
t.Errorf("test setup error: %v", err)
|
|
}
|
|
|
|
sj := batch.CronJob{
|
|
ObjectMeta: v1.ObjectMeta{
|
|
Name: "mycronjob",
|
|
Namespace: v1.NamespaceDefault,
|
|
UID: types.UID("1a2b3c"),
|
|
},
|
|
Spec: batch.CronJobSpec{
|
|
Schedule: schedule,
|
|
ConcurrencyPolicy: batch.AllowConcurrent,
|
|
JobTemplate: batch.JobTemplateSpec{},
|
|
},
|
|
}
|
|
{
|
|
// Case 1: no known start times, and none needed yet.
|
|
// Creation time is before T1.
|
|
sj.ObjectMeta.CreationTimestamp = metav1.Time{Time: T1.Add(-10 * time.Minute)}
|
|
// Current time is more than creation time, but less than T1.
|
|
now := T1.Add(-7 * time.Minute)
|
|
times, err := getRecentUnmetScheduleTimes(sj, now)
|
|
if err != nil {
|
|
t.Errorf("unexpected error: %v", err)
|
|
}
|
|
if len(times) != 0 {
|
|
t.Errorf("expected no start times, got: %v", times)
|
|
}
|
|
}
|
|
{
|
|
// Case 2: no known start times, and one needed.
|
|
// Creation time is before T1.
|
|
sj.ObjectMeta.CreationTimestamp = metav1.Time{Time: T1.Add(-10 * time.Minute)}
|
|
// Current time is after T1
|
|
now := T1.Add(2 * time.Second)
|
|
times, err := getRecentUnmetScheduleTimes(sj, now)
|
|
if err != nil {
|
|
t.Errorf("unexpected error: %v", err)
|
|
}
|
|
if len(times) != 1 {
|
|
t.Errorf("expected 1 start time, got: %v", times)
|
|
} else if !times[0].Equal(T1) {
|
|
t.Errorf("expected: %v, got: %v", T1, times[0])
|
|
}
|
|
}
|
|
{
|
|
// Case 3: known LastScheduleTime, no start needed.
|
|
// Creation time is before T1.
|
|
sj.ObjectMeta.CreationTimestamp = metav1.Time{Time: T1.Add(-10 * time.Minute)}
|
|
// Status shows a start at the expected time.
|
|
sj.Status.LastScheduleTime = &metav1.Time{Time: T1}
|
|
// Current time is after T1
|
|
now := T1.Add(2 * time.Minute)
|
|
times, err := getRecentUnmetScheduleTimes(sj, now)
|
|
if err != nil {
|
|
t.Errorf("unexpected error: %v", err)
|
|
}
|
|
if len(times) != 0 {
|
|
t.Errorf("expected 0 start times, got: , got: %v", times)
|
|
}
|
|
}
|
|
{
|
|
// Case 4: known LastScheduleTime, a start needed
|
|
// Creation time is before T1.
|
|
sj.ObjectMeta.CreationTimestamp = metav1.Time{Time: T1.Add(-10 * time.Minute)}
|
|
// Status shows a start at the expected time.
|
|
sj.Status.LastScheduleTime = &metav1.Time{Time: T1}
|
|
// Current time is after T1 and after T2
|
|
now := T2.Add(5 * time.Minute)
|
|
times, err := getRecentUnmetScheduleTimes(sj, now)
|
|
if err != nil {
|
|
t.Errorf("unexpected error: %v", err)
|
|
}
|
|
if len(times) != 1 {
|
|
t.Errorf("expected 2 start times, got: , got: %v", times)
|
|
} else if !times[0].Equal(T2) {
|
|
t.Errorf("expected: %v, got: %v", T1, times[0])
|
|
}
|
|
}
|
|
{
|
|
// Case 5: known LastScheduleTime, two starts needed
|
|
sj.ObjectMeta.CreationTimestamp = metav1.Time{Time: T1.Add(-2 * time.Hour)}
|
|
sj.Status.LastScheduleTime = &metav1.Time{Time: T1.Add(-1 * time.Hour)}
|
|
// Current time is after T1 and after T2
|
|
now := T2.Add(5 * time.Minute)
|
|
times, err := getRecentUnmetScheduleTimes(sj, now)
|
|
if err != nil {
|
|
t.Errorf("unexpected error: %v", err)
|
|
}
|
|
if len(times) != 2 {
|
|
t.Errorf("expected 2 start times, got: , got: %v", times)
|
|
} else {
|
|
if !times[0].Equal(T1) {
|
|
t.Errorf("expected: %v, got: %v", T1, times[0])
|
|
}
|
|
if !times[1].Equal(T2) {
|
|
t.Errorf("expected: %v, got: %v", T2, times[1])
|
|
}
|
|
}
|
|
}
|
|
{
|
|
// Case 6: now is way way ahead of last start time, and there is no deadline.
|
|
sj.ObjectMeta.CreationTimestamp = metav1.Time{Time: T1.Add(-2 * time.Hour)}
|
|
sj.Status.LastScheduleTime = &metav1.Time{Time: T1.Add(-1 * time.Hour)}
|
|
now := T2.Add(10 * 24 * time.Hour)
|
|
_, err := getRecentUnmetScheduleTimes(sj, now)
|
|
if err == nil {
|
|
t.Errorf("unexpected lack of error")
|
|
}
|
|
}
|
|
{
|
|
// Case 7: now is way way ahead of last start time, but there is a short deadline.
|
|
sj.ObjectMeta.CreationTimestamp = metav1.Time{Time: T1.Add(-2 * time.Hour)}
|
|
sj.Status.LastScheduleTime = &metav1.Time{Time: T1.Add(-1 * time.Hour)}
|
|
now := T2.Add(10 * 24 * time.Hour)
|
|
// Deadline is short
|
|
deadline := int64(2 * 60 * 60)
|
|
sj.Spec.StartingDeadlineSeconds = &deadline
|
|
_, err := getRecentUnmetScheduleTimes(sj, now)
|
|
if err != nil {
|
|
t.Errorf("unexpected error")
|
|
}
|
|
}
|
|
|
|
}
|