From 5646c126594e4d23359cedb652d53d97d4ece614 Mon Sep 17 00:00:00 2001 From: Mikalai Radchuk Date: Tue, 28 Nov 2023 16:51:00 +0000 Subject: [PATCH] Upgrade Deppy New version contains breaking changes: Deppy no longer has variable source API. Instead it accepts a slice of variables as input. Signed-off-by: Mikalai Radchuk --- cmd/manager/main.go | 9 ++-- cmd/resolutioncli/main.go | 29 +++++++++--- go.mod | 2 +- go.sum | 4 +- internal/controllers/operator_controller.go | 46 +++++++++++++++++-- .../controllers/operator_controller_test.go | 8 ++-- internal/controllers/suite_test.go | 7 +-- .../{variable_source.go => variables.go} | 43 ++--------------- ...iable_source_test.go => variables_test.go} | 13 ++---- 9 files changed, 86 insertions(+), 75 deletions(-) rename internal/controllers/{variable_source.go => variables.go} (60%) rename internal/controllers/{variable_source_test.go => variables_test.go} (89%) diff --git a/cmd/manager/main.go b/cmd/manager/main.go index bf646b316..136b63b0b 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, + BundleProvider: catalogClient, + Scheme: mgr.GetScheme(), + Resolver: solver.NewDeppySolver(), }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "Operator") os.Exit(1) diff --git a/cmd/resolutioncli/main.go b/cmd/resolutioncli/main.go index 846bf5500..3950ee596 100644 --- a/cmd/resolutioncli/main.go +++ b/cmd/resolutioncli/main.go @@ -31,6 +31,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client/fake" catalogd "github.com/operator-framework/catalogd/api/core/v1alpha1" + "github.com/operator-framework/deppy/pkg/deppy" "github.com/operator-framework/deppy/pkg/deppy/solver" rukpakv1alpha1 "github.com/operator-framework/rukpak/api/v1alpha1" @@ -143,14 +144,28 @@ func run(ctx context.Context, packageName, packageChannel, packageVersionRange, }, }) + resolver := solver.NewDeppySolver() + cl := clientBuilder.Build() catalogClient := newIndexRefClient(indexRef) + allBundles, err := catalogClient.Bundles(ctx) + if err != nil { + return err + } + operatorList := operatorsv1alpha1.OperatorList{} + if err := cl.List(ctx, &operatorList); err != nil { + return err + } + bundleDeploymentList := rukpakv1alpha1.BundleDeploymentList{} + if err := cl.List(ctx, &bundleDeploymentList); err != nil { + return err + } + variables, err := controllers.GenerateVariables(allBundles, operatorList.Items, bundleDeploymentList.Items) + if err != nil { + return err + } - resolver := solver.NewDeppySolver( - controllers.NewVariableSource(cl, catalogClient), - ) - - bundleImage, err := resolve(ctx, resolver, packageName) + bundleImage, err := resolve(resolver, variables, packageName) if err != nil { return err } @@ -159,8 +174,8 @@ func run(ctx context.Context, packageName, packageChannel, packageVersionRange, return nil } -func resolve(ctx context.Context, resolver *solver.DeppySolver, packageName string) (string, error) { - solution, err := resolver.Solve(ctx) +func resolve(resolver *solver.DeppySolver, variables []deppy.Variable, packageName string) (string, error) { + solution, err := resolver.Solve(variables) if err != nil { return "", err } diff --git a/go.mod b/go.mod index 30c094eb9..df7a2efa1 100644 --- a/go.mod +++ b/go.mod @@ -8,7 +8,7 @@ require ( github.com/go-logr/logr v1.3.0 github.com/google/go-cmp v0.6.0 github.com/operator-framework/catalogd v0.10.0 - github.com/operator-framework/deppy v0.1.0 + github.com/operator-framework/deppy v0.2.0 github.com/operator-framework/operator-registry v1.32.0 github.com/operator-framework/rukpak v0.16.0 github.com/spf13/pflag v1.0.5 diff --git a/go.sum b/go.sum index 046f3d48c..dc8490894 100644 --- a/go.sum +++ b/go.sum @@ -690,8 +690,8 @@ github.com/operator-framework/api v0.19.0 h1:QU1CTJU+CufoeneA5rsNlP/uP96s8vDHWUY github.com/operator-framework/api v0.19.0/go.mod h1:SCCslqke6AVOJ5JM+NqNE1CHuAgJLScsL66pnPaSMXs= github.com/operator-framework/catalogd v0.10.0 h1:T207IQfQCcd3f31bDPCgfJcwEvmaPGV8BotqIzIvnRo= github.com/operator-framework/catalogd v0.10.0/go.mod h1:xIeR5f/Spr5rHnLp0yOi0AYetWcHvBAx9n+K3S/va2A= -github.com/operator-framework/deppy v0.1.0 h1:Kj6SgSSMsPTbiWObYe7P1JPsW6CWkuVc+38RnPcZxGQ= -github.com/operator-framework/deppy v0.1.0/go.mod h1:3blHej0Hj0M17Ru2q3QrhN9OwB5/MMmFkWUmiInqs6A= +github.com/operator-framework/deppy v0.2.0 h1:BYdCAKli+ofFdnHPkpUKI9DygHL2A32CaDbEJk4jz6U= +github.com/operator-framework/deppy v0.2.0/go.mod h1:3blHej0Hj0M17Ru2q3QrhN9OwB5/MMmFkWUmiInqs6A= github.com/operator-framework/operator-registry v1.32.0 h1:RNazXYt3vBf5FZ+JrNNjq4bNh3tDwlkwZJnC+kmCeKk= github.com/operator-framework/operator-registry v1.32.0/go.mod h1:89VshAf6+n0V12vdh43+WOi8i1+XpY+kg6Ao4JO0y6k= github.com/operator-framework/rukpak v0.16.0 h1:d6iI7lYJbR5fHqw3vnAudB5SevAQ2dnQI7C6iOZyXJU= diff --git a/internal/controllers/operator_controller.go b/internal/controllers/operator_controller.go index a5be9dcbd..13a95107e 100644 --- a/internal/controllers/operator_controller.go +++ b/internal/controllers/operator_controller.go @@ -22,6 +22,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/solver" rukpakv1alpha1 "github.com/operator-framework/rukpak/api/v1alpha1" "k8s.io/apimachinery/pkg/api/equality" @@ -44,11 +45,18 @@ import ( olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables" ) +// BundleProvider provides the way to retrieve a list of Bundles from a source, +// generally from a catalog client of some kind. +type BundleProvider interface { + Bundles(ctx context.Context) ([]*catalogmetadata.Bundle, error) +} + // OperatorReconciler reconciles a Operator object type OperatorReconciler struct { client.Client - Scheme *runtime.Scheme - Resolver *solver.DeppySolver + BundleProvider BundleProvider + Scheme *runtime.Scheme + Resolver *solver.DeppySolver } //+kubebuilder:rbac:groups=operators.operatorframework.io,resources=operators,verbs=get;list;watch @@ -124,8 +132,19 @@ func (r *OperatorReconciler) reconcile(ctx context.Context, op *operatorsv1alpha setResolvedStatusConditionUnknown(&op.Status.Conditions, "validation has not been attempted as spec is invalid", op.GetGeneration()) return ctrl.Result{}, nil } + + // gather variables for resolution + variables, err := r.variables(ctx) + if err != nil { + op.Status.InstalledBundleResource = "" + setInstalledStatusConditionUnknown(&op.Status.Conditions, "installation has not been attempted due to failure to gather data for resolution", op.GetGeneration()) + op.Status.ResolvedBundleResource = "" + setResolvedStatusConditionFailed(&op.Status.Conditions, err.Error(), op.GetGeneration()) + return ctrl.Result{}, err + } + // run resolution - solution, err := r.Resolver.Solve(ctx) + solution, err := r.Resolver.Solve(variables) if err != nil { op.Status.InstalledBundleResource = "" setInstalledStatusConditionUnknown(&op.Status.Conditions, "installation has not been attempted as resolution failed", op.GetGeneration()) @@ -194,6 +213,27 @@ func (r *OperatorReconciler) reconcile(ctx context.Context, op *operatorsv1alpha return ctrl.Result{}, nil } +func (r *OperatorReconciler) variables(ctx context.Context) ([]deppy.Variable, error) { + allBundles, err := r.BundleProvider.Bundles(ctx) + if err != nil { + return nil, err + } + operatorList := operatorsv1alpha1.OperatorList{} + if err := r.Client.List(ctx, &operatorList); err != nil { + return nil, err + } + bundleDeploymentList := rukpakv1alpha1.BundleDeploymentList{} + if err := r.Client.List(ctx, &bundleDeploymentList); err != nil { + return nil, err + } + variables, err := GenerateVariables(allBundles, operatorList.Items, bundleDeploymentList.Items) + if err != nil { + return nil, err + } + + return variables, nil +} + func mapBDStatusToInstalledCondition(existingTypedBundleDeployment *rukpakv1alpha1.BundleDeployment, op *operatorsv1alpha1.Operator) { bundleDeploymentReady := apimeta.FindStatusCondition(existingTypedBundleDeployment.Status.Conditions, rukpakv1alpha1.TypeInstalled) if bundleDeploymentReady == nil { diff --git a/internal/controllers/operator_controller_test.go b/internal/controllers/operator_controller_test.go index a5b3504a2..0d6120cf6 100644 --- a/internal/controllers/operator_controller_test.go +++ b/internal/controllers/operator_controller_test.go @@ -117,7 +117,7 @@ func TestOperatorNonExistantVersion(t *testing.T) { require.NotNil(t, cond) require.Equal(t, metav1.ConditionUnknown, cond.Status) require.Equal(t, operatorsv1alpha1.ReasonInstallationStatusUnknown, cond.Reason) - require.Equal(t, "installation has not been attempted as resolution failed", cond.Message) + require.Equal(t, "installation has not been attempted due to failure to gather data for resolution", cond.Message) verifyInvariants(ctx, t, reconciler.Client, operator) require.NoError(t, cl.DeleteAllOf(ctx, &operatorsv1alpha1.Operator{})) @@ -834,7 +834,7 @@ func TestOperatorVersionNoChannel(t *testing.T) { require.NotNil(t, cond) require.Equal(t, metav1.ConditionUnknown, cond.Status) require.Equal(t, operatorsv1alpha1.ReasonInstallationStatusUnknown, cond.Reason) - require.Equal(t, "installation has not been attempted as resolution failed", cond.Message) + require.Equal(t, "installation has not been attempted due to failure to gather data for resolution", cond.Message) verifyInvariants(ctx, t, reconciler.Client, operator) require.NoError(t, cl.DeleteAllOf(ctx, &operatorsv1alpha1.Operator{})) @@ -882,7 +882,7 @@ func TestOperatorNoChannel(t *testing.T) { require.NotNil(t, cond) require.Equal(t, metav1.ConditionUnknown, cond.Status) require.Equal(t, operatorsv1alpha1.ReasonInstallationStatusUnknown, cond.Reason) - require.Equal(t, "installation has not been attempted as resolution failed", cond.Message) + require.Equal(t, "installation has not been attempted due to failure to gather data for resolution", cond.Message) verifyInvariants(ctx, t, reconciler.Client, operator) require.NoError(t, cl.DeleteAllOf(ctx, &operatorsv1alpha1.Operator{})) @@ -932,7 +932,7 @@ func TestOperatorNoVersion(t *testing.T) { require.NotNil(t, cond) require.Equal(t, metav1.ConditionUnknown, cond.Status) require.Equal(t, operatorsv1alpha1.ReasonInstallationStatusUnknown, cond.Reason) - require.Equal(t, "installation has not been attempted as resolution failed", cond.Message) + require.Equal(t, "installation has not been attempted due to failure to gather data for resolution", cond.Message) verifyInvariants(ctx, t, reconciler.Client, operator) require.NoError(t, cl.DeleteAllOf(ctx, &operatorsv1alpha1.Operator{})) diff --git a/internal/controllers/suite_test.go b/internal/controllers/suite_test.go index 3b10a032d..a1a228abf 100644 --- a/internal/controllers/suite_test.go +++ b/internal/controllers/suite_test.go @@ -48,9 +48,10 @@ func newClientAndReconciler(t *testing.T) (client.Client, *controllers.OperatorR cl := newClient(t) fakeCatalogClient := testutil.NewFakeCatalogClient(testBundleList) reconciler := &controllers.OperatorReconciler{ - Client: cl, - Scheme: sch, - Resolver: solver.NewDeppySolver(controllers.NewVariableSource(cl, &fakeCatalogClient)), + Client: cl, + BundleProvider: &fakeCatalogClient, + Scheme: sch, + Resolver: solver.NewDeppySolver(), } return cl, reconciler } diff --git a/internal/controllers/variable_source.go b/internal/controllers/variables.go similarity index 60% rename from internal/controllers/variable_source.go rename to internal/controllers/variables.go index e361c96fb..0c10c2a0a 100644 --- a/internal/controllers/variable_source.go +++ b/internal/controllers/variables.go @@ -17,10 +17,6 @@ limitations under the License. package controllers import ( - "context" - - "sigs.k8s.io/controller-runtime/pkg/client" - "github.com/operator-framework/deppy/pkg/deppy" rukpakv1alpha1 "github.com/operator-framework/rukpak/api/v1alpha1" @@ -29,46 +25,13 @@ import ( "github.com/operator-framework/operator-controller/internal/resolution/variablesources" ) -// BundleProvider provides the way to retrieve a list of Bundles from a source, -// generally from a catalog client of some kind. -type BundleProvider interface { - Bundles(ctx context.Context) ([]*catalogmetadata.Bundle, error) -} - -type VariableSource struct { - client client.Client - catalogClient BundleProvider -} - -func NewVariableSource(cl client.Client, catalogClient BundleProvider) *VariableSource { - return &VariableSource{ - client: cl, - catalogClient: catalogClient, - } -} - -func (v *VariableSource) GetVariables(ctx context.Context) ([]deppy.Variable, error) { - operatorList := operatorsv1alpha1.OperatorList{} - if err := v.client.List(ctx, &operatorList); err != nil { - return nil, err - } - - bundleDeploymentList := rukpakv1alpha1.BundleDeploymentList{} - if err := v.client.List(ctx, &bundleDeploymentList); err != nil { - return nil, err - } - - allBundles, err := v.catalogClient.Bundles(ctx) - if err != nil { - return nil, err - } - - requiredPackages, err := variablesources.MakeRequiredPackageVariables(allBundles, operatorList.Items) +func GenerateVariables(allBundles []*catalogmetadata.Bundle, operators []operatorsv1alpha1.Operator, bundleDeployments []rukpakv1alpha1.BundleDeployment) ([]deppy.Variable, error) { + requiredPackages, err := variablesources.MakeRequiredPackageVariables(allBundles, operators) if err != nil { return nil, err } - installedPackages, err := variablesources.MakeInstalledPackageVariables(allBundles, operatorList.Items, bundleDeploymentList.Items) + installedPackages, err := variablesources.MakeInstalledPackageVariables(allBundles, operators, bundleDeployments) if err != nil { return nil, err } diff --git a/internal/controllers/variable_source_test.go b/internal/controllers/variables_test.go similarity index 89% rename from internal/controllers/variable_source_test.go rename to internal/controllers/variables_test.go index 0753c49b3..9014533c6 100644 --- a/internal/controllers/variable_source_test.go +++ b/internal/controllers/variables_test.go @@ -1,7 +1,6 @@ package controllers_test import ( - "context" "encoding/json" "fmt" "testing" @@ -14,7 +13,6 @@ import ( "k8s.io/apimachinery/pkg/util/rand" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/utils/pointer" - "sigs.k8s.io/controller-runtime/pkg/client/fake" "github.com/operator-framework/deppy/pkg/deppy" "github.com/operator-framework/deppy/pkg/deppy/constraint" @@ -27,7 +25,6 @@ import ( "github.com/operator-framework/operator-controller/internal/catalogmetadata" "github.com/operator-framework/operator-controller/internal/controllers" olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables" - testutil "github.com/operator-framework/operator-controller/test/util" ) func TestVariableSource(t *testing.T) { @@ -64,7 +61,7 @@ func TestVariableSource(t *testing.T) { pkgName := "packageA" opName := fmt.Sprintf("operator-test-%s", rand.String(8)) - operator := &operatorsv1alpha1.Operator{ + operator := operatorsv1alpha1.Operator{ ObjectMeta: metav1.ObjectMeta{Name: opName}, Spec: operatorsv1alpha1.OperatorSpec{ PackageName: pkgName, @@ -73,7 +70,7 @@ func TestVariableSource(t *testing.T) { }, } - bd := &rukpakv1alpha1.BundleDeployment{ + bd := rukpakv1alpha1.BundleDeployment{ ObjectMeta: metav1.ObjectMeta{ Name: opName, OwnerReferences: []metav1.OwnerReference{ @@ -102,12 +99,8 @@ func TestVariableSource(t *testing.T) { }, }, } - fakeClient := fake.NewClientBuilder().WithScheme(sch).WithObjects(operator, bd).Build() - fakeCatalogClient := testutil.NewFakeCatalogClient(allBundles) - vs := controllers.NewVariableSource(fakeClient, &fakeCatalogClient) - - vars, err := vs.GetVariables(context.Background()) + vars, err := controllers.GenerateVariables(allBundles, []operatorsv1alpha1.Operator{operator}, []rukpakv1alpha1.BundleDeployment{bd}) require.NoError(t, err) expectedVars := []deppy.Variable{