Broker Relist (openshift#1183)

* Base commit

* Test updates for passing

* storage_interface_test updates

* NOTE: May not be correct, but was required for passing the tests

* Set default Duration in fuzzer

* RelistDuration + defaults and tests

* Generated code

* Initial controller-manager changes

* Fix bug in the defaults with Duration

As written, was always setting Duration to 15m0s when Behavior was set
to Duration

* PR feedback items + updated tests

* Generated feedback

* Newline separate fields

* Defualt RelistDuration only when set to Duration

* Handle RelistDuration nil when set to Duration

* Log error and return false from shouldReconcileServiceBroker
when RelistDuration is set to nil despite a Duration behavior

* Comment updates, spacing and content

* Validation and default tests

* Bump nil RelistDuration log to error

* Add controller_broker tests to trigger new paths

* Remove todos

* Fix test names

* Minor spacing

* Additional tests and feedback response

* Add comment for non-negative RelistRequests

* PR feedback items

* Set default RelistBehavior only in the absence of a value
* Error messages tweaks
* Use correct Behavior constants from types rather than inline strings
* Validate nil RelistDuration when RelistBehavior is set to Manual
* Remove redundant validation
* Fix accidental binary AND
* Test negative value for RelistRequests fails validation

* Use non-default vals to test storage serialization

* Add updated types.generated.go

* Fix linter complaints

* Prune last of the inline str behaviors

* Error message update based on feedback

* Format API field godocs appropriately

pkg/apis/servicecatalog/testing/fuzzer.go

@@ -21,6 +21,7 @@ import (
 	"math/rand"
 	"strconv"
 	"testing"
+	"time"
 
 	"github.com/google/gofuzz"
 	"github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog"
@@ -192,6 +193,11 @@ func FuzzerFor(t *testing.T, version schema.GroupVersion, src rand.Source) *fuzz
 			// Set the bytes field on the RawExtension
 			r.Raw = bytes
 		},
+		func(bs *servicecatalog.ServiceBrokerSpec, c fuzz.Continue) {
+			c.FuzzNoCustom(bs)
+			bs.RelistBehavior = servicecatalog.ServiceBrokerRelistBehaviorDuration
+			bs.RelistDuration = &metav1.Duration{Duration: 15 * time.Minute}
+		},
 		func(is *servicecatalog.ServiceInstanceSpec, c fuzz.Continue) {
 			c.FuzzNoCustom(is)
 			is.ExternalID = uuid.NewV4().String()

pkg/apis/servicecatalog/types.go

@@ -60,8 +60,33 @@ type ServiceBrokerSpec struct {
 	// CABundle is a PEM encoded CA bundle which will be used to validate a Broker's serving certificate.
 	// +optional
 	CABundle []byte
+
+	// RelistBehavior specifies the type of relist behavior the catalog should
+	// exhibit when relisting ServiceClasses available from a broker.
+	RelistBehavior ServiceBrokerRelistBehavior
+
+	// RelistDuration is the frequency by which a controller will relist the
+	// broker when the RelistBehavior is set to ServiceBrokerRelistBehaviorDuration.
+	RelistDuration *metav1.Duration
+
+	// RelistRequests is a strictly increasing, non-negative integer counter that
+	// can be manually incremented by a user to manually trigger a relist.
+	RelistRequests int64
 }
 
+// ServiceBrokerRelistBehavior represents a type of broker relist behavior.
+type ServiceBrokerRelistBehavior string
+
+const (
+	// ServiceBrokerRelistBehaviorDuration indicates that the broker will be
+	// relisted automatically after the specified duration has passed.
+	ServiceBrokerRelistBehaviorDuration ServiceBrokerRelistBehavior = "Duration"
+
+	// ServiceBrokerRelistBehaviorManual indicates that the broker is only
+	// relisted when the spec of the broker changes.
+	ServiceBrokerRelistBehaviorManual ServiceBrokerRelistBehavior = "Manual"
+)
+
 // ServiceBrokerAuthInfo is a union type that contains information on one of the authentication methods
 // the the service catalog and brokers may support, according to the OpenServiceBroker API
 // specification (https://github.com/openservicebrokerapi/servicebroker/blob/master/spec.md).

pkg/apis/servicecatalog/v1alpha1/defaults.go

@@ -18,13 +18,25 @@ package v1alpha1
 
 import (
 	"github.com/satori/go.uuid"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"time"
 )
 
 func addDefaultingFuncs(scheme *runtime.Scheme) error {
 	return RegisterDefaults(scheme)
 }
 
+func SetDefaults_ServiceBrokerSpec(spec *ServiceBrokerSpec) {
+	if spec.RelistBehavior == "" {
+		spec.RelistBehavior = ServiceBrokerRelistBehaviorDuration
+	}
+
+	if spec.RelistBehavior == ServiceBrokerRelistBehaviorDuration && spec.RelistDuration == nil {
+		spec.RelistDuration = &metav1.Duration{Duration: 15 * time.Minute}
+	}
+}
+
 func SetDefaults_ServiceInstanceSpec(spec *ServiceInstanceSpec) {
 	if spec.ExternalID == "" {
 		spec.ExternalID = uuid.NewV4().String()

pkg/apis/servicecatalog/v1alpha1/defaults_test.go

@@ -20,11 +20,13 @@ import (
 	"fmt"
 	"reflect"
 	"testing"
+	"time"
 
 	"github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog"
 	_ "github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog/install"
 	"github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog/testapi"
 	versioned "github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog/v1alpha1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/pkg/api"
@@ -68,6 +70,64 @@ func roundTrip(t *testing.T, obj runtime.Object) runtime.Object {
 	return obj3
 }
 
+func TestSetDefaultServiceBroker(t *testing.T) {
+	cases := []struct {
+		name     string
+		broker   *versioned.ServiceBroker
+		behavior versioned.ServiceBrokerRelistBehavior
+		duration *metav1.Duration
+	}{
+		{
+			name:     "neither duration or behavior set",
+			broker:   &versioned.ServiceBroker{},
+			behavior: versioned.ServiceBrokerRelistBehaviorDuration,
+			duration: &metav1.Duration{Duration: 15 * time.Minute},
+		},
+		{
+			name: "behavior set to manual",
+			broker: func() *versioned.ServiceBroker {
+				b := &versioned.ServiceBroker{}
+				b.Spec.RelistBehavior = versioned.ServiceBrokerRelistBehaviorManual
+				return b
+			}(),
+			behavior: versioned.ServiceBrokerRelistBehaviorManual,
+			duration: nil,
+		},
+		{
+			name: "behavior set to duration but no duration provided",
+			broker: func() *versioned.ServiceBroker {
+				b := &versioned.ServiceBroker{}
+				b.Spec.RelistBehavior = versioned.ServiceBrokerRelistBehaviorDuration
+				return b
+			}(),
+			behavior: versioned.ServiceBrokerRelistBehaviorDuration,
+			duration: &metav1.Duration{Duration: 15 * time.Minute},
+		},
+	}
+
+	for _, tc := range cases {
+		o := roundTrip(t, runtime.Object(tc.broker))
+		ab := o.(*versioned.ServiceBroker)
+		actualSpec := ab.Spec
+
+		if tc.behavior != actualSpec.RelistBehavior {
+			t.Errorf(
+				"%v: unexpected default RelistBehavior: expected %v, got %v",
+				tc.name, tc.behavior, actualSpec.RelistBehavior,
+			)
+		}
+
+		if tc.duration == nil && actualSpec.RelistDuration == nil {
+			continue
+		} else if *tc.duration != *actualSpec.RelistDuration {
+			t.Errorf(
+				"%v: unexpected RelistDuration: expected %v, got %v",
+				tc.name, tc.duration, actualSpec.RelistDuration,
+			)
+		}
+	}
+}
+
 func TestSetDefaultServiceInstance(t *testing.T) {
 	i := &versioned.ServiceInstance{}
 	obj2 := roundTrip(t, runtime.Object(i))