refactor: get the last execution time from the cleanup policy interface (#8531) (#8541)

Signed-off-by: Mariam Fahmy <mariam.fahmy@nirmata.com>
Co-authored-by: Mariam Fahmy <mariam.fahmy@nirmata.com>

api/kyverno/v2alpha1/cleanup_policy_interface.go

@@ -1,6 +1,8 @@
 package v2alpha1
 
 import (
+	"time"
+
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apimachinery/pkg/util/validation/field"
@@ -13,6 +15,8 @@ type CleanupPolicyInterface interface {
 	IsNamespaced() bool
 	GetSpec() *CleanupPolicySpec
 	GetStatus() *CleanupPolicyStatus
+	GetExecutionTime() (*time.Time, error)
+	GetNextExecutionTime(time.Time) (*time.Time, error)
 	Validate(sets.Set[string]) field.ErrorList
 	GetKind() string
 	GetAPIVersion() string

api/kyverno/v2alpha1/cleanup_policy_types.go

@@ -17,6 +17,9 @@ limitations under the License.
 package v2alpha1
 
 import (
+	"time"
+
+	"github.com/aptible/supercronic/cronexpr"
 	kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1"
 	kyvernov2beta1 "github.com/kyverno/kyverno/api/kyverno/v2beta1"
 	datautils "github.com/kyverno/kyverno/pkg/utils/data"
@@ -58,6 +61,27 @@ func (p *CleanupPolicy) GetStatus() *CleanupPolicyStatus {
 	return &p.Status
 }
 
+// GetExecutionTime returns the execution time of the policy
+func (p *CleanupPolicy) GetExecutionTime() (*time.Time, error) {
+	lastExecutionTime := p.Status.LastExecutionTime.Time
+	if lastExecutionTime.IsZero() {
+		creationTime := p.GetCreationTimestamp().Time
+		return p.GetNextExecutionTime(creationTime)
+	} else {
+		return p.GetNextExecutionTime(lastExecutionTime)
+	}
+}
+
+// GetNextExecutionTime returns the next execution time of the policy
+func (p *CleanupPolicy) GetNextExecutionTime(time time.Time) (*time.Time, error) {
+	cronExpr, err := cronexpr.Parse(p.Spec.Schedule)
+	if err != nil {
+		return nil, err
+	}
+	nextExecutionTime := cronExpr.Next(time)
+	return &nextExecutionTime, nil
+}
+
 // Validate implements programmatic validation
 func (p *CleanupPolicy) Validate(clusterResources sets.Set[string]) (errs field.ErrorList) {
 	errs = append(errs, kyvernov1.ValidatePolicyName(field.NewPath("metadata").Child("name"), p.Name)...)
@@ -123,6 +147,27 @@ func (p *ClusterCleanupPolicy) GetStatus() *CleanupPolicyStatus {
 	return &p.Status
 }
 
+// GetExecutionTime returns the execution time of the policy
+func (p *ClusterCleanupPolicy) GetExecutionTime() (*time.Time, error) {
+	lastExecutionTime := p.Status.LastExecutionTime.Time
+	if lastExecutionTime.IsZero() {
+		creationTime := p.GetCreationTimestamp().Time
+		return p.GetNextExecutionTime(creationTime)
+	} else {
+		return p.GetNextExecutionTime(lastExecutionTime)
+	}
+}
+
+// GetNextExecutionTime returns the next execution time of the policy
+func (p *ClusterCleanupPolicy) GetNextExecutionTime(time time.Time) (*time.Time, error) {
+	cronExpr, err := cronexpr.Parse(p.Spec.Schedule)
+	if err != nil {
+		return nil, err
+	}
+	nextExecutionTime := cronExpr.Next(time)
+	return &nextExecutionTime, nil
+}
+
 // GetKind returns the resource kind
 func (p *ClusterCleanupPolicy) GetKind() string {
 	return p.Kind

pkg/controllers/cleanup/controller.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"time"
 
-	"github.com/aptible/supercronic/cronexpr"
 	"github.com/go-logr/logr"
 	kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1"
 	kyvernov1beta1 "github.com/kyverno/kyverno/api/kyverno/v1beta1"
@@ -327,44 +326,31 @@ func (c *controller) reconcile(ctx context.Context, logger logr.Logger, key, nam
 		logger.Error(err, "unable to get the policy from policy informer")
 		return err
 	}
-	spec := policy.GetSpec()
-	cronExpr, err := cronexpr.Parse(spec.Schedule)
+
+	var nextExecutionTime *time.Time
+	executionTime, err := policy.GetExecutionTime()
 	if err != nil {
-		logger.Error(err, "unable to parse the schedule")
+		logger.Error(err, "failed to get the policy execution time")
 		return err
 	}
-
-	status := policy.GetStatus()
-	creationTime := policy.GetCreationTimestamp().Time
-	firstExecutionTime := cronExpr.Next(creationTime)
-
-	var nextExecutionTime time.Time
-	// In case it isn't the first execution of the cleanup policy.
-	if firstExecutionTime.Before(time.Now()) {
-		var executionTime time.Time
-		if status.LastExecutionTime.IsZero() {
-			executionTime = firstExecutionTime
-		} else {
-			executionTime = cronExpr.Next(status.LastExecutionTime.Time)
-		}
 	// In case it is the time to do the cleanup process
-		if time.Now().After(executionTime) {
+	if time.Now().After(*executionTime) {
 		err := c.cleanup(ctx, logger, policy)
 		if err != nil {
 			return err
 		}
-			c.updateCleanupPolicyStatus(ctx, policy, namespace, executionTime)
-			nextExecutionTime = cronExpr.Next(executionTime)
+		c.updateCleanupPolicyStatus(ctx, policy, namespace, *executionTime)
+		nextExecutionTime, err = policy.GetNextExecutionTime(*executionTime)
+		if err != nil {
+			logger.Error(err, "failed to get the policy next execution time")
+			return err
+		}
 	} else {
 		nextExecutionTime = executionTime
 	}
-	} else {
-		// In case it is the first execution of the cleanup policy.
-		nextExecutionTime = firstExecutionTime
-	}
 
 	// calculate the remaining time until deletion.
-	timeRemaining := time.Until(nextExecutionTime)
+	timeRemaining := time.Until(*nextExecutionTime)
 	// add the item back to the queue after the remaining time.
 	c.queue.AddAfter(key, timeRemaining)
 	return nil