Skip to content

Commit 21cd26d

Browse files
Bug: Only one policyviolation when policyViolationLimit=0
Create a policy with disabled set to true. Then, create a policyAutomation with "everyevent" mode and "policyViolationLimits" set to 0. Actual results: Only the violaition status of one cluster was passed into the ansiblejob Expected results: All violations status of all clusters should be passed Ref: https://issues.redhat.com/browse/ACM-4777 Signed-off-by: Yi Rae Kim <yikim@redhat.com>
1 parent 0370f38 commit 21cd26d

File tree

7 files changed

+373
-202
lines changed

7 files changed

+373
-202
lines changed

Makefile

+2
Original file line numberDiff line numberDiff line change
@@ -267,9 +267,11 @@ install-resources:
267267
kubectl create ns policy-propagator-test
268268
kubectl create ns managed1
269269
kubectl create ns managed2
270+
kubectl create ns managed3
270271
@echo creating cluster resources
271272
kubectl apply -f test/resources/managed1-cluster.yaml
272273
kubectl apply -f test/resources/managed2-cluster.yaml
274+
kubectl apply -f test/resources/managed3-cluster.yaml
273275
@echo setting a Hub cluster DNS name
274276
kubectl apply -f test/resources/case5_policy_automation/cluster-dns.yaml
275277

controllers/automation/policyPredicate.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
package automation
44

55
import (
6+
"github.com/google/go-cmp/cmp"
67
"sigs.k8s.io/controller-runtime/pkg/event"
78
"sigs.k8s.io/controller-runtime/pkg/predicate"
89

@@ -21,7 +22,7 @@ var policyPredicateFuncs = predicate.Funcs{
2122
//nolint:forcetypeassert
2223
plcObjOld := e.ObjectOld.(*policiesv1.Policy)
2324

24-
return plcObjNew.Status.ComplianceState != plcObjOld.Status.ComplianceState
25+
return !cmp.Equal(plcObjNew.Status.Status, plcObjOld.Status.Status)
2526
},
2627
CreateFunc: func(e event.CreateEvent) bool {
2728
return false

controllers/automation/policyautomation_controller.go

+12-1
Original file line numberDiff line numberDiff line change
@@ -360,7 +360,8 @@ func (r *PolicyAutomationReconciler) Reconcile(
360360
if len(targetList) > 0 {
361361
log.Info("Creating An Ansible job", "targetList", targetList)
362362
violationContext, _ := r.getViolationContext(ctx, policy, targetList, policyAutomation)
363-
err = common.CreateAnsibleJob(policyAutomation, r.DynamicClient, "scan", violationContext)
363+
err = common.CreateAnsibleJob(policyAutomation, r.DynamicClient, "scan",
364+
violationContext)
364365
if err != nil {
365366
return reconcile.Result{RequeueAfter: requeueAfter}, err
366367
}
@@ -381,6 +382,16 @@ func (r *PolicyAutomationReconciler) Reconcile(
381382
if len(targetList) > 0 {
382383
log.Info("Creating an Ansible job", "targetList", targetList)
383384

385+
AjExist, err := common.MatchPAGeneration(policyAutomation,
386+
r.DynamicClient, policyAutomation.GetGeneration())
387+
if err != nil {
388+
log.Error(err, "Failed to get Ansible job's generation")
389+
390+
return reconcile.Result{}, err
391+
}
392+
if AjExist {
393+
return reconcile.Result{}, nil
394+
}
384395
violationContext, _ := r.getViolationContext(ctx, policy, targetList, policyAutomation)
385396
err = common.CreateAnsibleJob(
386397
policyAutomation,

controllers/common/ansible.go

+61
Original file line numberDiff line numberDiff line change
@@ -5,18 +5,68 @@ package common
55
import (
66
"context"
77
"encoding/json"
8+
"fmt"
89
"reflect"
910
"regexp"
11+
"strconv"
1012
"strings"
1113

1214
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1315
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
1416
"k8s.io/apimachinery/pkg/runtime/schema"
1517
"k8s.io/client-go/dynamic"
18+
ctrl "sigs.k8s.io/controller-runtime"
1619

1720
policyv1beta1 "open-cluster-management.io/governance-policy-propagator/api/v1beta1"
1821
)
1922

23+
const (
24+
ControllerName string = "policy-automation"
25+
PolicyAutomationLabel string = "policy.open-cluster-management.io/policyautomation-name"
26+
PolicyAutomationGeneration string = "policy.open-cluster-management.io/policyautomation-generation"
27+
)
28+
29+
var log = ctrl.Log.WithName(ControllerName)
30+
31+
var ansibleJobRes = schema.GroupVersionResource{
32+
Group: "tower.ansible.com", Version: "v1alpha1",
33+
Resource: "ansiblejobs",
34+
}
35+
36+
// Check any ansiblejob is made by input genteration number
37+
// Returning "true" means the policy automation already created ansiblejob with the generation
38+
func MatchPAGeneration(policyAutomation *policyv1beta1.PolicyAutomation,
39+
dynamicClient dynamic.Interface, generation int64,
40+
) (bool, error) {
41+
ansiblejobList, err := dynamicClient.Resource(ansibleJobRes).Namespace(policyAutomation.GetNamespace()).List(
42+
context.TODO(), metav1.ListOptions{
43+
LabelSelector: fmt.Sprintf("%s=%s", PolicyAutomationLabel, policyAutomation.GetName()),
44+
},
45+
)
46+
if err != nil {
47+
log.Error(err, "Failed to get ansiblejob list")
48+
49+
return false, err
50+
}
51+
52+
ansiblejobLen := len(ansiblejobList.Items)
53+
// Check whether new PolicyAutomation
54+
if ansiblejobLen == 0 {
55+
return false, nil
56+
}
57+
58+
s := strconv.FormatInt(generation, 10)
59+
60+
for _, aj := range ansiblejobList.Items {
61+
annotations := aj.GetAnnotations()
62+
if annotations[PolicyAutomationGeneration] == s {
63+
return true, nil
64+
}
65+
}
66+
67+
return false, nil
68+
}
69+
2070
// CreateAnsibleJob creates ansiblejob with given PolicyAutomation
2171
func CreateAnsibleJob(policyAutomation *policyv1beta1.PolicyAutomation,
2272
dynamicClient dynamic.Interface, mode string, violationContext policyv1beta1.ViolationContext,
@@ -25,6 +75,12 @@ func CreateAnsibleJob(policyAutomation *policyv1beta1.PolicyAutomation,
2575
Object: map[string]interface{}{
2676
"apiVersion": "tower.ansible.com/v1alpha1",
2777
"kind": "AnsibleJob",
78+
"metadata": map[string]interface{}{
79+
"annotations": map[string]interface{}{
80+
PolicyAutomationGeneration: strconv.
81+
FormatInt(policyAutomation.GetGeneration(), 10),
82+
},
83+
},
2884
"spec": map[string]interface{}{
2985
"job_template_name": policyAutomation.Spec.Automation.Name,
3086
"tower_auth_secret": policyAutomation.Spec.Automation.TowerSecret,
@@ -63,6 +119,11 @@ func CreateAnsibleJob(policyAutomation *policyv1beta1.PolicyAutomation,
63119
mapExtraVars[fieldName] = value
64120
}
65121

122+
label := map[string]string{
123+
PolicyAutomationLabel: policyAutomation.GetName(),
124+
}
125+
ansibleJob.SetLabels(label)
126+
66127
ansibleJob.Object["spec"].(map[string]interface{})["extra_vars"] = mapExtraVars
67128

68129
if policyAutomation.Spec.Automation.JobTTL != nil {

go.mod

+1-1
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ require (
88
github.com/go-logr/zapr v1.2.3
99
github.com/onsi/ginkgo/v2 v2.9.4
1010
github.com/onsi/gomega v1.27.6
11+
github.com/google/go-cmp v0.5.9
1112
github.com/prometheus/client_golang v1.15.1
1213
github.com/spf13/pflag v1.0.5
1314
github.com/stolostron/go-log-utils v0.1.2
@@ -41,7 +42,6 @@ require (
4142
github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
4243
github.com/golang/protobuf v1.5.3 // indirect
4344
github.com/google/gnostic v0.6.9 // indirect
44-
github.com/google/go-cmp v0.5.9 // indirect
4545
github.com/google/gofuzz v1.2.0 // indirect
4646
github.com/google/pprof v0.0.0-20230502171905-255e3b9b56de // indirect
4747
github.com/google/uuid v1.3.0 // indirect

0 commit comments

Comments
 (0)