From 39bb9e94e650a260ab8b9a18b93069d1ce36e887 Mon Sep 17 00:00:00 2001
From: Daniel Henkel <daniel.henkel@datadoghq.com>
Date: Mon, 27 Nov 2023 09:35:37 +0100
Subject: [PATCH] keep existing PDB conditions when updating status

When the disruption controller updates the PDB status, it removes all conditions from the new status object and then re-adds the sufficient pods condition. Unfortunately, this behavior removes conditions set by other controllers, leading to multiple consecutive updates.
Therefore, this commit ensures that conditions are preserved during updates.
---
 pkg/controller/disruption/disruption.go      |  1 +
 pkg/controller/disruption/disruption_test.go | 31 ++++++++++++++++++++
 2 files changed, 32 insertions(+)

diff --git a/pkg/controller/disruption/disruption.go b/pkg/controller/disruption/disruption.go
index 2527e31480ebb..5a5ede4d3ae33 100644
--- a/pkg/controller/disruption/disruption.go
+++ b/pkg/controller/disruption/disruption.go
@@ -994,6 +994,7 @@ func (dc *DisruptionController) updatePdbStatus(ctx context.Context, pdb *policy
 		DisruptionsAllowed: disruptionsAllowed,
 		DisruptedPods:      disruptedPods,
 		ObservedGeneration: pdb.Generation,
+		Conditions:         newPdb.Status.Conditions,
 	}
 
 	pdbhelper.UpdateDisruptionAllowedCondition(newPdb)
diff --git a/pkg/controller/disruption/disruption_test.go b/pkg/controller/disruption/disruption_test.go
index 3e9fb4e4fd317..ac56f90f266c2 100644
--- a/pkg/controller/disruption/disruption_test.go
+++ b/pkg/controller/disruption/disruption_test.go
@@ -1565,6 +1565,37 @@ func TestStalePodDisruption(t *testing.T) {
 	}
 }
 
+func TestKeepExistingPDBConditionDuringSync(t *testing.T) {
+	_, ctx := ktesting.NewTestContext(t)
+	dc, ps := newFakeDisruptionController(ctx)
+
+	pdb, pdbName := newMinAvailablePodDisruptionBudget(t, intstr.FromInt32(3))
+	pdb.Spec.Selector = &metav1.LabelSelector{}
+
+	pdb.Status.Conditions = append(pdb.Status.Conditions, metav1.Condition{
+		Type:               "ExistingTestCondition",
+		Status:             metav1.ConditionTrue,
+		Message:            "This is a test condition",
+		Reason:             "Test",
+		LastTransitionTime: metav1.Now(),
+	})
+
+	add(t, dc.pdbStore, pdb)
+	if err := dc.sync(ctx, pdbName); err != nil {
+		t.Fatalf("Failed to sync PDB: %v", err)
+	}
+	ps.VerifyPdbStatus(t, pdbName, 0, 0, 3, 0, map[string]metav1.Time{})
+
+	actualPDB := ps.Get(pdbName)
+	condition := apimeta.FindStatusCondition(actualPDB.Status.Conditions, "ExistingTestCondition")
+	if len(actualPDB.Status.Conditions) != 2 {
+		t.Fatalf("Expected 2 conditions, but got %d", len(actualPDB.Status.Conditions))
+	}
+	if condition == nil {
+		t.Fatalf("Expected ExistingTestCondition condition, but didn't find it")
+	}
+}
+
 // waitForCacheCount blocks until the given cache store has the desired number
 // of items in it. This will return an error if the condition is not met after a
 // 10 second timeout.