Skip to content

Commit 890e13c

Browse files
openshift-merge-robotopenshift-merge-robot
authored
Merge pull request #1105 from ecordell/trust-grpc
Use the grpc response as the source of truth for update graph data
2 parents fb2879e + 2b343b5 commit 890e13c

30 files changed

+2009
-208
lines changed

‎go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ require (
2525
github.com/munnerz/goautoneg v0.0.0-20190414153302-2ae31c8b6b30 // indirect
2626
github.com/openshift/api v3.9.1-0.20190924102528-32369d4db2ad+incompatible
2727
github.com/openshift/client-go v0.0.0-20190923180330-3b6373338c9b
28-
github.com/operator-framework/operator-registry v1.5.1
28+
github.com/operator-framework/operator-registry v1.5.3
2929
github.com/pkg/errors v0.8.1
3030
github.com/prometheus/client_golang v0.9.3-0.20190127221311-3c4408c8b829
3131
github.com/sirupsen/logrus v1.4.2

‎go.sum

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -438,6 +438,8 @@ github.com/opentracing/opentracing-go v1.1.0/go.mod h1:UkNAQd3GIcIGf0SeVgPpRdFSt
438438
github.com/openzipkin/zipkin-go v0.1.6/go.mod h1:QgAqvLzwWbR/WpD4A3cGpPtJrZXNIiJc5AZX7/PBEpw=
439439
github.com/operator-framework/operator-registry v1.5.1 h1:8ruUOG6IBDVTAXYWKsv6hwr4yv/0SFPFPAYGCpcv97E=
440440
github.com/operator-framework/operator-registry v1.5.1/go.mod h1:agrQlkWOo1q8U1SAaLSS2WQ+Z9vswNT2M2HFib9iuLY=
441+
github.com/operator-framework/operator-registry v1.5.3 h1:az83WDwgB+tHsmVn+tFq72yQBbaUAye8e4+KkDQmzLs=
442+
github.com/operator-framework/operator-registry v1.5.3/go.mod h1:agrQlkWOo1q8U1SAaLSS2WQ+Z9vswNT2M2HFib9iuLY=
441443
github.com/otiai10/copy v1.0.1 h1:gtBjD8aq4nychvRZ2CyJvFWAw0aja+VHazDdruZKGZA=
442444
github.com/otiai10/copy v1.0.1/go.mod h1:8bMCJrAqOtN/d9oyh5HR7HhLQMvcGMpGdwRDYsfOCHc=
443445
github.com/otiai10/curr v0.0.0-20150429015615-9b4961190c95/go.mod h1:9qAhocn7zKJG+0mI8eUu6xqkFDYS2kb2saOteoSB3cE=

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,10 +63,11 @@ func (e *NamespaceGenerationEvolver) checkForUpdates() error {
6363
continue
6464
}
6565

66-
o, err := NewOperatorFromBundle(bundle, op.Identifier(), op.SourceInfo().StartingCSV, *key)
66+
o, err := NewOperatorFromBundle(bundle, op.SourceInfo().StartingCSV, *key)
6767
if err != nil {
6868
return errors.Wrap(err, "error parsing bundle")
6969
}
70+
o.SetReplaces(op.Identifier())
7071
if err := e.gen.AddOperator(o); err != nil {
7172
return errors.Wrap(err, "error calculating generation changes due to new bundle")
7273
}
@@ -90,7 +91,7 @@ func (e *NamespaceGenerationEvolver) addNewOperators(add map[OperatorSourceInfo]
9091
return errors.Wrapf(err, "%s not found", s)
9192
}
9293

93-
o, err := NewOperatorFromBundle(bundle, "", s.StartingCSV, *key)
94+
o, err := NewOperatorFromBundle(bundle, s.StartingCSV, *key)
9495
if err != nil {
9596
return errors.Wrap(err, "error parsing bundle")
9697
}
@@ -123,7 +124,7 @@ func (e *NamespaceGenerationEvolver) queryForRequiredAPIs() error {
123124
// attempt to find a bundle that provides that api
124125
if bundle, key, err := e.querier.FindProvider(*api, initialSource); err == nil {
125126
// add a bundle that provides the api to the generation
126-
o, err := NewOperatorFromBundle(bundle, "", "", *key)
127+
o, err := NewOperatorFromBundle(bundle, "", *key)
127128
if err != nil {
128129
return errors.Wrap(err, "error parsing bundle")
129130
}

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

Lines changed: 10 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -238,24 +238,11 @@ type Operator struct {
238238

239239
var _ OperatorSurface = &Operator{}
240240

241-
func NewOperatorFromBundle(bundle *api.Bundle, replaces string, startingCSV string, sourceKey CatalogKey) (*Operator, error) {
242-
if bundle.CsvJson == "" {
243-
return nil, fmt.Errorf("no csv json found")
244-
}
245-
csv := &registry.ClusterServiceVersion{}
246-
if err := json.Unmarshal([]byte(bundle.CsvJson), csv); err != nil {
247-
return nil, err
248-
}
249-
r := replaces
250-
if r == "" {
251-
r, _ = csv.GetReplaces()
252-
}
253-
254-
version, _ := csv.GetVersion()
255-
parsedVersion, err := semver.ParseTolerant(version)
256-
v := &parsedVersion
241+
func NewOperatorFromBundle(bundle *api.Bundle, startingCSV string, sourceKey CatalogKey) (*Operator, error) {
242+
parsedVersion, err := semver.ParseTolerant(bundle.Version)
243+
version := &parsedVersion
257244
if err != nil {
258-
v = nil
245+
version = nil
259246
}
260247
provided := APISet{}
261248
for _, gvk := range bundle.ProvidedApis {
@@ -290,14 +277,12 @@ func NewOperatorFromBundle(bundle *api.Bundle, replaces string, startingCSV stri
290277
}
291278
op.sourceInfo = sourceInfo
292279
op.bundle = bundle
293-
op.replaces = r
294280
return op, nil
295281
}
296282

297283
return &Operator{
298-
name: csv.GetName(),
299-
replaces: r,
300-
version: v,
284+
name: bundle.CsvName,
285+
version: version,
301286
providedAPIs: provided,
302287
requiredAPIs: required,
303288
bundle: bundle,
@@ -333,7 +318,6 @@ func NewOperatorFromV1Alpha1CSV(csv *v1alpha1.ClusterServiceVersion) (*Operator,
333318
return &Operator{
334319
name: csv.GetName(),
335320
version: &csv.Spec.Version.Version,
336-
replaces: csv.Spec.Replaces,
337321
providedAPIs: providedAPIs,
338322
requiredAPIs: requiredAPIs,
339323
sourceInfo: &ExistingOperator,
@@ -356,6 +340,10 @@ func (o *Operator) Replaces() string {
356340
return o.replaces
357341
}
358342

343+
func (o *Operator) SetReplaces(replacing string) {
344+
o.replaces = replacing
345+
}
346+
359347
func (o *Operator) Package() string {
360348
return o.bundle.PackageName
361349
}

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

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -917,6 +917,7 @@ func TestNewOperatorFromBundle(t *testing.T) {
917917
CsvName: "testBundle",
918918
PackageName: "testPackage",
919919
ChannelName: "testChannel",
920+
Version: version.String(),
920921
CsvJson: string(csvJson),
921922
Object: []string{string(csvJson)},
922923
}
@@ -987,6 +988,7 @@ func TestNewOperatorFromBundle(t *testing.T) {
987988
CsvName: "testBundle",
988989
PackageName: "testPackage",
989990
ChannelName: "testChannel",
991+
Version: version.String(),
990992
CsvJson: string(csvJsonWithApis),
991993
Object: []string{string(csvJsonWithApis), string(crdJson)},
992994
ProvidedApis: []*api.GroupVersionKind{
@@ -1043,12 +1045,11 @@ func TestNewOperatorFromBundle(t *testing.T) {
10431045
args: args{
10441046
bundle: bundleNoAPIs,
10451047
sourceKey: CatalogKey{Name: "source", Namespace: "testNamespace"},
1046-
replaces: "",
10471048
},
10481049
want: &Operator{
1050+
// lack of full api response falls back to csv name
10491051
name: "testCSV",
10501052
version: &version.Version,
1051-
replaces: "v1",
10521053
providedAPIs: EmptyAPISet(),
10531054
requiredAPIs: EmptyAPISet(),
10541055
bundle: bundleNoAPIs,
@@ -1064,12 +1065,10 @@ func TestNewOperatorFromBundle(t *testing.T) {
10641065
args: args{
10651066
bundle: bundleWithAPIs,
10661067
sourceKey: CatalogKey{Name: "source", Namespace: "testNamespace"},
1067-
replaces: "",
10681068
},
10691069
want: &Operator{
1070-
name: "testCSV",
1071-
version: &version.Version,
1072-
replaces: "v1",
1070+
name: "testBundle",
1071+
version: &version.Version,
10731072
providedAPIs: APISet{
10741073
opregistry.APIKey{
10751074
Group: "crd.group.com",
@@ -1111,14 +1110,13 @@ func TestNewOperatorFromBundle(t *testing.T) {
11111110
args: args{
11121111
bundle: bundleNoAPIs,
11131112
sourceKey: CatalogKey{Name: "source", Namespace: "testNamespace"},
1114-
replaces: "replaced",
11151113
},
11161114
want: &Operator{
1115+
// lack of full api response falls back to csv name
11171116
name: "testCSV",
11181117
providedAPIs: EmptyAPISet(),
11191118
requiredAPIs: EmptyAPISet(),
11201119
bundle: bundleNoAPIs,
1121-
replaces: "replaced",
11221120
version: &version.Version,
11231121
sourceInfo: &OperatorSourceInfo{
11241122
Package: "testPackage",
@@ -1132,7 +1130,6 @@ func TestNewOperatorFromBundle(t *testing.T) {
11321130
args: args{
11331131
bundle: bundleWithAPIsUnextracted,
11341132
sourceKey: CatalogKey{Name: "source", Namespace: "testNamespace"},
1135-
replaces: "replaced",
11361133
},
11371134
want: &Operator{
11381135
name: "testCSV",
@@ -1164,9 +1161,8 @@ func TestNewOperatorFromBundle(t *testing.T) {
11641161
Plural: "requiredapis",
11651162
}: struct{}{},
11661163
},
1167-
bundle: bundleWithAPIsUnextracted,
1168-
replaces: "replaced",
1169-
version: &version.Version,
1164+
bundle: bundleWithAPIsUnextracted,
1165+
version: &version.Version,
11701166
sourceInfo: &OperatorSourceInfo{
11711167
Package: "testPackage",
11721168
Channel: "testChannel",
@@ -1177,7 +1173,7 @@ func TestNewOperatorFromBundle(t *testing.T) {
11771173
}
11781174
for _, tt := range tests {
11791175
t.Run(tt.name, func(t *testing.T) {
1180-
got, err := NewOperatorFromBundle(tt.args.bundle, tt.args.replaces, "", tt.args.sourceKey)
1176+
got, err := NewOperatorFromBundle(tt.args.bundle, "", tt.args.sourceKey)
11811177
require.Equal(t, tt.wantErr, err)
11821178
require.Equal(t, tt.want, got)
11831179
})

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

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,10 @@ package resolver
44
import (
55
"context"
66
"fmt"
7-
"strings"
87

98
"github.com/blang/semver"
109
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
11-
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
1210
"k8s.io/apimachinery/pkg/util/errors"
13-
"k8s.io/apimachinery/pkg/util/yaml"
1411

1512
"github.com/operator-framework/operator-registry/pkg/api"
1613
"github.com/operator-framework/operator-registry/pkg/client"
@@ -180,22 +177,11 @@ func (q *NamespaceSourceQuerier) findChannelHead(currentVersion *semver.Version,
180177
return nil, err
181178
}
182179

183-
if latest.CsvJson == "" {
180+
if latest.SkipRange == "" {
184181
return nil, nil
185182
}
186183

187-
dec := yaml.NewYAMLOrJSONDecoder(strings.NewReader(latest.CsvJson), 10)
188-
unst := &unstructured.Unstructured{}
189-
if err := dec.Decode(unst); err != nil {
190-
return nil, err
191-
}
192-
193-
skipRange, ok := unst.GetAnnotations()[SkipPackageAnnotationKey]
194-
if !ok {
195-
return nil, nil
196-
}
197-
198-
r, err := semver.ParseRange(skipRange)
184+
r, err := semver.ParseRange(latest.SkipRange)
199185
if err != nil {
200186
return nil, err
201187
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -360,7 +360,7 @@ func TestNamespaceSourceQuerier_FindReplacement(t *testing.T) {
360360
require.NoError(t, err)
361361

362362
nextBundle := &api.Bundle{CsvName: "test.v1", PackageName: "testPkg", ChannelName: "testChannel"}
363-
latestBundle := &api.Bundle{CsvName: "latest", PackageName: "testPkg", ChannelName: "testChannel", CsvJson: string(csvJson), Object: []string{string(csvJson)}}
363+
latestBundle := &api.Bundle{CsvName: "latest", PackageName: "testPkg", ChannelName: "testChannel", CsvJson: string(csvJson), Object: []string{string(csvJson)}, SkipRange: ">= 1.0.0-0 < 1.0.0-1556661308", Version: latestVersion.String()}
364364

365365
csv.SetAnnotations(map[string]string{})
366366
csvUnstNoAnnotationJson, err := json.Marshal(csv)
@@ -491,7 +491,7 @@ func TestNamespaceSourceQuerier_FindReplacement(t *testing.T) {
491491
if err != nil {
492492
t.Log(err.Error())
493493
}
494-
require.Equal(t, tt.out.err, err)
494+
require.Equal(t, tt.out.err, err, "%v", err)
495495
require.Equal(t, tt.out.bundle, got)
496496
require.Equal(t, tt.out.key, key)
497497
})

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,7 @@ func bundle(name, pkg, channel, replaces string, providedCRDs, requiredCRDs, pro
253253
ChannelName: channel,
254254
CsvJson: string(csvJson),
255255
Object: objs,
256+
Version: "0.0.0",
256257
ProvidedApis: apiSetToGVk(providedCRDs, providedAPIServices),
257258
RequiredApis: apiSetToGVk(requiredCRDs, requiredAPIServices),
258259
}

‎test/e2e/installplan_e2e_test.go

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -962,8 +962,8 @@ func TestInstallPlanWithCRDSchemaChange(t *testing.T) {
962962
require.NoError(t, err)
963963

964964
subscriptionName := genName("sub-nginx-alpha-")
965-
// this subscription will be cleaned up below without the clean up function
966-
createSubscriptionForCatalog(t, crc, testNamespace, subscriptionName, mainCatalogSourceName, mainPackageName, stableChannel, "", v1alpha1.ApprovalAutomatic)
965+
cleanupSubscription := createSubscriptionForCatalog(t, crc, testNamespace, subscriptionName, mainCatalogSourceName, mainPackageName, stableChannel, "", v1alpha1.ApprovalAutomatic)
966+
defer cleanupSubscription()
967967

968968
subscription, err := fetchSubscription(t, crc, testNamespace, subscriptionName, subscriptionHasInstallPlanChecker)
969969
require.NoError(t, err)
@@ -1010,15 +1010,18 @@ func TestInstallPlanWithCRDSchemaChange(t *testing.T) {
10101010
// Attempt to get the catalog source before creating install plan(s)
10111011
_, err = fetchCatalogSource(t, crc, mainCatalogSourceName, testNamespace, catalogSourceRegistryPodSynced)
10121012
require.NoError(t, err)
1013+
10131014
// Update the subscription resource to point to the beta CSV
1014-
err = crc.OperatorsV1alpha1().Subscriptions(testNamespace).DeleteCollection(metav1.NewDeleteOptions(0), metav1.ListOptions{})
1015+
subscription, err = fetchSubscription(t, crc, testNamespace, subscriptionName, subscriptionHasInstallPlanChecker)
10151016
require.NoError(t, err)
1017+
require.NotNil(t, subscription)
10161018

1017-
// existing cleanup should remove this
1018-
subscriptionName = genName("sub-nginx-beta")
1019-
createSubscriptionForCatalog(t, crc, testNamespace, subscriptionName, mainCatalogSourceName, mainPackageName, betaChannel, "", v1alpha1.ApprovalAutomatic)
1019+
subscription.Spec.Channel = betaChannel
1020+
subscription, err = crc.OperatorsV1alpha1().Subscriptions(testNamespace).Update(subscription)
1021+
require.NoError(t, err)
10201022

1021-
subscription, err = fetchSubscription(t, crc, testNamespace, subscriptionName, subscriptionHasInstallPlanChecker)
1023+
// Wait for subscription to have a new installplan
1024+
subscription, err = fetchSubscription(t, crc, testNamespace, subscriptionName, subscriptionHasInstallPlanDifferentChecker(fetchedInstallPlan.GetName()))
10221025
require.NoError(t, err)
10231026
require.NotNil(t, subscription)
10241027

0 commit comments

Comments
 (0)