Skip to content

Commit

Permalink
Move bundle variable code into a func
Browse files Browse the repository at this point in the history 
Signed-off-by: Mikalai Radchuk <mradchuk@redhat.com>
  • Loading branch information
@m1kola
m1kola committed Nov 2, 2023
1 parent 98c2d04 commit d92550d
Show file tree
Hide file tree
Showing 2 changed files with 264 additions and 49 deletions.
122 changes: 74 additions & 48 deletions internal/resolution/variablesources/bundle.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,62 +2,36 @@ package variablesources

import (
"context"
"errors"
"fmt"
"sort"

"k8s.io/apimachinery/pkg/util/sets"

"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/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 MakeBundleVariables(
allBundles []*catalogmetadata.Bundle,
requiredPackages []*olmvariables.RequiredPackageVariable,
installedPackages []*olmvariables.InstalledPackageVariable,
) ([]*olmvariables.BundleVariable, error) {
bundleQueue := make([]*catalogmetadata.Bundle, 0, len(requiredPackages)+len(installedPackages))
for _, variable := range requiredPackages {
bundleQueue = append(bundleQueue, variable.Bundles()...)
}
}

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
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()...)
}
}

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
result := make([]*olmvariables.BundleVariable, 0, len(requiredPackages)+len(installedPackages))
visited := sets.Set[deppy.Identifier]{}
for len(bundleQueue) > 0 {
// pop head of queue
Expand All @@ -73,32 +47,34 @@ 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)
return nil, fmt.Errorf("could not determine dependencies for bundle with id %q: %w", id, err)
}

// add bundle dependencies to queue for processing
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)
return nil, errors.New("could not find package dependencies for bundle")
}
for i := 0; i < len(packageDependencyBundles); i++ {
bundle := packageDependencyBundles[i]
Expand All @@ -117,3 +93,53 @@ func (b *BundlesAndDepsVariableSource) filterBundleDependencies(allBundles []*ca

return dependencies, nil
}

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) {
variables := []deppy.Variable{}

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

allBundles, err := b.catalogClient.Bundles(ctx)
if err != nil {
return nil, err
}

requiredPackages := []*olmvariables.RequiredPackageVariable{}
installedPackages := []*olmvariables.InstalledPackageVariable{}
for _, variable := range variables {
switch v := variable.(type) {
case *olmvariables.RequiredPackageVariable:
requiredPackages = append(requiredPackages, v)
case *olmvariables.InstalledPackageVariable:
installedPackages = append(installedPackages, v)
}
}

bundles, err := MakeBundleVariables(allBundles, requiredPackages, installedPackages)
if err != nil {
return nil, err
}

for _, v := range bundles {
variables = append(variables, v)
}
return variables, nil
}
191 changes: 190 additions & 1 deletion internal/resolution/variablesources/bundle_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,208 @@ import (
"context"
"encoding/json"
"errors"
"sync"
"testing"

"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"github.com/operator-framework/deppy/pkg/deppy"
"github.com/operator-framework/deppy/pkg/deppy/constraint"
"github.com/operator-framework/deppy/pkg/deppy/input"
"github.com/operator-framework/operator-registry/alpha/declcfg"
"github.com/operator-framework/operator-registry/alpha/property"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/operator-framework/operator-controller/internal/catalogmetadata"
olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables"
"github.com/operator-framework/operator-controller/internal/resolution/variablesources"
testutil "github.com/operator-framework/operator-controller/test/util"
)

func TestMakeBundleVariables_ValidDepedencies(t *testing.T) {
const fakeCatalogName = "fake-catalog"
fakeChannel := catalogmetadata.Channel{Channel: declcfg.Channel{Name: "stable"}}
bundleSet := map[string]*catalogmetadata.Bundle{
// Test package which we will be using as input into
// the testable function
"test-package.v1.0.0": {
Bundle: declcfg.Bundle{
Name: "test-package.v1.0.0",
Package: "test-package",
Properties: []property.Property{
{Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "1.0.0"}`)},
{Type: property.TypePackageRequired, Value: json.RawMessage(`{"packageName": "first-level-dependency", "versionRange": ">=1.0.0 <2.0.0"}`)},
},
},
CatalogName: fakeCatalogName,
InChannels: []*catalogmetadata.Channel{&fakeChannel},
},

// First level dependency of test-package. Will be explicitly
// provided into the testable function as part of variable.
// This package must have at least one dependency with a version
// range so we can test that result has correct ordering:
// the testable function must give priority to newer versions.
"first-level-dependency.v1.0.0": {
Bundle: declcfg.Bundle{
Name: "first-level-dependency.v1.0.0",
Package: "first-level-dependency",
Properties: []property.Property{
{Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "first-level-dependency", "version": "1.0.0"}`)},
{Type: property.TypePackageRequired, Value: json.RawMessage(`{"packageName": "second-level-dependency", "versionRange": ">=1.0.0 <2.0.0"}`)},
},
},
CatalogName: fakeCatalogName,
InChannels: []*catalogmetadata.Channel{&fakeChannel},
},

// Second level dependency that matches requirements of the first level dependency.
"second-level-dependency.v1.0.0": {
Bundle: declcfg.Bundle{
Name: "second-level-dependency.v1.0.0",
Package: "second-level-dependency",
Properties: []property.Property{
{Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "second-level-dependency", "version": "1.0.0"}`)},
},
},
CatalogName: fakeCatalogName,
InChannels: []*catalogmetadata.Channel{&fakeChannel},
},

// Second level dependency that matches requirements of the first level dependency.
"second-level-dependency.v1.0.1": {
Bundle: declcfg.Bundle{
Name: "second-level-dependency.v1.0.1",
Package: "second-level-dependency",
Properties: []property.Property{
{Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "second-level-dependency", "version": "1.0.1"}`)},
},
},
CatalogName: fakeCatalogName,
InChannels: []*catalogmetadata.Channel{&fakeChannel},
},

// Second level dependency that does not match requirements of the first level dependency.
"second-level-dependency.v2.0.0": {
Bundle: declcfg.Bundle{
Name: "second-level-dependency.v2.0.0",
Package: "second-level-dependency",
Properties: []property.Property{
{Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "second-level-dependency", "version": "2.0.0"}`)},
},
},
CatalogName: fakeCatalogName,
InChannels: []*catalogmetadata.Channel{&fakeChannel},
},

// Package that is in a our fake catalog, but is not involved
// in this dependency chain. We need this to make sure that
// the testable function filters out unrelated bundles.
"uninvolved-package.v1.0.0": {
Bundle: declcfg.Bundle{
Name: "uninvolved-package.v1.0.0",
Package: "uninvolved-package",
Properties: []property.Property{
{Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "uninvolved-package", "version": "1.0.0"}`)},
},
},
CatalogName: fakeCatalogName,
InChannels: []*catalogmetadata.Channel{&fakeChannel},
},
}

allBundles := make([]*catalogmetadata.Bundle, 0, len(bundleSet))
for _, bundle := range bundleSet {
allBundles = append(allBundles, bundle)
}
requiredPackages := []*olmvariables.RequiredPackageVariable{
olmvariables.NewRequiredPackageVariable("test-package", []*catalogmetadata.Bundle{
bundleSet["first-level-dependency.v1.0.0"],
}),
}
installedPackages := []*olmvariables.InstalledPackageVariable{
olmvariables.NewInstalledPackageVariable("test-package", []*catalogmetadata.Bundle{
bundleSet["first-level-dependency.v1.0.0"],
}),
}

bundles, err := variablesources.MakeBundleVariables(allBundles, requiredPackages, installedPackages)
require.NoError(t, err)

// Each dependency must have a variable.
// Dependencies from the same package must be sorted by version
// with higher versions first.
expectedVariables := []*olmvariables.BundleVariable{
olmvariables.NewBundleVariable(
bundleSet["first-level-dependency.v1.0.0"],
[]*catalogmetadata.Bundle{
bundleSet["second-level-dependency.v1.0.1"],
bundleSet["second-level-dependency.v1.0.0"],
},
),
olmvariables.NewBundleVariable(
bundleSet["second-level-dependency.v1.0.1"],
nil,
),
olmvariables.NewBundleVariable(
bundleSet["second-level-dependency.v1.0.0"],
nil,
),
}
gocmpopts := []cmp.Option{
cmpopts.IgnoreUnexported(sync.RWMutex{}),
cmp.AllowUnexported(
sync.RWMutex{},
olmvariables.BundleVariable{},
input.SimpleVariable{},
constraint.DependencyConstraint{},
catalogmetadata.Bundle{},
),
cmpopts.IgnoreFields(
// Do not compare a func field
catalogmetadata.PackageRequired{}, "SemverRange",
),
}
require.Empty(t, cmp.Diff(bundles, expectedVariables, gocmpopts...))
}

func TestMakeBundleVariables_NonExistentDepedencies(t *testing.T) {
const fakeCatalogName = "fake-catalog"
fakeChannel := catalogmetadata.Channel{Channel: declcfg.Channel{Name: "stable"}}
bundleSet := map[string]*catalogmetadata.Bundle{
"test-package.v1.0.0": {
Bundle: declcfg.Bundle{
Name: "test-package.v1.0.0",
Package: "test-package",
Properties: []property.Property{
{Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "1.0.0"}`)},
{Type: property.TypePackageRequired, Value: json.RawMessage(`{"packageName": "first-level-dependency", "versionRange": ">=1.0.0 <2.0.0"}`)},
},
},
CatalogName: fakeCatalogName,
InChannels: []*catalogmetadata.Channel{&fakeChannel},
},
}

allBundles := make([]*catalogmetadata.Bundle, 0, len(bundleSet))
for _, bundle := range bundleSet {
allBundles = append(allBundles, bundle)
}
requiredPackages := []*olmvariables.RequiredPackageVariable{
olmvariables.NewRequiredPackageVariable("test-package", []*catalogmetadata.Bundle{
bundleSet["test-package.v1.0.0"],
}),
}
installedPackages := []*olmvariables.InstalledPackageVariable{}

bundles, err := variablesources.MakeBundleVariables(allBundles, requiredPackages, installedPackages)
assert.ErrorContains(t, err, `could not determine dependencies for bundle with id "fake-catalog-test-package-test-package.v1.0.0"`)
assert.Nil(t, bundles)
}

var _ = Describe("BundlesAndDepsVariableSource", func() {
var (
bdvs *variablesources.BundlesAndDepsVariableSource
Expand Down Expand Up @@ -374,7 +563,7 @@ var _ = Describe("BundlesAndDepsVariableSource", func() {
)
_, err := bdvs.GetVariables(context.TODO())
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("could not determine dependencies for bundle with id 'fake-catalog-test-package-bundle-2': could not find package dependencies for bundle 'bundle-2'"))
Expect(err.Error()).To(ContainSubstring(`could not determine dependencies for bundle with id "fake-catalog-test-package-bundle-2": could not find package dependencies for bundle`))
})

It("should return error if an inner variable source returns an error", func() {
Expand Down

0 comments on commit d92550d

Please sign in to comment.