add correct polling interval based on modulus

pkg/controller/operators/catalog/operator.go

@@ -630,12 +630,11 @@ func (o *Operator) syncRegistryServer(logger *logrus.Entry, in *v1alpha1.Catalog
 
 	logger.Debug("ensured registry server")
 
-	// if catalog polling is enabled, and the polling interval is less than the default OLM sync cycle (15 minutes)
-	// requeue the catalog sync based on the polling interval, so the next check comes ahead of the default sync
-	if out.Spec.UpdateStrategy != nil && out.Spec.UpdateStrategy.Interval.Duration < queueinformer.DefaultResyncPeriod {
+	// 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 := queueinformer.ResyncWithJitter(out.Spec.UpdateStrategy.Interval.Duration, 0.1)
-		o.catsrcQueueSet.RequeueAfter(out.GetNamespace(), out.GetName(), resyncPeriod())
+		resyncPeriod := reconciler.SyncRegistryUpdateInterval(out)
+		o.catsrcQueueSet.RequeueAfter(out.GetNamespace(), out.GetName(), queueinformer.ResyncWithJitter(resyncPeriod, 0.1)())
 		return
 	}
 

pkg/controller/registry/reconciler/grpc_polling.go

@@ -0,0 +1,41 @@
+package reconciler
+
+import (
+	"time"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	"github.com/operator-framework/api/pkg/operators/v1alpha1"
+	"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/queueinformer"
+)
+
+// 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 {
+	pollingInterval := source.Spec.UpdateStrategy.Interval.Duration
+	latestPoll := source.Status.LatestImageRegistryPoll
+	creationTimestamp := source.CreationTimestamp.Time
+
+	// Resync before next default sync if the polling interval is less than the default
+	if pollingInterval <= queueinformer.DefaultResyncPeriod {
+		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)
+}
+
+// defaultOr returns either the default resync period or the modulus of the polling interval and the default.
+// 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 {
+	if latestPoll.IsZero() {
+		latestPoll = &metav1.Time{Time: creationTimestamp}
+	}
+	// 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)
+	}
+	// return the default sync period otherwise: the next sync cycle will check again
+	return queueinformer.DefaultResyncPeriod
+}

pkg/controller/registry/reconciler/grpc_polling_test.go

@@ -0,0 +1,158 @@
+package reconciler
+
+import (
+	"testing"
+	"time"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	"github.com/operator-framework/api/pkg/operators/v1alpha1"
+	"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/queueinformer"
+)
+
+func TestSyncRegistryUpdateInterval(t *testing.T) {
+	tests := []struct {
+		name     string
+		source   *v1alpha1.CatalogSource
+		expected time.Duration
+	}{
+		{
+			name: "PollingInterval10Minutes/FirstUpdate",
+			source: &v1alpha1.CatalogSource{
+				Spec: v1alpha1.CatalogSourceSpec{
+					UpdateStrategy: &v1alpha1.UpdateStrategy{
+						RegistryPoll: &v1alpha1.RegistryPoll{
+							Interval: &metav1.Duration{
+								Duration: 10 * time.Minute,
+							},
+						},
+					},
+				},
+			},
+			expected: 10 * time.Minute,
+		},
+		{
+			name: "PollingInterval15Minutes/FirstUpdate",
+			source: &v1alpha1.CatalogSource{
+				Spec: v1alpha1.CatalogSourceSpec{
+					UpdateStrategy: &v1alpha1.UpdateStrategy{
+						RegistryPoll: &v1alpha1.RegistryPoll{
+							Interval: &metav1.Duration{
+								Duration: 15 * time.Minute,
+							},
+						},
+					},
+				},
+			},
+			expected: 15 * time.Minute,
+		},
+		{
+			name: "PollingInterval10Minutes/AlreadyUpdated",
+			source: &v1alpha1.CatalogSource{
+				Status: v1alpha1.CatalogSourceStatus{
+					LatestImageRegistryPoll: &metav1.Time{
+						time.Now().Add(-(5 * time.Minute)),
+					},
+				},
+				Spec: v1alpha1.CatalogSourceSpec{
+					UpdateStrategy: &v1alpha1.UpdateStrategy{
+						RegistryPoll: &v1alpha1.RegistryPoll{
+							Interval: &metav1.Duration{
+								Duration: 10 * time.Minute,
+							},
+						},
+					},
+				},
+			},
+			expected: 10 * time.Minute,
+		},
+		{
+			name: "PollingInterval40Minutes/FirstUpdate",
+			source: &v1alpha1.CatalogSource{
+				ObjectMeta: metav1.ObjectMeta{
+					CreationTimestamp: metav1.Time{
+						Time: time.Now().Add(-(35 * time.Minute)),
+					},
+				},
+				Spec: v1alpha1.CatalogSourceSpec{
+					UpdateStrategy: &v1alpha1.UpdateStrategy{
+						RegistryPoll: &v1alpha1.RegistryPoll{
+							Interval: &metav1.Duration{
+								Duration: 40 * time.Minute,
+							},
+						},
+					},
+				},
+			},
+			expected: 10 * time.Minute,
+		},
+		{
+			name: "PollingInterval40Minutes/AlreadyUpdated30MinutesAgo",
+			source: &v1alpha1.CatalogSource{
+				Status: v1alpha1.CatalogSourceStatus{
+					LatestImageRegistryPoll: &metav1.Time{
+						time.Now().Add(-(30 * time.Minute)),
+					},
+				},
+				Spec: v1alpha1.CatalogSourceSpec{
+					UpdateStrategy: &v1alpha1.UpdateStrategy{
+						RegistryPoll: &v1alpha1.RegistryPoll{
+							Interval: &metav1.Duration{
+								Duration: 40 * time.Minute,
+							},
+						},
+					},
+				},
+			},
+			expected: 10 * time.Minute,
+		},
+		{
+			name: "PollingInterval1hour/FirstUpdate",
+			source: &v1alpha1.CatalogSource{
+				ObjectMeta: metav1.ObjectMeta{
+					CreationTimestamp: metav1.Time{
+						Time: time.Now().Add(-(15 * time.Minute)),
+					},
+				},
+				Spec: v1alpha1.CatalogSourceSpec{
+					UpdateStrategy: &v1alpha1.UpdateStrategy{
+						RegistryPoll: &v1alpha1.RegistryPoll{
+							Interval: &metav1.Duration{
+								Duration: 1 * time.Hour,
+							},
+						},
+					},
+				},
+			},
+			expected: queueinformer.DefaultResyncPeriod,
+		},
+		{
+			name: "PollingInterval10Hours/AlreadyUpdated",
+			source: &v1alpha1.CatalogSource{
+				Status: v1alpha1.CatalogSourceStatus{
+					LatestImageRegistryPoll: &metav1.Time{
+						time.Now().Add(-(15 * time.Minute)),
+					},
+				},
+				Spec: v1alpha1.CatalogSourceSpec{
+					UpdateStrategy: &v1alpha1.UpdateStrategy{
+						RegistryPoll: &v1alpha1.RegistryPoll{
+							Interval: &metav1.Duration{
+								Duration: 10 * time.Hour,
+							},
+						},
+					},
+				},
+			},
+			expected: queueinformer.DefaultResyncPeriod,
+		},
+	}
+
+	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)
+		}
+	}
+}