Skip to content

Commit 38911d2

Browse files
author
Mikalai Radchuk
committed
Improve InstallPlan error handling
With this change we no set condition on subscriptions about errors during InstallPlan creation. Previously we were only logging these errors. Signed-off-by: Mikalai Radchuk <[email protected]>
1 parent 3030064 commit 38911d2

File tree

2 files changed

+126
-1
lines changed

2 files changed

+126
-1
lines changed

Diff for: pkg/controller/operators/catalog/operator.go

+14-1
Original file line numberDiff line numberDiff line change
@@ -1442,7 +1442,20 @@ func (o *Operator) syncResolvingNamespace(obj interface{}) error {
14421442

14431443
installPlanReference, err := o.ensureInstallPlan(logger, namespace, maxGeneration+1, subs, installPlanApproval, steps, bundleLookups)
14441444
if err != nil {
1445-
logger.WithError(err).Debug("error ensuring installplan")
1445+
err := fmt.Errorf("error ensuring InstallPlan: %s", err)
1446+
logger.Infof("%v", err)
1447+
1448+
_, updateErr := o.updateSubscriptionStatuses(
1449+
o.setSubsCond(subs, v1alpha1.SubscriptionCondition{
1450+
Type: v1alpha1.SubscriptionBundleUnpackFailed,
1451+
Reason: "EnsureInstallPlanFailed",
1452+
Message: err.Error(),
1453+
Status: corev1.ConditionTrue,
1454+
}))
1455+
if updateErr != nil {
1456+
logger.WithError(updateErr).Debug("failed to update subs conditions")
1457+
return updateErr
1458+
}
14461459
return err
14471460
}
14481461
updatedSubs = o.setIPReference(updatedSubs, maxGeneration+1, installPlanReference)

Diff for: pkg/controller/operators/catalog/subscriptions_test.go

+112
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1414
"k8s.io/apimachinery/pkg/runtime"
1515
utilerrors "k8s.io/apimachinery/pkg/util/errors"
16+
clitesting "k8s.io/client-go/testing"
1617
utilclocktesting "k8s.io/utils/clock/testing"
1718

1819
operatorsv1 "github.com/operator-framework/api/pkg/operators/v1"
@@ -570,6 +571,117 @@ func TestSyncSubscriptions(t *testing.T) {
570571
},
571572
},
572573
},
574+
{
575+
name: "NoStatus/NoCurrentCSV/EnsureInstallPlanFailed",
576+
fields: fields{
577+
clientOptions: []clientfake.Option{
578+
clientfake.WithSelfLinks(t),
579+
func(c clientfake.ClientsetDecorator) {
580+
c.PrependReactor("create", "installplans", func(a clitesting.Action) (bool, runtime.Object, error) {
581+
return true, nil, errors.New("fake install plan create error")
582+
})
583+
},
584+
},
585+
existingOLMObjs: []runtime.Object{
586+
&v1alpha1.Subscription{
587+
TypeMeta: metav1.TypeMeta{
588+
Kind: v1alpha1.SubscriptionKind,
589+
APIVersion: v1alpha1.SchemeGroupVersion.String(),
590+
},
591+
ObjectMeta: metav1.ObjectMeta{
592+
Name: "sub",
593+
Namespace: testNamespace,
594+
},
595+
Spec: &v1alpha1.SubscriptionSpec{
596+
CatalogSource: "src",
597+
CatalogSourceNamespace: testNamespace,
598+
},
599+
Status: v1alpha1.SubscriptionStatus{
600+
CurrentCSV: "",
601+
State: "",
602+
},
603+
},
604+
},
605+
resolveSteps: []*v1alpha1.Step{
606+
{
607+
Resolving: "csv.v.2",
608+
Resource: v1alpha1.StepResource{
609+
CatalogSource: "src",
610+
CatalogSourceNamespace: testNamespace,
611+
Group: v1alpha1.GroupName,
612+
Version: v1alpha1.GroupVersion,
613+
Kind: v1alpha1.ClusterServiceVersionKind,
614+
Name: "csv.v.2",
615+
Manifest: "{}",
616+
},
617+
},
618+
},
619+
resolveSubs: []*v1alpha1.Subscription{
620+
{
621+
TypeMeta: metav1.TypeMeta{
622+
Kind: v1alpha1.SubscriptionKind,
623+
APIVersion: v1alpha1.SchemeGroupVersion.String(),
624+
},
625+
ObjectMeta: metav1.ObjectMeta{
626+
Name: "sub",
627+
Namespace: testNamespace,
628+
},
629+
Spec: &v1alpha1.SubscriptionSpec{
630+
CatalogSource: "src",
631+
CatalogSourceNamespace: testNamespace,
632+
},
633+
},
634+
},
635+
},
636+
args: args{
637+
obj: &v1alpha1.Subscription{
638+
TypeMeta: metav1.TypeMeta{
639+
Kind: v1alpha1.SubscriptionKind,
640+
APIVersion: v1alpha1.SchemeGroupVersion.String(),
641+
},
642+
ObjectMeta: metav1.ObjectMeta{
643+
Name: "sub",
644+
Namespace: testNamespace,
645+
},
646+
Spec: &v1alpha1.SubscriptionSpec{
647+
CatalogSource: "src",
648+
CatalogSourceNamespace: testNamespace,
649+
},
650+
Status: v1alpha1.SubscriptionStatus{
651+
CurrentCSV: "",
652+
State: "",
653+
},
654+
},
655+
},
656+
wantSubscriptions: []*v1alpha1.Subscription{
657+
{
658+
TypeMeta: metav1.TypeMeta{
659+
Kind: v1alpha1.SubscriptionKind,
660+
APIVersion: v1alpha1.SubscriptionCRDAPIVersion,
661+
},
662+
ObjectMeta: metav1.ObjectMeta{
663+
Name: "sub",
664+
Namespace: testNamespace,
665+
},
666+
Spec: &v1alpha1.SubscriptionSpec{
667+
CatalogSource: "src",
668+
CatalogSourceNamespace: testNamespace,
669+
},
670+
Status: v1alpha1.SubscriptionStatus{
671+
LastUpdated: now,
672+
Conditions: []v1alpha1.SubscriptionCondition{
673+
{
674+
Type: v1alpha1.SubscriptionBundleUnpackFailed,
675+
Reason: "EnsureInstallPlanFailed",
676+
Message: "error ensuring InstallPlan: fake install plan create error",
677+
Status: corev1.ConditionTrue,
678+
},
679+
},
680+
},
681+
},
682+
},
683+
wantErr: errors.New("error ensuring InstallPlan: fake install plan create error"),
684+
},
573685
{
574686
name: "NoStatus/NoCurrentCSV/BundleLookupError",
575687
fields: fields{

0 commit comments

Comments
 (0)