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.

Sign up for GitHub

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

🌱 Reduce number of variable sources #460

Closed
wants to merge 1 commit into from
Closed

🌱 Reduce number of variable sources #460

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 2 additions & 1 deletion cmd/manager/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import (
"github.com/operator-framework/operator-controller/internal/catalogmetadata/cache"
catalogclient "github.com/operator-framework/operator-controller/internal/catalogmetadata/client"
"github.com/operator-framework/operator-controller/internal/controllers"
"github.com/operator-framework/operator-controller/internal/resolution/variablesources"
"github.com/operator-framework/operator-controller/pkg/features"
)

Expand Down Expand Up @@ -113,7 +114,7 @@ func main() {
Client: cl,
Scheme: mgr.GetScheme(),
Resolver: solver.NewDeppySolver(
controllers.NewVariableSource(cl, catalogClient),
variablesources.NewOLMVariableSource(cl, catalogClient),
),
}).SetupWithManager(mgr); err != nil {
setupLog.Error(err, "unable to create controller", "controller", "Operator")
Expand Down
42 changes: 31 additions & 11 deletions cmd/resolutioncli/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@ import (
"fmt"
"os"

"github.com/operator-framework/deppy/pkg/deppy/solver"
rukpakv1alpha1 "github.com/operator-framework/rukpak/api/v1alpha1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/serializer"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
Expand All @@ -32,10 +31,11 @@ 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/solver"
rukpakv1alpha1 "github.com/operator-framework/rukpak/api/v1alpha1"

operatorsv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1"
"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"
"github.com/operator-framework/operator-controller/internal/resolution/variablesources"
)
Expand Down Expand Up @@ -71,12 +71,12 @@ func main() {
ctx := context.Background()

var packageName string
var packageVersion string
var packageVersionRange string
var packageChannel string
var indexRef string
var inputDir string
flag.StringVar(&packageName, flagNamePackageName, "", "Name of the package to resolve")
flag.StringVar(&packageVersion, flagNamePackageVersion, "", "Version of the package")
flag.StringVar(&packageVersionRange, flagNamePackageVersion, "", "Version or version range of the package")
flag.StringVar(&packageChannel, flagNamePackageChannel, "", "Channel of the package")
// TODO: Consider adding support of multiple refs
flag.StringVar(&indexRef, flagNameIndexRef, "", "Index reference (FBC image or dir)")
Expand All @@ -89,7 +89,7 @@ func main() {
os.Exit(1)
}

err := run(ctx, packageName, packageVersion, packageChannel, indexRef, inputDir)
err := run(ctx, packageName, packageChannel, packageVersionRange, indexRef, inputDir)
if err != nil {
fmt.Fprintln(os.Stderr, err)
os.Exit(1)
Expand All @@ -108,7 +108,19 @@ func validateFlags(packageName, indexRef string) error {
return nil
}

func run(ctx context.Context, packageName, packageVersion, packageChannel, indexRef, inputDir string) error {
func run(ctx context.Context, packageName, packageChannel, packageVersionRange, indexRef, inputDir string) error {
// Using the fake Kubernetes client and creating objects
// in it from manifests & CLI flags is fine for PoC.
// But when/if we decide to proceed with CLI/offline resolution
// we will need to come up with a better way to create inputs
// for resolver when working with CLI.
//
// We will need to think about multiple types of inputs:
// - How to read required package (what we want to install/update)
// - How to read bundles from multiple catalogs
// - How to take into account cluster information. Some package
// will have constraints like "need Kubernetes version to be >= X"
// or "need >= 3 worker nodes").
clientBuilder := fake.NewClientBuilder().WithScheme(scheme)

if inputDir != "" {
Expand All @@ -120,14 +132,22 @@ func run(ctx context.Context, packageName, packageVersion, packageChannel, index
clientBuilder.WithRuntimeObjects(objects...)
}

clientBuilder.WithRuntimeObjects(&operatorsv1alpha1.Operator{
ObjectMeta: metav1.ObjectMeta{
Name: "resolutioncli-input-operator",
},
Spec: operatorsv1alpha1.OperatorSpec{
PackageName: packageName,
Channel: packageChannel,
Version: packageVersionRange,
},
})

cl := clientBuilder.Build()
catalogClient := newIndexRefClient(indexRef)

resolver := solver.NewDeppySolver(
append(
variablesources.NestedVariableSource{newPackageVariableSource(catalogClient, packageName, packageVersion, packageChannel)},
controllers.NewVariableSource(cl, catalogClient)...,
),
variablesources.NewOLMVariableSource(cl, catalogClient),
)

bundleImage, err := resolve(ctx, resolver, packageName)
Expand Down
44 changes: 0 additions & 44 deletions cmd/resolutioncli/variable_source.go
View file
Open in desktop

This file was deleted.

5 changes: 3 additions & 2 deletions internal/controllers/operator_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"github.com/operator-framework/operator-controller/internal/catalogmetadata"
"github.com/operator-framework/operator-controller/internal/conditionsets"
"github.com/operator-framework/operator-controller/internal/controllers"
"github.com/operator-framework/operator-controller/internal/resolution/variablesources"
"github.com/operator-framework/operator-controller/pkg/features"
testutil "github.com/operator-framework/operator-controller/test/util"
)
Expand All @@ -44,7 +45,7 @@ var _ = Describe("Operator Controller Test", func() {
reconciler = &controllers.OperatorReconciler{
Client: cl,
Scheme: sch,
Resolver: solver.NewDeppySolver(controllers.NewVariableSource(cl, &fakeCatalogClient)),
Resolver: solver.NewDeppySolver(variablesources.NewOLMVariableSource(cl, &fakeCatalogClient)),
}
})
When("the operator does not exist", func() {
Expand Down Expand Up @@ -1059,7 +1060,7 @@ func TestOperatorUpgrade(t *testing.T) {
reconciler := &controllers.OperatorReconciler{
Client: cl,
Scheme: sch,
Resolver: solver.NewDeppySolver(controllers.NewVariableSource(cl, &fakeCatalogClient)),
Resolver: solver.NewDeppySolver(variablesources.NewOLMVariableSource(cl, &fakeCatalogClient)),
}

t.Run("semver upgrade constraints", func(t *testing.T) {
Expand Down
42 changes: 0 additions & 42 deletions internal/controllers/variable_source.go
View file
Open in desktop

This file was deleted.

67 changes: 20 additions & 47 deletions ...riablesources/bundles_and_dependencies.go → ...rnal/resolution/variablesources/bundle.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,63 +1,34 @@
package variablesources
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I initially had everything in this package in two files: olm.go and olm_test.go. While olm.go was is still manageable, navigating olm_test.go was a bit difficult. So I decided to split into multiple files.

It resulted in a more complicated diff (while the files themselves are straightforward).


import (
"context"
"fmt"
"sort"

"github.com/operator-framework/deppy/pkg/deppy"
"github.com/operator-framework/deppy/pkg/deppy/input"
"k8s.io/apimachinery/pkg/util/sets"

"github.com/operator-framework/deppy/pkg/deppy"

"github.com/operator-framework/operator-controller/internal/catalogmetadata"
catalogfilter "github.com/operator-framework/operator-controller/internal/catalogmetadata/filter"
catalogsort "github.com/operator-framework/operator-controller/internal/catalogmetadata/sort"
olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables"
)

var _ input.VariableSource = &BundlesAndDepsVariableSource{}

type BundlesAndDepsVariableSource struct {
catalogClient BundleProvider
variableSources []input.VariableSource
}

func NewBundlesAndDepsVariableSource(catalogClient BundleProvider, inputVariableSources ...input.VariableSource) *BundlesAndDepsVariableSource {
return &BundlesAndDepsVariableSource{
catalogClient: catalogClient,
variableSources: inputVariableSources,
}
}

func (b *BundlesAndDepsVariableSource) GetVariables(ctx context.Context) ([]deppy.Variable, error) {
var variables []deppy.Variable

// extract required package variables
for _, variableSource := range b.variableSources {
inputVariables, err := variableSource.GetVariables(ctx)
if err != nil {
return nil, err
}
variables = append(variables, inputVariables...)
}

// create bundle queue for dependency resolution
func MakeBundleVariables(
allBundles []*catalogmetadata.Bundle,
requiredPackages []*olmvariables.RequiredPackageVariable,
installedPackages []*olmvariables.InstalledPackageVariable,
) ([]*olmvariables.BundleVariable, error) {
var bundleQueue []*catalogmetadata.Bundle
for _, variable := range variables {
switch v := variable.(type) {
case *olmvariables.RequiredPackageVariable:
bundleQueue = append(bundleQueue, v.Bundles()...)
case *olmvariables.InstalledPackageVariable:
bundleQueue = append(bundleQueue, v.Bundles()...)
}
for _, variable := range requiredPackages {
bundleQueue = append(bundleQueue, variable.Bundles()...)
}

allBundles, err := b.catalogClient.Bundles(ctx)
if err != nil {
return nil, err
for _, variable := range installedPackages {
bundleQueue = append(bundleQueue, variable.Bundles()...)
}

// build bundle and dependency variables
var result []*olmvariables.BundleVariable
visited := sets.Set[deppy.Identifier]{}
for len(bundleQueue) > 0 {
// pop head of queue
Expand All @@ -73,7 +44,7 @@ func (b *BundlesAndDepsVariableSource) GetVariables(ctx context.Context) ([]depp
visited.Insert(id)

// get bundle dependencies
dependencies, err := b.filterBundleDependencies(allBundles, head)
dependencies, err := filterBundleDependencies(allBundles, head)
if err != nil {
return nil, fmt.Errorf("could not determine dependencies for bundle with id '%s': %w", id, err)
}
Expand All @@ -82,21 +53,23 @@ func (b *BundlesAndDepsVariableSource) GetVariables(ctx context.Context) ([]depp
bundleQueue = append(bundleQueue, dependencies...)

// create variable
variables = append(variables, olmvariables.NewBundleVariable(head, dependencies))
result = append(result, olmvariables.NewBundleVariable(head, dependencies))
}

return variables, nil
return result, nil
}

func (b *BundlesAndDepsVariableSource) filterBundleDependencies(allBundles []*catalogmetadata.Bundle, bundle *catalogmetadata.Bundle) ([]*catalogmetadata.Bundle, error) {
func filterBundleDependencies(allBundles []*catalogmetadata.Bundle, bundle *catalogmetadata.Bundle) ([]*catalogmetadata.Bundle, error) {
var dependencies []*catalogmetadata.Bundle
added := sets.Set[deppy.Identifier]{}

// gather required package dependencies
// todo(perdasilva): disambiguate between not found and actual errors
requiredPackages, _ := bundle.RequiredPackages()
for _, requiredPackage := range requiredPackages {
packageDependencyBundles := catalogfilter.Filter(allBundles, catalogfilter.And(catalogfilter.WithPackageName(requiredPackage.PackageName), catalogfilter.InBlangSemverRange(requiredPackage.SemverRange)))
packageDependencyBundles := catalogfilter.Filter(allBundles, catalogfilter.And(
catalogfilter.WithPackageName(requiredPackage.PackageName),
catalogfilter.InBlangSemverRange(requiredPackage.SemverRange),
))
if len(packageDependencyBundles) == 0 {
return nil, fmt.Errorf("could not find package dependencies for bundle '%s'", bundle.Name)
}
Expand Down
57 changes: 0 additions & 57 deletions internal/resolution/variablesources/bundle_deployment.go
View file
Open in desktop

This file was deleted.

Loading