Add refetch before updating status

Especially when DeleteIfCreated is in use, if the status update fails,
the controller is no longer aware it created resources. A refetch
prevents the status update from failing due to conflicts.

Signed-off-by: Dale Haiducek <19750917+dhaiducek@users.noreply.github.com>

controllers/configurationpolicy_controller.go

@@ -517,13 +517,21 @@ func (r *ConfigurationPolicyReconciler) cleanUpChildObjects(plc policyv1.Configu
 		// determine whether object should be deleted
 		needsDelete := false
 
-		existing, _ := getObject(
+		existing, err := getObject(
 			namespaced,
 			object.Object.Metadata.Namespace,
 			object.Object.Metadata.Name,
 			mapping.Resource,
 			r.TargetK8sDynamicClient,
 		)
+		if err != nil {
+			log.Error(fmt.Errorf("failed to get object"), "Error")
+
+			deletionFailures = append(deletionFailures, gvk.String()+fmt.Sprintf(` "%s" in namespace %s`,
+				object.Object.Metadata.Name, object.Object.Metadata.Namespace))
+
+			continue
+		}
 
 		// object does not exist, no deletion logic needed
 		if existing == nil {
@@ -554,7 +562,7 @@ func (r *ConfigurationPolicyReconciler) cleanUpChildObjects(plc policyv1.Configu
 			// if object has already been deleted and is stuck, no need to redo delete request
 			_, deletionTimeFound, _ := unstructured.NestedString(existing.Object, "metadata", "deletionTimestamp")
 			if deletionTimeFound {
-				log.Error(err, "Error: tried to delete object, but delete is hanging")
+				log.Error(fmt.Errorf("tried to delete object, but delete is hanging"), "Error")
 
 				deletionFailures = append(deletionFailures, gvk.String()+fmt.Sprintf(` "%s" in namespace %s`,
 					object.Object.Metadata.Name, object.Object.Metadata.Namespace))
@@ -2692,7 +2700,7 @@ func (r *ConfigurationPolicyReconciler) addForUpdate(policy *policyv1.Configurat
 	policy.Status.LastEvaluated = time.Now().UTC().Format(time.RFC3339)
 	policy.Status.LastEvaluatedGeneration = policy.Generation
 
-	_, err := r.updatePolicyStatus(policy, sendEvent)
+	err := r.updatePolicyStatus(policy, sendEvent)
 	policyLog := log.WithValues("name", policy.Name, "namespace", policy.Namespace)
 
 	if k8serrors.IsConflict(err) {
@@ -2714,34 +2722,51 @@ func (r *ConfigurationPolicyReconciler) addForUpdate(policy *policyv1.Configurat
 func (r *ConfigurationPolicyReconciler) updatePolicyStatus(
 	policy *policyv1.ConfigurationPolicy,
 	sendEvent bool,
-) (*policyv1.ConfigurationPolicy, error) {
+) error {
 	if sendEvent {
 		log.V(1).Info("Sending parent policy compliance event")
 
 		// If the compliance event can't be created, then don't update the ConfigurationPolicy
 		// status. As long as that hasn't been updated, everything will be retried next loop.
 		if err := r.sendComplianceEvent(policy); err != nil {
-			return policy, err
+			return err
 		}
 	}
 
 	log.V(2).Info(
 		"Updating configurationPolicy status", "status", policy.Status.ComplianceState, "policy", policy.GetName(),
 	)
 
-	err := r.Status().Update(context.TODO(), policy)
-	if err != nil {
-		return policy, err
-	}
+	updatedStatus := policy.Status
+
+	maxRetries := 3
+	for i := 1; i <= maxRetries; i++ {
+		err := r.Get(context.TODO(), types.NamespacedName{Namespace: policy.Namespace, Name: policy.Name}, policy)
+		if err != nil {
+			log.Info(fmt.Sprintf("Failed to refresh policy; using previously fetched version: %s", err))
+		} else {
+			policy.Status = updatedStatus
+		}
+
+		err = r.Status().Update(context.TODO(), policy)
+		if err != nil {
+			if i == maxRetries {
+				return err
+			}
 
+			log.Info(fmt.Sprintf("Failed to update policy status. Retrying (attempt %d/%d): %s", i, maxRetries, err))
+		} else {
+			break
+		}
+	}
 	if sendEvent {
 		log.V(1).Info("Sending policy status update event")
 
 		r.Recorder.Event(policy, "Normal", "Policy updated",
 			fmt.Sprintf("Policy status is: %v", policy.Status.ComplianceState))
 	}
 
-	return nil, nil
+	return nil
 }
 
 func (r *ConfigurationPolicyReconciler) sendComplianceEvent(instance *policyv1.ConfigurationPolicy) error {

test/e2e/case20_delete_objects_test.go

@@ -609,7 +609,7 @@ var _ = Describe("Test objects are not deleted when the CRD is removed", Ordered
 	})
 })
 
-var _ = Describe("Clean up old object when configuraionpolicy is changed", Ordered, func() {
+var _ = Describe("Clean up old object when configurationpolicy is changed", Ordered, func() {
 	const (
 		oldPodName             string = "case29-name-changed-pod"
 		newPodName             string = "case29-name-changed-new"