Skip to content

Commit

Permalink
Fix tests, bump catalogd, address comments.
Browse files Browse the repository at this point in the history
Signed-off-by: dtfranz <dfranz@redhat.com>
  • Loading branch information
dtfranz authored and m1kola committed Sep 21, 2023
1 parent c02fe9b commit 9d88cb0
Show file tree
Hide file tree
Showing 27 changed files with 1,025 additions and 714 deletions.
14 changes: 7 additions & 7 deletions cmd/resolutioncli/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,24 +145,24 @@ func resolve(ctx context.Context, resolver *solver.DeppySolver, packageName stri
return "", err
}

bundleEntity, err := getBundleEntityFromSolution(solution, packageName)
bundle, err := bundleFromSolution(solution, packageName)
if err != nil {
return "", err
}

// Get the bundle image reference for the bundle
return bundleEntity.Image, nil
return bundle.Image, nil
}

func getBundleEntityFromSolution(solution *solver.Solution, packageName string) (*catalogmetadata.Bundle, error) {
func bundleFromSolution(solution *solver.Solution, packageName string) (*catalogmetadata.Bundle, error) {
for _, variable := range solution.SelectedVariables() {
switch v := variable.(type) {
case *olmvariables.BundleVariable:
entityPkgName := v.BundleEntity().Package
if packageName == entityPkgName {
return v.BundleEntity(), nil
bundlePkgName := v.Bundle().Package
if packageName == bundlePkgName {
return v.Bundle(), nil
}
}
}
return nil, fmt.Errorf("entity for package %q not found in solution", packageName)
return nil, fmt.Errorf("bundle for package %q not found in solution", packageName)
}
5 changes: 2 additions & 3 deletions cmd/resolutioncli/variable_source.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,13 @@ package main
import (
"github.com/operator-framework/deppy/pkg/deppy/input"

catalogclient "github.com/operator-framework/operator-controller/internal/catalogmetadata/client"
"github.com/operator-framework/operator-controller/internal/resolution/variablesources"
)

func newPackageVariableSource(catalog catalogclient.CatalogClient, packageName, packageVersion, packageChannel string) func(inputVariableSource input.VariableSource) (input.VariableSource, error) {
func newPackageVariableSource(catalogClient *indexRefClient, packageName, packageVersion, packageChannel string) func(inputVariableSource input.VariableSource) (input.VariableSource, error) {
return func(inputVariableSource input.VariableSource) (input.VariableSource, error) {
pkgSource, err := variablesources.NewRequiredPackageVariableSource(
catalog,
catalogClient,
packageName,
variablesources.InVersionRange(packageVersion),
variablesources.InChannel(packageChannel),
Expand Down
1 change: 0 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ require (
github.com/Masterminds/semver/v3 v3.2.1
github.com/blang/semver/v4 v4.0.0
github.com/go-logr/logr v1.2.4
github.com/google/go-cmp v0.5.9
github.com/onsi/ginkgo/v2 v2.12.1
github.com/onsi/gomega v1.27.10
github.com/operator-framework/api v0.17.4-0.20230223191600-0131a6301e42
Expand Down
4 changes: 0 additions & 4 deletions internal/catalogmetadata/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,6 @@ import (
"github.com/operator-framework/operator-controller/internal/catalogmetadata"
)

type CatalogClient interface {
Bundles(ctx context.Context) ([]*catalogmetadata.Bundle, error)
}

func New(cl client.Client) *Client {
return &Client{cl: cl}
}
Expand Down
24 changes: 12 additions & 12 deletions internal/controllers/operator_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,9 +154,9 @@ func (r *OperatorReconciler) reconcile(ctx context.Context, op *operatorsv1alpha
return ctrl.Result{}, unsat
}

// lookup the bundle entity in the solution that corresponds to the
// lookup the bundle in the solution that corresponds to the
// Operator's desired package name.
bundleEntity, err := r.getBundleEntityFromSolution(solution, op.Spec.PackageName)
bundle, err := r.bundleFromSolution(solution, op.Spec.PackageName)
if err != nil {
op.Status.InstalledBundleResource = ""
setInstalledStatusConditionUnknown(&op.Status.Conditions, "installation has not been attempted as resolution failed", op.GetGeneration())
Expand All @@ -165,11 +165,11 @@ func (r *OperatorReconciler) reconcile(ctx context.Context, op *operatorsv1alpha
return ctrl.Result{}, err
}

// Now we can set the Resolved Condition, and the resolvedBundleSource field to the bundleEntity.Image value.
op.Status.ResolvedBundleResource = bundleEntity.Image
setResolvedStatusConditionSuccess(&op.Status.Conditions, fmt.Sprintf("resolved to %q", bundleEntity.Image), op.GetGeneration())
// Now we can set the Resolved Condition, and the resolvedBundleSource field to the bundle.Image value.
op.Status.ResolvedBundleResource = bundle.Image
setResolvedStatusConditionSuccess(&op.Status.Conditions, fmt.Sprintf("resolved to %q", bundle.Image), op.GetGeneration())

mediaType, err := bundleEntity.MediaType()
mediaType, err := bundle.MediaType()
if err != nil {
setInstalledStatusConditionFailed(&op.Status.Conditions, err.Error(), op.GetGeneration())
return ctrl.Result{}, err
Expand All @@ -181,7 +181,7 @@ func (r *OperatorReconciler) reconcile(ctx context.Context, op *operatorsv1alpha
}
// Ensure a BundleDeployment exists with its bundle source from the bundle
// image we just looked up in the solution.
dep := r.generateExpectedBundleDeployment(*op, bundleEntity.Image, bundleProvisioner)
dep := r.generateExpectedBundleDeployment(*op, bundle.Image, bundleProvisioner)
if err := r.ensureBundleDeployment(ctx, dep); err != nil {
// originally Reason: operatorsv1alpha1.ReasonInstallationFailed
op.Status.InstalledBundleResource = ""
Expand Down Expand Up @@ -251,17 +251,17 @@ func mapBDStatusToInstalledCondition(existingTypedBundleDeployment *rukpakv1alph
}
}

func (r *OperatorReconciler) getBundleEntityFromSolution(solution *solver.Solution, packageName string) (*catalogmetadata.Bundle, error) {
func (r *OperatorReconciler) bundleFromSolution(solution *solver.Solution, packageName string) (*catalogmetadata.Bundle, error) {
for _, variable := range solution.SelectedVariables() {
switch v := variable.(type) {
case *olmvariables.BundleVariable:
entityPkgName := v.BundleEntity().Package
if packageName == entityPkgName {
return v.BundleEntity(), nil
bundlePkgName := v.Bundle().Package
if packageName == bundlePkgName {
return v.Bundle(), nil
}
}
}
return nil, fmt.Errorf("entity for package %q not found in solution", packageName)
return nil, fmt.Errorf("bundle for package %q not found in solution", packageName)

Check warning on line 264 in internal/controllers/operator_controller.go

View check run for this annotation

Codecov / codecov/patch

internal/controllers/operator_controller.go#L264

Added line #L264 was not covered by tests
}

func (r *OperatorReconciler) generateExpectedBundleDeployment(o operatorsv1alpha1.Operator, bundlePath string, bundleProvisioner string) *unstructured.Unstructured {
Expand Down
149 changes: 69 additions & 80 deletions internal/controllers/operator_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,14 @@ package controllers_test

import (
"context"
"encoding/json"
"fmt"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"github.com/operator-framework/deppy/pkg/deppy"
"github.com/operator-framework/deppy/pkg/deppy/input"
"github.com/operator-framework/deppy/pkg/deppy/solver"
"github.com/operator-framework/operator-registry/alpha/declcfg"
"github.com/operator-framework/operator-registry/alpha/property"
rukpakv1alpha1 "github.com/operator-framework/rukpak/api/v1alpha1"
apimeta "k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand All @@ -20,21 +21,25 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client/fake"

operatorsv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1"
"github.com/operator-framework/operator-controller/internal/catalogmetadata"
"github.com/operator-framework/operator-controller/internal/conditionsets"
"github.com/operator-framework/operator-controller/internal/controllers"
testutil "github.com/operator-framework/operator-controller/test/util"
)

var _ = Describe("Operator Controller Test", func() {
var (
ctx context.Context
reconciler *controllers.OperatorReconciler
ctx context.Context
fakeCatalogClient testutil.FakeCatalogClient
reconciler *controllers.OperatorReconciler
)
BeforeEach(func() {
ctx = context.Background()
fakeCatalogClient = testutil.NewFakeCatalogClient(testBundleList)
reconciler = &controllers.OperatorReconciler{
Client: cl,
Scheme: sch,
Resolver: solver.NewDeppySolver(testEntitySource, controllers.NewVariableSource(cl)),
Resolver: solver.NewDeppySolver(controllers.NewVariableSource(cl, &fakeCatalogClient)),
}
})
When("the operator does not exist", func() {
Expand Down Expand Up @@ -575,43 +580,6 @@ var _ = Describe("Operator Controller Test", func() {
})
})
})
When("the selected bundle's image ref cannot be parsed", func() {
const pkgName = "badimage"
BeforeEach(func() {
By("initializing cluster state")
operator = &operatorsv1alpha1.Operator{
ObjectMeta: metav1.ObjectMeta{Name: opKey.Name},
Spec: operatorsv1alpha1.OperatorSpec{PackageName: pkgName},
}
err := cl.Create(ctx, operator)
Expect(err).NotTo(HaveOccurred())
})
It("sets resolution failure status and returns an error", func() {
By("running reconcile")
res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: opKey})
Expect(res).To(Equal(ctrl.Result{}))
Expect(err).To(MatchError(ContainSubstring(`error determining bundle path for entity`)))

By("fetching updated operator after reconcile")
Expect(cl.Get(ctx, opKey, operator)).NotTo(HaveOccurred())

By("Checking the status fields")
Expect(operator.Status.ResolvedBundleResource).To(Equal(""))
Expect(operator.Status.InstalledBundleResource).To(Equal(""))

By("checking the expected conditions")
cond := apimeta.FindStatusCondition(operator.Status.Conditions, operatorsv1alpha1.TypeResolved)
Expect(cond).NotTo(BeNil())
Expect(cond.Status).To(Equal(metav1.ConditionFalse))
Expect(cond.Reason).To(Equal(operatorsv1alpha1.ReasonResolutionFailed))
Expect(cond.Message).To(ContainSubstring(`error determining bundle path for entity`))
cond = apimeta.FindStatusCondition(operator.Status.Conditions, operatorsv1alpha1.TypeInstalled)
Expect(cond).NotTo(BeNil())
Expect(cond.Status).To(Equal(metav1.ConditionUnknown))
Expect(cond.Reason).To(Equal(operatorsv1alpha1.ReasonInstallationStatusUnknown))
Expect(cond.Message).To(Equal("installation has not been attempted as resolution failed"))
})
})
When("the operator specifies a duplicate package", func() {
const pkgName = "prometheus"
var dupOperator *operatorsv1alpha1.Operator
Expand Down Expand Up @@ -1080,41 +1048,62 @@ func verifyConditionsInvariants(op *operatorsv1alpha1.Operator) {
}
}

var testEntitySource = input.NewCacheQuerier(map[deppy.Identifier]input.Entity{
"operatorhub/prometheus/0.37.0": *input.NewEntity("operatorhub/prometheus/0.37.0", map[string]string{
"olm.bundle.path": `"quay.io/operatorhubio/prometheus@sha256:3e281e587de3d03011440685fc4fb782672beab044c1ebadc42788ce05a21c35"`,
"olm.bundle.channelEntry": `{"name":"prometheus.0.37.0"}`,
"olm.channel": `{"channelName":"beta","priority":0}`,
"olm.package": `{"packageName":"prometheus","version":"0.37.0"}`,
"olm.gvk": `[]`,
}),
"operatorhub/prometheus/0.47.0": *input.NewEntity("operatorhub/prometheus/0.47.0", map[string]string{
"olm.bundle.path": `"quay.io/operatorhubio/prometheus@sha256:5b04c49d8d3eff6a338b56ec90bdf491d501fe301c9cdfb740e5bff6769a21ed"`,
"olm.bundle.channelEntry": `{"name":"prometheus.0.47.0"}`,
"olm.channel": `{"channelName":"beta","priority":0,"replaces":"prometheusoperator.0.37.0"}`,
"olm.package": `{"packageName":"prometheus","version":"0.47.0"}`,
"olm.gvk": `[]`,
}),
"operatorhub/badimage/0.1.0": *input.NewEntity("operatorhub/badimage/0.1.0", map[string]string{
"olm.bundle.path": `{"name": "quay.io/operatorhubio/badimage:v0.1.0"}`,
"olm.bundle.channelEntry": `{"name":"badimage.0.1.0"}`,
"olm.package": `{"packageName":"badimage","version":"0.1.0"}`,
"olm.gvk": `[]`,
}),
"operatorhub/plain/0.1.0": *input.NewEntity("operatorhub/plain/0.1.0", map[string]string{
"olm.bundle.path": `"quay.io/operatorhub/plain@sha256:plain"`,
"olm.bundle.channelEntry": `{"name":"plain.0.1.0"}`,
"olm.channel": `{"channelName":"beta","priority":0}`,
"olm.package": `{"packageName":"plain","version":"0.1.0"}`,
"olm.gvk": `[]`,
"olm.bundle.mediatype": `"plain+v0"`,
}),
"operatorhub/badmedia/0.1.0": *input.NewEntity("operatorhub/badmedia/0.1.0", map[string]string{
"olm.bundle.path": `"quay.io/operatorhub/badmedia@sha256:badmedia"`,
"olm.bundle.channelEntry": `{"name":"badmedia.0.1.0"}`,
"olm.channel": `{"channelName":"beta","priority":0}`,
"olm.package": `{"packageName":"badmedia","version":"0.1.0"}`,
"olm.gvk": `[]`,
"olm.bundle.mediatype": `"badmedia+v1"`,
}),
})
var betaChannel = catalogmetadata.Channel{Channel: declcfg.Channel{
Name: "beta",
Entries: []declcfg.ChannelEntry{
{
Name: "operatorhub/prometheus/0.37.0",
},
{
Name: "operatorhub/prometheus/0.47.0",
Replaces: "operatorhub/prometheus/0.37.0",
},
{
Name: "operatorhub/plain/0.1.0",
},
{
Name: "operatorhub/badmedia/0.1.0",
},
},
}}

var testBundleList = []*catalogmetadata.Bundle{
{Bundle: declcfg.Bundle{
Name: "operatorhub/prometheus/0.37.0",
Package: "prometheus",
Image: "quay.io/operatorhubio/prometheus@sha256:3e281e587de3d03011440685fc4fb782672beab044c1ebadc42788ce05a21c35",
Properties: []property.Property{
{Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"prometheus","version":"0.37.0"}`)},
{Type: property.TypeGVK, Value: json.RawMessage(`[]`)},
},
}, InChannels: []*catalogmetadata.Channel{&betaChannel}},
{Bundle: declcfg.Bundle{
Name: "operatorhub/prometheus/0.47.0",
Package: "prometheus",
Image: "quay.io/operatorhubio/prometheus@sha256:5b04c49d8d3eff6a338b56ec90bdf491d501fe301c9cdfb740e5bff6769a21ed",
Properties: []property.Property{
{Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"prometheus","version":"0.47.0"}`)},
{Type: property.TypeGVK, Value: json.RawMessage(`[]`)},
},
}, InChannels: []*catalogmetadata.Channel{&betaChannel}},
{Bundle: declcfg.Bundle{
Name: "operatorhub/plain/0.1.0",
Package: "plain",
Image: "quay.io/operatorhub/plain@sha256:plain",
Properties: []property.Property{
{Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"plain","version":"0.1.0"}`)},
{Type: property.TypeGVK, Value: json.RawMessage(`[]`)},
{Type: "olm.bundle.mediatype", Value: json.RawMessage(`"plain+v0"`)},
},
}, InChannels: []*catalogmetadata.Channel{&betaChannel}},
{Bundle: declcfg.Bundle{
Name: "operatorhub/badmedia/0.1.0",
Package: "badmedia",
Image: "quay.io/operatorhub/badmedia@sha256:badmedia",
Properties: []property.Property{
{Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"badmedia","version":"0.1.0"}`)},
{Type: property.TypeGVK, Value: json.RawMessage(`[]`)},
{Type: "olm.bundle.mediatype", Value: json.RawMessage(`"badmedia+v1"`)},
},
}, InChannels: []*catalogmetadata.Channel{&betaChannel}},
}
9 changes: 4 additions & 5 deletions internal/controllers/variable_source.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,20 +21,19 @@ import (

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

catalogclient "github.com/operator-framework/operator-controller/internal/catalogmetadata/client"
"github.com/operator-framework/operator-controller/internal/resolution/variablesources"
)

func NewVariableSource(cl client.Client, catalog catalogclient.CatalogClient) variablesources.NestedVariableSource {
func NewVariableSource(cl client.Client, catalogClient variablesources.BundleProvider) variablesources.NestedVariableSource {
return variablesources.NestedVariableSource{
func(inputVariableSource input.VariableSource) (input.VariableSource, error) {
return variablesources.NewOperatorVariableSource(cl, catalog, inputVariableSource), nil
return variablesources.NewOperatorVariableSource(cl, catalogClient, inputVariableSource), nil
},
func(inputVariableSource input.VariableSource) (input.VariableSource, error) {
return variablesources.NewBundleDeploymentVariableSource(cl, catalog, inputVariableSource), nil
return variablesources.NewBundleDeploymentVariableSource(cl, catalogClient, inputVariableSource), nil
},
func(inputVariableSource input.VariableSource) (input.VariableSource, error) {
return variablesources.NewBundlesAndDepsVariableSource(catalog, inputVariableSource), nil
return variablesources.NewBundlesAndDepsVariableSource(catalogClient, inputVariableSource), nil
},
func(inputVariableSource input.VariableSource) (input.VariableSource, error) {
return variablesources.NewCRDUniquenessConstraintsVariableSource(inputVariableSource), nil
Expand Down
2 changes: 1 addition & 1 deletion internal/resolution/variables/bundle.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ type BundleVariable struct {
dependencies []*catalogmetadata.Bundle
}

func (b *BundleVariable) BundleEntity() *catalogmetadata.Bundle {
func (b *BundleVariable) Bundle() *catalogmetadata.Bundle {
return b.bundle
}

Expand Down
Loading

0 comments on commit 9d88cb0

Please sign in to comment.