feat: add support for jobs that are both cron and event based

Before this change, a cronjob that also specifies events would produce
a cronjob that is only running after one of the events occurs and is
deleted after a non-listed event. Additionally, a cronjob is not marked
as "ready" until it runs at least once.

After this change, a cronjob is marked as ready unless it is running or
failed. Additionally, a job that specifies both events and schedule, a
cronjob is created for the schedule and a "regular" job for each event.
Once the event-based job succeeds, the job's status is read from the
cronjob. That means, if the "regular" job fails, then the job's status
will be "failed" until the "regular" job passed, regardless of what is
happening with the cronjob.

Signed-off-by: Donnie Adams <[email protected]>

go.mod

@@ -13,7 +13,7 @@ require (
 	github.com/acorn-io/mink v0.0.0-20240105015834-b1f7af4fadea
 	github.com/acorn-io/namegenerator v0.0.0-20220915160418-9e3d5a0ffe78
 	github.com/acorn-io/schemer v0.0.0-20240105014212-9739d5485208
-	github.com/acorn-io/z v0.0.0-20230714155009-a770ecbbdc45
+	github.com/acorn-io/z v0.0.0-20231104012607-4cab1b3ec5e5
 	github.com/adrg/xdg v0.4.0
 	github.com/aws/aws-sdk-go-v2 v1.20.0
 	github.com/aws/aws-sdk-go-v2/config v1.18.32

go.sum

@@ -127,8 +127,8 @@ github.com/acorn-io/namegenerator v0.0.0-20220915160418-9e3d5a0ffe78 h1:5zs9L/CX
 github.com/acorn-io/namegenerator v0.0.0-20220915160418-9e3d5a0ffe78/go.mod h1:/5647+1/L7m7Aq7upTLtfLznTLYttURzH7Y23LKrW0M=
 github.com/acorn-io/schemer v0.0.0-20240105014212-9739d5485208 h1:GpTdbiOxq3tybaMlUkCwfBco34Zi9P5/7ikbAZ+2BdM=
 github.com/acorn-io/schemer v0.0.0-20240105014212-9739d5485208/go.mod h1:oQ4BjkYtNmfKPUMvi+/tBC7xaeHmBWM59lfBjsS4Gkg=
-github.com/acorn-io/z v0.0.0-20230714155009-a770ecbbdc45 h1:+od+ijNLAt+tc33uZa71qkE4eyvubts45thVXz8J7DU=
-github.com/acorn-io/z v0.0.0-20230714155009-a770ecbbdc45/go.mod h1:5UO0+eOne2Zhvn7Ox5IiK4u+4dlCSLmHfTQWORRdEyo=
+github.com/acorn-io/z v0.0.0-20231104012607-4cab1b3ec5e5 h1:oQnpRt5KoANqwwUNzWFu+5I12Unfu/WZ330QHefxNc8=
+github.com/acorn-io/z v0.0.0-20231104012607-4cab1b3ec5e5/go.mod h1:5UO0+eOne2Zhvn7Ox5IiK4u+4dlCSLmHfTQWORRdEyo=
 github.com/adrg/xdg v0.4.0 h1:RzRqFcjH4nE5C6oTAxhBtoE2IRyjBSa62SCbyPidvls=
 github.com/adrg/xdg v0.4.0/go.mod h1:N6ag73EX4wyxeaoeHctc1mas01KZgsj5tYiAIwqJE/E=
 github.com/alecthomas/chroma v0.10.0 h1:7XDcGkCQopCNKjZHfYrNLraA+M7e0fMiJ/Mfikbfjek=

integration/run/job_test.go

@@ -0,0 +1,116 @@
+package run
+
+import (
+	"testing"
+
+	"github.com/acorn-io/runtime/integration/helper"
+	apiv1 "github.com/acorn-io/runtime/pkg/apis/api.acorn.io/v1"
+	"github.com/acorn-io/runtime/pkg/client"
+	crClient "sigs.k8s.io/controller-runtime/pkg/client"
+)
+
+func TestJobDelete(t *testing.T) {
+	helper.StartController(t)
+
+	ctx := helper.GetCTX(t)
+	c, _ := helper.ClientAndProject(t)
+
+	image, err := c.AcornImageBuild(ctx, "./testdata/jobfinalize/Acornfile", &client.AcornImageBuildOptions{
+		Cwd: "./testdata/jobfinalize",
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	app, err := c.AppRun(ctx, image.ID, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	app = helper.WaitForObject(t, helper.Watcher(t, c), new(apiv1.AppList), app, func(app *apiv1.App) bool {
+		return len(app.Finalizers) > 0
+	})
+
+	app, err = c.AppDelete(ctx, app.Name)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	_ = helper.EnsureDoesNotExist(ctx, func() (crClient.Object, error) {
+		return c.AppGet(ctx, app.Name)
+	})
+}
+
+func TestCronJobWithCreate(t *testing.T) {
+	helper.StartController(t)
+
+	ctx := helper.GetCTX(t)
+	c, _ := helper.ClientAndProject(t)
+
+	image, err := c.AcornImageBuild(ctx, "./testdata/jobs/cron-with-create/Acornfile", &client.AcornImageBuildOptions{
+		Cwd: "./testdata/jobs/cron-with-create",
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	app, err := c.AppRun(ctx, image.ID, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	app = helper.WaitForObject(t, helper.Watcher(t, c), new(apiv1.AppList), app, func(app *apiv1.App) bool {
+		return app.Status.Ready
+	})
+
+	app, err = c.AppDelete(ctx, app.Name)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	_ = helper.EnsureDoesNotExist(ctx, func() (crClient.Object, error) {
+		return c.AppGet(ctx, app.Name)
+	})
+}
+
+func TestCronJobWithUpdate(t *testing.T) {
+	helper.StartController(t)
+
+	ctx := helper.GetCTX(t)
+	c, _ := helper.ClientAndProject(t)
+
+	image, err := c.AcornImageBuild(ctx, "./testdata/jobs/cron-with-update/Acornfile", &client.AcornImageBuildOptions{
+		Cwd: "./testdata/jobs/cron-with-update",
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	app, err := c.AppRun(ctx, image.ID, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	app = helper.WaitForObject(t, helper.Watcher(t, c), new(apiv1.AppList), app, func(app *apiv1.App) bool {
+		return app.Status.Ready && app.Status.AppStatus.Jobs["update"].Skipped
+	})
+
+	app, err = c.AppUpdate(ctx, app.Name, &client.AppUpdateOptions{
+		DeployArgs: map[string]any{
+			"forceUpdateGen": 2,
+		},
+	})
+
+	app = helper.WaitForObject(t, helper.Watcher(t, c), new(apiv1.AppList), app, func(app *apiv1.App) bool {
+		return app.Status.Ready && !app.Status.AppStatus.Jobs["update"].Skipped
+	})
+
+	app, err = c.AppDelete(ctx, app.Name)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	_ = helper.EnsureDoesNotExist(ctx, func() (crClient.Object, error) {
+		return c.AppGet(ctx, app.Name)
+	})
+}

integration/run/run_test.go

@@ -1439,38 +1439,6 @@ func TestUsingComputeClasses(t *testing.T) {
 	}
 }
 
-func TestJobDelete(t *testing.T) {
-	helper.StartController(t)
-
-	ctx := helper.GetCTX(t)
-	c, _ := helper.ClientAndProject(t)
-
-	image, err := c.AcornImageBuild(ctx, "./testdata/jobfinalize/Acornfile", &client.AcornImageBuildOptions{
-		Cwd: "./testdata/jobfinalize",
-	})
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	app, err := c.AppRun(ctx, image.ID, nil)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	app = helper.WaitForObject(t, helper.Watcher(t, c), new(apiv1.AppList), app, func(app *apiv1.App) bool {
-		return len(app.Finalizers) > 0
-	})
-
-	app, err = c.AppDelete(ctx, app.Name)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	_ = helper.EnsureDoesNotExist(ctx, func() (crClient.Object, error) {
-		return c.AppGet(ctx, app.Name)
-	})
-}
-
 func TestAppWithBadRegion(t *testing.T) {
 	helper.StartController(t)
 

integration/run/testdata/jobs/cron-with-create/Acornfile

@@ -0,0 +1,7 @@
+jobs: create: {
+    image: "ghcr.io/acorn-io/images-mirror/busybox:latest"
+    schedule: "0 0 31 2 *" // February 31st, never runs
+    events: ["create"]
+    dirs: "/app": "./scripts"
+    command: "/app/run.sh"
+}

integration/run/testdata/jobs/cron-with-create/scripts/run.sh

@@ -0,0 +1,3 @@
+#!/bin/sh
+set -x -e
+[ "$ACORN_EVENT" = "create" ]

integration/run/testdata/jobs/cron-with-update/Acornfile

@@ -0,0 +1,11 @@
+args: {
+	forceUpdateGen: 1
+}
+
+jobs: update: {
+    image: "ghcr.io/acorn-io/images-mirror/busybox:latest"
+    schedule: "0 0 31 2 *" // February 31st, never runs
+    events: ["update"]
+    dirs: "/app": "./scripts"
+    command: "/app/run.sh"
+}

integration/run/testdata/jobs/cron-with-update/scripts/run.sh

@@ -0,0 +1,3 @@
+#!/bin/sh
+set -x -e
+[ "$ACORN_EVENT" = "update" ]

pkg/controller/appdefinition/jobs.go

@@ -46,13 +46,15 @@ func toJobs(req router.Request, appInstance *v1.AppInstance, pullSecrets *PullSe
 				addBusybox = true
 			}
 		}
-		job, err := toJob(req, appInstance, pullSecrets, tag, jobName, jobDef, interpolator, addBusybox)
+		jbs, err := toJobAndCronJob(req, appInstance, pullSecrets, tag, jobName, jobDef, interpolator, addBusybox)
 		if err != nil {
 			return nil, err
 		}
-		if job == nil {
+		if len(jbs) == 0 {
 			continue
 		}
+
+		job := jbs[0]
 		perms := v1.FindPermission(jobName, appInstance.Status.Permissions)
 		sa, err := toServiceAccount(req, job.GetName(), job.GetLabels(), stripPruneAndUpdate(job.GetAnnotations()), appInstance, perms)
 		if err != nil {
@@ -65,8 +67,10 @@ func toJobs(req router.Request, appInstance *v1.AppInstance, pullSecrets *PullSe
 			}
 			result = append(result, perms...)
 		}
-		result = append(result, sa, job)
+		result = append(result, sa)
+		result = append(result, jbs...)
 	}
+
 	return result, nil
 }
 
@@ -91,18 +95,16 @@ func setSecretOutputVolume(containers []corev1.Container) (result []corev1.Conta
 	return
 }
 
-func toJob(req router.Request, appInstance *v1.AppInstance, pullSecrets *PullSecrets, tag name.Reference, name string, container v1.Container, interpolator *secrets.Interpolator, addBusybox bool) (kclient.Object, error) {
+func toJobAndCronJob(req router.Request, appInstance *v1.AppInstance, pullSecrets *PullSecrets, tag name.Reference, name string, container v1.Container, interpolator *secrets.Interpolator, addBusybox bool) ([]kclient.Object, error) {
+	var result []kclient.Object
 	interpolator = interpolator.ForJob(name)
 	jobEventName := jobs.GetEvent(name, appInstance)
 
 	jobStatus := appInstance.Status.AppStatus.Jobs[name]
 	jobStatus.Skipped = !jobs.ShouldRunForEvent(jobEventName, container)
-	if appInstance.Status.AppStatus.Jobs == nil {
-		appInstance.Status.AppStatus.Jobs = make(map[string]v1.JobStatus, len(appInstance.Status.AppSpec.Jobs))
-	}
-	appInstance.Status.AppStatus.Jobs[name] = jobStatus
+	appInstance.Status.AppStatus.Jobs = z.AddToMap(appInstance.Status.AppStatus.Jobs, name, jobStatus)
 
-	if jobStatus.Skipped {
+	if jobStatus.Skipped && container.Schedule == "" {
 		return nil, nil
 	}
 
@@ -125,20 +127,20 @@ func toJob(req router.Request, appInstance *v1.AppInstance, pullSecrets *PullSec
 		return nil, err
 	}
 
-	baseAnnotations := labels.Merge(secretAnnotations, labels.GatherScoped(name, v1.LabelTypeJob,
-		appInstance.Status.AppSpec.Annotations, container.Annotations, appInstance.Spec.Annotations))
+	baseAnnotations := labels.Merge(
+		secretAnnotations,
+		labels.GatherScoped(name, v1.LabelTypeJob, appInstance.Status.AppSpec.Annotations, container.Annotations, appInstance.Spec.Annotations),
+	)
 
 	baseAnnotations[labels.AcornConfigHashAnnotation] = appInstance.Status.AppStatus.Jobs[name].ConfigHash
-
-	if appInstance.Generation > 0 {
-		baseAnnotations[labels.AcornAppGeneration] = strconv.FormatInt(appInstance.Generation, 10)
-	}
+	baseAnnotations[labels.AcornAppGeneration] = strconv.FormatInt(appInstance.Generation, 10)
 
 	podLabels, err := jobLabels(appInstance, container, name, interpolator,
 		labels.AcornManaged, "true",
 		labels.AcornAppPublicName, publicname.Get(appInstance),
 		labels.AcornJobName, name,
-		labels.AcornContainerName, "")
+		labels.AcornContainerName, "",
+	)
 	if err != nil {
 		return nil, err
 	}
@@ -173,52 +175,52 @@ func toJob(req router.Request, appInstance *v1.AppInstance, pullSecrets *PullSec
 		},
 	}
 
+	objectMeta := metav1.ObjectMeta{
+		Name:        name,
+		Namespace:   appInstance.Status.Namespace,
+		Labels:      jobSpec.Template.Labels,
+		Annotations: labels.Merge(getDependencyAnnotations(appInstance, name, container.Dependencies), baseAnnotations),
+	}
+
 	interpolator.AddMissingAnnotations(appInstance.GetStopped(), baseAnnotations)
 
-	if container.Schedule == "" {
-		jobSpec.BackoffLimit = z.Pointer[int32](1000)
+	if container.Schedule == "" || !jobStatus.Skipped {
 		job := &batchv1.Job{
-			ObjectMeta: metav1.ObjectMeta{
-				Name:        name,
-				Namespace:   appInstance.Status.Namespace,
-				Labels:      jobSpec.Template.Labels,
-				Annotations: labels.Merge(getDependencyAnnotations(appInstance, name, container.Dependencies), baseAnnotations),
-			},
-			Spec: jobSpec,
+			ObjectMeta: *objectMeta.DeepCopy(),
+			Spec:       *jobSpec.DeepCopy(),
 		}
+
+		job.Spec.BackoffLimit = z.Pointer[int32](1000)
 		job.Spec.Template.Spec.Containers = setJobEventName(setSecretOutputVolume(containers), jobEventName)
 		job.Spec.Template.Spec.InitContainers = setJobEventName(setSecretOutputVolume(initContainers), jobEventName)
 		job.Annotations[apply.AnnotationPrune] = "false"
 		if job.Annotations[apply.AnnotationUpdate] == "" {
 			// getDependencyAnnotations may set this annotation, so don't override here
 			job.Annotations[apply.AnnotationUpdate] = "true"
 		}
-		job.Annotations[labels.AcornAppGeneration] = strconv.FormatInt(appInstance.Generation, 10)
-		return job, nil
+
+		result = append(result, job)
 	}
 
-	cronJob := &batchv1.CronJob{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:        name,
-			Namespace:   appInstance.Status.Namespace,
-			Labels:      jobSpec.Template.Labels,
-			Annotations: labels.Merge(getDependencyAnnotations(appInstance, name, container.Dependencies), baseAnnotations),
-		},
-		Spec: batchv1.CronJobSpec{
-			FailedJobsHistoryLimit:     z.Pointer[int32](3),
-			SuccessfulJobsHistoryLimit: z.Pointer[int32](1),
-			ConcurrencyPolicy:          batchv1.ReplaceConcurrent,
-			Schedule:                   toCronJobSchedule(container.Schedule),
-			JobTemplate: batchv1.JobTemplateSpec{
-				ObjectMeta: metav1.ObjectMeta{
-					Labels: jobSpec.Template.Labels,
+	if container.Schedule != "" {
+		result = append(result, &batchv1.CronJob{
+			ObjectMeta: objectMeta,
+			Spec: batchv1.CronJobSpec{
+				FailedJobsHistoryLimit:     z.Pointer[int32](3),
+				SuccessfulJobsHistoryLimit: z.Pointer[int32](1),
+				ConcurrencyPolicy:          batchv1.ReplaceConcurrent,
+				Schedule:                   toCronJobSchedule(container.Schedule),
+				JobTemplate: batchv1.JobTemplateSpec{
+					ObjectMeta: metav1.ObjectMeta{
+						Labels: jobSpec.Template.Labels,
+					},
+					Spec: jobSpec,
 				},
-				Spec: jobSpec,
 			},
-		},
+		})
 	}
-	cronJob.Annotations[labels.AcornAppGeneration] = strconv.FormatInt(appInstance.Generation, 10)
-	return cronJob, nil
+
+	return result, nil
 }
 
 func toCronJobSchedule(schedule string) string {

pkg/controller/appdefinition/testdata/computeclass/job/expected.golden

@@ -53,6 +53,7 @@ spec:
   template:
     metadata:
       annotations:
+        acorn.io/app-generation: "0"
         acorn.io/config-hash: ""
         acorn.io/container-spec: '{"build":{"context":".","dockerfile":"Dockerfile"},"image":"image-name","metrics":{},"ports":[{"port":80,"protocol":"http","targetPort":81}],"probes":null,"sidecars":{"left":{"image":"foo","metrics":{},"ports":[{"port":90,"protocol":"tcp","targetPort":91}],"probes":null}}}'
       creationTimestamp: null

pkg/controller/appdefinition/testdata/deployspec/filter-user-labels/expected.golden

@@ -229,6 +229,7 @@ spec:
   template:
     metadata:
       annotations:
+        acorn.io/app-generation: "0"
         acorn.io/config-hash: ""
         acorn.io/container-spec: '{"annotations":{"admit-job.io":"test-admit-job-ann"},"build":{"context":".","dockerfile":"Dockerfile"},"image":"image-name","labels":{"allowed-job.io":"test-allowed-job-label"},"metrics":{},"ports":[{"port":80,"protocol":"http","targetPort":81}],"probes":null}'
         admit-job.io: test-admit-job-ann

pkg/controller/appdefinition/testdata/deployspec/labels/expected.golden

@@ -225,6 +225,7 @@ spec:
   template:
     metadata:
       annotations:
+        acorn.io/app-generation: "0"
         acorn.io/config-hash: ""
         acorn.io/container-spec: '{"annotations":{"jobAnn":"test-job-ann"},"build":{"context":".","dockerfile":"Dockerfile"},"image":"image-name","labels":{"jobLabel":"test-job-label"},"metrics":{},"ports":[{"port":80,"protocol":"http","targetPort":81}],"probes":null}'
         appSpecAnn: test-app-spec-ann