diff --git a/README.md b/README.md index dc417d7c..08982b9e 100644 --- a/README.md +++ b/README.md @@ -24,7 +24,7 @@ Procedure steps marked with an asterisk (`*`) are likely to change with future A name: operatorhubio spec: source: - type: image + type: Image image: ref: quay.io/operatorhubio/catalog:latest EOF @@ -57,7 +57,7 @@ Procedure steps marked with an asterisk (`*`) are likely to change with future A Image: Poll Interval: 10m0s Ref: quay.io/operatorhubio/catalog:latest - Type: image + Type: Image Status: Conditions: Last Transition Time: 2024-09-12T13:37:53Z @@ -79,7 +79,7 @@ Procedure steps marked with an asterisk (`*`) are likely to change with future A Last Unpacked: 2024-09-12T13:37:52Z Ref: quay.io/operatorhubio/catalog:latest Resolved Ref: quay.io/operatorhubio/catalog@sha256:4453a361198d39d0390fd8c1a7f07b5a5a3ae1e8dac9979ef0c4eba46299df16 - Type: image + Type: Image Events: ``` diff --git a/api/core/v1alpha1/clustercatalog_types.go b/api/core/v1alpha1/clustercatalog_types.go index e45b23da..b47d15bc 100644 --- a/api/core/v1alpha1/clustercatalog_types.go +++ b/api/core/v1alpha1/clustercatalog_types.go @@ -25,7 +25,7 @@ import ( type SourceType string const ( - SourceTypeImage SourceType = "image" + SourceTypeImage SourceType = "Image" TypeProgressing = "Progressing" TypeServing = "Serving" @@ -77,7 +77,7 @@ type ClusterCatalogSpec struct { // Below is a minimal example of a ClusterCatalogSpec that sources a catalog from an image: // // source: - // type: image + // type: Image // image: // ref: quay.io/operatorhubio/catalog:latest // @@ -91,7 +91,7 @@ type ClusterCatalogSpec struct { // When omitted, the default priority is 0. // +kubebuilder:default:=0 // +optional - Priority int32 `json:"priority,omitempty"` + Priority int32 `json:"priority"` } // ClusterCatalogStatus defines the observed state of ClusterCatalog @@ -128,51 +128,54 @@ type ClusterCatalogStatus struct { // lastUnpacked: "2024-09-10T12:22:13Z" // ref: quay.io/operatorhubio/catalog:latest // resolvedRef: quay.io/operatorhubio/catalog@sha256:c7392b4be033da629f9d665fec30f6901de51ce3adebeff0af579f311ee5cf1b - // type: image + // type: Image // +optional ResolvedSource *ResolvedCatalogSource `json:"resolvedSource,omitempty"` // contentURL is a cluster-internal URL from which on-cluster components // can read the content of a catalog // +optional ContentURL string `json:"contentURL,omitempty"` - // observedGeneration is the most recent generation observed for this ClusterCatalog. It corresponds to the - // ClusterCatalog's generation, which is updated on mutation by the API Server. - // +optional - ObservedGeneration int64 `json:"observedGeneration,omitempty"` + // lastUnpacked represents the time when the - // ClusterCatalog object was last unpacked. + // ClusterCatalog object was last unpacked successfully. // +optional LastUnpacked metav1.Time `json:"lastUnpacked,omitempty"` } // CatalogSource is a discriminated union of possible sources for a Catalog. +// CatalogSource contains the sourcing information for a Catalog +// +union +// +kubebuilder:validation:XValidation:rule="self.type == 'Image' && has(self.image)",message="source type 'Image' requires image field" type CatalogSource struct { // type is a required reference to the type of source the catalog is sourced from. // - // Allowed values are ["image"] + // Allowed values are ["Image"] // - // When this field is set to "image", the ClusterCatalog content will be sourced from an OCI image. + // When this field is set to "Image", the ClusterCatalog content will be sourced from an OCI image. // When using an image source, the image field must be set and must be the only field defined for this type. // // +unionDiscriminator - // +kubebuilder:validation:Enum:="image" + // +kubebuilder:validation:Enum:="Image" // +kubebuilder:validation:Required Type SourceType `json:"type"` - // image is used to configure how catalog contents are sourced from an OCI image. This field must be set when type is set to "image" and must be the only field defined for this type. + // image is used to configure how catalog contents are sourced from an OCI image. This field must be set when type is set to "Image" and must be the only field defined for this type. // +optional Image *ImageSource `json:"image,omitempty"` } // ResolvedCatalogSource is a discriminated union of resolution information for a Catalog. +// ResolvedCatalogSource contains the information about a sourced Catalog +// +union +// +kubebuilder:validation:XValidation:rule="self.type == 'Image' && has(self.image)",message="source type 'Image' requires image field" type ResolvedCatalogSource struct { // type is a reference to the type of source the catalog is sourced from. // - // It will be set to one of the following values: ["image"]. + // It will be set to one of the following values: ["Image"]. // - // When this field is set to "image", information about the resolved image source will be set in the 'image' field. + // When this field is set to "Image", information about the resolved image source will be set in the 'image' field. // // +unionDiscriminator - // +kubebuilder:validation:Enum:="image" + // +kubebuilder:validation:Enum:="Image" // +kubebuilder:validation:Required Type SourceType `json:"type"` // image is a field containing resolution information for a catalog sourced from an image. @@ -181,14 +184,10 @@ type ResolvedCatalogSource struct { // ResolvedImageSource provides information about the resolved source of a Catalog sourced from an image. type ResolvedImageSource struct { - // ref is the reference to a container image containing Catalog contents. + // ref contains the resolved sha256 image ref containing Catalog contents. Ref string `json:"ref"` - // resolvedRef is the resolved sha256 image ref containing Catalog contents. - ResolvedRef string `json:"resolvedRef"` - // lastPollAttempt is the time when the source image was last polled for new content. - LastPollAttempt metav1.Time `json:"lastPollAttempt"` - // lastUnpacked is the last time when the Catalog contents were successfully unpacked. - LastUnpacked metav1.Time `json:"lastUnpacked"` + // lastSuccessfulPollAttempt is the time when the resolved source was last successfully polled for new content. + LastSuccessfulPollAttempt metav1.Time `json:"lastSuccessfulPollAttempt"` } // ImageSource enables users to define the information required for sourcing a Catalog from an OCI image diff --git a/api/core/v1alpha1/clustercatalog_types_test.go b/api/core/v1alpha1/clustercatalog_types_test.go index 8e617b20..ee106bdb 100644 --- a/api/core/v1alpha1/clustercatalog_types_test.go +++ b/api/core/v1alpha1/clustercatalog_types_test.go @@ -2,6 +2,7 @@ package v1alpha1 import ( "context" + "fmt" "os" "testing" "time" @@ -23,7 +24,7 @@ func TestPollIntervalCELValidationRules(t *testing.T) { validators := fieldValidatorsFromFile(t, "../../../config/base/crd/bases/olm.operatorframework.io_clustercatalogs.yaml") pth := "openAPIV3Schema.properties.spec" validator, found := validators["v1alpha1"][pth] - assert.True(t, found) + require.True(t, found) for name, tc := range map[string]struct { spec ClusterCatalogSpec @@ -68,6 +69,84 @@ func TestPollIntervalCELValidationRules(t *testing.T) { } } +func TestSourceCELValidation(t *testing.T) { + validators := fieldValidatorsFromFile(t, "../../../config/base/crd/bases/olm.operatorframework.io_clustercatalogs.yaml") + pth := "openAPIV3Schema.properties.spec.properties.source" + validator, found := validators["v1alpha1"][pth] + require.True(t, found) + for name, tc := range map[string]struct { + source CatalogSource + wantErrs []string + }{ + "image source missing required image field": { + source: CatalogSource{ + Type: SourceTypeImage, + }, + wantErrs: []string{ + fmt.Sprintf("%s: Invalid value: \"object\": source type '%s' requires image field", pth, SourceTypeImage), + }, + }, + "image source with required image field": { + source: CatalogSource{ + Type: SourceTypeImage, + Image: &ImageSource{}, + }, + wantErrs: []string{}, + }, + } { + t.Run(name, func(t *testing.T) { + obj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&tc.source) //nolint:gosec + require.NoError(t, err) + errs := validator(obj, nil) + fmt.Println(errs) + require.Equal(t, len(tc.wantErrs), len(errs)) + for i := range tc.wantErrs { + got := errs[i].Error() + assert.Equal(t, tc.wantErrs[i], got) + } + }) + } +} + +func TestResolvedSourceCELValidation(t *testing.T) { + validators := fieldValidatorsFromFile(t, "../../../config/base/crd/bases/olm.operatorframework.io_clustercatalogs.yaml") + pth := "openAPIV3Schema.properties.status.properties.resolvedSource" + validator, found := validators["v1alpha1"][pth] + + require.True(t, found) + for name, tc := range map[string]struct { + source ResolvedCatalogSource + wantErrs []string + }{ + "image source missing required image field": { + source: ResolvedCatalogSource{ + Type: SourceTypeImage, + }, + wantErrs: []string{ + fmt.Sprintf("%s: Invalid value: \"object\": source type '%s' requires image field", pth, SourceTypeImage), + }, + }, + "image source with required image field": { + source: ResolvedCatalogSource{ + Type: SourceTypeImage, + Image: &ResolvedImageSource{}, + }, + wantErrs: []string{}, + }, + } { + t.Run(name, func(t *testing.T) { + obj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&tc.source) //nolint:gosec + require.NoError(t, err) + errs := validator(obj, nil) + require.Equal(t, len(tc.wantErrs), len(errs)) + for i := range tc.wantErrs { + got := errs[i].Error() + assert.Equal(t, tc.wantErrs[i], got) + } + }) + } +} + // fieldValidatorsFromFile extracts the CEL validators by version and JSONPath from a CRD file and returns // a validator func for testing against samples. func fieldValidatorsFromFile(t *testing.T, crdFilePath string) map[string]map[string]CELValidateFunc { diff --git a/api/core/v1alpha1/zz_generated.deepcopy.go b/api/core/v1alpha1/zz_generated.deepcopy.go index b68da00a..444ede9f 100644 --- a/api/core/v1alpha1/zz_generated.deepcopy.go +++ b/api/core/v1alpha1/zz_generated.deepcopy.go @@ -191,8 +191,7 @@ func (in *ResolvedCatalogSource) DeepCopy() *ResolvedCatalogSource { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ResolvedImageSource) DeepCopyInto(out *ResolvedImageSource) { *out = *in - in.LastPollAttempt.DeepCopyInto(&out.LastPollAttempt) - in.LastUnpacked.DeepCopyInto(&out.LastUnpacked) + in.LastSuccessfulPollAttempt.DeepCopyInto(&out.LastSuccessfulPollAttempt) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ResolvedImageSource. diff --git a/config/base/crd/bases/olm.operatorframework.io_clustercatalogs.yaml b/config/base/crd/bases/olm.operatorframework.io_clustercatalogs.yaml index 0aa06136..be4ff3b1 100644 --- a/config/base/crd/bases/olm.operatorframework.io_clustercatalogs.yaml +++ b/config/base/crd/bases/olm.operatorframework.io_clustercatalogs.yaml @@ -68,7 +68,7 @@ spec: Below is a minimal example of a ClusterCatalogSpec that sources a catalog from an image: source: - type: image + type: Image image: ref: quay.io/operatorhubio/catalog:latest @@ -77,7 +77,7 @@ spec: image: description: image is used to configure how catalog contents are sourced from an OCI image. This field must be set when type - is set to "image" and must be the only field defined for this + is set to "Image" and must be the only field defined for this type. properties: pollInterval: @@ -107,16 +107,19 @@ spec: description: |- type is a required reference to the type of source the catalog is sourced from. - Allowed values are ["image"] + Allowed values are ["Image"] - When this field is set to "image", the ClusterCatalog content will be sourced from an OCI image. + When this field is set to "Image", the ClusterCatalog content will be sourced from an OCI image. When using an image source, the image field must be set and must be the only field defined for this type. enum: - - image + - Image type: string required: - type type: object + x-kubernetes-validations: + - message: source type 'Image' requires image field + rule: self.type == 'Image' && has(self.image) required: - source type: object @@ -211,15 +214,9 @@ spec: lastUnpacked: description: |- lastUnpacked represents the time when the - ClusterCatalog object was last unpacked. + ClusterCatalog object was last unpacked successfully. format: date-time type: string - observedGeneration: - description: |- - observedGeneration is the most recent generation observed for this ClusterCatalog. It corresponds to the - ClusterCatalog's generation, which is updated on mutation by the API Server. - format: int64 - type: integer resolvedSource: description: |- resolvedSource contains information about the resolved source based on the source type. @@ -232,50 +229,42 @@ spec: lastUnpacked: "2024-09-10T12:22:13Z" ref: quay.io/operatorhubio/catalog:latest resolvedRef: quay.io/operatorhubio/catalog@sha256:c7392b4be033da629f9d665fec30f6901de51ce3adebeff0af579f311ee5cf1b - type: image + type: Image properties: image: description: image is a field containing resolution information for a catalog sourced from an image. properties: - lastPollAttempt: - description: lastPollAttempt is the time when the source image - was last polled for new content. - format: date-time - type: string - lastUnpacked: - description: lastUnpacked is the last time when the Catalog - contents were successfully unpacked. + lastSuccessfulPollAttempt: + description: lastSuccessfulPollAttempt is the time when the + resolved source was last successfully polled for new content. format: date-time type: string ref: - description: ref is the reference to a container image containing + description: ref contains the resolved sha256 image ref containing Catalog contents. type: string - resolvedRef: - description: resolvedRef is the resolved sha256 image ref - containing Catalog contents. - type: string required: - - lastPollAttempt - - lastUnpacked + - lastSuccessfulPollAttempt - ref - - resolvedRef type: object type: description: |- type is a reference to the type of source the catalog is sourced from. - It will be set to one of the following values: ["image"]. + It will be set to one of the following values: ["Image"]. - When this field is set to "image", information about the resolved image source will be set in the 'image' field. + When this field is set to "Image", information about the resolved image source will be set in the 'image' field. enum: - - image + - Image type: string required: - image - type type: object + x-kubernetes-validations: + - message: source type 'Image' requires image field + rule: self.type == 'Image' && has(self.image) type: object required: - metadata diff --git a/config/base/crd/kustomization.yaml b/config/base/crd/kustomization.yaml index 7c9b09f1..36c15128 100644 --- a/config/base/crd/kustomization.yaml +++ b/config/base/crd/kustomization.yaml @@ -4,11 +4,3 @@ resources: - bases/olm.operatorframework.io_clustercatalogs.yaml #+kubebuilder:scaffold:crdkustomizeresource - -patches: -- path: patches/catalog_validation.yaml - target: - group: apiextensions.k8s.io - version: v1 - kind: CustomResourceDefinition - name: catalogs.olm.operatorframework.io diff --git a/config/base/crd/patches/catalog_validation.yaml b/config/base/crd/patches/catalog_validation.yaml deleted file mode 100644 index 0ba9cda4..00000000 --- a/config/base/crd/patches/catalog_validation.yaml +++ /dev/null @@ -1,6 +0,0 @@ -# Union source type -- op: add - path: /spec/versions/0/schema/openAPIV3Schema/properties/spec/properties/source/oneOf - value: - - required: - - image \ No newline at end of file diff --git a/config/base/default/clustercatalogs/default-catalogs.yaml b/config/base/default/clustercatalogs/default-catalogs.yaml index 98386e0f..23bc9d28 100644 --- a/config/base/default/clustercatalogs/default-catalogs.yaml +++ b/config/base/default/clustercatalogs/default-catalogs.yaml @@ -5,7 +5,7 @@ metadata: namespace: olmv1-system spec: source: - type: image + type: Image image: ref: quay.io/operatorhubio/catalog:latest pollInterval: 10m diff --git a/config/samples/core_v1alpha1_clustercatalog.yaml b/config/samples/core_v1alpha1_clustercatalog.yaml index 980b7afb..a8f33097 100644 --- a/config/samples/core_v1alpha1_clustercatalog.yaml +++ b/config/samples/core_v1alpha1_clustercatalog.yaml @@ -5,7 +5,7 @@ metadata: spec: priority: 0 source: - type: image + type: Image image: pollInterval: 24h ref: quay.io/operatorhubio/catalog:latest diff --git a/docs/fetching-catalog-contents.md b/docs/fetching-catalog-contents.md index a016878e..93733abd 100644 --- a/docs/fetching-catalog-contents.md +++ b/docs/fetching-catalog-contents.md @@ -90,7 +90,7 @@ of a catalog can be read from: resolvedSource: image: ref: quay.io/operatorhubio/catalog@sha256:e53267559addc85227c2a7901ca54b980bc900276fc24d3f4db0549cb38ecf76 - type: image + type: Image ``` @@ -148,7 +148,7 @@ This section outlines a way of exposing the `Catalogd` Service's endpoints outsi name: operatorhubio spec: source: - type: image + type: Image image: ref: quay.io/operatorhubio/catalog:latest EOF diff --git a/internal/controllers/core/clustercatalog_controller.go b/internal/controllers/core/clustercatalog_controller.go index 9330ca93..e94471ee 100644 --- a/internal/controllers/core/clustercatalog_controller.go +++ b/internal/controllers/core/clustercatalog_controller.go @@ -158,14 +158,8 @@ func (r *ClusterCatalogReconciler) reconcile(ctx context.Context, catalog *v1alp } contentURL = r.Storage.ContentURL(catalog.Name) - var lastUnpacked metav1.Time - - if unpackResult != nil && unpackResult.ResolvedSource != nil && unpackResult.ResolvedSource.Image != nil { - lastUnpacked = unpackResult.ResolvedSource.Image.LastUnpacked - } - updateStatusProgressing(catalog, nil) - updateStatusServing(&catalog.Status, unpackResult, contentURL, catalog.Generation, lastUnpacked) + updateStatusServing(&catalog.Status, unpackResult, contentURL, catalog.GetGeneration()) var requeueAfter time.Duration switch catalog.Spec.Source.Type { @@ -183,10 +177,11 @@ func (r *ClusterCatalogReconciler) reconcile(ctx context.Context, catalog *v1alp func updateStatusProgressing(catalog *v1alpha1.ClusterCatalog, err error) { progressingCond := metav1.Condition{ - Type: v1alpha1.TypeProgressing, - Status: metav1.ConditionFalse, - Reason: v1alpha1.ReasonSucceeded, - Message: "Successfully unpacked and stored content from resolved source", + Type: v1alpha1.TypeProgressing, + Status: metav1.ConditionFalse, + Reason: v1alpha1.ReasonSucceeded, + Message: "Successfully unpacked and stored content from resolved source", + ObservedGeneration: catalog.GetGeneration(), } if err != nil { @@ -202,28 +197,29 @@ func updateStatusProgressing(catalog *v1alpha1.ClusterCatalog, err error) { meta.SetStatusCondition(&catalog.Status.Conditions, progressingCond) } -func updateStatusServing(status *v1alpha1.ClusterCatalogStatus, result *source.Result, contentURL string, generation int64, unpackedAt metav1.Time) { + +func updateStatusServing(status *v1alpha1.ClusterCatalogStatus, result *source.Result, contentURL string, generation int64) { status.ResolvedSource = result.ResolvedSource status.ContentURL = contentURL - status.ObservedGeneration = generation - status.LastUnpacked = unpackedAt + status.LastUnpacked = metav1.NewTime(result.UnpackTime) meta.SetStatusCondition(&status.Conditions, metav1.Condition{ - Type: v1alpha1.TypeServing, - Status: metav1.ConditionTrue, - Reason: v1alpha1.ReasonAvailable, - Message: "Serving desired content from resolved source", + Type: v1alpha1.TypeServing, + Status: metav1.ConditionTrue, + Reason: v1alpha1.ReasonAvailable, + Message: "Serving desired content from resolved source", + ObservedGeneration: generation, }) } -func updateStatusNotServing(status *v1alpha1.ClusterCatalogStatus) { +func updateStatusNotServing(status *v1alpha1.ClusterCatalogStatus, generation int64) { status.ResolvedSource = nil status.ContentURL = "" - status.ObservedGeneration = 0 status.LastUnpacked = metav1.Time{} meta.SetStatusCondition(&status.Conditions, metav1.Condition{ - Type: v1alpha1.TypeServing, - Status: metav1.ConditionFalse, - Reason: v1alpha1.ReasonUnavailable, + Type: v1alpha1.TypeServing, + Status: metav1.ConditionFalse, + Reason: v1alpha1.ReasonUnavailable, + ObservedGeneration: generation, }) } @@ -240,23 +236,21 @@ func (r *ClusterCatalogReconciler) needsUnpacking(catalog *v1alpha1.ClusterCatal if catalog.Spec.Source.Image == nil { return false } - // if the spec.Source.Image.Ref was changed, unpack the new ref - // NOTE: we must compare image reference WITHOUT sha hash here - // otherwise we will always be unpacking image even when poll interval not lapsed - if catalog.Spec.Source.Image.Ref != catalog.Status.ResolvedSource.Image.Ref { + if len(catalog.Status.Conditions) == 0 { return true } + for _, c := range catalog.Status.Conditions { + if c.ObservedGeneration != catalog.Generation { + return true + } + } // if pollInterval is nil, don't unpack again if catalog.Spec.Source.Image.PollInterval == nil { return false } // if it's not time to poll yet, and the CR wasn't changed don't unpack again - nextPoll := catalog.Status.ResolvedSource.Image.LastPollAttempt.Add(catalog.Spec.Source.Image.PollInterval.Duration) - if nextPoll.After(time.Now()) && catalog.Generation == catalog.Status.ObservedGeneration { - return false - } - // time to unpack - return true + nextPoll := catalog.Status.ResolvedSource.Image.LastSuccessfulPollAttempt.Add(catalog.Spec.Source.Image.PollInterval.Duration) + return !nextPoll.After(time.Now()) } // Compare resources - ignoring status & metadata.finalizers @@ -283,7 +277,7 @@ func NewFinalizers(localStorage storage.Instance, unpacker source.Unpacker) (crf updateStatusProgressing(catalog, err) return crfinalizer.Result{StatusUpdated: true}, err } - updateStatusNotServing(&catalog.Status) + updateStatusNotServing(&catalog.Status, catalog.GetGeneration()) if err := unpacker.Cleanup(ctx, catalog); err != nil { updateStatusProgressing(catalog, err) return crfinalizer.Result{StatusUpdated: true}, err diff --git a/internal/controllers/core/clustercatalog_controller_test.go b/internal/controllers/core/clustercatalog_controller_test.go index 4722e398..d46ec09b 100644 --- a/internal/controllers/core/clustercatalog_controller_test.go +++ b/internal/controllers/core/clustercatalog_controller_test.go @@ -141,9 +141,9 @@ func TestCatalogdControllerReconcile(t *testing.T) { }, Spec: catalogdv1alpha1.ClusterCatalogSpec{ Source: catalogdv1alpha1.CatalogSource{ - Type: "image", + Type: catalogdv1alpha1.SourceTypeImage, Image: &catalogdv1alpha1.ImageSource{ - Ref: "someimage:latest", + Ref: "my.org/someimage:latest", }, }, }, @@ -155,9 +155,9 @@ func TestCatalogdControllerReconcile(t *testing.T) { }, Spec: catalogdv1alpha1.ClusterCatalogSpec{ Source: catalogdv1alpha1.CatalogSource{ - Type: "image", + Type: catalogdv1alpha1.SourceTypeImage, Image: &catalogdv1alpha1.ImageSource{ - Ref: "someimage:latest", + Ref: "my.org/someimage:latest", }, }, }, @@ -186,9 +186,9 @@ func TestCatalogdControllerReconcile(t *testing.T) { }, Spec: catalogdv1alpha1.ClusterCatalogSpec{ Source: catalogdv1alpha1.CatalogSource{ - Type: "image", + Type: catalogdv1alpha1.SourceTypeImage, Image: &catalogdv1alpha1.ImageSource{ - Ref: "someimage:latest", + Ref: "my.org/someimage:latest", }, }, }, @@ -200,9 +200,9 @@ func TestCatalogdControllerReconcile(t *testing.T) { }, Spec: catalogdv1alpha1.ClusterCatalogSpec{ Source: catalogdv1alpha1.CatalogSource{ - Type: "image", + Type: catalogdv1alpha1.SourceTypeImage, Image: &catalogdv1alpha1.ImageSource{ - Ref: "someimage:latest", + Ref: "my.org/someimage:latest", }, }, }, @@ -225,7 +225,7 @@ func TestCatalogdControllerReconcile(t *testing.T) { FS: &fstest.MapFS{}, ResolvedSource: &catalogdv1alpha1.ResolvedCatalogSource{ Image: &catalogdv1alpha1.ResolvedImageSource{ - Ref: "someimage@someSHA256Digest", + Ref: "my.org/someimage@someSHA256Digest", }, }, }, @@ -238,9 +238,9 @@ func TestCatalogdControllerReconcile(t *testing.T) { }, Spec: catalogdv1alpha1.ClusterCatalogSpec{ Source: catalogdv1alpha1.CatalogSource{ - Type: "image", + Type: catalogdv1alpha1.SourceTypeImage, Image: &catalogdv1alpha1.ImageSource{ - Ref: "someimage:latest", + Ref: "my.org/someimage:latest", }, }, }, @@ -252,9 +252,9 @@ func TestCatalogdControllerReconcile(t *testing.T) { }, Spec: catalogdv1alpha1.ClusterCatalogSpec{ Source: catalogdv1alpha1.CatalogSource{ - Type: "image", + Type: catalogdv1alpha1.SourceTypeImage, Image: &catalogdv1alpha1.ImageSource{ - Ref: "someimage:latest", + Ref: "my.org/someimage:latest", }, }, }, @@ -274,7 +274,7 @@ func TestCatalogdControllerReconcile(t *testing.T) { }, ResolvedSource: &catalogdv1alpha1.ResolvedCatalogSource{ Image: &catalogdv1alpha1.ResolvedImageSource{ - Ref: "someimage@someSHA256Digest", + Ref: "my.org/someimage@someSHA256Digest", }, }, }, @@ -299,9 +299,9 @@ func TestCatalogdControllerReconcile(t *testing.T) { }, Spec: catalogdv1alpha1.ClusterCatalogSpec{ Source: catalogdv1alpha1.CatalogSource{ - Type: "image", + Type: catalogdv1alpha1.SourceTypeImage, Image: &catalogdv1alpha1.ImageSource{ - Ref: "someimage:latest", + Ref: "my.org/someimage:latest", }, }, }, @@ -313,9 +313,9 @@ func TestCatalogdControllerReconcile(t *testing.T) { }, Spec: catalogdv1alpha1.ClusterCatalogSpec{ Source: catalogdv1alpha1.CatalogSource{ - Type: "image", + Type: catalogdv1alpha1.SourceTypeImage, Image: &catalogdv1alpha1.ImageSource{ - Ref: "someimage:latest", + Ref: "my.org/someimage:latest", }, }, }, @@ -345,9 +345,9 @@ func TestCatalogdControllerReconcile(t *testing.T) { }, Spec: catalogdv1alpha1.ClusterCatalogSpec{ Source: catalogdv1alpha1.CatalogSource{ - Type: "image", + Type: catalogdv1alpha1.SourceTypeImage, Image: &catalogdv1alpha1.ImageSource{ - Ref: "someimage:latest", + Ref: "my.org/someimage:latest", }, }, }, @@ -359,9 +359,9 @@ func TestCatalogdControllerReconcile(t *testing.T) { }, Spec: catalogdv1alpha1.ClusterCatalogSpec{ Source: catalogdv1alpha1.CatalogSource{ - Type: "image", + Type: catalogdv1alpha1.SourceTypeImage, Image: &catalogdv1alpha1.ImageSource{ - Ref: "someimage:latest", + Ref: "my.org/someimage:latest", }, }, }, @@ -384,23 +384,19 @@ func TestCatalogdControllerReconcile(t *testing.T) { }, Spec: catalogdv1alpha1.ClusterCatalogSpec{ Source: catalogdv1alpha1.CatalogSource{ - Type: "image", + Type: catalogdv1alpha1.SourceTypeImage, Image: &catalogdv1alpha1.ImageSource{ - Ref: "someimage:latest", + Ref: "my.org/someimage:latest", }, }, }, Status: catalogdv1alpha1.ClusterCatalogStatus{ - ContentURL: "URL", - LastUnpacked: metav1.Time{}, - ObservedGeneration: 0, + ContentURL: "URL", + LastUnpacked: metav1.Time{}, ResolvedSource: &catalogdv1alpha1.ResolvedCatalogSource{ - Type: "image", + Type: catalogdv1alpha1.SourceTypeImage, Image: &catalogdv1alpha1.ResolvedImageSource{ - Ref: "", - ResolvedRef: "", - LastPollAttempt: metav1.Time{}, - LastUnpacked: metav1.Time{}, + Ref: "", }, }, Conditions: []metav1.Condition{ @@ -425,9 +421,9 @@ func TestCatalogdControllerReconcile(t *testing.T) { }, Spec: catalogdv1alpha1.ClusterCatalogSpec{ Source: catalogdv1alpha1.CatalogSource{ - Type: "image", + Type: catalogdv1alpha1.SourceTypeImage, Image: &catalogdv1alpha1.ImageSource{ - Ref: "someimage:latest", + Ref: "my.org/someimage:latest", }, }, }, @@ -468,9 +464,9 @@ func TestCatalogdControllerReconcile(t *testing.T) { }, Spec: catalogdv1alpha1.ClusterCatalogSpec{ Source: catalogdv1alpha1.CatalogSource{ - Type: "image", + Type: catalogdv1alpha1.SourceTypeImage, Image: &catalogdv1alpha1.ImageSource{ - Ref: "someimage:latest", + Ref: "my.org/someimage:latest", }, }, }, @@ -498,9 +494,9 @@ func TestCatalogdControllerReconcile(t *testing.T) { }, Spec: catalogdv1alpha1.ClusterCatalogSpec{ Source: catalogdv1alpha1.CatalogSource{ - Type: "image", + Type: catalogdv1alpha1.SourceTypeImage, Image: &catalogdv1alpha1.ImageSource{ - Ref: "someimage:latest", + Ref: "my.org/someimage:latest", }, }, }, @@ -539,9 +535,9 @@ func TestCatalogdControllerReconcile(t *testing.T) { }, Spec: catalogdv1alpha1.ClusterCatalogSpec{ Source: catalogdv1alpha1.CatalogSource{ - Type: "image", + Type: catalogdv1alpha1.SourceTypeImage, Image: &catalogdv1alpha1.ImageSource{ - Ref: "someimage:latest", + Ref: "my.org/someimage:latest", }, }, }, @@ -569,9 +565,9 @@ func TestCatalogdControllerReconcile(t *testing.T) { }, Spec: catalogdv1alpha1.ClusterCatalogSpec{ Source: catalogdv1alpha1.CatalogSource{ - Type: "image", + Type: catalogdv1alpha1.SourceTypeImage, Image: &catalogdv1alpha1.ImageSource{ - Ref: "someimage:latest", + Ref: "my.org/someimage:latest", }, }, }, @@ -640,9 +636,9 @@ func TestPollingRequeue(t *testing.T) { }, Spec: catalogdv1alpha1.ClusterCatalogSpec{ Source: catalogdv1alpha1.CatalogSource{ - Type: "image", + Type: catalogdv1alpha1.SourceTypeImage, Image: &catalogdv1alpha1.ImageSource{ - Ref: "someimage:latest", + Ref: "my.org/someimage:latest", }, }, }, @@ -657,9 +653,9 @@ func TestPollingRequeue(t *testing.T) { }, Spec: catalogdv1alpha1.ClusterCatalogSpec{ Source: catalogdv1alpha1.CatalogSource{ - Type: "image", + Type: catalogdv1alpha1.SourceTypeImage, Image: &catalogdv1alpha1.ImageSource{ - Ref: "someimage:latest", + Ref: "my.org/someimage:latest", PollInterval: &metav1.Duration{Duration: time.Minute * 5}, }, }, @@ -676,7 +672,7 @@ func TestPollingRequeue(t *testing.T) { FS: &fstest.MapFS{}, ResolvedSource: &catalogdv1alpha1.ResolvedCatalogSource{ Image: &catalogdv1alpha1.ResolvedImageSource{ - Ref: "someImage@someSHA256Digest", + Ref: "my.org/someImage@someSHA256Digest", }, }, }}, @@ -697,6 +693,8 @@ func TestPollingRequeue(t *testing.T) { } func TestPollingReconcilerUnpack(t *testing.T) { + oldDigest := "a5d4f4467250074216eb1ba1c36e06a3ab797d81c431427fc2aca97ecaf4e9d8" + newDigest := "f42337e7b85a46d83c94694638e2312e10ca16a03542399a65ba783c94a32b63" for name, tc := range map[string]struct { catalog *catalogdv1alpha1.ClusterCatalog expectedUnpackRun bool @@ -709,9 +707,9 @@ func TestPollingReconcilerUnpack(t *testing.T) { }, Spec: catalogdv1alpha1.ClusterCatalogSpec{ Source: catalogdv1alpha1.CatalogSource{ - Type: "image", + Type: catalogdv1alpha1.SourceTypeImage, Image: &catalogdv1alpha1.ImageSource{ - Ref: "someimage:latest", + Ref: "my.org/someimage:latest", PollInterval: &metav1.Duration{Duration: time.Minute * 5}, }, }, @@ -728,9 +726,9 @@ func TestPollingReconcilerUnpack(t *testing.T) { }, Spec: catalogdv1alpha1.ClusterCatalogSpec{ Source: catalogdv1alpha1.CatalogSource{ - Type: "image", + Type: catalogdv1alpha1.SourceTypeImage, Image: &catalogdv1alpha1.ImageSource{ - Ref: "someimage:latest", + Ref: "my.org/someimage:latest", }, }, }, @@ -738,23 +736,23 @@ func TestPollingReconcilerUnpack(t *testing.T) { ContentURL: "URL", Conditions: []metav1.Condition{ { - Type: catalogdv1alpha1.TypeProgressing, - Status: metav1.ConditionFalse, - Reason: catalogdv1alpha1.ReasonSucceeded, + Type: catalogdv1alpha1.TypeProgressing, + Status: metav1.ConditionFalse, + Reason: catalogdv1alpha1.ReasonSucceeded, + ObservedGeneration: 2, }, { - Type: catalogdv1alpha1.TypeServing, - Status: metav1.ConditionTrue, - Reason: catalogdv1alpha1.ReasonAvailable, + Type: catalogdv1alpha1.TypeServing, + Status: metav1.ConditionTrue, + Reason: catalogdv1alpha1.ReasonAvailable, + ObservedGeneration: 2, }, }, - ObservedGeneration: 2, ResolvedSource: &catalogdv1alpha1.ResolvedCatalogSource{ - Type: "image", + Type: catalogdv1alpha1.SourceTypeImage, Image: &catalogdv1alpha1.ResolvedImageSource{ - Ref: "someimage:latest", - ResolvedRef: "someimage@sha256:asdf123", - LastPollAttempt: metav1.Time{Time: time.Now().Add(-time.Minute * 5)}, + Ref: "my.org/someimage@sha256:" + oldDigest, + LastSuccessfulPollAttempt: metav1.Time{Time: time.Now().Add(-time.Minute * 5)}, }, }, }, @@ -770,9 +768,9 @@ func TestPollingReconcilerUnpack(t *testing.T) { }, Spec: catalogdv1alpha1.ClusterCatalogSpec{ Source: catalogdv1alpha1.CatalogSource{ - Type: "image", + Type: catalogdv1alpha1.SourceTypeImage, Image: &catalogdv1alpha1.ImageSource{ - Ref: "someimage:latest", + Ref: "my.org/someimage:latest", PollInterval: &metav1.Duration{Duration: time.Minute * 7}, }, }, @@ -781,23 +779,23 @@ func TestPollingReconcilerUnpack(t *testing.T) { ContentURL: "URL", Conditions: []metav1.Condition{ { - Type: catalogdv1alpha1.TypeProgressing, - Status: metav1.ConditionFalse, - Reason: catalogdv1alpha1.ReasonSucceeded, + Type: catalogdv1alpha1.TypeProgressing, + Status: metav1.ConditionFalse, + Reason: catalogdv1alpha1.ReasonSucceeded, + ObservedGeneration: 2, }, { - Type: catalogdv1alpha1.TypeServing, - Status: metav1.ConditionTrue, - Reason: catalogdv1alpha1.ReasonAvailable, + Type: catalogdv1alpha1.TypeServing, + Status: metav1.ConditionTrue, + Reason: catalogdv1alpha1.ReasonAvailable, + ObservedGeneration: 2, }, }, - ObservedGeneration: 2, ResolvedSource: &catalogdv1alpha1.ResolvedCatalogSource{ - Type: "image", + Type: catalogdv1alpha1.SourceTypeImage, Image: &catalogdv1alpha1.ResolvedImageSource{ - Ref: "someimage:latest", - ResolvedRef: "someimage@sha256:asdf123", - LastPollAttempt: metav1.Time{Time: time.Now().Add(-time.Minute * 5)}, + Ref: "my.org/someimage@sha256:" + oldDigest, + LastSuccessfulPollAttempt: metav1.Time{Time: time.Now().Add(-time.Minute * 5)}, }, }, }, @@ -813,9 +811,9 @@ func TestPollingReconcilerUnpack(t *testing.T) { }, Spec: catalogdv1alpha1.ClusterCatalogSpec{ Source: catalogdv1alpha1.CatalogSource{ - Type: "image", + Type: catalogdv1alpha1.SourceTypeImage, Image: &catalogdv1alpha1.ImageSource{ - Ref: "someimage:latest", + Ref: "my.org/someimage:latest", PollInterval: &metav1.Duration{Duration: time.Minute * 3}, }, }, @@ -824,30 +822,30 @@ func TestPollingReconcilerUnpack(t *testing.T) { ContentURL: "URL", Conditions: []metav1.Condition{ { - Type: catalogdv1alpha1.TypeProgressing, - Status: metav1.ConditionFalse, - Reason: catalogdv1alpha1.ReasonSucceeded, + Type: catalogdv1alpha1.TypeProgressing, + Status: metav1.ConditionFalse, + Reason: catalogdv1alpha1.ReasonSucceeded, + ObservedGeneration: 2, }, { - Type: catalogdv1alpha1.TypeServing, - Status: metav1.ConditionTrue, - Reason: catalogdv1alpha1.ReasonAvailable, + Type: catalogdv1alpha1.TypeServing, + Status: metav1.ConditionTrue, + Reason: catalogdv1alpha1.ReasonAvailable, + ObservedGeneration: 2, }, }, - ObservedGeneration: 2, ResolvedSource: &catalogdv1alpha1.ResolvedCatalogSource{ - Type: "image", + Type: catalogdv1alpha1.SourceTypeImage, Image: &catalogdv1alpha1.ResolvedImageSource{ - Ref: "someimage:latest", - ResolvedRef: "someimage@sha256:asdf123", - LastPollAttempt: metav1.Time{Time: time.Now().Add(-time.Minute * 5)}, + Ref: "my.org/someimage@sha256:" + oldDigest, + LastSuccessfulPollAttempt: metav1.Time{Time: time.Now().Add(-time.Minute * 5)}, }, }, }, }, expectedUnpackRun: true, }, - "ClusterCatalog not being resolved the first time, pollInterval mentioned, \"now\" is before next expected poll time, spec.image changed, unpack should run": { + "ClusterCatalog not being resolved the first time, pollInterval mentioned, \"now\" is before next expected poll time, generation changed, unpack should run": { catalog: &catalogdv1alpha1.ClusterCatalog{ ObjectMeta: metav1.ObjectMeta{ Name: "test-catalog", @@ -856,9 +854,9 @@ func TestPollingReconcilerUnpack(t *testing.T) { }, Spec: catalogdv1alpha1.ClusterCatalogSpec{ Source: catalogdv1alpha1.CatalogSource{ - Type: "image", + Type: catalogdv1alpha1.SourceTypeImage, Image: &catalogdv1alpha1.ImageSource{ - Ref: "someotherimage:latest", + Ref: "my.org/someotherimage@sha256:" + newDigest, PollInterval: &metav1.Duration{Duration: time.Minute * 7}, }, }, @@ -867,23 +865,23 @@ func TestPollingReconcilerUnpack(t *testing.T) { ContentURL: "URL", Conditions: []metav1.Condition{ { - Type: catalogdv1alpha1.TypeProgressing, - Status: metav1.ConditionFalse, - Reason: catalogdv1alpha1.ReasonSucceeded, + Type: catalogdv1alpha1.TypeProgressing, + Status: metav1.ConditionFalse, + Reason: catalogdv1alpha1.ReasonSucceeded, + ObservedGeneration: 3, }, { - Type: catalogdv1alpha1.TypeServing, - Status: metav1.ConditionTrue, - Reason: catalogdv1alpha1.ReasonAvailable, + Type: catalogdv1alpha1.TypeServing, + Status: metav1.ConditionTrue, + Reason: catalogdv1alpha1.ReasonAvailable, + ObservedGeneration: 2, }, }, - ObservedGeneration: 2, ResolvedSource: &catalogdv1alpha1.ResolvedCatalogSource{ - Type: "image", + Type: catalogdv1alpha1.SourceTypeImage, Image: &catalogdv1alpha1.ResolvedImageSource{ - Ref: "someimage:latest", - ResolvedRef: "someimage@sha256:asdf123", - LastPollAttempt: metav1.Time{Time: time.Now().Add(-time.Minute * 5)}, + Ref: "my.org/someimage@sha256:" + oldDigest, + LastSuccessfulPollAttempt: metav1.Time{Time: time.Now().Add(-time.Minute * 5)}, }, }, }, diff --git a/internal/source/containers_image.go b/internal/source/containers_image.go index aa151694..a95c0a2b 100644 --- a/internal/source/containers_image.go +++ b/internal/source/containers_image.go @@ -71,7 +71,7 @@ func (i *ContainersImageRegistry) Unpack(ctx context.Context, catalog *catalogdv panic(fmt.Sprintf("unexpected file at unpack path %q: expected a directory", unpackPath)) } l.Info("image already unpacked", "ref", imgRef.String(), "digest", canonicalRef.Digest().String()) - return successResult(catalog, unpackPath, canonicalRef, metav1.NewTime(unpackStat.ModTime())), nil + return successResult(unpackPath, canonicalRef, unpackStat.ModTime()), nil } ////////////////////////////////////////////////////// @@ -153,23 +153,22 @@ func (i *ContainersImageRegistry) Unpack(ctx context.Context, catalog *catalogdv return nil, fmt.Errorf("error deleting old images: %w", err) } - return successResult(catalog, unpackPath, canonicalRef, metav1.Now()), nil + return successResult(unpackPath, canonicalRef, time.Now()), nil } -func successResult(catalog *catalogdv1alpha1.ClusterCatalog, unpackPath string, canonicalRef reference.Canonical, lastUnpacked metav1.Time) *Result { +func successResult(unpackPath string, canonicalRef reference.Canonical, lastUnpacked time.Time) *Result { return &Result{ FS: os.DirFS(unpackPath), ResolvedSource: &catalogdv1alpha1.ResolvedCatalogSource{ Type: catalogdv1alpha1.SourceTypeImage, Image: &catalogdv1alpha1.ResolvedImageSource{ - Ref: catalog.Spec.Source.Image.Ref, - ResolvedRef: canonicalRef.String(), - LastPollAttempt: metav1.Time{Time: time.Now()}, - LastUnpacked: lastUnpacked, + Ref: canonicalRef.String(), + LastSuccessfulPollAttempt: metav1.NewTime(time.Now()), }, }, - State: StateUnpacked, - Message: fmt.Sprintf("unpacked %q successfully", canonicalRef), + State: StateUnpacked, + Message: fmt.Sprintf("unpacked %q successfully", canonicalRef), + UnpackTime: lastUnpacked, } } diff --git a/internal/source/containers_image_test.go b/internal/source/containers_image_test.go index 6ed83e02..d35c2c90 100644 --- a/internal/source/containers_image_test.go +++ b/internal/source/containers_image_test.go @@ -382,7 +382,7 @@ func TestImageRegistry(t *testing.T) { rs, err := imgReg.Unpack(ctx, tt.catalog) if !tt.wantErr { require.NoError(t, err) - assert.Equal(t, fmt.Sprintf("%s@sha256:%s", imgName.Context().Name(), digest.Hex), rs.ResolvedSource.Image.ResolvedRef) + assert.Equal(t, fmt.Sprintf("%s@sha256:%s", imgName.Context().Name(), digest.Hex), rs.ResolvedSource.Image.Ref) assert.Equal(t, source.StateUnpacked, rs.State) unpackDir := filepath.Join(testCache, tt.catalog.Name, digest.String()) @@ -396,15 +396,15 @@ func TestImageRegistry(t *testing.T) { // If the digest should already exist check that we actually hit it if tt.digestAlreadyExists { assert.Contains(t, buf.String(), "image already unpacked") - assert.Equal(t, rs.ResolvedSource.Image.LastUnpacked.Time, unpackDirStat.ModTime()) + assert.Equal(t, rs.UnpackTime, unpackDirStat.ModTime()) } else if tt.oldDigestExists { assert.NotContains(t, buf.String(), "image already unpacked") - assert.NotEqual(t, rs.ResolvedSource.Image.LastUnpacked.Time, oldDigestModTime) + assert.NotEqual(t, rs.UnpackTime, oldDigestModTime) assert.NoDirExists(t, oldDigestDir) } else { - require.NotNil(t, rs.ResolvedSource.Image.LastUnpacked) + require.NotNil(t, rs.UnpackTime) require.NotNil(t, rs.ResolvedSource.Image) - assert.False(t, rs.ResolvedSource.Image.LastUnpacked.IsZero()) + assert.False(t, rs.UnpackTime.IsZero()) } } else { assert.Error(t, err) diff --git a/internal/source/unpacker.go b/internal/source/unpacker.go index 270ff41c..db6ffc57 100644 --- a/internal/source/unpacker.go +++ b/internal/source/unpacker.go @@ -3,6 +3,7 @@ package source import ( "context" "io/fs" + "time" catalogdv1alpha1 "github.com/operator-framework/catalogd/api/core/v1alpha1" ) @@ -54,6 +55,9 @@ type Result struct { // Message is contextual information about the progress of unpacking the // catalog content. Message string + + // UnpackTime is the timestamp when the transition to the current State happened + UnpackTime time.Time } type State string diff --git a/test/tools/imageregistry/pre-upgrade-setup.sh b/test/tools/imageregistry/pre-upgrade-setup.sh index be7cd82b..5c2b4ed5 100755 --- a/test/tools/imageregistry/pre-upgrade-setup.sh +++ b/test/tools/imageregistry/pre-upgrade-setup.sh @@ -26,7 +26,7 @@ metadata: name: ${TEST_CLUSTER_CATALOG_NAME} spec: source: - type: image + type: Image image: ref: ${TEST_CLUSTER_CATALOG_IMAGE} EOF