Skip to content

Commit

Permalink
fix(subscriptions): respect startingCSV
Browse files Browse the repository at this point in the history
  • Loading branch information
@njhale
njhale committed Jan 30, 2019
1 parent f474ec8 commit 4660665
Show file tree
Hide file tree
Showing 29 changed files with 663 additions and 144 deletions.
3 changes: 2 additions & 1 deletion Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -150,10 +150,11 @@ container-mockgen:
docker build -t olm:mockgen -f mockgen.Dockerfile .
docker run --name temp-mockgen olm:mockgen /bin/true
docker cp temp-mockgen:/go/src/github.com/operator-framework/operator-lifecycle-manager/pkg/api/wrappers/wrappersfakes/. ./pkg/api/wrappers/wrappersfakes
docker cp temp-mockgen:/go/src/github.com/operator-framework/operator-lifecycle-manager/pkg/fakes/client-go/listers/. ./pkg/fakes/client-go/listers
docker cp temp-mockgen:/go/src/github.com/operator-framework/operator-lifecycle-manager/pkg/lib/operatorlister/operatorlisterfakes/. ./pkg/lib/operatorlister/operatorlisterfakes
docker cp temp-mockgen:/go/src/github.com/operator-framework/operator-lifecycle-manager/pkg/lib/operatorclient/mock_client.go ./pkg/lib/operatorclient/mock_client.go
docker cp temp-mockgen:/go/src/github.com/operator-framework/operator-lifecycle-manager/pkg/fakes/. ./pkg/fakes/.
docker cp temp-mockgen:/go/src/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver/fakes/fake_registry_client.go ./pkg/controller/registry/resolver/fakes/fake_registry_client.go
docker cp temp-mockgen:/go/src/github.com/operator-framework/operator-lifecycle-manager/pkg/package-server/client/fakes/fake_registry_client.go ./pkg/package-server/client/fakes/fake_registry_client.go
docker rm temp-mockgen

# Must be run in gopath: https://github.com/kubernetes/kubernetes/issues/67566
Expand Down
2 changes: 1 addition & 1 deletion go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ require (
github.com/openshift/api v3.9.1-0.20190129160438-bbc4289c54e0+incompatible
github.com/openshift/client-go v0.0.0-20190128154758-1540772775fa
github.com/openshift/library-go v0.0.0-20190125204812-22b2ba2f485f
github.com/operator-framework/operator-registry v1.0.4
github.com/operator-framework/operator-registry v1.0.5
github.com/peterbourgon/diskv v2.0.1+incompatible // indirect
github.com/pkg/errors v0.8.0
github.com/prometheus/client_golang v0.9.1
Expand Down
4 changes: 2 additions & 2 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,8 @@ github.com/openshift/library-go v0.0.0-20190125204812-22b2ba2f485f/go.mod h1:NBt
github.com/operator-framework/operator-lifecycle-manager v0.0.0-20181023032605-e838f7fb2186/go.mod h1:Ma5ZXd4S1vmMyewWlF7aO8CZiokR7Sd8dhSfkGkNU4U=
github.com/operator-framework/operator-lifecycle-manager v0.0.0-20190105193533-81104ffdc4fb/go.mod h1:XMyE4n2opUK4N6L45YGQkXXi8F9fD7XDYFv/CsS6V5I=
github.com/operator-framework/operator-registry v1.0.1/go.mod h1:1xEdZjjUg2hPEd52LG3YQ0jtwiwEGdm98S1TH5P4RAA=
github.com/operator-framework/operator-registry v1.0.4 h1:/xOuw0AFrnV/zjZQeEGtX/xH/ZoLOrNZqWaYU5bRQwM=
github.com/operator-framework/operator-registry v1.0.4/go.mod h1:hve6YwcjM2nGVlscLtNsp9sIIBkNZo6jlJgzWw7vP9s=
github.com/operator-framework/operator-registry v1.0.5 h1:LKhQvSeA3/PHSThchV6rquS8jy2ocvYRHqqCikkyumE=
github.com/operator-framework/operator-registry v1.0.5/go.mod h1:hve6YwcjM2nGVlscLtNsp9sIIBkNZo6jlJgzWw7vP9s=
github.com/pborman/uuid v1.2.0 h1:J7Q5mO4ysT1dv8hyrUGHb9+ooztCXu1D8MY8DZYsu3g=
github.com/pborman/uuid v1.2.0/go.mod h1:X/NO0urCmaxf9VXbdlT7C2Yzkj2IKimNn4k+gtPdI/k=
github.com/petar/GoLLRB v0.0.0-20130427215148-53be0d36a84c/go.mod h1:HUpKUBZnpzkdx0kD/+Yfuft+uD3zHGtXF/XJB14TUr4=
Expand Down
23 changes: 15 additions & 8 deletions pkg/controller/operators/catalog/operator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -526,14 +526,11 @@ func (o *Operator) syncResolvingNamespace(obj interface{}) error {
// get the set of sources that should be used for resolution and best-effort get their connections working
logger.Debug("resolving sources")
resolverSources := o.ensureResolverSources(logger, namespace)
querier := resolver.NewNamespaceSourceQuerier(resolverSources)

logger.Debug("checking if subscriptions need update")

subs, err := o.lister.OperatorsV1alpha1().SubscriptionLister().Subscriptions(namespace).List(labels.Everything())
if err != nil {
logger.WithError(err).Debug("couldn't list subscriptions")
return err
}

shouldUpdate := false
for _, sub := range subs {
Expand All @@ -551,7 +548,7 @@ func (o *Operator) syncResolvingNamespace(obj interface{}) error {
}

// record the current state of the desired corresponding CSV in the status. no-op if we don't know the csv yet.
sub, err = o.ensureSubscriptionCSVState(logger, sub)
sub, err = o.ensureSubscriptionCSVState(logger, sub, querier)
if err != nil {
return err
}
Expand All @@ -565,7 +562,7 @@ func (o *Operator) syncResolvingNamespace(obj interface{}) error {
logger.Debug("resolving subscriptions in namespace")

// resolve a set of steps to apply to a cluster, a set of subscriptions to create/update, and any errors
steps, subs, err := o.resolver.ResolveSteps(namespace, resolver.NewNamespaceSourceQuerier(resolverSources))
steps, subs, err := o.resolver.ResolveSteps(namespace, querier)
if err != nil {
return err
}
Expand Down Expand Up @@ -699,7 +696,7 @@ func (o *Operator) ensureSubscriptionInstallPlanState(logger *logrus.Entry, sub
return updated, nil
}

func (o *Operator) ensureSubscriptionCSVState(logger *logrus.Entry, sub *v1alpha1.Subscription) (*v1alpha1.Subscription, error) {
func (o *Operator) ensureSubscriptionCSVState(logger *logrus.Entry, sub *v1alpha1.Subscription, querier resolver.SourceQuerier) (*v1alpha1.Subscription, error) {
if sub.Status.CurrentCSV == "" {
return sub, nil
}
Expand All @@ -710,7 +707,17 @@ func (o *Operator) ensureSubscriptionCSVState(logger *logrus.Entry, sub *v1alpha
logger.WithError(err).WithField("currentCSV", sub.Status.CurrentCSV).Debug("error fetching csv listed in subscription status")
out.Status.State = v1alpha1.SubscriptionStateUpgradePending
} else {
out.Status.State = v1alpha1.SubscriptionStateAtLatest
// Check if an update is available for the current csv
if err := querier.Queryable(); err != nil {
return nil, err
}
bundle, _, _ := querier.FindReplacement(sub.Status.CurrentCSV, sub.Spec.Package, sub.Spec.Channel, resolver.CatalogKey{sub.Spec.CatalogSource, sub.Spec.CatalogSourceNamespace})
if bundle != nil {
out.Status.State = v1alpha1.SubscriptionStateUpgradeAvailable
} else {
out.Status.State = v1alpha1.SubscriptionStateAtLatest
}

out.Status.InstalledCSV = sub.Status.CurrentCSV
}

Expand Down
9 changes: 4 additions & 5 deletions pkg/controller/registry/resolver/evolver.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func (e *NamespaceGenerationEvolver) checkForUpdates() error {
continue
}

o, err := NewOperatorFromBundle(bundle, *key)
o, err := NewOperatorFromBundle(bundle, op.SourceInfo().StartingCSV, *key)
if err != nil {
return errors.Wrap(err, "error parsing bundle")
}
Expand All @@ -76,14 +76,13 @@ func (e *NamespaceGenerationEvolver) checkForUpdates() error {

func (e *NamespaceGenerationEvolver) addNewOperators(add map[OperatorSourceInfo]struct{}) error {
for s := range add {
bundle, key, err := e.querier.FindPackage(s.Package, s.Channel, s.Catalog)

bundle, key, err := e.querier.FindPackage(s.Package, s.Channel, s.StartingCSV, s.Catalog)
if err != nil {
// TODO: log or collect warnings
return errors.Wrapf(err, "%s not found", s)
}

o, err := NewOperatorFromBundle(bundle, *key)
o, err := NewOperatorFromBundle(bundle, s.StartingCSV, *key)
if err != nil {
return errors.Wrap(err, "error parsing bundle")
}
Expand All @@ -109,7 +108,7 @@ func (e *NamespaceGenerationEvolver) queryForRequiredAPIs() error {
// attempt to find a bundle that provides that api
if bundle, key, err := e.querier.FindProvider(*api); 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
102 changes: 79 additions & 23 deletions pkg/controller/registry/resolver/evolver_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func TestNamespaceGenerationEvolver(t *testing.T) {
},
}),
gen: NewGenerationFromOperators(
NewFakeOperatorSurface("op1", "pkgA", "c", "", "s", []opregistry.APIKey{{"g", "v", "k", "ks"}}, nil, nil, nil),
NewFakeOperatorSurface("op1", "pkgA", "c", "", "s", "", []opregistry.APIKey{{"g", "v", "k", "ks"}}, nil, nil, nil),
),
},
args: args{
Expand All @@ -68,10 +68,67 @@ func TestNamespaceGenerationEvolver(t *testing.T) {
},
},
wantGen: NewGenerationFromOperators(
NewFakeOperatorSurface("op1", "pkgA", "c", "", "s", []opregistry.APIKey{{"g", "v", "k", "ks"}}, nil, nil, nil),
NewFakeOperatorSurface("nothing.v1", "nothing", "channel", "", "catsrc", nil, nil, nil, nil),
NewFakeOperatorSurface("op1", "pkgA", "c", "", "s", "", []opregistry.APIKey{{"g", "v", "k", "ks"}}, nil, nil, nil),
NewFakeOperatorSurface("nothing.v1", "nothing", "channel", "", "catsrc", "", nil, nil, nil, nil),
),
},
{
name: "NoNewRequiredAPIs/StartingCSV",
fields: fields{
querier: NewFakeSourceQuerier(map[CatalogKey][]*opregistry.Bundle{
CatalogKey{"catsrc", "catsrc-namespace"}: {
bundle("csv1", "p", "c", "", EmptyAPISet(), EmptyAPISet(), EmptyAPISet(), EmptyAPISet()),
bundle("nothing.v1", "nothing", "channel", "", EmptyAPISet(), EmptyAPISet(), EmptyAPISet(), EmptyAPISet()),
bundle("nothing.v2", "nothing", "channel", "nothing.v1", EmptyAPISet(), EmptyAPISet(), EmptyAPISet(), EmptyAPISet()),
},
}),
gen: NewGenerationFromOperators(
NewFakeOperatorSurface("op1", "pkgA", "c", "", "s", "", []opregistry.APIKey{{"g", "v", "k", "ks"}}, nil, nil, nil),
),
},
args: args{
add: map[OperatorSourceInfo]struct{}{
OperatorSourceInfo{
Package: "nothing",
Channel: "channel",
StartingCSV: "nothing.v1",
Catalog: CatalogKey{"catsrc", "catsrc-namespace"},
}: {},
},
},
wantGen: NewGenerationFromOperators(
NewFakeOperatorSurface("op1", "pkgA", "c", "", "s", "", []opregistry.APIKey{{"g", "v", "k", "ks"}}, nil, nil, nil),
NewFakeOperatorSurface("nothing.v1", "nothing", "channel", "", "catsrc", "nothing.v1", nil, nil, nil, nil),
),
},
{
name: "NoNewRequiredAPIs/StartingCSV/NotFound",
fields: fields{
querier: NewFakeSourceQuerier(map[CatalogKey][]*opregistry.Bundle{
CatalogKey{"catsrc", "catsrc-namespace"}: {
bundle("csv1", "p", "c", "", EmptyAPISet(), EmptyAPISet(), EmptyAPISet(), EmptyAPISet()),
bundle("nothing.v2", "nothing", "channel", "", EmptyAPISet(), EmptyAPISet(), EmptyAPISet(), EmptyAPISet()),
},
}),
gen: NewGenerationFromOperators(
NewFakeOperatorSurface("op1", "pkgA", "c", "", "s", "", []opregistry.APIKey{{"g", "v", "k", "ks"}}, nil, nil, nil),
),
},
args: args{
add: map[OperatorSourceInfo]struct{}{
OperatorSourceInfo{
Package: "nothing",
Channel: "channel",
StartingCSV: "nothing.v1",
Catalog: CatalogKey{"catsrc", "catsrc-namespace"},
}: {},
},
},
wantGen: NewGenerationFromOperators(
NewFakeOperatorSurface("op1", "pkgA", "c", "", "s", "", []opregistry.APIKey{{"g", "v", "k", "ks"}}, nil, nil, nil),
),
wantErr: fmt.Errorf("{nothing channel nothing.v1 {catsrc catsrc-namespace}} not found: no bundle found"),
},
{
// the incoming subscription requires apis that can't be found
// this should contract back to the original set
Expand Down Expand Up @@ -106,7 +163,7 @@ func TestNamespaceGenerationEvolver(t *testing.T) {
},
}),
gen: NewGenerationFromOperators(
NewFakeOperatorSurface("op1", "pkgA", "c", "", "s", []opregistry.APIKey{{"g", "v", "k", "ks"}}, nil, nil, nil),
NewFakeOperatorSurface("op1", "pkgA", "c", "", "s", "", []opregistry.APIKey{{"g", "v", "k", "ks"}}, nil, nil, nil),
),
},
args: args{
Expand All @@ -119,7 +176,7 @@ func TestNamespaceGenerationEvolver(t *testing.T) {
},
},
wantGen: NewGenerationFromOperators(
NewFakeOperatorSurface("op1", "pkgA", "c", "", "s", []opregistry.APIKey{{"g", "v", "k", "ks"}}, nil, nil, nil),
NewFakeOperatorSurface("op1", "pkgA", "c", "", "s", "", []opregistry.APIKey{{"g", "v", "k", "ks"}}, nil, nil, nil),
),
},
{
Expand All @@ -145,8 +202,8 @@ func TestNamespaceGenerationEvolver(t *testing.T) {
},
},
wantGen: NewGenerationFromOperators(
NewFakeOperatorSurface("depender.v1", "depender", "channel", "", "catsrc", nil, []opregistry.APIKey{{"g", "v", "k", "ks"}}, nil, nil),
NewFakeOperatorSurface("provider.v1", "provider", "channel", "", "catsrc", []opregistry.APIKey{{"g", "v", "k", "ks"}}, nil, nil, nil),
NewFakeOperatorSurface("depender.v1", "depender", "channel", "", "catsrc", "", nil, []opregistry.APIKey{{"g", "v", "k", "ks"}}, nil, nil),
NewFakeOperatorSurface("provider.v1", "provider", "channel", "", "catsrc", "", []opregistry.APIKey{{"g", "v", "k", "ks"}}, nil, nil, nil),
),
},
{
Expand Down Expand Up @@ -207,9 +264,9 @@ func TestNamespaceGenerationEvolver(t *testing.T) {
},
},
wantGen: NewGenerationFromOperators(
NewFakeOperatorSurface("depender.v1", "depender", "channel", "", "catsrc", nil, []opregistry.APIKey{{"g", "v", "k", "ks"}}, nil, nil),
NewFakeOperatorSurface("provider.v1", "provider", "channel", "", "catsrc", []opregistry.APIKey{{"g", "v", "k", "ks"}}, []opregistry.APIKey{{"g2", "v2", "k2", "k2s"}}, nil, nil),
NewFakeOperatorSurface("provider2.v1", "provider2", "channel", "", "catsrc", []opregistry.APIKey{{"g2", "v2", "k2", "k2s"}}, nil, nil, nil),
NewFakeOperatorSurface("depender.v1", "depender", "channel", "", "catsrc", "", nil, []opregistry.APIKey{{"g", "v", "k", "ks"}}, nil, nil),
NewFakeOperatorSurface("provider.v1", "provider", "channel", "", "catsrc", "", []opregistry.APIKey{{"g", "v", "k", "ks"}}, []opregistry.APIKey{{"g2", "v2", "k2", "k2s"}}, nil, nil),
NewFakeOperatorSurface("provider2.v1", "provider2", "channel", "", "catsrc", "", []opregistry.APIKey{{"g2", "v2", "k2", "k2s"}}, nil, nil, nil),
),
},
{
Expand Down Expand Up @@ -243,9 +300,9 @@ func TestNamespaceGenerationEvolver(t *testing.T) {
},
},
wantGen: NewGenerationFromOperators(
NewFakeOperatorSurface("depender.v1", "depender", "channel", "", "catsrc", nil, []opregistry.APIKey{{"g", "v", "k", "ks"}}, nil, nil),
NewFakeOperatorSurface("provider.v1", "provider", "channel", "", "catsrc", []opregistry.APIKey{{"g", "v", "k", "ks"}}, nil, nil, []opregistry.APIKey{{"g2", "v2", "k2", "k2s"}}),
NewFakeOperatorSurface("provider2.v1", "provider2", "channel", "", "catsrc", nil, nil, []opregistry.APIKey{{"g2", "v2", "k2", "k2s"}}, nil),
NewFakeOperatorSurface("depender.v1", "depender", "channel", "", "catsrc", "", nil, []opregistry.APIKey{{"g", "v", "k", "ks"}}, nil, nil),
NewFakeOperatorSurface("provider.v1", "provider", "channel", "", "catsrc", "", []opregistry.APIKey{{"g", "v", "k", "ks"}}, nil, nil, []opregistry.APIKey{{"g2", "v2", "k2", "k2s"}}),
NewFakeOperatorSurface("provider2.v1", "provider2", "channel", "", "catsrc", "", nil, nil, []opregistry.APIKey{{"g2", "v2", "k2", "k2s"}}, nil),
),
},
{
Expand All @@ -265,8 +322,7 @@ func TestNamespaceGenerationEvolver(t *testing.T) {
},
}),
gen: NewGenerationFromOperators(
NewFakeOperatorSurface("original", "o", "c", "", "s",
[]opregistry.APIKey{{"g3", "v3", "k3", "k3s"}}, nil, nil, nil),
NewFakeOperatorSurface("original", "o", "c", "", "s", "", []opregistry.APIKey{{"g3", "v3", "k3", "k3s"}}, nil, nil, nil),
),
},
args: args{
Expand All @@ -279,10 +335,10 @@ func TestNamespaceGenerationEvolver(t *testing.T) {
},
},
wantGen: NewGenerationFromOperators(
NewFakeOperatorSurface("original", "o", "c", "", "s", []opregistry.APIKey{{"g3", "v3", "k3", "k3s"}}, nil, nil, nil),
NewFakeOperatorSurface("depender.v1", "depender", "channel", "", "catsrc", nil, []opregistry.APIKey{{"g", "v", "k", "ks"}}, nil, nil),
NewFakeOperatorSurface("provider.v1", "provider", "channel", "", "catsrc", []opregistry.APIKey{{"g", "v", "k", "ks"}}, []opregistry.APIKey{{"g2", "v2", "k2", "k2s"}}, nil, nil),
NewFakeOperatorSurface("provider2.v1", "provider2", "channel", "", "catsrc", []opregistry.APIKey{{"g2", "v2", "k2", "k2s"}}, []opregistry.APIKey{{"g3", "v3", "k3", "k3s"}}, nil, nil),
NewFakeOperatorSurface("original", "o", "c", "", "s", "", []opregistry.APIKey{{"g3", "v3", "k3", "k3s"}}, nil, nil, nil),
NewFakeOperatorSurface("depender.v1", "depender", "channel", "", "catsrc", "", nil, []opregistry.APIKey{{"g", "v", "k", "ks"}}, nil, nil),
NewFakeOperatorSurface("provider.v1", "provider", "channel", "", "catsrc", "", []opregistry.APIKey{{"g", "v", "k", "ks"}}, []opregistry.APIKey{{"g2", "v2", "k2", "k2s"}}, nil, nil),
NewFakeOperatorSurface("provider2.v1", "provider2", "channel", "", "catsrc", "", []opregistry.APIKey{{"g2", "v2", "k2", "k2s"}}, []opregistry.APIKey{{"g3", "v3", "k3", "k3s"}}, nil, nil),
),
},
{
Expand All @@ -299,7 +355,7 @@ func TestNamespaceGenerationEvolver(t *testing.T) {
},
}),
gen: NewGenerationFromOperators(
NewFakeOperatorSurface("original", "o", "c", "", "catsrc", nil, nil, nil, nil),
NewFakeOperatorSurface("original", "o", "c", "", "catsrc", "", nil, nil, nil, nil),
),
},
args: args{
Expand All @@ -312,9 +368,9 @@ func TestNamespaceGenerationEvolver(t *testing.T) {
},
},
wantGen: NewGenerationFromOperators(
NewFakeOperatorSurface("updated", "o", "c", "original", "catsrc", nil, nil, nil, nil),
NewFakeOperatorSurface("depender.v1", "depender", "channel", "", "catsrc", nil, []opregistry.APIKey{{"g", "v", "k", "ks"}}, nil, nil),
NewFakeOperatorSurface("provider.v1", "provider", "channel", "", "catsrc", []opregistry.APIKey{{"g", "v", "k", "ks"}}, nil, nil, nil),
NewFakeOperatorSurface("updated", "o", "c", "original", "catsrc", "", nil, nil, nil, nil),
NewFakeOperatorSurface("depender.v1", "depender", "channel", "", "catsrc", "", nil, []opregistry.APIKey{{"g", "v", "k", "ks"}}, nil, nil),
NewFakeOperatorSurface("provider.v1", "provider", "channel", "", "catsrc", "", []opregistry.APIKey{{"g", "v", "k", "ks"}}, nil, nil, nil),
),
},
}
Expand Down
Loading

0 comments on commit 4660665

Please sign in to comment.