Mark GetBundleForChannel as deprecated and trim its response.

The RPC api.Registry/GetBundleForChannel is consumed only by
package-server and by declarative index conversion. Neither reads any
field other than CsvName and CsvJson. The package-server generates one
call to this RPC per channel per package per catalog and is
responsible for needlessly large CPU utilization and resident set
sizes on registry pods. Removing the additional per-request processing
and database queries that populate unused fields makes the registry
server better able to absorb the bursty load from package-server.

Upstream-repository: operator-registry
Upstream-commit: 86907e18e912ee4c4cce76a6f6d6c1f518d423e3

Author: benluddy

staging/operator-registry/pkg/api/registry.proto

@@ -8,7 +8,9 @@ service Registry {
 	rpc ListPackages(ListPackageRequest) returns (stream PackageName) {}
 	rpc GetPackage(GetPackageRequest) returns (Package) {}
 	rpc GetBundle(GetBundleRequest) returns (Bundle) {}
-	rpc GetBundleForChannel(GetBundleInChannelRequest) returns (Bundle) {}
+	rpc GetBundleForChannel(GetBundleInChannelRequest) returns (Bundle) {
+		option deprecated = true;
+	}
 	rpc GetChannelEntriesThatReplace(GetAllReplacementsRequest) returns (stream ChannelEntry) {}
 	rpc GetBundleThatReplaces(GetReplacementRequest) returns (Bundle) {}
 	rpc GetChannelEntriesThatProvide(GetAllProvidersRequest) returns (stream ChannelEntry) {}

staging/operator-registry/pkg/registry/interface.go

@@ -37,7 +37,10 @@ type GRPCQuery interface {
 	// Get a bundle by its package name, channel name and csv name from the index
 	GetBundle(ctx context.Context, pkgName, channelName, csvName string) (*api.Bundle, error)
 
-	// Get the bundle in the specified package at the head of the specified channel
+	// Get the bundle in the specified package at the head of the
+	// specified channel. DEPRECATED. Returned bundles may have
+	// only the "name" and "csvJson" fields populated in order to
+	// support legacy usage.
 	GetBundleForChannel(ctx context.Context, pkgName string, channelName string) (*api.Bundle, error)
 
 	// Get all channel entries that say they replace this one

staging/operator-registry/pkg/registry/populator_test.go

@@ -171,7 +171,11 @@ func TestQuerierForImage(t *testing.T) {
 			{Group: "testapi.coreos.com", Version: "v1", Kind: "testapi"},
 		},
 	}
-	EqualBundles(t, *expectedBundle, *etcdBundleByChannel)
+	bareGetBundleForChannelResult := &api.Bundle{
+		CsvName: expectedBundle.CsvName,
+		CsvJson: expectedBundle.CsvJson + "\n",
+	}
+	EqualBundles(t, *bareGetBundleForChannelResult, *etcdBundleByChannel)
 
 	etcdBundle, err := store.GetBundle(context.TODO(), "etcd", "alpha", "etcdoperator.v0.9.2")
 	require.NoError(t, err)

staging/operator-registry/pkg/server/server_test.go

@@ -216,7 +216,13 @@ func testGetBundle(addr string, expected *api.Bundle) func(*testing.T) {
 }
 
 func TestGetBundleForChannel(t *testing.T) {
-	t.Run("Sqlite", testGetBundleForChannel(dbAddress, etcdoperator_v0_9_2("alpha", false, false)))
+	{
+		b := etcdoperator_v0_9_2("alpha", false, false)
+		t.Run("Sqlite", testGetBundleForChannel(dbAddress, &api.Bundle{
+			CsvName: b.CsvName,
+			CsvJson: b.CsvJson + "\n",
+		}))
+	}
 	t.Run("DeclarativeConfig", testGetBundleForChannel(cfgAddress, etcdoperator_v0_9_2("alpha", false, true)))
 }
 

staging/operator-registry/pkg/sqlite/configmap_test.go

@@ -175,7 +175,11 @@ func TestQuerierForConfigmap(t *testing.T) {
 			"{\"apiVersion\":\"apiextensions.k8s.io/v1beta1\",\"kind\":\"CustomResourceDefinition\",\"metadata\":{\"creationTimestamp\":null,\"name\":\"etcdbackups.etcd.database.coreos.com\"},\"spec\":{\"group\":\"etcd.database.coreos.com\",\"names\":{\"kind\":\"EtcdBackup\",\"listKind\":\"EtcdBackupList\",\"plural\":\"etcdbackups\",\"singular\":\"etcdbackup\"},\"scope\":\"Namespaced\",\"version\":\"v1beta2\",\"versions\":[{\"name\":\"v1beta2\",\"served\":true,\"storage\":true}]},\"status\":{\"acceptedNames\":{\"kind\":\"\",\"plural\":\"\"},\"conditions\":null,\"storedVersions\":null}}",
 			"{\"apiVersion\":\"apiextensions.k8s.io/v1beta1\",\"kind\":\"CustomResourceDefinition\",\"metadata\":{\"creationTimestamp\":null,\"name\":\"etcdrestores.etcd.database.coreos.com\"},\"spec\":{\"group\":\"etcd.database.coreos.com\",\"names\":{\"kind\":\"EtcdRestore\",\"listKind\":\"EtcdRestoreList\",\"plural\":\"etcdrestores\",\"singular\":\"etcdrestore\"},\"scope\":\"Namespaced\",\"version\":\"v1beta2\",\"versions\":[{\"name\":\"v1beta2\",\"served\":true,\"storage\":true}]},\"status\":{\"acceptedNames\":{\"kind\":\"\",\"plural\":\"\"},\"conditions\":null,\"storedVersions\":null}}"},
 	}
-	EqualBundles(t, *expectedBundle, *etcdBundleByChannel)
+	bareGetBundleForChannelResult := &api.Bundle{
+		CsvName: expectedBundle.CsvName,
+		CsvJson: expectedBundle.CsvJson + "\n",
+	}
+	EqualBundles(t, *bareGetBundleForChannelResult, *etcdBundleByChannel)
 
 	etcdBundle, err := store.GetBundle(context.TODO(), "etcd", "alpha", "etcdoperator.v0.9.2")
 	require.NoError(t, err)

staging/operator-registry/pkg/sqlite/directory_test.go

@@ -180,7 +180,11 @@ func TestQuerierForDirectory(t *testing.T) {
 			{Group: "etcd.database.coreos.com", Version: "v1beta2", Kind: "EtcdCluster", Plural: "etcdclusters"},
 		},
 	}
-	EqualBundles(t, *expectedBundle, *etcdBundleByChannel)
+	bareGetBundleForChannelResult := &api.Bundle{
+		CsvName: expectedBundle.CsvName,
+		CsvJson: expectedBundle.CsvJson + "\n",
+	}
+	EqualBundles(t, *bareGetBundleForChannelResult, *etcdBundleByChannel)
 
 	etcdBundle, err := store.GetBundle(context.TODO(), "etcd", "alpha", "etcdoperator.v0.9.2")
 	require.NoError(t, err)

staging/operator-registry/pkg/sqlite/query.go

@@ -287,64 +287,32 @@ func (s *SQLQuerier) GetBundle(ctx context.Context, pkgName, channelName, csvNam
 	return out, nil
 }
 
-func (s *SQLQuerier) GetBundleForChannel(ctx context.Context, pkgName string, channelName string) (*api.Bundle, error) {
-	query := `SELECT DISTINCT channel_entry.entry_id, operatorbundle.name, operatorbundle.bundle, operatorbundle.bundlepath, operatorbundle.version, operatorbundle.skiprange FROM channel
-              INNER JOIN operatorbundle ON channel.head_operatorbundle_name=operatorbundle.name
-              INNER JOIN channel_entry ON (channel_entry.channel_name = channel.name and channel_entry.package_name=channel.package_name and channel_entry.operatorbundle_name=operatorbundle.name)
-              WHERE channel.package_name=? AND channel.name=? LIMIT 1`
-	rows, err := s.db.QueryContext(ctx, query, pkgName, channelName)
+func (s *SQLQuerier) GetBundleForChannel(ctx context.Context, pkg string, channel string) (*api.Bundle, error) {
+	query := `
+SELECT operatorbundle.name, operatorbundle.csv FROM operatorbundle INNER JOIN channel
+ON channel.head_operatorbundle_name = operatorbundle.name
+WHERE channel.name = :channel AND channel.package_name = :package`
+	rows, err := s.db.QueryContext(ctx, query, sql.Named("channel", channel), sql.Named("package", pkg))
 	if err != nil {
 		return nil, err
 	}
 	defer rows.Close()
 
 	if !rows.Next() {
-		return nil, fmt.Errorf("no entry found for %s %s", pkgName, channelName)
-	}
-	var entryId sql.NullInt64
-	var name sql.NullString
-	var bundle sql.NullString
-	var bundlePath sql.NullString
-	var version sql.NullString
-	var skipRange sql.NullString
-	if err := rows.Scan(&entryId, &name, &bundle, &bundlePath, &version, &skipRange); err != nil {
-		return nil, err
-	}
-
-	out := &api.Bundle{}
-	if bundle.Valid && bundle.String != "" {
-		out, err = registry.BundleStringToAPIBundle(bundle.String)
-		if err != nil {
-			return nil, err
-		}
-	}
-	out.CsvName = name.String
-	out.PackageName = pkgName
-	out.ChannelName = channelName
-	out.BundlePath = bundlePath.String
-	out.Version = version.String
-	out.SkipRange = skipRange.String
-
-	provided, required, err := s.GetApisForEntry(ctx, entryId.Int64)
-	if err != nil {
-		return nil, err
-	}
-	out.ProvidedApis = provided
-	out.RequiredApis = required
-
-	dependencies, err := s.GetDependenciesForBundle(ctx, name.String, version.String, bundlePath.String)
-	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("no entry found for %s %s", pkg, channel)
 	}
-	out.Dependencies = dependencies
-
-	properties, err := s.GetPropertiesForBundle(ctx, name.String, version.String, bundlePath.String)
-	if err != nil {
+	var (
+		name sql.NullString
+		csv  sql.NullString
+	)
+	if err := rows.Scan(&name, &csv); err != nil {
 		return nil, err
 	}
-	out.Properties = properties
 
-	return out, nil
+	return &api.Bundle{
+		CsvName: name.String,
+		CsvJson: csv.String,
+	}, nil
 }
 
 func (s *SQLQuerier) GetChannelEntriesThatReplace(ctx context.Context, name string) (entries []*registry.ChannelEntry, err error) {