From dd84bba64c8b44887bbe0fb0d72d4b820c062ea5 Mon Sep 17 00:00:00 2001
From: Michail Kargakis <mkargaki@redhat.com>
Date: Tue, 12 Jan 2016 14:59:14 +0100
Subject: [PATCH] Fix job status conditions bloat

When a job is complete, the controller will indefinitely update its conditions
with a Complete condition. This change makes the controller exit the
reconcilation as soon as the job is already found to be marked as complete.
---
 pkg/controller/job/controller.go      |  8 ++++----
 pkg/controller/job/controller_test.go | 24 ++++++++++++++++++++++++
 2 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/pkg/controller/job/controller.go b/pkg/controller/job/controller.go
index 10fbe8052f..abb74ac670 100644
--- a/pkg/controller/job/controller.go
+++ b/pkg/controller/job/controller.go
@@ -330,11 +330,11 @@ func (jm *JobController) syncJob(key string) error {
 		now := unversioned.Now()
 		job.Status.StartTime = &now
 	}
+	// if job was finished previously, we don't want to redo the termination
+	if isJobFinished(&job) {
+		return nil
+	}
 	if pastActiveDeadline(&job) {
-		// if job was finished previously, we don't want to redo the termination
-		if isJobFinished(&job) {
-			return nil
-		}
 		// TODO: below code should be replaced with pod termination resulting in
 		// pod failures, rather than killing pods. Unfortunately none such solution
 		// exists ATM. There's an open discussion in the topic in
diff --git a/pkg/controller/job/controller_test.go b/pkg/controller/job/controller_test.go
index 6fa0b57aaa..528bac57da 100644
--- a/pkg/controller/job/controller_test.go
+++ b/pkg/controller/job/controller_test.go
@@ -359,7 +359,31 @@ func TestSyncPastDeadlineJobFinished(t *testing.T) {
 	if actual != nil {
 		t.Error("Unexpected job modification")
 	}
+}
 
+func TestSyncJobComplete(t *testing.T) {
+	client := client.NewOrDie(&client.Config{Host: "", GroupVersion: testapi.Default.GroupVersion()})
+	manager := NewJobController(client, controller.NoResyncPeriodFunc)
+	fakePodControl := controller.FakePodControl{}
+	manager.podControl = &fakePodControl
+	manager.podStoreSynced = alwaysReady
+
+	job := newJob(1, 1)
+	job.Status.Conditions = append(job.Status.Conditions, newCondition(extensions.JobComplete, "", ""))
+	manager.jobStore.Store.Add(job)
+	err := manager.syncJob(getKey(job, t))
+	if err != nil {
+		t.Fatalf("Unexpected error when syncing jobs %v", err)
+	}
+	uncastJob, _, err := manager.jobStore.Store.Get(job)
+	if err != nil {
+		t.Fatalf("Unexpected error when trying to get job from the store: %v", err)
+	}
+	actual := uncastJob.(*extensions.Job)
+	// Verify that after syncing a complete job, the conditions are the same.
+	if got, expected := len(actual.Status.Conditions), 1; got != expected {
+		t.Fatalf("Unexpected job status conditions amount; expected %d, got %d", expected, got)
+	}
 }
 
 func TestSyncJobDeleted(t *testing.T) {