Skip to content

Commit 9d88cb0

Browse files
dtfranzMikalai Radchuk
authored andcommitted
Fix tests, bump catalogd, address comments.
Signed-off-by: dtfranz <dfranz@redhat.com>
1 parent c02fe9b commit 9d88cb0

27 files changed

+1025
-714
lines changed

cmd/resolutioncli/main.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -145,24 +145,24 @@ func resolve(ctx context.Context, resolver *solver.DeppySolver, packageName stri
145145
return "", err
146146
}
147147

148-
bundleEntity, err := getBundleEntityFromSolution(solution, packageName)
148+
bundle, err := bundleFromSolution(solution, packageName)
149149
if err != nil {
150150
return "", err
151151
}
152152

153153
// Get the bundle image reference for the bundle
154-
return bundleEntity.Image, nil
154+
return bundle.Image, nil
155155
}
156156

157-
func getBundleEntityFromSolution(solution *solver.Solution, packageName string) (*catalogmetadata.Bundle, error) {
157+
func bundleFromSolution(solution *solver.Solution, packageName string) (*catalogmetadata.Bundle, error) {
158158
for _, variable := range solution.SelectedVariables() {
159159
switch v := variable.(type) {
160160
case *olmvariables.BundleVariable:
161-
entityPkgName := v.BundleEntity().Package
162-
if packageName == entityPkgName {
163-
return v.BundleEntity(), nil
161+
bundlePkgName := v.Bundle().Package
162+
if packageName == bundlePkgName {
163+
return v.Bundle(), nil
164164
}
165165
}
166166
}
167-
return nil, fmt.Errorf("entity for package %q not found in solution", packageName)
167+
return nil, fmt.Errorf("bundle for package %q not found in solution", packageName)
168168
}

cmd/resolutioncli/variable_source.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,13 @@ package main
1919
import (
2020
"github.com/operator-framework/deppy/pkg/deppy/input"
2121

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

26-
func newPackageVariableSource(catalog catalogclient.CatalogClient, packageName, packageVersion, packageChannel string) func(inputVariableSource input.VariableSource) (input.VariableSource, error) {
25+
func newPackageVariableSource(catalogClient *indexRefClient, packageName, packageVersion, packageChannel string) func(inputVariableSource input.VariableSource) (input.VariableSource, error) {
2726
return func(inputVariableSource input.VariableSource) (input.VariableSource, error) {
2827
pkgSource, err := variablesources.NewRequiredPackageVariableSource(
29-
catalog,
28+
catalogClient,
3029
packageName,
3130
variablesources.InVersionRange(packageVersion),
3231
variablesources.InChannel(packageChannel),

go.mod

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ require (
66
github.com/Masterminds/semver/v3 v3.2.1
77
github.com/blang/semver/v4 v4.0.0
88
github.com/go-logr/logr v1.2.4
9-
github.com/google/go-cmp v0.5.9
109
github.com/onsi/ginkgo/v2 v2.12.1
1110
github.com/onsi/gomega v1.27.10
1211
github.com/operator-framework/api v0.17.4-0.20230223191600-0131a6301e42

internal/catalogmetadata/client/client.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,6 @@ import (
1212
"github.com/operator-framework/operator-controller/internal/catalogmetadata"
1313
)
1414

15-
type CatalogClient interface {
16-
Bundles(ctx context.Context) ([]*catalogmetadata.Bundle, error)
17-
}
18-
1915
func New(cl client.Client) *Client {
2016
return &Client{cl: cl}
2117
}

internal/controllers/operator_controller.go

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -154,9 +154,9 @@ func (r *OperatorReconciler) reconcile(ctx context.Context, op *operatorsv1alpha
154154
return ctrl.Result{}, unsat
155155
}
156156

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

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

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

254-
func (r *OperatorReconciler) getBundleEntityFromSolution(solution *solver.Solution, packageName string) (*catalogmetadata.Bundle, error) {
254+
func (r *OperatorReconciler) bundleFromSolution(solution *solver.Solution, packageName string) (*catalogmetadata.Bundle, error) {
255255
for _, variable := range solution.SelectedVariables() {
256256
switch v := variable.(type) {
257257
case *olmvariables.BundleVariable:
258-
entityPkgName := v.BundleEntity().Package
259-
if packageName == entityPkgName {
260-
return v.BundleEntity(), nil
258+
bundlePkgName := v.Bundle().Package
259+
if packageName == bundlePkgName {
260+
return v.Bundle(), nil
261261
}
262262
}
263263
}
264-
return nil, fmt.Errorf("entity for package %q not found in solution", packageName)
264+
return nil, fmt.Errorf("bundle for package %q not found in solution", packageName)
265265
}
266266

267267
func (r *OperatorReconciler) generateExpectedBundleDeployment(o operatorsv1alpha1.Operator, bundlePath string, bundleProvisioner string) *unstructured.Unstructured {

internal/controllers/operator_controller_test.go

Lines changed: 69 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,14 @@ package controllers_test
22

33
import (
44
"context"
5+
"encoding/json"
56
"fmt"
67

78
. "github.com/onsi/ginkgo/v2"
89
. "github.com/onsi/gomega"
9-
"github.com/operator-framework/deppy/pkg/deppy"
10-
"github.com/operator-framework/deppy/pkg/deppy/input"
1110
"github.com/operator-framework/deppy/pkg/deppy/solver"
11+
"github.com/operator-framework/operator-registry/alpha/declcfg"
12+
"github.com/operator-framework/operator-registry/alpha/property"
1213
rukpakv1alpha1 "github.com/operator-framework/rukpak/api/v1alpha1"
1314
apimeta "k8s.io/apimachinery/pkg/api/meta"
1415
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -20,21 +21,25 @@ import (
2021
"sigs.k8s.io/controller-runtime/pkg/client/fake"
2122

2223
operatorsv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1"
24+
"github.com/operator-framework/operator-controller/internal/catalogmetadata"
2325
"github.com/operator-framework/operator-controller/internal/conditionsets"
2426
"github.com/operator-framework/operator-controller/internal/controllers"
27+
testutil "github.com/operator-framework/operator-controller/test/util"
2528
)
2629

2730
var _ = Describe("Operator Controller Test", func() {
2831
var (
29-
ctx context.Context
30-
reconciler *controllers.OperatorReconciler
32+
ctx context.Context
33+
fakeCatalogClient testutil.FakeCatalogClient
34+
reconciler *controllers.OperatorReconciler
3135
)
3236
BeforeEach(func() {
3337
ctx = context.Background()
38+
fakeCatalogClient = testutil.NewFakeCatalogClient(testBundleList)
3439
reconciler = &controllers.OperatorReconciler{
3540
Client: cl,
3641
Scheme: sch,
37-
Resolver: solver.NewDeppySolver(testEntitySource, controllers.NewVariableSource(cl)),
42+
Resolver: solver.NewDeppySolver(controllers.NewVariableSource(cl, &fakeCatalogClient)),
3843
}
3944
})
4045
When("the operator does not exist", func() {
@@ -575,43 +580,6 @@ var _ = Describe("Operator Controller Test", func() {
575580
})
576581
})
577582
})
578-
When("the selected bundle's image ref cannot be parsed", func() {
579-
const pkgName = "badimage"
580-
BeforeEach(func() {
581-
By("initializing cluster state")
582-
operator = &operatorsv1alpha1.Operator{
583-
ObjectMeta: metav1.ObjectMeta{Name: opKey.Name},
584-
Spec: operatorsv1alpha1.OperatorSpec{PackageName: pkgName},
585-
}
586-
err := cl.Create(ctx, operator)
587-
Expect(err).NotTo(HaveOccurred())
588-
})
589-
It("sets resolution failure status and returns an error", func() {
590-
By("running reconcile")
591-
res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: opKey})
592-
Expect(res).To(Equal(ctrl.Result{}))
593-
Expect(err).To(MatchError(ContainSubstring(`error determining bundle path for entity`)))
594-
595-
By("fetching updated operator after reconcile")
596-
Expect(cl.Get(ctx, opKey, operator)).NotTo(HaveOccurred())
597-
598-
By("Checking the status fields")
599-
Expect(operator.Status.ResolvedBundleResource).To(Equal(""))
600-
Expect(operator.Status.InstalledBundleResource).To(Equal(""))
601-
602-
By("checking the expected conditions")
603-
cond := apimeta.FindStatusCondition(operator.Status.Conditions, operatorsv1alpha1.TypeResolved)
604-
Expect(cond).NotTo(BeNil())
605-
Expect(cond.Status).To(Equal(metav1.ConditionFalse))
606-
Expect(cond.Reason).To(Equal(operatorsv1alpha1.ReasonResolutionFailed))
607-
Expect(cond.Message).To(ContainSubstring(`error determining bundle path for entity`))
608-
cond = apimeta.FindStatusCondition(operator.Status.Conditions, operatorsv1alpha1.TypeInstalled)
609-
Expect(cond).NotTo(BeNil())
610-
Expect(cond.Status).To(Equal(metav1.ConditionUnknown))
611-
Expect(cond.Reason).To(Equal(operatorsv1alpha1.ReasonInstallationStatusUnknown))
612-
Expect(cond.Message).To(Equal("installation has not been attempted as resolution failed"))
613-
})
614-
})
615583
When("the operator specifies a duplicate package", func() {
616584
const pkgName = "prometheus"
617585
var dupOperator *operatorsv1alpha1.Operator
@@ -1080,41 +1048,62 @@ func verifyConditionsInvariants(op *operatorsv1alpha1.Operator) {
10801048
}
10811049
}
10821050

1083-
var testEntitySource = input.NewCacheQuerier(map[deppy.Identifier]input.Entity{
1084-
"operatorhub/prometheus/0.37.0": *input.NewEntity("operatorhub/prometheus/0.37.0", map[string]string{
1085-
"olm.bundle.path": `"quay.io/operatorhubio/prometheus@sha256:3e281e587de3d03011440685fc4fb782672beab044c1ebadc42788ce05a21c35"`,
1086-
"olm.bundle.channelEntry": `{"name":"prometheus.0.37.0"}`,
1087-
"olm.channel": `{"channelName":"beta","priority":0}`,
1088-
"olm.package": `{"packageName":"prometheus","version":"0.37.0"}`,
1089-
"olm.gvk": `[]`,
1090-
}),
1091-
"operatorhub/prometheus/0.47.0": *input.NewEntity("operatorhub/prometheus/0.47.0", map[string]string{
1092-
"olm.bundle.path": `"quay.io/operatorhubio/prometheus@sha256:5b04c49d8d3eff6a338b56ec90bdf491d501fe301c9cdfb740e5bff6769a21ed"`,
1093-
"olm.bundle.channelEntry": `{"name":"prometheus.0.47.0"}`,
1094-
"olm.channel": `{"channelName":"beta","priority":0,"replaces":"prometheusoperator.0.37.0"}`,
1095-
"olm.package": `{"packageName":"prometheus","version":"0.47.0"}`,
1096-
"olm.gvk": `[]`,
1097-
}),
1098-
"operatorhub/badimage/0.1.0": *input.NewEntity("operatorhub/badimage/0.1.0", map[string]string{
1099-
"olm.bundle.path": `{"name": "quay.io/operatorhubio/badimage:v0.1.0"}`,
1100-
"olm.bundle.channelEntry": `{"name":"badimage.0.1.0"}`,
1101-
"olm.package": `{"packageName":"badimage","version":"0.1.0"}`,
1102-
"olm.gvk": `[]`,
1103-
}),
1104-
"operatorhub/plain/0.1.0": *input.NewEntity("operatorhub/plain/0.1.0", map[string]string{
1105-
"olm.bundle.path": `"quay.io/operatorhub/plain@sha256:plain"`,
1106-
"olm.bundle.channelEntry": `{"name":"plain.0.1.0"}`,
1107-
"olm.channel": `{"channelName":"beta","priority":0}`,
1108-
"olm.package": `{"packageName":"plain","version":"0.1.0"}`,
1109-
"olm.gvk": `[]`,
1110-
"olm.bundle.mediatype": `"plain+v0"`,
1111-
}),
1112-
"operatorhub/badmedia/0.1.0": *input.NewEntity("operatorhub/badmedia/0.1.0", map[string]string{
1113-
"olm.bundle.path": `"quay.io/operatorhub/badmedia@sha256:badmedia"`,
1114-
"olm.bundle.channelEntry": `{"name":"badmedia.0.1.0"}`,
1115-
"olm.channel": `{"channelName":"beta","priority":0}`,
1116-
"olm.package": `{"packageName":"badmedia","version":"0.1.0"}`,
1117-
"olm.gvk": `[]`,
1118-
"olm.bundle.mediatype": `"badmedia+v1"`,
1119-
}),
1120-
})
1051+
var betaChannel = catalogmetadata.Channel{Channel: declcfg.Channel{
1052+
Name: "beta",
1053+
Entries: []declcfg.ChannelEntry{
1054+
{
1055+
Name: "operatorhub/prometheus/0.37.0",
1056+
},
1057+
{
1058+
Name: "operatorhub/prometheus/0.47.0",
1059+
Replaces: "operatorhub/prometheus/0.37.0",
1060+
},
1061+
{
1062+
Name: "operatorhub/plain/0.1.0",
1063+
},
1064+
{
1065+
Name: "operatorhub/badmedia/0.1.0",
1066+
},
1067+
},
1068+
}}
1069+
1070+
var testBundleList = []*catalogmetadata.Bundle{
1071+
{Bundle: declcfg.Bundle{
1072+
Name: "operatorhub/prometheus/0.37.0",
1073+
Package: "prometheus",
1074+
Image: "quay.io/operatorhubio/prometheus@sha256:3e281e587de3d03011440685fc4fb782672beab044c1ebadc42788ce05a21c35",
1075+
Properties: []property.Property{
1076+
{Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"prometheus","version":"0.37.0"}`)},
1077+
{Type: property.TypeGVK, Value: json.RawMessage(`[]`)},
1078+
},
1079+
}, InChannels: []*catalogmetadata.Channel{&betaChannel}},
1080+
{Bundle: declcfg.Bundle{
1081+
Name: "operatorhub/prometheus/0.47.0",
1082+
Package: "prometheus",
1083+
Image: "quay.io/operatorhubio/prometheus@sha256:5b04c49d8d3eff6a338b56ec90bdf491d501fe301c9cdfb740e5bff6769a21ed",
1084+
Properties: []property.Property{
1085+
{Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"prometheus","version":"0.47.0"}`)},
1086+
{Type: property.TypeGVK, Value: json.RawMessage(`[]`)},
1087+
},
1088+
}, InChannels: []*catalogmetadata.Channel{&betaChannel}},
1089+
{Bundle: declcfg.Bundle{
1090+
Name: "operatorhub/plain/0.1.0",
1091+
Package: "plain",
1092+
Image: "quay.io/operatorhub/plain@sha256:plain",
1093+
Properties: []property.Property{
1094+
{Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"plain","version":"0.1.0"}`)},
1095+
{Type: property.TypeGVK, Value: json.RawMessage(`[]`)},
1096+
{Type: "olm.bundle.mediatype", Value: json.RawMessage(`"plain+v0"`)},
1097+
},
1098+
}, InChannels: []*catalogmetadata.Channel{&betaChannel}},
1099+
{Bundle: declcfg.Bundle{
1100+
Name: "operatorhub/badmedia/0.1.0",
1101+
Package: "badmedia",
1102+
Image: "quay.io/operatorhub/badmedia@sha256:badmedia",
1103+
Properties: []property.Property{
1104+
{Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"badmedia","version":"0.1.0"}`)},
1105+
{Type: property.TypeGVK, Value: json.RawMessage(`[]`)},
1106+
{Type: "olm.bundle.mediatype", Value: json.RawMessage(`"badmedia+v1"`)},
1107+
},
1108+
}, InChannels: []*catalogmetadata.Channel{&betaChannel}},
1109+
}

internal/controllers/variable_source.go

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,20 +21,19 @@ import (
2121

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

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

28-
func NewVariableSource(cl client.Client, catalog catalogclient.CatalogClient) variablesources.NestedVariableSource {
27+
func NewVariableSource(cl client.Client, catalogClient variablesources.BundleProvider) variablesources.NestedVariableSource {
2928
return variablesources.NestedVariableSource{
3029
func(inputVariableSource input.VariableSource) (input.VariableSource, error) {
31-
return variablesources.NewOperatorVariableSource(cl, catalog, inputVariableSource), nil
30+
return variablesources.NewOperatorVariableSource(cl, catalogClient, inputVariableSource), nil
3231
},
3332
func(inputVariableSource input.VariableSource) (input.VariableSource, error) {
34-
return variablesources.NewBundleDeploymentVariableSource(cl, catalog, inputVariableSource), nil
33+
return variablesources.NewBundleDeploymentVariableSource(cl, catalogClient, inputVariableSource), nil
3534
},
3635
func(inputVariableSource input.VariableSource) (input.VariableSource, error) {
37-
return variablesources.NewBundlesAndDepsVariableSource(catalog, inputVariableSource), nil
36+
return variablesources.NewBundlesAndDepsVariableSource(catalogClient, inputVariableSource), nil
3837
},
3938
func(inputVariableSource input.VariableSource) (input.VariableSource, error) {
4039
return variablesources.NewCRDUniquenessConstraintsVariableSource(inputVariableSource), nil

internal/resolution/variables/bundle.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ type BundleVariable struct {
1818
dependencies []*catalogmetadata.Bundle
1919
}
2020

21-
func (b *BundleVariable) BundleEntity() *catalogmetadata.Bundle {
21+
func (b *BundleVariable) Bundle() *catalogmetadata.Bundle {
2222
return b.bundle
2323
}
2424

0 commit comments

Comments
 (0)