Switch to catalogd's CatalogMetadata APIs
#343
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,6 +8,7 @@ | |
| catalogd "github.com/operator-framework/catalogd/api/core/v1alpha1" | ||
| "github.com/operator-framework/deppy/pkg/deppy" | ||
| "github.com/operator-framework/deppy/pkg/deppy/input" | ||
| "github.com/operator-framework/operator-registry/alpha/declcfg" | ||
| "github.com/operator-framework/operator-registry/alpha/property" | ||
| "sigs.k8s.io/controller-runtime/pkg/client" | ||
|
|
||
|
|
@@ -71,70 +72,119 @@ | |
| return nil | ||
| } | ||
|
|
||
| func getEntities(ctx context.Context, cl client.Client) (input.EntityList, error) { | ||
| allEntitiesList := input.EntityList{} | ||
|
|
||
| var catalogList catalogd.CatalogList | ||
| if err := cl.List(ctx, &catalogList); err != nil { | ||
| return nil, err | ||
| } | ||
| for _, catalog := range catalogList.Items { | ||
| channels, bundles, err := fetchCatalogMetadata(ctx, cl, catalog.Name) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| catalogEntitiesList, err := MetadataToEntities(catalog.Name, channels, bundles) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| allEntitiesList = append(allEntitiesList, catalogEntitiesList...) | ||
| } | ||
|
|
||
| return allEntitiesList, nil | ||
| } | ||
|
|
||
| func MetadataToEntities(catalogName string, channels []declcfg.Channel, bundles []declcfg.Bundle) (input.EntityList, error) { | ||
|
There was a problem hiding this comment. As I'm reviewing this function I have some concerns about it's performance. How frequently are we retrieving and transforming this data? If this is an issue, we can address it in a follow up. There was a problem hiding this comment. Could you please explain your concerns? I'm concerned about I think as part of #347 we should think about some cache which will be valid for the duration of one resolution. In the CLI we cache entites into the enttiy source property: operator-controller/cmd/resolutioncli/entity_source.go Lines 92 to 109 in b568602 In operator controller we can not do the same since entity source is resused between reconciliations. There was a problem hiding this comment. Had a chat with Alex on this. The consern is not about We came to an agreement that this is something which needs to be addressed, but not as part as this PR since this is problem with an entity source in general, not with the way we consume catalog medatada. Moreover entity sources will be removed soon as part of #347. @awgreene please correct me if I captured it incorrectly. There was a problem hiding this comment.
Is that true? I thought we had informer caches setup for the catalogd data such that There was a problem hiding this comment. Ah, you might be right! I totally forgot about informers. |
||
| entityList := input.EntityList{} | ||
|
|
||
| bundlesMap := map[string]*declcfg.Bundle{} | ||
| for i := range bundles { | ||
| bundleKey := fmt.Sprintf("%s-%s", bundles[i].Package, bundles[i].Name) | ||
| bundlesMap[bundleKey] = &bundles[i] | ||
| } | ||
|
|
||
| for _, ch := range channels { | ||
| for _, chEntry := range ch.Entries { | ||
| bundleKey := fmt.Sprintf("%s-%s", ch.Package, chEntry.Name) | ||
| bundle, ok := bundlesMap[bundleKey] | ||
| if !ok { | ||
| return nil, fmt.Errorf("bundle %q not found in catalog %q (package %q, channel %q)", chEntry.Name, catalogName, ch.Package, ch.Name) | ||
| } | ||
|
|
||
| props := map[string]string{} | ||
|
|
||
| for _, prop := range bundle.Properties { | ||
| switch prop.Type { | ||
| case property.TypePackage: | ||
| // this is already a json marshalled object, so it doesn't need to be marshalled | ||
| // like the other ones | ||
| props[property.TypePackage] = string(prop.Value) | ||
| case entities.PropertyBundleMediaType: | ||
| props[entities.PropertyBundleMediaType] = string(prop.Value) | ||
| } | ||
| } | ||
|
|
||
| imgValue, err := json.Marshal(bundle.Image) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| props[entities.PropertyBundlePath] = string(imgValue) | ||
|
|
||
| channelValue, _ := json.Marshal(property.Channel{ChannelName: ch.Name, Priority: 0}) | ||
| props[property.TypeChannel] = string(channelValue) | ||
| replacesValue, _ := json.Marshal(entities.ChannelEntry{ | ||
| Name: bundle.Name, | ||
| Replaces: chEntry.Replaces, | ||
| }) | ||
| props[entities.PropertyBundleChannelEntry] = string(replacesValue) | ||
|
|
||
| catalogScopedEntryName := fmt.Sprintf("%s-%s", catalogName, bundle.Name) | ||
| entity := input.Entity{ | ||
| ID: deppy.IdentifierFromString(fmt.Sprintf("%s%s%s", catalogScopedEntryName, bundle.Package, ch.Name)), | ||
| Properties: props, | ||
| } | ||
everettraven marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| entityList = append(entityList, entity) | ||
| } | ||
| } | ||
|
|
||
| return entityList, nil | ||
| } | ||
|
|
||
| func fetchCatalogMetadata(ctx context.Context, cl client.Client, catalogName string) ([]declcfg.Channel, []declcfg.Bundle, error) { | ||
| channels, err := fetchCatalogMetadataByScheme[declcfg.Channel](ctx, cl, declcfg.SchemaChannel, catalogName) | ||
| if err != nil { | ||
| return nil, nil, err | ||
| } | ||
| bundles, err := fetchCatalogMetadataByScheme[declcfg.Bundle](ctx, cl, declcfg.SchemaBundle, catalogName) | ||
| if err != nil { | ||
| return nil, nil, err | ||
everettraven marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| return channels, bundles, nil | ||
| } | ||
|
|
||
| type declcfgSchema interface { | ||
| declcfg.Package | declcfg.Bundle | declcfg.Channel | ||
| } | ||
|
|
||
| // TODO: Cleanup once https://github.com/golang/go/issues/45380 implemented | ||
| // We should be able to get rid of the schema arg and switch based on the type passed to this generic | ||
| func fetchCatalogMetadataByScheme[T declcfgSchema](ctx context.Context, cl client.Client, schema, catalogName string) ([]T, error) { | ||
| cmList := catalogd.CatalogMetadataList{} | ||
| if err := cl.List(ctx, &cmList, client.MatchingLabels{"schema": schema, "catalog": catalogName}); err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| contents := []T{} | ||
| for _, cm := range cmList.Items { | ||
| var content T | ||
| if err := json.Unmarshal(cm.Spec.Content, &content); err != nil { | ||
| return nil, err | ||
| } | ||
| contents = append(contents, content) | ||
| } | ||
|
|
||
| return contents, nil | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to fetch and loop through all the catalogs here? If we want to ensure we fetch all
CatalogMetadatafor allCatalogresources I think we can simplify this to listing allCatalogMetadataresources and then processing each one.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My idea is that we process one catalog at a time and let garbage collection do its thing while we move onto the next catalog.
But yes, we definitevely can fetch all the data from all catalogs and loop trough it, but the data won't be garbage collectable until after we process whole slice. So we need to find a balance between 1) how much we want to hold in memory and 2) how many network calls we make.
Notice: I haven't tested/measured memory consumption with multipe catalogs. So it is based on gut feeling and maybe it is not very beneficial from memory consumption point of view.
But another benefit of doing one catalog at a time is that it avoids doing more mapping between catalogs, channels and bundles.