Skip to content

Commit

Permalink
fix(NfsBackupSchedule): Replacement of cronexpr library
Browse files Browse the repository at this point in the history
  • Loading branch information
ravi-shankar-sap committed Jan 14, 2025
1 parent 6ca143f commit 638c010
Show file tree
Hide file tree
Showing 7 changed files with 95 additions and 25 deletions.
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ require (
cloud.google.com/go/redis v1.17.2
cloud.google.com/go/resourcemanager v1.10.2
github.com/3th1nk/cidr v0.2.0
github.com/adhocore/gronx v1.19.5
github.com/Azure/azure-sdk-for-go/sdk/azcore v1.16.0
github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.8.0
github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/network/armnetwork/v5 v5.2.0
Expand All @@ -33,7 +34,6 @@ require (
github.com/google/uuid v1.6.0
github.com/googleapis/gax-go/v2 v2.14.0
github.com/gophercloud/gophercloud/v2 v2.4.0
github.com/gorhill/cronexpr v0.0.0-20180427100037-88b0669f7d75
github.com/imdario/mergo v0.3.16
github.com/mitchellh/mapstructure v1.5.0
github.com/onsi/ginkgo/v2 v2.22.0
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ github.com/Masterminds/semver/v3 v3.3.1 h1:QtNSWtVZ3nBfk8mAOu/B6v7FMJ+NHTIgUPi7r
github.com/Masterminds/semver/v3 v3.3.1/go.mod h1:4V+yj/TJE1HU9XfppCwVMZq3I84lprf4nC11bSS5beM=
github.com/Masterminds/sprig/v3 v3.3.0 h1:mQh0Yrg1XPo6vjYXgtf5OtijNAKJRNcTdOOGZe3tPhs=
github.com/Masterminds/sprig/v3 v3.3.0/go.mod h1:Zy1iXRYNqNLUolqCpL4uhk6SHUMAOSCzdgBfDb35Lz0=
github.com/adhocore/gronx v1.19.5 h1:cwIG4nT1v9DvadxtHBe6MzE+FZ1JDvAUC45U2fl4eSQ=
github.com/adhocore/gronx v1.19.5/go.mod h1:7oUY1WAU8rEJWmAxXR2DN0JaO4gi9khSgKjiRypqteg=
github.com/andybalholm/brotli v1.1.1 h1:PR2pgnyFznKEugtsUo0xLdDop5SKXd5Qf5ysW+7XdTA=
github.com/andybalholm/brotli v1.1.1/go.mod h1:05ib4cKhjx3OQYUY22hTVd34Bc8upXjOLL2rKwwZBoA=
github.com/antlr4-go/antlr/v4 v4.13.0 h1:lxCg3LAv+EUK6t1i0y1V6/SLeUi0eKEKdhQAlS8TVTI=
Expand Down Expand Up @@ -224,8 +226,6 @@ github.com/googleapis/gax-go/v2 v2.14.0 h1:f+jMrjBPl+DL9nI4IQzLUxMq7XrAqFYB7hBPq
github.com/googleapis/gax-go/v2 v2.14.0/go.mod h1:lhBCnjdLrWRaPvLWhmc8IS24m9mr07qSYnHncrgo+zk=
github.com/gophercloud/gophercloud/v2 v2.4.0 h1:XhP5tVEH3ni66NSNK1+0iSO6kaGPH/6srtx6Cr+8eCg=
github.com/gophercloud/gophercloud/v2 v2.4.0/go.mod h1:uJWNpTgJPSl2gyzJqcU/pIAhFUWvIkp8eE8M15n9rs4=
github.com/gorhill/cronexpr v0.0.0-20180427100037-88b0669f7d75 h1:f0n1xnMSmBLzVfsMMvriDyA75NB/oBgILX2GcHXIQzY=
github.com/gorhill/cronexpr v0.0.0-20180427100037-88b0669f7d75/go.mod h1:g2644b03hfBX9Ov0ZBDgXXens4rxSxmqFBbhvKv2yVA=
github.com/gorilla/websocket v1.5.3 h1:saDtZ6Pbx/0u+bgYQ3q96pZgCzfhKXGPqt7kZ72aNNg=
github.com/gorilla/websocket v1.5.3/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE=
github.com/hashicorp/errwrap v1.1.0 h1:OxrOeh75EUXMY8TBjag2fzXGZ40LB6IKw45YeGUDY2I=
Expand Down
21 changes: 19 additions & 2 deletions pkg/skr/backupschedule/calculateRecurringSchedule.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package backupschedule
import (
"context"
"fmt"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"time"

cloudresourcesv1beta1 "github.com/kyma-project/cloud-manager/api/cloud-resources/v1beta1"
Expand Down Expand Up @@ -39,13 +40,29 @@ func calculateRecurringSchedule(ctx context.Context, st composed.State) (error,

//Evaluate next run times.
var nextRunTimes []time.Time
var err error
if schedule.GetStartTime() != nil && !schedule.GetStartTime().IsZero() && schedule.GetStartTime().Time.After(now) {
logger.WithValues("BackupSchedule", schedule.GetName()).Info("StartTime is in future, using it.")
nextRunTimes = state.cronExpression.NextN(schedule.GetStartTime().Time.UTC(), MaxSchedules)
nextRunTimes, err = state.NextN(schedule.GetStartTime().Time.UTC(), MaxSchedules)
} else {
logger.WithValues("BackupSchedule", schedule.GetName()).Info(fmt.Sprintf("Using current time %s", now))
nextRunTimes = state.cronExpression.NextN(now.UTC(), MaxSchedules)
nextRunTimes, err = state.NextN(now.UTC(), MaxSchedules)
}
if err != nil {
logger.Info("Not able to calculate next runtime(s).")

schedule.SetState(cloudresourcesv1beta1.JobStateError)
return composed.PatchStatus(schedule).
SetExclusiveConditions(metav1.Condition{
Type: cloudresourcesv1beta1.ConditionTypeError,
Status: metav1.ConditionTrue,
Reason: cloudresourcesv1beta1.ReasonInvalidCronExpression,
Message: err.Error(),
}).
SuccessError(composed.StopAndForget).
Run(ctx, state)
}

logger.WithValues("BackupSchedule", schedule.GetName()).Info(fmt.Sprintf("Next RunTime is %v", nextRunTimes[0]))

//Update the status of the schedule with the next run times
Expand Down
11 changes: 0 additions & 11 deletions pkg/skr/backupschedule/calculateRecurringSchedule_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package backupschedule

import (
"context"
"github.com/gorhill/cronexpr"
"github.com/kyma-project/cloud-manager/api/cloud-resources/v1beta1"
"github.com/stretchr/testify/suite"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -83,11 +82,6 @@ func (suite *calculateRecurringScheduleSuite) TestAlreadySetSchedule() {
err = factory.skrCluster.K8sClient().Status().Update(ctx, obj)
suite.Nil(err)

//Set cron expression in state
expr, err := cronexpr.Parse(obj.Spec.Schedule)
suite.Nil(err)
state.cronExpression = expr

//Invoke API under test
err, _ctx := calculateRecurringSchedule(ctx, state)

Expand Down Expand Up @@ -121,11 +115,6 @@ func (suite *calculateRecurringScheduleSuite) testSchedule(schedule string, star
err = factory.skrCluster.K8sClient().Status().Update(ctx, obj)
suite.Nil(err)

//Set cron expression in state
expr, err := cronexpr.Parse(obj.Spec.Schedule)
suite.Nil(err)
state.cronExpression = expr

//Invoke API under test
err, _ = calculateRecurringSchedule(ctx, state)

Expand Down
31 changes: 29 additions & 2 deletions pkg/skr/backupschedule/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ package backupschedule

import (
"context"
"github.com/gorhill/cronexpr"
"github.com/adhocore/gronx"
cloudcontrolv1beta1 "github.com/kyma-project/cloud-manager/api/cloud-control/v1beta1"
"github.com/kyma-project/cloud-manager/pkg/common/abstractions"
"github.com/kyma-project/cloud-manager/pkg/composed"
Expand All @@ -20,7 +20,7 @@ type State struct {
SourceRef composed.ObjWithConditions
Backups []client.Object

cronExpression *cronexpr.Expression
gronx *gronx.Gronx
nextRunTime time.Time
createRunCompleted bool
deleteRunCompleted bool
Expand Down Expand Up @@ -60,9 +60,36 @@ func (f *stateFactory) NewState(ctx context.Context, baseState composed.State) (
KcpCluster: f.kcpCluster,
SkrCluster: f.skrCluster,
env: f.env,
gronx: gronx.New(),
}, nil
}

func (s *State) ObjAsBackupSchedule() BackupSchedule {
return s.Obj().(BackupSchedule)
}

func (s *State) NextN(fromTime time.Time, n uint) ([]time.Time, error) {
var times []time.Time
if n <= 0 {
return times, nil
}

expression := s.ObjAsBackupSchedule().GetActiveSchedule()
if fromTime.IsZero() {
fromTime = time.Now().UTC()
}

fromTime, err := gronx.NextTickAfter(expression, fromTime, true)
if err != nil {
return times, nil
}
times = append(times, fromTime)
for range n - 1 {
fromTime, err = gronx.NextTickAfter(expression, fromTime, false)
if err != nil {
return times, err
}
times = append(times, fromTime)
}
return times, nil
}
20 changes: 13 additions & 7 deletions pkg/skr/backupschedule/validateSchedule.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ package backupschedule

import (
"context"
"github.com/gorhill/cronexpr"
"fmt"
cloudresourcesv1beta1 "github.com/kyma-project/cloud-manager/api/cloud-resources/v1beta1"
"github.com/kyma-project/cloud-manager/pkg/composed"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand All @@ -18,6 +18,11 @@ func validateSchedule(ctx context.Context, st composed.State) (error, context.Co
return nil, nil
}

//If the schedule has not changed, continue
if schedule.GetSchedule() == schedule.GetActiveSchedule() {
return nil, nil
}

ctx = composed.LoggerIntoCtx(ctx, logger)
logger.Info("Validating BackupSchedule - Cron Expression")

Expand All @@ -27,9 +32,9 @@ func validateSchedule(ctx context.Context, st composed.State) (error, context.Co
if sch == "" {
return nil, nil
}
expr, err := cronexpr.Parse(sch)
valid := state.gronx.IsValid(sch)

if err != nil {
if !valid {
logger.Info("Invalid cron expression")

schedule.SetState(cloudresourcesv1beta1.JobStateError)
Expand All @@ -38,15 +43,16 @@ func validateSchedule(ctx context.Context, st composed.State) (error, context.Co
Type: cloudresourcesv1beta1.ConditionTypeError,
Status: metav1.ConditionTrue,
Reason: cloudresourcesv1beta1.ReasonInvalidCronExpression,
Message: err.Error(),
Message: fmt.Sprintf("Invalid cron expression: %s", sch),
}).
SuccessError(composed.StopAndForget).
Run(ctx, state)
}

logger.Info("Validated Cron Expression")

state.cronExpression = expr

return nil, ctx
state.ObjAsBackupSchedule().SetActiveSchedule(sch)
return composed.PatchStatus(schedule).
SuccessError(composed.StopWithRequeue).
Run(ctx, state)
}
31 changes: 31 additions & 0 deletions pkg/skr/backupschedule/validateSchedule_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,28 @@ func (suite *validateScheduleSuite) TestWhenNfsScheduleIsDeleting() {
suite.Nil(_ctx)
}

func (suite *validateScheduleSuite) TestWhenScheduleHasNotChanged() {
obj := gcpNfsBackupSchedule.DeepCopy()
obj.Spec.Schedule = "* * * * *"
obj.Status.Schedule = "* * * * *"
factory, err := newTestStateFactoryWithObj(obj)
suite.Nil(err)

ctx, cancel := context.WithCancel(context.Background())
defer cancel()

//Get state object with GcpNfsBackupSchedule
state, err := factory.newStateWith(obj)
suite.Nil(err)

//Invoke API under test
err, _ctx := validateSchedule(ctx, state)

//validate expected return values
suite.Nil(err)
suite.Nil(_ctx)
}

func (suite *validateScheduleSuite) TestEmptyCronExpression() {
obj := gcpNfsBackupSchedule.DeepCopy()
factory, err := newTestStateFactoryWithObj(obj)
Expand Down Expand Up @@ -104,7 +126,16 @@ func (suite *validateScheduleSuite) TestValidCronExpression() {
err, _ = validateSchedule(ctx, state)

//validate expected return values
suite.Equal(composed.StopWithRequeue, err)

fromK8s := &v1beta1.GcpNfsBackupSchedule{}
err = factory.skrCluster.K8sClient().Get(ctx,
types.NamespacedName{Name: gcpNfsBackupSchedule.Name,
Namespace: gcpNfsBackupSchedule.Namespace},
fromK8s)
suite.Nil(err)
suite.Equal(obj.Spec.Schedule, fromK8s.Status.Schedule)

suite.Equal("* * * * *", state.ObjAsBackupSchedule().GetSchedule())
}

Expand Down

0 comments on commit 638c010

Please sign in to comment.