NO-ISSUE: Synchronize From Upstream Repositories

cmd/manager/main.go

@@ -314,8 +314,9 @@ func main() {
 	}
 
 	if err = (&controllers.ClusterCatalogReconciler{
-		Client: cl,
-		Cache:  catalogClientBackend,
+		Client:                cl,
+		CatalogCache:          catalogClientBackend,
+		CatalogCachePopulator: catalogClient,
 	}).SetupWithManager(mgr); err != nil {
 		setupLog.Error(err, "unable to create controller", "controller", "ClusterCatalog")
 		os.Exit(1)

commitchecker.yaml

@@ -1,4 +1,4 @@
-expectedMergeBase: cd0b096468d5d3ca6268dbef22d9b1eea789f771
+expectedMergeBase: cfd4bec28827a94e717f824a111ddcac3a144709
 upstreamBranch: main
 upstreamOrg: operator-framework
 upstreamRepo: operator-controller

go.mod

@@ -13,8 +13,8 @@ require (
 	github.com/go-logr/logr v1.4.2
 	github.com/google/go-cmp v0.6.0
 	github.com/google/go-containerregistry v0.20.2
-	github.com/onsi/ginkgo/v2 v2.20.2
-	github.com/onsi/gomega v1.34.2
+	github.com/onsi/ginkgo/v2 v2.21.0
+	github.com/onsi/gomega v1.35.1
 	github.com/opencontainers/go-digest v1.0.0
 	github.com/operator-framework/api v0.27.0
 	github.com/operator-framework/catalogd v0.35.0
@@ -116,7 +116,7 @@ require (
 	github.com/google/cel-go v0.20.1 // indirect
 	github.com/google/gnostic-models v0.6.8 // indirect
 	github.com/google/gofuzz v1.2.0 // indirect
-	github.com/google/pprof v0.0.0-20240827171923-fa2c70bbbfe5 // indirect
+	github.com/google/pprof v0.0.0-20241029153458-d1b30febd7db // indirect
 	github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 // indirect
 	github.com/google/uuid v1.6.0 // indirect
 	github.com/gorilla/mux v1.8.1 // indirect
@@ -230,7 +230,7 @@ require (
 	golang.org/x/term v0.25.0 // indirect
 	golang.org/x/text v0.19.0 // indirect
 	golang.org/x/time v0.5.0 // indirect
-	golang.org/x/tools v0.25.0 // indirect
+	golang.org/x/tools v0.26.0 // indirect
 	gomodules.xyz/jsonpatch/v2 v2.4.0 // indirect
 	google.golang.org/genproto v0.0.0-20240311173647-c811ad7063a7 // indirect
 	google.golang.org/genproto/googleapis/api v0.0.0-20240814211410-ddb44dafa142 // indirect

go.sum

@@ -310,8 +310,8 @@ github.com/google/gofuzz v1.2.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/
 github.com/google/martian v2.1.0+incompatible/go.mod h1:9I4somxYTbIHy5NJKHRl3wXiIaQGbYVAs8BPL6v8lEs=
 github.com/google/pprof v0.0.0-20181206194817-3ea8567a2e57/go.mod h1:zfwlbNMJ+OItoe0UupaVj+oy1omPYYDuagoSzA8v9mc=
 github.com/google/pprof v0.0.0-20190515194954-54271f7e092f/go.mod h1:zfwlbNMJ+OItoe0UupaVj+oy1omPYYDuagoSzA8v9mc=
-github.com/google/pprof v0.0.0-20240827171923-fa2c70bbbfe5 h1:5iH8iuqE5apketRbSFBy+X1V0o+l+8NF1avt4HWl7cA=
-github.com/google/pprof v0.0.0-20240827171923-fa2c70bbbfe5/go.mod h1:vavhavw2zAxS5dIdcRluK6cSGGPlZynqzFM8NdvU144=
+github.com/google/pprof v0.0.0-20241029153458-d1b30febd7db h1:097atOisP2aRj7vFgYQBbFN4U4JNXUNYpxael3UzMyo=
+github.com/google/pprof v0.0.0-20241029153458-d1b30febd7db/go.mod h1:vavhavw2zAxS5dIdcRluK6cSGGPlZynqzFM8NdvU144=
 github.com/google/renameio v0.1.0/go.mod h1:KWCgfxg9yswjAJkECMjeO8J8rahYeXnNhOm40UhjYkI=
 github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 h1:El6M4kTTCOh6aBiKaUGG7oYTSPP8MxqL4YI3kZKwcP4=
 github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510/go.mod h1:pupxD2MaaD3pAXIBCelhxNneeOaAeabZDe5s4K6zSpQ=
@@ -518,13 +518,13 @@ github.com/onsi/ginkgo v1.12.1/go.mod h1:zj2OWP4+oCPe1qIXoGWkgMRwljMUYCdkwsT2108
 github.com/onsi/ginkgo v1.13.0/go.mod h1:+REjRxOmWfHCjfv9TTWB1jD1Frx4XydAD3zm1lskyM0=
 github.com/onsi/ginkgo v1.16.4 h1:29JGrr5oVBm5ulCWet69zQkzWipVXIol6ygQUe/EzNc=
 github.com/onsi/ginkgo v1.16.4/go.mod h1:dX+/inL/fNMqNlz0e9LfyB9TswhZpCVdJM/Z6Vvnwo0=
-github.com/onsi/ginkgo/v2 v2.20.2 h1:7NVCeyIWROIAheY21RLS+3j2bb52W0W82tkberYytp4=
-github.com/onsi/ginkgo/v2 v2.20.2/go.mod h1:K9gyxPIlb+aIvnZ8bd9Ak+YP18w3APlR+5coaZoE2ag=
+github.com/onsi/ginkgo/v2 v2.21.0 h1:7rg/4f3rB88pb5obDgNZrNHrQ4e6WpjonchcpuBRnZM=
+github.com/onsi/ginkgo/v2 v2.21.0/go.mod h1:7Du3c42kxCUegi0IImZ1wUQzMBVecgIHjR1C+NkhLQo=
 github.com/onsi/gomega v1.4.1/go.mod h1:C1qb7wdrVGGVU+Z6iS04AVkA3Q65CEZX59MT0QO5uiA=
 github.com/onsi/gomega v1.7.1/go.mod h1:XdKZgCCFLUoM/7CFJVPcG8C1xQ1AJ0vpAezJrB7JYyY=
 github.com/onsi/gomega v1.10.1/go.mod h1:iN09h71vgCQne3DLsj+A5owkum+a2tYe+TOCB1ybHNo=
-github.com/onsi/gomega v1.34.2 h1:pNCwDkzrsv7MS9kpaQvVb1aVLahQXyJ/Tv5oAZMI3i8=
-github.com/onsi/gomega v1.34.2/go.mod h1:v1xfxRgk0KIsG+QOdm7p8UosrOzPYRo60fd3B/1Dukc=
+github.com/onsi/gomega v1.35.1 h1:Cwbd75ZBPxFSuZ6T+rN/WCb/gOc6YgFBXLlZLhC7Ds4=
+github.com/onsi/gomega v1.35.1/go.mod h1:PvZbdDc8J6XJEpDK4HCuRBm8a6Fzp9/DmhC9C7yFlog=
 github.com/opencontainers/go-digest v1.0.0 h1:apOUWs51W5PlhuyGyz9FCeeBIOUDA/6nW8Oi/yOhh5U=
 github.com/opencontainers/go-digest v1.0.0/go.mod h1:0JzlMkj0TRzQZfJkVvzbP0HBR3IKzErnv2BNG4W4MAM=
 github.com/opencontainers/image-spec v1.1.0 h1:8SG7/vwALn54lVB/0yZ/MMwhFrPYtpEHQb2IpWsCzug=
@@ -892,8 +892,8 @@ golang.org/x/tools v0.0.0-20191112195655-aa38f8e97acc/go.mod h1:b+2E5dAYhXwXZwtn
 golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
 golang.org/x/tools v0.0.0-20200619180055-7c47624df98f/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE=
 golang.org/x/tools v0.0.0-20210106214847-113979e3529a/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA=
-golang.org/x/tools v0.25.0 h1:oFU9pkj/iJgs+0DT+VMHrx+oBKs/LJMV+Uvg78sl+fE=
-golang.org/x/tools v0.25.0/go.mod h1:/vtpO8WL1N9cQC3FN5zPqb//fRXskFHbLKk4OW1Q7rg=
+golang.org/x/tools v0.26.0 h1:v/60pFQmzmT9ExmjDv2gGIfi3OqfKoEP6I5+umXlbnQ=
+golang.org/x/tools v0.26.0/go.mod h1:TPVVj70c7JJ3WCazhD8OdXcZg/og+b9+tH/KxylGwH0=
 golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=

internal/catalogmetadata/client/client.go

@@ -65,17 +65,10 @@ func (c *Client) GetPackage(ctx context.Context, catalog *catalogd.ClusterCatalo
 
 	catalogFsys, err := c.cache.Get(catalog.Name, catalog.Status.ResolvedSource.Image.Ref)
 	if err != nil {
-		return nil, fmt.Errorf("error retrieving catalog cache: %v", err)
+		return nil, fmt.Errorf("error retrieving cache for catalog %q: %v", catalog.Name, err)
 	}
 	if catalogFsys == nil {
-		// TODO: https://github.com/operator-framework/operator-controller/pull/1284
-		// For now we are still populating cache (if absent) on-demand,
-		// but we might end up just returning a "cache not found" error here
-		// once we implement cache population in the controller.
-		catalogFsys, err = c.PopulateCache(ctx, catalog)
-		if err != nil {
-			return nil, fmt.Errorf("error fetching catalog contents: %v", err)
-		}
+		return nil, fmt.Errorf("cache for catalog %q not found", catalog.Name)
 	}
 
 	pkgFsys, err := fs.Sub(catalogFsys, pkgName)

internal/catalogmetadata/client/client_test.go

@@ -62,7 +62,7 @@ func TestClientGetPackage(t *testing.T) {
 			catalog: defaultCatalog,
 			cache:   &fakeCache{getErr: errors.New("fetch error")},
 			assert: func(t *testing.T, dc *declcfg.DeclarativeConfig, err error) {
-				assert.ErrorContains(t, err, `error retrieving catalog cache`)
+				assert.ErrorContains(t, err, `error retrieving cache for catalog "catalog-1"`)
 			},
 		},
 		{
@@ -114,18 +114,7 @@ func TestClientGetPackage(t *testing.T) {
 				return testFS, nil
 			}},
 			assert: func(t *testing.T, fbc *declcfg.DeclarativeConfig, err error) {
-				require.NoError(t, err)
-				assert.Equal(t, &declcfg.DeclarativeConfig{Packages: []declcfg.Package{{Schema: declcfg.SchemaPackage, Name: "pkg-present"}}}, fbc)
-			},
-		},
-		{
-			name:    "cache unpopulated and fails to populate",
-			catalog: defaultCatalog,
-			pkgName: "pkg-present",
-			cache:   &fakeCache{putErr: errors.New("fake cache put error")},
-			assert: func(t *testing.T, fbc *declcfg.DeclarativeConfig, err error) {
-				assert.Nil(t, fbc)
-				assert.ErrorContains(t, err, "error fetching catalog contents")
+				assert.ErrorContains(t, err, `cache for catalog "catalog-1" not found`)
 			},
 		},
 	} {
@@ -278,7 +267,6 @@ type fakeCache struct {
 	getErr error
 
 	putFunc func(source string, errToCache error) (fs.FS, error)
-	putErr  error
 }
 
 func (c *fakeCache) Get(catalogName, resolvedRef string) (fs.FS, error) {
@@ -293,9 +281,6 @@ func (c *fakeCache) Put(catalogName, resolvedRef string, source io.Reader, errTo
 		}
 		return c.putFunc(buf.String(), errToCache)
 	}
-	if c.putErr != nil {
-		return nil, c.putErr
-	}
 
 	return nil, errors.New("unexpected error")
 }

internal/controllers/clustercatalog_controller.go

@@ -18,25 +18,30 @@ package controllers
 
 import (
 	"context"
+	"fmt"
+	"io/fs"
 
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	ctrl "sigs.k8s.io/controller-runtime"
-	"sigs.k8s.io/controller-runtime/pkg/builder"
 	"sigs.k8s.io/controller-runtime/pkg/client"
-	"sigs.k8s.io/controller-runtime/pkg/event"
-	"sigs.k8s.io/controller-runtime/pkg/predicate"
 
 	catalogd "github.com/operator-framework/catalogd/api/core/v1alpha1"
 )
 
-type CatalogCacheRemover interface {
+type CatalogCache interface {
+	Get(catalogName, resolvedRef string) (fs.FS, error)
 	Remove(catalogName string) error
 }
 
+type CatalogCachePopulator interface {
+	PopulateCache(ctx context.Context, catalog *catalogd.ClusterCatalog) (fs.FS, error)
+}
+
 // ClusterCatalogReconciler reconciles a ClusterCatalog object
 type ClusterCatalogReconciler struct {
 	client.Client
-	Cache CatalogCacheRemover
+	CatalogCache          CatalogCache
+	CatalogCachePopulator CatalogCachePopulator
 }
 
 //+kubebuilder:rbac:groups=olm.operatorframework.io,resources=clustercatalogs,verbs=get;list;watch
@@ -45,31 +50,44 @@ func (r *ClusterCatalogReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 	existingCatalog := &catalogd.ClusterCatalog{}
 	err := r.Client.Get(ctx, req.NamespacedName, existingCatalog)
 	if apierrors.IsNotFound(err) {
-		return ctrl.Result{}, r.Cache.Remove(req.Name)
+		if err := r.CatalogCache.Remove(req.Name); err != nil {
+			return ctrl.Result{}, fmt.Errorf("error removing cache for catalog %q: %v", req.Name, err)
+		}
+		return ctrl.Result{}, nil
 	}
 	if err != nil {
 		return ctrl.Result{}, err
 	}
+
+	if existingCatalog.Status.ResolvedSource == nil ||
+		existingCatalog.Status.ResolvedSource.Image == nil ||
+		existingCatalog.Status.ResolvedSource.Image.Ref == "" {
+		// Reference is not known yet - skip cache population with no error.
+		// Once the reference is resolved another reconcile cycle
+		// will be triggered and we will progress further.
+		return ctrl.Result{}, nil
+	}
+
+	catalogFsys, err := r.CatalogCache.Get(existingCatalog.Name, existingCatalog.Status.ResolvedSource.Image.Ref)
+	if err != nil {
+		return ctrl.Result{}, fmt.Errorf("error retrieving cache for catalog %q: %v", existingCatalog.Name, err)
+	}
+	if catalogFsys != nil {
+		// Cache already exists so we do not need to populate it
+		return ctrl.Result{}, nil
+	}
+
+	if _, err = r.CatalogCachePopulator.PopulateCache(ctx, existingCatalog); err != nil {
+		return ctrl.Result{}, fmt.Errorf("error populating cache for catalog %q: %v", existingCatalog.Name, err)
+	}
+
 	return ctrl.Result{}, nil
 }
 
 // SetupWithManager sets up the controller with the Manager.
 func (r *ClusterCatalogReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	_, err := ctrl.NewControllerManagedBy(mgr).
-		For(&catalogd.ClusterCatalog{}, builder.WithPredicates(predicate.Funcs{
-			CreateFunc: func(e event.CreateEvent) bool {
-				return false
-			},
-			UpdateFunc: func(e event.UpdateEvent) bool {
-				return false
-			},
-			DeleteFunc: func(e event.DeleteEvent) bool {
-				return true
-			},
-			GenericFunc: func(e event.GenericEvent) bool {
-				return false
-			},
-		})).
+		For(&catalogd.ClusterCatalog{}).
 		Build(r)
 
 	return err