From a7f848fdc312e5f862b48a43cedc9a023c8fd3b0 Mon Sep 17 00:00:00 2001 From: Mikalai Radchuk Date: Wed, 4 Oct 2023 14:09:27 +0100 Subject: [PATCH] Adds bundle data snapshotting for resolution We now snapshot bundle data in the client for the duration of single resolution. This reduces reads (from network or cache) and unmarshalling since data in memory. And it also ensures that different variable sources involved in the resolution process have the same view of catalogs and bundles. Signed-off-by: Mikalai Radchuk --- cmd/manager/main.go | 9 +++--- internal/catalogmetadata/client/client.go | 28 +++++++++++++++++++ internal/controllers/operator_controller.go | 10 +++++-- .../controllers/operator_controller_test.go | 11 ++++++-- 4 files changed, 47 insertions(+), 11 deletions(-) diff --git a/cmd/manager/main.go b/cmd/manager/main.go index bf646b316..b5d9bc1cd 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -110,11 +110,10 @@ func main() { catalogClient := catalogclient.New(cl, cache.NewFilesystemCache(cachePath, &http.Client{Timeout: 10 * time.Second})) if err = (&controllers.OperatorReconciler{ - Client: cl, - Scheme: mgr.GetScheme(), - Resolver: solver.NewDeppySolver( - controllers.NewVariableSource(cl, catalogClient), - ), + Client: cl, + Scheme: mgr.GetScheme(), + CatalogClient: catalogClient, + NewSolver: solver.NewDeppySolver, }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "Operator") os.Exit(1) diff --git a/internal/catalogmetadata/client/client.go b/internal/catalogmetadata/client/client.go index 80d879824..eb9bdc6f1 100644 --- a/internal/catalogmetadata/client/client.go +++ b/internal/catalogmetadata/client/client.go @@ -100,6 +100,34 @@ func (c *Client) Bundles(ctx context.Context) ([]*catalogmetadata.Bundle, error) return allBundles, nil } +func NewSnapshotClient(catalogClient *Client) *SnapshotClient { + return &SnapshotClient{ + Client: catalogClient, + } +} + +// SnapshotClient fetches data from catalogs and caches them for the lifetime of the +// SnapshotClient instance. Meaning that any change to catalogs after the first call +// of the client will not affect set of bundles returned by this instance. +// This is convenient for bundle resolution process where we want all components +// to have the same view of catalogs. +type SnapshotClient struct { + *Client + bundlesSnapshot []*catalogmetadata.Bundle +} + +func (c *SnapshotClient) Bundles(ctx context.Context) ([]*catalogmetadata.Bundle, error) { + if c.bundlesSnapshot == nil { + allBundles, err := c.Client.Bundles(ctx) + if err != nil { + return nil, err + } + c.bundlesSnapshot = allBundles + } + + return c.bundlesSnapshot, nil +} + func PopulateExtraFields(catalogName string, channels []*catalogmetadata.Channel, bundles []*catalogmetadata.Bundle) ([]*catalogmetadata.Bundle, error) { bundlesMap := map[string]*catalogmetadata.Bundle{} for i := range bundles { diff --git a/internal/controllers/operator_controller.go b/internal/controllers/operator_controller.go index 3b0ef5067..487905462 100644 --- a/internal/controllers/operator_controller.go +++ b/internal/controllers/operator_controller.go @@ -26,6 +26,7 @@ import ( "github.com/go-logr/logr" catalogd "github.com/operator-framework/catalogd/api/core/v1alpha1" "github.com/operator-framework/deppy/pkg/deppy" + "github.com/operator-framework/deppy/pkg/deppy/input" "github.com/operator-framework/deppy/pkg/deppy/solver" rukpakv1alpha1 "github.com/operator-framework/rukpak/api/v1alpha1" "k8s.io/apimachinery/pkg/api/equality" @@ -45,6 +46,7 @@ import ( operatorsv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" "github.com/operator-framework/operator-controller/internal/catalogmetadata" + catalogclient "github.com/operator-framework/operator-controller/internal/catalogmetadata/client" "github.com/operator-framework/operator-controller/internal/controllers/validators" olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables" ) @@ -52,8 +54,9 @@ import ( // OperatorReconciler reconciles a Operator object type OperatorReconciler struct { client.Client - Scheme *runtime.Scheme - Resolver *solver.DeppySolver + Scheme *runtime.Scheme + CatalogClient *catalogclient.Client + NewSolver func(variableSource input.VariableSource) *solver.DeppySolver } //+kubebuilder:rbac:groups=operators.operatorframework.io,resources=operators,verbs=get;list;watch @@ -130,7 +133,8 @@ func (r *OperatorReconciler) reconcile(ctx context.Context, op *operatorsv1alpha return ctrl.Result{}, nil } // run resolution - solution, err := r.Resolver.Solve(ctx) + variableSource := NewVariableSource(r.Client, catalogclient.NewSnapshotClient(r.CatalogClient)) + solution, err := r.NewSolver(variableSource).Solve(ctx) if err != nil { op.Status.InstalledBundleResource = "" setInstalledStatusConditionUnknown(&op.Status.Conditions, "installation has not been attempted as resolution failed", op.GetGeneration()) diff --git a/internal/controllers/operator_controller_test.go b/internal/controllers/operator_controller_test.go index 1f75c0518..837ff2fe1 100644 --- a/internal/controllers/operator_controller_test.go +++ b/internal/controllers/operator_controller_test.go @@ -7,6 +7,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "github.com/operator-framework/deppy/pkg/deppy/input" "github.com/operator-framework/deppy/pkg/deppy/solver" "github.com/operator-framework/operator-registry/alpha/declcfg" "github.com/operator-framework/operator-registry/alpha/property" @@ -37,9 +38,13 @@ var _ = Describe("Operator Controller Test", func() { ctx = context.Background() fakeCatalogClient = testutil.NewFakeCatalogClient(testBundleList) reconciler = &controllers.OperatorReconciler{ - Client: cl, - Scheme: sch, - Resolver: solver.NewDeppySolver(controllers.NewVariableSource(cl, &fakeCatalogClient)), + Client: cl, + Scheme: sch, + NewSolver: func(variableSource input.VariableSource) *solver.DeppySolver { + return solver.NewDeppySolver( + controllers.NewVariableSource(cl, &fakeCatalogClient), + ) + }, } }) When("the operator does not exist", func() {