Make timeout of any request to the broker globally configurable

charts/catalog/README.md

@@ -80,6 +80,7 @@ chart and their default values.
 | `controllerManager.healthcheck.enabled` | Enable readiness and liveliness probes | `true` |
 | `controllerManager.verbosity` | Log level; valid values are in the range 0 - 10 | `10` |
 | `controllerManager.resyncInterval` | How often the controller should resync informers; duration format (`20m`, `1h`, etc) | `5m` |
+| `controllerManager.osbApiRequestTimeout` | The maximum amount of timeout to any request to the broker; duration format (`60s`, `3m`, etc) | `60s` |
 | `controllerManager.brokerRelistInterval` | How often the controller should relist the catalogs of ready brokers; duration format (`20m`, `1h`, etc) | `24h` |
 | `controllerManager.brokerRelistIntervalActivated` | Whether or not the controller supports a --broker-relist-interval flag. If this is set to true, brokerRelistInterval will be used as the value for that flag. | `true` |
 | `controllerManager.profiling.disabled` | Disable profiling via web interface host:port/debug/pprof/ | `false` |

charts/catalog/templates/controller-manager-deployment.yaml

@@ -80,6 +80,10 @@ spec:
         - --operation-polling-maximum-backoff-duration
         - {{ .Values.controllerManager.operationPollingMaximumBackoffDuration }}
         {{- end }}
+        {{ if .Values.controllerManager.osbApiRequestTimeout -}}
+        - --osb-api-request-timeout
+        - {{ .Values.controllerManager.osbApiRequestTimeout }}
+        {{- end }}
         - --feature-gates
         - OriginatingIdentity={{.Values.originatingIdentityEnabled}}
         - --feature-gates

charts/catalog/values.yaml

@@ -149,6 +149,8 @@ controllerManager:
   brokerRelistIntervalActivated: true
    # The maximum amount of time to back-off while polling an OSB API operation; format is a duration (`20m`, `1h`, etc)
   operationPollingMaximumBackoffDuration: 20m
+  # The maximum amount of timeout to any request to the broker; format is a duration (`60s`, `3m`, etc)
+  osbApiRequestTimeout: 60s
   # enables profiling via web interface host:port/debug/pprof/
   profiling:
     # Disable profiling via web interface host:port/debug/pprof/

cmd/controller-manager/app/controller_manager.go

@@ -402,6 +402,7 @@ func StartControllers(s *options.ControllerManagerServer,
 		s.OperationPollingMaximumBackoffDuration,
 		s.ClusterIDConfigMapName,
 		s.ClusterIDConfigMapNamespace,
+		s.OSBAPITimeOut,
 	)
 	if err != nil {
 		return err

cmd/controller-manager/app/options/options.go

@@ -60,6 +60,7 @@ const (
 	defaultLeaderElectionNamespace                = "kube-system"
 	defaultReconciliationRetryDuration            = 7 * 24 * time.Hour
 	defaultOperationPollingMaximumBackoffDuration = 20 * time.Minute
+	defaultOSBAPITimeOut                          = 60 * time.Second
 )
 
 var defaultOSBAPIPreferredVersion = osb.LatestAPIVersion().HeaderValue()
@@ -78,6 +79,7 @@ func NewControllerManagerServer() *ControllerManagerServer {
 			ServiceBrokerRelistInterval:            defaultServiceBrokerRelistInterval,
 			OSBAPIContextProfile:                   defaultOSBAPIContextProfile,
 			OSBAPIPreferredVersion:                 defaultOSBAPIPreferredVersion,
+			OSBAPITimeOut:                          defaultOSBAPITimeOut,
 			ConcurrentSyncs:                        defaultConcurrentSyncs,
 			LeaderElection:                         leaderelectionconfig.DefaultLeaderElectionConfiguration(),
 			LeaderElectionNamespace:                defaultLeaderElectionNamespace,
@@ -119,6 +121,7 @@ func (s *ControllerManagerServer) AddFlags(fs *pflag.FlagSet) {
 	fs.StringVar(&s.LeaderElectionNamespace, "leader-election-namespace", s.LeaderElectionNamespace, "Namespace to use for leader election lock")
 	fs.DurationVar(&s.ReconciliationRetryDuration, "reconciliation-retry-duration", s.ReconciliationRetryDuration, "The maximum amount of time to retry reconciliations on a resource before failing")
 	fs.DurationVar(&s.OperationPollingMaximumBackoffDuration, "operation-polling-maximum-backoff-duration", s.OperationPollingMaximumBackoffDuration, "The maximum amount of time to back-off while polling an OSB API operation")
+	fs.DurationVar(&s.OSBAPITimeOut, "osb-api-request-timeout", s.OSBAPITimeOut, "The maximum amount of timeout to any request to the broker.")
 	s.SecureServingOptions.AddFlags(fs)
 	utilfeature.DefaultMutableFeatureGate.AddFlag(fs)
 	fs.StringVar(&s.ClusterIDConfigMapName, "cluster-id-configmap-name", controller.DefaultClusterIDConfigMapName, "k8s name for clusterid configmap")

pkg/apis/componentconfig/types.go

@@ -77,6 +77,9 @@ type ControllerManagerConfiguration struct {
 	OSBAPIContextProfile   bool
 	OSBAPIPreferredVersion string
 
+	// OSBAPITimeOut the length of the timeout of any request to the broker.
+	OSBAPITimeOut time.Duration
+
 	// ConcurrentSyncs is the number of resources, per resource type,
 	// that are allowed to sync concurrently. Larger number = more responsive
 	// SC operations, but more CPU (and network) load.

pkg/controller/case_test.go

@@ -131,6 +131,7 @@ func newControllerTest(t *testing.T) *controllerTest {
 		7*24*time.Hour,
 		"DefaultClusterIDConfigMapName",
 		"DefaultClusterIDConfigMapNamespace",
+		60*time.Second,
 	)
 	if err != nil {
 		t.Fatal(err)

pkg/controller/controller.go

@@ -95,13 +95,15 @@ func NewController(
 	operationPollingMaximumBackoffDuration time.Duration,
 	clusterIDConfigMapName string,
 	clusterIDConfigMapNamespace string,
+	osbAPITimeOut time.Duration,
 ) (Controller, error) {
 	controller := &controller{
 		kubeClient:                  kubeClient,
 		secretLister:                secretInformer.Lister(),
 		serviceCatalogClient:        serviceCatalogClient,
 		brokerRelistInterval:        brokerRelistInterval,
 		OSBAPIPreferredVersion:      osbAPIPreferredVersion,
+		OSBAPITimeOut:               osbAPITimeOut,
 		recorder:                    recorder,
 		reconciliationRetryDuration: reconciliationRetryDuration,
 		clusterServiceBrokerQueue:   workqueue.NewNamedRateLimitingQueue(workqueue.NewItemExponentialFailureRateLimiter(pollingStartInterval, operationPollingMaximumBackoffDuration), "cluster-service-broker"),
@@ -205,6 +207,7 @@ type controller struct {
 	secretLister                v1.SecretLister
 	brokerRelistInterval        time.Duration
 	OSBAPIPreferredVersion      string
+	OSBAPITimeOut               time.Duration
 	recorder                    record.EventRecorder
 	reconciliationRetryDuration time.Duration
 	clusterServiceBrokerQueue   workqueue.RateLimitingInterface
@@ -1271,14 +1274,15 @@ func isServiceInstanceOrphanMitigation(instance *v1beta1.ServiceInstance) bool {
 
 // NewClientConfigurationForBroker creates a new ClientConfiguration for connecting
 // to the specified Broker
-func NewClientConfigurationForBroker(meta metav1.ObjectMeta, commonSpec *v1beta1.CommonServiceBrokerSpec, authConfig *osb.AuthConfig) *osb.ClientConfiguration {
+func NewClientConfigurationForBroker(meta metav1.ObjectMeta, commonSpec *v1beta1.CommonServiceBrokerSpec, authConfig *osb.AuthConfig, osbAPITimeOut time.Duration) *osb.ClientConfiguration {
 	clientConfig := osb.DefaultClientConfiguration()
 	clientConfig.Name = meta.Name
 	clientConfig.URL = commonSpec.URL
 	clientConfig.AuthConfig = authConfig
 	clientConfig.EnableAlphaFeatures = true
 	clientConfig.Insecure = commonSpec.InsecureSkipTLSVerify
 	clientConfig.CAData = commonSpec.CABundle
+	clientConfig.TimeoutSeconds = int(osbAPITimeOut.Seconds())
 	return clientConfig
 }
 

pkg/controller/controller_clusterservicebroker.go

@@ -137,7 +137,7 @@ func (c *controller) clusterServiceBrokerClient(broker *v1beta1.ClusterServiceBr
 		}
 		return nil, err
 	}
-	clientConfig := NewClientConfigurationForBroker(broker.ObjectMeta, &broker.Spec.CommonServiceBrokerSpec, authConfig)
+	clientConfig := NewClientConfigurationForBroker(broker.ObjectMeta, &broker.Spec.CommonServiceBrokerSpec, authConfig, c.OSBAPITimeOut)
 	brokerClient, err := c.brokerClientManager.UpdateBrokerClient(NewClusterServiceBrokerKey(broker.Name), clientConfig)
 	if err != nil {
 		s := fmt.Sprintf("Error creating client for broker %q: %s", broker.Name, err)

pkg/controller/controller_clusterservicebroker_test.go

@@ -232,6 +232,21 @@ func TestShouldReconcileClusterServiceBroker(t *testing.T) {
 	}
 }
 
+// TestReconcileClusterServiceBrokerSetOSBTimeOut
+// verifies that timeout of any request to the
+// broker takes effect.
+func TestReconcileClusterServiceBrokerSetOSBTimeOut(t *testing.T) {
+	_, _, _, testController, _ := newTestController(t, getTestCatalogConfig())
+	testController.OSBAPITimeOut = time.Second * 80
+	if err := reconcileClusterServiceBroker(t, testController, getTestClusterServiceBroker()); err != nil {
+		t.Fatalf("This should not fail : %v", err)
+	}
+	createdClient := testController.brokerClientManager.clients[NewClusterServiceBrokerKey(getTestClusterServiceBroker().Name)]
+	if createdClient.clientConfig.TimeoutSeconds != int(testController.OSBAPITimeOut.Seconds()) {
+		t.Errorf("Unexpected OSBTimeOut: got %v, expeted %v", createdClient.clientConfig.TimeoutSeconds, int(testController.OSBAPITimeOut.Seconds()))
+	}
+}
+
 // TestReconcileClusterServiceBrokerExistingServiceClassAndServicePlan
 // verifies a simple, successful run of reconcileClusterServiceBroker() when a
 // ClusterServiceClass and plan already exist.  This test will cause

pkg/controller/controller_servicebroker.go

@@ -127,7 +127,7 @@ func (c *controller) serviceBrokerClient(broker *v1beta1.ServiceBroker) (osb.Cli
 		return nil, err
 	}
 
-	clientConfig := NewClientConfigurationForBroker(broker.ObjectMeta, &broker.Spec.CommonServiceBrokerSpec, authConfig)
+	clientConfig := NewClientConfigurationForBroker(broker.ObjectMeta, &broker.Spec.CommonServiceBrokerSpec, authConfig, c.OSBAPITimeOut)
 
 	brokerClient, err := c.brokerClientManager.UpdateBrokerClient(NewServiceBrokerKey(broker.Namespace, broker.Name), clientConfig)
 	if err != nil {

pkg/controller/controller_test.go

@@ -2354,6 +2354,7 @@ func newTestController(t *testing.T, config fakeosb.FakeClientConfiguration) (
 		7*24*time.Hour,
 		DefaultClusterIDConfigMapName,
 		DefaultClusterIDConfigMapNamespace,
+		60*time.Second,
 	)
 
 	if err != nil {

test/integration/controller_test.go

@@ -791,6 +791,7 @@ func newControllerTestTestController(ct *controllerTest) (
 		7*24*time.Hour,
 		controller.DefaultClusterIDConfigMapName,
 		controller.DefaultClusterIDConfigMapNamespace,
+		60*time.Second,
 	)
 	t.Log("controller start")
 	if err != nil {
@@ -957,6 +958,7 @@ func newTestController(t *testing.T) (
 		7*24*time.Hour,
 		controller.DefaultClusterIDConfigMapName,
 		controller.DefaultClusterIDConfigMapNamespace,
+		60*time.Second,
 	)
 	t.Log("controller start")
 	if err != nil {