diff --git a/pkg/app/reconcile_timer.go b/pkg/app/reconcile_timer.go index c6d925179..c29e7209c 100644 --- a/pkg/app/reconcile_timer.go +++ b/pkg/app/reconcile_timer.go @@ -4,10 +4,12 @@ package app import ( + "fmt" "math" "time" "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apis/kappctrl/v1alpha1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/wait" ) @@ -39,19 +41,9 @@ func (rt ReconcileTimer) IsReadyAt(timeAt time.Time) bool { return false } - var lastReconcileTime time.Time - - // Use latest deploy time if available, otherwise fallback to fetch - // If no timestamp is available, enqueue immediately - lastDeploy := rt.app.Status.Deploy - lastFetch := rt.app.Status.Fetch - if lastDeploy != nil && !lastDeploy.UpdatedAt.Time.IsZero() { - lastReconcileTime = rt.app.Status.Deploy.UpdatedAt.Time - } else { - if lastFetch == nil { - return true - } - lastReconcileTime = lastFetch.UpdatedAt.Time + lastReconcileTime, err := rt.lastReconcileTime() + if err != nil { + return true } if rt.hasReconcileStatus(v1alpha1.ReconcileFailed) { @@ -104,3 +96,33 @@ func (rt ReconcileTimer) applyJitter(t time.Duration) time.Duration { const appJitter time.Duration = 5 * time.Second return t - appJitter + wait.Jitter(appJitter, 1.0) } + +func (rt ReconcileTimer) lastReconcileTime() (time.Time, error) { + // Determine latest time from status and use that as the + // last reconcile time + lastReconcileTime := metav1.Time{} + times := []metav1.Time{} + if rt.app.Status.Fetch != nil { + times = append(times, rt.app.Status.Fetch.UpdatedAt) + } + + if rt.app.Status.Template != nil { + times = append(times, rt.app.Status.Template.UpdatedAt) + } + + if rt.app.Status.Deploy != nil { + times = append(times, rt.app.Status.Deploy.UpdatedAt) + } + + for _, time := range times { + if lastReconcileTime.Before(&time) { + lastReconcileTime = time + } + } + + if lastReconcileTime.IsZero() { + return time.Time{}, fmt.Errorf("could not determine time of last reconcile") + } + + return lastReconcileTime.Time, nil +} diff --git a/pkg/app/reconcile_timer_test.go b/pkg/app/reconcile_timer_test.go index eb433f344..d97e952d5 100644 --- a/pkg/app/reconcile_timer_test.go +++ b/pkg/app/reconcile_timer_test.go @@ -7,6 +7,7 @@ import ( "testing" "time" + "github.com/stretchr/testify/require" "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apis/kappctrl/v1alpha1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -193,3 +194,31 @@ func TestFailedIsReadyAt(t *testing.T) { t.Fatalf("Expected app to not be ready under syncPeriod of 2s") } } + +func TestIsReadyAtWithStaleDeployTime(t *testing.T) { + syncPeriod := 2 * time.Second + timeNow := time.Now() + timeOfReady := timeNow.Add(syncPeriod) + + app := v1alpha1.App{ + Spec: v1alpha1.AppSpec{ + SyncPeriod: &metav1.Duration{Duration: syncPeriod}, + }, + Status: v1alpha1.AppStatus{ + Fetch: &v1alpha1.AppStatusFetch{ + UpdatedAt: metav1.Time{Time: timeOfReady}, + Error: "I've failed you", + }, + Deploy: &v1alpha1.AppStatusDeploy{ + UpdatedAt: metav1.Time{Time: timeNow}, + }, + ConsecutiveReconcileFailures: 1, + GenericStatus: v1alpha1.GenericStatus{ + Conditions: []v1alpha1.AppCondition{{Type: v1alpha1.ReconcileFailed}}, + }, + }, + } + + isReady := NewReconcileTimer(app).IsReadyAt(timeOfReady.Add(1 * time.Second)) + require.False(t, isReady, "Expected app not to be ready, because deploy time is stale") +} diff --git a/pkg/pkgrepository/reconcile_timer.go b/pkg/pkgrepository/reconcile_timer.go index 7d5f382cd..8990f2b67 100644 --- a/pkg/pkgrepository/reconcile_timer.go +++ b/pkg/pkgrepository/reconcile_timer.go @@ -4,10 +4,12 @@ package pkgrepository import ( + "fmt" "math" "time" "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apis/kappctrl/v1alpha1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/wait" ) @@ -39,19 +41,9 @@ func (rt ReconcileTimer) IsReadyAt(timeAt time.Time) bool { return false } - var lastReconcileTime time.Time - - // Use latest deploy time if available, otherwise fallback to fetch - // If no timestamp is available, enqueue immediately - lastDeploy := rt.app.Status.Deploy - lastFetch := rt.app.Status.Fetch - if lastDeploy != nil && !lastDeploy.UpdatedAt.Time.IsZero() { - lastReconcileTime = rt.app.Status.Deploy.UpdatedAt.Time - } else { - if lastFetch == nil { - return true - } - lastReconcileTime = lastFetch.UpdatedAt.Time + lastReconcileTime, err := rt.lastReconcileTime() + if err != nil { + return true } if rt.hasReconcileStatus(v1alpha1.ReconcileFailed) { @@ -104,3 +96,33 @@ func (rt ReconcileTimer) applyJitter(t time.Duration) time.Duration { const appJitter time.Duration = 5 * time.Second return t - appJitter + wait.Jitter(appJitter, 1.0) } + +func (rt ReconcileTimer) lastReconcileTime() (time.Time, error) { + // Determine latest time from status and use that as the + // last reconcile time + lastReconcileTime := metav1.Time{} + times := []metav1.Time{} + if rt.app.Status.Fetch != nil { + times = append(times, rt.app.Status.Fetch.UpdatedAt) + } + + if rt.app.Status.Template != nil { + times = append(times, rt.app.Status.Template.UpdatedAt) + } + + if rt.app.Status.Deploy != nil { + times = append(times, rt.app.Status.Deploy.UpdatedAt) + } + + for _, time := range times { + if lastReconcileTime.Before(&time) { + lastReconcileTime = time + } + } + + if lastReconcileTime.IsZero() { + return time.Time{}, fmt.Errorf("could not determine time of last reconcile") + } + + return lastReconcileTime.Time, nil +} diff --git a/pkg/pkgrepository/reconcile_timer_test.go b/pkg/pkgrepository/reconcile_timer_test.go index 88b42075b..0d32ed4e1 100644 --- a/pkg/pkgrepository/reconcile_timer_test.go +++ b/pkg/pkgrepository/reconcile_timer_test.go @@ -7,6 +7,7 @@ import ( "testing" "time" + "github.com/stretchr/testify/require" "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apis/kappctrl/v1alpha1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -193,3 +194,31 @@ func TestFailedIsReadyAt(t *testing.T) { t.Fatalf("Expected app to not be ready under syncPeriod of 2s") } } + +func TestIsReadyAtWithStaleDeployTime(t *testing.T) { + syncPeriod := 2 * time.Second + timeNow := time.Now() + timeOfReady := timeNow.Add(syncPeriod) + + app := v1alpha1.App{ + Spec: v1alpha1.AppSpec{ + SyncPeriod: &metav1.Duration{Duration: syncPeriod}, + }, + Status: v1alpha1.AppStatus{ + Fetch: &v1alpha1.AppStatusFetch{ + UpdatedAt: metav1.Time{Time: timeOfReady}, + Error: "I've failed you", + }, + Deploy: &v1alpha1.AppStatusDeploy{ + UpdatedAt: metav1.Time{Time: timeNow}, + }, + ConsecutiveReconcileFailures: 1, + GenericStatus: v1alpha1.GenericStatus{ + Conditions: []v1alpha1.AppCondition{{Type: v1alpha1.ReconcileFailed}}, + }, + }, + } + + isReady := NewReconcileTimer(app).IsReadyAt(timeOfReady.Add(1 * time.Second)) + require.False(t, isReady, "Expected app not to be ready, because deploy time is stale") +}