Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug 1847540: resolve only default channels #1583

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
52 changes: 40 additions & 12 deletions pkg/controller/registry/registry_client.go
Expand Up @@ -86,9 +86,9 @@ func (rc *Client) FindBundleThatProvides(ctx context.Context, group, version, ki
if err != nil {
return nil, err
}
entry := rc.filterChannelEntries(it, excludedPkgName)
entry := rc.filterChannelEntries(ctx, it, excludedPkgName)
if entry == nil {
return nil, fmt.Errorf("Unable to find a channel entry which doesn't belong to package %s", excludedPkgName)
return nil, fmt.Errorf("Unable to find a channel entry which doesn't belong to package %s's default channel", excludedPkgName)
}
bundle, err := rc.Client.Registry.GetBundle(ctx, &registryapi.GetBundleRequest{PkgName: entry.PackageName, ChannelName: entry.ChannelName, CsvName: entry.BundleName})
if err != nil {
Expand All @@ -97,25 +97,53 @@ func (rc *Client) FindBundleThatProvides(ctx context.Context, group, version, ki
return bundle, nil
}

// FilterChannelEntries filters out a channel entries that provide the requested
// FilterChannelEntries filters out channel entries that provide the requested
// API and come from the same package with original operator and returns the
// first entry on the list
func (rc *Client) filterChannelEntries(it *ChannelEntryIterator, excludedPkgName string) *opregistry.ChannelEntry {
// first entry on the list from the default channel of that package
func (rc *Client) filterChannelEntries(ctx context.Context, it *ChannelEntryIterator, excludedPkgName string) *opregistry.ChannelEntry {
defChannels := make(map[string]string, 0)

var entries []*opregistry.ChannelEntry
for e := it.Next(); e != nil; e = it.Next() {
if e.PackageName != excludedPkgName {
entry := &opregistry.ChannelEntry{
PackageName: e.PackageName,
ChannelName: e.ChannelName,
BundleName: e.BundleName,
Replaces: e.Replaces,
// keep track of the default channel for each package
if _, ok := defChannels[e.PackageName]; !ok {
defChannel, err := rc.getDefaultPackageChannel(ctx, e.PackageName)
ecordell marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
continue
}
defChannels[e.PackageName] = defChannel
}

// only add entry to the list if the entry is in the default channel
if e.ChannelName == defChannels[e.PackageName] {
entry := &opregistry.ChannelEntry{
PackageName: e.PackageName,
ChannelName: e.ChannelName,
BundleName: e.BundleName,
Replaces: e.Replaces,
}
entries = append(entries, entry)
}
entries = append(entries, entry)
}
}

if entries != nil {
if len(entries) > 0 {
return entries[0]
}
return nil
}

// GetDefaultPackageChannel uses registry client to get the default
// channel name for a given package name
func (rc *Client) getDefaultPackageChannel(ctx context.Context, pkgName string) (string, error) {
pkg, err := rc.Client.Registry.GetPackage(ctx, &registryapi.GetPackageRequest{Name: pkgName})
dinhxuanvu marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return "", err
}
if pkg == nil {
return "", fmt.Errorf("package %s not found in registry", pkgName)
}

return pkg.DefaultChannelName, nil
}
8 changes: 7 additions & 1 deletion test/e2e/subscription_e2e_test.go
Expand Up @@ -1349,6 +1349,8 @@ var _ = Describe("Subscription", func() {
csvB := newCSV("nginx-b-dep", testNamespace, "", semver.MustParse("0.1.0"), []apiextensions.CustomResourceDefinition{crd}, nil, depNamedStrategy)
// csvC provides CRD2 in the different catalogsource with csvA (apackage)
csvC := newCSV("nginx-c-dep", testNamespace, "", semver.MustParse("0.1.0"), []apiextensions.CustomResourceDefinition{crd2}, nil, depNamedStrategy2)
// csvD provides CRD1 in the same catalogsource with csvA (apackage)
csvD := newCSV("nginx-d-dep", testNamespace, "", semver.MustParse("0.1.0"), []apiextensions.CustomResourceDefinition{crd}, nil, depNamedStrategy)

// Create PackageManifests 1
// Contain csvA, ABC and B
Expand All @@ -1364,6 +1366,7 @@ var _ = Describe("Subscription", func() {
{
PackageName: packageName2,
Channels: []registry.PackageChannel{
{Name: alphaChannel, CurrentCSVName: csvD.GetName()},
{Name: stableChannel, CurrentCSVName: csvB.GetName()},
},
DefaultChannelName: stableChannel,
Expand All @@ -1383,7 +1386,7 @@ var _ = Describe("Subscription", func() {
}

catalogSourceName := genName("catsrc")
catsrc, cleanup := createInternalCatalogSource(kubeClient, crClient, catalogSourceName, testNamespace, manifests, []apiextensions.CustomResourceDefinition{crd, crd2}, []v1alpha1.ClusterServiceVersion{csvA, csvABC, csvB})
catsrc, cleanup := createInternalCatalogSource(kubeClient, crClient, catalogSourceName, testNamespace, manifests, []apiextensions.CustomResourceDefinition{crd, crd2}, []v1alpha1.ClusterServiceVersion{csvA, csvABC, csvB, csvD})
defer cleanup()

// Ensure that the catalog source is resolved before we create a subscription.
Expand Down Expand Up @@ -1429,6 +1432,9 @@ var _ = Describe("Subscription", func() {
// Ensure csvABC is not installed
_, err = crClient.OperatorsV1alpha1().ClusterServiceVersions(testNamespace).Get(context.TODO(), csvABC.Name, metav1.GetOptions{})
require.Error(GinkgoT(), err)
// Ensure csvD is not installed -- this implies the dependent subscription selected the default channel
_, err = crClient.OperatorsV1alpha1().ClusterServiceVersions(testNamespace).Get(context.TODO(), csvD.Name, metav1.GetOptions{})
require.Error(GinkgoT(), err)
})
})

Expand Down