[release-2.6] Fix the conditions history in compliancyDetails

controllers/configurationpolicy_controller.go

@@ -46,7 +46,10 @@ import (
  common "open-cluster-management.io/config-policy-controller/pkg/common"
 )
 
-const ControllerName string = "configuration-policy-controller"
+const (
+ ControllerName string = "configuration-policy-controller"
+ complianceStatusConditionLimit int = 10
+)
 
 var log = ctrl.Log.WithName(ControllerName)
 
@@ -1018,7 +1021,7 @@ func addConditionToStatus(
 
  // do not add condition unless it does not already appear in the status
  if !checkMessageSimilarity(plc.Status.CompliancyDetails[index].Conditions, cond) {
- conditions := AppendCondition(plc.Status.CompliancyDetails[index].Conditions, cond, "", false)
+ conditions := AppendCondition(plc.Status.CompliancyDetails[index].Conditions, cond)
  plc.Status.CompliancyDetails[index].Conditions = conditions
  updateNeeded = true
  }
@@ -1569,8 +1572,7 @@ func (r *ConfigurationPolicyReconciler) getMapping(
  policy.Status.CompliancyDetails[index].ComplianceState = policyv1.NonCompliant
 
  if !checkMessageSimilarity(policy.Status.CompliancyDetails[index].Conditions, cond) {
- conditions := AppendCondition(policy.Status.CompliancyDetails[index].Conditions,
- cond, gvk.GroupKind().Kind, false)
+ conditions := AppendCondition(policy.Status.CompliancyDetails[index].Conditions, cond)
  policy.Status.CompliancyDetails[index].Conditions = conditions
  updateNeeded = true
  }
@@ -2280,26 +2282,19 @@ func (r *ConfigurationPolicyReconciler) checkAndUpdateResource(
 
 // AppendCondition check and appends conditions to the policy status
 func AppendCondition(
- conditions []policyv1.Condition, newCond *policyv1.Condition, resourceType string, resolved ...bool,
+ conditions []policyv1.Condition, newCond *policyv1.Condition,
 ) (conditionsRes []policyv1.Condition) {
- defer recoverFlow()
-
- lastIndex := len(conditions)
- if lastIndex > 0 {
- oldCond := conditions[lastIndex-1]
- if IsSimilarToLastCondition(oldCond, *newCond) {
- conditions[lastIndex-1] = *newCond
-
- return conditions
- }
- } else {
- // first condition => trigger event
- conditions = append(conditions, *newCond)
+ if len(conditions) != 0 && IsSimilarToLastCondition(conditions[len(conditions)-1], *newCond) {
+ conditions[len(conditions)-1] = *newCond
 
  return conditions
  }
 
- conditions[lastIndex-1] = *newCond
+ conditions = append(conditions, *newCond)
+
+ if len(conditions) > complianceStatusConditionLimit {
+ conditions = conditions[1:]
+ }
 
  return conditions
 }
@@ -2459,10 +2454,3 @@ func convertPolicyStatusToString(plc *policyv1.ConfigurationPolicy) (results str
 
  return result
 }
-
-func recoverFlow() {
- if r := recover(); r != nil {
- // V(-2) is the error level
- log.V(-2).Info("ALERT!!!! -> recovered from ", "recover", r)
- }
-}

controllers/configurationpolicy_controller_test.go

@@ -675,3 +675,69 @@ func TestShouldEvaluatePolicy(t *testing.T) {
  )
  }
 }
+
+func TestAppendCondition(t *testing.T) {
+ conditions := []policyv1.Condition{}
+ newCond1 := &policyv1.Condition{
+ Type: "violation",
+ LastTransitionTime: metav1.Time{Time: metav1.Now().Add(5 * time.Minute)},
+ Reason: "K8s `must have` object already exists",
+ Message: "some message",
+ }
+
+ conditions = AppendCondition(conditions, newCond1)
+ if len(conditions) != 1 {
+ t.Fatalf("Expected a single condition to be appended but the length was %d", len(conditions))
+ }
+
+ lastTransition := metav1.Time{Time: metav1.Now().Add(5 * time.Minute)}
+ newCond2 := &policyv1.Condition{
+ Type: "violation",
+ LastTransitionTime: lastTransition,
+ Reason: "K8s `must have` object already exists",
+ Message: "some message",
+ }
+ conditions = AppendCondition(conditions, newCond2)
+
+ if len(conditions) != 1 {
+ t.Fatalf("Expected the single condition to be updated and not appended but the length was %d", len(conditions))
+ }
+
+ if conditions[0].LastTransitionTime != lastTransition {
+ t.Fatal("The single condition was not updated with the last transition time")
+ }
+
+ newCond3 := &policyv1.Condition{
+ Type: "violation",
+ LastTransitionTime: lastTransition,
+ Reason: "K8s creation success",
+ Message: "some other message",
+ }
+ conditions = AppendCondition(conditions, newCond3)
+
+ if len(conditions) != 2 {
+ t.Fatalf("Expected the new condition to be appended but the length was %d", len(conditions))
+ }
+
+ if conditions[0].Reason != newCond2.Reason || conditions[1].Reason != newCond3.Reason {
+ t.Fatalf("Expected the new condition to be appended the order is incorrect")
+ }
+
+ for i := 0; i < complianceStatusConditionLimit+2; i++ {
+ newCond := &policyv1.Condition{
+ Type: "violation",
+ LastTransitionTime: lastTransition,
+ Reason: "K8s creation success",
+ Message: fmt.Sprintf("some other message%d", i),
+ }
+ conditions = AppendCondition(conditions, newCond)
+ }
+
+ if len(conditions) != complianceStatusConditionLimit {
+ t.Fatalf(
+ "Expected the conditions array to be limited to %d but the length was %d",
+ complianceStatusConditionLimit,
+ len(conditions),
+ )
+ }
+}

test/e2e/case1_pod_handling_test.go

@@ -6,6 +6,7 @@ package e2e
 import (
  . "github.com/onsi/ginkgo/v2"
  . "github.com/onsi/gomega"
+ "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 
  "open-cluster-management.io/config-policy-controller/test/utils"
 )
@@ -47,12 +48,32 @@ var _ = Describe("Test pod obj template handling", func() {
  plc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy,
  case1ConfigPolicyNameEnforce, testNamespace, true, defaultTimeoutSeconds)
  Expect(plc).NotTo(BeNil())
+
  Eventually(func() interface{} {
  managedPlc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy,
  case1ConfigPolicyNameEnforce, testNamespace, true, defaultTimeoutSeconds)
 
  return utils.GetComplianceState(managedPlc)
  }, defaultTimeoutSeconds, 1).Should(Equal("Compliant"))
+
+ By("verifying status.compliancyDetails[].conditions")
+ Eventually(func(g Gomega) {
+ managedPlc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy,
+ case1ConfigPolicyNameEnforce, testNamespace, true, defaultTimeoutSeconds)
+
+ compliancyDetails, _, _ := unstructured.NestedSlice(managedPlc.Object, "status", "compliancyDetails")
+ g.Expect(compliancyDetails).To(HaveLen(1))
+
+ conditions, ok := compliancyDetails[0].(map[string]interface{})["conditions"].([]interface{})
+ g.Expect(ok).To(BeTrue())
+ g.Expect(conditions).To(HaveLen(2))
+ g.Expect(conditions[0].(map[string]interface{})["reason"]).To(Equal("K8s creation success"))
+ g.Expect(conditions[1].(map[string]interface{})["reason"]).To(
+ Equal("K8s `must have` object already exists"),
+ )
+ }, defaultTimeoutSeconds, 1).Should(Succeed())
+
+ By("verifying that " + case1ConfigPolicyNameInform + " is compliant")
  Eventually(func() interface{} {
  informPlc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy,
  case1ConfigPolicyNameInform, testNamespace, true, defaultTimeoutSeconds)

test/utils/utils.go

@@ -206,8 +206,9 @@ func GetComplianceState(managedPlc *unstructured.Unstructured) (result interface
 func GetStatusMessage(managedPlc *unstructured.Unstructured) (result interface{}) {
  if managedPlc.Object["status"] != nil {
  detail := managedPlc.Object["status"].(map[string]interface{})["compliancyDetails"].([]interface{})[0]
+ conditions := detail.(map[string]interface{})["conditions"].([]interface{})
 
- return detail.(map[string]interface{})["conditions"].([]interface{})[0].(map[string]interface{})["message"]
+ return conditions[len(conditions)-1].(map[string]interface{})["message"]
  }
 
  return nil