Skip to content

Commit

Permalink
Add integration test for managedBy and cleanup of finalizers
Browse files Browse the repository at this point in the history
  • Loading branch information
mimowo committed Mar 6, 2024
1 parent e568a77 commit 401a4bd
Showing 1 changed file with 125 additions and 0 deletions.
125 changes: 125 additions & 0 deletions test/integration/job/job_test.go
Expand Up @@ -45,13 +45,15 @@ import (
clientset "k8s.io/client-go/kubernetes"
typedv1 "k8s.io/client-go/kubernetes/typed/batch/v1"
restclient "k8s.io/client-go/rest"
"k8s.io/client-go/tools/record"
"k8s.io/client-go/util/retry"
featuregatetesting "k8s.io/component-base/featuregate/testing"
basemetrics "k8s.io/component-base/metrics"
"k8s.io/component-base/metrics/testutil"
"k8s.io/klog/v2"
kubeapiservertesting "k8s.io/kubernetes/cmd/kube-apiserver/app/testing"
podutil "k8s.io/kubernetes/pkg/api/v1/pod"
"k8s.io/kubernetes/pkg/controller"
jobcontroller "k8s.io/kubernetes/pkg/controller/job"
"k8s.io/kubernetes/pkg/controller/job/metrics"
"k8s.io/kubernetes/pkg/features"
Expand Down Expand Up @@ -1503,6 +1505,129 @@ func TestManagedBy_RecreatedJob(t *testing.T) {
}
}

// TestManagedBy_UsingReservedJobFinalizers documents the behavior of the Job
// controller when there is a job with custom value of the managedBy field, creating
// pods with the batch.kubernetes.io/job-tracking finalizer. The built-in controller
// should not remove the finalizer. Note that, the use of the finalizer in jobs
// managed by external controllers is discouraged, but may potentially happen
// when one forks the controller and does not rename the finalizer.
func TestManagedBy_UsingReservedJobFinalizers(t *testing.T) {
customControllerName := "example.com/custom-job-controller"
defer featuregatetesting.SetFeatureGateDuringTest(t, feature.DefaultFeatureGate, features.JobManagedBy, true)()

closeFn, restConfig, clientSet, ns := setup(t, "managed-by-reserved-finalizers")
defer closeFn()
ctx, cancel := startJobControllerAndWaitForCaches(t, restConfig)
defer cancel()
resetMetrics()

jobSpec := batchv1.Job{
TypeMeta: metav1.TypeMeta{
APIVersion: "batch/v1",
Kind: "Job",
},
ObjectMeta: metav1.ObjectMeta{
Name: "custom-job-test",
Namespace: ns.Name,
},
Spec: batchv1.JobSpec{
Completions: ptr.To[int32](1),
Parallelism: ptr.To[int32](1),
Template: v1.PodTemplateSpec{
Spec: v1.PodSpec{
Containers: []v1.Container{
{
Name: "main-container",
Image: "foo",
},
},
},
},
ManagedBy: ptr.To(customControllerName),
},
}
// Create a job with custom managedBy
jobObj, err := createJobWithDefaults(ctx, clientSet, ns.Name, &jobSpec)
if err != nil {
t.Fatalf("Error %v when creating the job %q", err, klog.KObj(jobObj))
}

podControl := controller.RealPodControl{
KubeClient: clientSet,
Recorder: &record.FakeRecorder{},
}

// Create the pod manually simulating the behavior of the external controller
// indicated by the managedBy field. We create the pod with the built-in
// finalizer.
podTemplate := jobObj.Spec.Template.DeepCopy()
podTemplate.Finalizers = append(podTemplate.Finalizers, batchv1.JobTrackingFinalizer)
err = podControl.CreatePodsWithGenerateName(ctx, jobObj.Namespace, podTemplate, jobObj, metav1.NewControllerRef(jobObj, batchv1.SchemeGroupVersion.WithKind("Job")), "pod1")
if err != nil {
t.Fatalf("Error %v when creating a pod for job %q", err, klog.KObj(jobObj))
}

// Getting the list of pods for the Jobs to obtain the reference to the created pod.
jobPods, err := getJobPods(ctx, t, clientSet, jobObj, func(ps v1.PodStatus) bool { return true })
if err != nil {
t.Fatalf("Error %v getting the list of pods for job %q", err, klog.KObj(jobObj))
}
if len(jobPods) != 1 {
t.Fatalf("Unexpected number (%d) of pods for job: %v", len(jobPods), klog.KObj(jobObj))
}

// Marking the pod as finished (succeeded), before marking the parent job as complete.
podObj := jobPods[0]
podObj.Status.Phase = v1.PodSucceeded
podObj, err = clientSet.CoreV1().Pods(ns.Name).UpdateStatus(ctx, podObj, metav1.UpdateOptions{})
if err != nil {
t.Fatalf("Error %v when marking the %q pod as succeeded", err, klog.KObj(podObj))
}

// Mark the job as finished so that the built-in controller receives an event
// in reaction to each it would remove the pod's finalizer, if not for the
// custom managedBy field.
jobObj.Status.Conditions = append(jobObj.Status.Conditions, batchv1.JobCondition{
Type: batchv1.JobComplete,
Status: v1.ConditionTrue,
})
jobObj.Status.StartTime = ptr.To(metav1.Now())
jobObj.Status.CompletionTime = ptr.To(metav1.Now())

if jobObj, err = clientSet.BatchV1().Jobs(jobObj.Namespace).UpdateStatus(ctx, jobObj, metav1.UpdateOptions{}); err != nil {
t.Fatalf("Error %v when updating the job as finished %v", err, klog.KObj(jobObj))
}

podObj, err = clientSet.CoreV1().Pods(ns.Name).Get(ctx, podObj.Name, metav1.GetOptions{})
if err != nil {
t.Fatalf("Error %v when getting the latest version of the pod %v", err, klog.KObj(podObj))
}

// Update the pod so that the built-in controller receives an event in reaction
// to each it would remove the pod's finalizer, if not for the custom value
// of the managedBy field on the job.
podObj.Status.Conditions = append(podObj.Status.Conditions, v1.PodCondition{
Type: v1.PodConditionType("CustomCondition"),
Status: v1.ConditionTrue,
})
podObj, err = clientSet.CoreV1().Pods(ns.Name).UpdateStatus(ctx, podObj, metav1.UpdateOptions{})
if err != nil {
t.Fatalf("Error %v when adding a condition to the pod status %v", err, klog.KObj(podObj))
}

// Await a little bit to make it very likely that the finalizer would have
// been already removed, and verify it is still there.
time.Sleep(100 * time.Millisecond)
podObj, err = clientSet.CoreV1().Pods(ns.Name).Get(ctx, podObj.Name, metav1.GetOptions{})
if err != nil {
t.Fatalf("Error %v when getting the latest version of the pod %v", err, klog.KObj(podObj))
}

if len(podObj.Finalizers) != 1 {
t.Fatalf("Unexpected removal of the pod finalizer for %q, because the owner job %q has custom managedBy", klog.KObj(podObj), klog.KObj(jobObj))
}
}

func getIndexFailureCount(p *v1.Pod) (int, error) {
if p.Annotations == nil {
return 0, errors.New("no annotations found")
Expand Down

0 comments on commit 401a4bd

Please sign in to comment.