(techdebt): refactor catalog controller unit tests (#196)

* (techdebt): refactor catalog controller unit tests to no longer use Ginkgo and instead use the native Go testing and testify Signed-off-by: Bryce Palmer <everettraven@gmail.com> * remove rebase detritus, unnecessary IIFE, and featuregate comments/blocks. goimports. Signed-off-by: Bryce Palmer <everettraven@gmail.com> --------- Signed-off-by: Bryce Palmer <everettraven@gmail.com>
operator-framework · Oct 11, 2023 · 9f91fc5 · 9f91fc5
1 parent 9f3ba06
commit 9f91fc5
Show file tree

Hide file tree

Showing 6 changed files with 506 additions and 488 deletions.
diff --git a/cmd/manager/main.go b/cmd/manager/main.go
@@ -140,38 +140,36 @@ func main() {
 	}
 
 	var localStorage storage.Instance
-	if features.CatalogdFeatureGate.Enabled(features.HTTPServer) {
-		metrics.Registry.MustRegister(catalogdmetrics.RequestDurationMetric)
+	metrics.Registry.MustRegister(catalogdmetrics.RequestDurationMetric)
 
-		storeDir := filepath.Join(cacheDir, storageDir)
-		if err := os.MkdirAll(storeDir, 0700); err != nil {
-			setupLog.Error(err, "unable to create storage directory for catalogs")
-			os.Exit(1)
-		}
+	storeDir := filepath.Join(cacheDir, storageDir)
+	if err := os.MkdirAll(storeDir, 0700); err != nil {
+		setupLog.Error(err, "unable to create storage directory for catalogs")
+		os.Exit(1)
+	}
 
-		baseStorageURL, err := url.Parse(fmt.Sprintf("%s/catalogs/", httpExternalAddr))
-		if err != nil {
-			setupLog.Error(err, "unable to create base storage URL")
-			os.Exit(1)
-		}
+	baseStorageURL, err := url.Parse(fmt.Sprintf("%s/catalogs/", httpExternalAddr))
+	if err != nil {
+		setupLog.Error(err, "unable to create base storage URL")
+		os.Exit(1)
+	}
 
-		localStorage = storage.LocalDir{RootDir: storeDir, BaseURL: baseStorageURL}
-		shutdownTimeout := 30 * time.Second
-		catalogServer := server.Server{
-			Kind: "catalogs",
-			Server: &http.Server{
-				Addr:         catalogServerAddr,
-				Handler:      catalogdmetrics.AddMetricsToHandler(localStorage.StorageServerHandler()),
-				ReadTimeout:  5 * time.Second,
-				WriteTimeout: 10 * time.Second,
-			},
-			ShutdownTimeout: &shutdownTimeout,
-		}
+	localStorage = storage.LocalDir{RootDir: storeDir, BaseURL: baseStorageURL}
+	shutdownTimeout := 30 * time.Second
+	catalogServer := server.Server{
+		Kind: "catalogs",
+		Server: &http.Server{
+			Addr:         catalogServerAddr,
+			Handler:      catalogdmetrics.AddMetricsToHandler(localStorage.StorageServerHandler()),
+			ReadTimeout:  5 * time.Second,
+			WriteTimeout: 10 * time.Second,
+		},
+		ShutdownTimeout: &shutdownTimeout,
+	}
 
-		if err := mgr.Add(&catalogServer); err != nil {
-			setupLog.Error(err, "unable to start catalog server")
-			os.Exit(1)
-		}
+	if err := mgr.Add(&catalogServer); err != nil {
+		setupLog.Error(err, "unable to start catalog server")
+		os.Exit(1)
 	}
 
 	if err = (&corecontrollers.CatalogReconciler{

diff --git a/config/manager/manager.yaml b/config/manager/manager.yaml
@@ -74,7 +74,6 @@ spec:
         args:
         - --leader-elect
         - --metrics-bind-address=127.0.0.1:8080
-        - --feature-gates=HTTPServer=true
         - --http-external-address=http://catalogd-catalogserver.catalogd-system.svc
         image: controller:latest
         name: manager

diff --git a/pkg/controllers/core/catalog_controller.go b/pkg/controllers/core/catalog_controller.go
@@ -34,7 +34,6 @@ import (
 	"github.com/operator-framework/catalogd/api/core/v1alpha1"
 	"github.com/operator-framework/catalogd/internal/source"
 	catalogderrors "github.com/operator-framework/catalogd/pkg/errors"
-	"github.com/operator-framework/catalogd/pkg/features"
 	"github.com/operator-framework/catalogd/pkg/storage"
 )
 
@@ -112,11 +111,11 @@ func (r *CatalogReconciler) SetupWithManager(mgr ctrl.Manager) error {
 // linting from the linter that was fussing about this.
 // nolint:unparam
 func (r *CatalogReconciler) reconcile(ctx context.Context, catalog *v1alpha1.Catalog) (ctrl.Result, error) {
-	if features.CatalogdFeatureGate.Enabled(features.HTTPServer) && catalog.DeletionTimestamp.IsZero() && !controllerutil.ContainsFinalizer(catalog, fbcDeletionFinalizer) {
+	if catalog.DeletionTimestamp.IsZero() && !controllerutil.ContainsFinalizer(catalog, fbcDeletionFinalizer) {
 		controllerutil.AddFinalizer(catalog, fbcDeletionFinalizer)
 		return ctrl.Result{}, nil
 	}
-	if features.CatalogdFeatureGate.Enabled(features.HTTPServer) && !catalog.DeletionTimestamp.IsZero() {
+	if !catalog.DeletionTimestamp.IsZero() {
 		if err := r.Storage.Delete(catalog.Name); err != nil {
 			return ctrl.Result{}, updateStatusStorageDeleteError(&catalog.Status, err)
 		}
@@ -143,13 +142,11 @@ func (r *CatalogReconciler) reconcile(ctx context.Context, catalog *v1alpha1.Cat
 		// 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.
-		if features.CatalogdFeatureGate.Enabled(features.HTTPServer) {
-			err := r.Storage.Store(catalog.Name, unpackResult.FS)
-			if err != nil {
-				return ctrl.Result{}, updateStatusStorageError(&catalog.Status, fmt.Errorf("error storing fbc: %v", err))
-			}
-			contentURL = r.Storage.ContentURL(catalog.Name)
+		err := r.Storage.Store(catalog.Name, unpackResult.FS)
+		if err != nil {
+			return ctrl.Result{}, updateStatusStorageError(&catalog.Status, fmt.Errorf("error storing fbc: %v", err))
 		}
+		contentURL = r.Storage.ContentURL(catalog.Name)
 
 		updateStatusUnpacked(&catalog.Status, unpackResult, contentURL)
 		return ctrl.Result{}, nil