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.

Sign up for GitHub

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

Use the grpc response as the source of truth for update graph data #1105

Merged
merged 2 commits into from
Dec 20, 2019
Merged

Use the grpc response as the source of truth for update graph data #1105

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6b92879
feat(resolver): rely on info from catalog to determine updates
ecordell Nov 1, 2019
2b343b5
test(e2e): testing CRD upgrades shouldn't create a new subscription
ecordell Nov 6, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ require (
github.com/munnerz/goautoneg v0.0.0-20190414153302-2ae31c8b6b30 // indirect
github.com/openshift/api v3.9.1-0.20190924102528-32369d4db2ad+incompatible
github.com/openshift/client-go v0.0.0-20190923180330-3b6373338c9b
github.com/operator-framework/operator-registry v1.5.1
github.com/operator-framework/operator-registry v1.5.3
github.com/pkg/errors v0.8.1
github.com/prometheus/client_golang v0.9.3-0.20190127221311-3c4408c8b829
github.com/sirupsen/logrus v1.4.2
Expand Down
2 changes: 2 additions & 0 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -438,6 +438,8 @@ github.com/opentracing/opentracing-go v1.1.0/go.mod h1:UkNAQd3GIcIGf0SeVgPpRdFSt
github.com/openzipkin/zipkin-go v0.1.6/go.mod h1:QgAqvLzwWbR/WpD4A3cGpPtJrZXNIiJc5AZX7/PBEpw=
github.com/operator-framework/operator-registry v1.5.1 h1:8ruUOG6IBDVTAXYWKsv6hwr4yv/0SFPFPAYGCpcv97E=
github.com/operator-framework/operator-registry v1.5.1/go.mod h1:agrQlkWOo1q8U1SAaLSS2WQ+Z9vswNT2M2HFib9iuLY=
github.com/operator-framework/operator-registry v1.5.3 h1:az83WDwgB+tHsmVn+tFq72yQBbaUAye8e4+KkDQmzLs=
github.com/operator-framework/operator-registry v1.5.3/go.mod h1:agrQlkWOo1q8U1SAaLSS2WQ+Z9vswNT2M2HFib9iuLY=
github.com/otiai10/copy v1.0.1 h1:gtBjD8aq4nychvRZ2CyJvFWAw0aja+VHazDdruZKGZA=
github.com/otiai10/copy v1.0.1/go.mod h1:8bMCJrAqOtN/d9oyh5HR7HhLQMvcGMpGdwRDYsfOCHc=
github.com/otiai10/curr v0.0.0-20150429015615-9b4961190c95/go.mod h1:9qAhocn7zKJG+0mI8eUu6xqkFDYS2kb2saOteoSB3cE=
Expand Down
7 changes: 4 additions & 3 deletions pkg/controller/registry/resolver/evolver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,11 @@ func (e *NamespaceGenerationEvolver) checkForUpdates() error {
continue
}

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

o, err := NewOperatorFromBundle(bundle, "", s.StartingCSV, *key)
o, err := NewOperatorFromBundle(bundle, s.StartingCSV, *key)
if err != nil {
return errors.Wrap(err, "error parsing bundle")
}
Expand Down Expand Up @@ -123,7 +124,7 @@ func (e *NamespaceGenerationEvolver) queryForRequiredAPIs() error {
// attempt to find a bundle that provides that api
if bundle, key, err := e.querier.FindProvider(*api, initialSource); err == nil {
// add a bundle that provides the api to the generation
o, err := NewOperatorFromBundle(bundle, "", "", *key)
o, err := NewOperatorFromBundle(bundle, "", *key)
if err != nil {
return errors.Wrap(err, "error parsing bundle")
}
Expand Down
32 changes: 10 additions & 22 deletions pkg/controller/registry/resolver/operators.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,24 +238,11 @@ type Operator struct {

var _ OperatorSurface = &Operator{}

func NewOperatorFromBundle(bundle *api.Bundle, replaces string, startingCSV string, sourceKey CatalogKey) (*Operator, error) {
if bundle.CsvJson == "" {
return nil, fmt.Errorf("no csv json found")
}
csv := &registry.ClusterServiceVersion{}
if err := json.Unmarshal([]byte(bundle.CsvJson), csv); err != nil {
return nil, err
}
r := replaces
if r == "" {
r, _ = csv.GetReplaces()
}

version, _ := csv.GetVersion()
parsedVersion, err := semver.ParseTolerant(version)
v := &parsedVersion
func NewOperatorFromBundle(bundle *api.Bundle, startingCSV string, sourceKey CatalogKey) (*Operator, error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just asking because I don't know how this works very well:
since the replaces field in the db is an int64 and what is used here is a string that is the equivalent of the replaces field in the csv - does the o.Identifier() extract that replaces csv name from the database/query?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The values from replaces in the registry isn't seen by olm at all.

OLM queries for a replacement with FindReplacement, and uses whatever the registry returns as the value for the "replaces" field that it puts in the CSV.

parsedVersion, err := semver.ParseTolerant(bundle.Version)
version := &parsedVersion
if err != nil {
v = nil
version = nil
}
provided := APISet{}
for _, gvk := range bundle.ProvidedApis {
Expand Down Expand Up @@ -290,14 +277,12 @@ func NewOperatorFromBundle(bundle *api.Bundle, replaces string, startingCSV stri
}
op.sourceInfo = sourceInfo
op.bundle = bundle
op.replaces = r
return op, nil
}

return &Operator{
name: csv.GetName(),
replaces: r,
version: v,
name: bundle.CsvName,
version: version,
providedAPIs: provided,
requiredAPIs: required,
bundle: bundle,
Expand Down Expand Up @@ -333,7 +318,6 @@ func NewOperatorFromV1Alpha1CSV(csv *v1alpha1.ClusterServiceVersion) (*Operator,
return &Operator{
name: csv.GetName(),
version: &csv.Spec.Version.Version,
replaces: csv.Spec.Replaces,
providedAPIs: providedAPIs,
requiredAPIs: requiredAPIs,
sourceInfo: &ExistingOperator,
Expand All @@ -356,6 +340,10 @@ func (o *Operator) Replaces() string {
return o.replaces
}

func (o *Operator) SetReplaces(replacing string) {
o.replaces = replacing
}

func (o *Operator) Package() string {
return o.bundle.PackageName
}
Expand Down
22 changes: 9 additions & 13 deletions pkg/controller/registry/resolver/operators_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -917,6 +917,7 @@ func TestNewOperatorFromBundle(t *testing.T) {
CsvName: "testBundle",
PackageName: "testPackage",
ChannelName: "testChannel",
Version: version.String(),
CsvJson: string(csvJson),
Object: []string{string(csvJson)},
}
Expand Down Expand Up @@ -987,6 +988,7 @@ func TestNewOperatorFromBundle(t *testing.T) {
CsvName: "testBundle",
PackageName: "testPackage",
ChannelName: "testChannel",
Version: version.String(),
CsvJson: string(csvJsonWithApis),
Object: []string{string(csvJsonWithApis), string(crdJson)},
ProvidedApis: []*api.GroupVersionKind{
Expand Down Expand Up @@ -1043,12 +1045,11 @@ func TestNewOperatorFromBundle(t *testing.T) {
args: args{
bundle: bundleNoAPIs,
sourceKey: CatalogKey{Name: "source", Namespace: "testNamespace"},
replaces: "",
},
want: &Operator{
// lack of full api response falls back to csv name
name: "testCSV",
version: &version.Version,
replaces: "v1",
providedAPIs: EmptyAPISet(),
requiredAPIs: EmptyAPISet(),
bundle: bundleNoAPIs,
Expand All @@ -1064,12 +1065,10 @@ func TestNewOperatorFromBundle(t *testing.T) {
args: args{
bundle: bundleWithAPIs,
sourceKey: CatalogKey{Name: "source", Namespace: "testNamespace"},
replaces: "",
},
want: &Operator{
name: "testCSV",
version: &version.Version,
replaces: "v1",
name: "testBundle",
version: &version.Version,
providedAPIs: APISet{
opregistry.APIKey{
Group: "crd.group.com",
Expand Down Expand Up @@ -1111,14 +1110,13 @@ func TestNewOperatorFromBundle(t *testing.T) {
args: args{
bundle: bundleNoAPIs,
sourceKey: CatalogKey{Name: "source", Namespace: "testNamespace"},
replaces: "replaced",
},
want: &Operator{
// lack of full api response falls back to csv name
name: "testCSV",
providedAPIs: EmptyAPISet(),
requiredAPIs: EmptyAPISet(),
bundle: bundleNoAPIs,
replaces: "replaced",
version: &version.Version,
sourceInfo: &OperatorSourceInfo{
Package: "testPackage",
Expand All @@ -1132,7 +1130,6 @@ func TestNewOperatorFromBundle(t *testing.T) {
args: args{
bundle: bundleWithAPIsUnextracted,
sourceKey: CatalogKey{Name: "source", Namespace: "testNamespace"},
replaces: "replaced",
},
want: &Operator{
name: "testCSV",
Expand Down Expand Up @@ -1164,9 +1161,8 @@ func TestNewOperatorFromBundle(t *testing.T) {
Plural: "requiredapis",
}: struct{}{},
},
bundle: bundleWithAPIsUnextracted,
replaces: "replaced",
version: &version.Version,
bundle: bundleWithAPIsUnextracted,
version: &version.Version,
sourceInfo: &OperatorSourceInfo{
Package: "testPackage",
Channel: "testChannel",
Expand All @@ -1177,7 +1173,7 @@ func TestNewOperatorFromBundle(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := NewOperatorFromBundle(tt.args.bundle, tt.args.replaces, "", tt.args.sourceKey)
got, err := NewOperatorFromBundle(tt.args.bundle, "", tt.args.sourceKey)
require.Equal(t, tt.wantErr, err)
require.Equal(t, tt.want, got)
})
Expand Down
18 changes: 2 additions & 16 deletions pkg/controller/registry/resolver/querier.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,10 @@ package resolver
import (
"context"
"fmt"
"strings"

"github.com/blang/semver"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/util/errors"
"k8s.io/apimachinery/pkg/util/yaml"

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

if latest.CsvJson == "" {
if latest.SkipRange == "" {
return nil, nil
}

dec := yaml.NewYAMLOrJSONDecoder(strings.NewReader(latest.CsvJson), 10)
unst := &unstructured.Unstructured{}
if err := dec.Decode(unst); err != nil {
return nil, err
}

skipRange, ok := unst.GetAnnotations()[SkipPackageAnnotationKey]
if !ok {
return nil, nil
}

r, err := semver.ParseRange(skipRange)
r, err := semver.ParseRange(latest.SkipRange)
if err != nil {
return nil, err
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/controller/registry/resolver/querier_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,7 @@ func TestNamespaceSourceQuerier_FindReplacement(t *testing.T) {
require.NoError(t, err)

nextBundle := &api.Bundle{CsvName: "test.v1", PackageName: "testPkg", ChannelName: "testChannel"}
latestBundle := &api.Bundle{CsvName: "latest", PackageName: "testPkg", ChannelName: "testChannel", CsvJson: string(csvJson), Object: []string{string(csvJson)}}
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()}

csv.SetAnnotations(map[string]string{})
csvUnstNoAnnotationJson, err := json.Marshal(csv)
Expand Down Expand Up @@ -491,7 +491,7 @@ func TestNamespaceSourceQuerier_FindReplacement(t *testing.T) {
if err != nil {
t.Log(err.Error())
}
require.Equal(t, tt.out.err, err)
require.Equal(t, tt.out.err, err, "%v", err)
require.Equal(t, tt.out.bundle, got)
require.Equal(t, tt.out.key, key)
})
Expand Down
1 change: 1 addition & 0 deletions pkg/controller/registry/resolver/util_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,7 @@ func bundle(name, pkg, channel, replaces string, providedCRDs, requiredCRDs, pro
ChannelName: channel,
CsvJson: string(csvJson),
Object: objs,
Version: "0.0.0",
ProvidedApis: apiSetToGVk(providedCRDs, providedAPIServices),
RequiredApis: apiSetToGVk(requiredCRDs, requiredAPIServices),
}
Expand Down
17 changes: 10 additions & 7 deletions test/e2e/installplan_e2e_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -962,8 +962,8 @@ func TestInstallPlanWithCRDSchemaChange(t *testing.T) {
require.NoError(t, err)

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

subscription, err := fetchSubscription(t, crc, testNamespace, subscriptionName, subscriptionHasInstallPlanChecker)
require.NoError(t, err)
Expand Down Expand Up @@ -1010,15 +1010,18 @@ func TestInstallPlanWithCRDSchemaChange(t *testing.T) {
// Attempt to get the catalog source before creating install plan(s)
_, err = fetchCatalogSource(t, crc, mainCatalogSourceName, testNamespace, catalogSourceRegistryPodSynced)
require.NoError(t, err)

// Update the subscription resource to point to the beta CSV
err = crc.OperatorsV1alpha1().Subscriptions(testNamespace).DeleteCollection(metav1.NewDeleteOptions(0), metav1.ListOptions{})
subscription, err = fetchSubscription(t, crc, testNamespace, subscriptionName, subscriptionHasInstallPlanChecker)
require.NoError(t, err)
require.NotNil(t, subscription)

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

subscription, err = fetchSubscription(t, crc, testNamespace, subscriptionName, subscriptionHasInstallPlanChecker)
// Wait for subscription to have a new installplan
subscription, err = fetchSubscription(t, crc, testNamespace, subscriptionName, subscriptionHasInstallPlanDifferentChecker(fetchedInstallPlan.GetName()))
require.NoError(t, err)
require.NotNil(t, subscription)

Expand Down
Loading