Skip to content

Commit

Permalink
⚠ Status condition clean up (#379)
Browse files Browse the repository at this point in the history 
* ⚠ Status condition clean up

[RFC: ClusterCatalog Status Conditions](https://docs.google.com/document/d/1Kg8ovX8d25OqGa5utwcKbX3nN3obBl2KIYJHYz6IlKU/edit) implementation.

Closes [363](#363)
Closes [364](#364)
Closes [365](#365)

Co-authored-by: Ankita Thomas <ankithom@redhat.com>
Co-authored-by: Anik Bhattacharjee <anbhatta@redhat.com>

* Handle new finalizer code

Signed-off-by: Todd Short <todd.short@me.com>

* Set StatusUpdated to true for finalizers

Signed-off-by: Todd Short <todd.short@me.com>

---------

Signed-off-by: Todd Short <todd.short@me.com>
Co-authored-by: Ankita Thomas <ankithom@redhat.com>
Co-authored-by: Todd Short <todd.short@me.com>
  • Loading branch information
@anik120 @ankitathomas @tmshort
3 people committed Sep 20, 2024
1 parent e1d3605 commit 2dc04d9
Show file tree
Hide file tree
Showing 10 changed files with 325 additions and 267 deletions.
74 changes: 32 additions & 42 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,58 +40,48 @@ Procedure steps marked with an asterisk (`*`) are likely to change with future A
```sh
Name: operatorhubio
Namespace:
Labels: <none>
Labels: olm.operatorframework.io/metadata.name=operatorhubio
Annotations: <none>
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: <none>
```
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-service.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: <none>
```
1. Port forward the `catalogd-service` service in the `olmv1-system` namespace:
```sh
Expand Down
19 changes: 11 additions & 8 deletions api/core/v1alpha1/clustercatalog_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
ReasonTerminal = "Terminal"

MetadataNameLabel = "olm.operatorframework.io/metadata.name"
)
Expand All @@ -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.
Expand Down
3 changes: 3 additions & 0 deletions config/base/crd/bases/olm.operatorframework.io_clustercatalogs.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 3 additions & 7 deletions docs/fetching-catalog-contents.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
117 changes: 46 additions & 71 deletions internal/controllers/core/clustercatalog_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"
crfinalizer "sigs.k8s.io/controller-runtime/pkg/finalizer"
"sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/reconcile"

"github.com/operator-framework/catalogd/api/core/v1alpha1"
"github.com/operator-framework/catalogd/internal/source"
Expand Down Expand Up @@ -124,13 +125,6 @@ func (r *ClusterCatalogReconciler) SetupWithManager(mgr ctrl.Manager) error {
func (r *ClusterCatalogReconciler) reconcile(ctx context.Context, catalog *v1alpha1.ClusterCatalog) (ctrl.Result, error) {
finalizeResult, err := r.Finalizers.Finalize(ctx, catalog)
if err != nil {
// TODO: For now, this error handling follows the pattern of other error handling.
// Namely: zero just about everything out, throw our hands up, and return an error.
// This is not ideal, and we should consider a more nuanced approach that resolves
// as much status as possible before returning, or at least keeps previous state if
// it is properly labeled with its observed generation.
_ = updateStatusStorageError(&catalog.Status, err)
_ = updateStatusUnpackFailing(&catalog.Status, err)
return ctrl.Result{}, err
}
if finalizeResult.Updated || finalizeResult.StatusUpdated {
Expand All @@ -145,36 +139,33 @@ 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
// as the already unpacked content. If it does, we should skip this rest
// 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)

var lastUnpacked metav1.Time

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 {
Expand All @@ -186,73 +177,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.Is(err, reconcile.TerminalError(nil)) {
progressingCond.Status = metav1.ConditionFalse
progressingCond.Reason = v1alpha1.ReasonTerminal
}

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 {
Expand Down Expand Up @@ -308,12 +280,15 @@ func NewFinalizers(localStorage storage.Instance, unpacker source.Unpacker) (crf
panic("could not convert object to clusterCatalog")
}
if err := localStorage.Delete(catalog.Name); err != nil {
return crfinalizer.Result{}, updateStatusStorageDeleteError(&catalog.Status, err)
updateStatusProgressing(catalog, err)
return crfinalizer.Result{StatusUpdated: true}, err
}
updateStatusNotServing(&catalog.Status)
if err := unpacker.Cleanup(ctx, catalog); err != nil {
return crfinalizer.Result{}, updateStatusStorageDeleteError(&catalog.Status, err)
updateStatusProgressing(catalog, err)
return crfinalizer.Result{StatusUpdated: true}, err
}
return crfinalizer.Result{}, nil
return crfinalizer.Result{StatusUpdated: true}, nil
}))
if err != nil {
return f, err
Expand Down
Loading

0 comments on commit 2dc04d9

Please sign in to comment.