From d902ad38b141c5b2b55d95d25fd1abe165e36923 Mon Sep 17 00:00:00 2001 From: Anik Bhattacharjee Date: Tue, 17 Sep 2024 11:31:27 +0530 Subject: [PATCH] =?UTF-8?q?=E2=9A=A0=20Status=20condition=20clean=20up?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [RFC: ClusterCatalog Status Conditions](https://docs.google.com/document/d/1Kg8ovX8d25OqGa5utwcKbX3nN3obBl2KIYJHYz6IlKU/edit) implementation. Closes [363](operator-framework#363) Closes [364](operator-framework#364) Closes [365](operator-framework#365) Co-authored-by: Ankita Thomas Co-authored-by: Anik Bhattacharjee --- README.md | 74 ++--- api/core/v1alpha1/clustercatalog_types.go | 19 +- ....operatorframework.io_clustercatalogs.yaml | 3 + docs/fetching-catalog-contents.md | 10 +- .../core/clustercatalog_controller.go | 107 +++---- .../core/clustercatalog_controller_test.go | 291 +++++++++--------- internal/source/unpacker.go | 15 +- scripts/install.tpl.sh | 2 +- test/e2e/unpack_test.go | 16 +- test/tools/imageregistry/pre-upgrade-setup.sh | 2 +- test/upgrade/unpack_test.go | 17 +- 11 files changed, 273 insertions(+), 283 deletions(-) diff --git a/README.md b/README.md index 4ac7c639..ff1fbb56 100644 --- a/README.md +++ b/README.md @@ -40,58 +40,48 @@ Procedure steps marked with an asterisk (`*`) are likely to change with future A ```sh Name: operatorhubio Namespace: - Labels: + Labels: olm.operatorframework.io/metadata.name=operatorhubio Annotations: API Version: olm.operatorframework.io/v1alpha1 Kind: ClusterCatalog Metadata: - Creation Timestamp: 2023-06-23T18:35:13Z - Generation: 1 - Managed Fields: - API Version: olm.operatorframework.io/v1alpha1 - Fields Type: FieldsV1 - fieldsV1: - f:metadata: - f:annotations: - .: - f:kubectl.kubernetes.io/last-applied-configuration: - f:spec: - .: - f:source: - .: - f:image: - .: - f:ref: - f:type: - Manager: kubectl-client-side-apply - Operation: Update - Time: 2023-06-23T18:35:13Z - API Version: olm.operatorframework.io/v1alpha1 - Fields Type: FieldsV1 - fieldsV1: - f:status: - .: - f:conditions: - Manager: manager - Operation: Update - Subresource: status - Time: 2023-06-23T18:35:43Z - Resource Version: 1397 - UID: 709cee9d-c669-46e1-97d0-e97dcce8f388 + Creation Timestamp: 2024-09-12T13:37:04Z + Finalizers: + olm.operatorframework.io/delete-server-cache + Generation: 1 + Resource Version: 961 + UID: fa6bb9cf-1a36-4189-a7a0-83284c3f6f55 Spec: + Priority: 0 Source: Image: - Ref: quay.io/operatorhubio/catalog:latest - Type: image + Poll Interval: 10m0s + Ref: quay.io/operatorhubio/catalog:latest + Type: image Status: Conditions: - Last Transition Time: 2023-06-23T18:35:13Z - Message: - Reason: Unpacking + Last Transition Time: 2024-09-12T13:37:53Z + Message: Successfully unpacked and stored content from quay.io/operatorhubio/catalog:latest + Reason: Succeeded Status: False - Type: Unpacked - Events: - ``` + Type: Progressing + Last Transition Time: 2024-09-12T13:37:53Z + Message: Content from quay.io/operatorhubio/catalog:latest is being served + Reason: Available + Status: True + Type: Serving + Content URL: https://catalogd-catalogserver.olmv1-system.svc/catalogs/operatorhubio/all.json + Last Unpacked: 2024-09-12T13:37:52Z + Observed Generation: 1 + Resolved Source: + Image: + Last Poll Attempt: 2024-09-12T13:37:52Z + Last Unpacked: 2024-09-12T13:37:52Z + Ref: quay.io/operatorhubio/catalog:latest + Resolved Ref: quay.io/operatorhubio/catalog@sha256:4453a361198d39d0390fd8c1a7f07b5a5a3ae1e8dac9979ef0c4eba46299df16 + Type: image + Events: + ``` 1. Port forward the `catalogd-service` service in the `olmv1-system` namespace: ```sh diff --git a/api/core/v1alpha1/clustercatalog_types.go b/api/core/v1alpha1/clustercatalog_types.go index d22b8523..24993c32 100644 --- a/api/core/v1alpha1/clustercatalog_types.go +++ b/api/core/v1alpha1/clustercatalog_types.go @@ -27,15 +27,17 @@ type SourceType string const ( SourceTypeImage SourceType = "image" - TypeUnpacked = "Unpacked" - TypeDelete = "Delete" + TypeProgressing = "Progressing" + TypeServing = "Serving" - ReasonUnpackPending = "UnpackPending" - ReasonUnpacking = "Unpacking" - ReasonUnpackSuccessful = "UnpackSuccessful" - ReasonUnpackFailed = "UnpackFailed" - ReasonStorageFailed = "FailedToStore" - ReasonStorageDeleteFailed = "FailedToDelete" + // Serving reasons + ReasonAvailable = "Available" + ReasonUnavailable = "Unavailable" + + // Progressing reasons + ReasonSucceeded = "Succeeded" + ReasonRetrying = "Retrying" + ReasonUnrecoverable = "Unrecoverable" MetadataNameLabel = "olm.operatorframework.io/metadata.name" ) @@ -44,6 +46,7 @@ const ( //+kubebuilder:resource:scope=Cluster //+kubebuilder:subresource:status //+kubebuilder:printcolumn:name=LastUnpacked,type=date,JSONPath=`.status.lastUnpacked` +//+kubebuilder:printcolumn:name="Serving",type=string,JSONPath=`.status.conditions[?(@.type=="Serving")].status` //+kubebuilder:printcolumn:name=Age,type=date,JSONPath=`.metadata.creationTimestamp` // ClusterCatalog enables users to make File-Based Catalog (FBC) catalog data available to the cluster. diff --git a/config/base/crd/bases/olm.operatorframework.io_clustercatalogs.yaml b/config/base/crd/bases/olm.operatorframework.io_clustercatalogs.yaml index 41b42cb6..0aa06136 100644 --- a/config/base/crd/bases/olm.operatorframework.io_clustercatalogs.yaml +++ b/config/base/crd/bases/olm.operatorframework.io_clustercatalogs.yaml @@ -18,6 +18,9 @@ spec: - jsonPath: .status.lastUnpacked name: LastUnpacked type: date + - jsonPath: .status.conditions[?(@.type=="Serving")].status + name: Serving + type: string - jsonPath: .metadata.creationTimestamp name: Age type: date diff --git a/docs/fetching-catalog-contents.md b/docs/fetching-catalog-contents.md index 0cc95f92..a016878e 100644 --- a/docs/fetching-catalog-contents.md +++ b/docs/fetching-catalog-contents.md @@ -84,13 +84,9 @@ of a catalog can be read from: ```yaml status: - conditions: - - lastTransitionTime: "2023-09-14T15:21:18Z" - message: successfully unpacked the catalog image "quay.io/operatorhubio/catalog@sha256:e53267559addc85227c2a7901ca54b980bc900276fc24d3f4db0549cb38ecf76" - reason: UnpackSuccessful - status: "True" - type: Unpacked - contentURL: https://catalogd-service.olmv1-system.svc/catalogs/operatorhubio/all.json + . + . + contentURL: https://catalogd-catalogserver.olmv1-system.svc/catalogs/operatorhubio/all.json resolvedSource: image: ref: quay.io/operatorhubio/catalog@sha256:e53267559addc85227c2a7901ca54b980bc900276fc24d3f4db0549cb38ecf76 diff --git a/internal/controllers/core/clustercatalog_controller.go b/internal/controllers/core/clustercatalog_controller.go index 09b20ade..e59b3fec 100644 --- a/internal/controllers/core/clustercatalog_controller.go +++ b/internal/controllers/core/clustercatalog_controller.go @@ -122,10 +122,13 @@ func (r *ClusterCatalogReconciler) reconcile(ctx context.Context, catalog *v1alp } if !catalog.DeletionTimestamp.IsZero() { if err := r.Storage.Delete(catalog.Name); err != nil { - return ctrl.Result{}, updateStatusStorageDeleteError(&catalog.Status, err) + updateStatusProgressing(catalog, err) + return ctrl.Result{}, err } + updateStatusNotServing(&catalog.Status) if err := r.Unpacker.Cleanup(ctx, catalog); err != nil { - return ctrl.Result{}, updateStatusStorageDeleteError(&catalog.Status, err) + updateStatusProgressing(catalog, err) + return ctrl.Result{}, err } controllerutil.RemoveFinalizer(catalog, fbcDeletionFinalizer) return ctrl.Result{}, nil @@ -137,16 +140,12 @@ func (r *ClusterCatalogReconciler) reconcile(ctx context.Context, catalog *v1alp unpackResult, err := r.Unpacker.Unpack(ctx, catalog) if err != nil { - return ctrl.Result{}, updateStatusUnpackFailing(&catalog.Status, fmt.Errorf("source bundle content: %v", err)) + unpackErr := fmt.Errorf("source bundle content: %w", err) + updateStatusProgressing(catalog, unpackErr) + return ctrl.Result{}, unpackErr } switch unpackResult.State { - case source.StatePending: - updateStatusUnpackPending(&catalog.Status, unpackResult) - return ctrl.Result{}, nil - case source.StateUnpacking: - updateStatusUnpacking(&catalog.Status, unpackResult) - return ctrl.Result{}, nil case source.StateUnpacked: contentURL := "" // TODO: We should check to see if the unpacked result has the same content @@ -154,7 +153,9 @@ func (r *ClusterCatalogReconciler) reconcile(ctx context.Context, catalog *v1alp // of the unpacking steps. err := r.Storage.Store(ctx, catalog.Name, unpackResult.FS) if err != nil { - return ctrl.Result{}, updateStatusStorageError(&catalog.Status, fmt.Errorf("error storing fbc: %v", err)) + storageErr := fmt.Errorf("error storing fbc: %v", err) + updateStatusProgressing(catalog, storageErr) + return ctrl.Result{}, storageErr } contentURL = r.Storage.ContentURL(catalog.Name) @@ -162,11 +163,10 @@ func (r *ClusterCatalogReconciler) reconcile(ctx context.Context, catalog *v1alp if unpackResult != nil && unpackResult.ResolvedSource != nil && unpackResult.ResolvedSource.Image != nil { lastUnpacked = unpackResult.ResolvedSource.Image.LastUnpacked - } else { - lastUnpacked = metav1.Time{} } - updateStatusUnpacked(&catalog.Status, unpackResult, contentURL, catalog.Generation, lastUnpacked) + updateStatusProgressing(catalog, nil) + updateStatusServing(&catalog.Status, unpackResult, contentURL, catalog.Generation, lastUnpacked) var requeueAfter time.Duration switch catalog.Spec.Source.Type { @@ -178,73 +178,54 @@ func (r *ClusterCatalogReconciler) reconcile(ctx context.Context, catalog *v1alp return ctrl.Result{RequeueAfter: requeueAfter}, nil default: - return ctrl.Result{}, updateStatusUnpackFailing(&catalog.Status, fmt.Errorf("unknown unpack state %q: %v", unpackResult.State, err)) + panic(fmt.Sprintf("unknown unpack state %q", unpackResult.State)) } } -func updateStatusUnpackPending(status *v1alpha1.ClusterCatalogStatus, result *source.Result) { - status.ResolvedSource = nil - meta.SetStatusCondition(&status.Conditions, metav1.Condition{ - Type: v1alpha1.TypeUnpacked, +func updateStatusProgressing(catalog *v1alpha1.ClusterCatalog, err error) { + progressingCond := metav1.Condition{ + Type: v1alpha1.TypeProgressing, Status: metav1.ConditionFalse, - Reason: v1alpha1.ReasonUnpackPending, - Message: result.Message, - }) -} + Reason: v1alpha1.ReasonSucceeded, + Message: "Successfully unpacked and stored content from resolved source", + } -func updateStatusUnpacking(status *v1alpha1.ClusterCatalogStatus, result *source.Result) { - status.ResolvedSource = nil - meta.SetStatusCondition(&status.Conditions, metav1.Condition{ - Type: v1alpha1.TypeUnpacked, - Status: metav1.ConditionFalse, - Reason: v1alpha1.ReasonUnpacking, - Message: result.Message, - }) -} + if err != nil { + progressingCond.Status = metav1.ConditionTrue + progressingCond.Reason = v1alpha1.ReasonRetrying + progressingCond.Message = err.Error() + } -func updateStatusUnpacked(status *v1alpha1.ClusterCatalogStatus, result *source.Result, contentURL string, generation int64, lastUnpacked metav1.Time) { + if errors.As(err, &catalogderrors.Unrecoverable{}) { + progressingCond.Status = metav1.ConditionFalse + progressingCond.Reason = v1alpha1.ReasonUnrecoverable + } + + meta.SetStatusCondition(&catalog.Status.Conditions, progressingCond) +} +func updateStatusServing(status *v1alpha1.ClusterCatalogStatus, result *source.Result, contentURL string, generation int64, unpackedAt metav1.Time) { status.ResolvedSource = result.ResolvedSource status.ContentURL = contentURL status.ObservedGeneration = generation - status.LastUnpacked = lastUnpacked + status.LastUnpacked = unpackedAt meta.SetStatusCondition(&status.Conditions, metav1.Condition{ - Type: v1alpha1.TypeUnpacked, + Type: v1alpha1.TypeServing, Status: metav1.ConditionTrue, - Reason: v1alpha1.ReasonUnpackSuccessful, - Message: result.Message, - }) -} - -func updateStatusUnpackFailing(status *v1alpha1.ClusterCatalogStatus, err error) error { - status.ResolvedSource = nil - meta.SetStatusCondition(&status.Conditions, metav1.Condition{ - Type: v1alpha1.TypeUnpacked, - Status: metav1.ConditionFalse, - Reason: v1alpha1.ReasonUnpackFailed, - Message: err.Error(), + Reason: v1alpha1.ReasonAvailable, + Message: "Serving desired content from resolved source", }) - return err } -func updateStatusStorageError(status *v1alpha1.ClusterCatalogStatus, err error) error { +func updateStatusNotServing(status *v1alpha1.ClusterCatalogStatus) { status.ResolvedSource = nil + status.ContentURL = "" + status.ObservedGeneration = 0 + status.LastUnpacked = metav1.Time{} meta.SetStatusCondition(&status.Conditions, metav1.Condition{ - Type: v1alpha1.TypeUnpacked, - Status: metav1.ConditionFalse, - Reason: v1alpha1.ReasonStorageFailed, - Message: fmt.Sprintf("failed to store bundle: %s", err.Error()), - }) - return err -} - -func updateStatusStorageDeleteError(status *v1alpha1.ClusterCatalogStatus, err error) error { - meta.SetStatusCondition(&status.Conditions, metav1.Condition{ - Type: v1alpha1.TypeDelete, - Status: metav1.ConditionFalse, - Reason: v1alpha1.ReasonStorageDeleteFailed, - Message: fmt.Sprintf("failed to delete storage: %s", err.Error()), + Type: v1alpha1.TypeServing, + Status: metav1.ConditionFalse, + Reason: v1alpha1.ReasonUnavailable, }) - return err } func (r *ClusterCatalogReconciler) needsUnpacking(catalog *v1alpha1.ClusterCatalog) bool { diff --git a/internal/controllers/core/clustercatalog_controller_test.go b/internal/controllers/core/clustercatalog_controller_test.go index c5edb742..cffcba74 100644 --- a/internal/controllers/core/clustercatalog_controller_test.go +++ b/internal/controllers/core/clustercatalog_controller_test.go @@ -3,6 +3,7 @@ package core import ( "context" "errors" + "fmt" "io/fs" "net/http" "testing" @@ -16,6 +17,7 @@ import ( ctrl "sigs.k8s.io/controller-runtime" catalogdv1alpha1 "github.com/operator-framework/catalogd/api/core/v1alpha1" + catalogdv1alpha1Errors "github.com/operator-framework/catalogd/internal/errors" "github.com/operator-framework/catalogd/internal/source" "github.com/operator-framework/catalogd/internal/storage" ) @@ -27,24 +29,20 @@ type MockSource struct { // result is the result that should be returned when MockSource.Unpack is called result *source.Result - // shouldError determines whether or not the MockSource should return an error when MockSource.Unpack is called - shouldError bool + // error is the error to be returned when MockSource.Unpack is called + error error } func (ms *MockSource) Unpack(_ context.Context, _ *catalogdv1alpha1.ClusterCatalog) (*source.Result, error) { - if ms.shouldError { - return nil, errors.New("mocksource error") + if ms.error != nil { + return nil, ms.error } return ms.result, nil } func (ms *MockSource) Cleanup(_ context.Context, _ *catalogdv1alpha1.ClusterCatalog) error { - if ms.shouldError { - return errors.New("mocksource error") - } - - return nil + return ms.error } var _ storage.Instance = &MockStore{} @@ -83,7 +81,7 @@ func TestCatalogdControllerReconcile(t *testing.T) { for _, tt := range []struct { name string catalog *catalogdv1alpha1.ClusterCatalog - shouldErr bool + expectedError error shouldPanic bool expectedCatalog *catalogdv1alpha1.ClusterCatalog source source.Unpacker @@ -118,62 +116,19 @@ func TestCatalogdControllerReconcile(t *testing.T) { Status: catalogdv1alpha1.ClusterCatalogStatus{ Conditions: []metav1.Condition{ { - Type: catalogdv1alpha1.TypeUnpacked, - Status: metav1.ConditionFalse, - Reason: catalogdv1alpha1.ReasonUnpackFailed, - }, - }, - }, - }, - }, - { - name: "valid source type, unpack state == Pending, unpack state is reflected in status", - source: &MockSource{ - result: &source.Result{State: source.StatePending}, - }, - store: &MockStore{}, - catalog: &catalogdv1alpha1.ClusterCatalog{ - ObjectMeta: metav1.ObjectMeta{ - Name: "catalog", - Finalizers: []string{fbcDeletionFinalizer}, - }, - Spec: catalogdv1alpha1.ClusterCatalogSpec{ - Source: catalogdv1alpha1.CatalogSource{ - Type: "image", - Image: &catalogdv1alpha1.ImageSource{ - Ref: "someimage:latest", - }, - }, - }, - }, - expectedCatalog: &catalogdv1alpha1.ClusterCatalog{ - ObjectMeta: metav1.ObjectMeta{ - Name: "catalog", - Finalizers: []string{fbcDeletionFinalizer}, - }, - Spec: catalogdv1alpha1.ClusterCatalogSpec{ - Source: catalogdv1alpha1.CatalogSource{ - Type: "image", - Image: &catalogdv1alpha1.ImageSource{ - Ref: "someimage:latest", - }, - }, - }, - Status: catalogdv1alpha1.ClusterCatalogStatus{ - Conditions: []metav1.Condition{ - { - Type: catalogdv1alpha1.TypeUnpacked, + Type: catalogdv1alpha1.TypeProgressing, Status: metav1.ConditionFalse, - Reason: catalogdv1alpha1.ReasonUnpackPending, + Reason: catalogdv1alpha1.ReasonUnrecoverable, }, }, }, }, }, { - name: "valid source type, unpack state == Unpacking, unpack state is reflected in status", + name: "valid source type, unpack returns error, status updated to reflect error state and error is returned", + expectedError: fmt.Errorf("source bundle content: %w", fmt.Errorf("mocksource error")), source: &MockSource{ - result: &source.Result{State: source.StateUnpacking}, + error: errors.New("mocksource error"), }, store: &MockStore{}, catalog: &catalogdv1alpha1.ClusterCatalog{ @@ -206,64 +161,19 @@ func TestCatalogdControllerReconcile(t *testing.T) { Status: catalogdv1alpha1.ClusterCatalogStatus{ Conditions: []metav1.Condition{ { - Type: catalogdv1alpha1.TypeUnpacked, - Status: metav1.ConditionFalse, - Reason: catalogdv1alpha1.ReasonUnpacking, - }, - }, - }, - }, - }, - { - name: "valid source type, unpack state is unknown, unpack state is reflected in status and error is returned", - shouldErr: true, - source: &MockSource{ - result: &source.Result{State: "unknown"}, - }, - store: &MockStore{}, - catalog: &catalogdv1alpha1.ClusterCatalog{ - ObjectMeta: metav1.ObjectMeta{ - Name: "catalog", - Finalizers: []string{fbcDeletionFinalizer}, - }, - Spec: catalogdv1alpha1.ClusterCatalogSpec{ - Source: catalogdv1alpha1.CatalogSource{ - Type: "image", - Image: &catalogdv1alpha1.ImageSource{ - Ref: "someimage:latest", - }, - }, - }, - }, - expectedCatalog: &catalogdv1alpha1.ClusterCatalog{ - ObjectMeta: metav1.ObjectMeta{ - Name: "catalog", - Finalizers: []string{fbcDeletionFinalizer}, - }, - Spec: catalogdv1alpha1.ClusterCatalogSpec{ - Source: catalogdv1alpha1.CatalogSource{ - Type: "image", - Image: &catalogdv1alpha1.ImageSource{ - Ref: "someimage:latest", - }, - }, - }, - Status: catalogdv1alpha1.ClusterCatalogStatus{ - Conditions: []metav1.Condition{ - { - Type: catalogdv1alpha1.TypeUnpacked, - Status: metav1.ConditionFalse, - Reason: catalogdv1alpha1.ReasonUnpackFailed, + Type: catalogdv1alpha1.TypeProgressing, + Status: metav1.ConditionTrue, + Reason: catalogdv1alpha1.ReasonRetrying, }, }, }, }, }, { - name: "valid source type, unpack returns error, status updated to reflect error state and error is returned", - shouldErr: true, + name: "valid source type, unpack returns unrecoverable error, status updated to reflect unrecoverable error state and error is returned", + expectedError: fmt.Errorf("source bundle content: %w", catalogdv1alpha1Errors.NewUnrecoverable(fmt.Errorf("mocksource Unrecoverable error"))), source: &MockSource{ - shouldError: true, + error: catalogdv1alpha1Errors.NewUnrecoverable(errors.New("mocksource Unrecoverable error")), }, store: &MockStore{}, catalog: &catalogdv1alpha1.ClusterCatalog{ @@ -296,20 +206,25 @@ func TestCatalogdControllerReconcile(t *testing.T) { Status: catalogdv1alpha1.ClusterCatalogStatus{ Conditions: []metav1.Condition{ { - Type: catalogdv1alpha1.TypeUnpacked, + Type: catalogdv1alpha1.TypeProgressing, Status: metav1.ConditionFalse, - Reason: catalogdv1alpha1.ReasonUnpackFailed, + Reason: catalogdv1alpha1.ReasonUnrecoverable, }, }, }, }, }, { - name: "valid source type, unpack state == Unpacked, unpack state is reflected in status", + name: "valid source type, unpack state == Unpacked, should reflect in status that it's not progressing anymore, and is serving", source: &MockSource{ result: &source.Result{ State: source.StateUnpacked, FS: &fstest.MapFS{}, + ResolvedSource: &catalogdv1alpha1.ResolvedCatalogSource{ + Image: &catalogdv1alpha1.ResolvedImageSource{ + Ref: "someimage@someSHA256Digest", + }, + }, }, }, store: &MockStore{}, @@ -344,17 +259,27 @@ func TestCatalogdControllerReconcile(t *testing.T) { ContentURL: "URL", Conditions: []metav1.Condition{ { - Type: catalogdv1alpha1.TypeUnpacked, + Type: catalogdv1alpha1.TypeServing, Status: metav1.ConditionTrue, - Reason: catalogdv1alpha1.ReasonUnpackSuccessful, + Reason: catalogdv1alpha1.ReasonAvailable, + }, + { + Type: catalogdv1alpha1.TypeProgressing, + Status: metav1.ConditionFalse, + Reason: catalogdv1alpha1.ReasonSucceeded, + }, + }, + ResolvedSource: &catalogdv1alpha1.ResolvedCatalogSource{ + Image: &catalogdv1alpha1.ResolvedImageSource{ + Ref: "someimage@someSHA256Digest", }, }, }, }, }, { - name: "valid source type, unpack state == Unpacked, storage fails, failure reflected in status and error returned", - shouldErr: true, + name: "valid source type, unpack state == Unpacked, storage fails, failure reflected in status and error returned", + expectedError: fmt.Errorf("error storing fbc: mockstore store error"), source: &MockSource{ result: &source.Result{ State: source.StateUnpacked, @@ -394,9 +319,9 @@ func TestCatalogdControllerReconcile(t *testing.T) { Status: catalogdv1alpha1.ClusterCatalogStatus{ Conditions: []metav1.Condition{ { - Type: catalogdv1alpha1.TypeUnpacked, - Status: metav1.ConditionFalse, - Reason: catalogdv1alpha1.ReasonStorageFailed, + Type: catalogdv1alpha1.TypeProgressing, + Status: metav1.ConditionTrue, + Reason: catalogdv1alpha1.ReasonRetrying, }, }, }, @@ -452,6 +377,32 @@ func TestCatalogdControllerReconcile(t *testing.T) { }, }, }, + Status: catalogdv1alpha1.ClusterCatalogStatus{ + ContentURL: "URL", + LastUnpacked: metav1.Time{}, + ObservedGeneration: 0, + ResolvedSource: &catalogdv1alpha1.ResolvedCatalogSource{ + Type: "image", + Image: &catalogdv1alpha1.ResolvedImageSource{ + Ref: "", + ResolvedRef: "", + LastPollAttempt: metav1.Time{}, + LastUnpacked: metav1.Time{}, + }, + }, + Conditions: []metav1.Condition{ + { + Type: catalogdv1alpha1.TypeServing, + Status: metav1.ConditionTrue, + Reason: catalogdv1alpha1.ReasonAvailable, + }, + { + Type: catalogdv1alpha1.TypeProgressing, + Status: metav1.ConditionFalse, + Reason: catalogdv1alpha1.ReasonSucceeded, + }, + }, + }, }, expectedCatalog: &catalogdv1alpha1.ClusterCatalog{ ObjectMeta: metav1.ObjectMeta{ @@ -467,12 +418,27 @@ func TestCatalogdControllerReconcile(t *testing.T) { }, }, }, + Status: catalogdv1alpha1.ClusterCatalogStatus{ + ContentURL: "", + Conditions: []metav1.Condition{ + { + Type: catalogdv1alpha1.TypeServing, + Status: metav1.ConditionFalse, + Reason: catalogdv1alpha1.ReasonUnavailable, + }, + { + Type: catalogdv1alpha1.TypeProgressing, + Status: metav1.ConditionFalse, + Reason: catalogdv1alpha1.ReasonSucceeded, + }, + }, + }, }, }, { - name: "storage finalizer set, catalog deletion timestamp is not zero (or nil), storage delete failed, error returned and finalizer not removed", - shouldErr: true, - source: &MockSource{}, + name: "storage finalizer set, catalog deletion timestamp is not zero (or nil), storage delete failed, error returned, finalizer not removed and catalog continues serving", + expectedError: fmt.Errorf("mockstore delete error"), + source: &MockSource{}, store: &MockStore{ shouldError: true, }, @@ -490,6 +456,21 @@ func TestCatalogdControllerReconcile(t *testing.T) { }, }, }, + Status: catalogdv1alpha1.ClusterCatalogStatus{ + ContentURL: "URL", + Conditions: []metav1.Condition{ + { + Type: catalogdv1alpha1.TypeProgressing, + Status: metav1.ConditionFalse, + Reason: catalogdv1alpha1.ReasonSucceeded, + }, + { + Type: catalogdv1alpha1.TypeServing, + Status: metav1.ConditionTrue, + Reason: catalogdv1alpha1.ReasonAvailable, + }, + }, + }, }, expectedCatalog: &catalogdv1alpha1.ClusterCatalog{ ObjectMeta: metav1.ObjectMeta{ @@ -506,11 +487,17 @@ func TestCatalogdControllerReconcile(t *testing.T) { }, }, Status: catalogdv1alpha1.ClusterCatalogStatus{ + ContentURL: "URL", Conditions: []metav1.Condition{ { - Type: catalogdv1alpha1.TypeDelete, - Status: metav1.ConditionFalse, - Reason: catalogdv1alpha1.ReasonStorageDeleteFailed, + Type: catalogdv1alpha1.TypeProgressing, + Status: metav1.ConditionTrue, + Reason: catalogdv1alpha1.ReasonRetrying, + }, + { + Type: catalogdv1alpha1.TypeServing, + Status: metav1.ConditionTrue, + Reason: catalogdv1alpha1.ReasonAvailable, }, }, }, @@ -532,13 +519,10 @@ func TestCatalogdControllerReconcile(t *testing.T) { res, err := reconciler.reconcile(ctx, tt.catalog) assert.Equal(t, ctrl.Result{}, res) - - if !tt.shouldErr { - assert.NoError(t, err) - } else { - assert.Error(t, err) - } - diff := cmp.Diff(tt.expectedCatalog, tt.catalog, cmpopts.IgnoreFields(metav1.Condition{}, "Message", "LastTransitionTime")) + assert.Equal(t, tt.expectedError, err) + diff := cmp.Diff(tt.expectedCatalog, tt.catalog, + cmpopts.IgnoreFields(metav1.Condition{}, "Message", "LastTransitionTime"), + cmpopts.SortSlices(func(a, b metav1.Condition) bool { return a.Type < b.Type })) assert.Empty(t, diff, "comparing the expected Catalog") }) } @@ -591,6 +575,11 @@ func TestPollingRequeue(t *testing.T) { Unpacker: &MockSource{result: &source.Result{ State: source.StateUnpacked, FS: &fstest.MapFS{}, + ResolvedSource: &catalogdv1alpha1.ResolvedCatalogSource{ + Image: &catalogdv1alpha1.ResolvedImageSource{ + Ref: "someImage@someSHA256Digest", + }, + }, }}, Storage: &MockStore{}, } @@ -645,9 +634,14 @@ func TestPollingReconcilerUnpack(t *testing.T) { ContentURL: "URL", Conditions: []metav1.Condition{ { - Type: catalogdv1alpha1.TypeUnpacked, + Type: catalogdv1alpha1.TypeProgressing, + Status: metav1.ConditionFalse, + Reason: catalogdv1alpha1.ReasonSucceeded, + }, + { + Type: catalogdv1alpha1.TypeServing, Status: metav1.ConditionTrue, - Reason: catalogdv1alpha1.ReasonUnpackSuccessful, + Reason: catalogdv1alpha1.ReasonAvailable, }, }, ObservedGeneration: 2, @@ -683,9 +677,14 @@ func TestPollingReconcilerUnpack(t *testing.T) { ContentURL: "URL", Conditions: []metav1.Condition{ { - Type: catalogdv1alpha1.TypeUnpacked, + Type: catalogdv1alpha1.TypeProgressing, + Status: metav1.ConditionFalse, + Reason: catalogdv1alpha1.ReasonSucceeded, + }, + { + Type: catalogdv1alpha1.TypeServing, Status: metav1.ConditionTrue, - Reason: catalogdv1alpha1.ReasonUnpackSuccessful, + Reason: catalogdv1alpha1.ReasonAvailable, }, }, ObservedGeneration: 2, @@ -721,9 +720,14 @@ func TestPollingReconcilerUnpack(t *testing.T) { ContentURL: "URL", Conditions: []metav1.Condition{ { - Type: catalogdv1alpha1.TypeUnpacked, + Type: catalogdv1alpha1.TypeProgressing, + Status: metav1.ConditionFalse, + Reason: catalogdv1alpha1.ReasonSucceeded, + }, + { + Type: catalogdv1alpha1.TypeServing, Status: metav1.ConditionTrue, - Reason: catalogdv1alpha1.ReasonUnpackSuccessful, + Reason: catalogdv1alpha1.ReasonAvailable, }, }, ObservedGeneration: 2, @@ -759,9 +763,14 @@ func TestPollingReconcilerUnpack(t *testing.T) { ContentURL: "URL", Conditions: []metav1.Condition{ { - Type: catalogdv1alpha1.TypeUnpacked, + Type: catalogdv1alpha1.TypeProgressing, + Status: metav1.ConditionFalse, + Reason: catalogdv1alpha1.ReasonSucceeded, + }, + { + Type: catalogdv1alpha1.TypeServing, Status: metav1.ConditionTrue, - Reason: catalogdv1alpha1.ReasonUnpackSuccessful, + Reason: catalogdv1alpha1.ReasonAvailable, }, }, ObservedGeneration: 2, @@ -781,7 +790,7 @@ func TestPollingReconcilerUnpack(t *testing.T) { t.Run(name, func(t *testing.T) { reconciler := &ClusterCatalogReconciler{ Client: nil, - Unpacker: &MockSource{shouldError: true}, + Unpacker: &MockSource{error: errors.New("mocksource error")}, Storage: &MockStore{}, } _, err := reconciler.reconcile(context.Background(), tc.catalog) diff --git a/internal/source/unpacker.go b/internal/source/unpacker.go index 6115a21c..270ff41c 100644 --- a/internal/source/unpacker.go +++ b/internal/source/unpacker.go @@ -58,18 +58,7 @@ type Result struct { type State string -const ( - // StatePending conveys that a request for unpacking a catalog has been - // acknowledged, but not yet started. - StatePending State = "Pending" - - // StateUnpacking conveys that the source is currently unpacking a catalog. - // This state should be used when the catalog contents are being downloaded - // and processed. - StateUnpacking State = "Unpacking" - - // StateUnpacked conveys that the catalog has been successfully unpacked. - StateUnpacked State = "Unpacked" -) +// StateUnpacked conveys that the catalog has been successfully unpacked. +const StateUnpacked State = "Unpacked" const UnpackCacheDir = "unpack" diff --git a/scripts/install.tpl.sh b/scripts/install.tpl.sh index 7beb8663..d2dd6a80 100644 --- a/scripts/install.tpl.sh +++ b/scripts/install.tpl.sh @@ -39,4 +39,4 @@ kubectl apply -f "${catalogd_manifest}" kubectl_wait "olmv1-system" "deployment/catalogd-controller-manager" "60s" kubectl apply -f "${default_catalogs}" -kubectl wait --for=condition=Unpacked "clustercatalog/operatorhubio" --timeout="60s" \ No newline at end of file +kubectl wait --for=condition=Serving "clustercatalog/operatorhubio" --timeout="60s" \ No newline at end of file diff --git a/test/e2e/unpack_test.go b/test/e2e/unpack_test.go index 337d390f..d1d9745e 100644 --- a/test/e2e/unpack_test.go +++ b/test/e2e/unpack_test.go @@ -69,10 +69,10 @@ var _ = Describe("ClusterCatalog Unpacking", func() { Eventually(func(g Gomega) { err := c.Get(ctx, types.NamespacedName{Name: catalog.Name}, catalog) g.Expect(err).ToNot(HaveOccurred()) - cond := meta.FindStatusCondition(catalog.Status.Conditions, catalogd.TypeUnpacked) + cond := meta.FindStatusCondition(catalog.Status.Conditions, catalogd.TypeProgressing) g.Expect(cond).ToNot(BeNil()) - g.Expect(cond.Status).To(Equal(metav1.ConditionTrue)) - g.Expect(cond.Reason).To(Equal(catalogd.ReasonUnpackSuccessful)) + g.Expect(cond.Status).To(Equal(metav1.ConditionFalse)) + g.Expect(cond.Reason).To(Equal(catalogd.ReasonSucceeded)) }).Should(Succeed()) By("Checking that it has an appropriate name label") @@ -87,6 +87,16 @@ var _ = Describe("ClusterCatalog Unpacking", func() { expectedFBC, err := os.ReadFile("../../testdata/catalogs/test-catalog/expected_all.json") Expect(err).To(Not(HaveOccurred())) Expect(cmp.Diff(expectedFBC, actualFBC)).To(BeEmpty()) + + By("Ensuring ClusterCatalog has Status.Condition of Type = Serving with a status == True") + Eventually(func(g Gomega) { + err := c.Get(ctx, types.NamespacedName{Name: catalog.Name}, catalog) + g.Expect(err).ToNot(HaveOccurred()) + cond := meta.FindStatusCondition(catalog.Status.Conditions, catalogd.TypeServing) + g.Expect(cond).ToNot(BeNil()) + g.Expect(cond.Status).To(Equal(metav1.ConditionTrue)) + g.Expect(cond.Reason).To(Equal(catalogd.ReasonAvailable)) + }).Should(Succeed()) }) AfterEach(func() { Expect(c.Delete(ctx, catalog)).To(Succeed()) diff --git a/test/tools/imageregistry/pre-upgrade-setup.sh b/test/tools/imageregistry/pre-upgrade-setup.sh index a3b5f356..ad718017 100755 --- a/test/tools/imageregistry/pre-upgrade-setup.sh +++ b/test/tools/imageregistry/pre-upgrade-setup.sh @@ -31,4 +31,4 @@ spec: ref: ${TEST_CLUSTER_CATALOG_IMAGE} EOF -kubectl wait --for=condition=Unpacked --timeout=60s ClusterCatalog $TEST_CLUSTER_CATALOG_NAME +kubectl wait --for=condition=Serving=true --timeout=60s ClusterCatalog $TEST_CLUSTER_CATALOG_NAME diff --git a/test/upgrade/unpack_test.go b/test/upgrade/unpack_test.go index 92974d6a..b0300d8c 100644 --- a/test/upgrade/unpack_test.go +++ b/test/upgrade/unpack_test.go @@ -66,14 +66,14 @@ var _ = Describe("ClusterCatalog Unpacking", func() { Expect(found).To(BeTrue()) catalog := &catalogd.ClusterCatalog{} - By("Ensuring ClusterCatalog has Status.Condition of Unpacked with a status == True") + By("Ensuring ClusterCatalog has Status.Condition of Progressing with a status == False, reason == Succeeded") Eventually(func(g Gomega) { err := c.Get(ctx, types.NamespacedName{Name: testClusterCatalogName}, catalog) g.Expect(err).ToNot(HaveOccurred()) - cond := meta.FindStatusCondition(catalog.Status.Conditions, catalogd.TypeUnpacked) + cond := meta.FindStatusCondition(catalog.Status.Conditions, catalogd.TypeProgressing) g.Expect(cond).ToNot(BeNil()) - g.Expect(cond.Status).To(Equal(metav1.ConditionTrue)) - g.Expect(cond.Reason).To(Equal(catalogd.ReasonUnpackSuccessful)) + g.Expect(cond.Status).To(Equal(metav1.ConditionFalse)) + g.Expect(cond.Reason).To(Equal(catalogd.ReasonSucceeded)) }).Should(Succeed()) expectedFBC, err := os.ReadFile("../../testdata/catalogs/test-catalog/expected_all.json") @@ -86,6 +86,15 @@ var _ = Describe("ClusterCatalog Unpacking", func() { g.Expect(cmp.Diff(expectedFBC, actualFBC)).To(BeEmpty()) }).Should(Succeed()) + By("Ensuring ClusterCatalog has Status.Condition of Serving with a status == True, reason == Available") + Eventually(func(g Gomega) { + err := c.Get(ctx, types.NamespacedName{Name: testClusterCatalogName}, catalog) + g.Expect(err).ToNot(HaveOccurred()) + cond := meta.FindStatusCondition(catalog.Status.Conditions, catalogd.TypeServing) + g.Expect(cond).ToNot(BeNil()) + g.Expect(cond.Status).To(Equal(metav1.ConditionTrue)) + g.Expect(cond.Reason).To(Equal(catalogd.ReasonAvailable)) + }).Should(Succeed()) }) }) })