diff --git a/pkg/apis/servicecatalog/fake/const.go b/pkg/apis/servicecatalog/fake/const.go new file mode 100644 index 00000000000..23224f1c0b7 --- /dev/null +++ b/pkg/apis/servicecatalog/fake/const.go @@ -0,0 +1,47 @@ +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package fake + +const ( + // Namespace is a name used for test namespaces + Namespace = "test-ns" + // NamespaceUID is a UID used for test namespaces + NamespaceUID = "test-ns-uid" + + // BrokerURL is the URL used for test brokers + BrokerURL = "http://example.com" + // BrokerName is the name used for test brokers + BrokerName = "test-broker" + + // ServiceClassName is the name used for test service classes + ServiceClassName = "test-serviceclass" + // ServiceClassGUID is the GUID used for test service classes + ServiceClassGUID = "SCGUID" + // PlanName is the name used for test plans + PlanName = "test-plan" + // PlanGUID is the GUID used for test plans + PlanGUID = "PGUID" + //NonBindablePlanName is the name used for test plans that should not be bindable + NonBindablePlanName = "test-unbindable-plan" + // NonBindablePlanGUID is the GUID used for test plans that should not be bindable + NonBindablePlanGUID = "UNBINDABLE-PLAN" + + // InstanceName is a name used for test instances + InstanceName = "test-instance" + // InstanceGUID is the GUID used for test instances + InstanceGUID = "IGUID" +) diff --git a/pkg/apis/servicecatalog/fake/pointers.go b/pkg/apis/servicecatalog/fake/pointers.go new file mode 100644 index 00000000000..b57c38220a1 --- /dev/null +++ b/pkg/apis/servicecatalog/fake/pointers.go @@ -0,0 +1,21 @@ +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package fake + +func boolPtr(b bool) *bool { + return &b +} diff --git a/pkg/apis/servicecatalog/fake/types.go b/pkg/apis/servicecatalog/fake/types.go new file mode 100644 index 00000000000..e25e6541a53 --- /dev/null +++ b/pkg/apis/servicecatalog/fake/types.go @@ -0,0 +1,71 @@ +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package fake + +import ( + "github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog/v1alpha1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// GetBroker is a convenience function to get a *v1alpha1.Broker for use in tests +func GetBroker() *v1alpha1.Broker { + return &v1alpha1.Broker{ + ObjectMeta: metav1.ObjectMeta{Name: BrokerName}, + Spec: v1alpha1.BrokerSpec{ + URL: BrokerURL, + }, + } +} + +// GetServiceClass returns a ServiceClass that can be used by tests +func GetServiceClass() *v1alpha1.ServiceClass { + return &v1alpha1.ServiceClass{ + ObjectMeta: metav1.ObjectMeta{Name: ServiceClassName}, + BrokerName: BrokerName, + Description: "a test service", + ExternalID: ServiceClassGUID, + Bindable: true, + Plans: []v1alpha1.ServicePlan{ + { + Name: PlanName, + Description: "a test plan", + Free: true, + ExternalID: PlanGUID, + }, + { + Name: NonBindablePlanName, + Description: "a test plan", + Free: true, + ExternalID: NonBindablePlanGUID, + Bindable: boolPtr(false), + }, + }, + } + +} + +// GetInstance returns an Instance that can be used by tests +func GetInstance() *v1alpha1.Instance { + return &v1alpha1.Instance{ + ObjectMeta: metav1.ObjectMeta{Name: InstanceName, Namespace: Namespace}, + Spec: v1alpha1.InstanceSpec{ + ServiceClassName: ServiceClassName, + PlanName: PlanName, + ExternalID: InstanceGUID, + }, + } +} diff --git a/pkg/brokerapi/fake/data.go b/pkg/brokerapi/fake/data.go new file mode 100644 index 00000000000..12f0f2f7c9e --- /dev/null +++ b/pkg/brokerapi/fake/data.go @@ -0,0 +1,69 @@ +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package fake + +import ( + osb "github.com/pmorie/go-open-service-broker-client/v2" +) + +const ( + // ServiceClassName is the static name for service classes in test data + ServiceClassName = "testserviceclass" + // ServiceClassGUID is the static guid for service classes in test data + ServiceClassGUID = "testserviceclassGUID" + // PlanName is the static name for plans in test data + PlanName = "testplan" + // PlanGUID is the static name for plan GUIDs in test data + PlanGUID = "testPlanGUID" + // NonBindablePlanName is the static name for non-bindable plans in test data + NonBindablePlanName = "testNonBindablePlan" + // NonBindablePlanGUID is the static GUID for non-bindable plans in test data + NonBindablePlanGUID = "testNonBinablePlanGUID" +) + +func boolPtr(b bool) *bool { + return &b +} + +// GetTestCatalog returns a static osb.CatalogResponse for use in testing +func GetTestCatalog() *osb.CatalogResponse { + return &osb.CatalogResponse{ + Services: []osb.Service{ + { + Name: ServiceClassName, + ID: ServiceClassGUID, + Description: "a test service", + Bindable: true, + Plans: []osb.Plan{ + { + Name: PlanName, + Free: boolPtr(true), + ID: PlanGUID, + Description: "a test plan", + }, + { + Name: NonBindablePlanName, + Free: boolPtr(true), + ID: NonBindablePlanGUID, + Description: "a test plan", + Bindable: boolPtr(false), + }, + }, + }, + }, + } +} diff --git a/pkg/brokerapi/fake/server/convert_catalog.go b/pkg/brokerapi/fake/server/convert_catalog.go index 4ee84710b0a..2f2c961d7e8 100644 --- a/pkg/brokerapi/fake/server/convert_catalog.go +++ b/pkg/brokerapi/fake/server/convert_catalog.go @@ -17,13 +17,13 @@ limitations under the License. package server import ( - pkgbrokerapi "github.com/kubernetes-incubator/service-catalog/pkg/brokerapi" pivbrokerapi "github.com/pivotal-cf/brokerapi" + osb "github.com/pmorie/go-open-service-broker-client/v2" ) // ConvertCatalog converts a (github.com/kubernetes-incubator/service-catalog/pkg/brokerapi).Catalog // to an array of brokerapi.Services -func ConvertCatalog(cat *pkgbrokerapi.Catalog) []pivbrokerapi.Service { +func ConvertCatalog(cat *osb.CatalogResponse) []pivbrokerapi.Service { ret := make([]pivbrokerapi.Service, len(cat.Services)) for i, svc := range cat.Services { ret[i] = convertService(svc) @@ -31,27 +31,32 @@ func ConvertCatalog(cat *pkgbrokerapi.Catalog) []pivbrokerapi.Service { return ret } -func convertService(svc *pkgbrokerapi.Service) pivbrokerapi.Service { +func convertService(svc osb.Service) pivbrokerapi.Service { + updateable := false + if svc.PlanUpdatable != nil { + updateable = *svc.PlanUpdatable + } return pivbrokerapi.Service{ ID: svc.ID, Name: svc.Name, Description: svc.Description, Bindable: svc.Bindable, Tags: svc.Tags, - PlanUpdatable: svc.PlanUpdateable, + PlanUpdatable: updateable, Plans: convertPlans(svc.Plans), // TODO: convert Requires, Metadata, DashboardClient } } -func convertPlans(plans []pkgbrokerapi.ServicePlan) []pivbrokerapi.ServicePlan { +func convertPlans(plans []osb.Plan) []pivbrokerapi.ServicePlan { ret := make([]pivbrokerapi.ServicePlan, len(plans)) for i, plan := range plans { + ret[i] = pivbrokerapi.ServicePlan{ ID: plan.ID, Name: plan.Name, Description: plan.Description, - Free: &plan.Free, + Free: plan.Free, Bindable: plan.Bindable, // TODO: convert Metadata } diff --git a/pkg/controller/actions.go b/pkg/controller/actions.go new file mode 100644 index 00000000000..4abe6bad2db --- /dev/null +++ b/pkg/controller/actions.go @@ -0,0 +1,191 @@ +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "reflect" + "testing" + + "github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog/v1alpha1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + clientgotesting "k8s.io/client-go/testing" +) + +// TestNumberOfActions checks to ensure that actions has the given (in number) number of actions +// in it. Calls f with the failure if it doesn't have that number of actions +func TestNumberOfActions(t *testing.T, name string, f FailfFunc, actions []clientgotesting.Action, number int) bool { + logContext := "" + if len(name) > 0 { + logContext = name + ": " + } + + if e, a := number, len(actions); e != a { + t.Logf("%+v\n", actions) + f(t, "%vUnexpected number of actions: expected %v, got %v;\nactions: %+v", logContext, e, a, actions) + return false + } + + return true +} + +// AssertNumberOfActions fails if actions was not of length number +func AssertNumberOfActions(t *testing.T, actions []clientgotesting.Action, number int) { + TestNumberOfActions(t, "" /* name */, Fatalf, actions, number) +} + +// AssertUpdateStatus ensures that the status of action is an update on obj +func AssertUpdateStatus(t *testing.T, action clientgotesting.Action, obj interface{}) runtime.Object { + return AssertActionFor(t, action, "update", "status", obj) +} + +// AssertActionFor tests for that verb and subresource are set on action for the given object +func AssertActionFor(t *testing.T, action clientgotesting.Action, verb, subresource string, obj interface{}) runtime.Object { + r, _ := TestActionFor(t, "" /* name */, Fatalf, action, verb, subresource, obj) + return r +} + +// TestActionFor ensures that the given action, verb and subresource are set on obj +func TestActionFor( + t *testing.T, + name string, + f FailfFunc, + action clientgotesting.Action, + verb, + subresource string, + obj interface{}, +) (runtime.Object, bool) { + logContext := "" + if len(name) > 0 { + logContext = name + ": " + } + + if e, a := verb, action.GetVerb(); e != a { + f(t, "%vUnexpected verb: expected %v, got %v", logContext, e, a) + return nil, false + } + + var resource string + + switch obj.(type) { + case *v1alpha1.Broker: + resource = "brokers" + case *v1alpha1.ServiceClass: + resource = "serviceclasses" + case *v1alpha1.Instance: + resource = "instances" + case *v1alpha1.Binding: + resource = "bindings" + } + + if e, a := resource, action.GetResource().Resource; e != a { + f(t, "%vUnexpected resource; expected %v, got %v", logContext, e, a) + return nil, false + } + + if e, a := subresource, action.GetSubresource(); e != a { + f(t, "%vUnexpected subresource; expected %v, got %v", logContext, e, a) + return nil, false + } + + rtObject, ok := obj.(runtime.Object) + if !ok { + f(t, "%vObject %+v was not a runtime.Object", logContext, obj) + return nil, false + } + + paramAccessor, err := metav1.ObjectMetaFor(rtObject) + if err != nil { + f(t, "%vError creating ObjectMetaAccessor for param object %+v: %v", logContext, rtObject, err) + return nil, false + } + + var ( + objectMeta metav1.Object + fakeRtObject runtime.Object + ) + + switch verb { + case "get": + getAction, ok := action.(clientgotesting.GetAction) + if !ok { + f(t, "%vUnexpected type; failed to convert action %+v to DeleteAction", logContext, action) + return nil, false + } + + if e, a := paramAccessor.GetName(), getAction.GetName(); e != a { + f(t, "%vUnexpected name: expected %v, got %v", logContext, e, a) + return nil, false + } + + return nil, true + case "delete": + deleteAction, ok := action.(clientgotesting.DeleteAction) + if !ok { + f(t, "%vUnexpected type; failed to convert action %+v to DeleteAction", logContext, action) + return nil, false + } + + if e, a := paramAccessor.GetName(), deleteAction.GetName(); e != a { + f(t, "%vUnexpected name: expected %v, got %v", logContext, e, a) + return nil, false + } + + return nil, true + case "create": + createAction, ok := action.(clientgotesting.CreateAction) + if !ok { + f(t, "%vUnexpected type; failed to convert action %+v to CreateAction", logContext, action) + return nil, false + } + + fakeRtObject = createAction.GetObject() + objectMeta, err = metav1.ObjectMetaFor(fakeRtObject) + if err != nil { + f(t, "%vError creating ObjectMetaAccessor for %+v", logContext, fakeRtObject) + return nil, false + } + case "update": + updateAction, ok := action.(clientgotesting.UpdateAction) + if !ok { + f(t, "%vUnexpected type; failed to convert action %+v to UpdateAction", logContext, action) + return nil, false + } + + fakeRtObject = updateAction.GetObject() + objectMeta, err = metav1.ObjectMetaFor(fakeRtObject) + if err != nil { + f(t, "%vError creating ObjectMetaAccessor for %+v", logContext, fakeRtObject) + return nil, false + } + } + + if e, a := paramAccessor.GetName(), objectMeta.GetName(); e != a { + f(t, "%vUnexpected name: expected %v, got %v", logContext, e, a) + return nil, false + } + + fakeValue := reflect.ValueOf(fakeRtObject) + paramValue := reflect.ValueOf(obj) + + if e, a := paramValue.Type(), fakeValue.Type(); e != a { + f(t, "%vUnexpected type of object passed to fake client; expected %v, got %v", logContext, e, a) + return nil, false + } + + return fakeRtObject, true +} diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index 389ebc4dc8f..58bd99faa85 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -119,6 +119,10 @@ type Controller interface { // workers specifies the number of goroutines, per resource, processing work // from the resource workqueues Run(workers int, stopCh <-chan struct{}) + // PollingQueueLen returns the length of pending asynchronous instances to be polled + PollingQueueLen() int + // ReconcileInstance reconciles the given instance to its desired state + ReconcileInstance(*v1alpha1.Instance) error } // controller is a concrete Controller. @@ -144,6 +148,10 @@ type controller struct { pollingQueue workqueue.RateLimitingInterface } +func (c *controller) PollingQueueLen() int { + return c.pollingQueue.Len() +} + // Run runs the controller until the given stop channel can be read from. func (c *controller) Run(workers int, stopCh <-chan struct{}) { defer runtimeutil.HandleCrash() diff --git a/pkg/controller/controller_binding_test.go b/pkg/controller/controller_binding_test.go index 62e0f89f3bb..6d27485e5bd 100644 --- a/pkg/controller/controller_binding_test.go +++ b/pkg/controller/controller_binding_test.go @@ -59,7 +59,7 @@ func TestReconcileBindingNonExistingInstance(t *testing.T) { assertNumberOfBrokerActions(t, brokerActions, 0) actions := fakeCatalogClient.Actions() - assertNumberOfActions(t, actions, 1) + AssertNumberOfActions(t, actions, 1) // There should only be one action that says it failed because no such instance exists. updateAction := actions[0].(clientgotesting.UpdateAction) @@ -110,7 +110,7 @@ func TestReconcileBindingNonExistingServiceClass(t *testing.T) { assertNumberOfBrokerActions(t, brokerActions, 0) actions := fakeCatalogClient.Actions() - assertNumberOfActions(t, actions, 1) + AssertNumberOfActions(t, actions, 1) // There should only be one action that says it failed because no such service class. updatedBinding := assertUpdateStatus(t, actions[0], binding) @@ -188,12 +188,12 @@ func TestReconcileBindingWithParameters(t *testing.T) { }) actions := fakeCatalogClient.Actions() - assertNumberOfActions(t, actions, 1) + AssertNumberOfActions(t, actions, 1) updatedBinding := assertUpdateStatus(t, actions[0], binding) assertBindingReadyTrue(t, updatedBinding) kubeActions := fakeKubeClient.Actions() - assertNumberOfActions(t, kubeActions, 3) + AssertNumberOfActions(t, kubeActions, 3) // first action is a get on the namespace // second action is a get on the secret @@ -260,7 +260,7 @@ func TestReconcileBindingNonbindableServiceClass(t *testing.T) { assertNumberOfBrokerActions(t, brokerActions, 0) actions := fakeCatalogClient.Actions() - assertNumberOfActions(t, actions, 1) + AssertNumberOfActions(t, actions, 1) // There should only be one action that says binding was created updatedBinding := assertUpdateStatus(t, actions[0], binding) @@ -333,12 +333,12 @@ func TestReconcileBindingNonbindableServiceClassBindablePlan(t *testing.T) { }) actions := fakeCatalogClient.Actions() - assertNumberOfActions(t, actions, 1) + AssertNumberOfActions(t, actions, 1) updatedBinding := assertUpdateStatus(t, actions[0], binding) assertBindingReadyTrue(t, updatedBinding) kubeActions := fakeKubeClient.Actions() - assertNumberOfActions(t, kubeActions, 3) + AssertNumberOfActions(t, kubeActions, 3) // first action is a get on the namespace // second action is a get on the secret @@ -400,7 +400,7 @@ func TestReconcileBindingBindableServiceClassNonbindablePlan(t *testing.T) { assertNumberOfBrokerActions(t, brokerActions, 0) actions := fakeCatalogClient.Actions() - assertNumberOfActions(t, actions, 1) + AssertNumberOfActions(t, actions, 1) // There should only be one action that says binding was created updatedBinding := assertUpdateStatus(t, actions[0], binding) @@ -445,10 +445,10 @@ func TestReconcileBindingFailsWithInstanceAsyncOngoing(t *testing.T) { // verify no kube resources created. // No actions kubeActions := fakeKubeClient.Actions() - assertNumberOfActions(t, kubeActions, 0) + AssertNumberOfActions(t, kubeActions, 0) actions := fakeCatalogClient.Actions() - assertNumberOfActions(t, actions, 1) + AssertNumberOfActions(t, actions, 1) // There should only be one action that says binding was created updatedBinding := assertUpdateStatus(t, actions[0], binding) @@ -494,7 +494,7 @@ func TestReconcileBindingInstanceNotReady(t *testing.T) { assertNumberOfBrokerActions(t, brokerActions, 0) actions := fakeCatalogClient.Actions() - assertNumberOfActions(t, actions, 1) + AssertNumberOfActions(t, actions, 1) // There should only be one action that says binding was created updatedBinding := assertUpdateStatus(t, actions[0], binding) @@ -537,7 +537,7 @@ func TestReconcileBindingNamespaceError(t *testing.T) { assertNumberOfBrokerActions(t, brokerActions, 0) actions := fakeCatalogClient.Actions() - assertNumberOfActions(t, actions, 1) + AssertNumberOfActions(t, actions, 1) updatedBinding := assertUpdateStatus(t, actions[0], binding) assertBindingReadyFalse(t, updatedBinding) @@ -594,7 +594,7 @@ func TestReconcileBindingDelete(t *testing.T) { kubeActions := fakeKubeClient.Actions() // The two actions should be: // 0. Deleting the secret - assertNumberOfActions(t, kubeActions, 1) + AssertNumberOfActions(t, kubeActions, 1) deleteAction := kubeActions[0].(clientgotesting.DeleteActionImpl) if e, a := "delete", deleteAction.GetVerb(); e != a { @@ -610,7 +610,7 @@ func TestReconcileBindingDelete(t *testing.T) { // 0. Updating the ready condition // 1. Get against the binding in question // 2. Removing the finalizer - assertNumberOfActions(t, actions, 3) + AssertNumberOfActions(t, actions, 3) updatedBinding := assertUpdateStatus(t, actions[0], binding) assertBindingReadyFalse(t, updatedBinding) @@ -690,14 +690,14 @@ func TestReconcileBindingWithPodPresetTemplate(t *testing.T) { }) actions := fakeCatalogClient.Actions() - assertNumberOfActions(t, actions, 1) + AssertNumberOfActions(t, actions, 1) // There should only be one action that says binding was created updatedBinding := assertUpdateStatus(t, actions[0], binding) assertBindingReadyTrue(t, updatedBinding) kubeActions := fakeKubeClient.Actions() - assertNumberOfActions(t, kubeActions, 4) + AssertNumberOfActions(t, kubeActions, 4) action := kubeActions[2].(clientgotesting.CreateAction) if e, a := "create", action.GetVerb(); e != a { diff --git a/pkg/controller/controller_broker_test.go b/pkg/controller/controller_broker_test.go index 5c967c87372..c809104b623 100644 --- a/pkg/controller/controller_broker_test.go +++ b/pkg/controller/controller_broker_test.go @@ -128,7 +128,7 @@ func TestReconcileBrokerExistingServiceClass(t *testing.T) { assertGetCatalog(t, brokerActions[0]) actions := fakeCatalogClient.Actions() - assertNumberOfActions(t, actions, 2) + AssertNumberOfActions(t, actions, 2) // first action should be an update action for a service class assertUpdate(t, actions[0], testServiceClass) @@ -139,7 +139,7 @@ func TestReconcileBrokerExistingServiceClass(t *testing.T) { // verify no kube resources created kubeActions := fakeKubeClient.Actions() - assertNumberOfActions(t, kubeActions, 0) + AssertNumberOfActions(t, kubeActions, 0) } func TestReconcileBrokerExistingServiceClassDifferentExternalID(t *testing.T) { @@ -158,14 +158,14 @@ func TestReconcileBrokerExistingServiceClassDifferentExternalID(t *testing.T) { assertGetCatalog(t, brokerActions[0]) actions := fakeCatalogClient.Actions() - assertNumberOfActions(t, actions, 1) + AssertNumberOfActions(t, actions, 1) updatedBroker := assertUpdateStatus(t, actions[0], getTestBroker()) assertBrokerReadyFalse(t, updatedBroker) // verify no kube resources created kubeActions := fakeKubeClient.Actions() - assertNumberOfActions(t, kubeActions, 0) + AssertNumberOfActions(t, kubeActions, 0) events := getRecordedEvents(testController) assertNumEvents(t, events, 1) @@ -192,14 +192,14 @@ func TestReconcileBrokerExistingServiceClassDifferentBroker(t *testing.T) { assertGetCatalog(t, brokerActions[0]) actions := fakeCatalogClient.Actions() - assertNumberOfActions(t, actions, 1) + AssertNumberOfActions(t, actions, 1) updatedBroker := assertUpdateStatus(t, actions[0], getTestBroker()) assertBrokerReadyFalse(t, updatedBroker) // verify no kube resources created kubeActions := fakeKubeClient.Actions() - assertNumberOfActions(t, kubeActions, 0) + AssertNumberOfActions(t, kubeActions, 0) events := getRecordedEvents(testController) assertNumEvents(t, events, 1) @@ -233,7 +233,7 @@ func TestReconcileBrokerDelete(t *testing.T) { // Verify no core kube actions occurred kubeActions := fakeKubeClient.Actions() - assertNumberOfActions(t, kubeActions, 0) + AssertNumberOfActions(t, kubeActions, 0) actions := fakeCatalogClient.Actions() // The three actions should be: @@ -241,7 +241,7 @@ func TestReconcileBrokerDelete(t *testing.T) { // 1. Updating the ready condition // 2. Getting the broker // 3. Removing the finalizer - assertNumberOfActions(t, actions, 4) + AssertNumberOfActions(t, actions, 4) assertDelete(t, actions[0], testServiceClass) @@ -280,12 +280,12 @@ func TestReconcileBrokerErrorFetchingCatalog(t *testing.T) { assertGetCatalog(t, brokerActions[0]) actions := fakeCatalogClient.Actions() - assertNumberOfActions(t, actions, 1) + AssertNumberOfActions(t, actions, 1) updatedBroker := assertUpdateStatus(t, actions[0], broker) assertBrokerReadyFalse(t, updatedBroker) - assertNumberOfActions(t, fakeKubeClient.Actions(), 0) + AssertNumberOfActions(t, fakeKubeClient.Actions(), 0) events := getRecordedEvents(testController) assertNumEvents(t, events, 1) @@ -314,12 +314,12 @@ func TestReconcileBrokerZeroServices(t *testing.T) { assertGetCatalog(t, brokerActions[0]) actions := fakeCatalogClient.Actions() - assertNumberOfActions(t, actions, 1) + AssertNumberOfActions(t, actions, 1) updatedBroker := assertUpdateStatus(t, actions[0], broker) assertBrokerReadyFalse(t, updatedBroker) - assertNumberOfActions(t, fakeKubeClient.Actions(), 0) + AssertNumberOfActions(t, fakeKubeClient.Actions(), 0) events := getRecordedEvents(testController) assertNumEvents(t, events, 1) @@ -353,14 +353,14 @@ func TestReconcileBrokerWithAuthError(t *testing.T) { assertNumberOfBrokerActions(t, brokerActions, 0) actions := fakeCatalogClient.Actions() - assertNumberOfActions(t, actions, 1) + AssertNumberOfActions(t, actions, 1) updatedBroker := assertUpdateStatus(t, actions[0], broker) assertBrokerReadyFalse(t, updatedBroker) // verify one kube action occurred kubeActions := fakeKubeClient.Actions() - assertNumberOfActions(t, kubeActions, 1) + AssertNumberOfActions(t, kubeActions, 1) getAction := kubeActions[0].(clientgotesting.GetAction) if e, a := "get", getAction.GetVerb(); e != a { @@ -397,7 +397,7 @@ func TestReconcileBrokerWithReconcileError(t *testing.T) { assertGetCatalog(t, brokerActions[0]) actions := fakeCatalogClient.Actions() - assertNumberOfActions(t, actions, 2) + AssertNumberOfActions(t, actions, 2) createSCAction := actions[0].(clientgotesting.CreateAction) createdSC, ok := createSCAction.GetObject().(*v1alpha1.ServiceClass) @@ -411,7 +411,7 @@ func TestReconcileBrokerWithReconcileError(t *testing.T) { assertBrokerReadyFalse(t, updatedBroker) kubeActions := fakeKubeClient.Actions() - assertNumberOfActions(t, kubeActions, 0) + AssertNumberOfActions(t, kubeActions, 0) events := getRecordedEvents(testController) assertNumEvents(t, events, 1) diff --git a/pkg/controller/controller_instance.go b/pkg/controller/controller_instance.go index 38541e0a578..24bd5888125 100644 --- a/pkg/controller/controller_instance.go +++ b/pkg/controller/controller_instance.go @@ -203,9 +203,14 @@ func (c *controller) reconcileInstanceDelete(instance *v1alpha1.Instance) error return nil } -// reconcileInstance is the control-loop for reconciling Instances. An -// error is returned to indicate that the binding has not been fully +// ReconcileInstance is the Controller interface implementation. It is the control-loop for +// reconciling Instances. An error is returned to indicate that the binding has not been fully // processed and should be resubmitted at a later time. +func (c *controller) ReconcileInstance(i *v1alpha1.Instance) error { + return c.reconcileInstance(i) +} + +// reconcileInstance is the control-loop for reconciling Instances. func (c *controller) reconcileInstance(instance *v1alpha1.Instance) error { // If there's no async op in progress, determine whether the checksum diff --git a/pkg/controller/controller_instance_test.go b/pkg/controller/controller_instance_test.go index 06fb6560042..cb0bd15fbea 100644 --- a/pkg/controller/controller_instance_test.go +++ b/pkg/controller/controller_instance_test.go @@ -31,6 +31,7 @@ import ( checksum "github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog/checksum/versioned/v1alpha1" "github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog/v1alpha1" + fakebrokerserver "github.com/kubernetes-incubator/service-catalog/pkg/brokerapi/fake/server" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -61,7 +62,7 @@ func TestReconcileInstanceNonExistentServiceClass(t *testing.T) { assertNumberOfBrokerActions(t, brokerActions, 0) actions := fakeCatalogClient.Actions() - assertNumberOfActions(t, actions, 1) + AssertNumberOfActions(t, actions, 1) // There should only be one action that says it failed because no such class exists. updatedInstance := assertUpdateStatus(t, actions[0], instance) @@ -91,7 +92,7 @@ func TestReconcileInstanceNonExistentBroker(t *testing.T) { assertNumberOfBrokerActions(t, brokerActions, 0) actions := fakeCatalogClient.Actions() - assertNumberOfActions(t, actions, 1) + AssertNumberOfActions(t, actions, 1) // There should only be one action that says it failed because no such broker exists. updatedInstance := assertUpdateStatus(t, actions[0], instance) @@ -258,12 +259,12 @@ func TestReconcileInstanceWithParameters(t *testing.T) { }) actions := fakeCatalogClient.Actions() - assertNumberOfActions(t, actions, 1) + AssertNumberOfActions(t, actions, 1) // verify no kube resources created // One single action comes from getting namespace uid kubeActions := fakeKubeClient.Actions() - assertNumberOfActions(t, kubeActions, 1) + AssertNumberOfActions(t, kubeActions, 1) updatedInstance := assertUpdateStatus(t, actions[0], instance) assertInstanceReadyTrue(t, updatedInstance) @@ -314,11 +315,11 @@ func TestReconcileInstanceWithInvalidParameters(t *testing.T) { assertNumberOfBrokerActions(t, brokerActions, 0) actions := fakeCatalogClient.Actions() - assertNumberOfActions(t, actions, 1) + AssertNumberOfActions(t, actions, 1) // verify no kube resources created kubeActions := fakeKubeClient.Actions() - assertNumberOfActions(t, kubeActions, 0) + AssertNumberOfActions(t, kubeActions, 0) updatedInstance := assertUpdateStatus(t, actions[0], instance) assertInstanceReadyFalse(t, updatedInstance) @@ -364,10 +365,10 @@ func TestReconcileInstanceWithProvisionFailure(t *testing.T) { // verify no kube resources created // One single action comes from getting namespace uid kubeActions := fakeKubeClient.Actions() - assertNumberOfActions(t, kubeActions, 1) + AssertNumberOfActions(t, kubeActions, 1) actions := fakeCatalogClient.Actions() - assertNumberOfActions(t, actions, 1) + AssertNumberOfActions(t, actions, 1) updatedInstance := assertUpdateStatus(t, actions[0], instance) assertInstanceReadyFalse(t, updatedInstance) @@ -428,12 +429,12 @@ func TestReconcileInstance(t *testing.T) { } actions := fakeCatalogClient.Actions() - assertNumberOfActions(t, actions, 1) + AssertNumberOfActions(t, actions, 1) // verify no kube resources created. // One single action comes from getting namespace uid kubeActions := fakeKubeClient.Actions() - assertNumberOfActions(t, kubeActions, 1) + AssertNumberOfActions(t, kubeActions, 1) updatedInstance := assertUpdateStatus(t, actions[0], instance) assertInstanceReadyTrue(t, updatedInstance) @@ -491,7 +492,7 @@ func TestReconcileInstanceAsynchronous(t *testing.T) { }) actions := fakeCatalogClient.Actions() - assertNumberOfActions(t, actions, 1) + AssertNumberOfActions(t, actions, 1) // verify no kube resources created. // One single action comes from getting namespace uid @@ -516,7 +517,7 @@ func TestReconcileInstanceAsynchronous(t *testing.T) { if actualKey != expectedKey { t.Fatalf("got key as %q expected %q", actualKey, expectedKey) } - assertAsyncOpInProgressTrue(t, updatedInstance) + AssertAsyncOpInProgressTrue(t, updatedInstance) assertInstanceLastOperation(t, updatedInstance, testOperation) assertInstanceDashboardURL(t, updatedInstance, testDashboardURL) } @@ -562,7 +563,7 @@ func TestReconcileInstanceAsynchronousNoOperation(t *testing.T) { }) actions := fakeCatalogClient.Actions() - assertNumberOfActions(t, actions, 1) + AssertNumberOfActions(t, actions, 1) // verify no kube resources created. // One single action comes from getting namespace uid @@ -587,10 +588,115 @@ func TestReconcileInstanceAsynchronousNoOperation(t *testing.T) { if key != expectedKey { t.Fatalf("got key as %q expected %q", key, expectedKey) } - assertAsyncOpInProgressTrue(t, updatedInstance) + AssertAsyncOpInProgressTrue(t, updatedInstance) assertInstanceLastOperation(t, updatedInstance, "") } +// TestReconcileInstanceAsynchronousUnsupportedBrokerError tests to ensure that, on an asynchronous +// provision, an Instance's conditions get set with a Broker failure that is not one of the +// "expected" response codes in the OSB API spec for provision. +// See https://github.com/openservicebrokerapi/servicebroker/blob/master/spec.md#response-2 for +// the list of expected codes and a description of what we should do if another code is returned +func TestReconcileInstanceAsynchronousUnsupportedBrokerError(t *testing.T) { + const ( + brokerUsername = "testbrokeruser" + brokerPassword = "testbrokerpass" + ) + controllerItems, err := newTestControllerWithBrokerServer(brokerUsername, brokerPassword) + + if err != nil { + t.Fatal(err) + } + defer controllerItems.Close() + + fakeKubeClient := controllerItems.FakeKubeClient + fakeCatalogClient := controllerItems.FakeCatalogClient + fakeBrokerServerHandler := controllerItems.BrokerServerHandler + testController := controllerItems.Controller + sharedInformers := controllerItems.Informers + + fakeBrokerServerHandler.Catalog = fakebrokerserver.ConvertCatalog(getTestCatalog()) + + addGetNamespaceReaction(fakeKubeClient) + + // create the secret that the Broker resource will use for auth to the fake broker server + fakeKubeClient.AddReactor("get", "secrets", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, &v1.Secret{ + Data: map[string][]byte{ + "username": []byte(brokerUsername), + "password": []byte(brokerPassword), + }, + }, nil + }) + + // build the chain from Instance -> ServiceClass -> Broker + testBroker := getTestBroker() + testBroker.Spec.URL = controllerItems.BrokerServer.URL + testBroker.Spec.AuthInfo = &v1alpha1.BrokerAuthInfo{ + BasicAuthSecret: &v1.ObjectReference{ + Namespace: "test", + Name: "test", + }, + } + testServiceClass := getTestServiceClass() + testInstance := getTestInstance() + + sharedInformers.Brokers().Informer().GetStore().Add(testBroker) + sharedInformers.ServiceClasses().Informer().GetStore().Add(testServiceClass) + + fakeCatalogClient.AddReactor("get", "serviceclasses", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, testServiceClass, nil + }) + fakeCatalogClient.AddReactor("get", "brokers", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, testBroker, nil + }) + + // Make the provision return an error that is "unexpected" + fakeBrokerServerHandler.ProvisionRespError = errors.New("test provision error") + + // there should be nothing that is polling since no instances have been reconciled yet + if testController.pollingQueue.Len() != 0 { + t.Fatalf("Expected the polling queue to be empty") + } + + reconcileErr := testController.reconcileInstance(testInstance) + if reconcileErr == nil { + t.Fatalf("expected an error from reconcileInstance") + } + statusCodeErr, ok := reconcileErr.(osb.HTTPStatusCodeError) + if !ok { + t.Fatalf("expected an OSB client HTTPStatusCodeError") + } + if statusCodeErr.StatusCode != http.StatusInternalServerError { + t.Fatalf("expected an internal server error, got %d", statusCodeErr.StatusCode) + } + + actions := fakeCatalogClient.Actions() + AssertNumberOfActions(t, actions, 1) + + // verify that 2 kubernetes actions occurred - a GET to the ServiceClass, then a GET to the + // Broker + kubeActions := fakeKubeClient.Actions() + AssertNumberOfActions(t, kubeActions, 2) + + updatedInstance := assertUpdateStatus(t, actions[0], testInstance) + assertInstanceReadyFalse(t, updatedInstance) + + // there should be 1 request to the provision endpoint + numProvReqs := len(fakeBrokerServerHandler.ProvisionRequests) + if numProvReqs != 1 { + t.Fatalf("%d provision requests were made, expected 1", numProvReqs) + } + + // The item should not have been added to the polling queue for later processing + if testController.pollingQueue.Len() != 0 { + t.Fatalf("Expected polling queue to be empty") + } + AssertAsyncOpInProgressFalse(t, updatedInstance) + assertInstanceReadyFalse(t, updatedInstance) + AssertInstanceReadyCondition(t, updatedInstance, v1alpha1.ConditionFalse) +} + func TestReconcileInstanceNamespaceError(t *testing.T) { fakeKubeClient, fakeCatalogClient, fakeBrokerClient, testController, sharedInformers := newTestController(t, noFakeActions()) @@ -613,10 +719,10 @@ func TestReconcileInstanceNamespaceError(t *testing.T) { // verify no kube resources created. // One single action comes from getting namespace uid kubeActions := fakeKubeClient.Actions() - assertNumberOfActions(t, kubeActions, 1) + AssertNumberOfActions(t, kubeActions, 1) actions := fakeCatalogClient.Actions() - assertNumberOfActions(t, actions, 1) + AssertNumberOfActions(t, actions, 1) assertUpdateStatus(t, actions[0], instance) @@ -667,14 +773,14 @@ func TestReconcileInstanceDelete(t *testing.T) { // Verify no core kube actions occurred kubeActions := fakeKubeClient.Actions() - assertNumberOfActions(t, kubeActions, 0) + AssertNumberOfActions(t, kubeActions, 0) actions := fakeCatalogClient.Actions() // The three actions should be: // 0. Updating the ready condition // 1. Get against the instance // 2. Removing the finalizer - assertNumberOfActions(t, actions, 3) + AssertNumberOfActions(t, actions, 3) updatedInstance := assertUpdateStatus(t, actions[0], instance) assertInstanceReadyFalse(t, updatedInstance) @@ -718,13 +824,13 @@ func TestReconcileInstanceDeleteDoesNotInvokeBroker(t *testing.T) { // Verify no core kube actions occurred kubeActions := fakeKubeClient.Actions() - assertNumberOfActions(t, kubeActions, 0) + AssertNumberOfActions(t, kubeActions, 0) actions := fakeCatalogClient.Actions() // The three actions should be: // 0. Get against the instance // 1. Removing the finalizer - assertNumberOfActions(t, actions, 2) + AssertNumberOfActions(t, actions, 2) assertGet(t, actions[0], instance) updatedInstance := assertUpdateStatus(t, actions[1], instance) @@ -768,12 +874,12 @@ func TestPollServiceInstanceInProgressProvisioningWithOperation(t *testing.T) { } actions := fakeCatalogClient.Actions() - assertNumberOfActions(t, actions, 0) + AssertNumberOfActions(t, actions, 0) // verify no kube resources created. // No actions kubeActions := fakeKubeClient.Actions() - assertNumberOfActions(t, kubeActions, 0) + AssertNumberOfActions(t, kubeActions, 0) } func TestPollServiceInstanceSuccessProvisioningWithOperation(t *testing.T) { @@ -806,16 +912,16 @@ func TestPollServiceInstanceSuccessProvisioningWithOperation(t *testing.T) { // verify no kube resources created. // No actions kubeActions := fakeKubeClient.Actions() - assertNumberOfActions(t, kubeActions, 0) + AssertNumberOfActions(t, kubeActions, 0) actions := fakeCatalogClient.Actions() - assertNumberOfActions(t, actions, 1) + AssertNumberOfActions(t, actions, 1) updatedInstance := assertUpdateStatus(t, actions[0], instance) // Instance should be ready and there no longer is an async operation // in place. assertInstanceReadyTrue(t, updatedInstance) - assertAsyncOpInProgressFalse(t, updatedInstance) + AssertAsyncOpInProgressFalse(t, updatedInstance) } func TestPollServiceInstanceFailureProvisioningWithOperation(t *testing.T) { @@ -848,16 +954,16 @@ func TestPollServiceInstanceFailureProvisioningWithOperation(t *testing.T) { // verify no kube resources created. // No actions kubeActions := fakeKubeClient.Actions() - assertNumberOfActions(t, kubeActions, 0) + AssertNumberOfActions(t, kubeActions, 0) actions := fakeCatalogClient.Actions() - assertNumberOfActions(t, actions, 1) + AssertNumberOfActions(t, actions, 1) updatedInstance := assertUpdateStatus(t, actions[0], instance) // Instance should be not ready and there no longer is an async operation // in place. assertInstanceReadyFalse(t, updatedInstance) - assertAsyncOpInProgressFalse(t, updatedInstance) + AssertAsyncOpInProgressFalse(t, updatedInstance) } func TestPollServiceInstanceInProgressDeprovisioningWithOperationNoFinalizer(t *testing.T) { @@ -893,12 +999,12 @@ func TestPollServiceInstanceInProgressDeprovisioningWithOperationNoFinalizer(t * } actions := fakeCatalogClient.Actions() - assertNumberOfActions(t, actions, 0) + AssertNumberOfActions(t, actions, 0) // verify no kube resources created. // No actions kubeActions := fakeKubeClient.Actions() - assertNumberOfActions(t, kubeActions, 0) + AssertNumberOfActions(t, kubeActions, 0) } func TestPollServiceInstanceSuccessDeprovisioningWithOperationNoFinalizer(t *testing.T) { @@ -931,15 +1037,15 @@ func TestPollServiceInstanceSuccessDeprovisioningWithOperationNoFinalizer(t *tes // verify no kube resources created. // No actions kubeActions := fakeKubeClient.Actions() - assertNumberOfActions(t, kubeActions, 0) + AssertNumberOfActions(t, kubeActions, 0) actions := fakeCatalogClient.Actions() - assertNumberOfActions(t, actions, 1) + AssertNumberOfActions(t, actions, 1) updatedInstance := assertUpdateStatus(t, actions[0], instance) // Instance should have been deprovisioned - assertInstanceReadyCondition(t, updatedInstance, v1alpha1.ConditionFalse, successDeprovisionReason) - assertAsyncOpInProgressFalse(t, updatedInstance) + AssertInstanceReadyCondition(t, updatedInstance, v1alpha1.ConditionFalse, successDeprovisionReason) + AssertAsyncOpInProgressFalse(t, updatedInstance) events := getRecordedEvents(testController) assertNumEvents(t, events, 1) @@ -975,16 +1081,16 @@ func TestPollServiceInstanceFailureDeprovisioningWithOperation(t *testing.T) { // verify no kube resources created. // No actions kubeActions := fakeKubeClient.Actions() - assertNumberOfActions(t, kubeActions, 0) + AssertNumberOfActions(t, kubeActions, 0) actions := fakeCatalogClient.Actions() - assertNumberOfActions(t, actions, 1) + AssertNumberOfActions(t, actions, 1) updatedInstance := assertUpdateStatus(t, actions[0], instance) // Instance should be set to unknown since the operation on the broker // failed. - assertInstanceReadyCondition(t, updatedInstance, v1alpha1.ConditionUnknown, errorDeprovisionCalledReason) - assertAsyncOpInProgressFalse(t, updatedInstance) + AssertInstanceReadyCondition(t, updatedInstance, v1alpha1.ConditionUnknown, errorDeprovisionCalledReason) + AssertAsyncOpInProgressFalse(t, updatedInstance) events := getRecordedEvents(testController) assertNumEvents(t, events, 1) @@ -1020,15 +1126,15 @@ func TestPollServiceInstanceStatusGoneDeprovisioningWithOperationNoFinalizer(t * // verify no kube resources created. // No actions kubeActions := fakeKubeClient.Actions() - assertNumberOfActions(t, kubeActions, 0) + AssertNumberOfActions(t, kubeActions, 0) actions := fakeCatalogClient.Actions() - assertNumberOfActions(t, actions, 1) + AssertNumberOfActions(t, actions, 1) updatedInstance := assertUpdateStatus(t, actions[0], instance) // Instance should have been deprovisioned - assertInstanceReadyCondition(t, updatedInstance, v1alpha1.ConditionFalse, successDeprovisionReason) - assertAsyncOpInProgressFalse(t, updatedInstance) + AssertInstanceReadyCondition(t, updatedInstance, v1alpha1.ConditionFalse, successDeprovisionReason) + AssertAsyncOpInProgressFalse(t, updatedInstance) events := getRecordedEvents(testController) assertNumEvents(t, events, 1) @@ -1064,10 +1170,10 @@ func TestPollServiceInstanceBrokerError(t *testing.T) { // verify no kube resources created. // No actions kubeActions := fakeKubeClient.Actions() - assertNumberOfActions(t, kubeActions, 0) + AssertNumberOfActions(t, kubeActions, 0) actions := fakeCatalogClient.Actions() - assertNumberOfActions(t, actions, 0) + AssertNumberOfActions(t, actions, 0) events := getRecordedEvents(testController) assertNumEvents(t, events, 1) @@ -1102,17 +1208,17 @@ func TestPollServiceInstanceSuccessDeprovisioningWithOperationWithFinalizer(t *t // verify no kube resources created. // No actions kubeActions := fakeKubeClient.Actions() - assertNumberOfActions(t, kubeActions, 0) + AssertNumberOfActions(t, kubeActions, 0) actions := fakeCatalogClient.Actions() // The three actions should be: // 0. Updating the ready condition // 1. Get against the instance (updateFinalizers calls) // 2. Removing the finalizer - assertNumberOfActions(t, actions, 3) + AssertNumberOfActions(t, actions, 3) updatedInstance := assertUpdateStatus(t, actions[0], instance) - assertInstanceReadyCondition(t, updatedInstance, v1alpha1.ConditionFalse, successDeprovisionReason) + AssertInstanceReadyCondition(t, updatedInstance, v1alpha1.ConditionFalse, successDeprovisionReason) // Instance should have been deprovisioned assertGet(t, actions[1], instance) diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go index 0a891b90816..1bfeccdae50 100644 --- a/pkg/controller/controller_test.go +++ b/pkg/controller/controller_test.go @@ -21,18 +21,17 @@ import ( "errors" "net/http/httptest" "reflect" - "runtime/debug" "testing" "time" osb "github.com/pmorie/go-open-service-broker-client/v2" fakeosb "github.com/pmorie/go-open-service-broker-client/v2/fake" + faketypes "github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog/fake" "github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog/v1alpha1" + servicecatalogclientset "github.com/kubernetes-incubator/service-catalog/pkg/client/clientset_generated/clientset/fake" servicecataloginformers "github.com/kubernetes-incubator/service-catalog/pkg/client/informers_generated/externalversions" v1alpha1informers "github.com/kubernetes-incubator/service-catalog/pkg/client/informers_generated/externalversions/servicecatalog/v1alpha1" - - servicecatalogclientset "github.com/kubernetes-incubator/service-catalog/pkg/client/clientset_generated/clientset/fake" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" @@ -56,24 +55,24 @@ import ( // loops for the different catalog API resources. const ( - serviceClassGUID = "SCGUID" - planGUID = "PGUID" + serviceClassGUID = faketypes.ServiceClassGUID + planGUID = faketypes.PlanGUID nonbindableServiceClassGUID = "UNBINDABLE-SERVICE" - nonbindablePlanGUID = "UNBINDABLE-PLAN" - instanceGUID = "IGUID" + nonbindablePlanGUID = faketypes.NonBindablePlanGUID + instanceGUID = faketypes.InstanceGUID bindingGUID = "BGUID" - testBrokerName = "test-broker" - testServiceClassName = "test-serviceclass" + testBrokerName = faketypes.BrokerName + testServiceClassName = faketypes.ServiceClassName testNonbindableServiceClassName = "test-unbindable-serviceclass" - testPlanName = "test-plan" - testNonbindablePlanName = "test-unbindable-plan" - testInstanceName = "test-instance" + testPlanName = faketypes.PlanName + testNonbindablePlanName = faketypes.NonBindablePlanName + testInstanceName = faketypes.InstanceName testBindingName = "test-binding" - testNamespace = "test-ns" + testNamespace = faketypes.Namespace testBindingSecretName = "test-secret" testOperation = "test-operation" - testNsUID = "test-ns-uid" + testNsUID = faketypes.NamespaceUID ) var testDashboardURL = "http://dashboard" @@ -285,12 +284,7 @@ const instanceParameterSchemaBytes = `{ // broker used in most of the tests that need a broker func getTestBroker() *v1alpha1.Broker { - return &v1alpha1.Broker{ - ObjectMeta: metav1.ObjectMeta{Name: testBrokerName}, - Spec: v1alpha1.BrokerSpec{ - URL: "https://example.com", - }, - } + return faketypes.GetBroker() } func getTestBrokerWithStatus(status v1alpha1.ConditionStatus) *v1alpha1.Broker { @@ -308,28 +302,7 @@ func getTestBrokerWithStatus(status v1alpha1.ConditionStatus) *v1alpha1.Broker { // a bindable service class wired to the result of getTestBroker() func getTestServiceClass() *v1alpha1.ServiceClass { - return &v1alpha1.ServiceClass{ - ObjectMeta: metav1.ObjectMeta{Name: testServiceClassName}, - BrokerName: testBrokerName, - Description: "a test service", - ExternalID: serviceClassGUID, - Bindable: true, - Plans: []v1alpha1.ServicePlan{ - { - Name: testPlanName, - Description: "a test plan", - Free: true, - ExternalID: planGUID, - }, - { - Name: testNonbindablePlanName, - Description: "a test plan", - Free: true, - ExternalID: nonbindablePlanGUID, - Bindable: falsePtr(), - }, - }, - } + return faketypes.GetServiceClass() } // an unbindable service class wired to the result of getTestBroker() @@ -1060,39 +1033,21 @@ func getRecordedEvents(testController *controller) []string { func assertNumEvents(t *testing.T, strings []string, number int) { if e, a := number, len(strings); e != a { - fatalf(t, "Unexpected number of events: expected %v, got %v;\nevents: %+v", e, a, strings) + Fatalf(t, "Unexpected number of events: expected %v, got %v;\nevents: %+v", e, a, strings) } } -// failfFunc is a type that defines the common signatures of T.Fatalf and -// T.Errorf. -type failfFunc func(t *testing.T, msg string, args ...interface{}) - -func fatalf(t *testing.T, msg string, args ...interface{}) { - t.Log(string(debug.Stack())) - t.Fatalf(msg, args...) -} - -func errorf(t *testing.T, msg string, args ...interface{}) { - t.Log(string(debug.Stack())) - t.Errorf(msg, args...) -} - // assertion and expectation methods: // // - assertX will call t.Fatalf // - expectX will call t.Errorf and return a boolean, allowing you to drive a 'continue' // in a table-type test -func assertNumberOfActions(t *testing.T, actions []clientgotesting.Action, number int) { - testNumberOfActions(t, "" /* name */, fatalf, actions, number) -} - func expectNumberOfActions(t *testing.T, name string, actions []clientgotesting.Action, number int) bool { - return testNumberOfActions(t, name, errorf, actions, number) + return testNumberOfActions(t, name, Errorf, actions, number) } -func testNumberOfActions(t *testing.T, name string, f failfFunc, actions []clientgotesting.Action, number int) bool { +func testNumberOfActions(t *testing.T, name string, f FailfFunc, actions []clientgotesting.Action, number int) bool { logContext := "" if len(name) > 0 { logContext = name + ": " @@ -1124,7 +1079,7 @@ func assertUpdateStatus(t *testing.T, action clientgotesting.Action, obj interfa } func expectUpdateStatus(t *testing.T, name string, action clientgotesting.Action, obj interface{}) (runtime.Object, bool) { - return testActionFor(t, name, errorf, action, "update", "status", obj) + return TestActionFor(t, name, Errorf, action, "update", "status", obj) } func assertDelete(t *testing.T, action clientgotesting.Action, obj interface{}) { @@ -1132,132 +1087,10 @@ func assertDelete(t *testing.T, action clientgotesting.Action, obj interface{}) } func assertActionFor(t *testing.T, action clientgotesting.Action, verb, subresource string, obj interface{}) runtime.Object { - r, _ := testActionFor(t, "" /* name */, fatalf, action, verb, subresource, obj) + r, _ := TestActionFor(t, "" /* name */, Fatalf, action, verb, subresource, obj) return r } -func testActionFor(t *testing.T, name string, f failfFunc, action clientgotesting.Action, verb, subresource string, obj interface{}) (runtime.Object, bool) { - logContext := "" - if len(name) > 0 { - logContext = name + ": " - } - - if e, a := verb, action.GetVerb(); e != a { - f(t, "%vUnexpected verb: expected %v, got %v", logContext, e, a) - return nil, false - } - - var resource string - - switch obj.(type) { - case *v1alpha1.Broker: - resource = "brokers" - case *v1alpha1.ServiceClass: - resource = "serviceclasses" - case *v1alpha1.Instance: - resource = "instances" - case *v1alpha1.Binding: - resource = "bindings" - } - - if e, a := resource, action.GetResource().Resource; e != a { - f(t, "%vUnexpected resource; expected %v, got %v", logContext, e, a) - return nil, false - } - - if e, a := subresource, action.GetSubresource(); e != a { - f(t, "%vUnexpected subresource; expected %v, got %v", logContext, e, a) - return nil, false - } - - rtObject, ok := obj.(runtime.Object) - if !ok { - f(t, "%vObject %+v was not a runtime.Object", logContext, obj) - return nil, false - } - - paramAccessor, err := metav1.ObjectMetaFor(rtObject) - if err != nil { - f(t, "%vError creating ObjectMetaAccessor for param object %+v: %v", logContext, rtObject, err) - return nil, false - } - - var ( - objectMeta metav1.Object - fakeRtObject runtime.Object - ) - - switch verb { - case "get": - getAction, ok := action.(clientgotesting.GetAction) - if !ok { - f(t, "%vUnexpected type; failed to convert action %+v to DeleteAction", logContext, action) - return nil, false - } - - if e, a := paramAccessor.GetName(), getAction.GetName(); e != a { - f(t, "%vUnexpected name: expected %v, got %v", logContext, e, a) - return nil, false - } - - return nil, true - case "delete": - deleteAction, ok := action.(clientgotesting.DeleteAction) - if !ok { - f(t, "%vUnexpected type; failed to convert action %+v to DeleteAction", logContext, action) - return nil, false - } - - if e, a := paramAccessor.GetName(), deleteAction.GetName(); e != a { - f(t, "%vUnexpected name: expected %v, got %v", logContext, e, a) - return nil, false - } - - return nil, true - case "create": - createAction, ok := action.(clientgotesting.CreateAction) - if !ok { - f(t, "%vUnexpected type; failed to convert action %+v to CreateAction", logContext, action) - return nil, false - } - - fakeRtObject = createAction.GetObject() - objectMeta, err = metav1.ObjectMetaFor(fakeRtObject) - if err != nil { - f(t, "%vError creating ObjectMetaAccessor for %+v", logContext, fakeRtObject) - return nil, false - } - case "update": - updateAction, ok := action.(clientgotesting.UpdateAction) - if !ok { - f(t, "%vUnexpected type; failed to convert action %+v to UpdateAction", logContext, action) - return nil, false - } - - fakeRtObject = updateAction.GetObject() - objectMeta, err = metav1.ObjectMetaFor(fakeRtObject) - if err != nil { - f(t, "%vError creating ObjectMetaAccessor for %+v", logContext, fakeRtObject) - return nil, false - } - } - - if e, a := paramAccessor.GetName(), objectMeta.GetName(); e != a { - f(t, "%vUnexpected name: expected %v, got %v", logContext, e, a) - return nil, false - } - - fakeValue := reflect.ValueOf(fakeRtObject) - paramValue := reflect.ValueOf(obj) - - if e, a := paramValue.Type(), fakeValue.Type(); e != a { - f(t, "%vUnexpected type of object passed to fake client; expected %v, got %v", logContext, e, a) - return nil, false - } - - return fakeRtObject, true -} - func assertBrokerReadyTrue(t *testing.T, obj runtime.Object) { assertBrokerReadyCondition(t, obj, v1alpha1.ConditionTrue) } @@ -1269,60 +1102,16 @@ func assertBrokerReadyFalse(t *testing.T, obj runtime.Object) { func assertBrokerReadyCondition(t *testing.T, obj runtime.Object, status v1alpha1.ConditionStatus) { broker, ok := obj.(*v1alpha1.Broker) if !ok { - fatalf(t, "Couldn't convert object %+v into a *v1alpha1.Broker", obj) + Fatalf(t, "Couldn't convert object %+v into a *v1alpha1.Broker", obj) } for _, condition := range broker.Status.Conditions { if condition.Type == v1alpha1.BrokerConditionReady && condition.Status != status { - fatalf(t, "ready condition had unexpected status; expected %v, got %v", status, condition.Status) - } - } -} - -func assertInstanceReadyTrue(t *testing.T, obj runtime.Object) { - assertInstanceReadyCondition(t, obj, v1alpha1.ConditionTrue) -} - -func assertInstanceReadyFalse(t *testing.T, obj runtime.Object, reason ...string) { - assertInstanceReadyCondition(t, obj, v1alpha1.ConditionFalse, reason...) -} - -func assertInstanceReadyCondition(t *testing.T, obj runtime.Object, status v1alpha1.ConditionStatus, reason ...string) { - instance, ok := obj.(*v1alpha1.Instance) - if !ok { - fatalf(t, "Couldn't convert object %+v into a *v1alpha1.Instance", obj) - } - - for _, condition := range instance.Status.Conditions { - if condition.Type == v1alpha1.InstanceConditionReady && condition.Status != status { - fatalf(t, "ready condition had unexpected status; expected %v, got %v", status, condition.Status) - } - if len(reason) == 1 && condition.Reason != reason[0] { - fatalf(t, "unexpected reason; expected %v, got %v", reason[0], condition.Reason) + Fatalf(t, "ready condition had unexpected status; expected %v, got %v", status, condition.Status) } } } -func assertAsyncOpInProgressTrue(t *testing.T, obj runtime.Object) { - instance, ok := obj.(*v1alpha1.Instance) - if !ok { - t.Fatalf("Couldn't convert object %+v into a *v1alpha1.Instance", obj) - } - if !instance.Status.AsyncOpInProgress { - t.Fatalf("expected AsyncOpInProgress to be true but was %v", instance.Status.AsyncOpInProgress) - } -} - -func assertAsyncOpInProgressFalse(t *testing.T, obj runtime.Object) { - instance, ok := obj.(*v1alpha1.Instance) - if !ok { - t.Fatalf("Couldn't convert object %+v into a *v1alpha1.Instance", obj) - } - if instance.Status.AsyncOpInProgress { - t.Fatalf("expected AsyncOpInProgress to be false but was %v", instance.Status.AsyncOpInProgress) - } -} - func assertInstanceLastOperation(t *testing.T, obj runtime.Object, operation string) { instance, ok := obj.(*v1alpha1.Instance) if !ok { @@ -1360,16 +1149,16 @@ func assertBindingReadyFalse(t *testing.T, obj runtime.Object, reason ...string) func assertBindingReadyCondition(t *testing.T, obj runtime.Object, status v1alpha1.ConditionStatus, reason ...string) { binding, ok := obj.(*v1alpha1.Binding) if !ok { - fatalf(t, "Couldn't convert object %+v into a *v1alpha1.Binding", obj) + Fatalf(t, "Couldn't convert object %+v into a *v1alpha1.Binding", obj) } for _, condition := range binding.Status.Conditions { if condition.Type == v1alpha1.BindingConditionReady && condition.Status != status { t.Logf("ready condition: %+v", condition) - fatalf(t, "ready condition had unexpected status; expected %v, got %v", status, condition.Status) + Fatalf(t, "ready condition had unexpected status; expected %v, got %v", status, condition.Status) } if len(reason) == 1 && condition.Reason != reason[0] { - fatalf(t, "unexpected reason; expected %v, got %v", reason[0], condition.Reason) + Fatalf(t, "unexpected reason; expected %v, got %v", reason[0], condition.Reason) } } } @@ -1377,23 +1166,23 @@ func assertBindingReadyCondition(t *testing.T, obj runtime.Object, status v1alph func assertEmptyFinalizers(t *testing.T, obj runtime.Object) { accessor, err := metav1.ObjectMetaFor(obj) if err != nil { - fatalf(t, "Error creating ObjectMetaAccessor for param object %+v: %v", obj, err) + Fatalf(t, "Error creating ObjectMetaAccessor for param object %+v: %v", obj, err) } if len(accessor.GetFinalizers()) != 0 { - fatalf(t, "Unexpected number of finalizers; expected 0, got %v", len(accessor.GetFinalizers())) + Fatalf(t, "Unexpected number of finalizers; expected 0, got %v", len(accessor.GetFinalizers())) } } func assertNumberOfBrokerActions(t *testing.T, actions []fakeosb.Action, number int) { - testNumberOfBrokerActions(t, "" /* name */, fatalf, actions, number) + testNumberOfBrokerActions(t, "" /* name */, Fatalf, actions, number) } func expectNumberOfBrokerActions(t *testing.T, name string, actions []fakeosb.Action, number int) bool { - return testNumberOfBrokerActions(t, name, errorf, actions, number) + return testNumberOfBrokerActions(t, name, Errorf, actions, number) } -func testNumberOfBrokerActions(t *testing.T, name string, f failfFunc, actions []fakeosb.Action, number int) bool { +func testNumberOfBrokerActions(t *testing.T, name string, f FailfFunc, actions []fakeosb.Action, number int) bool { logContext := "" if len(name) > 0 { logContext = name + ": " @@ -1414,57 +1203,57 @@ func noFakeActions() fakeosb.FakeClientConfiguration { func assertGetCatalog(t *testing.T, action fakeosb.Action) { if e, a := fakeosb.GetCatalog, action.Type; e != a { - fatalf(t, "unexpected action type; expected %v, got %v", e, a) + Fatalf(t, "unexpected action type; expected %v, got %v", e, a) } } func assertProvision(t *testing.T, action fakeosb.Action, request *osb.ProvisionRequest) { if e, a := fakeosb.ProvisionInstance, action.Type; e != a { - fatalf(t, "unexpected action type; expected %v, got %v", e, a) + Fatalf(t, "unexpected action type; expected %v, got %v", e, a) } if e, a := request, action.Request; !reflect.DeepEqual(e, a) { - fatalf(t, "unexpected diff in provision request: %v\nexpected %+v\ngot %+v", diff.ObjectReflectDiff(e, a), e, a) + Fatalf(t, "unexpected diff in provision request: %v\nexpected %+v\ngot %+v", diff.ObjectReflectDiff(e, a), e, a) } } func assertDeprovision(t *testing.T, action fakeosb.Action, request *osb.DeprovisionRequest) { if e, a := fakeosb.DeprovisionInstance, action.Type; e != a { - fatalf(t, "unexpected action type; expected %v, got %v", e, a) + Fatalf(t, "unexpected action type; expected %v, got %v", e, a) } if e, a := request, action.Request; !reflect.DeepEqual(e, a) { - fatalf(t, "unexpected diff in deprovision request: %v\nexpected %+v\ngot %+v", diff.ObjectReflectDiff(e, a), e, a) + Fatalf(t, "unexpected diff in deprovision request: %v\nexpected %+v\ngot %+v", diff.ObjectReflectDiff(e, a), e, a) } } func assertPollLastOperation(t *testing.T, action fakeosb.Action, request *osb.LastOperationRequest) { if e, a := fakeosb.PollLastOperation, action.Type; e != a { - fatalf(t, "unexpected action type; expected %v, got %v", e, a) + Fatalf(t, "unexpected action type; expected %v, got %v", e, a) } if e, a := request, action.Request; !reflect.DeepEqual(e, a) { - fatalf(t, "unexpected diff in last operation request: %v\nexpected %+v\ngot %+v", diff.ObjectReflectDiff(e, a), e, a) + Fatalf(t, "unexpected diff in last operation request: %v\nexpected %+v\ngot %+v", diff.ObjectReflectDiff(e, a), e, a) } } func assertBind(t *testing.T, action fakeosb.Action, request *osb.BindRequest) { if e, a := fakeosb.Bind, action.Type; e != a { - fatalf(t, "unexpected action type; expected %v, got %v", e, a) + Fatalf(t, "unexpected action type; expected %v, got %v", e, a) } if e, a := request, action.Request; !reflect.DeepEqual(e, a) { - fatalf(t, "unexpected diff in bind request: %v\nexpected %+v\ngot %+v", diff.ObjectReflectDiff(e, a), e, a) + Fatalf(t, "unexpected diff in bind request: %v\nexpected %+v\ngot %+v", diff.ObjectReflectDiff(e, a), e, a) } } func assertUnbind(t *testing.T, action fakeosb.Action, request *osb.UnbindRequest) { if e, a := fakeosb.Unbind, action.Type; e != a { - fatalf(t, "unexpected action type; expected %v, got %v", e, a) + Fatalf(t, "unexpected action type; expected %v, got %v", e, a) } if e, a := request, action.Request; !reflect.DeepEqual(e, a) { - fatalf(t, "unexpected diff in bind request: %v\nexpected %+v\ngot %+v", diff.ObjectReflectDiff(e, a), e, a) + Fatalf(t, "unexpected diff in bind request: %v\nexpected %+v\ngot %+v", diff.ObjectReflectDiff(e, a), e, a) } } diff --git a/pkg/controller/fake.go b/pkg/controller/fake.go new file mode 100644 index 00000000000..cdb0e3a35dc --- /dev/null +++ b/pkg/controller/fake.go @@ -0,0 +1,95 @@ +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "net/http/httptest" + "time" + + fakebrokerserver "github.com/kubernetes-incubator/service-catalog/pkg/brokerapi/fake/server" + servicecatalogclientset "github.com/kubernetes-incubator/service-catalog/pkg/client/clientset_generated/clientset/fake" + servicecataloginformers "github.com/kubernetes-incubator/service-catalog/pkg/client/informers_generated/externalversions" + v1alpha1informers "github.com/kubernetes-incubator/service-catalog/pkg/client/informers_generated/externalversions/servicecatalog/v1alpha1" + osb "github.com/pmorie/go-open-service-broker-client/v2" + clientgofake "k8s.io/client-go/kubernetes/fake" + "k8s.io/client-go/tools/record" +) + +// TestControllerWithBrokerServer contains information about a controller that uses mock clients +// except one that points to a broker server that runs in-memory +type TestControllerWithBrokerServer struct { + FakeKubeClient *clientgofake.Clientset + FakeCatalogClient *servicecatalogclientset.Clientset + Controller Controller + Informers v1alpha1informers.Interface + BrokerServerHandler *fakebrokerserver.Handler + BrokerServer *httptest.Server +} + +// Close releases all resources associated with t. It generally should be called in a defer after +// calling NewTestControllerWithBrokerServer +func (t *TestControllerWithBrokerServer) Close() { + t.BrokerServer.Close() +} + +// NewTestControllerWithBrokerServer creates a new TestControllerWithBrokerServer, or returns a +// non-nil error if there was a problem creating it. When a non-nil TestControllerWithBrokerServer +// is returned, it should be Close()-ed when the caller is done using it +func NewTestControllerWithBrokerServer( + brokerUsername, + brokerPassword string, +) (*TestControllerWithBrokerServer, error) { + // create a fake kube client + fakeKubeClient := &clientgofake.Clientset{} + // create a fake sc client + fakeCatalogClient := &servicecatalogclientset.Clientset{} + + brokerHandler := fakebrokerserver.NewHandler() + brokerServer := fakebrokerserver.Run(brokerHandler, brokerUsername, brokerPassword) + + // create informers + informerFactory := servicecataloginformers.NewSharedInformerFactory(fakeCatalogClient, 0) + serviceCatalogSharedInformers := informerFactory.Servicecatalog().V1alpha1() + + fakeRecorder := record.NewFakeRecorder(5) + + // create a test controller + testController, err := NewController( + fakeKubeClient, + fakeCatalogClient.ServicecatalogV1alpha1(), + serviceCatalogSharedInformers.Brokers(), + serviceCatalogSharedInformers.ServiceClasses(), + serviceCatalogSharedInformers.Instances(), + serviceCatalogSharedInformers.Bindings(), + osb.NewClient, + 24*time.Hour, + true, /* enable OSB context profile */ + fakeRecorder, + ) + if err != nil { + return nil, err + } + + return &TestControllerWithBrokerServer{ + FakeKubeClient: fakeKubeClient, + FakeCatalogClient: fakeCatalogClient, + Controller: testController, + Informers: serviceCatalogSharedInformers, + BrokerServerHandler: brokerHandler, + BrokerServer: brokerServer, + }, nil +} diff --git a/pkg/controller/funcs.go b/pkg/controller/funcs.go new file mode 100644 index 00000000000..3bc24e30596 --- /dev/null +++ b/pkg/controller/funcs.go @@ -0,0 +1,37 @@ +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "runtime/debug" + "testing" +) + +// FailfFunc is a type that defines the common signatures of T.Fatalf and T.Errorf +type FailfFunc func(t *testing.T, msg string, args ...interface{}) + +// Fatalf is a FailfFunc that logs a stack trace and then calls t.Fatalf +func Fatalf(t *testing.T, msg string, args ...interface{}) { + t.Log(string(debug.Stack())) + t.Fatalf(msg, args...) +} + +// Errorf is a FailfFunc that logs a stack trace and then calls t.Errorf +func Errorf(t *testing.T, msg string, args ...interface{}) { + t.Log(string(debug.Stack())) + t.Errorf(msg, args...) +} diff --git a/pkg/controller/reactions.go b/pkg/controller/reactions.go new file mode 100644 index 00000000000..1eed8c9f4ab --- /dev/null +++ b/pkg/controller/reactions.go @@ -0,0 +1,38 @@ +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + faketypes "github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog/fake" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + clientgofake "k8s.io/client-go/kubernetes/fake" + "k8s.io/client-go/pkg/api/v1" + clientgotesting "k8s.io/client-go/testing" +) + +// AddGetNamespaceReaction adds a "get", "namespaces" reactor to fakeKubeClient for tests +func AddGetNamespaceReaction(fakeKubeClient *clientgofake.Clientset) { + fakeKubeClient.AddReactor("get", "namespaces", func(action clientgotesting.Action) (bool, runtime.Object, error) { + return true, &v1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + UID: types.UID(faketypes.NamespaceUID), + }, + }, nil + }) +} diff --git a/pkg/controller/status.go b/pkg/controller/status.go new file mode 100644 index 00000000000..a9a3590cb77 --- /dev/null +++ b/pkg/controller/status.go @@ -0,0 +1,88 @@ +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "testing" + + "github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog/v1alpha1" + "k8s.io/apimachinery/pkg/runtime" +) + +// AssertInstanceReadyTrue ensures that obj is an instance and has a ready condition of True +// on it +func AssertInstanceReadyTrue(t *testing.T, obj runtime.Object) { + AssertInstanceReadyCondition(t, obj, v1alpha1.ConditionTrue) +} + +// this function is here to maintain compatibility of existing controller test code +func assertInstanceReadyTrue(t *testing.T, obj runtime.Object) { + AssertInstanceReadyTrue(t, obj) +} + +// AssertInstanceReadyFalse ensures that obj is an instance, has a ready condition of False +// on it, and the reason for that ready condition is reason +func AssertInstanceReadyFalse(t *testing.T, obj runtime.Object, reason ...string) { + AssertInstanceReadyCondition(t, obj, v1alpha1.ConditionFalse, reason...) +} + +// this function is here to maintain compatibility of existing controller code +func assertInstanceReadyFalse(t *testing.T, obj runtime.Object, reason ...string) { + AssertInstanceReadyFalse(t, obj, reason...) +} + +// AssertAsyncOpInProgressFalse fails if obj does not have a condition on it that has the type +// of async op in progress and the status of it as false +func AssertAsyncOpInProgressFalse(t *testing.T, obj runtime.Object) { + instance, ok := obj.(*v1alpha1.Instance) + if !ok { + t.Fatalf("Couldn't convert object %+v into a *v1alpha1.Instance", obj) + } + if instance.Status.AsyncOpInProgress { + t.Fatalf("expected AsyncOpInProgress to be false but was %v", instance.Status.AsyncOpInProgress) + } +} + +// AssertAsyncOpInProgressTrue fails if obj does not have a condition on it that has the type +// of async op in progress and the status of it as true +func AssertAsyncOpInProgressTrue(t *testing.T, obj runtime.Object) { + instance, ok := obj.(*v1alpha1.Instance) + if !ok { + t.Fatalf("Couldn't convert object %+v into a *v1alpha1.Instance", obj) + } + if !instance.Status.AsyncOpInProgress { + t.Fatalf("expected AsyncOpInProgress to be true but was %v", instance.Status.AsyncOpInProgress) + } +} + +// AssertInstanceReadyCondition ensures that obj is an instance and that it has a ready condition +// with the given status and reason on it +func AssertInstanceReadyCondition(t *testing.T, obj runtime.Object, status v1alpha1.ConditionStatus, reason ...string) { + instance, ok := obj.(*v1alpha1.Instance) + if !ok { + Fatalf(t, "Couldn't convert object %+v into a *v1alpha1.Instance", obj) + } + + for _, condition := range instance.Status.Conditions { + if condition.Type == v1alpha1.InstanceConditionReady && condition.Status != status { + Fatalf(t, "ready condition had unexpected status; expected %v, got %v", status, condition.Status) + } + if len(reason) == 1 && condition.Reason != reason[0] { + Fatalf(t, "unexpected reason; expected %v, got %v", reason[0], condition.Reason) + } + } +} diff --git a/test/integration/controller_instance_test.go b/test/integration/controller_instance_test.go new file mode 100644 index 00000000000..7fd3707fde4 --- /dev/null +++ b/test/integration/controller_instance_test.go @@ -0,0 +1,138 @@ +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package integration + +import ( + "errors" + "net/http" + "testing" + + faketypes "github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog/fake" + "github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog/v1alpha1" + fakebrokerapi "github.com/kubernetes-incubator/service-catalog/pkg/brokerapi/fake" + fakebrokerserver "github.com/kubernetes-incubator/service-catalog/pkg/brokerapi/fake/server" + "github.com/kubernetes-incubator/service-catalog/pkg/controller" + osb "github.com/pmorie/go-open-service-broker-client/v2" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/pkg/api/v1" + clientgotesting "k8s.io/client-go/testing" +) + +// TestReconcileInstanceAsynchronousUnsupportedBrokerError tests to ensure that, on an asynchronous +// provision, an Instance's conditions get set with a Broker failure that is not one of the +// "expected" response codes in the OSB API spec for provision. +// See https://github.com/openservicebrokerapi/servicebroker/blob/master/spec.md#response-2 for +// the list of expected codes and a description of what we should do if another code is returned +func TestReconcileInstanceAsynchronousUnsupportedBrokerError(t *testing.T) { + const ( + brokerUsername = "testbrokeruser" + brokerPassword = "testbrokerpass" + ) + controllerItems, err := controller.NewTestControllerWithBrokerServer(brokerUsername, brokerPassword) + + if err != nil { + t.Fatal(err) + } + defer controllerItems.Close() + + fakeKubeClient := controllerItems.FakeKubeClient + fakeCatalogClient := controllerItems.FakeCatalogClient + fakeBrokerServerHandler := controllerItems.BrokerServerHandler + testController := controllerItems.Controller + sharedInformers := controllerItems.Informers + + fakeBrokerServerHandler.Catalog = fakebrokerserver.ConvertCatalog(fakebrokerapi.GetTestCatalog()) + + controller.AddGetNamespaceReaction(fakeKubeClient) + + // create the secret that the Broker resource will use for auth to the fake broker server + fakeKubeClient.AddReactor("get", "secrets", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, &v1.Secret{ + Data: map[string][]byte{ + "username": []byte(brokerUsername), + "password": []byte(brokerPassword), + }, + }, nil + }) + + // build the chain from Instance -> ServiceClass -> Broker + testBroker := faketypes.GetBroker() + testBroker.Spec.URL = controllerItems.BrokerServer.URL + testBroker.Spec.AuthInfo = &v1alpha1.BrokerAuthInfo{ + BasicAuthSecret: &v1.ObjectReference{ + Namespace: "test", + Name: "test", + }, + } + testServiceClass := faketypes.GetServiceClass() + testInstance := faketypes.GetInstance() + + sharedInformers.Brokers().Informer().GetStore().Add(testBroker) + sharedInformers.ServiceClasses().Informer().GetStore().Add(testServiceClass) + + fakeCatalogClient.AddReactor("get", "serviceclasses", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, testServiceClass, nil + }) + fakeCatalogClient.AddReactor("get", "brokers", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, testBroker, nil + }) + + // Make the provision return an error that is "unexpected" + fakeBrokerServerHandler.ProvisionRespError = errors.New("test provision error") + + // there should be nothing that is polling since no instances have been reconciled yet + if testController.PollingQueueLen() != 0 { + t.Fatalf("Expected the polling queue to be empty") + } + + reconcileErr := testController.ReconcileInstance(testInstance) + if reconcileErr == nil { + t.Fatalf("expected an error from reconcileInstance") + } + statusCodeErr, ok := reconcileErr.(osb.HTTPStatusCodeError) + if !ok { + t.Fatalf("expected an OSB client HTTPStatusCodeError, got %#v", reconcileErr) + } + if statusCodeErr.StatusCode != http.StatusInternalServerError { + t.Fatalf("expected an internal server error, got %d", statusCodeErr.StatusCode) + } + + actions := fakeCatalogClient.Actions() + controller.AssertNumberOfActions(t, actions, 1) + + // verify that 2 kubernetes actions occurred - a GET to the ServiceClass, then a GET to the + // Broker + kubeActions := fakeKubeClient.Actions() + controller.AssertNumberOfActions(t, kubeActions, 2) + + updatedInstance := controller.AssertUpdateStatus(t, actions[0], testInstance) + controller.AssertInstanceReadyFalse(t, updatedInstance) + + // there should be 1 request to the provision endpoint + numProvReqs := len(fakeBrokerServerHandler.ProvisionRequests) + if numProvReqs != 1 { + t.Fatalf("%d provision requests were made, expected 1", numProvReqs) + } + + // The item should not have been added to the polling queue for later processing + if testController.PollingQueueLen() != 0 { + t.Fatalf("Expected polling queue to be empty") + } + controller.AssertAsyncOpInProgressFalse(t, updatedInstance) + controller.AssertInstanceReadyFalse(t, updatedInstance) + controller.AssertInstanceReadyCondition(t, updatedInstance, v1alpha1.ConditionFalse) +}