Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[release-4.5] Bug 1928263: Fix zero-delay resyncs for certain registry update policies. #2006

Merged
merged 1 commit into from
Feb 18, 2021
Merged

[release-4.5] Bug 1928263: Fix zero-delay resyncs for certain registry update policies. #2006

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion pkg/controller/operators/catalog/operator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -628,7 +628,7 @@ func (o *Operator) syncRegistryServer(logger *logrus.Entry, in *v1alpha1.Catalog
// requeue the catalog sync based on the polling interval, for accurate syncs of catalogs with polling enabled
if out.Spec.UpdateStrategy != nil {
logger.Debugf("requeuing registry server sync based on polling interval %s", out.Spec.UpdateStrategy.Interval.Duration.String())
resyncPeriod := reconciler.SyncRegistryUpdateInterval(out)
resyncPeriod := reconciler.SyncRegistryUpdateInterval(out, time.Now())
o.catsrcQueueSet.RequeueAfter(out.GetNamespace(), out.GetName(), queueinformer.ResyncWithJitter(resyncPeriod, 0.1)())
return
}
Expand Down
16 changes: 9 additions & 7 deletions pkg/controller/registry/reconciler/grpc_polling.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
// SyncRegistryUpdateInterval returns a duration to use when requeuing the catalog source for reconciliation.
// This ensures that the catalog is being synced on the correct time interval based on its spec.
// Note: this function assumes the catalog has an update strategy set.
func SyncRegistryUpdateInterval(source *v1alpha1.CatalogSource) time.Duration {
func SyncRegistryUpdateInterval(source *v1alpha1.CatalogSource, now time.Time) time.Duration {
pollingInterval := source.Spec.UpdateStrategy.Interval.Duration
latestPoll := source.Status.LatestImageRegistryPoll
creationTimestamp := source.CreationTimestamp.Time
Expand All @@ -22,19 +22,21 @@ func SyncRegistryUpdateInterval(source *v1alpha1.CatalogSource) time.Duration {
return pollingInterval
}
// Resync based on the delta between the default sync and the actual last poll if the interval is greater than the default
return defaultOr(latestPoll, pollingInterval, creationTimestamp)
return defaultOr(latestPoll, pollingInterval, creationTimestamp, now)
}

// defaultOr returns either the default resync period or the modulus of the polling interval and the default.
// defaultOr returns either the default resync period or the time remaining until the next poll is due, whichever is smaller.
// For example, if the polling interval is 40 minutes, OLM will sync after ~30 minutes and see that the next sync
// for this catalog should be sooner than the default (15 minutes) and return 10 minutes (40 % 15).
func defaultOr(latestPoll *metav1.Time, pollingInterval time.Duration, creationTimestamp time.Time) time.Duration {
// for this catalog should be in 10 minutes, sooner than the default 15 minutes, and return 10 minutes.
func defaultOr(latestPoll *metav1.Time, pollingInterval time.Duration, creationTimestamp time.Time, now time.Time) time.Duration {
if latestPoll.IsZero() {
latestPoll = &metav1.Time{Time: creationTimestamp}
}

remaining := latestPoll.Add(pollingInterval).Sub(now)
// sync ahead of the default interval in the case where now + default sync is after the last sync plus the interval
if time.Now().Add(queueinformer.DefaultResyncPeriod).After(latestPoll.Add(pollingInterval)) {
return (pollingInterval % queueinformer.DefaultResyncPeriod).Round(time.Minute)
if remaining < queueinformer.DefaultResyncPeriod {
return remaining
}
// return the default sync period otherwise: the next sync cycle will check again
return queueinformer.DefaultResyncPeriod
Expand Down
44 changes: 33 additions & 11 deletions pkg/controller/registry/reconciler/grpc_polling_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
)

func TestSyncRegistryUpdateInterval(t *testing.T) {
now := time.Date(2021, time.January, 29, 14, 47, 0, 0, time.UTC)
tests := []struct {
name string
source *v1alpha1.CatalogSource
Expand Down Expand Up @@ -46,12 +47,32 @@ func TestSyncRegistryUpdateInterval(t *testing.T) {
},
expected: 15 * time.Minute,
},
{
name: "PollingIntervalMultipleOfDefaultResyncPeriod",
source: &v1alpha1.CatalogSource{
Spec: v1alpha1.CatalogSourceSpec{
UpdateStrategy: &v1alpha1.UpdateStrategy{
RegistryPoll: &v1alpha1.RegistryPoll{
Interval: &metav1.Duration{
Duration: 2 * queueinformer.DefaultResyncPeriod,
},
},
},
},
Status: v1alpha1.CatalogSourceStatus{
LatestImageRegistryPoll: &metav1.Time{
Time: now.Add(1*time.Second - 2*queueinformer.DefaultResyncPeriod),
},
},
},
expected: 1 * time.Second,
},
{
name: "PollingInterval10Minutes/AlreadyUpdated",
source: &v1alpha1.CatalogSource{
Status: v1alpha1.CatalogSourceStatus{
LatestImageRegistryPoll: &metav1.Time{
time.Now().Add(-(5 * time.Minute)),
Time: now.Add(-(5 * time.Minute)),
},
},
Spec: v1alpha1.CatalogSourceSpec{
Expand All @@ -71,7 +92,7 @@ func TestSyncRegistryUpdateInterval(t *testing.T) {
source: &v1alpha1.CatalogSource{
ObjectMeta: metav1.ObjectMeta{
CreationTimestamp: metav1.Time{
Time: time.Now().Add(-(35 * time.Minute)),
Time: now.Add(-(35 * time.Minute)),
},
},
Spec: v1alpha1.CatalogSourceSpec{
Expand All @@ -84,14 +105,14 @@ func TestSyncRegistryUpdateInterval(t *testing.T) {
},
},
},
expected: 10 * time.Minute,
expected: 5 * time.Minute,
},
{
name: "PollingInterval40Minutes/AlreadyUpdated30MinutesAgo",
source: &v1alpha1.CatalogSource{
Status: v1alpha1.CatalogSourceStatus{
LatestImageRegistryPoll: &metav1.Time{
time.Now().Add(-(30 * time.Minute)),
Time: now.Add(-(30 * time.Minute)),
},
},
Spec: v1alpha1.CatalogSourceSpec{
Expand All @@ -111,7 +132,7 @@ func TestSyncRegistryUpdateInterval(t *testing.T) {
source: &v1alpha1.CatalogSource{
ObjectMeta: metav1.ObjectMeta{
CreationTimestamp: metav1.Time{
Time: time.Now().Add(-(15 * time.Minute)),
Time: now.Add(-(15 * time.Minute)),
},
},
Spec: v1alpha1.CatalogSourceSpec{
Expand All @@ -131,7 +152,7 @@ func TestSyncRegistryUpdateInterval(t *testing.T) {
source: &v1alpha1.CatalogSource{
Status: v1alpha1.CatalogSourceStatus{
LatestImageRegistryPoll: &metav1.Time{
time.Now().Add(-(15 * time.Minute)),
Time: now.Add(-(15 * time.Minute)),
},
},
Spec: v1alpha1.CatalogSourceSpec{
Expand All @@ -149,10 +170,11 @@ func TestSyncRegistryUpdateInterval(t *testing.T) {
}

for _, tt := range tests {
t.Logf("name %s", tt.name)
d := SyncRegistryUpdateInterval(tt.source)
if d != tt.expected {
t.Fatalf("unexpected registry sync interval for %s: expected %#v got %#v", tt.name, tt.expected, d)
}
t.Run(tt.name, func(t *testing.T) {
d := SyncRegistryUpdateInterval(tt.source, now)
if d != tt.expected {
t.Errorf("unexpected registry sync interval for %s: expected %s got %s", tt.name, tt.expected, d)
}
})
}
}