From fbb729c2ed3a1113d825197e812b6afad3d1c8fc Mon Sep 17 00:00:00 2001 From: Anik Bhattacharjee Date: Fri, 6 Oct 2023 09:51:33 -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 | 31 +++++++ config/samples/core_v1alpha1_catalog.yaml | 1 + internal/source/image_registry_client.go | 10 ++- internal/source/image_registry_client_test.go | 84 +++++++++++++++++++ internal/source/unpacker.go | 6 ++ pkg/controllers/core/catalog_controller.go | 6 +- .../core/catalog_controller_test.go | 37 -------- test/e2e/cel_validation_test.go | 66 +++++++++++++++ 10 files changed, 218 insertions(+), 40 deletions(-) create mode 100644 test/e2e/cel_validation_test.go diff --git a/api/core/v1alpha1/catalog_types.go b/api/core/v1alpha1/catalog_types.go index 3ebf012f..29d36225 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.ref.find('sha256') != \"\" && self.source.image.pollInterval == \"\") || (self.source.image.ref.find('sha256') == \"\" && self.source.image.pollInterval == \"\") || (self.source.image.ref.find('sha256') == \"\" && self.source.image.pollInterval != \"\") ",message="cannot specify PollInterval while using tagged 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 logs the time when the image source was polled for new content. + LastPollAttempt metav1.Time `json:"lastPollAttempt,omitempty"` } // CatalogSource contains the sourcing information for a Catalog @@ -102,6 +105,12 @@ 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.). + // A default value of 24h is used if not specified. Specify 0 to disable polling. Note that + // PollInterval may not be specified for a catalog image referenced by a sha256 digest. + // +kubebuilder:default:="24h" + PollInterval *metav1.Duration `json:"pollInterval"` } 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..aa8d5eec 100644 --- a/config/crd/bases/catalogd.operatorframework.io_catalogs.yaml +++ b/config/crd/bases/catalogd.operatorframework.io_catalogs.yaml @@ -49,6 +49,15 @@ spec: description: Image is the catalog image that backs the content of this catalog. properties: + pollInterval: + default: 24h + 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.). A default value + of 24h is used if not specified. Specify 0 to disable polling. + 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. @@ -58,6 +67,7 @@ spec: containing Catalog contents. type: string required: + - pollInterval - ref type: object type: @@ -69,6 +79,12 @@ spec: required: - source type: object + x-kubernetes-validations: + - message: cannot specify PollInterval while using tagged image + rule: '!has(self.source.image.pollInterval) || (self.source.image.ref.find(''sha256'') + != "" && self.source.image.pollInterval == "") || (self.source.image.ref.find(''sha256'') + == "" && self.source.image.pollInterval == "") || (self.source.image.ref.find(''sha256'') + == "" && self.source.image.pollInterval != "") ' status: description: CatalogStatus defines the observed state of Catalog properties: @@ -146,6 +162,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 logs the time when the image source was + polled for new content. + format: date-time + type: string phase: type: string resolvedSource: @@ -156,6 +177,15 @@ spec: description: Image is the catalog image that backs the content of this catalog. properties: + pollInterval: + default: 24h + 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.). A default value + of 24h is used if not specified. Specify 0 to disable polling. + 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. @@ -165,6 +195,7 @@ spec: containing Catalog contents. type: string required: + - pollInterval - ref type: object type: diff --git a/config/samples/core_v1alpha1_catalog.yaml b/config/samples/core_v1alpha1_catalog.yaml index 1895a90a..5f1aedec 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: 1h ref: quay.io/operatorhubio/catalog:latest diff --git a/internal/source/image_registry_client.go b/internal/source/image_registry_client.go index 9e4628e8..ffe4aa1b 100644 --- a/internal/source/image_registry_client.go +++ b/internal/source/image_registry_client.go @@ -15,6 +15,8 @@ import ( gcrkube "github.com/google/go-containerregistry/pkg/authn/kubernetes" "github.com/google/go-containerregistry/pkg/name" "github.com/google/go-containerregistry/pkg/v1/remote" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/wait" "sigs.k8s.io/controller-runtime/pkg/log" catalogdv1alpha1 "github.com/operator-framework/catalogd/api/core/v1alpha1" @@ -106,7 +108,7 @@ func (i *ImageRegistry) Cleanup(_ context.Context, catalog *catalogdv1alpha1.Cat } func unpackedResult(fsys fs.FS, catalog *catalogdv1alpha1.Catalog, ref string) *Result { - return &Result{ + result := &Result{ FS: fsys, ResolvedSource: &catalogdv1alpha1.CatalogSource{ Type: catalogdv1alpha1.SourceTypeImage, @@ -117,6 +119,12 @@ func unpackedResult(fsys fs.FS, catalog *catalogdv1alpha1.Catalog, ref string) * }, State: StateUnpacked, } + if catalog.Spec.Source.Image.PollInterval.Duration == 0 { + result.RequeueAfter = nil + } else { + result.RequeueAfter = &metav1.Duration{Duration: wait.Jitter(catalog.Spec.Source.Image.PollInterval.Duration, 0.5)} + } + return result } // unpackImage unpacks a catalog image reference to the provided unpackPath, diff --git a/internal/source/image_registry_client_test.go b/internal/source/image_registry_client_test.go index 8e8cd792..bf5a8a9a 100644 --- a/internal/source/image_registry_client_test.go +++ b/internal/source/image_registry_client_test.go @@ -9,6 +9,7 @@ import ( "os" "path/filepath" "testing" + "time" "github.com/google/go-containerregistry/pkg/name" "github.com/google/go-containerregistry/pkg/registry" @@ -361,3 +362,86 @@ func TestImageRegistry(t *testing.T) { } }) } + +func TestPolling(t *testing.T) { + ctx := context.Background() + testCache := t.TempDir() + imgReg := &source.ImageRegistry{ + BaseCachePath: testCache, + } + // Start a new server running an image registry + srv := httptest.NewServer(registry.New()) + defer srv.Close() + + // parse the server url so we can grab just the host + url, err := url.Parse(srv.URL) + assert.NoError(t, err) + duration0s, _ := time.ParseDuration("0s") + duration24h, _ := time.ParseDuration("24h") + for _, tt := range []struct { + testName string + inputCatalog v1alpha1.Catalog + expectedResolvedSourcePollInterval *metav1.Duration + shouldError bool + }{ + { + testName: "When a poll interval is not specified, a default value of 24h is set in the resolved source as poll interval", + inputCatalog: v1alpha1.Catalog{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + }, + Spec: v1alpha1.CatalogSpec{ + Source: v1alpha1.CatalogSource{ + Type: v1alpha1.SourceTypeImage, + Image: &v1alpha1.ImageSource{ + Ref: fmt.Sprintf("%s/%s", url.Host, "test-image:latest"), + }, + }, + }, + }, + expectedResolvedSourcePollInterval: &metav1.Duration{Duration: duration24h}, + shouldError: false, + }, + { + testName: "When a poll interval of 0s is specified, no poll interval is set in the resolved source", + inputCatalog: v1alpha1.Catalog{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + }, + Spec: v1alpha1.CatalogSpec{ + Source: v1alpha1.CatalogSource{ + Type: v1alpha1.SourceTypeImage, + Image: &v1alpha1.ImageSource{ + Ref: fmt.Sprintf("%s/%s", url.Host, "test-image:latest"), + PollInterval: &metav1.Duration{Duration: duration0s}, + }, + }, + }, + }, + expectedResolvedSourcePollInterval: nil, + shouldError: false, + }, + } { + t.Run(t.Name(), func(t *testing.T) { + img, err := random.Image(20, 3) + assert.NoError(t, err) + img, err = mutate.Config(img, v1.Config{ + Labels: map[string]string{ + source.ConfigDirLabel: "/configs", + }, + }) + assert.NoError(t, err) + ref, err := name.ParseReference(tt.inputCatalog.Spec.Source.Image.Ref) + + assert.NoError(t, err) + remote.Write(ref, img) + res, err := imgReg.Unpack(ctx, &tt.inputCatalog) + if tt.shouldError { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + assert.Equal(t, res.ResolvedSource.Image.PollInterval, tt.expectedResolvedSourcePollInterval) + }) + } +} diff --git a/internal/source/unpacker.go b/internal/source/unpacker.go index 05ef8edd..909269ae 100644 --- a/internal/source/unpacker.go +++ b/internal/source/unpacker.go @@ -8,6 +8,8 @@ import ( "path" catalogdv1alpha1 "github.com/operator-framework/catalogd/api/core/v1alpha1" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) // TODO: This package is almost entirely copy/pasted from rukpak. We should look @@ -57,6 +59,10 @@ type Result struct { // Message is contextual information about the progress of unpacking the // catalog content. Message string + + // RequeueAfter contains the duration after which the controller should set + // up for Unpacking again + RequeueAfter *metav1.Duration } type State string diff --git a/pkg/controllers/core/catalog_controller.go b/pkg/controllers/core/catalog_controller.go index 4fe0a988..62a29330 100644 --- a/pkg/controllers/core/catalog_controller.go +++ b/pkg/controllers/core/catalog_controller.go @@ -151,7 +151,10 @@ func (r *CatalogReconciler) reconcile(ctx context.Context, catalog *v1alpha1.Cat } updateStatusUnpacked(&catalog.Status, unpackResult, contentURL) - return ctrl.Result{}, nil + if unpackResult.RequeueAfter == nil { + return ctrl.Result{}, nil + } + return ctrl.Result{RequeueAfter: unpackResult.RequeueAfter.Duration}, nil default: return ctrl.Result{}, updateStatusUnpackFailing(&catalog.Status, fmt.Errorf("unknown unpack state %q: %v", unpackResult.State, err)) } @@ -183,6 +186,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 759ce165..3ba9b578 100644 --- a/pkg/controllers/core/catalog_controller_test.go +++ b/pkg/controllers/core/catalog_controller_test.go @@ -135,43 +135,6 @@ var _ = Describe("Catalogd Controller Test", func() { BeforeEach(func() { catalogKey = types.NamespacedName{Name: fmt.Sprintf("catalogd-test-%s", rand.String(8))} }) - - When("the catalog specifies an invalid source", func() { - BeforeEach(func() { - By("initializing cluster state") - catalog = &v1alpha1.Catalog{ - ObjectMeta: metav1.ObjectMeta{Name: catalogKey.Name}, - Spec: v1alpha1.CatalogSpec{ - Source: v1alpha1.CatalogSource{ - Type: "invalid-source", - }, - }, - } - Expect(cl.Create(ctx, catalog)).To(Succeed()) - }) - - AfterEach(func() { - By("tearing down cluster state") - Expect(cl.Delete(ctx, catalog)).To(Succeed()) - }) - - It("should set unpacking status to failed and return an error", func() { - res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: catalogKey}) - Expect(res).To(Equal(ctrl.Result{})) - Expect(err).To(HaveOccurred()) - - // get the catalog and ensure status is set properly - cat := &v1alpha1.Catalog{} - Expect(cl.Get(ctx, catalogKey, cat)).To(Succeed()) - Expect(cat.Status.ResolvedSource).To(BeNil()) - Expect(cat.Status.Phase).To(Equal(v1alpha1.PhaseFailing)) - cond := meta.FindStatusCondition(cat.Status.Conditions, v1alpha1.TypeUnpacked) - Expect(cond).ToNot(BeNil()) - Expect(cond.Reason).To(Equal(v1alpha1.ReasonUnpackFailed)) - Expect(cond.Status).To(Equal(metav1.ConditionFalse)) - }) - }) - When("the catalog specifies a valid source", func() { BeforeEach(func() { By("initializing cluster state") diff --git a/test/e2e/cel_validation_test.go b/test/e2e/cel_validation_test.go new file mode 100644 index 00000000..9cd1f720 --- /dev/null +++ b/test/e2e/cel_validation_test.go @@ -0,0 +1,66 @@ +package e2e + +import ( + "context" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + catalogd "github.com/operator-framework/catalogd/api/core/v1alpha1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +var _ = FDescribe("CEL validation tests", func() { + var ( + ctx context.Context + catalog *catalogd.Catalog + ) + When("A Catalog with a digest based image ref is created with a poll interval", func() { + duration45m, _ := time.ParseDuration("45m") + BeforeEach(func() { + ctx = context.Background() + + catalog = &catalogd.Catalog{ + ObjectMeta: metav1.ObjectMeta{ + Name: catalogName, + }, + Spec: catalogd.CatalogSpec{ + Source: catalogd.CatalogSource{ + Type: catalogd.SourceTypeImage, + Image: &catalogd.ImageSource{ + Ref: "docker.io/test-image@sha256:asdf98234sd", + PollInterval: &metav1.Duration{Duration: duration45m}, + }, + }, + }, + } + }) + It("throws an error", func() { + err = c.Create(ctx, catalog) + Expect(err).To(HaveOccurred()) + }) + }) + When("A Catalog with a digest based image ref is created without a poll interval", func() { + BeforeEach(func() { + ctx = context.Background() + + catalog = &catalogd.Catalog{ + ObjectMeta: metav1.ObjectMeta{ + Name: catalogName, + }, + Spec: catalogd.CatalogSpec{ + Source: catalogd.CatalogSource{ + Type: catalogd.SourceTypeImage, + Image: &catalogd.ImageSource{ + Ref: "docker.io/test-image@sha256:asdf98234sd", + }, + }, + }, + } + }) + It("does not throw an error", func() { + err = c.Create(ctx, catalog) + Expect(err).ToNot(HaveOccurred()) + }) + }) +})