Keep the history of the past 5 triggered runs of DisruptionCron (#817)

* Refactor

* WIP not really working.

* Working stored history.

* Limit history size to 5.

* Fix lint.

* Update controllers/cron_rollout_helpers.go

Co-authored-by: Philip Thompson <philip.thompson@datadoghq.com>

* Rename struct as DisruptionCronTrigger.

* Address code review comments.

---------

Co-authored-by: Philip Thompson <philip.thompson@datadoghq.com>

Author: taihuynh167

api/v1beta1/disruption_cron_types.go

@@ -83,4 +83,14 @@ type DisruptionCronStatus struct {
 	// Time when the target resource was previously missing.
 	// +nullable
 	TargetResourcePreviouslyMissing *metav1.Time `json:"targetResourcePreviouslyMissing,omitempty"`
+
+	History []DisruptionCronTrigger `json:"history,omitempty"`
+}
+
+const MaxHistoryLen = 5
+
+type DisruptionCronTrigger struct {
+	Name      string      `json:"name,omitempty"`
+	Kind      string      `json:"kind,omitempty"`
+	CreatedAt metav1.Time `json:"createdAt,omitempty"`
 }

api/v1beta1/zz_generated.deepcopy.go

@@ -611,6 +611,18 @@ spec:
             status:
               description: DisruptionCronStatus defines the observed state of DisruptionCron
               properties:
+                history:
+                  items:
+                    properties:
+                      createdAt:
+                        format: date-time
+                        type: string
+                      kind:
+                        type: string
+                      name:
+                        type: string
+                    type: object
+                  type: array
                 lastScheduleTime:
                   description: The last time when the disruption was last successfully scheduled.
                   format: date-time

chart/templates/generated/chaos.datadoghq.com_disruptioncrons.yaml

@@ -8,6 +8,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"sort"
 	"time"
 
 	chaosv1beta1 "github.com/DataDog/chaos-controller/api/v1beta1"
@@ -174,41 +175,36 @@ func CreateDisruptionFromTemplate(ctx context.Context, cl client.Client, scheme
 }
 
 // getScheduledTimeForDisruption returns the scheduled time for a particular disruption.
-func getScheduledTimeForDisruption(disruption *chaosv1beta1.Disruption) (*time.Time, error) {
+func getScheduledTimeForDisruption(log *zap.SugaredLogger, disruption *chaosv1beta1.Disruption) time.Time {
 	timeRaw := disruption.Annotations[ScheduledAtAnnotation]
 	if len(timeRaw) == 0 {
-		return nil, nil
+		return time.Time{}
 	}
 
 	timeParsed, err := time.Parse(time.RFC3339, timeRaw)
 	if err != nil {
-		return nil, err
+		log.Errorw("unable to parse schedule time for child disruption", "err", err, "disruptionName", disruption.Name)
+		return time.Time{}
 	}
 
-	return &timeParsed, nil
+	return timeParsed
 }
 
-// getMostRecentScheduleTime returns the most recent scheduled time from a list of disruptions.
-func GetMostRecentScheduleTime(log *zap.SugaredLogger, disruptions *chaosv1beta1.DisruptionList) *time.Time {
-	var mostRecentScheduleTime *time.Time
+// GetMostRecentScheduleTime returns the most recent scheduled time from a list of disruptions.
+func GetMostRecentScheduleTime(log *zap.SugaredLogger, disruptions *chaosv1beta1.DisruptionList) time.Time {
+	length := len(disruptions.Items)
+	if length == 0 {
+		return time.Time{}
+	}
 
-	for _, disruption := range disruptions.Items {
-		scheduledTimeForDisruption, err := getScheduledTimeForDisruption(&disruption)
-		if err != nil {
-			log.Errorw("unable to parse schedule time for child disruption", "err", err, "disruption", disruption.Name)
-			continue
-		}
+	sort.Slice(disruptions.Items, func(i, j int) bool {
+		scheduleTime1 := getScheduledTimeForDisruption(log, &disruptions.Items[i])
+		scheduleTime2 := getScheduledTimeForDisruption(log, &disruptions.Items[j])
 
-		if scheduledTimeForDisruption != nil {
-			if mostRecentScheduleTime == nil {
-				mostRecentScheduleTime = scheduledTimeForDisruption
-			} else if mostRecentScheduleTime.Before(*scheduledTimeForDisruption) {
-				mostRecentScheduleTime = scheduledTimeForDisruption
-			}
-		}
-	}
+		return scheduleTime1.Before(scheduleTime2)
+	})
 
-	return mostRecentScheduleTime
+	return getScheduledTimeForDisruption(log, &disruptions.Items[length-1])
 }
 
 // generateDisruptionName produces a disruption name based on the specific CR controller, that's invoking it.

controllers/cron_rollout_helpers.go

@@ -54,6 +54,7 @@ func (r *DisruptionCronReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 		return ctrl.Result{}, client.IgnoreNotFound(err)
 	}
 
+	r.log.Infow("fetched last known history", "history", instance.Status.History)
 	DisruptionCronTags = []string{"disruptionCronName:" + instance.Name, "disruptionCronNamespace:" + instance.Namespace, "targetName:" + instance.Spec.TargetResource.Name}
 
 	if !instance.DeletionTimestamp.IsZero() {
@@ -162,6 +163,20 @@ func (r *DisruptionCronReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 	// Add the start time of the just initiated disruption to the status
 	instance.Status.LastScheduleTime = &metav1.Time{Time: missedRun}
 
+	// Add to history, then ensure only the last MaxHistoryLen items are kept
+	instance.Status.History = append(instance.Status.History, chaosv1beta1.DisruptionCronTrigger{
+		Name:      instance.ObjectMeta.Name,
+		Kind:      instance.TypeMeta.Kind,
+		CreatedAt: *instance.Status.LastScheduleTime,
+	})
+
+	if len(instance.Status.History) > chaosv1beta1.MaxHistoryLen {
+		instance.Status.History = instance.Status.History[len(instance.Status.History)-chaosv1beta1.MaxHistoryLen:]
+	}
+
+	r.log.Debugw("updating instance Status lastScheduleTime and history",
+		"lastScheduleTime", instance.Status.LastScheduleTime, "history", instance.Status.History)
+
 	if err := r.Client.Status().Update(ctx, instance); err != nil {
 		r.log.Warnw("unable to update LastScheduleTime of DisruptionCron status", "err", err)
 		return ctrl.Result{}, err
@@ -174,8 +189,8 @@ func (r *DisruptionCronReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 // based on the most recent schedule time among the given disruptions.
 func (r *DisruptionCronReconciler) updateLastScheduleTime(ctx context.Context, instance *chaosv1beta1.DisruptionCron, disruptions *chaosv1beta1.DisruptionList) error {
 	mostRecentScheduleTime := GetMostRecentScheduleTime(r.log, disruptions) // find the last run so we can update the status
-	if mostRecentScheduleTime != nil {
-		instance.Status.LastScheduleTime = &metav1.Time{Time: *mostRecentScheduleTime}
+	if !mostRecentScheduleTime.IsZero() {
+		instance.Status.LastScheduleTime = &metav1.Time{Time: mostRecentScheduleTime}
 		return r.Client.Status().Update(ctx, instance)
 	}
 

controllers/disruption_cron_controller.go

@@ -172,8 +172,8 @@ func (r *DisruptionRolloutReconciler) Reconcile(ctx context.Context, req ctrl.Re
 // based on the most recent schedule time among the given disruptions.
 func (r *DisruptionRolloutReconciler) updateLastScheduleTime(ctx context.Context, instance *chaosv1beta1.DisruptionRollout, disruptions *chaosv1beta1.DisruptionList) error {
 	mostRecentScheduleTime := GetMostRecentScheduleTime(r.log, disruptions) // find the last run so we can update the status
-	if mostRecentScheduleTime != nil {
-		instance.Status.LastScheduleTime = &metav1.Time{Time: *mostRecentScheduleTime}
+	if !mostRecentScheduleTime.IsZero() {
+		instance.Status.LastScheduleTime = &metav1.Time{Time: mostRecentScheduleTime}
 		return r.Client.Status().Update(ctx, instance)
 	}