From d320249c3bf485420716b96c9e823ddb536eab25 Mon Sep 17 00:00:00 2001 From: Lalatendu Mohanty Date: Thu, 17 Oct 2024 08:31:57 -0400 Subject: [PATCH] :sparkles: Adding Availability enum to catalog spec (#421) * [WIP] Adding Availability enum to catalog spec Signed-off-by: Lalatendu Mohanty * Removing the finalizer after the catalog is disabled Signed-off-by: Lalatendu Mohanty * Adding unit tests Signed-off-by: Lalatendu Mohanty --------- Signed-off-by: Lalatendu Mohanty --- api/core/v1alpha1/clustercatalog_types.go | 18 +++ cmd/manager/main.go | 1 + ....operatorframework.io_clustercatalogs.yaml | 15 ++ .../core/clustercatalog_controller.go | 60 +++++-- .../core/clustercatalog_controller_test.go | 152 ++++++++++++++++++ 5 files changed, 234 insertions(+), 12 deletions(-) diff --git a/api/core/v1alpha1/clustercatalog_types.go b/api/core/v1alpha1/clustercatalog_types.go index ecc88403..c33f3d50 100644 --- a/api/core/v1alpha1/clustercatalog_types.go +++ b/api/core/v1alpha1/clustercatalog_types.go @@ -33,6 +33,7 @@ const ( // Serving reasons ReasonAvailable = "Available" ReasonUnavailable = "Unavailable" + ReasonDisabled = "Disabled" // Progressing reasons ReasonSucceeded = "Succeeded" @@ -40,6 +41,9 @@ const ( ReasonBlocked = "Blocked" MetadataNameLabel = "olm.operatorframework.io/metadata.name" + + AvailabilityEnabled = "Enabled" + AvailabilityDisabled = "Disabled" ) //+kubebuilder:object:root=true @@ -92,6 +96,20 @@ type ClusterCatalogSpec struct { // +kubebuilder:default:=0 // +optional Priority int32 `json:"priority"` + + // Availability is an optional field that allows users to define whether the ClusterCatalog is utilized by the operator-controller. + // + // Allowed values are : ["Enabled", "Disabled"]. + // If set to "Enabled", the catalog will be used for updates, serving contents, and package installations. + // + // If set to "Disabled", catalogd will stop serving the catalog and the cached data will be removed. + // + // If unspecified, the default value is "Enabled" + // + // +kubebuilder:validation:Enum="Disabled";"Enabled" + // +kubebuilder:default="Enabled" + // +optional + Availability string `json:"availability,omitempty"` } // ClusterCatalogStatus defines the observed state of ClusterCatalog diff --git a/cmd/manager/main.go b/cmd/manager/main.go index a2d0175a..efdafe79 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -260,6 +260,7 @@ func main() { localStorage = storage.LocalDir{RootDir: storeDir, BaseURL: baseStorageURL} + // Config for the the catalogd web server catalogServerConfig := serverutil.CatalogServerConfig{ ExternalAddr: externalAddr, CatalogAddr: catalogServerAddr, diff --git a/config/base/crd/bases/olm.operatorframework.io_clustercatalogs.yaml b/config/base/crd/bases/olm.operatorframework.io_clustercatalogs.yaml index 67a0d774..6588a2aa 100644 --- a/config/base/crd/bases/olm.operatorframework.io_clustercatalogs.yaml +++ b/config/base/crd/bases/olm.operatorframework.io_clustercatalogs.yaml @@ -51,6 +51,21 @@ spec: spec: description: ClusterCatalogSpec defines the desired state of ClusterCatalog properties: + availability: + default: Enabled + description: |- + Availability is an optional field that allows users to define whether the ClusterCatalog is utilized by the operator-controller. + + Allowed values are : ["Enabled", "Disabled"]. + If set to "Enabled", the catalog will be used for updates, serving contents, and package installations. + + If set to "Disabled", catalogd will stop serving the catalog and the cached data will be removed. + + If unspecified, the default value is "Enabled" + enum: + - Disabled + - Enabled + type: string priority: default: 0 description: |- diff --git a/internal/controllers/core/clustercatalog_controller.go b/internal/controllers/core/clustercatalog_controller.go index a8ad497c..38f5293b 100644 --- a/internal/controllers/core/clustercatalog_controller.go +++ b/internal/controllers/core/clustercatalog_controller.go @@ -31,6 +31,7 @@ import ( "k8s.io/apimachinery/pkg/util/wait" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" crfinalizer "sigs.k8s.io/controller-runtime/pkg/finalizer" "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/reconcile" @@ -88,6 +89,7 @@ func (r *ClusterCatalogReconciler) Reconcile(ctx context.Context, req ctrl.Reque if err := r.Client.Get(ctx, req.NamespacedName, &existingCatsrc); err != nil { return ctrl.Result{}, client.IgnoreNotFound(err) } + reconciledCatsrc := existingCatsrc.DeepCopy() res, reconcileErr := r.reconcile(ctx, reconciledCatsrc) @@ -156,6 +158,25 @@ func (r *ClusterCatalogReconciler) SetupWithManager(mgr ctrl.Manager) error { // nolint:unparam func (r *ClusterCatalogReconciler) reconcile(ctx context.Context, catalog *v1alpha1.ClusterCatalog) (ctrl.Result, error) { l := log.FromContext(ctx) + // Check if the catalog availability is set to disabled, if true then + // unset content URL, delete it from the cache and set appropriate status + if catalog.Spec.Availability == v1alpha1.AvailabilityDisabled { + // Delete the catalog from local cache + err := r.deleteCatalogCache(ctx, catalog) + if err != nil { + return ctrl.Result{}, err + } + + // Set status.conditions[type=Progressing] to False as we are done with + // all that needs to be done with the catalog + updateStatusCatalogDisabled(&catalog.Status, catalog.GetGeneration()) + + // Remove the fbcDeletionFinalizer as we do not want a finalizer attached to the catalog + // when it is disabled. Because the finalizer serves no purpose now. + controllerutil.RemoveFinalizer(catalog, fbcDeletionFinalizer) + + return ctrl.Result{}, nil + } finalizeResult, err := r.finalizers.Finalize(ctx, catalog) if err != nil { @@ -315,6 +336,17 @@ func updateStatusServing(status *v1alpha1.ClusterCatalogStatus, result source.Re }) } +func updateStatusCatalogDisabled(status *v1alpha1.ClusterCatalogStatus, generation int64) { + progressingCond := metav1.Condition{ + Type: v1alpha1.TypeProgressing, + Status: metav1.ConditionFalse, + Reason: v1alpha1.ReasonDisabled, + Message: "Catalog availability is set to Disabled", + ObservedGeneration: generation, + } + meta.SetStatusCondition(&status.Conditions, progressingCond) +} + func updateStatusNotServing(status *v1alpha1.ClusterCatalogStatus, generation int64) { status.ResolvedSource = nil status.ContentURL = "" @@ -358,18 +390,8 @@ func (r *ClusterCatalogReconciler) setupFinalizers() error { if !ok { panic("could not convert object to clusterCatalog") } - if err := r.Storage.Delete(catalog.Name); err != nil { - updateStatusProgressing(&catalog.Status, catalog.GetGeneration(), err) - return crfinalizer.Result{StatusUpdated: true}, err - } - updateStatusNotServing(&catalog.Status, catalog.GetGeneration()) - if err := r.Unpacker.Cleanup(ctx, catalog); err != nil { - updateStatusProgressing(&catalog.Status, catalog.GetGeneration(), err) - return crfinalizer.Result{StatusUpdated: true}, err - } - - r.deleteStoredCatalog(catalog.Name) - return crfinalizer.Result{StatusUpdated: true}, nil + err := r.deleteCatalogCache(ctx, catalog) + return crfinalizer.Result{StatusUpdated: true}, err })) if err != nil { return err @@ -383,3 +405,17 @@ func (r *ClusterCatalogReconciler) deleteStoredCatalog(catalogName string) { defer r.storedCatalogsMu.Unlock() delete(r.storedCatalogs, catalogName) } + +func (r *ClusterCatalogReconciler) deleteCatalogCache(ctx context.Context, catalog *v1alpha1.ClusterCatalog) error { + if err := r.Storage.Delete(catalog.Name); err != nil { + updateStatusProgressing(&catalog.Status, catalog.GetGeneration(), err) + return err + } + updateStatusNotServing(&catalog.Status, catalog.GetGeneration()) + if err := r.Unpacker.Cleanup(ctx, catalog); err != nil { + updateStatusProgressing(&catalog.Status, catalog.GetGeneration(), err) + return err + } + r.deleteStoredCatalog(catalog.Name) + return nil +} diff --git a/internal/controllers/core/clustercatalog_controller_test.go b/internal/controllers/core/clustercatalog_controller_test.go index 347fb8c6..938b08b2 100644 --- a/internal/controllers/core/clustercatalog_controller_test.go +++ b/internal/controllers/core/clustercatalog_controller_test.go @@ -589,6 +589,158 @@ func TestCatalogdControllerReconcile(t *testing.T) { }, }, }, + { + name: "catalog availability set to disabled, ContentURL should get unset", + source: &MockSource{ + result: &source.Result{ + State: source.StateUnpacked, + FS: &fstest.MapFS{}, + }, + }, + store: &MockStore{}, + catalog: &catalogdv1alpha1.ClusterCatalog{ + ObjectMeta: metav1.ObjectMeta{ + Name: "catalog", + }, + Spec: catalogdv1alpha1.ClusterCatalogSpec{ + Source: catalogdv1alpha1.CatalogSource{ + Type: catalogdv1alpha1.SourceTypeImage, + Image: &catalogdv1alpha1.ImageSource{ + Ref: "my.org/someimage:latest", + }, + }, + Availability: "Disabled", + }, + Status: catalogdv1alpha1.ClusterCatalogStatus{ + ContentURL: "URL", + LastUnpacked: metav1.Time{}, + ResolvedSource: &catalogdv1alpha1.ResolvedCatalogSource{ + Type: catalogdv1alpha1.SourceTypeImage, + Image: &catalogdv1alpha1.ResolvedImageSource{ + Ref: "", + }, + }, + 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{ + Name: "catalog", + }, + Spec: catalogdv1alpha1.ClusterCatalogSpec{ + Source: catalogdv1alpha1.CatalogSource{ + Type: catalogdv1alpha1.SourceTypeImage, + Image: &catalogdv1alpha1.ImageSource{ + Ref: "my.org/someimage:latest", + }, + }, + Availability: "Disabled", + }, + Status: catalogdv1alpha1.ClusterCatalogStatus{ + ContentURL: "", + Conditions: []metav1.Condition{ + { + Type: catalogdv1alpha1.TypeServing, + Status: metav1.ConditionFalse, + Reason: catalogdv1alpha1.ReasonUnavailable, + }, + { + Type: catalogdv1alpha1.TypeProgressing, + Status: metav1.ConditionFalse, + Reason: catalogdv1alpha1.ReasonDisabled, + }, + }, + }, + }, + }, + { + name: "catalog availability set to disabled, finalizer should get removed", + source: &MockSource{ + result: &source.Result{ + State: source.StateUnpacked, + FS: &fstest.MapFS{}, + }, + }, + store: &MockStore{}, + catalog: &catalogdv1alpha1.ClusterCatalog{ + ObjectMeta: metav1.ObjectMeta{ + Name: "catalog", + Finalizers: []string{fbcDeletionFinalizer}, + }, + Spec: catalogdv1alpha1.ClusterCatalogSpec{ + Source: catalogdv1alpha1.CatalogSource{ + Type: catalogdv1alpha1.SourceTypeImage, + Image: &catalogdv1alpha1.ImageSource{ + Ref: "my.org/someimage:latest", + }, + }, + Availability: "Disabled", + }, + Status: catalogdv1alpha1.ClusterCatalogStatus{ + ContentURL: "URL", + LastUnpacked: metav1.Time{}, + ResolvedSource: &catalogdv1alpha1.ResolvedCatalogSource{ + Type: catalogdv1alpha1.SourceTypeImage, + Image: &catalogdv1alpha1.ResolvedImageSource{ + Ref: "", + }, + }, + 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{ + Name: "catalog", + Finalizers: []string{}, + }, + Spec: catalogdv1alpha1.ClusterCatalogSpec{ + Source: catalogdv1alpha1.CatalogSource{ + Type: catalogdv1alpha1.SourceTypeImage, + Image: &catalogdv1alpha1.ImageSource{ + Ref: "my.org/someimage:latest", + }, + }, + Availability: "Disabled", + }, + Status: catalogdv1alpha1.ClusterCatalogStatus{ + ContentURL: "", + Conditions: []metav1.Condition{ + { + Type: catalogdv1alpha1.TypeServing, + Status: metav1.ConditionFalse, + Reason: catalogdv1alpha1.ReasonUnavailable, + }, + { + Type: catalogdv1alpha1.TypeProgressing, + Status: metav1.ConditionFalse, + Reason: catalogdv1alpha1.ReasonDisabled, + }, + }, + }, + }, + }, } { t.Run(tt.name, func(t *testing.T) { reconciler := &ClusterCatalogReconciler{