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

Jump to bottom

Move data fetching to a single place #508

Merged
merged 1 commit into from
Nov 3, 2023
Merged

Move data fetching to a single place #508

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
52 changes: 47 additions & 5 deletions internal/controllers/variable_source.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,26 +17,68 @@ limitations under the License.
package controllers

import (
"context"

"sigs.k8s.io/controller-runtime/pkg/client"

"github.com/operator-framework/deppy/pkg/deppy"
"github.com/operator-framework/deppy/pkg/deppy/input"
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/resolution/variablesources"
)

func NewVariableSource(cl client.Client, catalogClient variablesources.BundleProvider) variablesources.NestedVariableSource {
return variablesources.NestedVariableSource{
// BundleProvider provides the way to retrieve a list of Bundles from a source,
// generally from a catalog client of some kind.
type BundleProvider interface {
m1kola marked this conversation as resolved.
Show resolved Hide resolved
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
}

// We are in process of getting rid of extra variable sources.
// See this for progress: https://github.com/operator-framework/operator-controller/issues/437
vs := variablesources.NestedVariableSource{
func(inputVariableSource input.VariableSource) (input.VariableSource, error) {
return variablesources.NewOperatorVariableSource(cl, catalogClient, inputVariableSource), nil
return variablesources.NewOperatorVariableSource(operatorList.Items, allBundles, inputVariableSource), nil
},
func(inputVariableSource input.VariableSource) (input.VariableSource, error) {
return variablesources.NewBundleDeploymentVariableSource(cl, catalogClient, inputVariableSource), nil
return variablesources.NewBundleDeploymentVariableSource(bundleDeploymentList.Items, allBundles, inputVariableSource), nil
},
func(inputVariableSource input.VariableSource) (input.VariableSource, error) {
return variablesources.NewBundlesAndDepsVariableSource(catalogClient, inputVariableSource), nil
return variablesources.NewBundlesAndDepsVariableSource(allBundles, inputVariableSource), nil
},
func(inputVariableSource input.VariableSource) (input.VariableSource, error) {
return variablesources.NewCRDUniquenessConstraintsVariableSource(inputVariableSource), nil
},
}
return vs.GetVariables(ctx)
}
25 changes: 11 additions & 14 deletions internal/resolution/variablesources/bundle_deployment.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,25 +3,27 @@ package variablesources
import (
"context"

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

"github.com/operator-framework/deppy/pkg/deppy"
"github.com/operator-framework/deppy/pkg/deppy/input"
rukpakv1alpha1 "github.com/operator-framework/rukpak/api/v1alpha1"
"k8s.io/apimachinery/pkg/util/sets"
"sigs.k8s.io/controller-runtime/pkg/client"

"github.com/operator-framework/operator-controller/internal/catalogmetadata"
)

var _ input.VariableSource = &BundleDeploymentVariableSource{}

type BundleDeploymentVariableSource struct {
client client.Client
catalogClient BundleProvider
bundleDeployments []rukpakv1alpha1.BundleDeployment
allBundles []*catalogmetadata.Bundle
inputVariableSource input.VariableSource
}

func NewBundleDeploymentVariableSource(cl client.Client, catalogClient BundleProvider, inputVariableSource input.VariableSource) *BundleDeploymentVariableSource {
func NewBundleDeploymentVariableSource(bundleDeployments []rukpakv1alpha1.BundleDeployment, allBundles []*catalogmetadata.Bundle, inputVariableSource input.VariableSource) *BundleDeploymentVariableSource {
return &BundleDeploymentVariableSource{
client: cl,
catalogClient: catalogClient,
bundleDeployments: bundleDeployments,
allBundles: allBundles,
inputVariableSource: inputVariableSource,
}
}
Expand All @@ -32,20 +34,15 @@ func (o *BundleDeploymentVariableSource) GetVariables(ctx context.Context) ([]de
variableSources = append(variableSources, o.inputVariableSource)
}

bundleDeployments := rukpakv1alpha1.BundleDeploymentList{}
if err := o.client.List(ctx, &bundleDeployments); err != nil {
return nil, err
}

processed := sets.Set[string]{}
for _, bundleDeployment := range bundleDeployments.Items {
for _, bundleDeployment := range o.bundleDeployments {
sourceImage := bundleDeployment.Spec.Template.Spec.Source.Image
if sourceImage != nil && sourceImage.Ref != "" {
if processed.Has(sourceImage.Ref) {
continue
}
processed.Insert(sourceImage.Ref)
ips, err := NewInstalledPackageVariableSource(o.catalogClient, bundleDeployment.Spec.Template.Spec.Source.Image.Ref)
ips, err := NewInstalledPackageVariableSource(o.allBundles, bundleDeployment.Spec.Template.Spec.Source.Image.Ref)
if err != nil {
return nil, err
}
Expand Down
30 changes: 10 additions & 20 deletions internal/resolution/variablesources/bundle_deployment_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,26 +13,15 @@ import (
"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"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/fake"

"github.com/operator-framework/deppy/pkg/deppy"
rukpakv1alpha1 "github.com/operator-framework/rukpak/api/v1alpha1"
)

func BundleDeploymentFakeClient(objects ...client.Object) client.Client {
scheme := runtime.NewScheme()
utilruntime.Must(rukpakv1alpha1.AddToScheme(scheme))
return fake.NewClientBuilder().WithScheme(scheme).WithObjects(objects...).Build()
}

func bundleDeployment(name, image string) *rukpakv1alpha1.BundleDeployment {
return &rukpakv1alpha1.BundleDeployment{
func bundleDeployment(name, image string) rukpakv1alpha1.BundleDeployment {
return rukpakv1alpha1.BundleDeployment{
ObjectMeta: metav1.ObjectMeta{
Name: name,
},
Expand All @@ -53,7 +42,6 @@ func bundleDeployment(name, image string) *rukpakv1alpha1.BundleDeployment {
}

var _ = Describe("BundleDeploymentVariableSource", func() {
var fakeCatalogClient testutil.FakeCatalogClient
var betaChannel catalogmetadata.Channel
var stableChannel catalogmetadata.Channel
var testBundleList []*catalogmetadata.Bundle
Expand Down Expand Up @@ -111,14 +99,14 @@ var _ = Describe("BundleDeploymentVariableSource", func() {
},
}, InChannels: []*catalogmetadata.Channel{&stableChannel}},
}

fakeCatalogClient = testutil.NewFakeCatalogClient(testBundleList)
})

It("should produce RequiredPackage variables", func() {
cl := BundleDeploymentFakeClient(bundleDeployment("prometheus", "quay.io/operatorhubio/prometheus@sha256:3e281e587de3d03011440685fc4fb782672beab044c1ebadc42788ce05a21c35"))
bundleDeployments := []rukpakv1alpha1.BundleDeployment{
bundleDeployment("prometheus", "quay.io/operatorhubio/prometheus@sha256:3e281e587de3d03011440685fc4fb782672beab044c1ebadc42788ce05a21c35"),
}

bdVariableSource := variablesources.NewBundleDeploymentVariableSource(cl, &fakeCatalogClient, &MockRequiredPackageSource{})
bdVariableSource := variablesources.NewBundleDeploymentVariableSource(bundleDeployments, testBundleList, &MockRequiredPackageSource{})
variables, err := bdVariableSource.GetVariables(context.Background())
Expect(err).ToNot(HaveOccurred())

Expand All @@ -137,9 +125,11 @@ var _ = Describe("BundleDeploymentVariableSource", func() {
})))
})
It("should return an error if the bundleDeployment image doesn't match any operator resource", func() {
cl := BundleDeploymentFakeClient(bundleDeployment("prometheus", "quay.io/operatorhubio/prometheus@sha256:nonexistent"))
bundleDeployments := []rukpakv1alpha1.BundleDeployment{
bundleDeployment("prometheus", "quay.io/operatorhubio/prometheus@sha256:nonexistent"),
}

bdVariableSource := variablesources.NewBundleDeploymentVariableSource(cl, &fakeCatalogClient, &MockRequiredPackageSource{})
bdVariableSource := variablesources.NewBundleDeploymentVariableSource(bundleDeployments, testBundleList, &MockRequiredPackageSource{})
_, err := bdVariableSource.GetVariables(context.Background())
Expect(err.Error()).To(Equal("bundleImage \"quay.io/operatorhubio/prometheus@sha256:nonexistent\" not found"))
})
Expand Down
14 changes: 0 additions & 14 deletions internal/resolution/variablesources/bundle_provider.go
View file
Open in desktop

This file was deleted.

13 changes: 4 additions & 9 deletions internal/resolution/variablesources/bundles_and_dependencies.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,13 @@ import (
var _ input.VariableSource = &BundlesAndDepsVariableSource{}

type BundlesAndDepsVariableSource struct {
catalogClient BundleProvider
allBundles []*catalogmetadata.Bundle
variableSources []input.VariableSource
}

func NewBundlesAndDepsVariableSource(catalogClient BundleProvider, inputVariableSources ...input.VariableSource) *BundlesAndDepsVariableSource {
func NewBundlesAndDepsVariableSource(allBundles []*catalogmetadata.Bundle, inputVariableSources ...input.VariableSource) *BundlesAndDepsVariableSource {
return &BundlesAndDepsVariableSource{
catalogClient: catalogClient,
allBundles: allBundles,
variableSources: inputVariableSources,
}
}
Expand Down Expand Up @@ -52,11 +52,6 @@ func (b *BundlesAndDepsVariableSource) GetVariables(ctx context.Context) ([]depp
}
}

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

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

// get bundle dependencies
dependencies, err := b.filterBundleDependencies(allBundles, head)
dependencies, err := b.filterBundleDependencies(b.allBundles, head)
if err != nil {
return nil, fmt.Errorf("could not determine dependencies for bundle with id '%s': %w", id, err)
}
Expand Down
20 changes: 7 additions & 13 deletions internal/resolution/variablesources/bundles_and_dependencies_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,12 @@ import (
"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"
)

var _ = Describe("BundlesAndDepsVariableSource", func() {
var (
bdvs *variablesources.BundlesAndDepsVariableSource
testBundleList []*catalogmetadata.Bundle
fakeCatalogClient testutil.FakeCatalogClient
bdvs *variablesources.BundlesAndDepsVariableSource
testBundleList []*catalogmetadata.Bundle
)

BeforeEach(func() {
Expand Down Expand Up @@ -222,12 +220,11 @@ var _ = Describe("BundlesAndDepsVariableSource", func() {
InChannels: []*catalogmetadata.Channel{&channel},
},
}
fakeCatalogClient = testutil.NewFakeCatalogClient(testBundleList)
bdvs = variablesources.NewBundlesAndDepsVariableSource(
&fakeCatalogClient,
testBundleList,
&MockRequiredPackageSource{
ResultSet: []deppy.Variable{
// must match data in fakeCatalogClient
// must match data in testBundleList
olmvariables.NewRequiredPackageVariable("test-package", []*catalogmetadata.Bundle{
{
CatalogName: "fake-catalog",
Expand Down Expand Up @@ -259,7 +256,7 @@ var _ = Describe("BundlesAndDepsVariableSource", func() {
},
&MockRequiredPackageSource{
ResultSet: []deppy.Variable{
// must match data in fakeCatalogClient
// must match data in testBundleList
olmvariables.NewRequiredPackageVariable("test-package-2", []*catalogmetadata.Bundle{
// test-package-2 required package - no dependencies
{
Expand Down Expand Up @@ -335,13 +332,10 @@ var _ = Describe("BundlesAndDepsVariableSource", func() {
})

It("should return error if dependencies not found", func() {
emptyCatalogClient := testutil.NewFakeCatalogClient(make([]*catalogmetadata.Bundle, 0))

bdvs = variablesources.NewBundlesAndDepsVariableSource(
&emptyCatalogClient,
[]*catalogmetadata.Bundle{},
&MockRequiredPackageSource{
ResultSet: []deppy.Variable{
// must match data in fakeCatalogClient
olmvariables.NewRequiredPackageVariable("test-package", []*catalogmetadata.Bundle{
{
CatalogName: "fake-catalog",
Expand Down Expand Up @@ -379,7 +373,7 @@ var _ = Describe("BundlesAndDepsVariableSource", func() {

It("should return error if an inner variable source returns an error", func() {
bdvs = variablesources.NewBundlesAndDepsVariableSource(
&fakeCatalogClient,
testBundleList,
&MockRequiredPackageSource{Error: errors.New("fake error")},
)
_, err := bdvs.GetVariables(context.TODO())
Expand Down
25 changes: 10 additions & 15 deletions internal/resolution/variablesources/installed_package.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,14 @@ import (
var _ input.VariableSource = &InstalledPackageVariableSource{}

type InstalledPackageVariableSource struct {
catalogClient BundleProvider
successors successorsFunc
bundleImage string
allBundles []*catalogmetadata.Bundle
successors successorsFunc
bundleImage string
}

func (r *InstalledPackageVariableSource) GetVariables(ctx context.Context) ([]deppy.Variable, error) {
allBundles, err := r.catalogClient.Bundles(ctx)
if err != nil {
return nil, err
}

func (r *InstalledPackageVariableSource) GetVariables(_ context.Context) ([]deppy.Variable, error) {
// find corresponding bundle for the installed content
resultSet := catalogfilter.Filter(allBundles, catalogfilter.WithBundleImage(r.bundleImage))
resultSet := catalogfilter.Filter(r.allBundles, catalogfilter.WithBundleImage(r.bundleImage))
if len(resultSet) == 0 {
return nil, r.notFoundError()
}
Expand All @@ -44,7 +39,7 @@ func (r *InstalledPackageVariableSource) GetVariables(ctx context.Context) ([]de
})
installedBundle := resultSet[0]

upgradeEdges, err := r.successors(allBundles, installedBundle)
upgradeEdges, err := r.successors(r.allBundles, installedBundle)
if err != nil {
return nil, err
}
Expand All @@ -60,16 +55,16 @@ func (r *InstalledPackageVariableSource) notFoundError() error {
return fmt.Errorf("bundleImage %q not found", r.bundleImage)
}

func NewInstalledPackageVariableSource(catalogClient BundleProvider, bundleImage string) (*InstalledPackageVariableSource, error) {
func NewInstalledPackageVariableSource(allBundles []*catalogmetadata.Bundle, bundleImage string) (*InstalledPackageVariableSource, error) {
successors := legacySemanticsSuccessors
if features.OperatorControllerFeatureGate.Enabled(features.ForceSemverUpgradeConstraints) {
successors = semverSuccessors
}

return &InstalledPackageVariableSource{
catalogClient: catalogClient,
bundleImage: bundleImage,
successors: successors,
allBundles: allBundles,
bundleImage: bundleImage,
successors: successors,
}, nil
}

Expand Down
Loading