diff --git a/README.md b/README.md index 60714e98..60c66b85 100644 --- a/README.md +++ b/README.md @@ -87,9 +87,14 @@ Procedure steps marked with an asterisk (`*`) are likely to change with future A Conditions: Last Transition Time: 2023-06-23T18:35:13Z Message: - Reason: Unpacking + Reason: Progressing Status: False - Type: Unpacked + Type: Succeeded + Last Transition Time: 2023-06-23T18:35:13Z + Message: + Reason: Serving + Status: True + Type: Available Events: ``` diff --git a/api/core/v1alpha1/clustercatalog_types.go b/api/core/v1alpha1/clustercatalog_types.go index ab9469a6..d3f54402 100644 --- a/api/core/v1alpha1/clustercatalog_types.go +++ b/api/core/v1alpha1/clustercatalog_types.go @@ -26,15 +26,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" ) @@ -43,6 +45,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 is the Schema for the ClusterCatalogs API diff --git a/config/base/crd/bases/olm.operatorframework.io_clustercatalogs.yaml b/config/base/crd/bases/olm.operatorframework.io_clustercatalogs.yaml index 62771fac..70608236 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 d94bd687..08fe818b 100644 --- a/docs/fetching-catalog-contents.md +++ b/docs/fetching-catalog-contents.md @@ -86,10 +86,15 @@ of a catalog can be read from: 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 + message: successfully unpacked and stored content from "quay.io/operatorhubio/catalog@sha256:e53267559addc85227c2a7901ca54b980bc900276fc24d3f4db0549cb38ecf76" + reason: Succeeded + status: "False" + type: Progressing + - lastTransitionTime: "2023-09-14T15:21:18Z" + message: Content from "quay.io/operatorhubio/catalog@sha256:e53267559addc85227c2a7901ca54b980bc900276fc24d3f4db0549cb38ecf76" is being served + reason: Available + status: "True" + type: Serving contentURL: https://catalogd-catalogserver.olmv1-system.svc/catalogs/operatorhubio/all.json resolvedSource: image: diff --git a/hack/scripts/demo-script.sh b/hack/scripts/demo-script.sh index 34495c73..21732fa1 100755 --- a/hack/scripts/demo-script.sh +++ b/hack/scripts/demo-script.sh @@ -19,7 +19,7 @@ kubectl apply -f config/samples/core_v1alpha1_catalog.yaml # shows catalog-sample kubectl get catalog -A # waiting for catalog to report ready status -time kubectl wait --for=condition=Unpacked catalog/operatorhubio --timeout=1m +time kubectl wait --for=condition=Serving catalog/operatorhubio --timeout=1m # port forward the catalogd-catalogserver service to interact with the HTTP server serving catalog contents (kubectl -n olmv1-system port-forward svc/catalogd-catalogserver 8080:80)& diff --git a/hack/scripts/gzip-demo-script.sh b/hack/scripts/gzip-demo-script.sh index 4b92033c..3a3ffdc5 100755 --- a/hack/scripts/gzip-demo-script.sh +++ b/hack/scripts/gzip-demo-script.sh @@ -9,7 +9,7 @@ kubectl apply -f $HOME/devel/tmp/operatorhubio-clustercatalog.yaml # shows catalog kubectl get clustercatalog -A # waiting for clustercatalog to report ready status -time kubectl wait --for=condition=Unpacked clustercatalog/operatorhubio --timeout=1m +time kubectl wait --for=condition=Serving clustercatalog/operatorhubio --timeout=1m # port forward the catalogd-catalogserver service to interact with the HTTP server serving catalog contents (kubectl -n olmv1-system port-forward svc/catalogd-catalogserver 8080:443)& diff --git a/internal/controllers/core/clustercatalog_controller.go b/internal/controllers/core/clustercatalog_controller.go index 926ce55f..7aafcefd 100644 --- a/internal/controllers/core/clustercatalog_controller.go +++ b/internal/controllers/core/clustercatalog_controller.go @@ -124,10 +124,11 @@ 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) + return ctrl.Result{}, updateStatusProgressing(catalog, err) } + updateStatusNotServing(&catalog.Status) if err := r.Unpacker.Cleanup(ctx, catalog); err != nil { - return ctrl.Result{}, updateStatusStorageDeleteError(&catalog.Status, err) + return ctrl.Result{}, updateStatusProgressing(catalog, err) } controllerutil.RemoveFinalizer(catalog, fbcDeletionFinalizer) return ctrl.Result{}, nil @@ -139,16 +140,10 @@ 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)) + return ctrl.Result{}, updateStatusProgressing(catalog, err) } 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 @@ -156,7 +151,7 @@ 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)) + return ctrl.Result{}, updateStatusProgressing(catalog, fmt.Errorf("error storing fbc: %v", err)) } contentURL = r.Storage.ContentURL(catalog.Name) @@ -168,7 +163,8 @@ func (r *ClusterCatalogReconciler) reconcile(ctx context.Context, catalog *v1alp 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 { @@ -180,73 +176,60 @@ 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, - Status: metav1.ConditionFalse, - Reason: v1alpha1.ReasonUnpackPending, - Message: result.Message, - }) -} - -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, - }) +func updateStatusProgressing(catalog *v1alpha1.ClusterCatalog, err error) error { + var unrecov *catalogderrors.Unrecoverable + if err == nil { + catalog.Status.ResolvedSource = nil + meta.SetStatusCondition(&catalog.Status.Conditions, metav1.Condition{ + Type: v1alpha1.TypeProgressing, + Status: metav1.ConditionFalse, + Reason: v1alpha1.ReasonSucceeded, + Message: fmt.Sprintf("Successfully unpacked and stored content from %s", catalog.Spec.Source.Image.Ref), + }) + } else if errors.As(err, &unrecov) { + meta.SetStatusCondition(&catalog.Status.Conditions, metav1.Condition{ + Type: v1alpha1.TypeProgressing, + Status: metav1.ConditionFalse, + Reason: v1alpha1.ReasonUnrecoverable, + Message: err.Error(), + }) + return err + } else { + meta.SetStatusCondition(&catalog.Status.Conditions, metav1.Condition{ + Type: v1alpha1.TypeProgressing, + Status: metav1.ConditionTrue, + Reason: v1alpha1.ReasonRetrying, + Message: err.Error(), + }) + return err + } + return nil } -func updateStatusUnpacked(status *v1alpha1.ClusterCatalogStatus, result *source.Result, contentURL string, generation int64, lastUnpacked metav1.Time) { +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(), - }) - return err -} - -func updateStatusStorageError(status *v1alpha1.ClusterCatalogStatus, err error) error { - status.ResolvedSource = nil - 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()), + Reason: v1alpha1.ReasonAvailable, + Message: fmt.Sprintf("Content from %s is being served", status.ResolvedSource.Image.Ref), }) - return err } -func updateStatusStorageDeleteError(status *v1alpha1.ClusterCatalogStatus, err error) error { +func updateStatusNotServing(status *v1alpha1.ClusterCatalogStatus) { + status.ContentURL = "" 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 { @@ -258,6 +241,9 @@ func (r *ClusterCatalogReconciler) needsUnpacking(catalog *v1alpha1.ClusterCatal if !r.Storage.ContentExists(catalog.Name) { return true } + if !servingStatusConditionExists(catalog) { + return true + } // if there is no spec.Source.Image, don't unpack again if catalog.Spec.Source.Image == nil { return false @@ -278,3 +264,12 @@ func (r *ClusterCatalogReconciler) needsUnpacking(catalog *v1alpha1.ClusterCatal // time to unpack return true } + +func servingStatusConditionExists(catalog *v1alpha1.ClusterCatalog) bool { + for _, cond := range catalog.Status.Conditions { + if cond.Type == v1alpha1.TypeServing && cond.Status == metav1.ConditionTrue { + return true + } + } + return false +} diff --git a/internal/controllers/core/clustercatalog_controller_test.go b/internal/controllers/core/clustercatalog_controller_test.go index ebf7b6d5..5d1c9b55 100644 --- a/internal/controllers/core/clustercatalog_controller_test.go +++ b/internal/controllers/core/clustercatalog_controller_test.go @@ -16,6 +16,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" ) @@ -29,9 +30,16 @@ type MockSource struct { // shouldError determines whether or not the MockSource should return an error when MockSource.Unpack is called shouldError bool + + // shouldErrorUnrecoverble determines whether or not the MockSource should return an error when MockSource.Unpack is called + // is Unrecoverable + shouldErrorUnrecoverable bool } func (ms *MockSource) Unpack(_ context.Context, _ *catalogdv1alpha1.ClusterCatalog) (*source.Result, error) { + if ms.shouldErrorUnrecoverable { + return nil, catalogdv1alpha1Errors.NewUnrecoverable(errors.New("mocksource Unrecoverable error")) + } if ms.shouldError { return nil, errors.New("mocksource error") } @@ -117,107 +125,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, - Status: metav1.ConditionFalse, - Reason: catalogdv1alpha1.ReasonUnpackPending, - }, - }, - }, - }, - }, - { - name: "valid source type, unpack state == Unpacking, unpack state is reflected in status", - source: &MockSource{ - result: &source.Result{State: source.StateUnpacking}, - }, - 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.ReasonUnpacking, + Type: catalogdv1alpha1.TypeProgressing, + Status: metav1.ConditionTrue, + Reason: catalogdv1alpha1.ReasonRetrying, }, }, }, }, }, { - name: "valid source type, unpack state is unknown, unpack state is reflected in status and error is returned", + name: "valid source type, unpack returns error, status updated to reflect error state and error is returned", shouldErr: true, source: &MockSource{ - result: &source.Result{State: "unknown"}, + shouldError: true, }, store: &MockStore{}, catalog: &catalogdv1alpha1.ClusterCatalog{ @@ -250,19 +170,19 @@ func TestCatalogdControllerReconcile(t *testing.T) { 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", + name: "valid source type, unpack returns unrecoverable error, status updated to reflect unrecoverable error state and error is returned", shouldErr: true, source: &MockSource{ - shouldError: true, + shouldErrorUnrecoverable: true, }, store: &MockStore{}, catalog: &catalogdv1alpha1.ClusterCatalog{ @@ -295,20 +215,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{}, @@ -343,9 +268,19 @@ 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", }, }, }, @@ -393,9 +328,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, }, }, }, @@ -434,7 +369,7 @@ func TestCatalogdControllerReconcile(t *testing.T) { }, }, { - name: "storage finalizer set, catalog deletion timestamp is not zero (or nil), finalizer removed", + name: "storage finalizer set, catalog deletion timestamp is not zero (or nil), finalizer removed, serving status condition is set to false", source: &MockSource{}, store: &MockStore{}, catalog: &catalogdv1alpha1.ClusterCatalog{ @@ -451,6 +386,16 @@ func TestCatalogdControllerReconcile(t *testing.T) { }, }, }, + Status: catalogdv1alpha1.ClusterCatalogStatus{ + ContentURL: "URL", + Conditions: []metav1.Condition{ + { + Type: catalogdv1alpha1.TypeServing, + Status: metav1.ConditionTrue, + Reason: catalogdv1alpha1.ReasonAvailable, + }, + }, + }, }, expectedCatalog: &catalogdv1alpha1.ClusterCatalog{ ObjectMeta: metav1.ObjectMeta{ @@ -466,6 +411,16 @@ func TestCatalogdControllerReconcile(t *testing.T) { }, }, }, + Status: catalogdv1alpha1.ClusterCatalogStatus{ + ContentURL: "", + Conditions: []metav1.Condition{ + { + Type: catalogdv1alpha1.TypeServing, + Status: metav1.ConditionFalse, + Reason: catalogdv1alpha1.ReasonUnavailable, + }, + }, + }, }, }, { @@ -507,9 +462,9 @@ func TestCatalogdControllerReconcile(t *testing.T) { Status: catalogdv1alpha1.ClusterCatalogStatus{ Conditions: []metav1.Condition{ { - Type: catalogdv1alpha1.TypeDelete, - Status: metav1.ConditionFalse, - Reason: catalogdv1alpha1.ReasonStorageDeleteFailed, + Type: catalogdv1alpha1.TypeProgressing, + Status: metav1.ConditionTrue, + Reason: catalogdv1alpha1.ReasonRetrying, }, }, }, @@ -535,7 +490,9 @@ func TestCatalogdControllerReconcile(t *testing.T) { } else { assert.Error(t, err) } - diff := cmp.Diff(tt.expectedCatalog, tt.catalog, cmpopts.IgnoreFields(metav1.Condition{}, "Message", "LastTransitionTime")) + 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") }) } @@ -590,6 +547,11 @@ func TestPollingRequeue(t *testing.T) { catalogdv1alpha1.SourceTypeImage: &MockSource{result: &source.Result{ State: source.StateUnpacked, FS: &fstest.MapFS{}, + ResolvedSource: &catalogdv1alpha1.ResolvedCatalogSource{ + Image: &catalogdv1alpha1.ResolvedImageSource{ + Ref: "someImage@someSHA256Digest", + }, + }, }}, }, ), @@ -646,9 +608,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, @@ -684,9 +651,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, @@ -722,9 +694,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, @@ -760,9 +737,52 @@ 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.ReasonAvailable, + }, + }, + ObservedGeneration: 2, + ResolvedSource: &catalogdv1alpha1.ResolvedCatalogSource{ + Type: "image", + Image: &catalogdv1alpha1.ResolvedImageSource{ + Ref: "someimage:latest", + ResolvedRef: "someimage@sha256:asdf123", + LastPollAttempt: metav1.Time{Time: time.Now().Add(-time.Minute * 5)}, + }, + }, + }, + }, + expectedUnpackRun: true, + }, + "ClusterCatalog not being resolved the first time, condition Serving missing, unpack should run": { + catalog: &catalogdv1alpha1.ClusterCatalog{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-catalog", + Finalizers: []string{fbcDeletionFinalizer}, + Generation: 3, + }, + Spec: catalogdv1alpha1.ClusterCatalogSpec{ + Source: catalogdv1alpha1.CatalogSource{ + Type: "image", + Image: &catalogdv1alpha1.ImageSource{ + Ref: "someotherimage:latest", + PollInterval: &metav1.Duration{Duration: time.Minute * 7}, + }, + }, + }, + Status: catalogdv1alpha1.ClusterCatalogStatus{ + ContentURL: "URL", + Conditions: []metav1.Condition{ + { + Type: catalogdv1alpha1.TypeProgressing, Status: metav1.ConditionTrue, - Reason: catalogdv1alpha1.ReasonUnpackSuccessful, + Reason: catalogdv1alpha1.ReasonRetrying, }, }, ObservedGeneration: 2, diff --git a/internal/source/unpacker.go b/internal/source/unpacker.go index f2ca09a2..576540a7 100644 --- a/internal/source/unpacker.go +++ b/internal/source/unpacker.go @@ -61,19 +61,8 @@ 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" type unpacker struct { sources map[catalogdv1alpha1.SourceType]Unpacker 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 d0f0d201..efd28307 100644 --- a/test/e2e/unpack_test.go +++ b/test/e2e/unpack_test.go @@ -70,10 +70,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") @@ -88,6 +88,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/upgrade/unpack_test.go b/test/upgrade/unpack_test.go index 83dcaba4..bec9797f 100644 --- a/test/upgrade/unpack_test.go +++ b/test/upgrade/unpack_test.go @@ -21,14 +21,14 @@ var _ = Describe("ClusterCatalog Unpacking", func() { It("Successfully unpacks catalog contents", func() { ctx := context.Background() 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") @@ -41,6 +41,15 @@ var _ = Describe("ClusterCatalog Unpacking", func() { g.Expect(cmp.Diff(expectedFBC, actualFBC)).To(BeEmpty()) }).Should(Succeed()) + By("Ensuring ClusterCatalog has Status.Condition of Type = Serving with a status == True") + 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()) }) }) })