Resolve only default channels

Non deterministic behavior was introduced where, in some cases, the
dependency resolver would select a channel entry from the registry
that was not part of the default channel and create a subscription
on that channel. This bugfix updates the registry client's filter
function to ignore non default channel entries.

Author: kevinrizza

pkg/controller/registry/registry_client.go

@@ -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 {
@@ -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)
+				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})
+	if err != nil {
+		return "", err
+	}
+	if pkg == nil {
+		return "", fmt.Errorf("package %s not found in registry", pkgName)
+	}
+
+	return pkg.DefaultChannelName, nil
+}

test/e2e/subscription_e2e_test.go

@@ -1497,6 +1497,8 @@ func TestCreateNewSubscriptionWithDependenciesSamePackage(t *testing.T) {
 	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
@@ -1512,6 +1514,7 @@ func TestCreateNewSubscriptionWithDependenciesSamePackage(t *testing.T) {
 		{
 			PackageName: packageName2,
 			Channels: []registry.PackageChannel{
+				{Name: alphaChannel, CurrentCSVName: csvD.GetName()},
 				{Name: stableChannel, CurrentCSVName: csvB.GetName()},
 			},
 			DefaultChannelName: stableChannel,
@@ -1531,7 +1534,7 @@ func TestCreateNewSubscriptionWithDependenciesSamePackage(t *testing.T) {
 	}
 
 	catalogSourceName := genName("catsrc")
-	catsrc, cleanup := createInternalCatalogSource(t, kubeClient, crClient, catalogSourceName, testNamespace, manifests, []apiextensions.CustomResourceDefinition{crd, crd2}, []v1alpha1.ClusterServiceVersion{csvA, csvABC, csvB})
+	catsrc, cleanup := createInternalCatalogSource(t, 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.
@@ -1577,6 +1580,9 @@ func TestCreateNewSubscriptionWithDependenciesSamePackage(t *testing.T) {
 	// Ensure csvABC is not installed
 	_, err = crClient.OperatorsV1alpha1().ClusterServiceVersions(testNamespace).Get(csvABC.Name, metav1.GetOptions{})
 	require.Error(t, err)
+	// Ensure csvD is not installed -- this implies the dependent subscription selected the default channel
+	_, err = crClient.OperatorsV1alpha1().ClusterServiceVersions(testNamespace).Get(csvD.Name, metav1.GetOptions{})
+	require.Error(t, err)
 }
 
 func checkDeploymentWithPodConfiguration(t *testing.T, client operatorclient.ClientInterface, csv *v1alpha1.ClusterServiceVersion, envVar []corev1.EnvVar, volumes []corev1.Volume, volumeMounts []corev1.VolumeMount) {