[feature] Understanding packages from bundle directory #241
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| package bundle | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "path" | ||
| "sort" | ||
|
|
||
| "github.com/operator-framework/operator-registry/pkg/registry" | ||
| ) | ||
|
|
||
| type bundleDirInterpreter struct { | ||
| bundleCsvName string | ||
| pkg *registry.Package | ||
| } | ||
|
|
||
| func NewBundleDirInterperter(bundleDir string) (*bundleDirInterpreter, error) { | ||
| csv, err := registry.ReadCSVFromBundleDirectory(bundleDir) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("error loading CSV from bundle directory, %v", err) | ||
| } | ||
|
|
||
| pkgDir, err := registry.NewPackageGraphLoaderFromDir(path.Join(bundleDir, "..")) | ||
|
There was a problem hiding this comment. I'm not sure this is what we want to do here. This reads in the directory of files and generates a graph, and tells you if there are problems. But the generate command is used for a single bundle. That single bundle may be applied to any target index, and has nothing to do with the bundles that happen to be in in the filesystem next to it. I think this would be fine without the graph loader in this PR at all. Thoughts? There was a problem hiding this comment. But I thought the whole purpose of this though is to automagically decide what channels a given bundle is. How can we do that without attempting to build the graph for a given package? There was a problem hiding this comment. Thanks for the explanation - commented before ☕️ ! I think there's a slight impedance mismatch here - this option is now only really useful if you're attempting convert an entire directory of the old format, right? Should this really be part of the normal command for generating/buliding single bundles when it's so tied to the set? There was a problem hiding this comment. I think the trigger for it should be "if you didn't pass in these parameters explicitly, let's go see if we can find them for you" rather than it being an explicit opt in/opt out feature. IMO that's a better experience There was a problem hiding this comment. I second that this should be an experience enhancement type of feature based on the fact that you have the full-blown format with bundles nested in a package along with other bundles. Without it, it does not have any information we would be looking for. The After all, this feature is just auto-filling |
||
| if err != nil { | ||
| return nil, fmt.Errorf("error loading package from directory, %v", err) | ||
| } | ||
|
|
||
| p, err := pkgDir.Generate() | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| return &bundleDirInterpreter{bundleCsvName: csv.GetName(), pkg: p}, nil | ||
| } | ||
|
|
||
| func (b *bundleDirInterpreter) GetBundleChannels() (channelNames []string) { | ||
| for channelName, channel := range b.pkg.Channels { | ||
| for bundle, _ := range channel.Nodes { | ||
| if bundle.CsvName == b.bundleCsvName { | ||
| channelNames = append(channelNames, channelName) | ||
| break | ||
| } | ||
| } | ||
| } | ||
| sort.Strings(channelNames) | ||
| return | ||
| } | ||
|
|
||
| func (b *bundleDirInterpreter) GetDefaultChannel() string { | ||
| return b.pkg.DefaultChannel | ||
| } | ||
|
|
||
| func (b *bundleDirInterpreter) GetPackageName() string { | ||
| return b.pkg.Name | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,68 @@ | ||
| package bundle | ||
|
|
||
| import ( | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/assert" | ||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| func TestBundleDirectoryInterpreter(t *testing.T) { | ||
| tests := []struct { | ||
| dir string | ||
| fail bool | ||
| bundleChannels []string | ||
| defaultChannel string | ||
| PackageName string | ||
| }{ | ||
| { | ||
| dir: "../../registry/testdata/validPackages/etcd/0.6.1", | ||
| fail: false, | ||
| bundleChannels: []string{"alpha", "beta", "stable"}, | ||
| defaultChannel: "alpha", | ||
| PackageName: "etcd", | ||
| }, | ||
| { | ||
| dir: "../../registry/testdata/validPackages/etcd/0.9.0", | ||
| fail: false, | ||
| bundleChannels: []string{"alpha", "beta", "stable"}, | ||
| defaultChannel: "alpha", | ||
| PackageName: "etcd", | ||
| }, | ||
| { | ||
| dir: "../../registry/testdata/validPackages/etcd/0.9.2", | ||
| fail: false, | ||
| bundleChannels: []string{"alpha", "stable"}, | ||
| defaultChannel: "alpha", | ||
| PackageName: "etcd", | ||
| }, | ||
| { | ||
| dir: "../../registry/testdata/validPackages/prometheus/0.14.0", | ||
| fail: false, | ||
| bundleChannels: []string{"preview"}, | ||
| defaultChannel: "preview", | ||
| PackageName: "prometheus", | ||
| }, | ||
| { | ||
| dir: "../../registry/testdata/invalidPackges/3scale-community-operator/0.3.0", | ||
| fail: true, | ||
| }, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run("Loading Package Graph from "+tt.dir, func(t *testing.T) { | ||
| bundle, err := NewBundleDirInterperter(tt.dir) | ||
| if tt.fail { | ||
| assert.Error(t, err) | ||
| return | ||
| } | ||
| require.NoError(t, err) | ||
|
|
||
| assert.EqualValues(t, tt.bundleChannels, bundle.GetBundleChannels()) | ||
|
|
||
| assert.EqualValues(t, tt.defaultChannel, bundle.GetDefaultChannel()) | ||
|
|
||
| assert.EqualValues(t, tt.PackageName, bundle.GetPackageName()) | ||
| }) | ||
| } | ||
| } |
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.
log/notification added at the info level to suggestion inference took place.