Skip to content

Commit fef533c

Browse files
openshift-merge-robotopenshift-merge-robot
authored
Merge pull request #1080 from awgreene/initialSource
Bug 1762769: Prioritize APIs from same CatSrc
2 parents 36db200 + b49bee6 commit fef533c

File tree

5 files changed

+179
-7
lines changed

5 files changed

+179
-7
lines changed

‎pkg/controller/registry/resolver/evolver.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,8 +112,15 @@ func (e *NamespaceGenerationEvolver) queryForRequiredAPIs() error {
112112
}
113113
e.gen.MarkAPIChecked(*api)
114114

115+
// identify the initialSource
116+
initialSource := CatalogKey{}
117+
for _, operator := range e.gen.MissingAPIs()[*api] {
118+
initialSource = operator.SourceInfo().Catalog
119+
break
120+
}
121+
115122
// attempt to find a bundle that provides that api
116-
if bundle, key, err := e.querier.FindProvider(*api); err == nil {
123+
if bundle, key, err := e.querier.FindProvider(*api, initialSource); err == nil {
117124
// add a bundle that provides the api to the generation
118125
o, err := NewOperatorFromBundle(bundle, "", "", *key)
119126
if err != nil {

‎pkg/controller/registry/resolver/querier.go

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ type SourceRef struct {
2222
}
2323

2424
type SourceQuerier interface {
25-
FindProvider(api opregistry.APIKey) (*opregistry.Bundle, *CatalogKey, error)
25+
FindProvider(api opregistry.APIKey, initialSource CatalogKey) (*opregistry.Bundle, *CatalogKey, error)
2626
FindBundle(pkgName, channelName, bundleName string, initialSource CatalogKey) (*opregistry.Bundle, *CatalogKey, error)
2727
FindLatestBundle(pkgName, channelName string, initialSource CatalogKey) (*opregistry.Bundle, *CatalogKey, error)
2828
FindReplacement(currentVersion *semver.Version, bundleName, pkgName, channelName string, initialSource CatalogKey) (*opregistry.Bundle, *CatalogKey, error)
@@ -48,7 +48,18 @@ func (q *NamespaceSourceQuerier) Queryable() error {
4848
return nil
4949
}
5050

51-
func (q *NamespaceSourceQuerier) FindProvider(api opregistry.APIKey) (*opregistry.Bundle, *CatalogKey, error) {
51+
func (q *NamespaceSourceQuerier) FindProvider(api opregistry.APIKey, initialSource CatalogKey) (*opregistry.Bundle, *CatalogKey, error) {
52+
if initialSource.Name != "" && initialSource.Namespace != "" {
53+
source, ok := q.sources[initialSource]
54+
if ok {
55+
if bundle, err := source.GetBundleThatProvides(context.TODO(), api.Group, api.Version, api.Kind); err == nil {
56+
return bundle, &initialSource, nil
57+
}
58+
if bundle, err := source.GetBundleThatProvides(context.TODO(), api.Plural+"."+api.Group, api.Version, api.Kind); err == nil {
59+
return bundle, &initialSource, nil
60+
}
61+
}
62+
}
5263
for key, source := range q.sources {
5364
if bundle, err := source.GetBundleThatProvides(context.TODO(), api.Group, api.Version, api.Kind); err == nil {
5465
return bundle, &key, nil

‎pkg/controller/registry/resolver/querier_test.go

Lines changed: 47 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -106,20 +106,33 @@ func TestNamespaceSourceQuerier_Queryable(t *testing.T) {
106106

107107
func TestNamespaceSourceQuerier_FindProvider(t *testing.T) {
108108
fakeSource := fakes.FakeInterface{}
109+
fakeSource2 := fakes.FakeInterface{}
109110
sources := map[CatalogKey]client.Interface{
110111
CatalogKey{"test", "ns"}: &fakeSource,
112+
CatalogKey{"test2", "ns"}: &fakeSource2,
111113
}
112114

113115
bundle := opregistry.NewBundle("test", "testPkg", "testChannel")
116+
bundle2 := opregistry.NewBundle("test2", "testPkg2", "testChannel2")
114117
fakeSource.GetBundleThatProvidesStub = func(ctx context.Context, group, version, kind string) (*opregistry.Bundle, error) {
118+
if group != "group" || version != "version" || kind != "kind" {
119+
return nil, fmt.Errorf("Not Found")
120+
}
115121
return bundle, nil
116122
}
123+
fakeSource2.GetBundleThatProvidesStub = func(ctx context.Context, group, version, kind string) (*opregistry.Bundle, error) {
124+
if group != "group2" || version != "version2" || kind != "kind2" {
125+
return nil, fmt.Errorf("Not Found")
126+
}
127+
return bundle2, nil
128+
}
117129

118130
type fields struct {
119131
sources map[CatalogKey]client.Interface
120132
}
121133
type args struct {
122134
api opregistry.APIKey
135+
catalogKey CatalogKey
123136
}
124137
type out struct {
125138
bundle *opregistry.Bundle
@@ -138,6 +151,7 @@ func TestNamespaceSourceQuerier_FindProvider(t *testing.T) {
138151
},
139152
args: args{
140153
api: opregistry.APIKey{"group", "version", "kind", "plural"},
154+
catalogKey: CatalogKey{},
141155
},
142156
out: out{
143157
bundle: bundle,
@@ -151,23 +165,52 @@ func TestNamespaceSourceQuerier_FindProvider(t *testing.T) {
151165
},
152166
args: args{
153167
api: opregistry.APIKey{"group", "version", "kind", "plural"},
168+
catalogKey: CatalogKey{},
154169
},
155170
out: out{
156171
bundle: nil,
157172
key: nil,
158173
err: fmt.Errorf("group/version/kind (plural) not provided by a package in any CatalogSource"),
159174
},
160175
},
176+
{
177+
fields: fields{
178+
sources: sources,
179+
},
180+
args: args{
181+
api: opregistry.APIKey{"group2", "version2", "kind2", "plural2"},
182+
catalogKey: CatalogKey{Name: "test2", Namespace: "ns"},
183+
},
184+
out: out{
185+
bundle: bundle2,
186+
key: &CatalogKey{Name: "test2", Namespace: "ns"},
187+
err: nil,
188+
},
189+
},
190+
{
191+
fields: fields{
192+
sources: sources,
193+
},
194+
args: args{
195+
api: opregistry.APIKey{"group2", "version2", "kind2", "plural2"},
196+
catalogKey: CatalogKey{Name: "test3", Namespace: "ns"},
197+
},
198+
out: out{
199+
bundle: bundle2,
200+
key: &CatalogKey{Name: "test2", Namespace: "ns"},
201+
err: nil,
202+
},
203+
},
161204
}
162205
for _, tt := range tests {
163206
t.Run(tt.name, func(t *testing.T) {
164207
q := &NamespaceSourceQuerier{
165208
sources: tt.fields.sources,
166209
}
167-
bundle, key, err := q.FindProvider(tt.args.api)
168-
require.Equal(t, err, tt.out.err)
169-
require.Equal(t, bundle, tt.out.bundle)
170-
require.Equal(t, key, tt.out.key)
210+
bundle, key, err := q.FindProvider(tt.args.api, tt.args.catalogKey)
211+
require.Equal(t, tt.out.err, err)
212+
require.Equal(t, tt.out.bundle, bundle)
213+
require.Equal(t, tt.out.key, key)
171214
})
172215
}
173216
}

‎test/e2e/subscription_e2e_test.go

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1362,6 +1362,48 @@ func TestCreateNewSubscriptionWithPodConfig(t *testing.T) {
13621362
checkDeploymentWithPodConfiguration(t, kubeClient, csv, podConfig.Env, podConfig.Volumes, podConfig.VolumeMounts)
13631363
}
13641364

1365+
1366+
func TestCreateNewSubscriptionWithDependencies(t *testing.T) {
1367+
defer cleaner.NotifyTestComplete(t, true)
1368+
1369+
kubeClient := newKubeClient(t)
1370+
crClient := newCRClient(t)
1371+
1372+
permissions := deploymentPermissions(t)
1373+
1374+
catsrc, subSpec, catsrcCleanup := newCatalogSourceWithDependencies(t, kubeClient, crClient, "podconfig", testNamespace, permissions)
1375+
defer catsrcCleanup()
1376+
1377+
// Ensure that the catalog source is resolved before we create a subscription.
1378+
_, err := fetchCatalogSource(t, crClient, catsrc.GetName(), testNamespace, catalogSourceRegistryPodSynced)
1379+
require.NoError(t, err)
1380+
1381+
// Create duplicates of the CatalogSource
1382+
for i := 0; i < 10; i++ {
1383+
duplicateCatsrc, _, duplicateCatSrcCleanup := newCatalogSourceWithDependencies(t, kubeClient, crClient, "podconfig", testNamespace, permissions)
1384+
defer duplicateCatSrcCleanup()
1385+
1386+
// Ensure that the catalog source is resolved before we create a subscription.
1387+
_, err = fetchCatalogSource(t, crClient, duplicateCatsrc.GetName(), testNamespace, catalogSourceRegistryPodSynced)
1388+
require.NoError(t, err)
1389+
}
1390+
1391+
// Create a subscription that has a dependency
1392+
subscriptionName := genName("podconfig-sub-")
1393+
cleanupSubscription := createSubscriptionForCatalogWithSpec(t, crClient, testNamespace, subscriptionName, subSpec)
1394+
defer cleanupSubscription()
1395+
1396+
subscription, err := fetchSubscription(t, crClient, testNamespace, subscriptionName, subscriptionStateAtLatestChecker)
1397+
require.NoError(t, err)
1398+
require.NotNil(t, subscription)
1399+
1400+
// Check that a single catalog source was used to resolve the InstallPlan
1401+
installPlan, err:= fetchInstallPlan(t, crClient, subscription.Status.InstallPlanRef.Name, buildInstallPlanPhaseCheckFunc(v1alpha1.InstallPlanPhaseComplete))
1402+
require.NoError(t, err)
1403+
require.Len(t, installPlan.Status.CatalogSources,1)
1404+
1405+
}
1406+
13651407
func checkDeploymentWithPodConfiguration(t *testing.T, client operatorclient.ClientInterface, csv *v1alpha1.ClusterServiceVersion, envVar []corev1.EnvVar, volumes []corev1.Volume, volumeMounts []corev1.VolumeMount) {
13661408
resolver := install.StrategyResolver{}
13671409

‎test/e2e/user_defined_sa_test.go

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,75 @@ func newCatalogSource(t *testing.T, kubeclient operatorclient.ClientInterface, c
310310
return
311311
}
312312

313+
314+
func newCatalogSourceWithDependencies(t *testing.T, kubeclient operatorclient.ClientInterface, crclient versioned.Interface, prefix, namespace string, permissions []install.StrategyDeploymentPermissions) (catsrc *v1alpha1.CatalogSource, subscriptionSpec *v1alpha1.SubscriptionSpec, cleanup cleanupFunc) {
315+
crdPlural := genName("ins")
316+
crdName := crdPlural + ".cluster.com"
317+
318+
crd := apiextensions.CustomResourceDefinition{
319+
ObjectMeta: metav1.ObjectMeta{
320+
Name: crdName,
321+
},
322+
Spec: apiextensions.CustomResourceDefinitionSpec{
323+
Group: "cluster.com",
324+
Version: "v1alpha1",
325+
Names: apiextensions.CustomResourceDefinitionNames{
326+
Plural: crdPlural,
327+
Singular: crdPlural,
328+
Kind: crdPlural,
329+
ListKind: "list" + crdPlural,
330+
},
331+
Scope: "Namespaced",
332+
},
333+
}
334+
335+
prefixFunc := func(s string) string {
336+
return fmt.Sprintf("%s-%s-", prefix, s)
337+
}
338+
339+
// Create CSV
340+
packageName1 := genName(prefixFunc("package"))
341+
packageName2 := genName(prefixFunc("package"))
342+
stableChannel := "stable"
343+
344+
namedStrategy := newNginxInstallStrategy(genName(prefixFunc("dep")), permissions, nil)
345+
csvA := newCSV("nginx-req-dep", namespace, "", semver.MustParse("0.1.0"), nil, []apiextensions.CustomResourceDefinition{crd}, namedStrategy)
346+
csvB := newCSV("nginx-dependency", namespace, "", semver.MustParse("0.1.0"), []apiextensions.CustomResourceDefinition{crd}, nil, namedStrategy)
347+
348+
// Create PackageManifests
349+
manifests := []registry.PackageManifest{
350+
{
351+
PackageName: packageName1,
352+
Channels: []registry.PackageChannel{
353+
{Name: stableChannel, CurrentCSVName: csvA.GetName()},
354+
},
355+
DefaultChannelName: stableChannel,
356+
},
357+
{
358+
PackageName: packageName2,
359+
Channels: []registry.PackageChannel{
360+
{Name: stableChannel, CurrentCSVName: csvB.GetName()},
361+
},
362+
DefaultChannelName: stableChannel,
363+
},
364+
}
365+
366+
catalogSourceName := genName(prefixFunc("catsrc"))
367+
catsrc, cleanup = createInternalCatalogSource(t, kubeclient, crclient, catalogSourceName, namespace, manifests, []apiextensions.CustomResourceDefinition{crd}, []v1alpha1.ClusterServiceVersion{csvA, csvB})
368+
require.NotNil(t, catsrc)
369+
require.NotNil(t, cleanup)
370+
371+
subscriptionSpec = &v1alpha1.SubscriptionSpec{
372+
CatalogSource: catsrc.GetName(),
373+
CatalogSourceNamespace: catsrc.GetNamespace(),
374+
Package: packageName1,
375+
Channel: stableChannel,
376+
StartingCSV: csvA.GetName(),
377+
InstallPlanApproval: v1alpha1.ApprovalAutomatic,
378+
}
379+
return
380+
}
381+
313382
func mustHaveCondition(t *testing.T, ip *v1alpha1.InstallPlan, conditionType v1alpha1.InstallPlanConditionType) (condition *v1alpha1.InstallPlanCondition) {
314383
for i := range ip.Status.Conditions {
315384
if ip.Status.Conditions[i].Type == conditionType {

0 commit comments

Comments
 (0)