Skip to content

Commit 1ccaab9

Browse files
anik120anik120
committed
fix(sub): Reset ResolutionFailed cond when error is resolved
In #2269, a new condition was introduced for Subscription to indicate any dependency resolution error in it's message. However, when the case of the error was resolved, the condition status (true) and the message was sticking around. This PR fixes the issue, and makes sure that the condition status is set to false, and the message and reason of the condition are cleared off. Signed-off-by: Anik Bhattacharjee <[email protected]>
1 parent 455975c commit 1ccaab9

File tree

3 files changed

+96
-16
lines changed

3 files changed

+96
-16
lines changed

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

+16-11
Original file line numberDiff line numberDiff line change
@@ -976,22 +976,23 @@ func (o *Operator) syncResolvingNamespace(obj interface{}) error {
976976
// not-satisfiable error
977977
if _, ok := err.(solver.NotSatisfiable); ok {
978978
logger.WithError(err).Debug("resolution failed")
979-
updateErr := o.setSubsCond(subs, v1alpha1.SubscriptionResolutionFailed, "ConstraintsNotSatisfiable", err.Error(), true)
979+
_, updateErr := o.setSubsCond(subs, v1alpha1.SubscriptionResolutionFailed, "ConstraintsNotSatisfiable", err.Error(), true)
980980
if updateErr != nil {
981981
logger.WithError(updateErr).Debug("failed to update subs conditions")
982982
}
983983
return nil
984984
}
985-
updateErr := o.setSubsCond(subs, v1alpha1.SubscriptionResolutionFailed, "ErrorPreventedResolution", err.Error(), true)
985+
_, updateErr := o.setSubsCond(subs, v1alpha1.SubscriptionResolutionFailed, "ErrorPreventedResolution", err.Error(), true)
986986
if updateErr != nil {
987987
logger.WithError(updateErr).Debug("failed to update subs conditions")
988988
}
989989
return err
990990
} else {
991-
updateErr := o.setSubsCond(subs, v1alpha1.SubscriptionResolutionFailed, "", "", false)
991+
subs, updateErr := o.setSubsCond(updatedSubs, v1alpha1.SubscriptionResolutionFailed, "", "", false)
992992
if updateErr != nil {
993993
logger.WithError(updateErr).Debug("failed to update subs conditions")
994994
}
995+
updatedSubs = subs
995996
}
996997

997998
// create installplan if anything updated
@@ -1269,7 +1270,7 @@ func (o *Operator) createInstallPlan(namespace string, gen int, subs []*v1alpha1
12691270
return reference.GetReference(res)
12701271
}
12711272

1272-
func (o *Operator) setSubsCond(subs []*v1alpha1.Subscription, condType v1alpha1.SubscriptionConditionType, reason, message string, setTrue bool) error {
1273+
func (o *Operator) setSubsCond(subs []*v1alpha1.Subscription, condType v1alpha1.SubscriptionConditionType, reason, message string, setTrue bool) ([]*v1alpha1.Subscription, error) {
12731274
var (
12741275
errs []error
12751276
mu sync.Mutex
@@ -1284,26 +1285,30 @@ func (o *Operator) setSubsCond(subs []*v1alpha1.Subscription, condType v1alpha1.
12841285
cond.Reason = reason
12851286
cond.Message = message
12861287
if setTrue {
1288+
if cond.Status == corev1.ConditionTrue {
1289+
continue
1290+
}
12871291
cond.Status = corev1.ConditionTrue
12881292
} else {
1293+
if cond.Status == corev1.ConditionFalse {
1294+
continue
1295+
}
12891296
cond.Status = corev1.ConditionFalse
12901297
}
12911298
sub.Status.SetCondition(cond)
12921299

12931300
wg.Add(1)
1294-
go func(s v1alpha1.Subscription) {
1301+
go func(sub v1alpha1.Subscription) {
12951302
defer wg.Done()
12961303

12971304
update := func() error {
12981305
// Update the status of the latest revision
1299-
latest, err := o.client.OperatorsV1alpha1().Subscriptions(s.GetNamespace()).Get(context.TODO(), s.GetName(), getOpts)
1306+
latest, err := o.client.OperatorsV1alpha1().Subscriptions(sub.GetNamespace()).Get(context.TODO(), sub.GetName(), getOpts)
13001307
if err != nil {
13011308
return err
13021309
}
1303-
1304-
latest.Status = s.Status
1305-
_, err = o.client.OperatorsV1alpha1().Subscriptions(s.Namespace).UpdateStatus(context.TODO(), latest, updateOpts)
1306-
1310+
latest.Status = sub.Status
1311+
_, err = o.client.OperatorsV1alpha1().Subscriptions(sub.Namespace).UpdateStatus(context.TODO(), latest, updateOpts)
13071312
return err
13081313
}
13091314
if err := retry.RetryOnConflict(retry.DefaultRetry, update); err != nil {
@@ -1315,7 +1320,7 @@ func (o *Operator) setSubsCond(subs []*v1alpha1.Subscription, condType v1alpha1.
13151320
}
13161321
wg.Wait()
13171322

1318-
return utilerrors.NewAggregate(errs)
1323+
return subs, utilerrors.NewAggregate(errs)
13191324
}
13201325

13211326
type UnpackedBundleReference struct {

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

+48
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,14 @@ func TestSyncSubscriptions(t *testing.T) {
155155
},
156156
LastUpdated: now,
157157
InstallPlanGeneration: 1,
158+
Conditions: []v1alpha1.SubscriptionCondition{
159+
{
160+
Type: "ResolutionFailed",
161+
Status: corev1.ConditionFalse,
162+
Reason: "",
163+
Message: "",
164+
},
165+
},
158166
},
159167
},
160168
},
@@ -298,6 +306,14 @@ func TestSyncSubscriptions(t *testing.T) {
298306
},
299307
LastUpdated: now,
300308
InstallPlanGeneration: 1,
309+
Conditions: []v1alpha1.SubscriptionCondition{
310+
{
311+
Type: "ResolutionFailed",
312+
Status: corev1.ConditionFalse,
313+
Reason: "",
314+
Message: "",
315+
},
316+
},
301317
},
302318
},
303319
},
@@ -446,6 +462,14 @@ func TestSyncSubscriptions(t *testing.T) {
446462
},
447463
InstallPlanGeneration: 1,
448464
LastUpdated: now,
465+
Conditions: []v1alpha1.SubscriptionCondition{
466+
{
467+
Type: "ResolutionFailed",
468+
Status: corev1.ConditionFalse,
469+
Reason: "",
470+
Message: "",
471+
},
472+
},
449473
},
450474
},
451475
},
@@ -599,6 +623,14 @@ func TestSyncSubscriptions(t *testing.T) {
599623
},
600624
LastUpdated: now,
601625
InstallPlanGeneration: 1,
626+
Conditions: []v1alpha1.SubscriptionCondition{
627+
{
628+
Type: "ResolutionFailed",
629+
Status: corev1.ConditionFalse,
630+
Reason: "",
631+
Message: "",
632+
},
633+
},
602634
},
603635
},
604636
},
@@ -775,6 +807,14 @@ func TestSyncSubscriptions(t *testing.T) {
775807
},
776808
LastUpdated: now,
777809
InstallPlanGeneration: 1,
810+
Conditions: []v1alpha1.SubscriptionCondition{
811+
{
812+
Type: "ResolutionFailed",
813+
Status: corev1.ConditionFalse,
814+
Reason: "",
815+
Message: "",
816+
},
817+
},
778818
},
779819
},
780820
},
@@ -958,6 +998,14 @@ func TestSyncSubscriptions(t *testing.T) {
958998
},
959999
LastUpdated: now,
9601000
InstallPlanGeneration: 2,
1001+
Conditions: []v1alpha1.SubscriptionCondition{
1002+
{
1003+
Type: "ResolutionFailed",
1004+
Status: corev1.ConditionFalse,
1005+
Reason: "",
1006+
Message: "",
1007+
},
1008+
},
9611009
},
9621010
},
9631011
},

Diff for: ‎test/e2e/subscription_e2e_test.go

+32-5
Original file line numberDiff line numberDiff line change
@@ -2113,8 +2113,11 @@ var _ = Describe("Subscription", func() {
21132113
teardown func()
21142114
cleanup func()
21152115
packages []registry.PackageManifest
2116-
subName = genName("test-subscription")
2117-
catSrcName = genName("test-catalog")
2116+
crd = newCRD(genName("foo-"))
2117+
csvA operatorsv1alpha1.ClusterServiceVersion
2118+
csvB operatorsv1alpha1.ClusterServiceVersion
2119+
subName = genName("test-subscription-")
2120+
catSrcName = genName("test-catalog-")
21182121
)
21192122

21202123
BeforeEach(func() {
@@ -2130,10 +2133,9 @@ var _ = Describe("Subscription", func() {
21302133
DefaultChannelName: "alpha",
21312134
},
21322135
}
2133-
crd := newCRD(genName("foo"))
2134-
csv := newCSV("csvA", testNamespace, "", semver.MustParse("1.0.0"), nil, []apiextensions.CustomResourceDefinition{crd}, nil)
2136+
csvA = newCSV("csvA", testNamespace, "", semver.MustParse("1.0.0"), nil, []apiextensions.CustomResourceDefinition{crd}, nil)
21352137

2136-
_, teardown = createInternalCatalogSource(c, ctx.Ctx().OperatorClient(), catSrcName, testNamespace, packages, nil, []operatorsv1alpha1.ClusterServiceVersion{csv})
2138+
_, teardown = createInternalCatalogSource(c, ctx.Ctx().OperatorClient(), catSrcName, testNamespace, packages, nil, []operatorsv1alpha1.ClusterServiceVersion{csvA})
21372139

21382140
// Ensure that the catalog source is resolved before we create a subscription.
21392141
_, err := fetchCatalogSourceOnStatus(crc, catSrcName, testNamespace, catalogSourceRegistryPodSynced)
@@ -2157,6 +2159,31 @@ var _ = Describe("Subscription", func() {
21572159
}).Should(Equal(corev1.ConditionTrue))
21582160
})
21592161

2162+
When("the required API is made available", func() {
2163+
BeforeEach(func() {
2164+
newPkg := registry.PackageManifest{
2165+
PackageName: "PackageB",
2166+
Channels: []registry.PackageChannel{
2167+
{Name: "alpha", CurrentCSVName: "csvB"},
2168+
},
2169+
DefaultChannelName: "alpha",
2170+
}
2171+
packages = append(packages, newPkg)
2172+
2173+
csvB = newCSV("csvB", testNamespace, "", semver.MustParse("1.0.0"), []apiextensions.CustomResourceDefinition{crd}, nil, nil)
2174+
2175+
updateInternalCatalog(GinkgoT(), c, crc, catSrcName, testNamespace, []apiextensions.CustomResourceDefinition{crd}, []operatorsv1alpha1.ClusterServiceVersion{csvA, csvB}, packages)
2176+
})
2177+
It("the ResolutionFailed condition previously set in it's status that indicated the resolution error is cleared off", func() {
2178+
Eventually(func() (corev1.ConditionStatus, error) {
2179+
sub, err := crc.OperatorsV1alpha1().Subscriptions(testNamespace).Get(context.Background(), subName, metav1.GetOptions{})
2180+
if err != nil {
2181+
return corev1.ConditionUnknown, err
2182+
}
2183+
return sub.Status.GetCondition(operatorsv1alpha1.SubscriptionResolutionFailed).Status, nil
2184+
}).Should(Equal(corev1.ConditionFalse))
2185+
})
2186+
})
21602187
})
21612188

21622189
When("an unannotated ClusterServiceVersion exists with an associated Subscription", func() {

0 commit comments

Comments
 (0)