From 19c528c35726ca81832f085ba7c29a09598279ad Mon Sep 17 00:00:00 2001 From: Anik Bhattacharjee Date: Thu, 12 Oct 2023 15:36:45 -0400 Subject: [PATCH] (feature) Implement polling image source in intervals Implements https://docs.google.com/document/d/1iWSrWL9pYRJ5Ua3VYErkK1Q2lAusBUeDCh66Ew4lDbQ/edit?usp=sharing Closes #180 Signed-off-by: Anik Bhattacharjee --- api/core/v1alpha1/catalog_types.go | 9 ++- api/core/v1alpha1/zz_generated.deepcopy.go | 8 ++- ...atalogd.operatorframework.io_catalogs.yaml | 25 +++++++ config/samples/core_v1alpha1_catalog.yaml | 1 + internal/source/image_registry_client.go | 5 +- pkg/controllers/core/catalog_controller.go | 12 +++- .../core/catalog_controller_test.go | 65 ++++++++++++++++++- 7 files changed, 119 insertions(+), 6 deletions(-) diff --git a/api/core/v1alpha1/catalog_types.go b/api/core/v1alpha1/catalog_types.go index 3ebf012f..12fcee74 100644 --- 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,6 +87,8 @@ 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 @@ -93,7 +96,7 @@ 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 index 6d85fd21..0b962cee 100644 --- a/api/core/v1alpha1/zz_generated.deepcopy.go +++ b/api/core/v1alpha1/zz_generated.deepcopy.go @@ -91,7 +91,7 @@ func (in *CatalogSource) DeepCopyInto(out *CatalogSource) { if in.Image != nil { in, out := &in.Image, &out.Image *out = new(ImageSource) - **out = **in + (*in).DeepCopyInto(*out) } } @@ -136,6 +136,7 @@ func (in *CatalogStatus) DeepCopyInto(out *CatalogStatus) { *out = new(CatalogSource) (*in).DeepCopyInto(*out) } + in.LastPollAttempt.DeepCopyInto(&out.LastPollAttempt) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CatalogStatus. @@ -151,6 +152,11 @@ func (in *CatalogStatus) DeepCopy() *CatalogStatus { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ImageSource) DeepCopyInto(out *ImageSource) { *out = *in + if in.PollInterval != nil { + in, out := &in.PollInterval, &out.PollInterval + *out = new(v1.Duration) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ImageSource. diff --git a/config/crd/bases/catalogd.operatorframework.io_catalogs.yaml b/config/crd/bases/catalogd.operatorframework.io_catalogs.yaml index b2c87681..f7957cb6 100644 --- 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 index 1895a90a..0158ab20 100644 --- 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 index bc55f26b..7b8d716d 100644 --- 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 index 303ea6ff..f23582e5 100644 --- 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 index 31fe413b..7326c011 100644 --- 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)) + } +}