Skip to content

Commit cb9b5db

Browse files
Refactor rootStatusPolicy to make it as common function
Signed-off-by: Yi Rae Kim <[email protected]>
1 parent 1e61f36 commit cb9b5db

15 files changed

+169
-1289
lines changed

controllers/common/common.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ func IsInClusterNamespace(ctx context.Context, c client.Client, ns string) (bool
4747
return true, nil
4848
}
4949

50-
func IsReplicatedPolicy(c client.Client, policy client.Object) (bool, error) {
50+
func IsReplicatedPolicy(ctx context.Context, c client.Client, policy client.Object) (bool, error) {
5151
rootPlcName := policy.GetLabels()[RootPolicyLabel]
5252
if rootPlcName == "" {
5353
return false, nil
@@ -58,7 +58,7 @@ func IsReplicatedPolicy(c client.Client, policy client.Object) (bool, error) {
5858
return false, fmt.Errorf("invalid value set in %s: %w", RootPolicyLabel, err)
5959
}
6060

61-
return IsInClusterNamespace(context.TODO(), c, policy.GetNamespace())
61+
return IsInClusterNamespace(ctx, c, policy.GetNamespace())
6262
}
6363

6464
// IsForPolicyOrPolicySet returns true if any of the subjects of the PlacementBinding are Policies
@@ -270,7 +270,7 @@ func GetRepPoliciesInPlacementBinding(
270270
// Use this for removing duplicated policies
271271
rootPolicyRequest := GetPoliciesInPlacementBinding(ctx, c, pb)
272272

273-
result := []reconcile.Request{}
273+
result := make([]reconcile.Request, 0, len(rootPolicyRequest)*len(decisions))
274274

275275
for _, rp := range rootPolicyRequest {
276276
for _, pd := range decisions {
@@ -324,9 +324,9 @@ const (
324324
PlacementRule PlacementRefKinds = "PlacementRule"
325325
)
326326

327-
// GetRootPolicyResult find and filter placementbindings which have namespace and placementRef.name.
327+
// GetRootPolicyRequests find and filter placementbindings which have namespace and placementRef.name.
328328
// Gather all root policies under placementbindings
329-
func GetRootPolicyResult(ctx context.Context, c client.Client,
329+
func GetRootPolicyRequests(ctx context.Context, c client.Client,
330330
namespace, placementRefName string, refKind PlacementRefKinds,
331331
) ([]reconcile.Request, error) {
332332
kindGroupMap := map[PlacementRefKinds]string{

controllers/common/common_status_update.go

Lines changed: 73 additions & 115 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,12 @@
1-
// Copyright (c) 2023 Red Hat, Inc.
21
// Copyright Contributors to the Open Cluster Management project
32

43
package common
54

65
import (
76
"context"
8-
"errors"
97
"fmt"
8+
"reflect"
109
"sort"
11-
"strings"
1210

1311
k8serrors "k8s.io/apimachinery/pkg/api/errors"
1412
"k8s.io/apimachinery/pkg/types"
@@ -20,22 +18,22 @@ import (
2018
policiesv1beta1 "open-cluster-management.io/governance-policy-propagator/api/v1beta1"
2119
)
2220

23-
// RootStatusUpdat update Root policy status with bound decisions and placements
24-
func RootStatusUpdate(c client.Client, rootPolicy *policiesv1.Policy) error {
25-
placements, decisions, err := GetClusterDecisions(c, rootPolicy)
21+
// RootStatusUpdate updates the root policy status with bound decisions, placements, and cluster status.
22+
func RootStatusUpdate(ctx context.Context, c client.Client, rootPolicy *policiesv1.Policy) (DecisionSet, error) {
23+
placements, decisions, err := GetClusterDecisions(ctx, c, rootPolicy)
2624
if err != nil {
2725
log.Info("Failed to get any placement decisions. Giving up on the request.")
2826

29-
return errors.New("could not get the placement decisions")
27+
return nil, err
3028
}
3129

32-
cpcs, cpcsErr := CalculatePerClusterStatus(c, rootPolicy, decisions)
30+
cpcs, cpcsErr := CalculatePerClusterStatus(ctx, c, rootPolicy, decisions)
3331
if cpcsErr != nil {
3432
// If there is a new replicated policy, then its lookup is expected to fail - it hasn't been created yet.
3533
log.Error(cpcsErr, "Failed to get at least one replicated policy, but that may be expected. Ignoring.")
3634
}
3735

38-
err = c.Get(context.TODO(),
36+
err = c.Get(ctx,
3937
types.NamespacedName{
4038
Namespace: rootPolicy.Namespace,
4139
Name: rootPolicy.Name,
@@ -44,26 +42,32 @@ func RootStatusUpdate(c client.Client, rootPolicy *policiesv1.Policy) error {
4442
log.Error(err, "Failed to refresh the cached policy. Will use existing policy.")
4543
}
4644

47-
// make a copy of the original status
48-
originalCPCS := make([]*policiesv1.CompliancePerClusterStatus, len(rootPolicy.Status.Status))
49-
copy(originalCPCS, rootPolicy.Status.Status)
45+
complianceState := CalculateRootCompliance(cpcs)
46+
47+
if reflect.DeepEqual(rootPolicy.Status.Status, cpcs) &&
48+
rootPolicy.Status.ComplianceState == complianceState &&
49+
reflect.DeepEqual(rootPolicy.Status.Placement, placements) {
50+
return decisions, nil
51+
}
52+
53+
log.Info("Updating the root policy status", "RootPolicyName", rootPolicy.Name, "Namespace", rootPolicy.Namespace)
5054

5155
rootPolicy.Status.Status = cpcs
52-
rootPolicy.Status.ComplianceState = CalculateRootCompliance(cpcs)
56+
rootPolicy.Status.ComplianceState = complianceState
5357
rootPolicy.Status.Placement = placements
5458

55-
err = c.Status().Update(context.TODO(), rootPolicy)
59+
err = c.Status().Update(ctx, rootPolicy)
5660
if err != nil {
57-
return err
61+
return nil, err
5862
}
5963

60-
return nil
64+
return decisions, nil
6165
}
6266

6367
// GetPolicyPlacementDecisions retrieves the placement decisions for a input PlacementBinding when
6468
// the policy is bound within it. It can return an error if the PlacementBinding is invalid, or if
6569
// a required lookup fails.
66-
func GetPolicyPlacementDecisions(c client.Client,
70+
func GetPolicyPlacementDecisions(ctx context.Context, c client.Client,
6771
instance *policiesv1.Policy, pb *policiesv1.PlacementBinding,
6872
) (decisions []appsv1.PlacementDecision, placements []*policiesv1.Placement, err error) {
6973
if !HasValidPlacementRef(pb) {
@@ -91,7 +95,7 @@ func GetPolicyPlacementDecisions(c client.Client,
9195
if _, exists := policySetSubjects[subject.Name]; !exists {
9296
policySetSubjects[subject.Name] = struct{}{}
9397

94-
if IsPolicyInPolicySet(c, instance.GetName(), subject.Name, pb.GetNamespace()) {
98+
if IsPolicyInPolicySet(ctx, c, instance.GetName(), subject.Name, pb.GetNamespace()) {
9599
placements = append(placements, &policiesv1.Placement{
96100
PlacementBinding: pb.GetName(),
97101
PolicySet: subject.Name,
@@ -115,7 +119,7 @@ func GetPolicyPlacementDecisions(c client.Client,
115119
switch pb.PlacementRef.Kind {
116120
case "PlacementRule":
117121
plr := &appsv1.PlacementRule{}
118-
if err := c.Get(context.TODO(), refNN, plr); err != nil && !k8serrors.IsNotFound(err) {
122+
if err := c.Get(ctx, refNN, plr); err != nil && !k8serrors.IsNotFound(err) {
119123
return nil, nil, fmt.Errorf("failed to check for PlacementRule '%v': %w", pb.PlacementRef.Name, err)
120124
}
121125

@@ -124,7 +128,7 @@ func GetPolicyPlacementDecisions(c client.Client,
124128
}
125129
case "Placement":
126130
pl := &clusterv1beta1.Placement{}
127-
if err := c.Get(context.TODO(), refNN, pl); err != nil && !k8serrors.IsNotFound(err) {
131+
if err := c.Get(ctx, refNN, pl); err != nil && !k8serrors.IsNotFound(err) {
128132
return nil, nil, fmt.Errorf("failed to check for Placement '%v': %w", pb.PlacementRef.Name, err)
129133
}
130134

@@ -143,154 +147,108 @@ func GetPolicyPlacementDecisions(c client.Client,
143147
return nil, placements, nil
144148
}
145149

146-
decisions, err = GetDecisions(c, pb)
150+
decisions, err = GetDecisions(ctx, c, pb)
147151

148152
return decisions, placements, err
149153
}
150154

151-
// GetAllClusterDecisions calculates which managed clusters should have a replicated policy, and
152-
// whether there are any BindingOverrides for that cluster. The placements array it returns is
153-
// sorted by PlacementBinding name. It can return an error if the PlacementBinding is invalid, or if
154-
// a required lookup fails.
155-
func GetAllClusterDecisions(
155+
type DecisionSet map[appsv1.PlacementDecision]bool
156+
157+
// GetClusterDecisions identifies all managed clusters which should have a replicated policy using the root policy
158+
// This returns unique decisions and placements that are NOT under Restricted subset.
159+
// Also this function returns placements that are under restricted subset.
160+
// But these placements include decisions which are under non-restricted subset.
161+
// In other words, this function returns placements which include at least one decision under non-restricted subset.
162+
func GetClusterDecisions(
163+
ctx context.Context,
156164
c client.Client,
157-
instance *policiesv1.Policy, pbList *policiesv1.PlacementBindingList,
165+
rootPolicy *policiesv1.Policy,
158166
) (
159-
decisions map[appsv1.PlacementDecision]policiesv1.BindingOverrides, placements []*policiesv1.Placement, err error,
167+
[]*policiesv1.Placement, DecisionSet, error,
160168
) {
161-
decisions = make(map[appsv1.PlacementDecision]policiesv1.BindingOverrides)
169+
log := log.WithValues("policyName", rootPolicy.GetName(), "policyNamespace", rootPolicy.GetNamespace())
170+
decisions := make(map[appsv1.PlacementDecision]bool)
171+
172+
pbList := &policiesv1.PlacementBindingList{}
162173

163-
// Process all placement bindings without subFilter
174+
err := c.List(ctx, pbList, &client.ListOptions{Namespace: rootPolicy.GetNamespace()})
175+
if err != nil {
176+
log.Error(err, "Could not list the placement bindings")
177+
178+
return nil, decisions, err
179+
}
180+
181+
placements := []*policiesv1.Placement{}
182+
183+
// Gather all placements and decisions when it is NOT policiesv1.Restricted
164184
for i, pb := range pbList.Items {
165185
if pb.SubFilter == policiesv1.Restricted {
166186
continue
167187
}
168188

169-
plcDecisions, plcPlacements, err := GetPolicyPlacementDecisions(c, instance, &pbList.Items[i])
189+
plcDecisions, plcPlacements, err := GetPolicyPlacementDecisions(ctx, c, rootPolicy, &pbList.Items[i])
170190
if err != nil {
171191
return nil, nil, err
172192
}
173193

174194
if len(plcDecisions) == 0 {
175-
log.Info("No placement decisions to process for this policy from this binding",
176-
"policyName", instance.GetName(), "bindingName", pb.GetName())
195+
log.Info("No placement decisions to process for this policy from this non-restricted binding",
196+
"policyName", rootPolicy.GetName(), "bindingName", pb.GetName())
177197
}
178198

179-
for _, decision := range plcDecisions {
180-
if overrides, ok := decisions[decision]; ok {
181-
// Found cluster in the decision map
182-
if strings.EqualFold(pb.BindingOverrides.RemediationAction, string(policiesv1.Enforce)) {
183-
overrides.RemediationAction = strings.ToLower(string(policiesv1.Enforce))
184-
decisions[decision] = overrides
185-
}
186-
} else {
187-
// No found cluster in the decision map, add it to the map
188-
decisions[decision] = policiesv1.BindingOverrides{
189-
// empty string if pb.BindingOverrides.RemediationAction is not defined
190-
RemediationAction: strings.ToLower(pb.BindingOverrides.RemediationAction),
191-
}
192-
}
199+
// Decisions are all unique
200+
for _, plcDecision := range plcDecisions {
201+
decisions[plcDecision] = true
193202
}
194203

195204
placements = append(placements, plcPlacements...)
196205
}
197206

198-
if len(decisions) == 0 {
199-
sort.Slice(placements, func(i, j int) bool {
200-
return placements[i].PlacementBinding < placements[j].PlacementBinding
201-
})
202-
203-
// No decisions, and subfilters can't add decisions, so we can stop early.
204-
return nil, placements, nil
205-
}
206-
207-
// Process all placement bindings with subFilter:restricted
207+
// Gather placements which have at least one decision that is included in NON-Restricted
208208
for i, pb := range pbList.Items {
209209
if pb.SubFilter != policiesv1.Restricted {
210210
continue
211211
}
212212

213213
foundInDecisions := false
214214

215-
plcDecisions, plcPlacements, err := GetPolicyPlacementDecisions(c, instance, &pbList.Items[i])
215+
plcDecisions, plcPlacements, err := GetPolicyPlacementDecisions(ctx, c, rootPolicy, &pbList.Items[i])
216216
if err != nil {
217217
return nil, nil, err
218218
}
219219

220220
if len(plcDecisions) == 0 {
221-
log.Info("No placement decisions to process for this policy from this binding",
222-
"policyName", instance.GetName(), "bindingName", pb.GetName())
221+
log.Info("No placement decisions to process for this policy from this restricted binding",
222+
"policyName", rootPolicy.GetName(), "bindingName", pb.GetName())
223223
}
224224

225-
for _, decision := range plcDecisions {
226-
if overrides, ok := decisions[decision]; ok {
227-
// Found cluster in the decision map
225+
// Decisions are all unique
226+
for _, plcDecision := range plcDecisions {
227+
if _, ok := decisions[plcDecision]; ok {
228228
foundInDecisions = true
229-
230-
if strings.EqualFold(pb.BindingOverrides.RemediationAction, string(policiesv1.Enforce)) {
231-
overrides.RemediationAction = strings.ToLower(string(policiesv1.Enforce))
232-
decisions[decision] = overrides
233-
}
234229
}
230+
231+
decisions[plcDecision] = true
235232
}
236233

237234
if foundInDecisions {
238235
placements = append(placements, plcPlacements...)
239236
}
240237
}
241238

242-
sort.Slice(placements, func(i, j int) bool {
243-
return placements[i].PlacementBinding < placements[j].PlacementBinding
244-
})
245-
246-
return decisions, placements, nil
247-
}
248-
249-
type DecisionSet map[appsv1.PlacementDecision]bool
250-
251-
// getClusterDecisions identifies all managed clusters which should have a replicated policy using RootPolicy
252-
func GetClusterDecisions(
253-
c client.Client,
254-
instance *policiesv1.Policy,
255-
) (
256-
[]*policiesv1.Placement, DecisionSet, error,
257-
) {
258-
log := log.WithValues("policyName", instance.GetName(), "policyNamespace", instance.GetNamespace())
259-
decisions := make(map[appsv1.PlacementDecision]bool)
260-
261-
pbList := &policiesv1.PlacementBindingList{}
262-
263-
err := c.List(context.TODO(), pbList, &client.ListOptions{Namespace: instance.GetNamespace()})
264-
if err != nil {
265-
log.Error(err, "Could not list the placement bindings")
266-
267-
return nil, decisions, err
268-
}
269-
270-
allClusterDecisions, placements, err := GetAllClusterDecisions(c, instance, pbList)
271-
if err != nil {
272-
return placements, decisions, err
273-
}
274-
275-
if allClusterDecisions == nil {
276-
allClusterDecisions = make(map[appsv1.PlacementDecision]policiesv1.BindingOverrides)
277-
}
278-
279-
for dec := range allClusterDecisions {
280-
decisions[dec] = true
281-
}
282-
283239
return placements, decisions, nil
284240
}
285241

286242
// CalculatePerClusterStatus lists up all policies replicated from the input policy, and stores
287243
// their compliance states in the result list. The result is sorted by cluster name. An error
288244
// will be returned if lookup of a replicated policy fails, but all lookups will still be attempted.
289245
func CalculatePerClusterStatus(
246+
ctx context.Context,
290247
c client.Client,
291-
instance *policiesv1.Policy, decisions DecisionSet,
248+
rootPolicy *policiesv1.Policy,
249+
decisions DecisionSet,
292250
) ([]*policiesv1.CompliancePerClusterStatus, error) {
293-
if instance.Spec.Disabled {
251+
if rootPolicy.Spec.Disabled {
294252
return nil, nil
295253
}
296254

@@ -301,10 +259,10 @@ func CalculatePerClusterStatus(
301259
for dec := range decisions {
302260
replicatedPolicy := &policiesv1.Policy{}
303261
key := types.NamespacedName{
304-
Namespace: dec.ClusterNamespace, Name: instance.Namespace + "." + instance.Name,
262+
Namespace: dec.ClusterNamespace, Name: rootPolicy.Namespace + "." + rootPolicy.Name,
305263
}
306264

307-
err := c.Get(context.TODO(), key, replicatedPolicy)
265+
err := c.Get(ctx, key, replicatedPolicy)
308266
if err != nil {
309267
if k8serrors.IsNotFound(err) {
310268
status = append(status, &policiesv1.CompliancePerClusterStatus{
@@ -332,7 +290,7 @@ func CalculatePerClusterStatus(
332290
return status, lookupErr
333291
}
334292

335-
func IsPolicyInPolicySet(c client.Client, policyName, policySetName, namespace string) bool {
293+
func IsPolicyInPolicySet(ctx context.Context, c client.Client, policyName, policySetName, namespace string) bool {
336294
log := log.WithValues("policyName", policyName, "policySetName", policySetName, "policyNamespace", namespace)
337295

338296
policySet := policiesv1beta1.PolicySet{}
@@ -341,7 +299,7 @@ func IsPolicyInPolicySet(c client.Client, policyName, policySetName, namespace s
341299
Namespace: namespace,
342300
}
343301

344-
if err := c.Get(context.TODO(), setNN, &policySet); err != nil {
302+
if err := c.Get(ctx, setNN, &policySet); err != nil {
345303
log.Error(err, "Failed to get the policyset")
346304

347305
return false

0 commit comments

Comments
 (0)