Skip to content

Commit

Permalink
Bug: 2 ansiblejobs were created when disabled policy, "Disabled+Manua…
Browse files Browse the repository at this point in the history
…l" mode

Solution:  Add resourceV verification

Signed-off-by: Yi Rae Kim <yikim@redhat.com>
  • Loading branch information
yiraeChristineKim authored and openshift-merge-robot committed Jun 20, 2023
1 parent 80bd403 commit 0f36dc0
Show file tree
Hide file tree
Showing 3 changed files with 310 additions and 146 deletions.
14 changes: 14 additions & 0 deletions controllers/automation/policyautomation_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,20 @@ func (r *PolicyAutomationReconciler) Reconcile(
}

if policyAutomation.Annotations["policy.open-cluster-management.io/rerun"] == "true" {
AjExist, err := common.MatchPAResouceV(policyAutomation,
r.DynamicClient, policyAutomation.GetResourceVersion())
if err != nil {
log.Error(err, "Failed to compare Ansible job's resourceVersion")

return reconcile.Result{}, err
}

if AjExist {
log.Info("Ansiblejob already exist under this policyautomation resourceVersion")

return reconcile.Result{}, nil
}

targetList := common.FindNonCompliantClustersForPolicy(policy)
log.Info(
"Creating an Ansible job", "mode", "manual",
Expand Down
35 changes: 29 additions & 6 deletions controllers/common/ansible.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ const (
ControllerName string = "policy-automation"
PolicyAutomationLabel string = "policy.open-cluster-management.io/policyautomation-name"
PolicyAutomationGeneration string = "policy.open-cluster-management.io/policyautomation-generation"
// policyautomation-ResouceVersion
PolicyAutomationResouceV string = "policy.open-cluster-management.io/policyautomation-resource-version"
)

var log = ctrl.Log.WithName(ControllerName)
Expand All @@ -49,12 +51,6 @@ func MatchPAGeneration(policyAutomation *policyv1beta1.PolicyAutomation,
return false, err
}

ansiblejobLen := len(ansiblejobList.Items)
// Check whether new PolicyAutomation
if ansiblejobLen == 0 {
return false, nil
}

s := strconv.FormatInt(generation, 10)

for _, aj := range ansiblejobList.Items {
Expand All @@ -67,6 +63,32 @@ func MatchPAGeneration(policyAutomation *policyv1beta1.PolicyAutomation,
return false, nil
}

// Check any ansiblejob is made by current resourceVersion number
// Returning "true" means the policy automation already created ansiblejob with this resourceVersion
func MatchPAResouceV(policyAutomation *policyv1beta1.PolicyAutomation,
dynamicClient dynamic.Interface, resourceVersion string,
) (bool, error) {
ansiblejobList, err := dynamicClient.Resource(ansibleJobRes).Namespace(policyAutomation.GetNamespace()).List(
context.TODO(), metav1.ListOptions{
LabelSelector: fmt.Sprintf("%s=%s", PolicyAutomationLabel, policyAutomation.GetName()),
},
)
if err != nil {
log.Error(err, "Failed to get ansiblejob list")

return false, err
}

for _, aj := range ansiblejobList.Items {
annotations := aj.GetAnnotations()
if annotations[PolicyAutomationResouceV] == resourceVersion {
return true, nil
}
}

return false, nil
}

// CreateAnsibleJob creates ansiblejob with given PolicyAutomation
func CreateAnsibleJob(policyAutomation *policyv1beta1.PolicyAutomation,
dynamicClient dynamic.Interface, mode string, violationContext policyv1beta1.ViolationContext,
Expand All @@ -79,6 +101,7 @@ func CreateAnsibleJob(policyAutomation *policyv1beta1.PolicyAutomation,
"annotations": map[string]interface{}{
PolicyAutomationGeneration: strconv.
FormatInt(policyAutomation.GetGeneration(), 10),
PolicyAutomationResouceV: policyAutomation.GetResourceVersion(),
},
},
"spec": map[string]interface{}{
Expand Down
Loading

0 comments on commit 0f36dc0

Please sign in to comment.