Skip to content

Commit

Permalink
Removes error silencing from IsFailForwardEnabled (#2957)
Browse files Browse the repository at this point in the history 
Signed-off-by: Mikalai Radchuk <mradchuk@redhat.com>
  • Loading branch information
@m1kola
m1kola authored Jun 12, 2023
1 parent c29863b commit 08a95ad
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 4 deletions.
4 changes: 2 additions & 2 deletions pkg/controller/registry/resolver/fail_forward.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ import (
// backwards compatible.
func IsFailForwardEnabled(ogLister v1listers.OperatorGroupNamespaceLister) (bool, error) {
ogs, err := ogLister.List(labels.Everything())
if err != nil || len(ogs) == 0 {
return false, nil
if err != nil {
return false, err
}
if len(ogs) != 1 {
return false, fmt.Errorf("found %d operatorGroups, expected 1", len(ogs))
Expand Down
10 changes: 9 additions & 1 deletion pkg/controller/registry/resolver/resolver_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,10 @@ import (
"github.com/sirupsen/logrus/hooks/test"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

"github.com/operator-framework/api/pkg/constraints"
operatorsv1 "github.com/operator-framework/api/pkg/operators/v1"
"github.com/operator-framework/api/pkg/operators/v1alpha1"
"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver/cache"
"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver/solver"
Expand Down Expand Up @@ -1373,6 +1375,12 @@ func TestSolveOperators_TransferApiOwnership(t *testing.T) {

namespace := "olm"
catalog := cache.SourceKey{Name: "community", Namespace: namespace}
og := &operatorsv1.OperatorGroup{
ObjectMeta: metav1.ObjectMeta{
Name: "og",
Namespace: namespace,
},
}

phases := []struct {
subs []*v1alpha1.Subscription
Expand Down Expand Up @@ -1442,7 +1450,7 @@ func TestSolveOperators_TransferApiOwnership(t *testing.T) {
key: cache.NewVirtualSourceKey(namespace),
csvLister: &csvs,
subLister: fakeSubscriptionLister(p.subs),
ogLister: fakeOperatorGroupLister{},
ogLister: fakeOperatorGroupLister{og},
logger: logger,
},
}),
Expand Down
8 changes: 7 additions & 1 deletion pkg/controller/registry/resolver/source_csvs_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,12 @@ func (f fakeOperatorGroupLister) Get(name string) (*operatorsv1.OperatorGroup, e
}

func TestPropertiesAnnotationHonored(t *testing.T) {
og := &operatorsv1.OperatorGroup{
ObjectMeta: metav1.ObjectMeta{
Name: "og",
Namespace: "fake-ns",
},
}
src := &csvSource{
csvLister: fakeCSVLister{
&v1alpha1.ClusterServiceVersion{
Expand All @@ -478,7 +484,7 @@ func TestPropertiesAnnotationHonored(t *testing.T) {
},
},
subLister: fakeSubscriptionLister{},
ogLister: fakeOperatorGroupLister{},
ogLister: fakeOperatorGroupLister{og},
}
ss, err := src.Snapshot(context.Background())
require.NoError(t, err)
Expand Down
34 changes: 34 additions & 0 deletions pkg/controller/registry/resolver/step_resolver_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,7 @@ func TestResolver(t *testing.T) {
name: "SubscriptionOmitsChannel",
clusterState: []runtime.Object{
newSub(namespace, "package", "", catalog),
newOperatorGroup("foo", namespace),
},
bundlesByCatalog: map[resolvercache.SourceKey][]*api.Bundle{
catalog: {
Expand All @@ -354,6 +355,7 @@ func TestResolver(t *testing.T) {
name: "SubscriptionWithNoCandidates/Error",
clusterState: []runtime.Object{
newSub(namespace, "a", "alpha", catalog),
newOperatorGroup("foo", namespace),
},
out: resolverTestOut{
solverError: solver.NotSatisfiable{
Expand All @@ -372,6 +374,7 @@ func TestResolver(t *testing.T) {
name: "SubscriptionWithNoCandidatesInPackage/Error",
clusterState: []runtime.Object{
newSub(namespace, "a", "alpha", catalog),
newOperatorGroup("foo", namespace),
},
bundlesByCatalog: map[resolvercache.SourceKey][]*api.Bundle{
catalog: {
Expand All @@ -395,6 +398,7 @@ func TestResolver(t *testing.T) {
name: "SubscriptionWithNoCandidatesInChannel/Error",
clusterState: []runtime.Object{
newSub(namespace, "a", "alpha", catalog),
newOperatorGroup("foo", namespace),
},
bundlesByCatalog: map[resolvercache.SourceKey][]*api.Bundle{
catalog: {
Expand All @@ -418,6 +422,7 @@ func TestResolver(t *testing.T) {
name: "SubscriptionWithNoCandidatesWithStartingCSVName/Error",
clusterState: []runtime.Object{
newSub(namespace, "a", "alpha", catalog, withStartingCSV("notfound")),
newOperatorGroup("foo", namespace),
},
bundlesByCatalog: map[resolvercache.SourceKey][]*api.Bundle{
catalog: {
Expand All @@ -441,6 +446,7 @@ func TestResolver(t *testing.T) {
name: "SingleNewSubscription/NoDeps",
clusterState: []runtime.Object{
newSub(namespace, "a", "alpha", catalog),
newOperatorGroup("foo", namespace),
},
bundlesByCatalog: map[resolvercache.SourceKey][]*api.Bundle{
catalog: {
Expand All @@ -460,6 +466,7 @@ func TestResolver(t *testing.T) {
name: "SingleNewSubscription/ResolveOne",
clusterState: []runtime.Object{
newSub(namespace, "a", "alpha", catalog),
newOperatorGroup("foo", namespace),
},
bundlesByCatalog: map[resolvercache.SourceKey][]*api.Bundle{
catalog: {
Expand All @@ -482,6 +489,7 @@ func TestResolver(t *testing.T) {
name: "SingleNewSubscription/ResolveOne/BundlePath",
clusterState: []runtime.Object{
newSub(namespace, "a", "alpha", catalog),
newOperatorGroup("foo", namespace),
},
bundlesByCatalog: map[resolvercache.SourceKey][]*api.Bundle{
catalog: {
Expand Down Expand Up @@ -528,6 +536,7 @@ func TestResolver(t *testing.T) {
name: "SingleNewSubscription/ResolveOne/AdditionalBundleObjects",
clusterState: []runtime.Object{
newSub(namespace, "a", "alpha", catalog),
newOperatorGroup("foo", namespace),
},
bundlesByCatalog: map[resolvercache.SourceKey][]*api.Bundle{
catalog: {
Expand All @@ -550,6 +559,7 @@ func TestResolver(t *testing.T) {
name: "SingleNewSubscription/ResolveOne/AdditionalBundleObjects/Service",
clusterState: []runtime.Object{
newSub(namespace, "a", "alpha", catalog),
newOperatorGroup("foo", namespace),
},
bundlesByCatalog: map[resolvercache.SourceKey][]*api.Bundle{
catalog: {
Expand All @@ -572,6 +582,7 @@ func TestResolver(t *testing.T) {
name: "SingleNewSubscription/DependencyMissing",
clusterState: []runtime.Object{
newSub(namespace, "a", "alpha", catalog),
newOperatorGroup("foo", namespace),
},
bundlesByCatalog: map[resolvercache.SourceKey][]*api.Bundle{
catalog: {
Expand Down Expand Up @@ -606,6 +617,7 @@ func TestResolver(t *testing.T) {
clusterState: []runtime.Object{
existingSub(namespace, "a.v1", "a", "alpha", catalog),
existingOperator(namespace, "a.v1", "a", "alpha", "", Provides1, nil, nil, nil),
newOperatorGroup("foo", namespace),
},
bundlesByCatalog: map[resolvercache.SourceKey][]*api.Bundle{
catalog: {
Expand All @@ -620,6 +632,7 @@ func TestResolver(t *testing.T) {
existingSub(namespace, "a.v1", "a", "alpha", catalog),
existingSub(namespace, "b.v1", "b", "alpha", catalog),
existingOperator(namespace, "a.v1", "a", "alpha", "", Provides1, nil, nil, nil),
newOperatorGroup("foo", namespace),
},
bundlesByCatalog: map[resolvercache.SourceKey][]*api.Bundle{
catalog: {
Expand All @@ -638,6 +651,7 @@ func TestResolver(t *testing.T) {
clusterState: []runtime.Object{
newSub(namespace, "a", "alpha", catalog),
newSub(namespace, "a", "beta", catalog),
newOperatorGroup("foo", namespace),
},
bundlesByCatalog: map[resolvercache.SourceKey][]*api.Bundle{
catalog: {
Expand All @@ -664,6 +678,7 @@ func TestResolver(t *testing.T) {
s.Name = s.Name + "-2"
return
}(),
newOperatorGroup("foo", namespace),
},
bundlesByCatalog: map[resolvercache.SourceKey][]*api.Bundle{
catalog: {
Expand All @@ -689,6 +704,7 @@ func TestResolver(t *testing.T) {
clusterState: []runtime.Object{
existingOperator("ns1", "a.v1", "a", "alpha", "", nil, nil, nil, nil),
existingOperator("ns2", "a.v1", "a", "alpha", "", nil, nil, nil, nil),
newOperatorGroup("foo", namespace),
},
out: nothing,
},
Expand All @@ -697,6 +713,7 @@ func TestResolver(t *testing.T) {
clusterState: []runtime.Object{
existingSub(namespace, "a.v1", "a", "alpha", catalog),
existingOperator(namespace, "a.v1", "a", "alpha", "", Provides1, nil, nil, nil),
newOperatorGroup("foo", namespace),
},
bundlesByCatalog: map[resolvercache.SourceKey][]*api.Bundle{
catalog: {
Expand All @@ -718,6 +735,7 @@ func TestResolver(t *testing.T) {
clusterState: []runtime.Object{
existingSub(namespace, "a.v1", "a", "alpha", catalog),
existingOperator(namespace, "a.v1", "a", "alpha", "", Provides1, nil, nil, nil),
newOperatorGroup("foo", namespace),
},
bundlesByCatalog: map[resolvercache.SourceKey][]*api.Bundle{catalog: {
stripManifests(withBundlePath(bundle("a.v2", "a", "alpha", "a.v1", Provides1, nil, nil, nil), "quay.io/test/bundle@sha256:abcd"))},
Expand Down Expand Up @@ -759,6 +777,7 @@ func TestResolver(t *testing.T) {
name: "InstalledSub/NoRunningOperator",
clusterState: []runtime.Object{
existingSub(namespace, "a.v1", "a", "alpha", catalog),
newOperatorGroup("foo", namespace),
},
bundlesByCatalog: map[resolvercache.SourceKey][]*api.Bundle{
catalog: {
Expand All @@ -778,6 +797,7 @@ func TestResolver(t *testing.T) {
clusterState: []runtime.Object{
existingSub(namespace, "a.v1", "a", "alpha", catalog),
existingOperator(namespace, "a.v1", "a", "alpha", "", Provides1, nil, nil, nil),
newOperatorGroup("foo", namespace),
},
bundlesByCatalog: map[resolvercache.SourceKey][]*api.Bundle{
catalog: {
Expand All @@ -802,6 +822,7 @@ func TestResolver(t *testing.T) {
clusterState: []runtime.Object{
existingSub(namespace, "a.v1", "a", "alpha", catalog),
existingOperator(namespace, "a.v1", "a", "alpha", "", nil, nil, Provides1, nil),
newOperatorGroup("foo", namespace),
},
bundlesByCatalog: map[resolvercache.SourceKey][]*api.Bundle{
catalog: {
Expand All @@ -827,6 +848,7 @@ func TestResolver(t *testing.T) {
existingSub(namespace, "a.v1", "a", "alpha", catalog),
existingOperator(namespace, "a.v1", "a", "alpha", "", Provides1, nil, nil, nil),
newSub(namespace, "b", "beta", catalog),
newOperatorGroup("foo", namespace),
},
bundlesByCatalog: map[resolvercache.SourceKey][]*api.Bundle{
catalog: {
Expand All @@ -851,6 +873,7 @@ func TestResolver(t *testing.T) {
clusterState: []runtime.Object{
existingSub(namespace, "a.v1", "a", "alpha", catalog),
newSub(namespace, "b", "beta", catalog),
newOperatorGroup("foo", namespace),
},
bundlesByCatalog: map[resolvercache.SourceKey][]*api.Bundle{
catalog: {
Expand All @@ -873,6 +896,7 @@ func TestResolver(t *testing.T) {
clusterState: []runtime.Object{
existingSub(namespace, "a.v1", "a", "alpha", catalog),
newSub(namespace, "b", "beta", catalog),
newOperatorGroup("foo", namespace),
},
bundlesByCatalog: map[resolvercache.SourceKey][]*api.Bundle{
catalog: {
Expand All @@ -898,6 +922,7 @@ func TestResolver(t *testing.T) {
existingOperator(namespace, "a.v1", "a", "alpha", "", Provides1, Requires2, nil, nil),
existingSub(namespace, "b.v1", "b", "alpha", catalog),
existingOperator(namespace, "b.v1", "b", "alpha", "", Provides2, Requires1, nil, nil),
newOperatorGroup("foo", namespace),
},
bundlesByCatalog: map[resolvercache.SourceKey][]*api.Bundle{
catalog: {
Expand All @@ -924,6 +949,7 @@ func TestResolver(t *testing.T) {
existingOperator(namespace, "a.v1", "a", "alpha", "", Provides1, nil, nil, nil),
existingSub(namespace, "b.v1", "b", "alpha", catalog),
existingOperator(namespace, "b.v1", "b", "alpha", "", nil, Requires1, nil, nil),
newOperatorGroup("foo", namespace),
},
bundlesByCatalog: map[resolvercache.SourceKey][]*api.Bundle{
catalog: {
Expand All @@ -946,6 +972,7 @@ func TestResolver(t *testing.T) {
name: "PicksOlderProvider",
clusterState: []runtime.Object{
newSub(namespace, "b", "alpha", catalog),
newOperatorGroup("foo", namespace),
},
bundlesByCatalog: map[resolvercache.SourceKey][]*api.Bundle{
catalog: {
Expand All @@ -970,6 +997,7 @@ func TestResolver(t *testing.T) {
clusterState: []runtime.Object{
existingSub(namespace, "a.v1", "a", "alpha", catalog),
existingOperator(namespace, "a.v1", "a", "alpha", "", Provides1, nil, nil, nil),
newOperatorGroup("foo", namespace),
},
bundlesByCatalog: map[resolvercache.SourceKey][]*api.Bundle{catalog: {
bundle("a.v3", "a", "alpha", "a.v2", nil, nil, nil, nil, withVersion("1.0.0"), withSkipRange("< 1.0.0")),
Expand All @@ -990,6 +1018,7 @@ func TestResolver(t *testing.T) {
existingSub(namespace, "b.v1", "b", "beta", catalog),
existingOperator(namespace, "a.v1", "a", "alpha", "", nil, Requires1, nil, nil),
existingOperator(namespace, "b.v1", "b", "beta", "", Provides1, nil, nil, nil),
newOperatorGroup("foo", namespace),
},
bundlesByCatalog: map[resolvercache.SourceKey][]*api.Bundle{
catalog: {
Expand All @@ -1015,6 +1044,7 @@ func TestResolver(t *testing.T) {
clusterState: []runtime.Object{
existingSub(namespace, "a.v1", "a", "alpha", catalog),
existingOperator(namespace, "a.v1", "a", "alpha", "", Provides1, nil, nil, nil),
newOperatorGroup("foo", namespace),
},
bundlesByCatalog: map[resolvercache.SourceKey][]*api.Bundle{catalog: {
bundle("a.v2", "a", "alpha", "", nil, nil, nil, nil, withVersion("1.0.0"), withSkipRange("< 1.0.0")),
Expand All @@ -1034,6 +1064,7 @@ func TestResolver(t *testing.T) {
name: "NewSub/StartingCSV",
clusterState: []runtime.Object{
newSub(namespace, "a", "alpha", catalog, withStartingCSV("a.v2")),
newOperatorGroup("foo", namespace),
},
bundlesByCatalog: map[resolvercache.SourceKey][]*api.Bundle{catalog: {
bundle("a.v1", "a", "alpha", "", nil, nil, nil, nil),
Expand All @@ -1054,6 +1085,7 @@ func TestResolver(t *testing.T) {
clusterState: []runtime.Object{
existingSub(namespace, "a.v1", "a", "alpha", catalog),
existingOperator(namespace, "a.v1", "a", "alpha", "", Provides1, nil, nil, nil),
newOperatorGroup("foo", namespace),
},
bundlesByCatalog: map[resolvercache.SourceKey][]*api.Bundle{catalog: {
bundle("a.v2", "a", "alpha", "", nil, nil, nil, nil, withVersion("1.0.0"), withSkips([]string{"a.v1"})),
Expand All @@ -1074,6 +1106,7 @@ func TestResolver(t *testing.T) {
existingSub(namespace, "a.v2", "a", "alpha", catalog),
existingOperator(namespace, "a.v1", "a", "alpha", "", Provides1, nil, nil, nil, withPhase(v1alpha1.CSVPhaseReplacing)),
existingOperator(namespace, "a.v2", "a", "alpha", "a.v1", Provides1, nil, nil, nil, withPhase(v1alpha1.CSVPhaseFailed)),
newOperatorGroup("foo", namespace),
},
bundlesByCatalog: map[resolvercache.SourceKey][]*api.Bundle{catalog: {
bundle("a.v1", "a", "alpha", "", Provides1, nil, nil, nil, withVersion("1.0.0")),
Expand Down Expand Up @@ -1121,6 +1154,7 @@ func TestResolver(t *testing.T) {
existingOperator(namespace, "a.v1", "a", "alpha", "", Provides1, nil, nil, nil, withPhase(v1alpha1.CSVPhaseReplacing)),
existingOperator(namespace, "a.v2", "a", "alpha", "a.v1", Provides1, nil, nil, nil, withPhase(v1alpha1.CSVPhaseReplacing)),
existingOperator(namespace, "a.v3", "a", "alpha", "a.v2", Provides1, nil, nil, nil, withPhase(v1alpha1.CSVPhaseFailed)),
newOperatorGroup("foo", namespace),
},
bundlesByCatalog: map[resolvercache.SourceKey][]*api.Bundle{catalog: {
bundle("a.v1", "a", "alpha", "", Provides1, nil, nil, nil, withVersion("1.0.0")),
Expand Down

0 comments on commit 08a95ad

Please sign in to comment.