From ba7041aac097abc50289528dd546e61dc6117167 Mon Sep 17 00:00:00 2001 From: Ben Luddy Date: Wed, 28 Jul 2021 13:39:45 -0400 Subject: [PATCH 1/5] Aggregate properties/dependencies in-query for ListBundles. (#720) This fixes the result set to contain one row per result Bundle instead of potentially several, which should allow for gRPC result streaming. Signed-off-by: Ben Luddy Upstream-repository: operator-registry Upstream-commit: e7f618928629a3492246e30bceebc0ccd55c4b11 --- staging/operator-registry/pkg/sqlite/query.go | 144 ++++++++---------- .../pkg/sqlite/query_sql_test.go | 4 +- .../pkg/sqlite/query_test.go | 51 +++---- .../operator-registry/pkg/sqlite/query.go | 144 ++++++++---------- 4 files changed, 144 insertions(+), 199 deletions(-) diff --git a/staging/operator-registry/pkg/sqlite/query.go b/staging/operator-registry/pkg/sqlite/query.go index 5b68340359..c8f7489b6b 100644 --- a/staging/operator-registry/pkg/sqlite/query.go +++ b/staging/operator-registry/pkg/sqlite/query.go @@ -970,6 +970,16 @@ tip (depth) AS ( INNER JOIN channel_entry AS skipped_entry ON skips_entry.skips = skipped_entry.entry_id GROUP BY all_entry.operatorbundle_name, all_entry.package_name, all_entry.channel_name +), +merged_properties (bundle_name, merged) AS ( + SELECT operatorbundle_name, json_group_array(json_object('type', properties.type, 'value', properties.value)) + FROM properties + GROUP BY operatorbundle_name +), +merged_dependencies (bundle_name, merged) AS ( + SELECT operatorbundle_name, json_group_array(json_object('type', dependencies.type, 'value', CAST(dependencies.value AS TEXT))) + FROM dependencies + GROUP BY operatorbundle_name ) SELECT replaces_bundle.entry_id, @@ -982,10 +992,8 @@ SELECT skips_bundle.skips, operatorbundle.version, operatorbundle.skiprange, - dependencies.type, - dependencies.value, - properties.type, - properties.value + merged_dependencies.merged, + merged_properties.merged FROM replaces_bundle INNER JOIN operatorbundle ON replaces_bundle.operatorbundle_name = operatorbundle.name @@ -993,10 +1001,10 @@ SELECT ON replaces_bundle.operatorbundle_name = skips_bundle.operatorbundle_name AND replaces_bundle.package_name = skips_bundle.package_name AND replaces_bundle.channel_name = skips_bundle.channel_name - LEFT OUTER JOIN dependencies - ON operatorbundle.name = dependencies.operatorbundle_name - LEFT OUTER JOIN properties - ON operatorbundle.name = properties.operatorbundle_name` + LEFT OUTER JOIN merged_dependencies + ON operatorbundle.name = merged_dependencies.bundle_name + LEFT OUTER JOIN merged_properties + ON operatorbundle.name = merged_properties.bundle_name` func (s *SQLQuerier) ListBundles(ctx context.Context) ([]*api.Bundle, error) { rows, err := s.db.QueryContext(ctx, listBundlesQuery) @@ -1006,7 +1014,6 @@ func (s *SQLQuerier) ListBundles(ctx context.Context) ([]*api.Bundle, error) { defer rows.Close() var bundles []*api.Bundle - bundlesMap := map[string]*api.Bundle{} for rows.Next() { var ( entryID sql.NullInt64 @@ -1019,12 +1026,10 @@ func (s *SQLQuerier) ListBundles(ctx context.Context) ([]*api.Bundle, error) { skips sql.NullString version sql.NullString skipRange sql.NullString - depType sql.NullString - depValue sql.NullString - propType sql.NullString - propValue sql.NullString + deps sql.NullString + props sql.NullString ) - if err := rows.Scan(&entryID, &bundle, &bundlePath, &bundleName, &pkgName, &channelName, &replaces, &skips, &version, &skipRange, &depType, &depValue, &propType, &propValue); err != nil { + if err := rows.Scan(&entryID, &bundle, &bundlePath, &bundleName, &pkgName, &channelName, &replaces, &skips, &version, &skipRange, &deps, &props); err != nil { return nil, err } @@ -1032,88 +1037,60 @@ func (s *SQLQuerier) ListBundles(ctx context.Context) ([]*api.Bundle, error) { continue } - bundleKey := fmt.Sprintf("%s/%s/%s/%s", bundleName.String, version.String, bundlePath.String, channelName.String) - bundleItem, ok := bundlesMap[bundleKey] - if ok { - if depType.Valid && depValue.Valid { - bundleItem.Dependencies = append(bundleItem.Dependencies, &api.Dependency{ - Type: depType.String, - Value: depValue.String, - }) - } - - if propType.Valid && propValue.Valid { - bundleItem.Properties = append(bundleItem.Properties, &api.Property{ - Type: propType.String, - Value: propValue.String, - }) - } - } else { - // Create new bundle - out := &api.Bundle{} - if bundle.Valid && bundle.String != "" { - out, err = registry.BundleStringToAPIBundle(bundle.String) - if err != nil { - return nil, err - } - } - - out.CsvName = bundleName.String - out.PackageName = pkgName.String - out.ChannelName = channelName.String - out.BundlePath = bundlePath.String - out.Version = version.String - out.SkipRange = skipRange.String - out.Replaces = replaces.String - if skips.Valid { - out.Skips = strings.Split(skips.String, ",") - } - - provided, required, err := s.GetApisForEntry(ctx, entryID.Int64) + out := &api.Bundle{} + if bundle.Valid && bundle.String != "" { + out, err = registry.BundleStringToAPIBundle(bundle.String) if err != nil { return nil, err } - if len(provided) > 0 { - out.ProvidedApis = provided - } - if len(required) > 0 { - out.RequiredApis = required - } + } + out.CsvName = bundleName.String + out.PackageName = pkgName.String + out.ChannelName = channelName.String + out.BundlePath = bundlePath.String + out.Version = version.String + out.SkipRange = skipRange.String + out.Replaces = replaces.String + + if skips.Valid { + out.Skips = strings.Split(skips.String, ",") + } - if depType.Valid && depValue.Valid { - out.Dependencies = []*api.Dependency{{ - Type: depType.String, - Value: depValue.String, - }} - } + provided, required, err := s.GetApisForEntry(ctx, entryID.Int64) + if err != nil { + return nil, err + } + if len(provided) > 0 { + out.ProvidedApis = provided + } + if len(required) > 0 { + out.RequiredApis = required + } - if propType.Valid && propValue.Valid { - out.Properties = []*api.Property{{ - Type: propType.String, - Value: propValue.String, - }} + if deps.Valid { + if err := json.Unmarshal([]byte(deps.String), &out.Dependencies); err != nil { + return nil, err } - - bundlesMap[bundleKey] = out } - } + out.Dependencies = uniqueDeps(out.Dependencies) - for _, v := range bundlesMap { - if len(v.Dependencies) > 1 { - newDeps := unique(v.Dependencies) - v.Dependencies = newDeps - } - if len(v.Properties) > 1 { - newProps := uniqueProps(v.Properties) - v.Properties = newProps + if props.Valid { + if err := json.Unmarshal([]byte(props.String), &out.Properties); err != nil { + return nil, err + } } - bundles = append(bundles, v) + out.Properties = uniqueProps(out.Properties) + + bundles = append(bundles, out) } return bundles, nil } -func unique(deps []*api.Dependency) []*api.Dependency { +func uniqueDeps(deps []*api.Dependency) []*api.Dependency { + if len(deps) <= 1 { + return deps + } keys := make(map[string]struct{}) var list []*api.Dependency for _, entry := range deps { @@ -1127,6 +1104,9 @@ func unique(deps []*api.Dependency) []*api.Dependency { } func uniqueProps(props []*api.Property) []*api.Property { + if len(props) <= 1 { + return props + } keys := make(map[string]struct{}) var list []*api.Property for _, entry := range props { diff --git a/staging/operator-registry/pkg/sqlite/query_sql_test.go b/staging/operator-registry/pkg/sqlite/query_sql_test.go index 81468231e7..3ba82f5b46 100644 --- a/staging/operator-registry/pkg/sqlite/query_sql_test.go +++ b/staging/operator-registry/pkg/sqlite/query_sql_test.go @@ -42,7 +42,7 @@ func TestListBundlesQuery(t *testing.T) { c interface{} name, actual sql.NullString ) - if err := rows.Scan(&c, &c, &c, &name, &c, &c, &actual, &c, &c, &c, &c, &c, &c, &c); err != nil { + if err := rows.Scan(&c, &c, &c, &name, &c, &c, &actual, &c, &c, &c, &c, &c); err != nil { t.Fatalf("unexpected error during row scan: %v", err) } expected, ok := replacements[name] @@ -107,7 +107,7 @@ func TestListBundlesQuery(t *testing.T) { c interface{} actual result ) - if err := rows.Scan(&c, &c, &c, &actual.Name, &c, &c, &actual.Replaces, &actual.Skips, &c, &c, &c, &c, &c, &c); err != nil { + if err := rows.Scan(&c, &c, &c, &actual.Name, &c, &c, &actual.Replaces, &actual.Skips, &c, &c, &c, &c); err != nil { t.Fatalf("unexpected error during row scan: %v", err) } r, ok := expected[actual.Name] diff --git a/staging/operator-registry/pkg/sqlite/query_test.go b/staging/operator-registry/pkg/sqlite/query_test.go index c657771b73..110c4e14e0 100644 --- a/staging/operator-registry/pkg/sqlite/query_test.go +++ b/staging/operator-registry/pkg/sqlite/query_test.go @@ -15,20 +15,18 @@ import ( func TestListBundles(t *testing.T) { type Columns struct { - EntryID sql.NullInt64 - Bundle sql.NullString - BundlePath sql.NullString - BundleName sql.NullString - PackageName sql.NullString - ChannelName sql.NullString - Replaces sql.NullString - Skips sql.NullString - Version sql.NullString - SkipRange sql.NullString - DependencyType sql.NullString - DependencyValue sql.NullString - PropertyType sql.NullString - PropertyValue sql.NullString + EntryID sql.NullInt64 + Bundle sql.NullString + BundlePath sql.NullString + BundleName sql.NullString + PackageName sql.NullString + ChannelName sql.NullString + Replaces sql.NullString + Skips sql.NullString + Version sql.NullString + SkipRange sql.NullString + Dependencies sql.NullString + Properties sql.NullString } var NoRows sqlitefakes.FakeRowScanner @@ -195,27 +193,14 @@ func TestListBundles(t *testing.T) { q.QueryContextReturns(&NoRows, nil) q.QueryContextReturnsOnCall(0, &r, nil) r.NextReturnsOnCall(0, true) - r.NextReturnsOnCall(1, true) cols := []Columns{ { - BundleName: sql.NullString{Valid: true, String: "BundleName"}, - Version: sql.NullString{Valid: true, String: "Version"}, - ChannelName: sql.NullString{Valid: true, String: "ChannelName"}, - BundlePath: sql.NullString{Valid: true, String: "BundlePath"}, - DependencyType: sql.NullString{Valid: true, String: "Dependency1Type"}, - DependencyValue: sql.NullString{Valid: true, String: "Dependency1Value"}, - PropertyType: sql.NullString{Valid: true, String: "Property1Type"}, - PropertyValue: sql.NullString{Valid: true, String: "Property1Value"}, - }, - { - BundleName: sql.NullString{Valid: true, String: "BundleName"}, - Version: sql.NullString{Valid: true, String: "Version"}, - ChannelName: sql.NullString{Valid: true, String: "ChannelName"}, - BundlePath: sql.NullString{Valid: true, String: "BundlePath"}, - DependencyType: sql.NullString{Valid: true, String: "Dependency2Type"}, - DependencyValue: sql.NullString{Valid: true, String: "Dependency2Value"}, - PropertyType: sql.NullString{Valid: true, String: "Property2Type"}, - PropertyValue: sql.NullString{Valid: true, String: "Property2Value"}, + BundleName: sql.NullString{Valid: true, String: "BundleName"}, + Version: sql.NullString{Valid: true, String: "Version"}, + ChannelName: sql.NullString{Valid: true, String: "ChannelName"}, + BundlePath: sql.NullString{Valid: true, String: "BundlePath"}, + Dependencies: sql.NullString{Valid: true, String: `[{"type":"Dependency1Type","value":"Dependency1Value"},{"type":"Dependency2Type","value":"Dependency2Value"}]`}, + Properties: sql.NullString{Valid: true, String: `[{"type":"Property1Type","value":"Property1Value"},{"type":"Property2Type","value":"Property2Value"}]`}, }, } var i int diff --git a/vendor/github.com/operator-framework/operator-registry/pkg/sqlite/query.go b/vendor/github.com/operator-framework/operator-registry/pkg/sqlite/query.go index 5b68340359..c8f7489b6b 100644 --- a/vendor/github.com/operator-framework/operator-registry/pkg/sqlite/query.go +++ b/vendor/github.com/operator-framework/operator-registry/pkg/sqlite/query.go @@ -970,6 +970,16 @@ tip (depth) AS ( INNER JOIN channel_entry AS skipped_entry ON skips_entry.skips = skipped_entry.entry_id GROUP BY all_entry.operatorbundle_name, all_entry.package_name, all_entry.channel_name +), +merged_properties (bundle_name, merged) AS ( + SELECT operatorbundle_name, json_group_array(json_object('type', properties.type, 'value', properties.value)) + FROM properties + GROUP BY operatorbundle_name +), +merged_dependencies (bundle_name, merged) AS ( + SELECT operatorbundle_name, json_group_array(json_object('type', dependencies.type, 'value', CAST(dependencies.value AS TEXT))) + FROM dependencies + GROUP BY operatorbundle_name ) SELECT replaces_bundle.entry_id, @@ -982,10 +992,8 @@ SELECT skips_bundle.skips, operatorbundle.version, operatorbundle.skiprange, - dependencies.type, - dependencies.value, - properties.type, - properties.value + merged_dependencies.merged, + merged_properties.merged FROM replaces_bundle INNER JOIN operatorbundle ON replaces_bundle.operatorbundle_name = operatorbundle.name @@ -993,10 +1001,10 @@ SELECT ON replaces_bundle.operatorbundle_name = skips_bundle.operatorbundle_name AND replaces_bundle.package_name = skips_bundle.package_name AND replaces_bundle.channel_name = skips_bundle.channel_name - LEFT OUTER JOIN dependencies - ON operatorbundle.name = dependencies.operatorbundle_name - LEFT OUTER JOIN properties - ON operatorbundle.name = properties.operatorbundle_name` + LEFT OUTER JOIN merged_dependencies + ON operatorbundle.name = merged_dependencies.bundle_name + LEFT OUTER JOIN merged_properties + ON operatorbundle.name = merged_properties.bundle_name` func (s *SQLQuerier) ListBundles(ctx context.Context) ([]*api.Bundle, error) { rows, err := s.db.QueryContext(ctx, listBundlesQuery) @@ -1006,7 +1014,6 @@ func (s *SQLQuerier) ListBundles(ctx context.Context) ([]*api.Bundle, error) { defer rows.Close() var bundles []*api.Bundle - bundlesMap := map[string]*api.Bundle{} for rows.Next() { var ( entryID sql.NullInt64 @@ -1019,12 +1026,10 @@ func (s *SQLQuerier) ListBundles(ctx context.Context) ([]*api.Bundle, error) { skips sql.NullString version sql.NullString skipRange sql.NullString - depType sql.NullString - depValue sql.NullString - propType sql.NullString - propValue sql.NullString + deps sql.NullString + props sql.NullString ) - if err := rows.Scan(&entryID, &bundle, &bundlePath, &bundleName, &pkgName, &channelName, &replaces, &skips, &version, &skipRange, &depType, &depValue, &propType, &propValue); err != nil { + if err := rows.Scan(&entryID, &bundle, &bundlePath, &bundleName, &pkgName, &channelName, &replaces, &skips, &version, &skipRange, &deps, &props); err != nil { return nil, err } @@ -1032,88 +1037,60 @@ func (s *SQLQuerier) ListBundles(ctx context.Context) ([]*api.Bundle, error) { continue } - bundleKey := fmt.Sprintf("%s/%s/%s/%s", bundleName.String, version.String, bundlePath.String, channelName.String) - bundleItem, ok := bundlesMap[bundleKey] - if ok { - if depType.Valid && depValue.Valid { - bundleItem.Dependencies = append(bundleItem.Dependencies, &api.Dependency{ - Type: depType.String, - Value: depValue.String, - }) - } - - if propType.Valid && propValue.Valid { - bundleItem.Properties = append(bundleItem.Properties, &api.Property{ - Type: propType.String, - Value: propValue.String, - }) - } - } else { - // Create new bundle - out := &api.Bundle{} - if bundle.Valid && bundle.String != "" { - out, err = registry.BundleStringToAPIBundle(bundle.String) - if err != nil { - return nil, err - } - } - - out.CsvName = bundleName.String - out.PackageName = pkgName.String - out.ChannelName = channelName.String - out.BundlePath = bundlePath.String - out.Version = version.String - out.SkipRange = skipRange.String - out.Replaces = replaces.String - if skips.Valid { - out.Skips = strings.Split(skips.String, ",") - } - - provided, required, err := s.GetApisForEntry(ctx, entryID.Int64) + out := &api.Bundle{} + if bundle.Valid && bundle.String != "" { + out, err = registry.BundleStringToAPIBundle(bundle.String) if err != nil { return nil, err } - if len(provided) > 0 { - out.ProvidedApis = provided - } - if len(required) > 0 { - out.RequiredApis = required - } + } + out.CsvName = bundleName.String + out.PackageName = pkgName.String + out.ChannelName = channelName.String + out.BundlePath = bundlePath.String + out.Version = version.String + out.SkipRange = skipRange.String + out.Replaces = replaces.String + + if skips.Valid { + out.Skips = strings.Split(skips.String, ",") + } - if depType.Valid && depValue.Valid { - out.Dependencies = []*api.Dependency{{ - Type: depType.String, - Value: depValue.String, - }} - } + provided, required, err := s.GetApisForEntry(ctx, entryID.Int64) + if err != nil { + return nil, err + } + if len(provided) > 0 { + out.ProvidedApis = provided + } + if len(required) > 0 { + out.RequiredApis = required + } - if propType.Valid && propValue.Valid { - out.Properties = []*api.Property{{ - Type: propType.String, - Value: propValue.String, - }} + if deps.Valid { + if err := json.Unmarshal([]byte(deps.String), &out.Dependencies); err != nil { + return nil, err } - - bundlesMap[bundleKey] = out } - } + out.Dependencies = uniqueDeps(out.Dependencies) - for _, v := range bundlesMap { - if len(v.Dependencies) > 1 { - newDeps := unique(v.Dependencies) - v.Dependencies = newDeps - } - if len(v.Properties) > 1 { - newProps := uniqueProps(v.Properties) - v.Properties = newProps + if props.Valid { + if err := json.Unmarshal([]byte(props.String), &out.Properties); err != nil { + return nil, err + } } - bundles = append(bundles, v) + out.Properties = uniqueProps(out.Properties) + + bundles = append(bundles, out) } return bundles, nil } -func unique(deps []*api.Dependency) []*api.Dependency { +func uniqueDeps(deps []*api.Dependency) []*api.Dependency { + if len(deps) <= 1 { + return deps + } keys := make(map[string]struct{}) var list []*api.Dependency for _, entry := range deps { @@ -1127,6 +1104,9 @@ func unique(deps []*api.Dependency) []*api.Dependency { } func uniqueProps(props []*api.Property) []*api.Property { + if len(props) <= 1 { + return props + } keys := make(map[string]struct{}) var list []*api.Property for _, entry := range props { From 1b907c7ede5103ba9b28248545e3aa829d449992 Mon Sep 17 00:00:00 2001 From: Ben Luddy Date: Thu, 29 Jul 2021 09:51:23 -0400 Subject: [PATCH 2/5] Set legacy API fields in ListBundles using available properties. (#724) For each bundle in its result, ListBundles has been making three separate database queries -- for properties, dependencies, and plural API names -- via GetApisForEntry in order to populate the bundle fields providedApis and requiredApis. Properties and dependencies are already in hand and can be reused. Setting plural names on the elements of providedApis and requiredApis is pointless because plurals are discarded by the catalog operator at runtime. Signed-off-by: Ben Luddy Upstream-repository: operator-registry Upstream-commit: 5364719fc15a53fb6cdc9645ad4b05b778f77346 --- staging/operator-registry/pkg/sqlite/query.go | 49 ++++++++++++++----- .../operator-registry/pkg/sqlite/query.go | 49 ++++++++++++++----- 2 files changed, 76 insertions(+), 22 deletions(-) diff --git a/staging/operator-registry/pkg/sqlite/query.go b/staging/operator-registry/pkg/sqlite/query.go index c8f7489b6b..29206f1298 100644 --- a/staging/operator-registry/pkg/sqlite/query.go +++ b/staging/operator-registry/pkg/sqlite/query.go @@ -1056,22 +1056,12 @@ func (s *SQLQuerier) ListBundles(ctx context.Context) ([]*api.Bundle, error) { out.Skips = strings.Split(skips.String, ",") } - provided, required, err := s.GetApisForEntry(ctx, entryID.Int64) - if err != nil { - return nil, err - } - if len(provided) > 0 { - out.ProvidedApis = provided - } - if len(required) > 0 { - out.RequiredApis = required - } - if deps.Valid { if err := json.Unmarshal([]byte(deps.String), &out.Dependencies); err != nil { return nil, err } } + buildLegacyRequiredAPIs(out.Dependencies, &out.RequiredApis) out.Dependencies = uniqueDeps(out.Dependencies) if props.Valid { @@ -1079,6 +1069,7 @@ func (s *SQLQuerier) ListBundles(ctx context.Context) ([]*api.Bundle, error) { return nil, err } } + buildLegacyProvidedAPIs(out.Properties, &out.ProvidedApis) out.Properties = uniqueProps(out.Properties) bundles = append(bundles, out) @@ -1087,6 +1078,42 @@ func (s *SQLQuerier) ListBundles(ctx context.Context) ([]*api.Bundle, error) { return bundles, nil } +func buildLegacyRequiredAPIs(src []*api.Dependency, dst *[]*api.GroupVersionKind) error { + for _, p := range src { + if p.GetType() != registry.GVKType { + continue + } + var value registry.GVKDependency + if err := json.Unmarshal([]byte(p.GetValue()), &value); err != nil { + return err + } + *dst = append(*dst, &api.GroupVersionKind{ + Group: value.Group, + Version: value.Version, + Kind: value.Kind, + }) + } + return nil +} + +func buildLegacyProvidedAPIs(src []*api.Property, dst *[]*api.GroupVersionKind) error { + for _, p := range src { + if p.GetType() != registry.GVKType { + continue + } + var value registry.GVKProperty + if err := json.Unmarshal([]byte(p.GetValue()), &value); err != nil { + return err + } + *dst = append(*dst, &api.GroupVersionKind{ + Group: value.Group, + Version: value.Version, + Kind: value.Kind, + }) + } + return nil +} + func uniqueDeps(deps []*api.Dependency) []*api.Dependency { if len(deps) <= 1 { return deps diff --git a/vendor/github.com/operator-framework/operator-registry/pkg/sqlite/query.go b/vendor/github.com/operator-framework/operator-registry/pkg/sqlite/query.go index c8f7489b6b..29206f1298 100644 --- a/vendor/github.com/operator-framework/operator-registry/pkg/sqlite/query.go +++ b/vendor/github.com/operator-framework/operator-registry/pkg/sqlite/query.go @@ -1056,22 +1056,12 @@ func (s *SQLQuerier) ListBundles(ctx context.Context) ([]*api.Bundle, error) { out.Skips = strings.Split(skips.String, ",") } - provided, required, err := s.GetApisForEntry(ctx, entryID.Int64) - if err != nil { - return nil, err - } - if len(provided) > 0 { - out.ProvidedApis = provided - } - if len(required) > 0 { - out.RequiredApis = required - } - if deps.Valid { if err := json.Unmarshal([]byte(deps.String), &out.Dependencies); err != nil { return nil, err } } + buildLegacyRequiredAPIs(out.Dependencies, &out.RequiredApis) out.Dependencies = uniqueDeps(out.Dependencies) if props.Valid { @@ -1079,6 +1069,7 @@ func (s *SQLQuerier) ListBundles(ctx context.Context) ([]*api.Bundle, error) { return nil, err } } + buildLegacyProvidedAPIs(out.Properties, &out.ProvidedApis) out.Properties = uniqueProps(out.Properties) bundles = append(bundles, out) @@ -1087,6 +1078,42 @@ func (s *SQLQuerier) ListBundles(ctx context.Context) ([]*api.Bundle, error) { return bundles, nil } +func buildLegacyRequiredAPIs(src []*api.Dependency, dst *[]*api.GroupVersionKind) error { + for _, p := range src { + if p.GetType() != registry.GVKType { + continue + } + var value registry.GVKDependency + if err := json.Unmarshal([]byte(p.GetValue()), &value); err != nil { + return err + } + *dst = append(*dst, &api.GroupVersionKind{ + Group: value.Group, + Version: value.Version, + Kind: value.Kind, + }) + } + return nil +} + +func buildLegacyProvidedAPIs(src []*api.Property, dst *[]*api.GroupVersionKind) error { + for _, p := range src { + if p.GetType() != registry.GVKType { + continue + } + var value registry.GVKProperty + if err := json.Unmarshal([]byte(p.GetValue()), &value); err != nil { + return err + } + *dst = append(*dst, &api.GroupVersionKind{ + Group: value.Group, + Version: value.Version, + Kind: value.Kind, + }) + } + return nil +} + func uniqueDeps(deps []*api.Dependency) []*api.Dependency { if len(deps) <= 1 { return deps From d6d2b6d14e6d48f30b3c40da47fccec725f3399e Mon Sep 17 00:00:00 2001 From: Alexander Greene Date: Thu, 29 Jul 2021 11:35:23 -0400 Subject: [PATCH 3/5] Update Registry to stream bundles (#723) This commit introduces a change to the registry's ListBundles command so it may stream bundles as they become available. Signed-off-by: Alexander Greene Upstream-repository: operator-registry Upstream-commit: 36205150680349edc0c816d0dac8585780e30b6f --- .../operator-registry/pkg/registry/empty.go | 4 +++ .../pkg/registry/interface.go | 7 +++++ .../operator-registry/pkg/registry/query.go | 29 +++++++++++++++---- .../operator-registry/pkg/server/server.go | 12 +------- staging/operator-registry/pkg/sqlite/query.go | 28 ++++++++++++------ .../operator-registry/pkg/registry/empty.go | 4 +++ .../pkg/registry/interface.go | 7 +++++ .../operator-registry/pkg/registry/query.go | 29 +++++++++++++++---- .../operator-registry/pkg/server/server.go | 12 +------- .../operator-registry/pkg/sqlite/query.go | 28 ++++++++++++------ 10 files changed, 110 insertions(+), 50 deletions(-) diff --git a/staging/operator-registry/pkg/registry/empty.go b/staging/operator-registry/pkg/registry/empty.go index fd4676eb6c..936f39ccab 100644 --- a/staging/operator-registry/pkg/registry/empty.go +++ b/staging/operator-registry/pkg/registry/empty.go @@ -100,6 +100,10 @@ func (EmptyQuery) ListBundles(ctx context.Context) ([]*api.Bundle, error) { return nil, errors.New("empty querier: cannot list bundles") } +func (EmptyQuery) SendBundles(ctx context.Context, stream BundleSender) error { + return errors.New("empty querier: cannot stream bundles") +} + func (EmptyQuery) GetDependenciesForBundle(ctx context.Context, name, version, path string) (dependencies []*api.Dependency, err error) { return nil, errors.New("empty querier: cannot get dependencies for bundle") } diff --git a/staging/operator-registry/pkg/registry/interface.go b/staging/operator-registry/pkg/registry/interface.go index fd77636117..454cec3869 100644 --- a/staging/operator-registry/pkg/registry/interface.go +++ b/staging/operator-registry/pkg/registry/interface.go @@ -17,10 +17,17 @@ type Load interface { ClearNonHeadBundles() error } +type BundleSender interface { + Send(*api.Bundle) error +} + type GRPCQuery interface { // List all available package names in the index ListPackages(ctx context.Context) ([]string, error) + // Sends all available bundles in the index + SendBundles(ctx context.Context, stream BundleSender) error + // List all available bundles in the index ListBundles(ctx context.Context) (bundles []*api.Bundle, err error) diff --git a/staging/operator-registry/pkg/registry/query.go b/staging/operator-registry/pkg/registry/query.go index 9acd9d48fa..6292525488 100644 --- a/staging/operator-registry/pkg/registry/query.go +++ b/staging/operator-registry/pkg/registry/query.go @@ -13,6 +13,14 @@ type Querier struct { pkgs model.Model } +type SliceBundleSender []*api.Bundle + +func (s *SliceBundleSender) Send(b *api.Bundle) error { + + *s = append(*s, b) + return nil +} + var _ GRPCQuery = &Querier{} func NewQuerier(packages model.Model) *Querier { @@ -29,21 +37,32 @@ func (q Querier) ListPackages(_ context.Context) ([]string, error) { return packages, nil } -func (q Querier) ListBundles(_ context.Context) ([]*api.Bundle, error) { - var bundles []*api.Bundle +func (q Querier) ListBundles(ctx context.Context) ([]*api.Bundle, error) { + var bundleSender SliceBundleSender + err := q.SendBundles(ctx, &bundleSender) + if err != nil { + return nil, err + } + + return bundleSender, nil +} + +func (q Querier) SendBundles(_ context.Context, s BundleSender) error { for _, pkg := range q.pkgs { for _, ch := range pkg.Channels { for _, b := range ch.Bundles { apiBundle, err := api.ConvertModelBundleToAPIBundle(*b) if err != nil { - return nil, fmt.Errorf("convert bundle %q: %v", b.Name, err) + return fmt.Errorf("convert bundle %q: %v", b.Name, err) + } + if err := s.Send(apiBundle); err != nil { + return err } - bundles = append(bundles, apiBundle) } } } - return bundles, nil + return nil } func (q Querier) GetPackage(_ context.Context, name string) (*PackageManifest, error) { diff --git a/staging/operator-registry/pkg/server/server.go b/staging/operator-registry/pkg/server/server.go index 9fe1592c3e..64b87ce70e 100644 --- a/staging/operator-registry/pkg/server/server.go +++ b/staging/operator-registry/pkg/server/server.go @@ -33,17 +33,7 @@ func (s *RegistryServer) ListPackages(req *api.ListPackageRequest, stream api.Re } func (s *RegistryServer) ListBundles(req *api.ListBundlesRequest, stream api.Registry_ListBundlesServer) error { - bundles, err := s.store.ListBundles(stream.Context()) - if err != nil { - return err - } - for _, b := range bundles { - if err := stream.Send(b); err != nil { - return err - } - } - - return nil + return s.store.SendBundles(stream.Context(), stream) } func (s *RegistryServer) GetPackage(ctx context.Context, req *api.GetPackageRequest) (*api.Package, error) { diff --git a/staging/operator-registry/pkg/sqlite/query.go b/staging/operator-registry/pkg/sqlite/query.go index 29206f1298..0b8153d32f 100644 --- a/staging/operator-registry/pkg/sqlite/query.go +++ b/staging/operator-registry/pkg/sqlite/query.go @@ -1006,14 +1006,13 @@ SELECT LEFT OUTER JOIN merged_properties ON operatorbundle.name = merged_properties.bundle_name` -func (s *SQLQuerier) ListBundles(ctx context.Context) ([]*api.Bundle, error) { +func (s *SQLQuerier) SendBundles(ctx context.Context, stream registry.BundleSender) error { rows, err := s.db.QueryContext(ctx, listBundlesQuery) if err != nil { - return nil, err + return err } defer rows.Close() - var bundles []*api.Bundle for rows.Next() { var ( entryID sql.NullInt64 @@ -1030,7 +1029,7 @@ func (s *SQLQuerier) ListBundles(ctx context.Context) ([]*api.Bundle, error) { props sql.NullString ) if err := rows.Scan(&entryID, &bundle, &bundlePath, &bundleName, &pkgName, &channelName, &replaces, &skips, &version, &skipRange, &deps, &props); err != nil { - return nil, err + return err } if !bundleName.Valid || !version.Valid || !bundlePath.Valid || !channelName.Valid { @@ -1041,7 +1040,7 @@ func (s *SQLQuerier) ListBundles(ctx context.Context) ([]*api.Bundle, error) { if bundle.Valid && bundle.String != "" { out, err = registry.BundleStringToAPIBundle(bundle.String) if err != nil { - return nil, err + return err } } out.CsvName = bundleName.String @@ -1058,7 +1057,7 @@ func (s *SQLQuerier) ListBundles(ctx context.Context) ([]*api.Bundle, error) { if deps.Valid { if err := json.Unmarshal([]byte(deps.String), &out.Dependencies); err != nil { - return nil, err + return err } } buildLegacyRequiredAPIs(out.Dependencies, &out.RequiredApis) @@ -1066,16 +1065,27 @@ func (s *SQLQuerier) ListBundles(ctx context.Context) ([]*api.Bundle, error) { if props.Valid { if err := json.Unmarshal([]byte(props.String), &out.Properties); err != nil { - return nil, err + return err } } buildLegacyProvidedAPIs(out.Properties, &out.ProvidedApis) out.Properties = uniqueProps(out.Properties) + if err := stream.Send(out); err != nil { + return err + } + } - bundles = append(bundles, out) + return nil +} + +func (s *SQLQuerier) ListBundles(ctx context.Context) ([]*api.Bundle, error) { + var bundleSender registry.SliceBundleSender + err := s.SendBundles(ctx, &bundleSender) + if err != nil { + return nil, err } + return bundleSender, nil - return bundles, nil } func buildLegacyRequiredAPIs(src []*api.Dependency, dst *[]*api.GroupVersionKind) error { diff --git a/vendor/github.com/operator-framework/operator-registry/pkg/registry/empty.go b/vendor/github.com/operator-framework/operator-registry/pkg/registry/empty.go index fd4676eb6c..936f39ccab 100644 --- a/vendor/github.com/operator-framework/operator-registry/pkg/registry/empty.go +++ b/vendor/github.com/operator-framework/operator-registry/pkg/registry/empty.go @@ -100,6 +100,10 @@ func (EmptyQuery) ListBundles(ctx context.Context) ([]*api.Bundle, error) { return nil, errors.New("empty querier: cannot list bundles") } +func (EmptyQuery) SendBundles(ctx context.Context, stream BundleSender) error { + return errors.New("empty querier: cannot stream bundles") +} + func (EmptyQuery) GetDependenciesForBundle(ctx context.Context, name, version, path string) (dependencies []*api.Dependency, err error) { return nil, errors.New("empty querier: cannot get dependencies for bundle") } diff --git a/vendor/github.com/operator-framework/operator-registry/pkg/registry/interface.go b/vendor/github.com/operator-framework/operator-registry/pkg/registry/interface.go index fd77636117..454cec3869 100644 --- a/vendor/github.com/operator-framework/operator-registry/pkg/registry/interface.go +++ b/vendor/github.com/operator-framework/operator-registry/pkg/registry/interface.go @@ -17,10 +17,17 @@ type Load interface { ClearNonHeadBundles() error } +type BundleSender interface { + Send(*api.Bundle) error +} + type GRPCQuery interface { // List all available package names in the index ListPackages(ctx context.Context) ([]string, error) + // Sends all available bundles in the index + SendBundles(ctx context.Context, stream BundleSender) error + // List all available bundles in the index ListBundles(ctx context.Context) (bundles []*api.Bundle, err error) diff --git a/vendor/github.com/operator-framework/operator-registry/pkg/registry/query.go b/vendor/github.com/operator-framework/operator-registry/pkg/registry/query.go index 9acd9d48fa..6292525488 100644 --- a/vendor/github.com/operator-framework/operator-registry/pkg/registry/query.go +++ b/vendor/github.com/operator-framework/operator-registry/pkg/registry/query.go @@ -13,6 +13,14 @@ type Querier struct { pkgs model.Model } +type SliceBundleSender []*api.Bundle + +func (s *SliceBundleSender) Send(b *api.Bundle) error { + + *s = append(*s, b) + return nil +} + var _ GRPCQuery = &Querier{} func NewQuerier(packages model.Model) *Querier { @@ -29,21 +37,32 @@ func (q Querier) ListPackages(_ context.Context) ([]string, error) { return packages, nil } -func (q Querier) ListBundles(_ context.Context) ([]*api.Bundle, error) { - var bundles []*api.Bundle +func (q Querier) ListBundles(ctx context.Context) ([]*api.Bundle, error) { + var bundleSender SliceBundleSender + err := q.SendBundles(ctx, &bundleSender) + if err != nil { + return nil, err + } + + return bundleSender, nil +} + +func (q Querier) SendBundles(_ context.Context, s BundleSender) error { for _, pkg := range q.pkgs { for _, ch := range pkg.Channels { for _, b := range ch.Bundles { apiBundle, err := api.ConvertModelBundleToAPIBundle(*b) if err != nil { - return nil, fmt.Errorf("convert bundle %q: %v", b.Name, err) + return fmt.Errorf("convert bundle %q: %v", b.Name, err) + } + if err := s.Send(apiBundle); err != nil { + return err } - bundles = append(bundles, apiBundle) } } } - return bundles, nil + return nil } func (q Querier) GetPackage(_ context.Context, name string) (*PackageManifest, error) { diff --git a/vendor/github.com/operator-framework/operator-registry/pkg/server/server.go b/vendor/github.com/operator-framework/operator-registry/pkg/server/server.go index 9fe1592c3e..64b87ce70e 100644 --- a/vendor/github.com/operator-framework/operator-registry/pkg/server/server.go +++ b/vendor/github.com/operator-framework/operator-registry/pkg/server/server.go @@ -33,17 +33,7 @@ func (s *RegistryServer) ListPackages(req *api.ListPackageRequest, stream api.Re } func (s *RegistryServer) ListBundles(req *api.ListBundlesRequest, stream api.Registry_ListBundlesServer) error { - bundles, err := s.store.ListBundles(stream.Context()) - if err != nil { - return err - } - for _, b := range bundles { - if err := stream.Send(b); err != nil { - return err - } - } - - return nil + return s.store.SendBundles(stream.Context(), stream) } func (s *RegistryServer) GetPackage(ctx context.Context, req *api.GetPackageRequest) (*api.Package, error) { diff --git a/vendor/github.com/operator-framework/operator-registry/pkg/sqlite/query.go b/vendor/github.com/operator-framework/operator-registry/pkg/sqlite/query.go index 29206f1298..0b8153d32f 100644 --- a/vendor/github.com/operator-framework/operator-registry/pkg/sqlite/query.go +++ b/vendor/github.com/operator-framework/operator-registry/pkg/sqlite/query.go @@ -1006,14 +1006,13 @@ SELECT LEFT OUTER JOIN merged_properties ON operatorbundle.name = merged_properties.bundle_name` -func (s *SQLQuerier) ListBundles(ctx context.Context) ([]*api.Bundle, error) { +func (s *SQLQuerier) SendBundles(ctx context.Context, stream registry.BundleSender) error { rows, err := s.db.QueryContext(ctx, listBundlesQuery) if err != nil { - return nil, err + return err } defer rows.Close() - var bundles []*api.Bundle for rows.Next() { var ( entryID sql.NullInt64 @@ -1030,7 +1029,7 @@ func (s *SQLQuerier) ListBundles(ctx context.Context) ([]*api.Bundle, error) { props sql.NullString ) if err := rows.Scan(&entryID, &bundle, &bundlePath, &bundleName, &pkgName, &channelName, &replaces, &skips, &version, &skipRange, &deps, &props); err != nil { - return nil, err + return err } if !bundleName.Valid || !version.Valid || !bundlePath.Valid || !channelName.Valid { @@ -1041,7 +1040,7 @@ func (s *SQLQuerier) ListBundles(ctx context.Context) ([]*api.Bundle, error) { if bundle.Valid && bundle.String != "" { out, err = registry.BundleStringToAPIBundle(bundle.String) if err != nil { - return nil, err + return err } } out.CsvName = bundleName.String @@ -1058,7 +1057,7 @@ func (s *SQLQuerier) ListBundles(ctx context.Context) ([]*api.Bundle, error) { if deps.Valid { if err := json.Unmarshal([]byte(deps.String), &out.Dependencies); err != nil { - return nil, err + return err } } buildLegacyRequiredAPIs(out.Dependencies, &out.RequiredApis) @@ -1066,16 +1065,27 @@ func (s *SQLQuerier) ListBundles(ctx context.Context) ([]*api.Bundle, error) { if props.Valid { if err := json.Unmarshal([]byte(props.String), &out.Properties); err != nil { - return nil, err + return err } } buildLegacyProvidedAPIs(out.Properties, &out.ProvidedApis) out.Properties = uniqueProps(out.Properties) + if err := stream.Send(out); err != nil { + return err + } + } - bundles = append(bundles, out) + return nil +} + +func (s *SQLQuerier) ListBundles(ctx context.Context) ([]*api.Bundle, error) { + var bundleSender registry.SliceBundleSender + err := s.SendBundles(ctx, &bundleSender) + if err != nil { + return nil, err } + return bundleSender, nil - return bundles, nil } func buildLegacyRequiredAPIs(src []*api.Dependency, dst *[]*api.GroupVersionKind) error { From 36a8575d97b161279ec697a538024cf49147f41e Mon Sep 17 00:00:00 2001 From: Ben Luddy Date: Thu, 29 Jul 2021 13:42:23 -0400 Subject: [PATCH 4/5] Omit manifests from ListBundles result if bundle image is available. (#725) The "object" and "csvJson" fields of bundles returned from ListBundles is only consumed by the catalog operator in cases where no bundle image reference is present. Signed-off-by: Ben Luddy Upstream-repository: operator-registry Upstream-commit: 6a065947e60f3f2afc1dd3b934cbc802696a1807 --- .../cmd/opm/registry/serve.go | 2 +- .../cmd/registry-server/main.go | 2 +- .../operator-registry/pkg/registry/query.go | 8 +++ .../pkg/registry/query_test.go | 6 +- staging/operator-registry/pkg/sqlite/query.go | 37 ++++++++--- .../pkg/sqlite/query_sql_test.go | 65 +++++++++++++++++-- .../cmd/opm/registry/serve.go | 2 +- .../cmd/registry-server/main.go | 2 +- .../operator-registry/pkg/registry/query.go | 8 +++ .../operator-registry/pkg/sqlite/query.go | 37 ++++++++--- 10 files changed, 142 insertions(+), 27 deletions(-) diff --git a/staging/operator-registry/cmd/opm/registry/serve.go b/staging/operator-registry/cmd/opm/registry/serve.go index 74957a2c41..d8281e0cc2 100644 --- a/staging/operator-registry/cmd/opm/registry/serve.go +++ b/staging/operator-registry/cmd/opm/registry/serve.go @@ -95,7 +95,7 @@ func serveFunc(cmd *cobra.Command, args []string) error { logger.WithError(err).Warnf("couldn't migrate db") } - store := sqlite.NewSQLLiteQuerierFromDb(db) + store := sqlite.NewSQLLiteQuerierFromDb(db, sqlite.OmitManifests(true)) // sanity check that the db is available tables, err := store.ListTables(context.TODO()) diff --git a/staging/operator-registry/cmd/registry-server/main.go b/staging/operator-registry/cmd/registry-server/main.go index 9cd9e63ff3..67a6bd4e95 100644 --- a/staging/operator-registry/cmd/registry-server/main.go +++ b/staging/operator-registry/cmd/registry-server/main.go @@ -96,7 +96,7 @@ func runCmdFunc(cmd *cobra.Command, args []string) error { logger.WithError(err).Warnf("couldn't migrate db") } - store := sqlite.NewSQLLiteQuerierFromDb(db) + store := sqlite.NewSQLLiteQuerierFromDb(db, sqlite.OmitManifests(true)) // sanity check that the db is available tables, err := store.ListTables(context.TODO()) diff --git a/staging/operator-registry/pkg/registry/query.go b/staging/operator-registry/pkg/registry/query.go index 6292525488..20f0e9f95d 100644 --- a/staging/operator-registry/pkg/registry/query.go +++ b/staging/operator-registry/pkg/registry/query.go @@ -56,6 +56,14 @@ func (q Querier) SendBundles(_ context.Context, s BundleSender) error { if err != nil { return fmt.Errorf("convert bundle %q: %v", b.Name, err) } + if apiBundle.BundlePath != "" { + // The SQLite-based server + // configures its querier to + // omit these fields when + // bundle path is set. + apiBundle.CsvJson = "" + apiBundle.Object = nil + } if err := s.Send(apiBundle); err != nil { return err } diff --git a/staging/operator-registry/pkg/registry/query_test.go b/staging/operator-registry/pkg/registry/query_test.go index 0973230e3c..c5951a73d7 100644 --- a/staging/operator-registry/pkg/registry/query_test.go +++ b/staging/operator-registry/pkg/registry/query_test.go @@ -164,7 +164,11 @@ func TestQuerier_ListBundles(t *testing.T) { bundles, err := testModelQuerier.ListBundles(context.TODO()) require.NoError(t, err) require.NotNil(t, bundles) - require.Equal(t, 12, len(bundles)) + require.Len(t, bundles, 12) + for _, b := range bundles { + require.Zero(t, b.CsvJson) + require.Zero(t, b.Object) + } } func TestQuerier_ListPackages(t *testing.T) { diff --git a/staging/operator-registry/pkg/sqlite/query.go b/staging/operator-registry/pkg/sqlite/query.go index 0b8153d32f..fb24204941 100644 --- a/staging/operator-registry/pkg/sqlite/query.go +++ b/staging/operator-registry/pkg/sqlite/query.go @@ -35,25 +35,44 @@ func (a dbQuerierAdapter) QueryContext(ctx context.Context, query string, args . type SQLQuerier struct { db Querier + querierConfig } var _ registry.Query = &SQLQuerier{} -func NewSQLLiteQuerier(dbFilename string) (*SQLQuerier, error) { +type querierConfig struct { + omitManifests bool +} + +type SQLiteQuerierOption func(*querierConfig) + +// If true, ListBundles will omit inline manifests (the object and +// csvJson fields) from response elements that contain a bundle image +// reference. +func OmitManifests(b bool) SQLiteQuerierOption { + return func(c *querierConfig) { + c.omitManifests = b + } +} + +func NewSQLLiteQuerier(dbFilename string, opts ...SQLiteQuerierOption) (*SQLQuerier, error) { db, err := OpenReadOnly(dbFilename) if err != nil { return nil, err } - - return &SQLQuerier{dbQuerierAdapter{db}}, nil + return NewSQLLiteQuerierFromDb(db, opts...), nil } -func NewSQLLiteQuerierFromDb(db *sql.DB) *SQLQuerier { - return &SQLQuerier{dbQuerierAdapter{db}} +func NewSQLLiteQuerierFromDb(db *sql.DB, opts ...SQLiteQuerierOption) *SQLQuerier { + return NewSQLLiteQuerierFromDBQuerier(dbQuerierAdapter{db}, opts...) } -func NewSQLLiteQuerierFromDBQuerier(q Querier) *SQLQuerier { - return &SQLQuerier{q} +func NewSQLLiteQuerierFromDBQuerier(q Querier, opts ...SQLiteQuerierOption) *SQLQuerier { + sq := SQLQuerier{db: q} + for _, opt := range opts { + opt(&sq.querierConfig) + } + return &sq } func (s *SQLQuerier) ListTables(ctx context.Context) ([]string, error) { @@ -983,7 +1002,7 @@ merged_dependencies (bundle_name, merged) AS ( ) SELECT replaces_bundle.entry_id, - operatorbundle.bundle, + CASE WHEN :omit_manifests AND length(coalesce(operatorbundle.bundlepath, "")) > 0 THEN NULL ELSE operatorbundle.bundle END, operatorbundle.bundlepath, operatorbundle.name, replaces_bundle.package_name, @@ -1007,7 +1026,7 @@ SELECT ON operatorbundle.name = merged_properties.bundle_name` func (s *SQLQuerier) SendBundles(ctx context.Context, stream registry.BundleSender) error { - rows, err := s.db.QueryContext(ctx, listBundlesQuery) + rows, err := s.db.QueryContext(ctx, listBundlesQuery, sql.Named("omit_manifests", s.omitManifests)) if err != nil { return err } diff --git a/staging/operator-registry/pkg/sqlite/query_sql_test.go b/staging/operator-registry/pkg/sqlite/query_sql_test.go index 3ba82f5b46..73a19db705 100644 --- a/staging/operator-registry/pkg/sqlite/query_sql_test.go +++ b/staging/operator-registry/pkg/sqlite/query_sql_test.go @@ -12,9 +12,10 @@ import ( func TestListBundlesQuery(t *testing.T) { for _, tt := range []struct { - Name string - Setup func(t *testing.T, db *sql.DB) - Expect func(t *testing.T, rows *sql.Rows) + Name string + Setup func(t *testing.T, db *sql.DB) + Expect func(t *testing.T, rows *sql.Rows) + OmitManfests bool }{ { Name: "replacement comes from channel entry", @@ -126,6 +127,62 @@ func TestListBundlesQuery(t *testing.T) { } }, }, + { + Name: "manifests omitted without bundlepath", + OmitManfests: true, + Setup: func(t *testing.T, db *sql.DB) { + for _, stmt := range []string{ + `insert into package (name, default_channel) values ("package", "channel")`, + `insert into channel (name, package_name, head_operatorbundle_name) values ("channel", "package", "bundle")`, + `insert into operatorbundle (name, bundle) values ("bundle-a", "{}")`, + `insert into channel_entry (package_name, channel_name, operatorbundle_name, entry_id, depth) values ("package", "channel", "bundle-a", 1, 0)`, + } { + if _, err := db.Exec(stmt); err != nil { + t.Fatalf("unexpected error executing setup statements: %v", err) + } + } + + }, + Expect: func(t *testing.T, rows *sql.Rows) { + require := require.New(t) + require.True(rows.Next()) + var ( + c interface{} + bundle sql.NullString + ) + require.NoError(rows.Scan(&c, &bundle, &c, &c, &c, &c, &c, &c, &c, &c, &c, &c)) + require.Equal(sql.NullString{Valid: true, String: "{}"}, bundle) + require.False(rows.Next()) + }, + }, + { + Name: "manifests not omitted with bundlepath", + OmitManfests: true, + Setup: func(t *testing.T, db *sql.DB) { + for _, stmt := range []string{ + `insert into package (name, default_channel) values ("package", "channel")`, + `insert into channel (name, package_name, head_operatorbundle_name) values ("channel", "package", "bundle")`, + `insert into operatorbundle (name, bundle, bundlepath) values ("bundle-a", "{}", "path")`, + `insert into channel_entry (package_name, channel_name, operatorbundle_name, entry_id, depth) values ("package", "channel", "bundle-a", 1, 0)`, + } { + if _, err := db.Exec(stmt); err != nil { + t.Fatalf("unexpected error executing setup statements: %v", err) + } + } + + }, + Expect: func(t *testing.T, rows *sql.Rows) { + require := require.New(t) + require.True(rows.Next()) + var ( + c interface{} + bundle sql.NullString + ) + require.NoError(rows.Scan(&c, &bundle, &c, &c, &c, &c, &c, &c, &c, &c, &c, &c)) + require.Equal(sql.NullString{Valid: false, String: ""}, bundle) + require.False(rows.Next()) + }, + }, } { t.Run(tt.Name, func(t *testing.T) { ctx := context.Background() @@ -143,7 +200,7 @@ func TestListBundlesQuery(t *testing.T) { _, err = db.Exec("PRAGMA foreign_keys = ON") require.NoError(t, err) - rows, err := db.QueryContext(ctx, listBundlesQuery) + rows, err := db.QueryContext(ctx, listBundlesQuery, sql.Named("omit_manifests", tt.OmitManfests)) if err != nil { t.Fatalf("unexpected error executing list bundles query: %v", err) } diff --git a/vendor/github.com/operator-framework/operator-registry/cmd/opm/registry/serve.go b/vendor/github.com/operator-framework/operator-registry/cmd/opm/registry/serve.go index 74957a2c41..d8281e0cc2 100644 --- a/vendor/github.com/operator-framework/operator-registry/cmd/opm/registry/serve.go +++ b/vendor/github.com/operator-framework/operator-registry/cmd/opm/registry/serve.go @@ -95,7 +95,7 @@ func serveFunc(cmd *cobra.Command, args []string) error { logger.WithError(err).Warnf("couldn't migrate db") } - store := sqlite.NewSQLLiteQuerierFromDb(db) + store := sqlite.NewSQLLiteQuerierFromDb(db, sqlite.OmitManifests(true)) // sanity check that the db is available tables, err := store.ListTables(context.TODO()) diff --git a/vendor/github.com/operator-framework/operator-registry/cmd/registry-server/main.go b/vendor/github.com/operator-framework/operator-registry/cmd/registry-server/main.go index 9cd9e63ff3..67a6bd4e95 100644 --- a/vendor/github.com/operator-framework/operator-registry/cmd/registry-server/main.go +++ b/vendor/github.com/operator-framework/operator-registry/cmd/registry-server/main.go @@ -96,7 +96,7 @@ func runCmdFunc(cmd *cobra.Command, args []string) error { logger.WithError(err).Warnf("couldn't migrate db") } - store := sqlite.NewSQLLiteQuerierFromDb(db) + store := sqlite.NewSQLLiteQuerierFromDb(db, sqlite.OmitManifests(true)) // sanity check that the db is available tables, err := store.ListTables(context.TODO()) diff --git a/vendor/github.com/operator-framework/operator-registry/pkg/registry/query.go b/vendor/github.com/operator-framework/operator-registry/pkg/registry/query.go index 6292525488..20f0e9f95d 100644 --- a/vendor/github.com/operator-framework/operator-registry/pkg/registry/query.go +++ b/vendor/github.com/operator-framework/operator-registry/pkg/registry/query.go @@ -56,6 +56,14 @@ func (q Querier) SendBundles(_ context.Context, s BundleSender) error { if err != nil { return fmt.Errorf("convert bundle %q: %v", b.Name, err) } + if apiBundle.BundlePath != "" { + // The SQLite-based server + // configures its querier to + // omit these fields when + // bundle path is set. + apiBundle.CsvJson = "" + apiBundle.Object = nil + } if err := s.Send(apiBundle); err != nil { return err } diff --git a/vendor/github.com/operator-framework/operator-registry/pkg/sqlite/query.go b/vendor/github.com/operator-framework/operator-registry/pkg/sqlite/query.go index 0b8153d32f..fb24204941 100644 --- a/vendor/github.com/operator-framework/operator-registry/pkg/sqlite/query.go +++ b/vendor/github.com/operator-framework/operator-registry/pkg/sqlite/query.go @@ -35,25 +35,44 @@ func (a dbQuerierAdapter) QueryContext(ctx context.Context, query string, args . type SQLQuerier struct { db Querier + querierConfig } var _ registry.Query = &SQLQuerier{} -func NewSQLLiteQuerier(dbFilename string) (*SQLQuerier, error) { +type querierConfig struct { + omitManifests bool +} + +type SQLiteQuerierOption func(*querierConfig) + +// If true, ListBundles will omit inline manifests (the object and +// csvJson fields) from response elements that contain a bundle image +// reference. +func OmitManifests(b bool) SQLiteQuerierOption { + return func(c *querierConfig) { + c.omitManifests = b + } +} + +func NewSQLLiteQuerier(dbFilename string, opts ...SQLiteQuerierOption) (*SQLQuerier, error) { db, err := OpenReadOnly(dbFilename) if err != nil { return nil, err } - - return &SQLQuerier{dbQuerierAdapter{db}}, nil + return NewSQLLiteQuerierFromDb(db, opts...), nil } -func NewSQLLiteQuerierFromDb(db *sql.DB) *SQLQuerier { - return &SQLQuerier{dbQuerierAdapter{db}} +func NewSQLLiteQuerierFromDb(db *sql.DB, opts ...SQLiteQuerierOption) *SQLQuerier { + return NewSQLLiteQuerierFromDBQuerier(dbQuerierAdapter{db}, opts...) } -func NewSQLLiteQuerierFromDBQuerier(q Querier) *SQLQuerier { - return &SQLQuerier{q} +func NewSQLLiteQuerierFromDBQuerier(q Querier, opts ...SQLiteQuerierOption) *SQLQuerier { + sq := SQLQuerier{db: q} + for _, opt := range opts { + opt(&sq.querierConfig) + } + return &sq } func (s *SQLQuerier) ListTables(ctx context.Context) ([]string, error) { @@ -983,7 +1002,7 @@ merged_dependencies (bundle_name, merged) AS ( ) SELECT replaces_bundle.entry_id, - operatorbundle.bundle, + CASE WHEN :omit_manifests AND length(coalesce(operatorbundle.bundlepath, "")) > 0 THEN NULL ELSE operatorbundle.bundle END, operatorbundle.bundlepath, operatorbundle.name, replaces_bundle.package_name, @@ -1007,7 +1026,7 @@ SELECT ON operatorbundle.name = merged_properties.bundle_name` func (s *SQLQuerier) SendBundles(ctx context.Context, stream registry.BundleSender) error { - rows, err := s.db.QueryContext(ctx, listBundlesQuery) + rows, err := s.db.QueryContext(ctx, listBundlesQuery, sql.Named("omit_manifests", s.omitManifests)) if err != nil { return err } From d4bc691a289ae689d0f93e7c48ea48bb8f05e85d Mon Sep 17 00:00:00 2001 From: Ben Luddy Date: Thu, 29 Jul 2021 15:58:20 -0400 Subject: [PATCH 5/5] Set soft_heap_limit for SQLite-based registry servers. (#728) Registry pods receive low request volumes and are not latency-sensitive, so there's little downside to advising SQLite to keep its page cache as small as possible. Signed-off-by: Ben Luddy Upstream-repository: operator-registry Upstream-commit: 5fc0f42215d752a43e1fb052a71d37b36b4c57e8 --- staging/operator-registry/cmd/opm/registry/serve.go | 4 ++++ staging/operator-registry/cmd/registry-server/main.go | 4 ++++ .../operator-registry/cmd/opm/registry/serve.go | 4 ++++ .../operator-registry/cmd/registry-server/main.go | 4 ++++ 4 files changed, 16 insertions(+) diff --git a/staging/operator-registry/cmd/opm/registry/serve.go b/staging/operator-registry/cmd/opm/registry/serve.go index d8281e0cc2..ff402c58b3 100644 --- a/staging/operator-registry/cmd/opm/registry/serve.go +++ b/staging/operator-registry/cmd/opm/registry/serve.go @@ -90,6 +90,10 @@ func serveFunc(cmd *cobra.Command, args []string) error { return err } + if _, err := db.ExecContext(context.TODO(), `PRAGMA soft_heap_limit=1`); err != nil { + logger.WithError(err).Warnf("error setting soft heap limit for sqlite") + } + // migrate to the latest version if err := migrate(cmd, db); err != nil { logger.WithError(err).Warnf("couldn't migrate db") diff --git a/staging/operator-registry/cmd/registry-server/main.go b/staging/operator-registry/cmd/registry-server/main.go index 67a6bd4e95..529a02a9cb 100644 --- a/staging/operator-registry/cmd/registry-server/main.go +++ b/staging/operator-registry/cmd/registry-server/main.go @@ -91,6 +91,10 @@ func runCmdFunc(cmd *cobra.Command, args []string) error { return err } + if _, err := db.ExecContext(context.TODO(), `PRAGMA soft_heap_limit=1`); err != nil { + logger.WithError(err).Warnf("error setting soft heap limit for sqlite") + } + // migrate to the latest version if err := migrate(cmd, db); err != nil { logger.WithError(err).Warnf("couldn't migrate db") diff --git a/vendor/github.com/operator-framework/operator-registry/cmd/opm/registry/serve.go b/vendor/github.com/operator-framework/operator-registry/cmd/opm/registry/serve.go index d8281e0cc2..ff402c58b3 100644 --- a/vendor/github.com/operator-framework/operator-registry/cmd/opm/registry/serve.go +++ b/vendor/github.com/operator-framework/operator-registry/cmd/opm/registry/serve.go @@ -90,6 +90,10 @@ func serveFunc(cmd *cobra.Command, args []string) error { return err } + if _, err := db.ExecContext(context.TODO(), `PRAGMA soft_heap_limit=1`); err != nil { + logger.WithError(err).Warnf("error setting soft heap limit for sqlite") + } + // migrate to the latest version if err := migrate(cmd, db); err != nil { logger.WithError(err).Warnf("couldn't migrate db") diff --git a/vendor/github.com/operator-framework/operator-registry/cmd/registry-server/main.go b/vendor/github.com/operator-framework/operator-registry/cmd/registry-server/main.go index 67a6bd4e95..529a02a9cb 100644 --- a/vendor/github.com/operator-framework/operator-registry/cmd/registry-server/main.go +++ b/vendor/github.com/operator-framework/operator-registry/cmd/registry-server/main.go @@ -91,6 +91,10 @@ func runCmdFunc(cmd *cobra.Command, args []string) error { return err } + if _, err := db.ExecContext(context.TODO(), `PRAGMA soft_heap_limit=1`); err != nil { + logger.WithError(err).Warnf("error setting soft heap limit for sqlite") + } + // migrate to the latest version if err := migrate(cmd, db); err != nil { logger.WithError(err).Warnf("couldn't migrate db")