Skip to content

Commit

Permalink
Handle policy recreation race condition
Browse files Browse the repository at this point in the history
When the configuration policy is recreated after the
List call completes in the loop in the PeriodicallyExecConfigPolicies
method, the old configuration policy status gets set on the new
configuration policy which can lead to a compliance event not being set
on the replicated parent policy.

Relates:
https://issues.redhat.com/browse/ACM-10500

Co-authored-by: Justin Kulikauskas <jkulikau@redhat.com>
Signed-off-by: mprahl <mprahl@users.noreply.github.com>
  • Loading branch information
mprahl and JustinKuli committed Mar 28, 2024
1 parent 182d147 commit 7db6d28
Showing 1 changed file with 16 additions and 0 deletions.
16 changes: 16 additions & 0 deletions controllers/configurationpolicy_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -2982,6 +2982,7 @@ func (r *ConfigurationPolicyReconciler) updatePolicyStatus(
"Updating configurationPolicy status", "status", policy.Status.ComplianceState, "policy", policy.GetName(),
)

evaluatedUID := policy.UID
updatedStatus := policy.Status

maxRetries := 3
Expand All @@ -2991,6 +2992,21 @@ func (r *ConfigurationPolicyReconciler) updatePolicyStatus(
log.Info(fmt.Sprintf("Failed to refresh policy; using previously fetched version: %s", err))
} else {
policy.Status = updatedStatus

// If the policy was recreated after it was retrieved from the cache, there could be no compliance
// set on the parent policy. Updating the status here may prevent a compliance event from being generated on
// future evaluations so skipping the status update is very important. If sendEvent was true, there is still
// value to send the compliance event to the parent policy so that this history is not lost before the
// policy was recreated.
if evaluatedUID != policy.UID {
log.Info("The ConfigurationPolicy was recreated after it was evaluated. Skipping the status update.")

// Reset the original UID so that if there are more status updates (i.e. batches), the status on the
// API server is never updated.
policy.UID = evaluatedUID

break
}
}

err = r.Status().Update(context.TODO(), policy)
Expand Down

0 comments on commit 7db6d28

Please sign in to comment.