address comments

charts/catalog/README.md

@@ -55,6 +55,7 @@ chart and their default values.
 | `controllerManager.brokerRelistInterval` | How often the controller should relist the catalogs of ready brokers; duration format (`20m`, `1h`, etc) | `24h` |
 | `useAggregator` | whether or not to set up the controller-manager to go through the main Kubernetes API server's API aggregator | `true` |
 | `rbacEnable` | If true, create & use RBAC resources | `true` |
+| `asyncBindingOperationsEnabled` | Whether or not alpha support for async binding operations is enabled | `false` |
 
 Specify each parameter using the `--set key=value[,key=value]` argument to
 `helm install`.

pkg/controller/controller.go

@@ -176,7 +176,10 @@ func (c *controller) Run(workers int, stopCh <-chan struct{}) {
 		createWorker(c.instanceQueue, "ServiceInstance", maxRetries, true, c.reconcileServiceInstanceKey, stopCh, &waitGroup)
 		createWorker(c.bindingQueue, "ServiceBinding", maxRetries, true, c.reconcileServiceBindingKey, stopCh, &waitGroup)
 		createWorker(c.instancePollingQueue, "InstancePoller", maxRetries, false, c.requeueServiceInstanceForPoll, stopCh, &waitGroup)
-		createWorker(c.bindingPollingQueue, "BindingPoller", maxRetries, false, c.requeueServiceBindingForPoll, stopCh, &waitGroup)
+
+		if utilfeature.DefaultFeatureGate.Enabled(scfeatures.AsyncBindingOperations) {
+			createWorker(c.bindingPollingQueue, "BindingPoller", maxRetries, false, c.requeueServiceBindingForPoll, stopCh, &waitGroup)
+		}
 	}
 
 	<-stopCh
@@ -492,7 +495,6 @@ func convertCatalog(in *osb.CatalogResponse) ([]*v1beta1.ClusterServiceClass, []
 	for i, svc := range in.Services {
 		serviceClasses[i] = &v1beta1.ClusterServiceClass{
 			Spec: v1beta1.ClusterServiceClassSpec{
-				BindingRetrievable: svc.BindingRetrievable,
 				Bindable:           svc.Bindable,
 				PlanUpdatable:      (svc.PlanUpdatable != nil && *svc.PlanUpdatable),
 				ExternalID:         svc.ID,
@@ -503,6 +505,10 @@ func convertCatalog(in *osb.CatalogResponse) ([]*v1beta1.ClusterServiceClass, []
 			},
 		}
 
+		if utilfeature.DefaultFeatureGate.Enabled(scfeatures.AsyncBindingOperations) {
+			serviceClasses[i].Spec.BindingRetrievable = svc.BindingRetrievable,
+		}
+
 		if svc.Metadata != nil {
 			metadata, err := json.Marshal(svc.Metadata)
 			if err != nil {

pkg/controller/controller_binding.go

@@ -89,7 +89,13 @@ func (c *controller) bindingAdd(obj interface{}) {
 }
 
 func (c *controller) bindingUpdate(oldObj, newObj interface{}) {
-	c.bindingAdd(newObj)
+	// Bindings with ongoing asynchronous operations will be manually added
+	// to the polling queue by the reconciler. They should be ignored here in
+	// order to enforce polling rate-limiting.
+	binding:= newObj.(*v1beta1.ServiceBinding)
+	if !binding.Status.AsyncOpInProgress {
+		c.bindingAdd(newObj)
+	}
 }
 
 func (c *controller) bindingDelete(obj interface{}) {
@@ -409,6 +415,11 @@ func (c *controller) reconcileServiceBinding(binding *v1beta1.ServiceBinding) er
 			BindResource: &osb.BindResource{AppGUID: &appGUID},
 		}
 
+		// Asynchronous binding operations is currently ALPHA and not
+		// enabled by default. To use this feature, you must enable the
+		// AsyncBindingOperations feature gate. This may be easily set
+		// by setting `asyncBindingOperationsEnabled=true` when
+		// deploying the Service Catalog via the Helm charts.
 		if serviceClass.Spec.BindingRetrievable &&
 			utilfeature.DefaultFeatureGate.Enabled(scfeatures.AsyncBindingOperations) {
 
@@ -660,6 +671,11 @@ func (c *controller) reconcileServiceBinding(binding *v1beta1.ServiceBinding) er
 			PlanID:     servicePlan.Spec.ExternalID,
 		}
 
+		// Asynchronous binding operations is currently ALPHA and not
+		// enabled by default. To use this feature, you must enable the
+		// AsyncBindingOperations feature gate. This may be easily set
+		// by setting `asyncBindingOperationsEnabled=true` when
+		// deploying the Service Catalog via the Helm charts.
 		if serviceClass.Spec.BindingRetrievable &&
 			utilfeature.DefaultFeatureGate.Enabled(scfeatures.AsyncBindingOperations) {
 
@@ -1162,7 +1178,6 @@ func (c *controller) pollServiceBinding(binding *v1beta1.ServiceBinding) error {
 	// deleting or mitigating an orphan; this is more readable than
 	// checking the timestamps in various places.
 	mitigatingOrphan := binding.Status.OrphanMitigationInProgress
-	creating := binding.Status.CurrentOperation == v1beta1.ServiceBindingOperationBind && !mitigatingOrphan
 	deleting := false
 	if binding.Status.CurrentOperation == v1beta1.ServiceBindingOperationUnbind || mitigatingOrphan {
 		deleting = true
@@ -1191,7 +1206,7 @@ func (c *controller) pollServiceBinding(binding *v1beta1.ServiceBinding) error {
 			)
 		}
 
-		if !creating {
+		if deleting {
 			clearServiceBindingCurrentOperation(binding)
 
 			if _, err := c.updateServiceBindingStatus(binding); err != nil {
@@ -1552,7 +1567,10 @@ func (c *controller) checkPollingServiceBindingForReconciliationRetryTimeout(bin
 	pcb := pretty.NewContextBuilder(pretty.ServiceBinding, binding.Namespace, binding.Name)
 
 	mitigatingOrphan := binding.Status.OrphanMitigationInProgress
-	creating := binding.Status.CurrentOperation == v1beta1.ServiceBindingOperationBind && !mitigatingOrphan
+	deleting := false
+	if binding.Status.CurrentOperation == v1beta1.ServiceBindingOperationUnbind || mitigatingOrphan {
+		deleting = true
+	}
 
 	s := "Stopping reconciliation retries because too much time has elapsed"
 	glog.Infof(pcb.Message(s))
@@ -1576,7 +1594,7 @@ func (c *controller) checkPollingServiceBindingForReconciliationRetryTimeout(bin
 		)
 	}
 
-	if !creating {
+	if deleting {
 		clearServiceBindingCurrentOperation(binding)
 
 		if _, err := c.updateServiceBindingStatus(binding); err != nil {