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.

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

[release-1.32] Remove TTLSecondsAfterFinished from Eventing jobs #2791

Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,7 @@ func (r *ReconcileKnativeKafka) transform(manifest *mf.Manifest, instance *serve
socommon.ApplyCABundlesTransform(),
operatorcommon.OverridesTransform(instance.Spec.Workloads, logging.FromContext(context.TODO())),
socommon.ConfigMapVolumeChecksumTransform(context.Background(), r.client, dependentConfigMaps),
socommon.JobsRemoveTTLSecondsAfterFinished(),
injectNamespacedBrokerMonitoring(r.client)), socommon.DeprecatedAPIsTranformersFromConfig()...)
tfs = append(tfs, rbacProxyTranforms...)

Expand Down
16 changes: 16 additions & 0 deletions openshift-knative-operator/pkg/common/job.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,19 @@ func VersionedJobNameTransform() mf.Transformer {
return nil
}
}

func JobsRemoveTTLSecondsAfterFinished() mf.Transformer {
return func(u *unstructured.Unstructured) error {
if u.GetKind() == "Job" {
job := &batchv1.Job{}
if err := scheme.Scheme.Convert(u, job, nil); err != nil {
return err
}
if job.Spec.TTLSecondsAfterFinished != nil {
job.Spec.TTLSecondsAfterFinished = nil
}
return scheme.Scheme.Convert(job, u, nil)
}
return nil
}
}
19 changes: 19 additions & 0 deletions openshift-knative-operator/pkg/common/job_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/google/go-cmp/cmp"
batchv1 "k8s.io/api/batch/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/utils/ptr"
util "knative.dev/operator/pkg/reconciler/common/testing"
)

Expand Down Expand Up @@ -48,6 +49,24 @@ func TestJobGeneratedNameTransform(t *testing.T) {

}

func TestJobsRemoveTTLSecondsAfterFinished(t *testing.T) {
got := createJob("", "gen")
got.Spec.TTLSecondsAfterFinished = ptr.To[int32](32)

expected := createJob("", "gen")
expected.Spec.TTLSecondsAfterFinished = nil

u := util.MakeUnstructured(t, &got)
if err := JobsRemoveTTLSecondsAfterFinished()(&u); err != nil {
t.Fatal("Unexpected error from transformer", err)
}

expectedU := util.MakeUnstructured(t, &expected)
if diff := cmp.Diff(u, expectedU); diff != "" {
t.Errorf("Got = %#v, want = %#v\n%s", u, expectedU, diff)
}
}

func createJob(name, gen string) batchv1.Job {
return batchv1.Job{
TypeMeta: metav1.TypeMeta{
Expand Down
1 change: 1 addition & 0 deletions openshift-knative-operator/pkg/eventing/extension.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ func (e *extension) Transformers(ke base.KComponent) []mf.Transformer {
common.VersionedJobNameTransform(),
common.InjectCommonEnvironment(),
common.ApplyCABundlesTransform(),
common.JobsRemoveTTLSecondsAfterFinished(),
}
tf = append(tf, monitoring.GetEventingTransformers(ke)...)
return append(tf, common.DeprecatedAPIsTranformers(e.kubeclient.Discovery())...)
Expand Down
17 changes: 13 additions & 4 deletions test/e2e/knative_eventing_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,20 @@ package e2e

import (
"context"
"fmt"
"testing"

"github.com/openshift-knative/serverless-operator/test"
"github.com/openshift-knative/serverless-operator/test/monitoringe2e"
"github.com/openshift-knative/serverless-operator/test/upgrade"
"github.com/openshift-knative/serverless-operator/test/v1beta1"
batchv1 "k8s.io/api/batch/v1"
"knative.dev/pkg/client/injection/kube/client"
"knative.dev/pkg/injection/clients/dynamicclient"
"knative.dev/pkg/logging"
logtesting "knative.dev/pkg/logging/testing"
"knative.dev/pkg/ptr"

"github.com/openshift-knative/serverless-operator/test"
"github.com/openshift-knative/serverless-operator/test/monitoringe2e"
"github.com/openshift-knative/serverless-operator/test/upgrade"
"github.com/openshift-knative/serverless-operator/test/v1beta1"
)

const (
Expand Down Expand Up @@ -71,6 +74,12 @@ func TestKnativeEventing(t *testing.T) {
upgrade.VerifyPostInstallJobs(caCtx, upgrade.VerifyPostJobsConfig{
Namespace: eventingNamespace,
FailOnNoJobs: true,
ValidateJob: func(j batchv1.Job) error {
if j.Spec.TTLSecondsAfterFinished != nil {
return fmt.Errorf("job %s/%s has TTLSecondsAfterFinished", eventingNamespace, j.Name)
}
return nil
},
})
})

Expand Down
11 changes: 10 additions & 1 deletion test/e2ekafka/knative_kafka_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@ package e2ekafka

import (
"context"
"fmt"
"testing"

"github.com/openshift-knative/serverless-operator/test/v1alpha1"
batchv1 "k8s.io/api/batch/v1"
corev1 "k8s.io/api/core/v1"
eventingv1 "knative.dev/eventing/pkg/apis/eventing/v1"
duckv1 "knative.dev/pkg/apis/duck/v1"
Expand All @@ -14,6 +15,8 @@ import (
logtesting "knative.dev/pkg/logging/testing"
"knative.dev/pkg/ptr"

"github.com/openshift-knative/serverless-operator/test/v1alpha1"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"knative.dev/eventing-kafka-broker/control-plane/pkg/apis/messaging/v1beta1"

Expand Down Expand Up @@ -193,6 +196,12 @@ func TestKnativeKafka(t *testing.T) {
upgrade.VerifyPostInstallJobs(caCtx, upgrade.VerifyPostJobsConfig{
Namespace: knativeKafkaNamespace,
FailOnNoJobs: true,
ValidateJob: func(j batchv1.Job) error {
if j.Spec.TTLSecondsAfterFinished != nil {
return fmt.Errorf("job %s/%s has TTLSecondsAfterFinished", knativeKafkaNamespace, j.Name)
}
return nil
},
})
})
}
11 changes: 10 additions & 1 deletion test/upgrade/verify_jobs.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,20 @@ import (
"fmt"
"time"

"github.com/openshift-knative/serverless-operator/test"
"golang.org/x/sync/errgroup"
batchv1 "k8s.io/api/batch/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/wait"
"knative.dev/pkg/test/upgrade"

"github.com/openshift-knative/serverless-operator/test"
)

type VerifyPostJobsConfig struct {
Namespace string
FailOnNoJobs bool
ValidateJob func(j batchv1.Job) error
}

func VerifyPostInstallJobs(ctx *test.Context, cfg VerifyPostJobsConfig) upgrade.Operation {
Expand Down Expand Up @@ -44,6 +47,12 @@ func verifyPostInstallJobs(ctx context.Context, testCtx *test.Context, c upgrade
for _, j := range jobs.Items {
j := j

if cfg.ValidateJob != nil {
if err := cfg.ValidateJob(j); err != nil {
return fmt.Errorf("failed to validate job %s: %w", j.Name, err)
}
}

if j.Status.Succeeded > 0 {
// We don't need to wait for a job that is already succeeded.
// In addition, an already succeeded job might go away due to the job's TTL.
Expand Down
Loading