diff --git a/go.mod b/go.mod index 335bc35b0..7ddf96385 100644 --- a/go.mod +++ b/go.mod @@ -11,6 +11,7 @@ require ( github.com/onsi/ginkgo/v2 v2.11.0 github.com/onsi/gomega v1.27.8 github.com/prometheus/client_golang v1.14.0 + github.com/robfig/cron/v3 v3.0.1 github.com/spf13/pflag v1.0.5 go.uber.org/zap v1.24.0 golang.org/x/exp v0.0.0-20230213192124-5e25df0256eb @@ -99,7 +100,6 @@ require ( github.com/prometheus/client_model v0.3.0 // indirect github.com/prometheus/common v0.37.0 // indirect github.com/prometheus/procfs v0.8.0 // indirect - github.com/robfig/cron/v3 v3.0.1 // indirect github.com/russross/blackfriday/v2 v2.1.0 // indirect github.com/sirupsen/logrus v1.8.1 // indirect github.com/spf13/afero v1.8.2 // indirect diff --git a/pkg/health/condition/builder.go b/pkg/health/condition/builder.go index a26d6cb70..8b471cdec 100644 --- a/pkg/health/condition/builder.go +++ b/pkg/health/condition/builder.go @@ -23,7 +23,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -// skipMergeConditions contain the list of conditions we dont want to add to the list if not recalculated +// skipMergeConditions contain the list of conditions we don't want to add to the list if not recalculated var skipMergeConditions = map[druidv1alpha1.ConditionType]struct{}{ druidv1alpha1.ConditionTypeReady: {}, druidv1alpha1.ConditionTypeAllMembersReady: {}, @@ -111,7 +111,7 @@ func (b *defaultBuilder) Build(replicas int32) []druidv1alpha1.Condition { if condition.Status == "" { condition.Status = druidv1alpha1.ConditionUnknown } - condition.Reason = ConditionNotChecked + condition.Reason = NotChecked condition.Message = "etcd cluster has been scaled down" } else { condition.Status = res.Status() diff --git a/pkg/health/condition/check_all_members.go b/pkg/health/condition/check_all_members.go index b6f270cbb..3bc5cfb56 100644 --- a/pkg/health/condition/check_all_members.go +++ b/pkg/health/condition/check_all_members.go @@ -18,12 +18,13 @@ import ( "context" druidv1alpha1 "github.com/gardener/etcd-druid/api/v1alpha1" + "github.com/go-logr/logr" "sigs.k8s.io/controller-runtime/pkg/client" ) type allMembersReady struct{} -func (a *allMembersReady) Check(_ context.Context, etcd druidv1alpha1.Etcd) Result { +func (a *allMembersReady) Check(_ context.Context, _ logr.Logger, etcd druidv1alpha1.Etcd) Result { if len(etcd.Status.Members) == 0 { return &result{ conType: druidv1alpha1.ConditionTypeAllMembersReady, diff --git a/pkg/health/condition/check_all_members_test.go b/pkg/health/condition/check_all_members_test.go index d62967558..9f0473da5 100644 --- a/pkg/health/condition/check_all_members_test.go +++ b/pkg/health/condition/check_all_members_test.go @@ -22,11 +22,16 @@ import ( druidv1alpha1 "github.com/gardener/etcd-druid/api/v1alpha1" . "github.com/gardener/etcd-druid/pkg/health/condition" + "github.com/go-logr/logr" ) var _ = Describe("AllMembersReadyCheck", func() { Describe("#Check", func() { - var readyMember, notReadyMember druidv1alpha1.EtcdMemberStatus + var ( + readyMember, notReadyMember druidv1alpha1.EtcdMemberStatus + + logger = logr.Discard() + ) BeforeEach(func() { readyMember = druidv1alpha1.EtcdMemberStatus{ @@ -53,7 +58,7 @@ var _ = Describe("AllMembersReadyCheck", func() { } check := AllMembersCheck(nil) - result := check.Check(context.TODO(), etcd) + result := check.Check(context.TODO(), logger, etcd) Expect(result.ConditionType()).To(Equal(druidv1alpha1.ConditionTypeAllMembersReady)) Expect(result.Status()).To(Equal(druidv1alpha1.ConditionTrue)) @@ -74,7 +79,7 @@ var _ = Describe("AllMembersReadyCheck", func() { } check := AllMembersCheck(nil) - result := check.Check(context.TODO(), etcd) + result := check.Check(context.TODO(), logger, etcd) Expect(result.ConditionType()).To(Equal(druidv1alpha1.ConditionTypeAllMembersReady)) Expect(result.Status()).To(Equal(druidv1alpha1.ConditionFalse)) @@ -94,7 +99,7 @@ var _ = Describe("AllMembersReadyCheck", func() { } check := AllMembersCheck(nil) - result := check.Check(context.TODO(), etcd) + result := check.Check(context.TODO(), logger, etcd) Expect(result.ConditionType()).To(Equal(druidv1alpha1.ConditionTypeAllMembersReady)) Expect(result.Status()).To(Equal(druidv1alpha1.ConditionFalse)) @@ -114,7 +119,7 @@ var _ = Describe("AllMembersReadyCheck", func() { } check := AllMembersCheck(nil) - result := check.Check(context.TODO(), etcd) + result := check.Check(context.TODO(), logger, etcd) Expect(result.ConditionType()).To(Equal(druidv1alpha1.ConditionTypeAllMembersReady)) Expect(result.Status()).To(Equal(druidv1alpha1.ConditionUnknown)) diff --git a/pkg/health/condition/check_backup_ready.go b/pkg/health/condition/check_backup_ready.go index 077adaea6..2576716ca 100644 --- a/pkg/health/condition/check_backup_ready.go +++ b/pkg/health/condition/check_backup_ready.go @@ -20,8 +20,10 @@ import ( "time" druidv1alpha1 "github.com/gardener/etcd-druid/api/v1alpha1" - coordinationv1 "k8s.io/api/coordination/v1" + "github.com/gardener/etcd-druid/pkg/utils" + "github.com/go-logr/logr" + coordinationv1 "k8s.io/api/coordination/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -37,11 +39,11 @@ const ( BackupFailed string = "BackupFailed" // Unknown is a constant that means that the etcd backup status is currently not known Unknown string = "Unknown" - // ConditionNotChecked is a constant that means that the etcd backup status has not been updated or rechecked - ConditionNotChecked string = "ConditionNotChecked" + // NotChecked is a constant that means that the etcd backup status has not been updated or rechecked + NotChecked string = "NotChecked" ) -func (a *backupReadyCheck) Check(ctx context.Context, etcd druidv1alpha1.Etcd) Result { +func (a *backupReadyCheck) Check(ctx context.Context, logger logr.Logger, etcd druidv1alpha1.Etcd) Result { //Default case result := &result{ conType: druidv1alpha1.ConditionTypeBackupReady, @@ -56,17 +58,18 @@ func (a *backupReadyCheck) Check(ctx context.Context, etcd druidv1alpha1.Etcd) R return nil } - //Fetch snapshot leases + // Fetch snapshot leases var ( - fullSnapErr, incrSnapErr error - fullSnapLease = &coordinationv1.Lease{} - deltaSnapLease = &coordinationv1.Lease{} + err, fullSnapErr, deltaSnapErr error + fullSnapshotInterval = 1 * time.Hour + fullSnapLease = &coordinationv1.Lease{} + deltaSnapLease = &coordinationv1.Lease{} ) - fullSnapErr = a.cl.Get(ctx, types.NamespacedName{Name: getFullSnapLeaseName(&etcd), Namespace: etcd.ObjectMeta.Namespace}, fullSnapLease) - incrSnapErr = a.cl.Get(ctx, types.NamespacedName{Name: getDeltaSnapLeaseName(&etcd), Namespace: etcd.ObjectMeta.Namespace}, deltaSnapLease) + fullSnapErr = a.cl.Get(ctx, types.NamespacedName{Name: getFullSnapLeaseName(&etcd), Namespace: etcd.Namespace}, fullSnapLease) + deltaSnapErr = a.cl.Get(ctx, types.NamespacedName{Name: getDeltaSnapLeaseName(&etcd), Namespace: etcd.Namespace}, deltaSnapLease) - //Set status to Unknown if errors in fetching snapshot leases or lease never renewed - if fullSnapErr != nil || incrSnapErr != nil || (fullSnapLease.Spec.RenewTime == nil && deltaSnapLease.Spec.RenewTime == nil) { + // Set status to Unknown if errors in fetching snapshot leases or lease never renewed + if fullSnapErr != nil || deltaSnapErr != nil || (fullSnapLease.Spec.RenewTime == nil && deltaSnapLease.Spec.RenewTime == nil) { return result } @@ -74,25 +77,35 @@ func (a *backupReadyCheck) Check(ctx context.Context, etcd druidv1alpha1.Etcd) R fullLeaseRenewTime := fullSnapLease.Spec.RenewTime fullLeaseCreateTime := &fullSnapLease.ObjectMeta.CreationTimestamp + // TODO: make etcd.Spec.Backup.FullSnapshotSchedule non-optional, since it is mandatory to + // set the full snapshot schedule, or introduce defaulting webhook to add default value for this field + if etcd.Spec.Backup.FullSnapshotSchedule != nil { + if fullSnapshotInterval, err = utils.ComputeScheduleInterval(*etcd.Spec.Backup.FullSnapshotSchedule); err != nil { + logger.Error(err, "unable to compute full snapshot duration from full snapshot schedule", "fullSnapshotSchedule", *etcd.Spec.Backup.FullSnapshotSchedule) + return result + } + } + if fullLeaseRenewTime == nil && deltaLeaseRenewTime != nil { // Most probable during reconcile of existing clusters if fresh leases are created - // Treat backup as succeeded if delta snap lease renewal happens in the required time window and full snap lease is not older than 24h. - if time.Since(deltaLeaseRenewTime.Time) < 2*etcd.Spec.Backup.DeltaSnapshotPeriod.Duration && time.Since(fullLeaseCreateTime.Time) < 24*time.Hour { + // Treat backup as succeeded if delta snap lease renewal happens in the required time window + // and full snap lease is not older than full snapshot duration. + if time.Since(deltaLeaseRenewTime.Time) < 2*etcd.Spec.Backup.DeltaSnapshotPeriod.Duration && time.Since(fullLeaseCreateTime.Time) < fullSnapshotInterval { result.reason = BackupSucceeded result.message = "Delta snapshot backup succeeded" result.status = druidv1alpha1.ConditionTrue return result } } else if deltaLeaseRenewTime == nil && fullLeaseRenewTime != nil { - //Most probable during a startup scenario for new clusters - //Special case. Return Unknown condition for some time to allow delta backups to start up + // Most probable during a startup scenario for new clusters + // Special case. Return Unknown condition for some time to allow delta backups to start up if time.Since(fullLeaseRenewTime.Time) > 5*etcd.Spec.Backup.DeltaSnapshotPeriod.Duration { result.message = "Periodic delta snapshots not started yet" return result } } else if deltaLeaseRenewTime != nil && fullLeaseRenewTime != nil { - //Both snap leases are maintained. Both are expected to be renewed periodically - if time.Since(deltaLeaseRenewTime.Time) < 2*etcd.Spec.Backup.DeltaSnapshotPeriod.Duration && time.Since(fullLeaseRenewTime.Time) < 24*time.Hour { + // Both snap leases are maintained. Both are expected to be renewed periodically + if time.Since(deltaLeaseRenewTime.Time) < 2*etcd.Spec.Backup.DeltaSnapshotPeriod.Duration && time.Since(fullLeaseRenewTime.Time) < fullSnapshotInterval { result.reason = BackupSucceeded result.message = "Snapshot backup succeeded" result.status = druidv1alpha1.ConditionTrue @@ -100,8 +113,8 @@ func (a *backupReadyCheck) Check(ctx context.Context, etcd druidv1alpha1.Etcd) R } } - //Cases where snapshot leases are not updated for a long time - //If snapshot leases are present and leases aren't updated, it is safe to assume that backup is not healthy + // Cases where snapshot leases are not updated for a long time + // If snapshot leases are present and leases aren't updated, it is safe to assume that backup is not healthy if etcd.Status.Conditions != nil { var prevBackupReadyStatus druidv1alpha1.Condition @@ -122,16 +135,16 @@ func (a *backupReadyCheck) Check(ctx context.Context, etcd druidv1alpha1.Etcd) R } } - //Transition to "Unknown" state is we cannot prove a "True" state + // Transition to "Unknown" state is we cannot prove a "True" state return result } func getDeltaSnapLeaseName(etcd *druidv1alpha1.Etcd) string { - return fmt.Sprintf("%s-delta-snap", string(etcd.ObjectMeta.Name)) + return fmt.Sprintf("%s-delta-snap", etcd.Name) } func getFullSnapLeaseName(etcd *druidv1alpha1.Etcd) string { - return fmt.Sprintf("%s-full-snap", string(etcd.ObjectMeta.Name)) + return fmt.Sprintf("%s-full-snap", etcd.Name) } // BackupReadyCheck returns a check for the "BackupReady" condition. diff --git a/pkg/health/condition/check_backup_ready_test.go b/pkg/health/condition/check_backup_ready_test.go index 5e55c7224..a1f35f271 100644 --- a/pkg/health/condition/check_backup_ready_test.go +++ b/pkg/health/condition/check_backup_ready_test.go @@ -18,20 +18,21 @@ import ( "context" "time" - druidv1alpha1 "github.com/gardener/etcd-druid/api/v1alpha1" - . "github.com/gardener/etcd-druid/pkg/health/condition" - mockclient "github.com/gardener/etcd-druid/pkg/mock/controller-runtime/client" - "github.com/golang/mock/gomock" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - v1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" - "sigs.k8s.io/controller-runtime/pkg/client" + druidv1alpha1 "github.com/gardener/etcd-druid/api/v1alpha1" + . "github.com/gardener/etcd-druid/pkg/health/condition" + mockclient "github.com/gardener/etcd-druid/pkg/mock/controller-runtime/client" coordinationv1 "k8s.io/api/coordination/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/utils/pointer" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" ) var _ = Describe("BackupReadyCheck", func() { @@ -47,6 +48,7 @@ var _ = Describe("BackupReadyCheck", func() { }, } deltaSnapshotDuration = 2 * time.Minute + logger = ctrl.Log.WithName("backup-ready-checker") etcd = druidv1alpha1.Etcd{ ObjectMeta: v1.ObjectMeta{ @@ -56,6 +58,7 @@ var _ = Describe("BackupReadyCheck", func() { Spec: druidv1alpha1.EtcdSpec{ Replicas: 1, Backup: druidv1alpha1.BackupSpec{ + FullSnapshotSchedule: pointer.String("0 0 * * *"), // at 00:00 every day DeltaSnapshotPeriod: &v1.Duration{ Duration: deltaSnapshotDuration, }, @@ -90,7 +93,7 @@ var _ = Describe("BackupReadyCheck", func() { }) Context("With no snapshot leases present", func() { - It("Should return Unknown rediness", func() { + It("Should return Unknown readiness", func() { cl.EXPECT().Get(context.TODO(), gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( func(_ context.Context, _ client.ObjectKey, er *coordinationv1.Lease, _ ...client.GetOption) error { return &noLeaseError @@ -98,7 +101,7 @@ var _ = Describe("BackupReadyCheck", func() { ).AnyTimes() check := BackupReadyCheck(cl) - result := check.Check(context.TODO(), etcd) + result := check.Check(context.TODO(), logger, etcd) Expect(result).ToNot(BeNil()) Expect(result.ConditionType()).To(Equal(druidv1alpha1.ConditionTypeBackupReady)) @@ -117,7 +120,7 @@ var _ = Describe("BackupReadyCheck", func() { ).AnyTimes() check := BackupReadyCheck(cl) - result := check.Check(context.TODO(), etcd) + result := check.Check(context.TODO(), logger, etcd) Expect(result).ToNot(BeNil()) Expect(result.ConditionType()).To(Equal(druidv1alpha1.ConditionTypeBackupReady)) @@ -125,7 +128,7 @@ var _ = Describe("BackupReadyCheck", func() { Expect(result.Reason()).To(Equal(BackupSucceeded)) }) - It("Should set status to BackupSucceeded if delta snap lease is recently created and empty full snap lease has been created in the last 24h", func() { + It("Should set status to BackupSucceeded if delta snap lease is recently created and empty full snap lease has been created recently", func() { cl.EXPECT().Get(context.TODO(), types.NamespacedName{Name: "test-etcd-full-snap", Namespace: "default"}, gomock.Any(), gomock.Any()).DoAndReturn( func(_ context.Context, _ client.ObjectKey, le *coordinationv1.Lease, _ ...client.GetOption) error { *le = lease @@ -143,7 +146,7 @@ var _ = Describe("BackupReadyCheck", func() { ).AnyTimes() check := BackupReadyCheck(cl) - result := check.Check(context.TODO(), etcd) + result := check.Check(context.TODO(), logger, etcd) Expect(result).ToNot(BeNil()) Expect(result.ConditionType()).To(Equal(druidv1alpha1.ConditionTypeBackupReady)) @@ -169,7 +172,7 @@ var _ = Describe("BackupReadyCheck", func() { ).AnyTimes() check := BackupReadyCheck(cl) - result := check.Check(context.TODO(), etcd) + result := check.Check(context.TODO(), logger, etcd) Expect(result).ToNot(BeNil()) Expect(result.ConditionType()).To(Equal(druidv1alpha1.ConditionTypeBackupReady)) @@ -198,7 +201,7 @@ var _ = Describe("BackupReadyCheck", func() { } check := BackupReadyCheck(cl) - result := check.Check(context.TODO(), etcd) + result := check.Check(context.TODO(), logger, etcd) Expect(result).ToNot(BeNil()) Expect(result.ConditionType()).To(Equal(druidv1alpha1.ConditionTypeBackupReady)) @@ -226,7 +229,7 @@ var _ = Describe("BackupReadyCheck", func() { } check := BackupReadyCheck(cl) - result := check.Check(context.TODO(), etcd) + result := check.Check(context.TODO(), logger, etcd) Expect(result).ToNot(BeNil()) Expect(result.ConditionType()).To(Equal(druidv1alpha1.ConditionTypeBackupReady)) @@ -245,7 +248,7 @@ var _ = Describe("BackupReadyCheck", func() { etcd.Spec.Backup.Store = nil check := BackupReadyCheck(cl) - result := check.Check(context.TODO(), etcd) + result := check.Check(context.TODO(), logger, etcd) Expect(result).To(BeNil()) etcd.Spec.Backup.Store = &druidv1alpha1.StoreSpec{ @@ -265,7 +268,7 @@ var _ = Describe("BackupReadyCheck", func() { etcd.Spec.Backup.Store.Provider = nil check := BackupReadyCheck(cl) - result := check.Check(context.TODO(), etcd) + result := check.Check(context.TODO(), logger, etcd) Expect(result).To(BeNil()) etcd.Spec.Backup.Store.Provider = &storageProvider diff --git a/pkg/health/condition/check_ready.go b/pkg/health/condition/check_ready.go index e0ef3d1b2..6e5674456 100644 --- a/pkg/health/condition/check_ready.go +++ b/pkg/health/condition/check_ready.go @@ -18,12 +18,13 @@ import ( "context" druidv1alpha1 "github.com/gardener/etcd-druid/api/v1alpha1" + "github.com/go-logr/logr" "sigs.k8s.io/controller-runtime/pkg/client" ) type readyCheck struct{} -func (r *readyCheck) Check(_ context.Context, etcd druidv1alpha1.Etcd) Result { +func (r *readyCheck) Check(_ context.Context, _ logr.Logger, etcd druidv1alpha1.Etcd) Result { // TODO: remove this case as soon as leases are completely supported by etcd-backup-restore if len(etcd.Status.Members) == 0 { diff --git a/pkg/health/condition/check_ready_test.go b/pkg/health/condition/check_ready_test.go index 03fb65d32..827bfda73 100644 --- a/pkg/health/condition/check_ready_test.go +++ b/pkg/health/condition/check_ready_test.go @@ -17,15 +17,21 @@ package condition_test import ( "context" - druidv1alpha1 "github.com/gardener/etcd-druid/api/v1alpha1" - . "github.com/gardener/etcd-druid/pkg/health/condition" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + + druidv1alpha1 "github.com/gardener/etcd-druid/api/v1alpha1" + . "github.com/gardener/etcd-druid/pkg/health/condition" + "github.com/go-logr/logr" ) var _ = Describe("ReadyCheck", func() { Describe("#Check", func() { - var readyMember, notReadyMember, unknownMember druidv1alpha1.EtcdMemberStatus + var ( + readyMember, notReadyMember, unknownMember druidv1alpha1.EtcdMemberStatus + + logger = logr.Discard() + ) BeforeEach(func() { readyMember = druidv1alpha1.EtcdMemberStatus{ @@ -52,7 +58,7 @@ var _ = Describe("ReadyCheck", func() { } check := ReadyCheck(nil) - result := check.Check(context.TODO(), etcd) + result := check.Check(context.TODO(), logger, etcd) Expect(result.ConditionType()).To(Equal(druidv1alpha1.ConditionTypeReady)) Expect(result.Status()).To(Equal(druidv1alpha1.ConditionTrue)) @@ -70,7 +76,7 @@ var _ = Describe("ReadyCheck", func() { } check := ReadyCheck(nil) - result := check.Check(context.TODO(), etcd) + result := check.Check(context.TODO(), logger, etcd) Expect(result.ConditionType()).To(Equal(druidv1alpha1.ConditionTypeReady)) Expect(result.Status()).To(Equal(druidv1alpha1.ConditionTrue)) @@ -88,7 +94,7 @@ var _ = Describe("ReadyCheck", func() { } check := ReadyCheck(nil) - result := check.Check(context.TODO(), etcd) + result := check.Check(context.TODO(), logger, etcd) Expect(result.ConditionType()).To(Equal(druidv1alpha1.ConditionTypeReady)) Expect(result.Status()).To(Equal(druidv1alpha1.ConditionTrue)) @@ -106,7 +112,7 @@ var _ = Describe("ReadyCheck", func() { } check := ReadyCheck(nil) - result := check.Check(context.TODO(), etcd) + result := check.Check(context.TODO(), logger, etcd) Expect(result.ConditionType()).To(Equal(druidv1alpha1.ConditionTypeReady)) Expect(result.Status()).To(Equal(druidv1alpha1.ConditionFalse)) @@ -123,7 +129,7 @@ var _ = Describe("ReadyCheck", func() { } check := ReadyCheck(nil) - result := check.Check(context.TODO(), etcd) + result := check.Check(context.TODO(), logger, etcd) Expect(result.ConditionType()).To(Equal(druidv1alpha1.ConditionTypeReady)) Expect(result.Status()).To(Equal(druidv1alpha1.ConditionUnknown)) @@ -131,5 +137,4 @@ var _ = Describe("ReadyCheck", func() { }) }) }) - }) diff --git a/pkg/health/condition/types.go b/pkg/health/condition/types.go index 98c3a3b7a..d7832d85a 100644 --- a/pkg/health/condition/types.go +++ b/pkg/health/condition/types.go @@ -18,11 +18,12 @@ import ( "context" druidv1alpha1 "github.com/gardener/etcd-druid/api/v1alpha1" + "github.com/go-logr/logr" ) // Checker is an interface to check the etcd resource and to return condition results. type Checker interface { - Check(ctx context.Context, etcd druidv1alpha1.Etcd) Result + Check(ctx context.Context, logger logr.Logger, etcd druidv1alpha1.Etcd) Result } // Result encapsulates a condition result diff --git a/pkg/health/status/check.go b/pkg/health/status/check.go index e742f99c1..b4b3aa362 100644 --- a/pkg/health/status/check.go +++ b/pkg/health/status/check.go @@ -74,24 +74,24 @@ func (c *Checker) Check(ctx context.Context, logger logr.Logger, etcd *druidv1al } // Execute condition checks after the etcd member checks because we need their result here. - return c.executeConditionChecks(ctx, etcd) + return c.executeConditionChecks(ctx, logger, etcd) } // executeConditionChecks runs all registered condition checks **in parallel**. -func (c *Checker) executeConditionChecks(ctx context.Context, etcd *druidv1alpha1.Etcd) error { +func (c *Checker) executeConditionChecks(ctx context.Context, logger logr.Logger, etcd *druidv1alpha1.Etcd) error { var ( resultCh = make(chan condition.Result) wg sync.WaitGroup ) - // Run condition checks in parallel since they work independently from each other. + // Run condition checks in parallel since they work independent of each other. for _, newCheck := range c.conditionCheckFns { c := newCheck(c.cl) wg.Add(1) go (func() { defer wg.Done() - resultCh <- c.Check(ctx, *etcd) + resultCh <- c.Check(ctx, logger, *etcd) })() } diff --git a/pkg/health/status/check_test.go b/pkg/health/status/check_test.go index f7dee7f65..abd411b10 100644 --- a/pkg/health/status/check_test.go +++ b/pkg/health/status/check_test.go @@ -18,22 +18,21 @@ import ( "context" "time" - "k8s.io/utils/pointer" - - "github.com/go-logr/logr" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + . "github.com/onsi/gomega/gstruct" + druidv1alpha1 "github.com/gardener/etcd-druid/api/v1alpha1" "github.com/gardener/etcd-druid/pkg/health/condition" "github.com/gardener/etcd-druid/pkg/health/etcdmember" + . "github.com/gardener/etcd-druid/pkg/health/status" + "github.com/gardener/gardener/pkg/utils/test" - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - . "github.com/onsi/gomega/gstruct" + "github.com/go-logr/logr" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/pointer" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/log" - - druidv1alpha1 "github.com/gardener/etcd-druid/api/v1alpha1" - . "github.com/gardener/etcd-druid/pkg/health/status" ) var _ = Describe("Check", func() { @@ -214,16 +213,16 @@ func (r *conditionResult) Message() string { return r.ConMessage } -type testChecker struct { +type conditionTestChecker struct { result *conditionResult } -func (t *testChecker) Check(_ context.Context, _ druidv1alpha1.Etcd) condition.Result { +func (t *conditionTestChecker) Check(_ context.Context, _ logr.Logger, _ druidv1alpha1.Etcd) condition.Result { return t.result } func createConditionCheck(conType druidv1alpha1.ConditionType, status druidv1alpha1.ConditionStatus, reason, message string) condition.Checker { - return &testChecker{ + return &conditionTestChecker{ result: &conditionResult{ ConType: conType, ConStatus: status, diff --git a/pkg/utils/miscellaneous.go b/pkg/utils/miscellaneous.go index cbacbe39e..0cf71458d 100644 --- a/pkg/utils/miscellaneous.go +++ b/pkg/utils/miscellaneous.go @@ -16,7 +16,9 @@ package utils import ( "fmt" + "time" + "github.com/robfig/cron/v3" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -77,3 +79,20 @@ func Max(x, y int) int { } return x } + +// ComputeScheduleInterval computes the interval between two activations for the given cron schedule. +// Assumes that every cron activation is at equal intervals apart, based on cron schedules such as +// "once every X hours", "once every Y days", "at 1:00pm on every Tuesday", etc. +// TODO: write a new function to accurately compute the previous activation time from the cron schedule +// in order to compute when the previous activation of the cron schedule was supposed to have occurred, +// instead of relying on the assumption that all the cron activations are evenly spaced. +func ComputeScheduleInterval(cronSchedule string) (time.Duration, error) { + schedule, err := cron.ParseStandard(cronSchedule) + if err != nil { + return 0, err + } + + nextScheduledTime := schedule.Next(time.Now()) + nextNextScheduledTime := schedule.Next(nextScheduledTime) + return nextNextScheduledTime.Sub(nextScheduledTime), nil +} diff --git a/pkg/utils/miscellaneous_test.go b/pkg/utils/miscellaneous_test.go new file mode 100644 index 000000000..874aa5859 --- /dev/null +++ b/pkg/utils/miscellaneous_test.go @@ -0,0 +1,54 @@ +// Copyright 2023 SAP SE or an SAP affiliate company +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package utils + +import ( + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("Tests for miscellaneous utility functions", func() { + Describe("#ComputeScheduleInterval", func() { + It("should compute schedule duration as 24h when schedule set to 00:00 everyday", func() { + schedule := "0 0 * * *" + duration, err := ComputeScheduleInterval(schedule) + Expect(err).To(Not(HaveOccurred())) + Expect(duration).To(Equal(24 * time.Hour)) + }) + + It("should compute schedule duration as 24h when schedule set to 15:30 everyday", func() { + schedule := "30 15 * * *" + duration, err := ComputeScheduleInterval(schedule) + Expect(err).To(Not(HaveOccurred())) + Expect(duration).To(Equal(24 * time.Hour)) + }) + + It("should compute schedule duration as 12h when schedule set to 03:30 and 15:30 everyday", func() { + schedule := "30 3,15 * * *" + duration, err := ComputeScheduleInterval(schedule) + Expect(err).To(Not(HaveOccurred())) + Expect(duration).To(Equal(12 * time.Hour)) + }) + + It("should compute schedule duration as 7d when schedule set to 15:30 on every Tuesday", func() { + schedule := "30 15 * * 2" + duration, err := ComputeScheduleInterval(schedule) + Expect(err).To(Not(HaveOccurred())) + Expect(duration).To(Equal(7 * 24 * time.Hour)) + }) + }) +})