Skip to content

Commit

Permalink
Upgrade Deppy (#560)
Browse files Browse the repository at this point in the history
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 <mradchuk@redhat.com>
  • Loading branch information
m1kola authored Dec 4, 2023
1 parent d1cef1f commit ba21cdf
Show file tree
Hide file tree
Showing 9 changed files with 82 additions and 75 deletions.
9 changes: 4 additions & 5 deletions cmd/manager/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
29 changes: 22 additions & 7 deletions cmd/resolutioncli/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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
}
Expand All @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
42 changes: 39 additions & 3 deletions internal/controllers/operator_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand Down Expand Up @@ -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 vars for resolution
vars, 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(vars)
if err != nil {
op.Status.InstalledBundleResource = ""
setInstalledStatusConditionUnknown(&op.Status.Conditions, "installation has not been attempted as resolution failed", op.GetGeneration())
Expand Down Expand Up @@ -194,6 +213,23 @@ 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
}

return GenerateVariables(allBundles, operatorList.Items, bundleDeploymentList.Items)
}

func mapBDStatusToInstalledCondition(existingTypedBundleDeployment *rukpakv1alpha1.BundleDeployment, op *operatorsv1alpha1.Operator) {
bundleDeploymentReady := apimeta.FindStatusCondition(existingTypedBundleDeployment.Status.Conditions, rukpakv1alpha1.TypeInstalled)
if bundleDeploymentReady == nil {
Expand Down
8 changes: 4 additions & 4 deletions internal/controllers/operator_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}))
Expand Down Expand Up @@ -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{}))
Expand Down Expand Up @@ -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{}))
Expand Down Expand Up @@ -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{}))
Expand Down
7 changes: 4 additions & 3 deletions internal/controllers/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand All @@ -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
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package controllers_test

import (
"context"
"encoding/json"
"fmt"
"testing"
Expand All @@ -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"
Expand All @@ -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) {
Expand Down Expand Up @@ -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,
Expand All @@ -73,7 +70,7 @@ func TestVariableSource(t *testing.T) {
},
}

bd := &rukpakv1alpha1.BundleDeployment{
bd := rukpakv1alpha1.BundleDeployment{
ObjectMeta: metav1.ObjectMeta{
Name: opName,
OwnerReferences: []metav1.OwnerReference{
Expand Down Expand Up @@ -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{
Expand Down

0 comments on commit ba21cdf

Please sign in to comment.