(feature) Implement polling image source in intervals

Implements https://docs.google.com/document/d/1iWSrWL9pYRJ5Ua3VYErkK1Q2lAusBUeDCh66Ew4lDbQ/edit?usp=sharing Closes operator-framework#180 Signed-off-by: Anik Bhattacharjee <anbhatta@redhat.com>
anik120 · Oct 16, 2023 · 19c528c · 19c528c
1 parent bd8cfbd
commit 19c528c
Show file tree

Hide file tree

Showing 7 changed files with 119 additions and 6 deletions.
diff --git a/api/core/v1alpha1/catalog_types.go b/api/core/v1alpha1/catalog_types.go
@@ -70,6 +70,7 @@ type CatalogList struct {
 }
 
 // CatalogSpec defines the desired state of Catalog
+// +kubebuilder:validation:XValidation:rule="!has(self.source.image.pollInterval) || (self.source.image.pollInterval == \"\") || (self.source.image.ref.find('@sha256:') == \"\")",message="cannot specify PollInterval while using digest-based image"
 type CatalogSpec struct {
 	// Source is the source of a Catalog that contains Operators' metadata in the FBC format
 	// https://olm.operatorframework.io/docs/reference/file-based-catalogs/#docs
@@ -86,14 +87,16 @@ type CatalogStatus struct {
 	// ContentURL is a cluster-internal address that on-cluster components
 	// can read the content of a catalog from
 	ContentURL string `json:"contentURL,omitempty"`
+	// LastPollAttempt is the time when the source resolved was last polled for new content.
+	LastPollAttempt metav1.Time `json:"lastPollAttempt,omitempty"`
 }
 
 // CatalogSource contains the sourcing information for a Catalog
 type CatalogSource struct {
 	// Type defines the kind of Catalog content being sourced.
 	Type SourceType `json:"type"`
 	// Image is the catalog image that backs the content of this catalog.
-	Image *ImageSource `json:"image,omitempty"`
+	Image *ImageSource `json:"image"`
 }
 
 // ImageSource contains information required for sourcing a Catalog from an OCI image
@@ -102,6 +105,10 @@ type ImageSource struct {
 	Ref string `json:"ref"`
 	// PullSecret contains the name of the image pull secret in the namespace that catalogd is deployed.
 	PullSecret string `json:"pullSecret,omitempty"`
+	// PollInterval indicates the interval at which the image source should be polled for new content,
+	// specified as a duration (e.g., "5m", "1h", "24h", "etc".). Note that PollInterval may not be
+	// specified for a catalog image referenced by a sha256 digest.
+	PollInterval *metav1.Duration `json:"pollInterval,omitempty"`
 }
 
 func init() {

diff --git a/api/core/v1alpha1/zz_generated.deepcopy.go b/api/core/v1alpha1/zz_generated.deepcopy.go
diff --git a/config/crd/bases/catalogd.operatorframework.io_catalogs.yaml b/config/crd/bases/catalogd.operatorframework.io_catalogs.yaml
@@ -49,6 +49,13 @@ spec:
                     description: Image is the catalog image that backs the content
                       of this catalog.
                     properties:
+                      pollInterval:
+                        description: PollInterval indicates the interval at which
+                          the image source should be polled for new content, specified
+                          as a duration (e.g., "5m", "1h", "24h", "etc".). Note that
+                          PollInterval may not be specified for a catalog image referenced
+                          by a sha256 digest.
+                        type: string
                       pullSecret:
                         description: PullSecret contains the name of the image pull
                           secret in the namespace that catalogd is deployed.
@@ -64,11 +71,16 @@ spec:
                     description: Type defines the kind of Catalog content being sourced.
                     type: string
                 required:
+                - image
                 - type
                 type: object
             required:
             - source
             type: object
+            x-kubernetes-validations:
+            - message: cannot specify PollInterval while using digest-based image
+              rule: '!has(self.source.image.pollInterval) || (self.source.image.pollInterval
+                == "") || (self.source.image.ref.find(''@sha256:'') == "")'
           status:
             description: CatalogStatus defines the observed state of Catalog
             properties:
@@ -146,6 +158,11 @@ spec:
                 description: ContentURL is a cluster-internal address that on-cluster
                   components can read the content of a catalog from
                 type: string
+              lastPollAttempt:
+                description: LastPollAttempt is the time when the source resolved
+                  was last polled for new content.
+                format: date-time
+                type: string
               phase:
                 type: string
               resolvedSource:
@@ -156,6 +173,13 @@ spec:
                     description: Image is the catalog image that backs the content
                       of this catalog.
                     properties:
+                      pollInterval:
+                        description: PollInterval indicates the interval at which
+                          the image source should be polled for new content, specified
+                          as a duration (e.g., "5m", "1h", "24h", "etc".). Note that
+                          PollInterval may not be specified for a catalog image referenced
+                          by a sha256 digest.
+                        type: string
                       pullSecret:
                         description: PullSecret contains the name of the image pull
                           secret in the namespace that catalogd is deployed.
@@ -171,6 +195,7 @@ spec:
                     description: Type defines the kind of Catalog content being sourced.
                     type: string
                 required:
+                - image
                 - type
                 type: object
             type: object

diff --git a/config/samples/core_v1alpha1_catalog.yaml b/config/samples/core_v1alpha1_catalog.yaml
@@ -6,4 +6,5 @@ spec:
   source:
     type: image
     image:
+      pollInterval: 24h
       ref: quay.io/operatorhubio/catalog:latest
diff --git a/internal/source/image_registry_client.go b/internal/source/image_registry_client.go
@@ -119,8 +119,9 @@ func unpackedResult(fsys fs.FS, catalog *catalogdv1alpha1.Catalog, ref string) *
 		ResolvedSource: &catalogdv1alpha1.CatalogSource{
 			Type: catalogdv1alpha1.SourceTypeImage,
 			Image: &catalogdv1alpha1.ImageSource{
-				Ref:        ref,
-				PullSecret: catalog.Spec.Source.Image.PullSecret,
+				Ref:          ref,
+				PullSecret:   catalog.Spec.Source.Image.PullSecret,
+				PollInterval: catalog.Spec.Source.Image.PollInterval,
 			},
 		},
 		State: StateUnpacked,

diff --git a/pkg/controllers/core/catalog_controller.go b/pkg/controllers/core/catalog_controller.go
@@ -20,12 +20,14 @@ import (
 	"context" // #nosec
 	"errors"
 	"fmt"
+	"time"
 
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/equality"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	apimacherrors "k8s.io/apimachinery/pkg/util/errors"
+	"k8s.io/apimachinery/pkg/util/wait"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
@@ -149,7 +151,14 @@ func (r *CatalogReconciler) reconcile(ctx context.Context, catalog *v1alpha1.Cat
 		contentURL = r.Storage.ContentURL(catalog.Name)
 
 		updateStatusUnpacked(&catalog.Status, unpackResult, contentURL)
-		return ctrl.Result{}, nil
+		requeueAfter := time.Second * 0
+		switch catalog.Spec.Source.Type {
+		case v1alpha1.SourceTypeImage:
+			if catalog.Spec.Source.Image.PollInterval != nil {
+				requeueAfter = wait.Jitter(catalog.Spec.Source.Image.PollInterval.Duration, 0.1)
+			}
+		}
+		return ctrl.Result{RequeueAfter: requeueAfter}, nil
 	default:
 		return ctrl.Result{}, updateStatusUnpackFailing(&catalog.Status, fmt.Errorf("unknown unpack state %q: %v", unpackResult.State, err))
 	}
@@ -181,6 +190,7 @@ func updateStatusUnpacked(status *v1alpha1.CatalogStatus, result *source.Result,
 	status.ResolvedSource = result.ResolvedSource
 	status.ContentURL = contentURL
 	status.Phase = v1alpha1.PhaseUnpacked
+	status.LastPollAttempt = metav1.Now()
 	meta.SetStatusCondition(&status.Conditions, metav1.Condition{
 		Type:    v1alpha1.TypeUnpacked,
 		Status:  metav1.ConditionTrue,

diff --git a/pkg/controllers/core/catalog_controller_test.go b/pkg/controllers/core/catalog_controller_test.go
@@ -12,6 +12,7 @@ import (
 	"github.com/google/go-cmp/cmp"
 	"github.com/stretchr/testify/assert"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/wait"
 	ctrl "sigs.k8s.io/controller-runtime"
 
 	"github.com/google/go-cmp/cmp/cmpopts"
@@ -539,9 +540,71 @@ func TestCatalogdControllerReconcile(t *testing.T) {
 			} else {
 				assert.Error(t, err)
 			}
-
+			tt.catalog.Status.LastPollAttempt = metav1.Time{}
 			diff := cmp.Diff(tt.expectedCatalog, tt.catalog, cmpopts.IgnoreFields(metav1.Condition{}, "Message", "LastTransitionTime"))
 			assert.Empty(t, diff, "comparing the expected Catalog")
 		})
 	}
 }
+
+func TestPolling(t *testing.T) {
+	for _, tt := range []struct {
+		name                 string
+		catalog              *catalogdv1alpha1.Catalog
+		expectedRequeueAfter time.Duration
+	}{
+		{
+			name: "Catalog with tag based image ref without any poll interval specified, requeueAfter set to 0, ie polling disabled",
+			catalog: &catalogdv1alpha1.Catalog{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:       "test-catalog",
+					Finalizers: []string{fbcDeletionFinalizer},
+				},
+				Spec: catalogdv1alpha1.CatalogSpec{
+					Source: catalogdv1alpha1.CatalogSource{
+						Type: "image",
+						Image: &catalogdv1alpha1.ImageSource{
+							Ref: "someimage:latest",
+						},
+					},
+				},
+			},
+			expectedRequeueAfter: time.Second * 0,
+		},
+		{
+			name: "Catalog with tag based image ref with poll interval specified, requeueAfter set to wait.jitter(pollInterval)",
+			catalog: &catalogdv1alpha1.Catalog{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:       "test-catalog",
+					Finalizers: []string{fbcDeletionFinalizer},
+				},
+				Spec: catalogdv1alpha1.CatalogSpec{
+					Source: catalogdv1alpha1.CatalogSource{
+						Type: "image",
+						Image: &catalogdv1alpha1.ImageSource{
+							Ref:          "someimage:latest",
+							PollInterval: &metav1.Duration{Duration: time.Minute * 5},
+						},
+					},
+				},
+			},
+			expectedRequeueAfter: time.Minute * 5,
+		},
+	} {
+		reconciler := &CatalogReconciler{
+			Client: nil,
+			Unpacker: source.NewUnpacker(
+				map[catalogdv1alpha1.SourceType]source.Unpacker{
+					catalogdv1alpha1.SourceTypeImage: &MockSource{result: &source.Result{
+						State: source.StateUnpacked,
+						FS:    &fstest.MapFS{},
+					}},
+				},
+			),
+			Storage: &MockStore{},
+		}
+		res, _ := reconciler.reconcile(context.Background(), tt.catalog)
+		assert.GreaterOrEqual(t, res.RequeueAfter, tt.expectedRequeueAfter)
+		assert.LessOrEqual(t, res.RequeueAfter, wait.Jitter(res.RequeueAfter, 0.1))
+	}
+}