Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit c5230bd

Browse files
joelanfordjoelanford
committedJul 11, 2024··
wip2: switch to resolver interface
Signed-off-by: Joe Lanford <[email protected]>
1 parent afdfe7b commit c5230bd

File tree

16 files changed

+300
-1044
lines changed

16 files changed

+300
-1044
lines changed
 

‎cmd/manager/main.go

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"context"
2121
"flag"
2222
"fmt"
23+
"github.com/operator-framework/operator-controller/internal/resolve"
2324
"net/url"
2425
"os"
2526
"path/filepath"
@@ -230,16 +231,19 @@ func main() {
230231
}
231232

232233
if err = (&controllers.ClusterExtensionReconciler{
233-
Client: cl,
234-
CatalogPackageProvider: catalogClient,
235-
ActionClientGetter: acg,
236-
Unpacker: unpacker,
237-
Storage: localStorage,
238-
InstalledBundleGetter: &controllers.DefaultInstalledBundleGetter{ActionClientGetter: acg},
239-
Handler: registryv1handler.HandlerFunc(registry.HandleBundleDeployment),
240-
Finalizers: clusterExtensionFinalizers,
241-
CaCertDir: caCertDir,
242-
Preflights: preflights,
234+
Client: cl,
235+
Resolver: &resolve.CatalogResolver{
236+
Client: cl,
237+
InstalledBundleGetter: &controllers.DefaultInstalledBundleGetter{ActionClientGetter: acg},
238+
CatalogPackageProvider: catalogClient,
239+
},
240+
ActionClientGetter: acg,
241+
Unpacker: unpacker,
242+
Storage: localStorage,
243+
Handler: registryv1handler.HandlerFunc(registry.HandleBundleDeployment),
244+
Finalizers: clusterExtensionFinalizers,
245+
CaCertDir: caCertDir,
246+
Preflights: preflights,
243247
}).SetupWithManager(mgr); err != nil {
244248
setupLog.Error(err, "unable to create controller", "controller", "ClusterExtension")
245249
os.Exit(1)

‎go.mod

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ require (
2929
sigs.k8s.io/controller-runtime v0.18.4
3030
)
3131

32+
replace github.com/operator-framework/rukpak => ../rukpak
33+
3234
require (
3335
carvel.dev/kapp v0.62.1-0.20240508153820-7d8a03ed7ccf // indirect
3436
cloud.google.com/go/compute/metadata v0.3.0 // indirect

‎go.sum

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -614,8 +614,6 @@ github.com/operator-framework/operator-lib v0.14.0 h1:er+BgZymZD1im2wytLJiPLZpGA
614614
github.com/operator-framework/operator-lib v0.14.0/go.mod h1:wUu4Xb9xzXnIpglvaZ3yucTMSlqGXHIoUEH9+5gWiu0=
615615
github.com/operator-framework/operator-registry v1.44.0 h1:NW5/xHYR77J2EUYm+6iBER1WNGLNS8gM+G5GBQWqTTs=
616616
github.com/operator-framework/operator-registry v1.44.0/go.mod h1:55I4XJ//Erir98Mm9OlD8UURWE0HQgL/zlvpGF+gkig=
617-
github.com/operator-framework/rukpak v0.24.0 h1:zxLyk931w4isFiTPox4soXxh/eQJr5sjq15gHcb6dlE=
618-
github.com/operator-framework/rukpak v0.24.0/go.mod h1:peTAGzf4gmU+in2RJen84ZQ8lkdB3m6qy+nfNiVv0RY=
619617
github.com/otiai10/copy v1.14.0 h1:dCI/t1iTdYGtkvCuBG2BgR6KZa83PTclw4U5n2wAllU=
620618
github.com/otiai10/copy v1.14.0/go.mod h1:ECfuL02W+/FkTWZWgQqXPWZgW9oeKCSQ5qVfSc4qc4w=
621619
github.com/otiai10/mint v1.5.1 h1:XaPLeE+9vGbuyEHem1JNk3bYc7KKqyI/na0/mLd/Kks=

‎internal/bundleutil/bundle.go

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,6 @@ package bundleutil
33
import (
44
"encoding/json"
55
"fmt"
6-
"sync"
7-
86
bsemver "github.com/blang/semver/v4"
97

108
"github.com/operator-framework/operator-registry/alpha/declcfg"
@@ -13,19 +11,7 @@ import (
1311
ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1"
1412
)
1513

16-
var VersionGetter = &versionGetter{cache: make(map[*declcfg.Bundle]*bsemver.Version)}
17-
18-
type versionGetter struct {
19-
mu sync.Mutex
20-
cache map[*declcfg.Bundle]*bsemver.Version
21-
}
22-
23-
func (v *versionGetter) GetVersion(b *declcfg.Bundle) (*bsemver.Version, error) {
24-
v.mu.Lock()
25-
defer v.mu.Unlock()
26-
if vers, ok := v.cache[b]; ok {
27-
return &(*vers), nil
28-
}
14+
func GetVersion(b declcfg.Bundle) (*bsemver.Version, error) {
2915
for _, p := range b.Properties {
3016
if p.Type == property.TypePackage {
3117
var pkg property.Package
@@ -36,7 +22,6 @@ func (v *versionGetter) GetVersion(b *declcfg.Bundle) (*bsemver.Version, error)
3622
if err != nil {
3723
return nil, err
3824
}
39-
v.cache[b] = &vers
4025
return &(*(&vers)), nil
4126
}
4227
}

‎internal/catalogmetadata/client/client.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@ import (
55
"errors"
66
"fmt"
77
"io/fs"
8-
"testing/fstest"
9-
108
"k8s.io/apimachinery/pkg/api/meta"
119
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1210

@@ -43,8 +41,6 @@ func (c *Client) GetPackage(ctx context.Context, catalog *catalogd.ClusterCatalo
4341
return nil, fmt.Errorf("catalog %q is not unpacked", catalog.Name)
4442
}
4543

46-
var _ fs.SubFS = fstest.MapFS{}
47-
4844
catalogFsys, err := c.fetcher.FetchCatalogContents(ctx, catalog)
4945
if err != nil {
5046
return nil, fmt.Errorf("error fetching catalog contents: %v", err)

‎internal/catalogmetadata/filter/bundle_predicates.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import (
1010

1111
func InMastermindsSemverRange(semverRange *mmsemver.Constraints) Predicate[declcfg.Bundle] {
1212
return func(b declcfg.Bundle) bool {
13-
bVersion, err := bundleutil.VersionGetter.GetVersion(&b)
13+
bVersion, err := bundleutil.GetVersion(b)
1414
if err != nil {
1515
return false
1616
}

‎internal/catalogmetadata/filter/successors_test.go

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
package filter
22

33
import (
4-
"sort"
4+
"github.com/operator-framework/operator-controller/internal/catalogmetadata/sort"
5+
"slices"
56
"testing"
67

78
bsemver "github.com/blang/semver/v4"
@@ -17,7 +18,6 @@ import (
1718

1819
ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1"
1920
"github.com/operator-framework/operator-controller/internal/bundleutil"
20-
catalogsort "github.com/operator-framework/operator-controller/internal/catalogmetadata/sort"
2121
"github.com/operator-framework/operator-controller/pkg/features"
2222
)
2323

@@ -181,9 +181,7 @@ func TestSuccessorsPredicateWithForceSemverUpgradeConstraintsEnabled(t *testing.
181181
result := Filter(allBundles, successors)
182182

183183
// sort before comparison for stable order
184-
sort.Slice(result, func(i, j int) bool {
185-
return catalogsort.ByVersion(&result[i], &result[j]) < 0
186-
})
184+
slices.SortFunc(result, sort.ByVersion)
187185

188186
gocmpopts := []cmp.Option{
189187
cmpopts.IgnoreUnexported(declcfg.Bundle{}),
@@ -340,9 +338,7 @@ func TestSuccessorsPredicateWithForceSemverUpgradeConstraintsDisabled(t *testing
340338
result := Filter(allBundles, successors)
341339

342340
// sort before comparison for stable order
343-
sort.Slice(result, func(i, j int) bool {
344-
return catalogsort.ByVersion(&result[i], &result[j]) < 0
345-
})
341+
slices.SortFunc(result, sort.ByVersion)
346342

347343
gocmpopts := []cmp.Option{
348344
cmpopts.IgnoreUnexported(declcfg.Bundle{}),

‎internal/catalogmetadata/sort/sort.go

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package sort
22

33
import (
4+
"fmt"
45
"k8s.io/apimachinery/pkg/util/sets"
56

67
"github.com/operator-framework/operator-registry/alpha/declcfg"
@@ -10,26 +11,28 @@ import (
1011

1112
// ByVersion is a sort "less" function that orders bundles
1213
// in inverse version order (higher versions on top).
13-
func ByVersion(b1, b2 *declcfg.Bundle) int {
14-
v1, err1 := bundleutil.VersionGetter.GetVersion(b1)
15-
v2, err2 := bundleutil.VersionGetter.GetVersion(b2)
14+
func ByVersion(b1, b2 declcfg.Bundle) int {
15+
v1, err1 := bundleutil.GetVersion(b1)
16+
v2, err2 := bundleutil.GetVersion(b2)
1617
if err1 != nil || err2 != nil {
1718
return compareErrors(err1, err2)
1819
}
1920

21+
fmt.Println(v1, v2, v2.Compare(*v1))
22+
2023
// Check for "greater than" because
2124
// we want higher versions on top
2225
return v2.Compare(*v1)
2326
}
2427

25-
func ByDeprecationFunc(deprecation declcfg.Deprecation) func(a, b *declcfg.Bundle) int {
28+
func ByDeprecationFunc(deprecation declcfg.Deprecation) func(a, b declcfg.Bundle) int {
2629
deprecatedBundles := sets.New[string]()
2730
for _, entry := range deprecation.Entries {
2831
if entry.Reference.Schema == declcfg.SchemaBundle {
2932
deprecatedBundles.Insert(entry.Reference.Name)
3033
}
3134
}
32-
return func(a, b *declcfg.Bundle) int {
35+
return func(a, b declcfg.Bundle) int {
3336
aDeprecated := deprecatedBundles.Has(a.Name)
3437
bDeprecated := deprecatedBundles.Has(b.Name)
3538
if aDeprecated && !bDeprecated {

‎internal/catalogmetadata/sort/sort_test.go

Lines changed: 6 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,14 @@ package sort_test
22

33
import (
44
"encoding/json"
5-
"sort"
5+
"github.com/operator-framework/operator-controller/internal/catalogmetadata/sort"
6+
"slices"
67
"testing"
78

89
"github.com/stretchr/testify/assert"
910

1011
"github.com/operator-framework/operator-registry/alpha/declcfg"
1112
"github.com/operator-framework/operator-registry/alpha/property"
12-
13-
catalogsort "github.com/operator-framework/operator-controller/internal/catalogmetadata/sort"
1413
)
1514

1615
func TestByVersion(t *testing.T) {
@@ -56,27 +55,13 @@ func TestByVersion(t *testing.T) {
5655

5756
t.Run("all bundles valid", func(t *testing.T) {
5857
toSort := []declcfg.Bundle{b3, b2, b1}
59-
sort.Slice(toSort, func(i, j int) bool {
60-
return catalogsort.ByVersion(&toSort[i], &toSort[j]) < 0
61-
})
62-
63-
assert.Len(t, toSort, 3)
64-
assert.Equal(t, b1, toSort[0])
65-
assert.Equal(t, b3, toSort[1])
66-
assert.Equal(t, b2, toSort[2])
58+
slices.SortStableFunc(toSort, sort.ByVersion)
59+
assert.Equal(t, []declcfg.Bundle{b1, b3, b2}, toSort)
6760
})
6861

6962
t.Run("some bundles are missing version", func(t *testing.T) {
7063
toSort := []declcfg.Bundle{b3, b4noVersion, b2, b5empty, b1}
71-
sort.Slice(toSort, func(i, j int) bool {
72-
return catalogsort.ByVersion(&toSort[i], &toSort[j]) < 0
73-
})
74-
75-
assert.Len(t, toSort, 5)
76-
assert.Equal(t, b1, toSort[0])
77-
assert.Equal(t, b3, toSort[1])
78-
assert.Equal(t, b2, toSort[2])
79-
assert.Equal(t, b4noVersion, toSort[3])
80-
assert.Equal(t, b5empty, toSort[4])
64+
slices.SortStableFunc(toSort, sort.ByVersion)
65+
assert.Equal(t, []declcfg.Bundle{b1, b3, b2, b4noVersion, b5empty}, toSort)
8166
})
8267
}

‎internal/controllers/clusterextension_controller.go

Lines changed: 58 additions & 165 deletions
Original file line numberDiff line numberDiff line change
@@ -21,19 +21,11 @@
2121
"context"
2222
"errors"
2323
"fmt"
24-
"github.com/operator-framework/operator-controller/internal/httputil"
2524
"io"
26-
"sigs.k8s.io/controller-runtime/pkg/builder"
27-
"sigs.k8s.io/controller-runtime/pkg/event"
28-
"sigs.k8s.io/controller-runtime/pkg/predicate"
29-
"slices"
30-
"sort"
3125
"strings"
3226
"sync"
3327
"time"
3428

35-
mmsemver "github.com/Masterminds/semver/v3"
36-
bsemver "github.com/blang/semver/v4"
3729
"github.com/go-logr/logr"
3830
"helm.sh/helm/v3/pkg/action"
3931
"helm.sh/helm/v3/pkg/chart"
@@ -50,12 +42,15 @@
5042
"k8s.io/apimachinery/pkg/util/sets"
5143
apimachyaml "k8s.io/apimachinery/pkg/util/yaml"
5244
ctrl "sigs.k8s.io/controller-runtime"
45+
"sigs.k8s.io/controller-runtime/pkg/builder"
5346
"sigs.k8s.io/controller-runtime/pkg/cache"
5447
"sigs.k8s.io/controller-runtime/pkg/client"
5548
crcontroller "sigs.k8s.io/controller-runtime/pkg/controller"
49+
"sigs.k8s.io/controller-runtime/pkg/event"
5650
crfinalizer "sigs.k8s.io/controller-runtime/pkg/finalizer"
5751
crhandler "sigs.k8s.io/controller-runtime/pkg/handler"
5852
"sigs.k8s.io/controller-runtime/pkg/log"
53+
"sigs.k8s.io/controller-runtime/pkg/predicate"
5954
"sigs.k8s.io/controller-runtime/pkg/reconcile"
6055
"sigs.k8s.io/controller-runtime/pkg/source"
6156

@@ -64,50 +59,40 @@
6459
helmclient "github.com/operator-framework/helm-operator-plugins/pkg/client"
6560
"github.com/operator-framework/operator-registry/alpha/declcfg"
6661
"github.com/operator-framework/operator-registry/alpha/property"
6762
rukpakv1alpha2 "github.com/operator-framework/rukpak/api/v1alpha2"
6863
registryv1handler "github.com/operator-framework/rukpak/pkg/handler"
6964
crdupgradesafety "github.com/operator-framework/rukpak/pkg/preflights/crdupgradesafety"
7065
rukpaksource "github.com/operator-framework/rukpak/pkg/source"
7166
"github.com/operator-framework/rukpak/pkg/storage"
7267
"github.com/operator-framework/rukpak/pkg/util"
7368

7469
ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1"
7570
"github.com/operator-framework/operator-controller/internal/bundleutil"
76-
"github.com/operator-framework/operator-controller/internal/catalogmetadata/filter"
77-
catsort "github.com/operator-framework/operator-controller/internal/catalogmetadata/sort"
7871
"github.com/operator-framework/operator-controller/internal/conditionsets"
72+
"github.com/operator-framework/operator-controller/internal/httputil"
7973
"github.com/operator-framework/operator-controller/internal/labels"
74+
"github.com/operator-framework/operator-controller/internal/resolve"
8075
)
8176

8277
const (
8378
maxHelmReleaseHistory = 10
8479
)
8580

86-
// CatalogPackageProvider provides the way to retrieve package metadata from a catalog.
87-
type CatalogPackageProvider interface {
88-
GetPackage(context.Context, *catalogd.ClusterCatalog, string) (*declcfg.DeclarativeConfig, error)
89-
}
90-
9181
// ClusterExtensionReconciler reconciles a ClusterExtension object
9282
type ClusterExtensionReconciler struct {
9383
client.Client
94-
CatalogPackageProvider CatalogPackageProvider
95-
Unpacker rukpaksource.Unpacker
96-
ActionClientGetter helmclient.ActionClientGetter
97-
Storage storage.Storage
98-
Handler registryv1handler.Handler
99-
dynamicWatchMutex sync.RWMutex
100-
dynamicWatchGVKs sets.Set[schema.GroupVersionKind]
101-
controller crcontroller.Controller
102-
cache cache.Cache
103-
InstalledBundleGetter InstalledBundleGetter
104-
Finalizers crfinalizer.Finalizers
105-
CaCertDir string
106-
Preflights []Preflight
107-
}
108-
109-
type InstalledBundleGetter interface {
110-
GetInstalledBundle(ctx context.Context, ext *ocv1alpha1.ClusterExtension) (*ocv1alpha1.BundleMetadata, error)
84+
Resolver resolve.Resolver
85+
Unpacker rukpaksource.Unpacker
86+
ActionClientGetter helmclient.ActionClientGetter
87+
Storage storage.Storage
88+
Handler registryv1handler.Handler
89+
dynamicWatchMutex sync.RWMutex
90+
dynamicWatchGVKs sets.Set[schema.GroupVersionKind]
91+
controller crcontroller.Controller
92+
cache cache.Cache
93+
Finalizers crfinalizer.Finalizers
94+
CaCertDir string
95+
Preflights []Preflight
11196
}
11297

11398
// Preflight is a check that should be run before making any changes to the cluster
@@ -145,30 +130,31 @@
145130
if err := r.Client.Get(ctx, req.NamespacedName, existingExt); err != nil {
146131
return ctrl.Result{}, client.IgnoreNotFound(err)
147132
}
148-
149-
var updateError error
133+
l.V(1).Info("reconciling", "clusterextension", existingExt.Name, "resourceVersion", existingExt.ResourceVersion)
150134

151135
reconciledExt := existingExt.DeepCopy()
152136
res, err := r.reconcile(ctx, reconciledExt)
153-
updateError = errors.Join(updateError, err)
137+
updateError := err
154138

155139
// Do checks before any Update()s, as Update() may modify the resource structure!
156140
updateStatus := !equality.Semantic.DeepEqual(existingExt.Status, reconciledExt.Status)
157141
updateFinalizers := !equality.Semantic.DeepEqual(existingExt.Finalizers, reconciledExt.Finalizers)
158142
unexpectedFieldsChanged := checkForUnexpectedFieldChange(*existingExt, *reconciledExt)
159143

160144
if updateStatus {
161-
err = r.Client.Status().Update(ctx, reconciledExt)
162-
updateError = errors.Join(updateError, err)
145+
if err := r.Client.Status().Update(ctx, reconciledExt); err != nil {
146+
updateError = errors.Join(updateError, fmt.Errorf("error updating status: %v", err))
147+
}
163148
}
164149

165150
if unexpectedFieldsChanged {
166151
panic("spec or metadata changed by reconciler")
167152
}
168153

169154
if updateFinalizers {
170-
err = r.Client.Update(ctx, reconciledExt)
171-
updateError = errors.Join(updateError, err)
155+
if err := r.Client.Update(ctx, reconciledExt); err != nil {
156+
updateError = errors.Join(updateError, fmt.Errorf("error updating finalizers: %v", err))
157+
}
172158
}
173159

174160
return res, updateError
@@ -250,7 +236,7 @@
250236
}
251237

252238
// run resolution
253-
resolvedBundle, resolvedBundleVersion, resolvedDeprecation, err := r.resolve(ctx, *ext)
239+
resolvedBundle, resolvedBundleVersion, resolvedDeprecation, err := r.Resolver.Resolve(ctx, *ext)
254240
if err != nil {
255241
// Note: We don't distinguish between resolution-specific errors and generic errors
256242
ext.Status.ResolvedBundle = nil
@@ -423,6 +409,12 @@
423409
source.Kind(r.cache,
424410
obj,
425411
crhandler.EnqueueRequestForOwner(r.Scheme(), r.RESTMapper(), ext, crhandler.OnlyControllerOwner()),
412+
predicate.Funcs{
413+
CreateFunc: func(ce event.CreateEvent) bool { return false },
414+
UpdateFunc: func(ue event.UpdateEvent) bool { return true },
415+
DeleteFunc: func(de event.DeleteEvent) bool { return true },
416+
GenericFunc: func(ge event.GenericEvent) bool { return true },
417+
},
426418
),
427419
); err != nil {
428420
return err
@@ -442,118 +434,6 @@
442434
return ctrl.Result{}, nil
443435
}
444436

445-
// resolve returns a Bundle from the catalog that needs to get installed on the cluster.
446-
func (r *ClusterExtensionReconciler) resolve(ctx context.Context, ext ocv1alpha1.ClusterExtension) (*declcfg.Bundle, *bsemver.Version, *declcfg.Deprecation, error) {
447-
clusterCatalogs := &catalogd.ClusterCatalogList{}
448-
if err := r.Client.List(ctx, clusterCatalogs); err != nil {
449-
return nil, nil, nil, fmt.Errorf("error listing cluster catalogs: %w", err)
450-
}
451-
452-
installedBundle, err := r.InstalledBundleGetter.GetInstalledBundle(ctx, &ext)
453-
if err != nil {
454-
return nil, nil, nil, err
455-
}
456-
457-
packageName := ext.Spec.PackageName
458-
versionRange := ext.Spec.Version
459-
channelName := ext.Spec.Channel
460-
461-
var (
462-
resolvedBundle *declcfg.Bundle
463-
resolvedDeprecation *declcfg.Deprecation
464-
)
465-
466-
for _, cat := range clusterCatalogs.Items {
467-
packageFBC, err := r.CatalogPackageProvider.GetPackage(ctx, &cat, ext.Spec.PackageName)
468-
if err != nil {
469-
return nil, nil, nil, fmt.Errorf("error catalog metadata: %w", err)
470-
}
471-
472-
var predicates []filter.Predicate[declcfg.Bundle]
473-
if channelName != "" {
474-
channels := slices.DeleteFunc(packageFBC.Channels, func(c declcfg.Channel) bool {
475-
return channelName != "" && c.Name != channelName
476-
})
477-
predicates = append(predicates, filter.InAnyChannel(channels...))
478-
}
479-
480-
if versionRange != "" {
481-
vRange, err := mmsemver.NewConstraint(versionRange)
482-
if err != nil {
483-
return nil, nil, nil, fmt.Errorf("invalid version range %q: %w", versionRange, err)
484-
}
485-
predicates = append(predicates, filter.InMastermindsSemverRange(vRange))
486-
}
487-
488-
if ext.Spec.UpgradeConstraintPolicy != ocv1alpha1.UpgradeConstraintPolicyIgnore && installedBundle != nil {
489-
successorPredicate, err := filter.SuccessorsOf(installedBundle, packageFBC.Channels...)
490-
if err != nil {
491-
return nil, nil, nil, fmt.Errorf("error finding upgrade edges: %w", err)
492-
}
493-
predicates = append(predicates, successorPredicate)
494-
}
495-
496-
// Apply the predicates to get the candidate bundles
497-
packageFBC.Bundles = filter.Filter(packageFBC.Bundles, filter.And(predicates...))
498-
if len(packageFBC.Bundles) == 0 {
499-
continue
500-
}
501-
502-
// If this package has a deprecation, we:
503-
// 1. Want to sort deprecated bundles to the end of the list
504-
// 2. Want to keep track of it so that we can return it if we end
505-
// up resolving a bundle from this package.
506-
byDeprecation := func(a, b *declcfg.Bundle) int { return 0 }
507-
var thisDeprecation *declcfg.Deprecation
508-
if len(packageFBC.Deprecations) > 0 {
509-
thisDeprecation = &packageFBC.Deprecations[0]
510-
byDeprecation = catsort.ByDeprecationFunc(*thisDeprecation)
511-
}
512-
513-
// Sort the bundles by deprecation and then by version
514-
515-
sort.SliceStable(packageFBC.Bundles, func(a, b int) bool {
516-
if lessDep := byDeprecation(&packageFBC.Bundles[a], &packageFBC.Bundles[b]); lessDep != 0 {
517-
return lessDep < 0
518-
}
519-
return catsort.ByVersion(&packageFBC.Bundles[a], &packageFBC.Bundles[b]) < 0
520-
})
521-
522-
thisBundle := &packageFBC.Bundles[0]
523-
if resolvedBundle != nil {
524-
// Cases where we stick with `resolvedBundle`:
525-
// 1. If `thisBundle` is deprecated and `resolvedBundle` is not
526-
// 2. If `thisBundle` and `resolvedBundle` have the same deprecation status AND `resolvedBundle` is a higher version
527-
cmpDeprecation := byDeprecation(resolvedBundle, thisBundle)
528-
cmpVersion := catsort.ByVersion(resolvedBundle, thisBundle)
529-
if cmpDeprecation < 0 || (cmpDeprecation == 0 && cmpVersion < 0) {
530-
continue
531-
}
532-
}
533-
resolvedBundle = thisBundle
534-
resolvedDeprecation = thisDeprecation
535-
}
536-
537-
if resolvedBundle == nil {
538-
switch {
539-
case versionRange != "" && channelName != "":
540-
return nil, nil, nil, fmt.Errorf("no package %q matching version %q in channel %q found", packageName, versionRange, channelName)
541-
case versionRange != "":
542-
return nil, nil, nil, fmt.Errorf("no package %q matching version %q found", packageName, versionRange)
543-
case channelName != "":
544-
return nil, nil, nil, fmt.Errorf("no package %q in channel %q found", packageName, channelName)
545-
default:
546-
return nil, nil, nil, fmt.Errorf("no package %q found", packageName)
547-
}
548-
}
549-
550-
resolvedBundleVersion, err := bundleutil.VersionGetter.GetVersion(resolvedBundle)
551-
if err != nil {
552-
return nil, nil, nil, fmt.Errorf("error getting bundle version: %w", err)
553-
}
554-
return resolvedBundle, resolvedBundleVersion, resolvedDeprecation, nil
555-
}
556-
557437
// SetDeprecationStatus will set the appropriate deprecation statuses for a ClusterExtension
558438
// based on the provided bundle
559439
func SetDeprecationStatus(ext *ocv1alpha1.ClusterExtension, bundleName string, deprecation *declcfg.Deprecation) {
@@ -577,28 +457,20 @@
577457
}
578458
}
579459

460+
// first get ordered deprecation messages that we'll join in the Deprecated condition message
580461
var deprecationMessages []string
581462
for _, conditionType := range []string{
582463
ocv1alpha1.TypePackageDeprecated,
583464
ocv1alpha1.TypeChannelDeprecated,
584465
ocv1alpha1.TypeBundleDeprecated,
585466
} {
586-
entry, ok := deprecations[conditionType]
587-
status, reason, message := metav1.ConditionFalse, "", ""
588-
if ok {
589-
status, reason, message = metav1.ConditionTrue, ocv1alpha1.ReasonDeprecated, entry.Message
590-
deprecationMessages = append(deprecationMessages, message)
467+
if entry, ok := deprecations[conditionType]; ok {
468+
deprecationMessages = append(deprecationMessages, entry.Message)
591469
}
592-
apimeta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{
593-
Type: conditionType,
594-
Reason: reason,
595-
Status: status,
596-
Message: message,
597-
ObservedGeneration: ext.Generation,
598-
})
599470
}
600471

601-
status, reason, message := metav1.ConditionFalse, "", ""
472+
// next, set the Deprecated condition
473+
status, reason, message := metav1.ConditionFalse, ocv1alpha1.ReasonDeprecated, ""
602474
if len(deprecationMessages) > 0 {
603475
status, reason, message = metav1.ConditionTrue, ocv1alpha1.ReasonDeprecated, strings.Join(deprecationMessages, ";")
604476
}
@@ -609,6 +481,27 @@
609481
Message: message,
610482
ObservedGeneration: ext.Generation,
611483
})
484+
485+
// finally, set the individual deprecation conditions for package, channel, and bundle
486+
for _, conditionType := range []string{
487+
ocv1alpha1.TypePackageDeprecated,
488+
ocv1alpha1.TypeChannelDeprecated,
489+
ocv1alpha1.TypeBundleDeprecated,
490+
} {
491+
entry, ok := deprecations[conditionType]
492+
status, reason, message := metav1.ConditionFalse, ocv1alpha1.ReasonDeprecated, ""
493+
if ok {
494+
status, reason, message = metav1.ConditionTrue, ocv1alpha1.ReasonDeprecated, entry.Message
495+
deprecationMessages = append(deprecationMessages, message)
496+
}
497+
apimeta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{
498+
Type: conditionType,
499+
Reason: reason,
500+
Status: status,
501+
Message: message,
502+
ObservedGeneration: ext.Generation,
503+
})
504+
}
612505
}
613506

614507
func (r *ClusterExtensionReconciler) generateBundleDeploymentForUnpack(ctx context.Context, bundlePath string, ce *ocv1alpha1.ClusterExtension) *rukpakv1alpha2.BundleDeployment {

‎internal/controllers/clusterextension_controller_test.go

Lines changed: 19 additions & 785 deletions
Large diffs are not rendered by default.

‎internal/controllers/clusterextension_registryv1_validation_test.go

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@ import (
44
"context"
55
"encoding/json"
66
"fmt"
7+
bsemver "github.com/blang/semver/v4"
8+
"github.com/operator-framework/operator-controller/internal/bundleutil"
9+
"github.com/operator-framework/operator-controller/internal/resolve"
710
"testing"
811

912
"github.com/stretchr/testify/assert"
@@ -22,7 +25,6 @@ import (
2225

2326
ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1"
2427
"github.com/operator-framework/operator-controller/internal/controllers"
25-
"github.com/operator-framework/operator-controller/internal/testutil"
2628
)
2729

2830
func TestClusterExtensionRegistryV1DisallowDependencies(t *testing.T) {
@@ -89,28 +91,25 @@ func TestClusterExtensionRegistryV1DisallowDependencies(t *testing.T) {
8991
defer func() {
9092
require.NoError(t, cl.DeleteAllOf(ctx, &ocv1alpha1.ClusterExtension{}))
9193
}()
92-
fakeCatalogClient := testutil.NewFakeCatalogClient([]declcfg.DeclarativeConfig{
93-
{
94-
Packages: []declcfg.Package{{Name: tt.bundle.Package}},
95-
Channels: []declcfg.Channel{{Package: tt.bundle.Package, Name: "test", Entries: []declcfg.ChannelEntry{{Name: tt.bundle.Package}}}},
96-
Bundles: []declcfg.Bundle{tt.bundle},
97-
},
94+
resolver := resolve.Func(func(ctx context.Context, ext ocv1alpha1.ClusterExtension) (*declcfg.Bundle, *bsemver.Version, *declcfg.Deprecation, error) {
95+
v, err := bundleutil.GetVersion(tt.bundle)
96+
if err != nil {
97+
return nil, nil, nil, err
98+
}
99+
return &tt.bundle, v, nil, nil
98100
})
99101
mockUnpacker := unpacker.(*MockUnpacker)
100102
// Set up the Unpack method to return a result with StatePending
101103
mockUnpacker.On("Unpack", mock.Anything, mock.AnythingOfType("*v1alpha2.BundleDeployment")).Return(&source.Result{
102104
State: source.StatePending,
103105
}, nil)
104-
// Create and configure the mock InstalledBundleGetter
105-
mockInstalledBundleGetter := &MockInstalledBundleGetter{}
106106

107107
reconciler := &controllers.ClusterExtensionReconciler{
108-
Client: cl,
109-
CatalogPackageProvider: &fakeCatalogClient,
110-
ActionClientGetter: helmClientGetter,
111-
Unpacker: unpacker,
112-
InstalledBundleGetter: mockInstalledBundleGetter,
113-
Finalizers: crfinalizer.NewFinalizers(),
108+
Client: cl,
109+
Resolver: resolver,
110+
ActionClientGetter: helmClientGetter,
111+
Unpacker: unpacker,
112+
Finalizers: crfinalizer.NewFinalizers(),
114113
}
115114

116115
installNamespace := fmt.Sprintf("test-ns-%s", rand.String(8))

‎internal/controllers/suite_test.go

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -36,14 +36,12 @@ import (
3636
crfinalizer "sigs.k8s.io/controller-runtime/pkg/finalizer"
3737

3838
helmclient "github.com/operator-framework/helm-operator-plugins/pkg/client"
39-
"github.com/operator-framework/operator-registry/alpha/declcfg"
4039
"github.com/operator-framework/rukpak/api/v1alpha2"
4140
"github.com/operator-framework/rukpak/pkg/source"
4241
"github.com/operator-framework/rukpak/pkg/storage"
4342

4443
ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1"
4544
"github.com/operator-framework/operator-controller/internal/controllers"
46-
"github.com/operator-framework/operator-controller/internal/testutil"
4745
)
4846

4947
// MockUnpacker is a mock of Unpacker interface
@@ -120,19 +118,15 @@ func (m *MockInstalledBundleGetter) GetInstalledBundle(ctx context.Context, ext
120118

121119
func newClientAndReconciler(t *testing.T, bundle *ocv1alpha1.BundleMetadata) (client.Client, *controllers.ClusterExtensionReconciler) {
122120
cl := newClient(t)
123-
fakeCatalogClient := testutil.NewFakeCatalogClient([]declcfg.DeclarativeConfig{testPackage})
124-
125121
mockInstalledBundleGetter := &MockInstalledBundleGetter{}
126122
mockInstalledBundleGetter.SetBundle(bundle)
127123

128124
reconciler := &controllers.ClusterExtensionReconciler{
129-
Client: cl,
130-
CatalogPackageProvider: &fakeCatalogClient,
131-
ActionClientGetter: helmClientGetter,
132-
Unpacker: unpacker,
133-
Storage: store,
134-
InstalledBundleGetter: mockInstalledBundleGetter,
135-
Finalizers: crfinalizer.NewFinalizers(),
125+
Client: cl,
126+
ActionClientGetter: helmClientGetter,
127+
Unpacker: unpacker,
128+
Storage: store,
129+
Finalizers: crfinalizer.NewFinalizers(),
136130
}
137131
return cl, reconciler
138132
}

‎internal/resolve/catalog.go

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
package resolve
2+
3+
import (
4+
"context"
5+
"fmt"
6+
"slices"
7+
8+
mmsemver "github.com/Masterminds/semver/v3"
9+
bsemver "github.com/blang/semver/v4"
10+
"sigs.k8s.io/controller-runtime/pkg/client"
11+
12+
catalogd "github.com/operator-framework/catalogd/api/core/v1alpha1"
13+
"github.com/operator-framework/operator-registry/alpha/declcfg"
14+
15+
ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1"
16+
"github.com/operator-framework/operator-controller/internal/bundleutil"
17+
"github.com/operator-framework/operator-controller/internal/catalogmetadata/filter"
18+
catsort "github.com/operator-framework/operator-controller/internal/catalogmetadata/sort"
19+
)
20+
21+
type CatalogResolver struct {
22+
Client client.Client
23+
InstalledBundleGetter InstalledBundleGetter
24+
CatalogPackageProvider CatalogPackageProvider
25+
}
26+
27+
// CatalogPackageProvider provides the way to retrieve package metadata from a catalog.
28+
type CatalogPackageProvider interface {
29+
GetPackage(context.Context, *catalogd.ClusterCatalog, string) (*declcfg.DeclarativeConfig, error)
30+
}
31+
32+
type InstalledBundleGetter interface {
33+
GetInstalledBundle(ctx context.Context, ext *ocv1alpha1.ClusterExtension) (*ocv1alpha1.BundleMetadata, error)
34+
}
35+
36+
// Resolve returns a Bundle from a catalog that needs to get installed on the cluster.
37+
func (r *CatalogResolver) Resolve(ctx context.Context, ext ocv1alpha1.ClusterExtension) (*declcfg.Bundle, *bsemver.Version, *declcfg.Deprecation, error) {
38+
clusterCatalogs := &catalogd.ClusterCatalogList{}
39+
if err := r.Client.List(ctx, clusterCatalogs); err != nil {
40+
return nil, nil, nil, fmt.Errorf("error listing cluster catalogs: %w", err)
41+
}
42+
43+
installedBundle, err := r.InstalledBundleGetter.GetInstalledBundle(ctx, &ext)
44+
if err != nil {
45+
return nil, nil, nil, err
46+
}
47+
48+
packageName := ext.Spec.PackageName
49+
versionRange := ext.Spec.Version
50+
channelName := ext.Spec.Channel
51+
52+
var (
53+
resolvedBundle *declcfg.Bundle
54+
resolvedDeprecation *declcfg.Deprecation
55+
)
56+
57+
for _, cat := range clusterCatalogs.Items {
58+
packageFBC, err := r.CatalogPackageProvider.GetPackage(ctx, &cat, ext.Spec.PackageName)
59+
if err != nil {
60+
return nil, nil, nil, fmt.Errorf("error catalog metadata: %w", err)
61+
}
62+
63+
var predicates []filter.Predicate[declcfg.Bundle]
64+
if channelName != "" {
65+
channels := slices.DeleteFunc(packageFBC.Channels, func(c declcfg.Channel) bool {
66+
return channelName != "" && c.Name != channelName
67+
})
68+
predicates = append(predicates, filter.InAnyChannel(channels...))
69+
}
70+
71+
if versionRange != "" {
72+
vRange, err := mmsemver.NewConstraint(versionRange)
73+
if err != nil {
74+
return nil, nil, nil, fmt.Errorf("invalid version range %q: %w", versionRange, err)
75+
}
76+
predicates = append(predicates, filter.InMastermindsSemverRange(vRange))
77+
}
78+
79+
if ext.Spec.UpgradeConstraintPolicy != ocv1alpha1.UpgradeConstraintPolicyIgnore && installedBundle != nil {
80+
successorPredicate, err := filter.SuccessorsOf(installedBundle, packageFBC.Channels...)
81+
if err != nil {
82+
return nil, nil, nil, fmt.Errorf("error finding upgrade edges: %w", err)
83+
}
84+
predicates = append(predicates, successorPredicate)
85+
}
86+
87+
// Apply the predicates to get the candidate bundles
88+
packageFBC.Bundles = filter.Filter(packageFBC.Bundles, filter.And(predicates...))
89+
if len(packageFBC.Bundles) == 0 {
90+
continue
91+
}
92+
93+
// If this package has a deprecation, we:
94+
// 1. Want to sort deprecated bundles to the end of the list
95+
// 2. Want to keep track of it so that we can return it if we end
96+
// up resolving a bundle from this package.
97+
byDeprecation := func(a, b declcfg.Bundle) int { return 0 }
98+
var thisDeprecation *declcfg.Deprecation
99+
if len(packageFBC.Deprecations) > 0 {
100+
thisDeprecation = &packageFBC.Deprecations[0]
101+
byDeprecation = catsort.ByDeprecationFunc(*thisDeprecation)
102+
}
103+
104+
// Sort the bundles by deprecation and then by version
105+
slices.SortStableFunc(packageFBC.Bundles, func(a, b declcfg.Bundle) int {
106+
if lessDep := byDeprecation(a, b); lessDep != 0 {
107+
return lessDep
108+
}
109+
return catsort.ByVersion(a, b)
110+
})
111+
112+
thisBundle := packageFBC.Bundles[0]
113+
if resolvedBundle != nil {
114+
// Cases where we stick with `resolvedBundle`:
115+
// 1. If `thisBundle` is deprecated and `resolvedBundle` is not
116+
// 2. If `thisBundle` and `resolvedBundle` have the same deprecation status AND `resolvedBundle` is a higher version
117+
cmpDeprecation := byDeprecation(*resolvedBundle, thisBundle)
118+
cmpVersion := catsort.ByVersion(*resolvedBundle, thisBundle)
119+
if cmpDeprecation < 0 || (cmpDeprecation == 0 && cmpVersion < 0) {
120+
continue
121+
}
122+
}
123+
resolvedBundle = &thisBundle
124+
resolvedDeprecation = thisDeprecation
125+
}
126+
127+
if resolvedBundle == nil {
128+
errPrefix := ""
129+
if installedBundle != nil {
130+
errPrefix = fmt.Sprintf("error upgrading from currently installed version %q: ", installedBundle.Version)
131+
}
132+
switch {
133+
case versionRange != "" && channelName != "":
134+
return nil, nil, nil, fmt.Errorf("%sno package %q matching version %q in channel %q found", errPrefix, packageName, versionRange, channelName)
135+
case versionRange != "":
136+
return nil, nil, nil, fmt.Errorf("%sno package %q matching version %q found", errPrefix, packageName, versionRange)
137+
case channelName != "":
138+
return nil, nil, nil, fmt.Errorf("%sno package %q in channel %q found", errPrefix, packageName, channelName)
139+
default:
140+
return nil, nil, nil, fmt.Errorf("%sno package %q found", errPrefix, packageName)
141+
}
142+
}
143+
144+
resolvedBundleVersion, err := bundleutil.GetVersion(*resolvedBundle)
145+
if err != nil {
146+
return nil, nil, nil, fmt.Errorf("error getting bundle version: %w", err)
147+
}
148+
return resolvedBundle, resolvedBundleVersion, resolvedDeprecation, nil
149+
}

‎internal/resolve/resolver.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
package resolve
2+
3+
import (
4+
"context"
5+
bsemver "github.com/blang/semver/v4"
6+
ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1"
7+
"github.com/operator-framework/operator-registry/alpha/declcfg"
8+
)
9+
10+
type Resolver interface {
11+
Resolve(ctx context.Context, ext ocv1alpha1.ClusterExtension) (*declcfg.Bundle, *bsemver.Version, *declcfg.Deprecation, error)
12+
}
13+
14+
type Func func(ctx context.Context, ext ocv1alpha1.ClusterExtension) (*declcfg.Bundle, *bsemver.Version, *declcfg.Deprecation, error)
15+
16+
func (f Func) Resolve(ctx context.Context, ext ocv1alpha1.ClusterExtension) (*declcfg.Bundle, *bsemver.Version, *declcfg.Deprecation, error) {
17+
return f(ctx, ext)
18+
}

‎test/e2e/cluster_extension_install_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ func TestClusterExtensionInstallRegistry(t *testing.T) {
118118
}
119119
assert.Equal(ct, metav1.ConditionTrue, cond.Status)
120120
assert.Equal(ct, ocv1alpha1.ReasonSuccess, cond.Reason)
121-
assert.Contains(ct, cond.Message, "Instantiated bundle")
121+
assert.Contains(ct, cond.Message, "Installed bundle")
122122
assert.NotEmpty(ct, clusterExtension.Status.InstalledBundle)
123123
}, pollDuration, pollInterval)
124124
}

0 commit comments

Comments
 (0)
Please sign in to comment.