Skip to content

Commit cc816eb

Browse files
n3wscottpmorie
n3wscott
authored and
pmorie
committed
When a SI is unbindable, mark the binding request failed. (openshift#1522)
1 parent 41290e9 commit cc816eb

File tree

4 files changed

+55
-38
lines changed

4 files changed

+55
-38
lines changed

‎pkg/controller/controller_binding.go

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,14 @@ func (c *controller) reconcileServiceBinding(binding *v1beta1.ServiceBinding) er
254254
errorNonbindableClusterServiceClassReason,
255255
s,
256256
)
257+
setServiceBindingCondition(
258+
toUpdate,
259+
v1beta1.ServiceBindingConditionFailed,
260+
v1beta1.ConditionTrue,
261+
errorNonbindableClusterServiceClassReason,
262+
s,
263+
)
264+
clearServiceBindingCurrentOperation(toUpdate)
257265
if _, err := c.updateServiceBindingStatus(toUpdate); err != nil {
258266
return err
259267
}
@@ -461,7 +469,7 @@ func (c *controller) reconcileServiceBinding(binding *v1beta1.ServiceBinding) er
461469
v1beta1.ConditionFalse,
462470
errorBindCallReason,
463471
"Bind call failed. "+s)
464-
c.clearServiceBindingCurrentOperation(toUpdate)
472+
clearServiceBindingCurrentOperation(toUpdate)
465473
if _, err := c.updateServiceBindingStatus(toUpdate); err != nil {
466474
return err
467475
}
@@ -490,7 +498,7 @@ func (c *controller) reconcileServiceBinding(binding *v1beta1.ServiceBinding) er
490498
v1beta1.ConditionTrue,
491499
errorReconciliationRetryTimeoutReason,
492500
s)
493-
c.clearServiceBindingCurrentOperation(toUpdate)
501+
clearServiceBindingCurrentOperation(toUpdate)
494502
if _, err := c.updateServiceBindingStatus(toUpdate); err != nil {
495503
return err
496504
}
@@ -546,7 +554,7 @@ func (c *controller) reconcileServiceBinding(binding *v1beta1.ServiceBinding) er
546554
return err
547555
}
548556

549-
c.clearServiceBindingCurrentOperation(toUpdate)
557+
clearServiceBindingCurrentOperation(toUpdate)
550558

551559
setServiceBindingCondition(
552560
toUpdate,
@@ -662,7 +670,7 @@ func (c *controller) reconcileServiceBinding(binding *v1beta1.ServiceBinding) er
662670
errorUnbindCallReason,
663671
"Unbind call failed. "+s)
664672
}
665-
c.clearServiceBindingCurrentOperation(toUpdate)
673+
clearServiceBindingCurrentOperation(toUpdate)
666674
if _, err := c.updateServiceBindingStatus(toUpdate); err != nil {
667675
return err
668676
}
@@ -696,7 +704,7 @@ func (c *controller) reconcileServiceBinding(binding *v1beta1.ServiceBinding) er
696704
errorReconciliationRetryTimeoutReason,
697705
s)
698706
}
699-
c.clearServiceBindingCurrentOperation(toUpdate)
707+
clearServiceBindingCurrentOperation(toUpdate)
700708
if _, err := c.updateServiceBindingStatus(toUpdate); err != nil {
701709
return err
702710
}
@@ -731,7 +739,7 @@ func (c *controller) reconcileServiceBinding(binding *v1beta1.ServiceBinding) er
731739
}
732740

733741
toUpdate.Status.ExternalProperties = nil
734-
c.clearServiceBindingCurrentOperation(toUpdate)
742+
clearServiceBindingCurrentOperation(toUpdate)
735743
if _, err := c.updateServiceBindingStatus(toUpdate); err != nil {
736744
return err
737745
}
@@ -995,7 +1003,7 @@ func (c *controller) recordStartOfServiceBindingOperation(toUpdate *v1beta1.Serv
9951003
// clearServiceBindingCurrentOperation sets the fields of the binding's
9961004
// Status to indicate that there is no current operation being performed. The
9971005
// Status is *not* recorded in the registry.
998-
func (c *controller) clearServiceBindingCurrentOperation(toUpdate *v1beta1.ServiceBinding) {
1006+
func clearServiceBindingCurrentOperation(toUpdate *v1beta1.ServiceBinding) {
9991007
toUpdate.Status.CurrentOperation = ""
10001008
toUpdate.Status.OperationStartTime = nil
10011009
toUpdate.Status.ReconciledGeneration = toUpdate.Generation

‎pkg/controller/controller_binding_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -527,11 +527,11 @@ func TestReconcileServiceBindingNonbindableClusterServiceClass(t *testing.T) {
527527

528528
// There should only be one action that says binding was created
529529
updatedServiceBinding := assertUpdateStatus(t, actions[0], binding)
530-
assertServiceBindingErrorBeforeRequest(t, updatedServiceBinding, errorNonbindableClusterServiceClassReason, binding)
530+
assertServiceBindingFailedBeforeRequest(t, updatedServiceBinding, errorNonbindableClusterServiceClassReason, binding)
531531
assertServiceBindingOrphanMitigationSet(t, updatedServiceBinding, false)
532+
assertServiceBindingReconciledGeneration(t, updatedServiceBinding, binding.Generation)
532533

533534
events := getRecordedEvents(testController)
534-
assertNumEvents(t, events, 1)
535535

536536
expectedEvent := warningEventBuilder(errorNonbindableClusterServiceClassReason).msgf(
537537
"References a non-bindable ClusterServiceClass (K8S: %q ExternalName: %q) and Plan (%q) combination",
@@ -692,7 +692,7 @@ func TestReconcileServiceBindingBindableClusterServiceClassNonbindablePlan(t *te
692692

693693
// There should only be one action that says binding was created
694694
updatedServiceBinding := assertUpdateStatus(t, actions[0], binding)
695-
assertServiceBindingErrorBeforeRequest(t, updatedServiceBinding, errorNonbindableClusterServiceClassReason, binding)
695+
assertServiceBindingFailedBeforeRequest(t, updatedServiceBinding, errorNonbindableClusterServiceClassReason, binding)
696696
assertServiceBindingOrphanMitigationSet(t, updatedServiceBinding, false)
697697

698698
events := getRecordedEvents(testController)

‎pkg/controller/controller_instance.go

Lines changed: 28 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -324,7 +324,7 @@ func (c *controller) reconcileServiceInstanceDelete(instance *v1beta1.ServiceIns
324324
s)
325325
}
326326

327-
c.clearServiceInstanceCurrentOperation(toUpdate)
327+
clearServiceInstanceCurrentOperation(toUpdate)
328328
toUpdate.Status.DeprovisionStatus = v1beta1.ServiceInstanceDeprovisionStatusFailed
329329

330330
if _, err := c.updateServiceInstanceStatus(toUpdate); err != nil {
@@ -466,7 +466,7 @@ func (c *controller) reconcileServiceInstanceDelete(instance *v1beta1.ServiceIns
466466
s)
467467
}
468468

469-
c.clearServiceInstanceCurrentOperation(toUpdate)
469+
clearServiceInstanceCurrentOperation(toUpdate)
470470
toUpdate.Status.DeprovisionStatus = v1beta1.ServiceInstanceDeprovisionStatusFailed
471471
if _, err := c.updateServiceInstanceStatus(toUpdate); err != nil {
472472
return err
@@ -517,7 +517,7 @@ func (c *controller) reconcileServiceInstanceDelete(instance *v1beta1.ServiceIns
517517

518518
glog.V(5).Info(pcb.Message("Deprovision call to broker succeeded, finalizing"))
519519

520-
c.clearServiceInstanceCurrentOperation(toUpdate)
520+
clearServiceInstanceCurrentOperation(toUpdate)
521521
toUpdate.Status.ExternalProperties = nil
522522
toUpdate.Status.DeprovisionStatus = v1beta1.ServiceInstanceDeprovisionStatusSucceeded
523523

@@ -877,7 +877,7 @@ func (c *controller) reconcileServiceInstance(instance *v1beta1.ServiceInstance)
877877
reason,
878878
fmt.Sprintf("ClusterServiceBroker returned a failure for %v call; operation will not be retried: %v", provisionOrUpdateText, s))
879879

880-
c.clearServiceInstanceCurrentOperation(toUpdate)
880+
clearServiceInstanceCurrentOperation(toUpdate)
881881

882882
if _, err := c.updateServiceInstanceStatus(toUpdate); err != nil {
883883
return err
@@ -924,7 +924,7 @@ func (c *controller) reconcileServiceInstance(instance *v1beta1.ServiceInstance)
924924
v1beta1.ConditionFalse,
925925
reason,
926926
message)
927-
c.clearServiceInstanceCurrentOperation(toUpdate)
927+
clearServiceInstanceCurrentOperation(toUpdate)
928928
}
929929

930930
if _, err := c.updateServiceInstanceStatus(toUpdate); err != nil {
@@ -958,7 +958,7 @@ func (c *controller) reconcileServiceInstance(instance *v1beta1.ServiceInstance)
958958
errorReconciliationRetryTimeoutReason,
959959
s)
960960
}
961-
c.clearServiceInstanceCurrentOperation(toUpdate)
961+
clearServiceInstanceCurrentOperation(toUpdate)
962962
if _, err := c.updateServiceInstanceStatus(toUpdate); err != nil {
963963
return err
964964
}
@@ -1047,7 +1047,7 @@ func (c *controller) reconcileServiceInstance(instance *v1beta1.ServiceInstance)
10471047
))
10481048

10491049
toUpdate.Status.ExternalProperties = toUpdate.Status.InProgressProperties
1050-
c.clearServiceInstanceCurrentOperation(toUpdate)
1050+
clearServiceInstanceCurrentOperation(toUpdate)
10511051

10521052
// TODO: process response
10531053
setServiceInstanceCondition(
@@ -1124,7 +1124,7 @@ func (c *controller) pollServiceInstance(instance *v1beta1.ServiceInstance) erro
11241124
}
11251125

11261126
if !provisioning {
1127-
c.clearServiceInstanceCurrentOperation(toUpdate)
1127+
clearServiceInstanceCurrentOperation(toUpdate)
11281128
toUpdate.Status.DeprovisionStatus = v1beta1.ServiceInstanceDeprovisionStatusFailed
11291129
} else {
11301130
c.setServiceInstanceStartOrphanMitigation(toUpdate)
@@ -1198,7 +1198,7 @@ func (c *controller) pollServiceInstance(instance *v1beta1.ServiceInstance) erro
11981198
message = successDeprovisionMessage
11991199
}
12001200

1201-
c.clearServiceInstanceCurrentOperation(toUpdate)
1201+
clearServiceInstanceCurrentOperation(toUpdate)
12021202
toUpdate.Status.ExternalProperties = nil
12031203
toUpdate.Status.DeprovisionStatus = v1beta1.ServiceInstanceDeprovisionStatusSucceeded
12041204

@@ -1279,7 +1279,7 @@ func (c *controller) pollServiceInstance(instance *v1beta1.ServiceInstance) erro
12791279
}
12801280

12811281
if !provisioning {
1282-
c.clearServiceInstanceCurrentOperation(toUpdate)
1282+
clearServiceInstanceCurrentOperation(toUpdate)
12831283
} else {
12841284
c.setServiceInstanceStartOrphanMitigation(toUpdate)
12851285
}
@@ -1377,7 +1377,7 @@ func (c *controller) pollServiceInstance(instance *v1beta1.ServiceInstance) erro
13771377
}
13781378

13791379
if !provisioning {
1380-
c.clearServiceInstanceCurrentOperation(toUpdate)
1380+
clearServiceInstanceCurrentOperation(toUpdate)
13811381
} else {
13821382
c.setServiceInstanceStartOrphanMitigation(toUpdate)
13831383
}
@@ -1443,7 +1443,7 @@ func (c *controller) pollServiceInstance(instance *v1beta1.ServiceInstance) erro
14431443
toUpdate := clone.(*v1beta1.ServiceInstance)
14441444

14451445
toUpdate.Status.ExternalProperties = toUpdate.Status.InProgressProperties
1446-
c.clearServiceInstanceCurrentOperation(toUpdate)
1446+
clearServiceInstanceCurrentOperation(toUpdate)
14471447
if deleting {
14481448
toUpdate.Status.DeprovisionStatus = v1beta1.ServiceInstanceDeprovisionStatusSucceeded
14491449
}
@@ -1509,7 +1509,7 @@ func (c *controller) pollServiceInstance(instance *v1beta1.ServiceInstance) erro
15091509
toUpdate := clone.(*v1beta1.ServiceInstance)
15101510

15111511
if !deleting {
1512-
c.clearServiceInstanceCurrentOperation(toUpdate)
1512+
clearServiceInstanceCurrentOperation(toUpdate)
15131513
setServiceInstanceCondition(
15141514
toUpdate,
15151515
v1beta1.ServiceInstanceConditionReady,
@@ -1537,7 +1537,7 @@ func (c *controller) pollServiceInstance(instance *v1beta1.ServiceInstance) erro
15371537
glog.Info(pcb.Message(s))
15381538
c.recorder.Event(instance, corev1.EventTypeWarning, errorReconciliationRetryTimeoutReason, s)
15391539

1540-
c.clearServiceInstanceCurrentOperation(toUpdate)
1540+
clearServiceInstanceCurrentOperation(toUpdate)
15411541
toUpdate.Status.DeprovisionStatus = v1beta1.ServiceInstanceDeprovisionStatusFailed
15421542

15431543
setServiceInstanceCondition(
@@ -1621,7 +1621,7 @@ func (c *controller) pollServiceInstance(instance *v1beta1.ServiceInstance) erro
16211621
}
16221622

16231623
if !provisioning {
1624-
c.clearServiceInstanceCurrentOperation(toUpdate)
1624+
clearServiceInstanceCurrentOperation(toUpdate)
16251625
} else {
16261626
c.setServiceInstanceStartOrphanMitigation(toUpdate)
16271627
}
@@ -1980,19 +1980,6 @@ func (c *controller) recordStartOfServiceInstanceOperation(toUpdate *v1beta1.Ser
19801980
return c.updateServiceInstanceStatus(toUpdate)
19811981
}
19821982

1983-
// clearServiceInstanceCurrentOperation sets the fields of the instance's Status
1984-
// to indicate that there is no current operation being performed. The Status
1985-
// is *not* recorded in the registry.
1986-
func (c *controller) clearServiceInstanceCurrentOperation(toUpdate *v1beta1.ServiceInstance) {
1987-
toUpdate.Status.CurrentOperation = ""
1988-
toUpdate.Status.OperationStartTime = nil
1989-
toUpdate.Status.AsyncOpInProgress = false
1990-
toUpdate.Status.OrphanMitigationInProgress = false
1991-
toUpdate.Status.LastOperation = nil
1992-
toUpdate.Status.InProgressProperties = nil
1993-
toUpdate.Status.ReconciledGeneration = toUpdate.Generation
1994-
}
1995-
19961983
// setServiceInstanceStartOrphanMitigation sets the fields of the instance's
19971984
// Status to indicate that orphan mitigation is starting. The Status is *not*
19981985
// recorded in the registry.
@@ -2083,6 +2070,19 @@ func (c *controller) checkForRemovedClassAndPlan(instance *v1beta1.ServiceInstan
20832070
return fmt.Errorf(s)
20842071
}
20852072

2073+
// clearServiceInstanceCurrentOperation sets the fields of the instance's Status
2074+
// to indicate that there is no current operation being performed. The Status
2075+
// is *not* recorded in the registry.
2076+
func clearServiceInstanceCurrentOperation(toUpdate *v1beta1.ServiceInstance) {
2077+
toUpdate.Status.CurrentOperation = ""
2078+
toUpdate.Status.OperationStartTime = nil
2079+
toUpdate.Status.AsyncOpInProgress = false
2080+
toUpdate.Status.OrphanMitigationInProgress = false
2081+
toUpdate.Status.LastOperation = nil
2082+
toUpdate.Status.InProgressProperties = nil
2083+
toUpdate.Status.ReconciledGeneration = toUpdate.Generation
2084+
}
2085+
20862086
// shouldStartOrphanMitigation returns whether an error with the given status
20872087
// code indicates that orphan migitation should start.
20882088
func shouldStartOrphanMitigation(statusCode int) bool {

‎pkg/controller/controller_test.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2274,6 +2274,15 @@ func assertServiceBindingErrorBeforeRequest(t *testing.T, obj runtime.Object, re
22742274
assertServiceBindingExternalPropertiesUnchanged(t, obj, originalBinding)
22752275
}
22762276

2277+
func assertServiceBindingFailedBeforeRequest(t *testing.T, obj runtime.Object, reason string, originalBinding *v1beta1.ServiceBinding) {
2278+
assertServiceBindingReadyFalse(t, obj, reason)
2279+
assertServiceBindingCurrentOperationClear(t, obj)
2280+
assertServiceBindingOperationStartTimeSet(t, obj, false)
2281+
assertServiceBindingReconciledGeneration(t, obj, originalBinding.Generation)
2282+
assertServiceBindingInProgressPropertiesNil(t, obj)
2283+
assertServiceBindingExternalPropertiesUnchanged(t, obj, originalBinding)
2284+
}
2285+
22772286
func assertServiceBindingOperationInProgress(t *testing.T, obj runtime.Object, operation v1beta1.ServiceBindingOperation, originalBinding *v1beta1.ServiceBinding) {
22782287
assertServiceBindingOperationInProgressWithParameters(t, obj, operation, nil, "", originalBinding)
22792288
}

0 commit comments

Comments
 (0)