Skip to content

Commit d2a3dae

Browse files
Record OperatorPolicy history in status
Refs: - https://issues.redhat.com/browse/ACM-20803 Signed-off-by: Justin Kulikauskas <[email protected]>
1 parent d68e13e commit d2a3dae

File tree

7 files changed

+106
-32
lines changed

7 files changed

+106
-32
lines changed

api/v1beta1/operatorpolicy_types.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,10 @@ type OperatorPolicyStatus struct {
242242
// policy is waiting for OLM to resolve the situation. If in the recent past,
243243
// the policy may update the status of the Subscription.
244244
SubscriptionInterventionTime *metav1.Time `json:"subscriptionInterventionTime,omitempty"`
245+
246+
// History is a list of the most recent compliance messages for this operator policy.
247+
// The first entry is the most recent, and the list is limited to 10 entries.
248+
History []policyv1.HistoryEvent `json:"history,omitempty"`
245249
}
246250

247251
// RelatedObjsOfKind iterates over the related objects in the status and returns a map of the index

api/v1beta1/zz_generated.deepcopy.go

Lines changed: 7 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

controllers/operatorpolicy_controller.go

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -267,18 +267,12 @@ func (r *OperatorPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Reque
267267

268268
errs := make([]error, 0)
269269

270-
conditionsToEmit, conditionChanged, err := r.handleResources(ctx, policy)
270+
conditionsToEmit, statusChanged, err := r.handleResources(ctx, policy)
271271
if err != nil {
272272
errs = append(errs, err)
273273
}
274274

275-
if conditionChanged || !reflect.DeepEqual(policy.Status, originalStatus) {
276-
if err := r.Status().Update(ctx, policy); err != nil {
277-
errs = append(errs, err)
278-
}
279-
}
280-
281-
if conditionChanged {
275+
if statusChanged {
282276
// Add an event for the "final" state of the policy, otherwise this only has the
283277
// "early" events (and possibly has zero events).
284278
conditionsToEmit = append(conditionsToEmit, calculateComplianceCondition(policy))
@@ -288,6 +282,33 @@ func (r *OperatorPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Reque
288282
if err := r.emitComplianceEvent(ctx, policy, cond); err != nil {
289283
errs = append(errs, err)
290284
}
285+
286+
var latestEvent policyv1.HistoryEvent
287+
288+
if len(policy.Status.History) > 0 {
289+
latestEvent = policy.Status.History[0]
290+
}
291+
292+
if latestEvent.Message != cond.Message {
293+
statusChanged = true
294+
newEvent := policyv1.HistoryEvent{
295+
LastTimestamp: metav1.MicroTime(cond.LastTransitionTime),
296+
Message: cond.Message,
297+
}
298+
299+
policy.Status.History = append([]policyv1.HistoryEvent{newEvent}, policy.Status.History...)
300+
301+
maxHistoryLength := 10 // At some point, we may make this length configurable
302+
if len(policy.Status.History) > maxHistoryLength {
303+
policy.Status.History = policy.Status.History[:maxHistoryLength]
304+
}
305+
}
306+
}
307+
308+
if statusChanged || !reflect.DeepEqual(policy.Status, originalStatus) {
309+
if err := r.Status().Update(ctx, policy); err != nil {
310+
errs = append(errs, err)
311+
}
291312
}
292313

293314
result := reconcile.Result{}

controllers/operatorpolicy_status.go

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import (
66
"slices"
77
"sort"
88
"strings"
9-
"time"
109

1110
operatorv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1"
1211
appsv1 "k8s.io/api/apps/v1"
@@ -302,13 +301,27 @@ func calculateComplianceCondition(policy *policyv1beta1.OperatorPolicy) metav1.C
302301
}
303302
}
304303

304+
message := strings.Join(messages, ", ")
305+
306+
if foundNonCompliant {
307+
message = "NonCompliant; " + message
308+
} else {
309+
message = "Compliant; " + message
310+
}
311+
312+
maxMessageLength := 4096
313+
if len(message) > maxMessageLength {
314+
truncMsg := "...[truncated]"
315+
message = message[:(maxMessageLength-len(truncMsg))] + truncMsg
316+
}
317+
305318
if foundNonCompliant {
306319
return metav1.Condition{
307320
Type: compliantConditionType,
308321
Status: metav1.ConditionFalse,
309322
LastTransitionTime: metav1.Now(),
310323
Reason: "NonCompliant",
311-
Message: "NonCompliant; " + strings.Join(messages, ", "),
324+
Message: message,
312325
}
313326
}
314327

@@ -317,7 +330,7 @@ func calculateComplianceCondition(policy *policyv1beta1.OperatorPolicy) metav1.C
317330
Status: metav1.ConditionTrue,
318331
LastTransitionTime: metav1.Now(),
319332
Reason: "Compliant",
320-
Message: "Compliant; " + strings.Join(messages, ", "),
333+
Message: message,
321334
}
322335
}
323336

@@ -334,11 +347,11 @@ func (r *OperatorPolicyReconciler) emitComplianceEvent(
334347
}
335348

336349
ownerRef := policy.OwnerReferences[0]
337-
now := time.Now()
350+
timestamp := complianceCondition.LastTransitionTime
338351
event := &corev1.Event{
339352
ObjectMeta: metav1.ObjectMeta{
340353
// This event name matches the convention of recorders from client-go
341-
Name: fmt.Sprintf("%v.%x", ownerRef.Name, now.UnixNano()),
354+
Name: fmt.Sprintf("%v.%x", ownerRef.Name, timestamp.UnixNano()),
342355
Namespace: policy.Namespace,
343356
},
344357
InvolvedObject: corev1.ObjectReference{
@@ -354,8 +367,8 @@ func (r *OperatorPolicyReconciler) emitComplianceEvent(
354367
Component: ControllerName,
355368
Host: r.InstanceName,
356369
},
357-
FirstTimestamp: metav1.NewTime(now),
358-
LastTimestamp: metav1.NewTime(now),
370+
FirstTimestamp: timestamp,
371+
LastTimestamp: timestamp,
359372
Count: 1,
360373
Type: "Normal",
361374
Action: "ComplianceStateUpdate",

deploy/crds/kustomize_operatorpolicy/policy.open-cluster-management.io_operatorpolicies.yaml

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,21 @@ spec:
310310
x-kubernetes-list-map-keys:
311311
- type
312312
x-kubernetes-list-type: map
313+
history:
314+
description: |-
315+
History is a list of the most recent compliance messages for this operator policy.
316+
The first entry is the most recent, and the list is limited to 10 entries.
317+
items:
318+
description: HistoryEvent is a timestamped message representing
319+
the policy compliance state at that time.
320+
properties:
321+
lastTimestamp:
322+
format: date-time
323+
type: string
324+
message:
325+
type: string
326+
type: object
327+
type: array
313328
observedGeneration:
314329
description: ObservedGeneration is the latest generation observed
315330
by the controller.

deploy/crds/policy.open-cluster-management.io_operatorpolicies.yaml

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -312,6 +312,21 @@ spec:
312312
x-kubernetes-list-map-keys:
313313
- type
314314
x-kubernetes-list-type: map
315+
history:
316+
description: |-
317+
History is a list of the most recent compliance messages for this operator policy.
318+
The first entry is the most recent, and the list is limited to 10 entries.
319+
items:
320+
description: HistoryEvent is a timestamped message representing
321+
the policy compliance state at that time.
322+
properties:
323+
lastTimestamp:
324+
format: date-time
325+
type: string
326+
message:
327+
type: string
328+
type: object
329+
type: array
315330
observedGeneration:
316331
description: ObservedGeneration is the latest generation observed
317332
by the controller.

test/e2e/case43_standalone_hub_templates_test.go

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import (
1515
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1616
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
1717

18-
policyv1 "open-cluster-management.io/config-policy-controller/api/v1"
1918
"open-cluster-management.io/config-policy-controller/test/utils"
2019
)
2120

@@ -316,10 +315,10 @@ var _ = Describe("When standalone-hub-templates is enabled", Ordered, Label("hub
316315

317316
By("Checking the compliance message")
318317
desiredMessage := "NonCompliant; failed to resolve the template"
319-
Eventually(func() []policyv1.HistoryEvent {
320-
return utils.GetHistoryEvents(clientManagedDynamic, gvrConfigPolicy,
321-
policyName, testNamespace, desiredMessage)
322-
}, defaultTimeoutSeconds, 1).ShouldNot(BeEmpty())
318+
Eventually(func() []string {
319+
return utils.GetHistoryMessages(clientManagedDynamic, gvrOperatorPolicy,
320+
policyName, testNamespace, "")
321+
}, defaultTimeoutSeconds, 1).Should(ContainElement(ContainSubstring(desiredMessage)))
323322
Eventually(func() []v1.Event {
324323
return utils.GetMatchingEvents(clientManaged, testNamespace, parentPolicyName,
325324
fmt.Sprintf("policy: %v/%v", testNamespace, policyName), desiredMessage, defaultTimeoutSeconds)
@@ -331,10 +330,10 @@ var _ = Describe("When standalone-hub-templates is enabled", Ordered, Label("hub
331330

332331
By("Checking the compliance message")
333332
desiredMessage := `NonCompliant; the operator namespace \('hello'\) does not exist,`
334-
Eventually(func() []policyv1.HistoryEvent {
335-
return utils.GetHistoryEvents(clientManagedDynamic, gvrConfigPolicy,
336-
policyName, testNamespace, desiredMessage)
337-
}, defaultTimeoutSeconds, 1).ShouldNot(BeEmpty())
333+
Eventually(func() []string {
334+
return utils.GetHistoryMessages(clientManagedDynamic, gvrOperatorPolicy,
335+
policyName, testNamespace, "")
336+
}, defaultTimeoutSeconds, 1).Should(ContainElement(MatchRegexp(desiredMessage)))
338337
Eventually(func() []v1.Event {
339338
return utils.GetMatchingEvents(clientManaged, testNamespace, parentPolicyName,
340339
fmt.Sprintf("policy: %v/%v", testNamespace, policyName), desiredMessage, defaultTimeoutSeconds)
@@ -349,10 +348,10 @@ var _ = Describe("When standalone-hub-templates is enabled", Ordered, Label("hub
349348

350349
By("Checking the compliance message")
351350
desiredMessage := `NonCompliant; the operator namespace \('changed'\) does not exist,`
352-
Eventually(func() []policyv1.HistoryEvent {
353-
return utils.GetHistoryEvents(clientManagedDynamic, gvrConfigPolicy,
354-
policyName, testNamespace, desiredMessage)
355-
}, defaultTimeoutSeconds, 1).ShouldNot(BeEmpty())
351+
Eventually(func() []string {
352+
return utils.GetHistoryMessages(clientManagedDynamic, gvrOperatorPolicy,
353+
policyName, testNamespace, "")
354+
}, defaultTimeoutSeconds, 1).Should(ContainElement(MatchRegexp(desiredMessage)))
356355
Eventually(func() []v1.Event {
357356
return utils.GetMatchingEvents(clientManaged, testNamespace, parentPolicyName,
358357
fmt.Sprintf("policy: %v/%v", testNamespace, policyName), desiredMessage, defaultTimeoutSeconds)
@@ -365,10 +364,10 @@ var _ = Describe("When standalone-hub-templates is enabled", Ordered, Label("hub
365364

366365
By("Checking the compliance message")
367366
desiredMessage := `NonCompliant; the namespace '{{hub fromConfigMap`
368-
Eventually(func() []policyv1.HistoryEvent {
369-
return utils.GetHistoryEvents(clientManagedDynamic, gvrConfigPolicy,
370-
policyName, testNamespace, desiredMessage)
371-
}, defaultTimeoutSeconds, 1).ShouldNot(BeEmpty())
367+
Eventually(func() []string {
368+
return utils.GetHistoryMessages(clientManagedDynamic, gvrOperatorPolicy,
369+
policyName, testNamespace, "")
370+
}, defaultTimeoutSeconds, 1).Should(ContainElement(ContainSubstring(desiredMessage)))
372371
Eventually(func() []v1.Event {
373372
return utils.GetMatchingEvents(clientManaged, testNamespace, parentPolicyName,
374373
fmt.Sprintf("policy: %v/%v", testNamespace, policyName), desiredMessage, defaultTimeoutSeconds)

0 commit comments

Comments
 (0)