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

🌱 Upgrade Deppy #560

Merged
merged 1 commit into from
Dec 4, 2023
Merged
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
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