Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: (feature): Update controller to use Storage interface #129

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions cmd/manager/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import (
"github.com/spf13/pflag"

"github.com/operator-framework/catalogd/internal/source"
"github.com/operator-framework/catalogd/internal/storage"
"github.com/operator-framework/catalogd/internal/version"
corecontrollers "github.com/operator-framework/catalogd/pkg/controllers/core"
"github.com/operator-framework/catalogd/pkg/features"
Expand Down Expand Up @@ -116,9 +117,12 @@ func main() {
os.Exit(1)
}

store := &storage.TempStorage{}

if err = (&corecontrollers.CatalogReconciler{
Client: mgr.GetClient(),
Unpacker: unpacker,
Storage: store,
}).SetupWithManager(mgr); err != nil {
setupLog.Error(err, "unable to create controller", "controller", "Catalog")
os.Exit(1)
Expand All @@ -134,6 +138,13 @@ func main() {
os.Exit(1)
}

if features.CatalogdFeatureGate.Enabled(features.HttpServer) {
if err := mgr.AddMetricsExtraHandler("/storage/", store); err != nil {
setupLog.Error(err, "unable to set up storage")
os.Exit(1)
}
}

if profiling {
pprofer := profile.NewPprofer()
if err := pprofer.ConfigureControllerManager(mgr); err != nil {
Expand Down
2 changes: 1 addition & 1 deletion config/default/manager_auth_proxy_patch.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -50,5 +50,5 @@ spec:
- "--health-probe-bind-address=:8081"
- "--metrics-bind-address=127.0.0.1:8080"
- "--leader-elect"
- "--feature-gates=PackagesBundleMetadataAPIs=true"
- "--feature-gates=HttpServer=true"

46 changes: 46 additions & 0 deletions internal/storage/storage.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
package storage

import (
"context"
"io/fs"
"net/http"

"sigs.k8s.io/controller-runtime/pkg/client"
)

type Storage interface {
Loader
Storer
}

type Loader interface {
Load(ctx context.Context, owner client.Object) (fs.FS, error)
}

type Storer interface {
Store(ctx context.Context, owner client.Object, bundle fs.FS) error
Delete(ctx context.Context, owner client.Object) error

http.Handler
URLFor(ctx context.Context, owner client.Object) (string, error)
}

type fallbackLoaderStorage struct {
Storage
fallbackLoader Loader
}

func WithFallbackLoader(s Storage, fallback Loader) Storage {
return &fallbackLoaderStorage{
Storage: s,
fallbackLoader: fallback,
}
}

func (s *fallbackLoaderStorage) Load(ctx context.Context, owner client.Object) (fs.FS, error) {
fsys, err := s.Storage.Load(ctx, owner)
if err != nil {
return s.fallbackLoader.Load(ctx, owner)
}
return fsys, nil
}
41 changes: 41 additions & 0 deletions internal/storage/temp.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package storage

import (
"context"
"fmt"
"io/fs"
"log"
"net/http"
"testing/fstest"

"sigs.k8s.io/controller-runtime/pkg/client"
)

var _ Storage = &TempStorage{}

var _ http.Handler = &TempStorage{}

type TempStorage struct{}

func (ts *TempStorage) Store(ctx context.Context, owner client.Object, bundle fs.FS) error {

Check warning on line 20 in internal/storage/temp.go

View workflow job for this annotation

GitHub Actions / lint

unused-parameter: parameter 'ctx' seems to be unused, consider removing or renaming it as _ (revive)
log.Printf("Storing contents for %s", owner.GetName())
return nil
}

func (ts *TempStorage) Delete(ctx context.Context, owner client.Object) error {

Check warning on line 25 in internal/storage/temp.go

View workflow job for this annotation

GitHub Actions / lint

unused-parameter: parameter 'ctx' seems to be unused, consider removing or renaming it as _ (revive)
log.Printf("Deleting contents for %s", owner.GetName())
return nil
}

func (ts *TempStorage) URLFor(ctx context.Context, owner client.Object) (string, error) {

Check warning on line 30 in internal/storage/temp.go

View workflow job for this annotation

GitHub Actions / lint

unused-parameter: parameter 'ctx' seems to be unused, consider removing or renaming it as _ (revive)
return fmt.Sprintf("%s-tempstorage-url", owner.GetName()), nil
}

func (ts *TempStorage) ServeHTTP(rw http.ResponseWriter, req *http.Request) {

Check warning on line 34 in internal/storage/temp.go

View workflow job for this annotation

GitHub Actions / lint

unused-parameter: parameter 'req' seems to be unused, consider removing or renaming it as _ (revive)
// Eat any errors since this is temporary
rw.Write([]byte("TempStorage serving some HTTP!"))

Check failure on line 36 in internal/storage/temp.go

View workflow job for this annotation

GitHub Actions / lint

Error return value of `rw.Write` is not checked (errcheck)
}

func (ts *TempStorage) Load(ctx context.Context, owner client.Object) (fs.FS, error) {
return &fstest.MapFS{}, nil
}
8 changes: 8 additions & 0 deletions pkg/controllers/core/catalog_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import (

"github.com/operator-framework/catalogd/api/core/v1alpha1"
"github.com/operator-framework/catalogd/internal/source"
"github.com/operator-framework/catalogd/internal/storage"
"github.com/operator-framework/catalogd/pkg/features"
)

Expand All @@ -50,6 +51,7 @@ import (
type CatalogReconciler struct {
client.Client
Unpacker source.Unpacker
Storage storage.Storage
}

//+kubebuilder:rbac:groups=catalogd.operatorframework.io,resources=catalogs,verbs=get;list;watch;create;update;patch;delete
Expand Down Expand Up @@ -164,6 +166,12 @@ func (r *CatalogReconciler) reconcile(ctx context.Context, catalog *v1alpha1.Cat
}
}

if features.CatalogdFeatureGate.Enabled(features.HttpServer) {
if err = r.Storage.Store(ctx, catalog, unpackResult.FS); err != nil {
return ctrl.Result{}, updateStatusUnpackFailing(&catalog.Status, fmt.Errorf("storing catalog: %v", err))
}
}

updateStatusUnpacked(&catalog.Status, unpackResult)
return ctrl.Result{}, nil
default:
Expand Down
53 changes: 53 additions & 0 deletions pkg/controllers/core/catalog_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -505,6 +505,59 @@ var _ = Describe("Catalogd Controller Test", func() {
})
})
})
// TODO(everettraven): Implement proper testing logic when there is an implementation to test
When("the HttpServer feature gate is enabled", func() {
BeforeEach(func() {
Expect(features.CatalogdFeatureGate.SetFromMap(map[string]bool{
string(features.HttpServer): true,
})).NotTo(HaveOccurred())

// reconcile
// res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: catalogKey})
// Expect(res).To(Equal(ctrl.Result{}))
// Expect(err).ToNot(HaveOccurred())
})

AfterEach(func() {
// clean up catalogmetadata
// Expect(cl.DeleteAllOf(ctx, &v1alpha1.CatalogMetadata{})).To(Succeed())
Expect(features.CatalogdFeatureGate.SetFromMap(map[string]bool{
string(features.HttpServer): false,
})).NotTo(HaveOccurred())
})

It("should do something", func() {})

When("creating another Catalog", func() {
var (
// tempCatalog *v1alpha1.Catalog
)
BeforeEach(func() {
// tempCatalog = &v1alpha1.Catalog{
// ObjectMeta: metav1.ObjectMeta{Name: "tempedout"},
// Spec: v1alpha1.CatalogSpec{
// Source: v1alpha1.CatalogSource{
// Type: "image",
// Image: &v1alpha1.ImageSource{
// Ref: "somecatalog:latest",
// },
// },
// },
// }

// Expect(cl.Create(ctx, tempCatalog)).To(Succeed())
// res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: types.NamespacedName{Name: "tempedout"}})
// Expect(res).To(Equal(ctrl.Result{}))
// Expect(err).ToNot(HaveOccurred())
})

AfterEach(func() {
// Expect(cl.Delete(ctx, tempCatalog)).NotTo(HaveOccurred())
})

It("should do something", func() {})
})
})
})
})
})
Expand Down
2 changes: 2 additions & 0 deletions pkg/features/features.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

CatalogMetadataAPI featuregate.Feature = "CatalogMetadataAPI"
PackagesBundleMetadataAPIs featuregate.Feature = "PackagesBundleMetadataAPIs"
HttpServer featuregate.Feature = "HttpServer"

Check warning on line 14 in pkg/features/features.go

View workflow job for this annotation

GitHub Actions / lint

var-naming: const HttpServer should be HTTPServer (revive)
)

var catalogdFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{
Expand All @@ -19,6 +20,7 @@

PackagesBundleMetadataAPIs: {Default: false, PreRelease: featuregate.Deprecated},
CatalogMetadataAPI: {Default: false, PreRelease: featuregate.Alpha},
HttpServer: {Default: false, PreRelease: featuregate.Alpha},
}

var CatalogdFeatureGate featuregate.MutableFeatureGate = featuregate.NewFeatureGate()
Expand Down
Loading