From e941b81f100bd766af19d62b65a492264fcd0f88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Mi=C5=9Bkiewicz?= Date: Fri, 3 Jan 2020 12:58:20 +0100 Subject: [PATCH] Sustain service broker registration (#76) sustain broker registration if service instances exists --- Gopkg.lock | 2 + charts/helm-broker/templates/rbac.yaml | 3 + internal/controller/addons_controller.go | 7 +- internal/controller/addons_controller_test.go | 46 +--- internal/controller/broker/broker_facade.go | 2 +- internal/controller/broker_controller.go | 111 ++++++++ internal/controller/broker_controller_test.go | 136 ++++++++++ .../controller/cluster_addons_controller.go | 4 +- .../cluster_addons_controller_test.go | 49 +--- .../controller/cluster_broker_controller.go | 94 +++++++ .../cluster_broker_controller_test.go | 141 ++++++++++ internal/controller/common.go | 28 +- internal/controller/common_test.go | 10 - internal/controller/ext.go | 5 + internal/controller/instance/facade.go | 95 +++++++ internal/controller/instance/facade_test.go | 119 +++++++++ internal/controller/setup.go | 11 + internal/helm/client.go | 10 +- internal/helm/dep.go | 3 +- internal/helm/fake.go | 7 + .../crds/sc/clusterserviceclass.yaml | 30 +++ test/integration/crds/sc/serviceclass.yaml | 30 +++ test/integration/crds/sc/serviceinstance.yaml | 33 +++ test/integration/integration_test.go | 75 ++++++ test/integration/suite_test.go | 250 +++++++++++++++++- 25 files changed, 1183 insertions(+), 118 deletions(-) create mode 100644 internal/controller/broker_controller.go create mode 100644 internal/controller/broker_controller_test.go create mode 100644 internal/controller/cluster_broker_controller.go create mode 100644 internal/controller/cluster_broker_controller_test.go create mode 100644 internal/controller/instance/facade.go create mode 100644 internal/controller/instance/facade_test.go create mode 100644 internal/helm/fake.go create mode 100644 test/integration/crds/sc/clusterserviceclass.yaml create mode 100644 test/integration/crds/sc/serviceclass.yaml create mode 100644 test/integration/crds/sc/serviceinstance.yaml diff --git a/Gopkg.lock b/Gopkg.lock index 66b54bd5..60823e91 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -2068,8 +2068,10 @@ "sigs.k8s.io/controller-runtime/pkg/client/fake", "sigs.k8s.io/controller-runtime/pkg/controller", "sigs.k8s.io/controller-runtime/pkg/envtest", + "sigs.k8s.io/controller-runtime/pkg/event", "sigs.k8s.io/controller-runtime/pkg/handler", "sigs.k8s.io/controller-runtime/pkg/manager", + "sigs.k8s.io/controller-runtime/pkg/predicate", "sigs.k8s.io/controller-runtime/pkg/reconcile", "sigs.k8s.io/controller-runtime/pkg/runtime/scheme", "sigs.k8s.io/controller-runtime/pkg/runtime/signals", diff --git a/charts/helm-broker/templates/rbac.yaml b/charts/helm-broker/templates/rbac.yaml index 9ccb5bbc..20eba348 100644 --- a/charts/helm-broker/templates/rbac.yaml +++ b/charts/helm-broker/templates/rbac.yaml @@ -17,6 +17,9 @@ rules: - apiGroups: ["servicecatalog.k8s.io"] resources: ["servicebrokers", "clusterservicebrokers"] verbs: ["create","delete","list","get","update", "watch"] +- apiGroups: ["servicecatalog.k8s.io"] + resources: ["serviceinstance", "serviceclass", "clusterserviceclass"] + verbs: ["list","get","update", "watch"] - apiGroups: ["rafter.kyma-project.io"] resources: ["clusterassetgroups", "assetgroups"] verbs: ["get", "create", "update", "delete", "list", "watch"] diff --git a/internal/controller/addons_controller.go b/internal/controller/addons_controller.go index f596265f..4cbbd866 100644 --- a/internal/controller/addons_controller.go +++ b/internal/controller/addons_controller.go @@ -27,12 +27,15 @@ type ReconcileAddonsConfiguration struct { } // NewReconcileAddonsConfiguration returns a new reconcile.Reconciler -func NewReconcileAddonsConfiguration(mgr manager.Manager, addonGetterFactory addonGetterFactory, chartStorage chartStorage, addonStorage addonStorage, brokerFacade brokerFacade, docsProvider docsProvider, brokerSyncer brokerSyncer, templateService templateService, tmpDir string, log logrus.FieldLogger) reconcile.Reconciler { +func NewReconcileAddonsConfiguration(mgr manager.Manager, addonGetterFactory addonGetterFactory, + chartStorage chartStorage, addonStorage addonStorage, brokerFacade brokerFacade, docsProvider docsProvider, + brokerSyncer brokerSyncer, templateService templateService, tmpDir string, log logrus.FieldLogger) reconcile.Reconciler { return &ReconcileAddonsConfiguration{ log: log.WithField("controller", "addons"), Client: mgr.GetClient(), - common: newControllerCommon(mgr.GetClient(), addonGetterFactory, addonStorage, chartStorage, docsProvider, brokerSyncer, brokerFacade, templateService, path.Join(tmpDir, "addon-loader-dst"), log), + common: newControllerCommon(mgr.GetClient(), addonGetterFactory, addonStorage, chartStorage, + docsProvider, brokerSyncer, brokerFacade, templateService, path.Join(tmpDir, "addon-loader-dst"), log), } } diff --git a/internal/controller/addons_controller_test.go b/internal/controller/addons_controller_test.go index 353786d9..7250a356 100644 --- a/internal/controller/addons_controller_test.go +++ b/internal/controller/addons_controller_test.go @@ -61,12 +61,12 @@ func TestReconcileAddonsConfiguration_AddAddonsProcess(t *testing.T) { } } ts.brokerFacade.On("Exist").Return(false, nil).Once() - ts.brokerFacade.On("Create").Return(nil).Once() ts.addonGetterFactory.On("NewGetter", fixAddonsCfg.Spec.Repositories[0].URL, path.Join(tmpDir, "addon-loader-dst")).Return(ts.addonGetter, nil).Once() defer ts.assertExpectations() // WHEN - reconciler := NewReconcileAddonsConfiguration(ts.mgr, ts.addonGetterFactory, ts.chartStorage, ts.addonStorage, ts.brokerFacade, ts.docsProvider, ts.brokerSyncer, ts.templateService, tmpDir, spy.NewLogDummy()) + reconciler := NewReconcileAddonsConfiguration(ts.mgr, ts.addonGetterFactory, ts.chartStorage, ts.addonStorage, + ts.brokerFacade, ts.docsProvider, ts.brokerSyncer, ts.templateService, tmpDir, spy.NewLogDummy()) // THEN result, err := reconciler.Reconcile(reconcile.Request{NamespacedName: types.NamespacedName{Namespace: fixAddonsCfg.Namespace, Name: fixAddonsCfg.Name}}) @@ -105,7 +105,8 @@ func TestReconcileAddonsConfiguration_AddAddonsProcess_ErrorIfBrokerExist(t *tes defer ts.assertExpectations() // WHEN - reconciler := NewReconcileAddonsConfiguration(ts.mgr, ts.addonGetterFactory, ts.chartStorage, ts.addonStorage, ts.brokerFacade, ts.docsProvider, ts.brokerSyncer, ts.templateService, tmpDir, spy.NewLogDummy()) + reconciler := NewReconcileAddonsConfiguration(ts.mgr, ts.addonGetterFactory, ts.chartStorage, ts.addonStorage, + ts.brokerFacade, ts.docsProvider, ts.brokerSyncer, ts.templateService, tmpDir, spy.NewLogDummy()) // THEN result, err := reconciler.Reconcile(reconcile.Request{NamespacedName: types.NamespacedName{Namespace: fixAddonsCfg.Namespace, Name: fixAddonsCfg.Name}}) @@ -139,12 +140,12 @@ func TestReconcileAddonsConfiguration_UpdateAddonsProcess(t *testing.T) { } } ts.brokerFacade.On("Exist").Return(false, nil).Once() - ts.brokerFacade.On("Create").Return(nil).Once() ts.addonGetterFactory.On("NewGetter", fixAddonsCfg.Spec.Repositories[0].URL, path.Join(tmpDir, "addon-loader-dst")).Return(ts.addonGetter, nil).Once() defer ts.assertExpectations() // WHEN - reconciler := NewReconcileAddonsConfiguration(ts.mgr, ts.addonGetterFactory, ts.chartStorage, ts.addonStorage, ts.brokerFacade, ts.docsProvider, ts.brokerSyncer, ts.templateService, tmpDir, spy.NewLogDummy()) + reconciler := NewReconcileAddonsConfiguration(ts.mgr, ts.addonGetterFactory, ts.chartStorage, ts.addonStorage, + ts.brokerFacade, ts.docsProvider, ts.brokerSyncer, ts.templateService, tmpDir, spy.NewLogDummy()) // THEN result, err := reconciler.Reconcile(reconcile.Request{NamespacedName: types.NamespacedName{Namespace: fixAddonsCfg.Namespace, Name: fixAddonsCfg.Name}}) @@ -175,7 +176,8 @@ func TestReconcileAddonsConfiguration_UpdateAddonsProcess_ConflictingAddons(t *t defer ts.assertExpectations() // WHEN - reconciler := NewReconcileAddonsConfiguration(ts.mgr, ts.addonGetterFactory, ts.chartStorage, ts.addonStorage, ts.brokerFacade, ts.docsProvider, ts.brokerSyncer, ts.templateService, tmpDir, spy.NewLogDummy()) + reconciler := NewReconcileAddonsConfiguration(ts.mgr, ts.addonGetterFactory, ts.chartStorage, ts.addonStorage, + ts.brokerFacade, ts.docsProvider, ts.brokerSyncer, ts.templateService, tmpDir, spy.NewLogDummy()) // THEN result, err := reconciler.Reconcile(reconcile.Request{NamespacedName: types.NamespacedName{Namespace: fixAddonsCfg.Namespace, Name: fixAddonsCfg.Name}}) @@ -195,11 +197,11 @@ func TestReconcileAddonsConfiguration_DeleteAddonsProcess(t *testing.T) { ts := getTestSuite(t, fixAddonsCfg) tmpDir := os.TempDir() - ts.brokerFacade.On("Delete").Return(nil).Once() defer ts.assertExpectations() // WHEN - reconciler := NewReconcileAddonsConfiguration(ts.mgr, ts.addonGetterFactory, ts.chartStorage, ts.addonStorage, ts.brokerFacade, ts.docsProvider, ts.brokerSyncer, ts.templateService, tmpDir, spy.NewLogDummy()) + reconciler := NewReconcileAddonsConfiguration(ts.mgr, ts.addonGetterFactory, ts.chartStorage, ts.addonStorage, + ts.brokerFacade, ts.docsProvider, ts.brokerSyncer, ts.templateService, tmpDir, spy.NewLogDummy()) // THEN result, err := reconciler.Reconcile(reconcile.Request{NamespacedName: types.NamespacedName{Namespace: fixAddonsCfg.Namespace, Name: fixAddonsCfg.Name}}) @@ -219,11 +221,11 @@ func TestReconcileAddonsConfiguration_DeleteAddonsProcess_ReconcileOtherAddons(t ts := getTestSuite(t, fixAddonsCfg, failedAddCfg) tmpDir := os.TempDir() - ts.brokerFacade.On("Delete").Return(nil).Once() defer ts.assertExpectations() // WHEN - reconciler := NewReconcileAddonsConfiguration(ts.mgr, ts.addonGetterFactory, ts.chartStorage, ts.addonStorage, ts.brokerFacade, ts.docsProvider, ts.brokerSyncer, ts.templateService, tmpDir, spy.NewLogDummy()) + reconciler := NewReconcileAddonsConfiguration(ts.mgr, ts.addonGetterFactory, ts.chartStorage, ts.addonStorage, + ts.brokerFacade, ts.docsProvider, ts.brokerSyncer, ts.templateService, tmpDir, spy.NewLogDummy()) // THEN result, err := reconciler.Reconcile(reconcile.Request{NamespacedName: types.NamespacedName{Namespace: fixAddonsCfg.Namespace, Name: fixAddonsCfg.Name}}) @@ -241,30 +243,6 @@ func TestReconcileAddonsConfiguration_DeleteAddonsProcess_ReconcileOtherAddons(t assert.NotContains(t, res.Finalizers, v1alpha1.FinalizerAddonsConfiguration) } -func TestReconcileAddonsConfiguration_DeleteAddonsProcess_Error(t *testing.T) { - // GIVEN - fixAddonsCfg := fixDeletedAddonsConfiguration() - ts := getTestSuite(t, fixAddonsCfg) - tmpDir := os.TempDir() - - ts.brokerFacade.On("Delete").Return(errors.New("")).Once() - defer ts.assertExpectations() - - // WHEN - reconciler := NewReconcileAddonsConfiguration(ts.mgr, ts.addonGetterFactory, ts.chartStorage, ts.addonStorage, ts.brokerFacade, ts.docsProvider, ts.brokerSyncer, ts.templateService, tmpDir, spy.NewLogDummy()) - - // THEN - result, err := reconciler.Reconcile(reconcile.Request{NamespacedName: types.NamespacedName{Namespace: fixAddonsCfg.Namespace, Name: fixAddonsCfg.Name}}) - assert.Error(t, err) - assert.False(t, result.Requeue) - assert.Equal(t, result.RequeueAfter, time.Second*15) - - res := v1alpha1.AddonsConfiguration{} - err = ts.mgr.GetClient().Get(context.Background(), types.NamespacedName{Namespace: fixAddonsCfg.Namespace, Name: fixAddonsCfg.Name}, &res) - assert.NoError(t, err) - assert.Contains(t, res.Finalizers, v1alpha1.FinalizerAddonsConfiguration) -} - func fixRepositories() []v1alpha1.StatusRepository { return []v1alpha1.StatusRepository{ { diff --git a/internal/controller/broker/broker_facade.go b/internal/controller/broker/broker_facade.go index 427d10e7..67d62180 100644 --- a/internal/controller/broker/broker_facade.go +++ b/internal/controller/broker/broker_facade.go @@ -24,7 +24,7 @@ const ( BrokerLabelValue = "true" ) -// Facade is responsible for creation k8s objects for namespaced broker +// Facade is responsible for creation k8s objects for namespaced broker. The Facade is not thread-safe. type Facade struct { client client.Client systemNamespace string diff --git a/internal/controller/broker_controller.go b/internal/controller/broker_controller.go new file mode 100644 index 00000000..3cb8a0b7 --- /dev/null +++ b/internal/controller/broker_controller.go @@ -0,0 +1,111 @@ +package controller + +import ( + "context" + + "github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog/v1beta1" + "github.com/kyma-project/helm-broker/internal/controller/broker" + "github.com/kyma-project/helm-broker/pkg/apis/addons/v1alpha1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/handler" + "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/predicate" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + "sigs.k8s.io/controller-runtime/pkg/source" +) + +// BrokerController is a controller which reacts on changes for ServiceInstance, ServiceBroker and AddonsConfiguration. +// Only this controller should create/delete ServiceBroker. +type BrokerController struct { + instanceChecker instanceChecker + cli client.Client + + namespacedBrokerFacade brokerFacade +} + +var createDeletePredicate = predicate.Funcs{ + CreateFunc: func(_ event.CreateEvent) bool { return true }, + DeleteFunc: func(_ event.DeleteEvent) bool { return true }, + UpdateFunc: func(_ event.UpdateEvent) bool { return false }, +} + +var eventHandler = &handler.EnqueueRequestsFromMapFunc{ + ToRequests: handler.ToRequestsFunc( + func(mp handler.MapObject) []reconcile.Request { + return []reconcile.Request{{NamespacedName: types.NamespacedName{Namespace: mp.Meta.GetNamespace()}}} + }, + )} + +// NewBrokerController creates BrokerController instance. +func NewBrokerController(checker instanceChecker, cli client.Client, bFacade brokerFacade) *BrokerController { + return &BrokerController{ + instanceChecker: checker, + cli: cli, + namespacedBrokerFacade: bFacade, + } +} + +// Start starts the controller +func (sbc *BrokerController) Start(mgr manager.Manager) error { + // Create a new controller + c, err := controller.New("broker-controller", mgr, controller.Options{Reconciler: sbc}) + if err != nil { + return err + } + + // Watch for changes to ServiceInstance + err = c.Watch(&source.Kind{Type: &v1beta1.ServiceInstance{}}, eventHandler, createDeletePredicate) + if err != nil { + return err + } + + err = c.Watch(&source.Kind{Type: &v1alpha1.AddonsConfiguration{}}, eventHandler, createDeletePredicate) + if err != nil { + return err + } + + err = c.Watch(&source.Kind{Type: &v1beta1.ServiceBroker{}}, eventHandler, predicate.Funcs{ + CreateFunc: func(e event.CreateEvent) bool { return e.Meta.GetName() == broker.NamespacedBrokerName }, + DeleteFunc: func(e event.DeleteEvent) bool { return e.Meta.GetName() == broker.NamespacedBrokerName }, + UpdateFunc: func(_ event.UpdateEvent) bool { return false }, + }) + if err != nil { + return err + } + + return nil +} + +// Reconcile checks if the (cluster) service broker must be removed +func (sbc *BrokerController) Reconcile(request reconcile.Request) (reconcile.Result, error) { + currentNamespace := request.Namespace + + sbc.namespacedBrokerFacade.SetNamespace(request.Namespace) + sbExists, err := sbc.namespacedBrokerFacade.Exist() + if err != nil { + return reconcile.Result{}, err + } + // list addons configurations + acList := v1alpha1.AddonsConfigurationList{} + err = sbc.cli.List(context.TODO(), &client.ListOptions{Namespace: currentNamespace}, &acList) + if err != nil { + return reconcile.Result{}, err + } + anyNamespacedConfigExists := len(acList.Items) > 0 + + instanceByNamespacedExists, err := sbc.instanceChecker.AnyServiceInstanceExistsForNamespacedServiceBroker(currentNamespace) + if err != nil { + return reconcile.Result{}, err + } + if sbExists && !anyNamespacedConfigExists && !instanceByNamespacedExists { + sbc.namespacedBrokerFacade.Delete() + } + if !sbExists && (anyNamespacedConfigExists || instanceByNamespacedExists) { + sbc.namespacedBrokerFacade.Create() + } + + return reconcile.Result{}, nil +} diff --git a/internal/controller/broker_controller_test.go b/internal/controller/broker_controller_test.go new file mode 100644 index 00000000..5434559d --- /dev/null +++ b/internal/controller/broker_controller_test.go @@ -0,0 +1,136 @@ +package controller_test + +import ( + "context" + "testing" + + "github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog/v1beta1" + "github.com/kyma-project/helm-broker/internal/controller" + "github.com/kyma-project/helm-broker/internal/controller/broker" + "github.com/kyma-project/helm-broker/internal/controller/instance" + "github.com/kyma-project/helm-broker/internal/platform/logger/spy" + "github.com/kyma-project/helm-broker/pkg/apis/addons/v1alpha1" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/kubernetes/scheme" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + "k8s.io/apimachinery/pkg/api/errors" +) + +const ( + readyAddonsConfigName = "readyAC" +) + +func TestBrokerControllerReconcile_CreateSB(t *testing.T) { + // given + svc, cli := prepareBrokerController(t, fixReadyAddonsConfiguration()) + + // when + res, err := svc.Reconcile(fixRequest()) + + // then + require.NoError(t, err) + assert.False(t, res.Requeue) + _, err = getServiceBroker(cli, "default") + assert.NoError(t, err) +} + +func TestBrokerControllerReconcile_DeleteSB(t *testing.T) { + // given + svc, cli := prepareBrokerController(t, fixServiceBroker()) + + // when + res, err := svc.Reconcile(fixRequest()) + + // then + require.NoError(t, err) + assert.False(t, res.Requeue) + _, err = getServiceBroker(cli, "default") + assert.True(t, errors.IsNotFound(err)) +} + +func TestBrokerControllerReconcile_BlockDeletionByExistingInstances(t *testing.T) { + // given + svc, cli := prepareBrokerController(t, fixServiceBroker(), fixServiceInstance(), fixServiceClass()) + + // when + res, err := svc.Reconcile(fixRequest()) + + // then + require.NoError(t, err) + assert.False(t, res.Requeue) + _, err = getServiceBroker(cli, "default") + assert.NoError(t, err) +} + +func getServiceBroker(cli client.Client, ns string) (*v1beta1.ServiceBroker, error) { + var obj v1beta1.ServiceBroker + err := cli.Get(context.TODO(), client.ObjectKey{Namespace: "default", Name: broker.NamespacedBrokerName}, &obj) + return &obj, err +} + +func fixReadyAddonsConfiguration() *v1alpha1.AddonsConfiguration { + return &v1alpha1.AddonsConfiguration{ + ObjectMeta: metav1.ObjectMeta{ + Name: readyAddonsConfigName, + Namespace: "default", + }, + Spec: v1alpha1.AddonsConfigurationSpec{ + CommonAddonsConfigurationSpec: v1alpha1.CommonAddonsConfigurationSpec{}, + }, + Status: v1alpha1.AddonsConfigurationStatus{ + CommonAddonsConfigurationStatus: v1alpha1.CommonAddonsConfigurationStatus{ + Phase: v1alpha1.AddonsConfigurationReady, + }, + }, + } +} + +func fixServiceBroker() *v1beta1.ServiceBroker { + return &v1beta1.ServiceBroker{ + ObjectMeta: metav1.ObjectMeta{ + Name: broker.NamespacedBrokerName, + Namespace: "default", + }, + Spec: v1beta1.ServiceBrokerSpec{}, + } +} + +func fixServiceInstance() *v1beta1.ServiceInstance { + return &v1beta1.ServiceInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: "si", + Namespace: "default", + }, + Spec: v1beta1.ServiceInstanceSpec{ + ServiceClassRef: &v1beta1.LocalObjectReference{ + Name: "sc-service", + }, + }} +} + +func fixServiceClass() *v1beta1.ServiceClass { + return &v1beta1.ServiceClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "sc-service", + Namespace: "default", + }, + Spec: v1beta1.ServiceClassSpec{ + ServiceBrokerName: broker.NamespacedBrokerName, + }} +} + +func prepareBrokerController(t *testing.T, initObjs ...runtime.Object) (*controller.BrokerController, client.Client) { + require.NoError(t, v1beta1.AddToScheme(scheme.Scheme)) + require.NoError(t, v1alpha1.AddToScheme(scheme.Scheme)) + cli := fake.NewFakeClientWithScheme(scheme.Scheme, initObjs...) + iChecker := instance.New(cli, broker.NamespacedBrokerName) + bFacade := broker.NewBrokersFacade(cli, "default", broker.NamespacedBrokerName, spy.NewLogDummy()) + svc := controller.NewBrokerController(iChecker, cli, bFacade) + + return svc, cli +} diff --git a/internal/controller/cluster_addons_controller.go b/internal/controller/cluster_addons_controller.go index 77eda460..a2bf2fc6 100644 --- a/internal/controller/cluster_addons_controller.go +++ b/internal/controller/cluster_addons_controller.go @@ -27,7 +27,9 @@ type ReconcileClusterAddonsConfiguration struct { } // NewReconcileClusterAddonsConfiguration returns a new reconcile.Reconciler -func NewReconcileClusterAddonsConfiguration(mgr manager.Manager, addonGetterFactory addonGetterFactory, chartStorage chartStorage, addonStorage addonStorage, brokerFacade brokerFacade, docsProvider docsProvider, brokerSyncer brokerSyncer, templateService templateService, tmpDir string, log logrus.FieldLogger) reconcile.Reconciler { +func NewReconcileClusterAddonsConfiguration(mgr manager.Manager, addonGetterFactory addonGetterFactory, chartStorage chartStorage, + addonStorage addonStorage, brokerFacade brokerFacade, docsProvider docsProvider, brokerSyncer brokerSyncer, + templateService templateService, tmpDir string, log logrus.FieldLogger) reconcile.Reconciler { return &ReconcileClusterAddonsConfiguration{ log: log.WithField("controller", "cluster-addons"), Client: mgr.GetClient(), diff --git a/internal/controller/cluster_addons_controller_test.go b/internal/controller/cluster_addons_controller_test.go index 22314885..2dbb987b 100644 --- a/internal/controller/cluster_addons_controller_test.go +++ b/internal/controller/cluster_addons_controller_test.go @@ -51,12 +51,12 @@ func TestReconcileClusterAddonsConfiguration_AddAddonsProcess(t *testing.T) { } } ts.brokerFacade.On("Exist").Return(false, nil).Once() - ts.brokerFacade.On("Create").Return(nil).Once() ts.addonGetterFactory.On("NewGetter", fixAddonsCfg.Spec.Repositories[0].URL, path.Join(tmpDir, "cluster-addon-loader-dst")).Return(ts.addonGetter, nil).Once() defer ts.assertExpectations() // WHEN - reconciler := NewReconcileClusterAddonsConfiguration(ts.mgr, ts.addonGetterFactory, ts.chartStorage, ts.addonStorage, ts.brokerFacade, ts.docsProvider, ts.brokerSyncer, ts.templateService, os.TempDir(), spy.NewLogDummy()) + reconciler := NewReconcileClusterAddonsConfiguration(ts.mgr, ts.addonGetterFactory, ts.chartStorage, ts.addonStorage, + ts.brokerFacade, ts.docsProvider, ts.brokerSyncer, ts.templateService, os.TempDir(), spy.NewLogDummy()) // THEN result, err := reconciler.Reconcile(reconcile.Request{NamespacedName: types.NamespacedName{Name: fixAddonsCfg.Name}}) @@ -96,7 +96,8 @@ func TestReconcileClusterAddonsConfiguration_AddAddonsProcess_Error(t *testing.T defer ts.assertExpectations() // WHEN - reconciler := NewReconcileClusterAddonsConfiguration(ts.mgr, ts.addonGetterFactory, ts.chartStorage, ts.addonStorage, ts.brokerFacade, ts.docsProvider, ts.brokerSyncer, ts.templateService, os.TempDir(), spy.NewLogDummy()) + reconciler := NewReconcileClusterAddonsConfiguration(ts.mgr, ts.addonGetterFactory, ts.chartStorage, ts.addonStorage, + ts.brokerFacade, ts.docsProvider, ts.brokerSyncer, ts.templateService, os.TempDir(), spy.NewLogDummy()) // THEN result, err := reconciler.Reconcile(reconcile.Request{NamespacedName: types.NamespacedName{Name: fixAddonsCfg.Name}}) @@ -131,12 +132,12 @@ func TestReconcileClusterAddonsConfiguration_UpdateAddonsProcess(t *testing.T) { } ts.brokerFacade.On("Exist").Return(false, nil).Once() - ts.brokerFacade.On("Create").Return(nil).Once() ts.addonGetterFactory.On("NewGetter", fixAddonsCfg.Spec.Repositories[0].URL, path.Join(tmpDir, "cluster-addon-loader-dst")).Return(ts.addonGetter, nil).Once() defer ts.assertExpectations() // WHEN - reconciler := NewReconcileClusterAddonsConfiguration(ts.mgr, ts.addonGetterFactory, ts.chartStorage, ts.addonStorage, ts.brokerFacade, ts.docsProvider, ts.brokerSyncer, ts.templateService, os.TempDir(), spy.NewLogDummy()) + reconciler := NewReconcileClusterAddonsConfiguration(ts.mgr, ts.addonGetterFactory, ts.chartStorage, ts.addonStorage, + ts.brokerFacade, ts.docsProvider, ts.brokerSyncer, ts.templateService, os.TempDir(), spy.NewLogDummy()) // THEN result, err := reconciler.Reconcile(reconcile.Request{NamespacedName: types.NamespacedName{Name: fixAddonsCfg.Name}}) @@ -170,7 +171,8 @@ func TestReconcileClusterAddonsConfiguration_UpdateAddonsProcess_ConflictingAddo defer ts.assertExpectations() // WHEN - reconciler := NewReconcileClusterAddonsConfiguration(ts.mgr, ts.addonGetterFactory, ts.chartStorage, ts.addonStorage, ts.brokerFacade, ts.docsProvider, ts.brokerSyncer, ts.templateService, os.TempDir(), spy.NewLogDummy()) + reconciler := NewReconcileClusterAddonsConfiguration(ts.mgr, ts.addonGetterFactory, ts.chartStorage, + ts.addonStorage, ts.brokerFacade, ts.docsProvider, ts.brokerSyncer, ts.templateService, os.TempDir(), spy.NewLogDummy()) // THEN result, err := reconciler.Reconcile(reconcile.Request{NamespacedName: types.NamespacedName{Name: fixAddonsCfg.Name}}) @@ -189,11 +191,9 @@ func TestReconcileClusterAddonsConfiguration_DeleteAddonsProcess(t *testing.T) { fixAddonsCfg := fixDeletedClusterAddonsConfiguration() ts := getClusterTestSuite(t, fixAddonsCfg) - ts.brokerFacade.On("Delete").Return(nil).Once() - defer ts.assertExpectations() - // WHEN - reconciler := NewReconcileClusterAddonsConfiguration(ts.mgr, ts.addonGetterFactory, ts.chartStorage, ts.addonStorage, ts.brokerFacade, ts.docsProvider, ts.brokerSyncer, ts.templateService, os.TempDir(), spy.NewLogDummy()) + reconciler := NewReconcileClusterAddonsConfiguration(ts.mgr, ts.addonGetterFactory, ts.chartStorage, ts.addonStorage, + ts.brokerFacade, ts.docsProvider, ts.brokerSyncer, ts.templateService, os.TempDir(), spy.NewLogDummy()) // THEN result, err := reconciler.Reconcile(reconcile.Request{NamespacedName: types.NamespacedName{Name: fixAddonsCfg.Name}}) @@ -212,11 +212,9 @@ func TestReconcileClusterAddonsConfiguration_DeleteAddonsProcess_ReconcileOtherA fixAddonsCfg := fixDeletedClusterAddonsConfiguration() ts := getClusterTestSuite(t, fixAddonsCfg, failedAddCfg) - ts.brokerFacade.On("Delete").Return(nil).Once() - defer ts.assertExpectations() - // WHEN - reconciler := NewReconcileClusterAddonsConfiguration(ts.mgr, ts.addonGetterFactory, ts.chartStorage, ts.addonStorage, ts.brokerFacade, ts.docsProvider, ts.brokerSyncer, ts.templateService, os.TempDir(), spy.NewLogDummy()) + reconciler := NewReconcileClusterAddonsConfiguration(ts.mgr, ts.addonGetterFactory, ts.chartStorage, ts.addonStorage, + ts.brokerFacade, ts.docsProvider, ts.brokerSyncer, ts.templateService, os.TempDir(), spy.NewLogDummy()) // THEN result, err := reconciler.Reconcile(reconcile.Request{NamespacedName: types.NamespacedName{Name: fixAddonsCfg.Name}}) @@ -234,29 +232,6 @@ func TestReconcileClusterAddonsConfiguration_DeleteAddonsProcess_ReconcileOtherA assert.NotContains(t, res.Finalizers, v1alpha1.FinalizerAddonsConfiguration) } -func TestReconcileClusterAddonsConfiguration_DeleteAddonsProcess_Error(t *testing.T) { - // GIVEN - fixAddonsCfg := fixDeletedClusterAddonsConfiguration() - ts := getClusterTestSuite(t, fixAddonsCfg) - - ts.brokerFacade.On("Delete").Return(errors.New("")).Once() - defer ts.assertExpectations() - - // WHEN - reconciler := NewReconcileClusterAddonsConfiguration(ts.mgr, ts.addonGetterFactory, ts.chartStorage, ts.addonStorage, ts.brokerFacade, ts.docsProvider, ts.brokerSyncer, ts.templateService, os.TempDir(), spy.NewLogDummy()) - - // THEN - result, err := reconciler.Reconcile(reconcile.Request{NamespacedName: types.NamespacedName{Name: fixAddonsCfg.Name}}) - assert.Error(t, err) - assert.False(t, result.Requeue) - assert.Equal(t, result.RequeueAfter, time.Second*15) - - res := v1alpha1.ClusterAddonsConfiguration{} - err = ts.mgr.GetClient().Get(context.Background(), types.NamespacedName{Name: fixAddonsCfg.Name}, &res) - assert.NoError(t, err) - assert.Contains(t, res.Finalizers, v1alpha1.FinalizerAddonsConfiguration) -} - type clusterTestSuite struct { t *testing.T mgr manager.Manager diff --git a/internal/controller/cluster_broker_controller.go b/internal/controller/cluster_broker_controller.go new file mode 100644 index 00000000..08155c96 --- /dev/null +++ b/internal/controller/cluster_broker_controller.go @@ -0,0 +1,94 @@ +package controller + +import ( + "context" + + "github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog/v1beta1" + "github.com/kyma-project/helm-broker/pkg/apis/addons/v1alpha1" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/predicate" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + "sigs.k8s.io/controller-runtime/pkg/source" +) + +// ClusterBrokerController is a controller which reacts on changes for ServiceInstance, ClusterServiceBroker and ClusterAddonsConfiguration. +// Only this controller should create/delete ClusterServiceBroker. +type ClusterBrokerController struct { + instanceChecker instanceChecker + cli client.Client + + clusterBrokerFacade brokerFacade + clusterBrokerName string +} + +// NewClusterBrokerController creates ClusterBrokerController instance. +func NewClusterBrokerController(checker instanceChecker, cli client.Client, bFacade brokerFacade, clusterBrokerName string) *ClusterBrokerController { + return &ClusterBrokerController{ + instanceChecker: checker, + cli: cli, + clusterBrokerFacade: bFacade, + clusterBrokerName: clusterBrokerName, + } +} + +// Start starts the controller +func (sbc *ClusterBrokerController) Start(mgr manager.Manager) error { + // Create a new controller + c, err := controller.New("cluster-broker-controller", mgr, controller.Options{Reconciler: sbc}) + if err != nil { + return err + } + + // Watch for changes to ServiceInstance, ClusterAddonsConfiguration, ClusterServiceBroker + err = c.Watch(&source.Kind{Type: &v1beta1.ServiceInstance{}}, eventHandler, createDeletePredicate) + if err != nil { + return err + } + + err = c.Watch(&source.Kind{Type: &v1alpha1.ClusterAddonsConfiguration{}}, eventHandler, createDeletePredicate) + if err != nil { + return err + } + + err = c.Watch(&source.Kind{Type: &v1beta1.ClusterServiceBroker{}}, eventHandler, predicate.Funcs{ + // filter out all other ClusterServiceBroker, only "helm-broker" is interesting for us + CreateFunc: func(e event.CreateEvent) bool { return e.Meta.GetName() == sbc.clusterBrokerName }, + DeleteFunc: func(e event.DeleteEvent) bool { return e.Meta.GetName() == sbc.clusterBrokerName }, + UpdateFunc: func(_ event.UpdateEvent) bool { return false }, + }) + if err != nil { + return err + } + + return nil +} + +// Reconcile checks if the cluster service broker must be removed +func (sbc *ClusterBrokerController) Reconcile(request reconcile.Request) (reconcile.Result, error) { + csbExists, err := sbc.clusterBrokerFacade.Exist() + if err != nil { + return reconcile.Result{}, err + } + cacList := v1alpha1.ClusterAddonsConfigurationList{} + err = sbc.cli.List(context.TODO(), &client.ListOptions{}, &cacList) + if err != nil { + return reconcile.Result{}, err + } + anyClusterAddonsConfigExists := len(cacList.Items) > 0 + instanceExists, err := sbc.instanceChecker.AnyServiceInstanceExistsForClusterServiceBroker() + if err != nil { + return reconcile.Result{}, err + } + + if csbExists && (!anyClusterAddonsConfigExists && !instanceExists) { + sbc.clusterBrokerFacade.Delete() + } + if !csbExists && (anyClusterAddonsConfigExists || instanceExists) { + sbc.clusterBrokerFacade.Create() + } + + return reconcile.Result{}, nil +} diff --git a/internal/controller/cluster_broker_controller_test.go b/internal/controller/cluster_broker_controller_test.go new file mode 100644 index 00000000..6e155063 --- /dev/null +++ b/internal/controller/cluster_broker_controller_test.go @@ -0,0 +1,141 @@ +package controller_test + +import ( + "context" + "testing" + + "github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog/v1beta1" + "github.com/kyma-project/helm-broker/internal/controller" + "github.com/kyma-project/helm-broker/internal/controller/broker" + "github.com/kyma-project/helm-broker/internal/controller/instance" + "github.com/kyma-project/helm-broker/internal/platform/logger/spy" + "github.com/kyma-project/helm-broker/pkg/apis/addons/v1alpha1" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/kubernetes/scheme" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + "k8s.io/apimachinery/pkg/api/errors" +) + +func TestClusterBrokerControllerReconcile_CreateCSB(t *testing.T) { + // given + svc, cli := prepareClusterBrokerController(t, fixReadyClusterAddonsConfiguration()) + + // when + res, err := svc.Reconcile(fixRequest()) + + // then + require.NoError(t, err) + assert.False(t, res.Requeue) + _, err = getClusterServiceBroker(cli) + assert.NoError(t, err) +} + +func TestClusterBrokerControllerReconcile_DeleteSB(t *testing.T) { + // given + svc, cli := prepareClusterBrokerController(t, fixClusterServiceBroker()) + + // when + res, err := svc.Reconcile(fixRequest()) + + // then + require.NoError(t, err) + assert.False(t, res.Requeue) + _, err = getClusterServiceBroker(cli) + assert.True(t, errors.IsNotFound(err)) +} + +func TestClusterBrokerControllerReconcile_BlockDeletionByExistingInstances(t *testing.T) { + // given + svc, cli := prepareClusterBrokerController(t, fixClusterServiceBroker(), fixServiceInstanceForClusterServiceClass(), fixClusterServiceClass()) + + // when + res, err := svc.Reconcile(fixRequest()) + + // then + require.NoError(t, err) + assert.False(t, res.Requeue) + _, err = getClusterServiceBroker(cli) + assert.NoError(t, err) +} + +func fixRequest() reconcile.Request { + return reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: readyAddonsConfigName, + Namespace: "default", + }, + } +} + +func getClusterServiceBroker(cli client.Client) (*v1beta1.ClusterServiceBroker, error) { + var obj v1beta1.ClusterServiceBroker + err := cli.Get(context.TODO(), client.ObjectKey{Name: broker.NamespacedBrokerName}, &obj) + return &obj, err +} + +func fixReadyClusterAddonsConfiguration() *v1alpha1.ClusterAddonsConfiguration { + return &v1alpha1.ClusterAddonsConfiguration{ + ObjectMeta: metav1.ObjectMeta{ + Name: readyAddonsConfigName, + Namespace: "default", + }, + Spec: v1alpha1.ClusterAddonsConfigurationSpec{ + CommonAddonsConfigurationSpec: v1alpha1.CommonAddonsConfigurationSpec{}, + }, + Status: v1alpha1.ClusterAddonsConfigurationStatus{ + CommonAddonsConfigurationStatus: v1alpha1.CommonAddonsConfigurationStatus{ + Phase: v1alpha1.AddonsConfigurationReady, + }, + }, + } +} + +func fixClusterServiceBroker() *v1beta1.ClusterServiceBroker { + return &v1beta1.ClusterServiceBroker{ + ObjectMeta: metav1.ObjectMeta{ + Name: broker.NamespacedBrokerName, + }, + Spec: v1beta1.ClusterServiceBrokerSpec{}, + } +} + +func fixServiceInstanceForClusterServiceClass() *v1beta1.ServiceInstance { + return &v1beta1.ServiceInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: "si", + Namespace: "default", + }, + Spec: v1beta1.ServiceInstanceSpec{ + ClusterServiceClassRef: &v1beta1.ClusterObjectReference{ + Name: "sc-service", + }, + }} +} + +func fixClusterServiceClass() *v1beta1.ClusterServiceClass { + return &v1beta1.ClusterServiceClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "sc-service", + }, + Spec: v1beta1.ClusterServiceClassSpec{ + ClusterServiceBrokerName: broker.NamespacedBrokerName, + }} +} + +func prepareClusterBrokerController(t *testing.T, initObjs ...runtime.Object) (*controller.ClusterBrokerController, client.Client) { + require.NoError(t, v1beta1.AddToScheme(scheme.Scheme)) + require.NoError(t, v1alpha1.AddToScheme(scheme.Scheme)) + cli := fake.NewFakeClientWithScheme(scheme.Scheme, initObjs...) + iChecker := instance.New(cli, broker.NamespacedBrokerName) + bFacade := broker.NewClusterBrokersFacade(cli, "default", "helm-broker", broker.NamespacedBrokerName, spy.NewLogDummy()) + svc := controller.NewClusterBrokerController(iChecker, cli, bFacade, broker.NamespacedBrokerName) + + return svc, cli +} diff --git a/internal/controller/common.go b/internal/controller/common.go index 453b56ed..dfd6c91f 100644 --- a/internal/controller/common.go +++ b/internal/controller/common.go @@ -42,7 +42,9 @@ type common struct { trace string } -func newControllerCommon(client client.Client, addonGetterFactory addonGetterFactory, addonStorage addonStorage, chartStorage chartStorage, docsProvider docsProvider, brokerSyncer brokerSyncer, brokerFacade brokerFacade, templateService templateService, dstPath string, log logrus.FieldLogger) *common { +func newControllerCommon(client client.Client, addonGetterFactory addonGetterFactory, addonStorage addonStorage, + chartStorage chartStorage, docsProvider docsProvider, brokerSyncer brokerSyncer, brokerFacade brokerFacade, + templateService templateService, dstPath string, log logrus.FieldLogger) *common { return &common{ addonGetterFactory: addonGetterFactory, @@ -68,7 +70,7 @@ func newControllerCommon(client client.Client, addonGetterFactory addonGetterFac func (c *common) SetWorkingNamespace(namespace string) { c.namespace = internal.Namespace(namespace) for _, svc := range []NamespacedService{ - c.brokerSyncer, c.brokerFacade, c.docsProvider, c.commonClient, c.templateService, + c.brokerFacade, c.brokerSyncer, c.docsProvider, c.commonClient, c.templateService, } { svc.SetNamespace(namespace) } @@ -206,33 +208,27 @@ func (c *common) OnDelete(addon *internal.CommonAddon) error { return errors.Wrap(err, "while listing addons configurations") } - deleteBroker := true for _, ad := range adds { if ad.Status.Phase != v1alpha1.AddonsConfigurationReady { c.log.Infof("- reprocessing unblocked conflicting configuration `%s`", ad.Meta.Name) if err := c.commonClient.ReprocessRequest(ad.Meta.Name); err != nil { return errors.Wrapf(err, "while requesting reprocess addons configuration %s", ad.Meta.Name) } - } else { - deleteBroker = false - } - } - if deleteBroker { - if err := c.brokerFacade.Delete(); err != nil { - return errors.Wrap(err, "while deleting broker") } } - addonRemoved := false + anyAddonRemoved := false for _, repo := range addon.Status.Repositories { for _, ad := range repo.Addons { - addonRemoved, err = c.removeAddon(ad) + removed, err := c.removeAddon(ad) + anyAddonRemoved = anyAddonRemoved || removed if err != nil && !storage.IsNotFoundError(err) { return errors.Wrapf(err, "while deleting addon with charts for addon %s", ad.Name) } } } - if !deleteBroker && addonRemoved { + // if there was any removal processed - resync broker + if anyAddonRemoved { if err := c.brokerSyncer.Sync(); err != nil { return errors.Wrapf(err, "while syncing broker for addon %s", addon.Meta.Name) } @@ -426,11 +422,7 @@ func (c *common) ensureBroker() error { if err != nil { return errors.Wrap(err, "while checking if Broker exist") } - if !exist { - if err := c.brokerFacade.Create(); err != nil { - return errors.Wrap(err, "while creating Broker") - } - } else { + if exist { if err := c.brokerSyncer.Sync(); err != nil { return errors.Wrap(err, "while syncing Broker") } diff --git a/internal/controller/common_test.go b/internal/controller/common_test.go index 8e7d513c..36fdd766 100644 --- a/internal/controller/common_test.go +++ b/internal/controller/common_test.go @@ -51,7 +51,6 @@ func TestCommon_OnAdd(t *testing.T) { ts.docsProvider, ts.brokerSyncer, ts.brokerFacade, ts.templateService, "", logrus.New()) ts.brokerFacade.On("Exist").Return(false, nil) - ts.brokerFacade.On("Create").Return(nil).Once() err := common.OnAdd(tc.addon, tc.lastStatus) require.NoError(t, err) @@ -111,8 +110,6 @@ func TestCommon_OnDelete(t *testing.T) { common := newControllerCommon(ts.mgr.GetClient(), ts.addonGetterFactory, ts.addonStorage, ts.chartStorage, ts.docsProvider, ts.brokerSyncer, ts.brokerFacade, ts.templateService, "", logrus.New()) - ts.brokerFacade.On("Delete").Return(nil).Once() - err := common.OnDelete(tc.addon) require.NoError(t, err) }) @@ -155,7 +152,6 @@ func TestCommon_OnAdd_NamespaceScoped(t *testing.T) { ts.docsProvider, ts.brokerSyncer, ts.brokerFacade, ts.templateService, "", logrus.New()) ts.brokerFacade.On("Exist").Return(false, nil) - ts.brokerFacade.On("Create").Return(nil).Once() common.SetWorkingNamespace(tc.addon.Meta.Namespace) @@ -219,8 +215,6 @@ func TestCommon_OnDelete_NamespaceScoped(t *testing.T) { common := newControllerCommon(ts.mgr.GetClient(), ts.addonGetterFactory, ts.addonStorage, ts.chartStorage, ts.docsProvider, ts.brokerSyncer, ts.brokerFacade, ts.templateService, "", logrus.New()) - ts.brokerFacade.On("Delete").Return(nil).Once() - common.SetWorkingNamespace(tc.addon.Meta.Namespace) err := common.OnDelete(tc.addon) @@ -244,8 +238,6 @@ func TestCommon_PrepareForProcessing(t *testing.T) { common := newControllerCommon(ts.mgr.GetClient(), ts.addonGetterFactory, ts.addonStorage, ts.chartStorage, ts.docsProvider, ts.brokerSyncer, ts.brokerFacade, ts.templateService, "", logrus.New()) - ts.brokerFacade.On("Delete").Return(nil).Once() - err := common.PrepareForProcessing(tc.addon) require.NoError(t, err) }) @@ -267,8 +259,6 @@ func TestCommon_PrepareForProcessing_NamespaceScoped(t *testing.T) { common := newControllerCommon(ts.mgr.GetClient(), ts.addonGetterFactory, ts.addonStorage, ts.chartStorage, ts.docsProvider, ts.brokerSyncer, ts.brokerFacade, ts.templateService, "", logrus.New()) - ts.brokerFacade.On("Delete").Return(nil).Once() - common.SetWorkingNamespace(tc.addon.Meta.Namespace) err := common.PrepareForProcessing(tc.addon) diff --git a/internal/controller/ext.go b/internal/controller/ext.go index 34e41f10..a6d2deae 100644 --- a/internal/controller/ext.go +++ b/internal/controller/ext.go @@ -81,3 +81,8 @@ type commonReconciler interface { Reconcile(addon *internal.CommonAddon, trace string) (reconcile.Result, error) SetWorkingNamespace(namespace string) } + +type instanceChecker interface { + AnyServiceInstanceExistsForNamespacedServiceBroker(namespace string) (bool, error) + AnyServiceInstanceExistsForClusterServiceBroker() (bool, error) +} diff --git a/internal/controller/instance/facade.go b/internal/controller/instance/facade.go new file mode 100644 index 00000000..11c8a307 --- /dev/null +++ b/internal/controller/instance/facade.go @@ -0,0 +1,95 @@ +package instance + +import ( + "context" + + "github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog/v1beta1" + "github.com/kyma-project/helm-broker/internal/controller/broker" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +// Facade is responsible to provide information about service instances. +type Facade struct { + client client.Client + clusterBrokerName string +} + +// New creates new Facade instance +func New(c client.Client, clusterBrokerName string) *Facade { + return &Facade{ + client: c, + clusterBrokerName: clusterBrokerName, + } +} + +// AnyServiceInstanceExistsForNamespacedServiceBroker checks whether there is at least one service instance created with helm broker service class. +func (f *Facade) AnyServiceInstanceExistsForNamespacedServiceBroker(namespace string) (bool, error) { + instanceList := &v1beta1.ServiceInstanceList{} + err := f.client.List(context.TODO(), &client.ListOptions{Namespace: namespace}, instanceList) + if err != nil { + return false, err + } + if len(instanceList.Items) == 0 { + return false, nil + } + + classList := &v1beta1.ServiceClassList{} + err = f.client.List(context.TODO(), &client.ListOptions{Namespace: namespace}, classList) + if err != nil { + return false, err + } + + appBrokerClassNames := map[string]struct{}{} + for _, c := range classList.Items { + if c.Spec.ServiceBrokerName == broker.NamespacedBrokerName { + appBrokerClassNames[c.Name] = struct{}{} + } + } + + // iterate over all instances + for _, inst := range instanceList.Items { + // check only created with namespaced service class + if inst.Spec.ServiceClassRef != nil { + if _, exists := appBrokerClassNames[inst.Spec.ServiceClassRef.Name]; exists { + return true, nil + } + } + } + return false, nil +} + +// AnyServiceInstanceExistsForClusterServiceBroker checks whether there is at least one service instance created with helm broker cluster service class. +func (f *Facade) AnyServiceInstanceExistsForClusterServiceBroker() (bool, error) { + instanceList := &v1beta1.ServiceInstanceList{} + err := f.client.List(context.TODO(), &client.ListOptions{}, instanceList) + if err != nil { + return false, err + } + if len(instanceList.Items) == 0 { + return false, nil + } + + classList := &v1beta1.ClusterServiceClassList{} + err = f.client.List(context.TODO(), &client.ListOptions{}, classList) + if err != nil { + return false, err + } + + brokerClassNames := map[string]struct{}{} + for _, cl := range classList.Items { + if cl.Spec.ClusterServiceBrokerName == f.clusterBrokerName { + brokerClassNames[cl.Name] = struct{}{} + } + } + + // iterate over all instances + for _, inst := range instanceList.Items { + // check only created with cluster wide service class + if inst.Spec.ClusterServiceClassRef != nil { + if _, exists := brokerClassNames[inst.Spec.ClusterServiceClassRef.Name]; exists { + return true, nil + } + } + } + return false, nil +} diff --git a/internal/controller/instance/facade_test.go b/internal/controller/instance/facade_test.go new file mode 100644 index 00000000..1793a88b --- /dev/null +++ b/internal/controller/instance/facade_test.go @@ -0,0 +1,119 @@ +package instance_test + +import ( + "testing" + + "github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog/v1beta1" + "github.com/kyma-project/helm-broker/internal/controller/broker" + "github.com/kyma-project/helm-broker/internal/controller/instance" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes/scheme" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +const ( + namespace = "testing-ns" + clusterBrokerName = "helmbroker" +) + +func TestFacadeExistingHBInstance(t *testing.T) { + // given + require.NoError(t, v1beta1.AddToScheme(scheme.Scheme)) + cli := fake.NewFakeClientWithScheme(scheme.Scheme, fixHBServiceClass(), fixServiceClass(), fixHBServiceInstnace()) + facade := instance.New(cli, clusterBrokerName) + + // when / then + exists, err := facade.AnyServiceInstanceExistsForNamespacedServiceBroker(namespace) + + require.NoError(t, err) + assert.True(t, exists) +} + +func TestFacadeNoInstances(t *testing.T) { + // given + require.NoError(t, v1beta1.AddToScheme(scheme.Scheme)) + cli := fake.NewFakeClientWithScheme(scheme.Scheme, fixHBServiceClass(), fixServiceClass()) + facade := instance.New(cli, clusterBrokerName) + + // when / then + exists, err := facade.AnyServiceInstanceExistsForNamespacedServiceBroker(namespace) + + require.NoError(t, err) + assert.False(t, exists) +} + +func TestFacadeNoHBInstances(t *testing.T) { + // given + require.NoError(t, v1beta1.AddToScheme(scheme.Scheme)) + cli := fake.NewFakeClientWithScheme(scheme.Scheme, fixHBServiceClass(), fixServiceClass(), fixServiceInstnaceClusterServiceClass(), fixServiceInstance()) + facade := instance.New(cli, clusterBrokerName) + + // when / then + exists, err := facade.AnyServiceInstanceExistsForNamespacedServiceBroker(namespace) + + require.NoError(t, err) + assert.False(t, exists) +} + +func fixHBServiceInstnace() *v1beta1.ServiceInstance { + return &v1beta1.ServiceInstance{ + ObjectMeta: v1.ObjectMeta{ + Name: "si-application01", + Namespace: namespace, + }, + Spec: v1beta1.ServiceInstanceSpec{ + ServiceClassRef: &v1beta1.LocalObjectReference{ + Name: "a-class", + }, + }} +} + +func fixServiceInstance() *v1beta1.ServiceInstance { + return &v1beta1.ServiceInstance{ + ObjectMeta: v1.ObjectMeta{ + Name: "si-application01", + Namespace: namespace, + }, + Spec: v1beta1.ServiceInstanceSpec{ + ServiceClassRef: &v1beta1.LocalObjectReference{ + Name: "sc-service", + }, + }} +} + +func fixServiceInstnaceClusterServiceClass() *v1beta1.ServiceInstance { + return &v1beta1.ServiceInstance{ + ObjectMeta: v1.ObjectMeta{ + Name: "si-clusterserviceclass", + Namespace: namespace, + }, + Spec: v1beta1.ServiceInstanceSpec{ + ClusterServiceClassRef: &v1beta1.ClusterObjectReference{ + Name: "some-class", + }, + }} +} + +func fixHBServiceClass() *v1beta1.ServiceClass { + return &v1beta1.ServiceClass{ + ObjectMeta: v1.ObjectMeta{ + Name: "a-class", + Namespace: namespace, + }, + Spec: v1beta1.ServiceClassSpec{ + ServiceBrokerName: broker.NamespacedBrokerName, + }} +} + +func fixServiceClass() *v1beta1.ServiceClass { + return &v1beta1.ServiceClass{ + ObjectMeta: v1.ObjectMeta{ + Name: "sc-service", + Namespace: namespace, + }, + Spec: v1beta1.ServiceClassSpec{ + ServiceBrokerName: "other-broker", + }} +} diff --git a/internal/controller/setup.go b/internal/controller/setup.go index e2c9171f..166df0ba 100644 --- a/internal/controller/setup.go +++ b/internal/controller/setup.go @@ -10,6 +10,7 @@ import ( "github.com/kyma-project/helm-broker/internal/config" "github.com/kyma-project/helm-broker/internal/controller/broker" "github.com/kyma-project/helm-broker/internal/controller/docs" + "github.com/kyma-project/helm-broker/internal/controller/instance" "github.com/kyma-project/helm-broker/internal/controller/repository" "github.com/kyma-project/helm-broker/internal/rafter" "github.com/kyma-project/helm-broker/internal/storage" @@ -82,6 +83,8 @@ func SetupAndStartController(cfg *rest.Config, ctrCfg *config.ControllerConfig, addonGetterFactory, err := provider.NewClientFactory(allowedGetters, addon.NewLoader(ctrCfg.TmpDir, lg), ctrCfg.DocumentationEnabled, lg) fatalOnError(err, "cannot setup addon getter") + instChecker := instance.New(mgr.GetClient(), ctrCfg.ClusterServiceBrokerName) + // Creating controllers lg.Info("Setting up controller") acReconcile := NewReconcileAddonsConfiguration(mgr, addonGetterFactory, sFact.Chart(), sFact.Addon(), sbFacade, dtProvider, sbSyncer, templateService, ctrCfg.TmpDir, lg) @@ -94,6 +97,14 @@ func SetupAndStartController(cfg *rest.Config, ctrCfg *config.ControllerConfig, err = cacController.Start(mgr) fatalOnError(err, "unable to start ClusterAddonsConfigurationController") + bController := NewBrokerController(instChecker, mgr.GetClient(), broker.NewBrokersFacade(mgr.GetClient(), ctrCfg.Namespace, ctrCfg.ServiceName, lg)) + err = bController.Start(mgr) + fatalOnError(err, "unable to start BrokerController") + + cbController := NewClusterBrokerController(instChecker, mgr.GetClient(), csbFacade, ctrCfg.ClusterServiceBrokerName) + err = cbController.Start(mgr) + fatalOnError(err, "unable to start ClusterBrokerController") + return mgr } diff --git a/internal/helm/client.go b/internal/helm/client.go index be6dba5f..6e8ee38d 100644 --- a/internal/helm/client.go +++ b/internal/helm/client.go @@ -42,6 +42,8 @@ func NewClient(cfg Config, log *logrus.Entry) *Client { client.tlscfg = tlscfg } + client.helmClientFactory = client.helmClient + return client } @@ -52,6 +54,8 @@ type Client struct { tlscfg *tls.Config tlsEnabled bool log *logrus.Entry + + helmClientFactory func() DeleteInstaller } // Install is installing chart release @@ -62,7 +66,7 @@ func (cli *Client) Install(c *chart.Chart, values internal.ChartValues, releaseN if err != nil { return nil, errors.Wrapf(err, "while marshalling chart values: [%v]", values) } - resp, err := cli.helmClient().InstallReleaseFromChart(c, string(namespace), + resp, err := cli.helmClientFactory().InstallReleaseFromChart(c, string(namespace), helm.InstallWait(true), helm.InstallTimeout(int64(installTimeout.Seconds())), helm.ValueOverrides(byteValues), @@ -76,13 +80,13 @@ func (cli *Client) Install(c *chart.Chart, values internal.ChartValues, releaseN // Delete is deleting release of the chart func (cli *Client) Delete(releaseName internal.ReleaseName) error { cli.log.WithField("purge", deleteWithPurge).Infof("Deleting chart with release name [%s]", releaseName) - if _, err := cli.helmClient().DeleteRelease(string(releaseName), helm.DeletePurge(deleteWithPurge)); err != nil { + if _, err := cli.helmClientFactory().DeleteRelease(string(releaseName), helm.DeletePurge(deleteWithPurge)); err != nil { return errors.Wrapf(err, "while deleting release name: [%s]", releaseName) } return nil } -func (cli *Client) helmClient() helmDeleteInstaller { +func (cli *Client) helmClient() DeleteInstaller { // helm client is not thread safe - // // helm.ConnectTimeout option is REQUIRED, because of this issue: diff --git a/internal/helm/dep.go b/internal/helm/dep.go index 3261a573..37f98463 100644 --- a/internal/helm/dep.go +++ b/internal/helm/dep.go @@ -6,7 +6,8 @@ import ( rls "k8s.io/helm/pkg/proto/hapi/services" ) -type helmDeleteInstaller interface { +// DeleteInstaller defines necessary methods for communications with Tiller +type DeleteInstaller interface { InstallReleaseFromChart(chart *chart.Chart, ns string, opts ...helm.InstallOption) (*rls.InstallReleaseResponse, error) DeleteRelease(rlsName string, opts ...helm.DeleteOption) (*rls.UninstallReleaseResponse, error) } diff --git a/internal/helm/fake.go b/internal/helm/fake.go new file mode 100644 index 00000000..13f2a33b --- /dev/null +++ b/internal/helm/fake.go @@ -0,0 +1,7 @@ +package helm + +// WithHelmClientFactory creates HelmClient with specified DeleteInstaller factory +func (cli *Client) WithHelmClientFactory(factory func() DeleteInstaller) *Client { + cli.helmClientFactory = factory + return cli +} diff --git a/test/integration/crds/sc/clusterserviceclass.yaml b/test/integration/crds/sc/clusterserviceclass.yaml new file mode 100644 index 00000000..3f0136d4 --- /dev/null +++ b/test/integration/crds/sc/clusterserviceclass.yaml @@ -0,0 +1,30 @@ +apiVersion: apiextensions.k8s.io/v1beta1 +kind: CustomResourceDefinition +metadata: + name: clusterserviceclasses.servicecatalog.k8s.io + labels: + svcat: "true" +spec: + group: servicecatalog.k8s.io + version: v1beta1 + scope: Cluster + names: + plural: clusterserviceclasses + singular: clusterserviceclass + kind: ClusterServiceClass + # categories is a list of grouped resources the custom resource belongs to. + categories: + - all + - svcat + additionalPrinterColumns: + - name: External-Name + type: string + JSONPath: .spec.externalName + - name: Broker + type: string + JSONPath: .spec.clusterServiceBrokerName + - name: Age + type: date + JSONPath: .metadata.creationTimestamp + subresources: + status: {} diff --git a/test/integration/crds/sc/serviceclass.yaml b/test/integration/crds/sc/serviceclass.yaml new file mode 100644 index 00000000..20ad32f5 --- /dev/null +++ b/test/integration/crds/sc/serviceclass.yaml @@ -0,0 +1,30 @@ +apiVersion: apiextensions.k8s.io/v1beta1 +kind: CustomResourceDefinition +metadata: + name: serviceclasses.servicecatalog.k8s.io + labels: + svcat: "true" +spec: + group: servicecatalog.k8s.io + version: v1beta1 + scope: Namespaced + names: + plural: serviceclasses + singular: serviceclass + kind: ServiceClass + # categories is a list of grouped resources the custom resource belongs to. + categories: + - all + - svcat + additionalPrinterColumns: + - name: External-Name + type: string + JSONPath: .spec.externalName + - name: Broker + type: string + JSONPath: .spec.serviceBrokerName + - name: Age + type: date + JSONPath: .metadata.creationTimestamp + subresources: + status: {} diff --git a/test/integration/crds/sc/serviceinstance.yaml b/test/integration/crds/sc/serviceinstance.yaml new file mode 100644 index 00000000..416d80e9 --- /dev/null +++ b/test/integration/crds/sc/serviceinstance.yaml @@ -0,0 +1,33 @@ +apiVersion: apiextensions.k8s.io/v1beta1 +kind: CustomResourceDefinition +metadata: + name: serviceinstances.servicecatalog.k8s.io + labels: + svcat: "true" +spec: + group: servicecatalog.k8s.io + version: v1beta1 + scope: Namespaced + names: + plural: serviceinstances + singular: serviceinstance + kind: ServiceInstance + # categories is a list of grouped resources the custom resource belongs to. + categories: + - all + - svcat + additionalPrinterColumns: + - name: Class + type: string + JSONPath: .status.userSpecifiedClassName + - name: Plan + type: string + JSONPath: .status.userSpecifiedPlanName + - name: Status + type: string + JSONPath: .status.lastConditionState + - name: Age + type: date + JSONPath: .metadata.creationTimestamp + subresources: + status: {} diff --git a/test/integration/integration_test.go b/test/integration/integration_test.go index 64503a3f..669ed3ea 100644 --- a/test/integration/integration_test.go +++ b/test/integration/integration_test.go @@ -126,6 +126,7 @@ func TestGetCatalogHappyPath(t *testing.T) { suite.createAddonsConfiguration("prod", c.addonName, []string{redisAndAccTestRepo}, c.kind) suite.waitForAddonsConfigurationPhase("prod", c.addonName, v1alpha1.AddonsConfigurationReady) suite.waitForServicesInCatalogEndpoint("ns/prod", []string{c.redisID, c.testID}) + suite.waitForServiceBrokerRegistered("prod") // when suite.updateAddonsConfigurationRepositories("stage", c.addonName, []string{}, c.kind) @@ -183,6 +184,80 @@ func TestGetCatalogHappyPath(t *testing.T) { } } +func TestProvisioning(t *testing.T) { + // given + suite := newTestSuiteAndStartControllers(t, false, false) + defer suite.tearDown() + + suite.createAddonsConfiguration("stage", addonsConfigName, []string{redisRepo}, sourceHTTP) + suite.waitForServicesInCatalogEndpoint("ns/stage", []string{redisAddonID}) + suite.waitForNumberOfReleases(0) + + // when + suite.provisionInstanceFromServiceClass("ns/stage", "stage") + + // then + suite.waitForNumberOfReleases(1) +} + +func TestUnregisteringServiceBroker(t *testing.T) { + // given + suite := newTestSuiteAndStartControllers(t, false, false) + defer suite.tearDown() + + suite.createAddonsConfiguration("stage", addonsConfigName, []string{redisRepo}, sourceHTTP) + suite.waitForServiceBrokerRegistered("stage") + suite.waitForServicesInCatalogEndpoint("ns/stage", []string{redisAddonID}) + + // when + suite.provisionInstanceFromServiceClass("ns/stage", "stage") + + // then + suite.waitForNumberOfReleases(1) + + // when + suite.deleteAddonsConfiguration("stage", addonsConfigName) + + // then + suite.waitForServiceBrokerRegistered("stage") + + // when + suite.deprovisionInstance("ns/stage", "stage") + + // then + suite.waitForServiceBrokerNotRegistered("stage") +} + +func TestUnregisteringClusterServiceBroker(t *testing.T) { + // given + suite := newTestSuiteAndStartControllers(t, false, false) + defer suite.tearDown() + + suite.createClusterAddonsConfiguration(addonsConfigName, []string{redisRepo}, sourceHTTP) + suite.waitForClusterServiceBrokerRegistered() + suite.waitForServicesInCatalogEndpoint("cluster", []string{redisAddonID}) + + // when + suite.provisionInstanceFromClusterServiceClass("cluster", "stage") + + // then + suite.waitForNumberOfReleases(1) + + // when + suite.deleteClusterAddonsConfiguration(addonsConfigName) + + // then + suite.waitForServicesInCatalogEndpoint("cluster", []string{}) + // CSB still is registered because of the existing instance + suite.waitForClusterServiceBrokerRegistered() + + // when + suite.deprovisionInstance("cluster", "stage") + + // then + suite.waitForClusterServiceBrokerNotRegistered() +} + // TestAddonsConflicts check Helm Broker contract with conflicts on Addons. // It's tested only with HTTP, testing other protocols do not make sense, cause // conflicts resolving is in higher layer, so it's protocol agnostic. diff --git a/test/integration/suite_test.go b/test/integration/suite_test.go index 259dafa1..2351b1a7 100644 --- a/test/integration/suite_test.go +++ b/test/integration/suite_test.go @@ -36,6 +36,8 @@ import ( "github.com/kyma-project/helm-broker/internal/broker" "github.com/kyma-project/helm-broker/internal/config" "github.com/kyma-project/helm-broker/internal/controller" + broker2 "github.com/kyma-project/helm-broker/internal/controller/broker" + "github.com/kyma-project/helm-broker/internal/helm" "github.com/kyma-project/helm-broker/internal/rafter" "github.com/kyma-project/helm-broker/internal/rafter/automock" "github.com/kyma-project/helm-broker/internal/storage" @@ -43,6 +45,7 @@ import ( "github.com/kyma-project/helm-broker/pkg/apis" "github.com/kyma-project/helm-broker/pkg/apis/addons/v1alpha1" dtv1beta1 "github.com/kyma-project/helm-broker/pkg/apis/rafter/v1beta1" + helm2 "k8s.io/helm/pkg/helm" ) func init() { @@ -81,6 +84,10 @@ const ( basicPassword = "pAssword{" basicUsername = "user001" + + instanceID = "instance-id-001" + + clusterBrokerName = "helm-broker" ) func newTestSuiteAndStartControllers(t *testing.T, docsEnabled, httpBasicAuth bool) *testSuite { @@ -108,8 +115,13 @@ func newTestSuite(t *testing.T, docsEnabled, httpBasicAuth bool) *testSuite { require.NoError(t, err) logger := logrus.New() + fakeHelmClient := &helm2.FakeClient{} + helmClient := helm.NewClient(helm.Config{}, logger.WithField("service", "helmclient")).WithHelmClientFactory(func() helm.DeleteInstaller { + return fakeHelmClient + }) + brokerServer := broker.New(sFact.Addon(), sFact.Chart(), sFact.InstanceOperation(), sFact.BindOperation(), sFact.Instance(), sFact.InstanceBindData(), - bind.NewRenderer(), bind.NewResolver(k8sClientset.CoreV1()), nil, logger.WithField("test", "int")) + bind.NewRenderer(), bind.NewResolver(k8sClientset.CoreV1()), helmClient, logger.WithField("test", "int")) // OSB API Server server := httptest.NewServer(brokerServer.CreateHandler()) @@ -155,10 +167,11 @@ func newTestSuite(t *testing.T, docsEnabled, httpBasicAuth bool) *testSuite { return &testSuite{ t: t, - dynamicClient: dynamicClient, - repoServer: repoServer, - server: server, - k8sClient: k8sClientset, + dynamicClient: dynamicClient, + repoServer: repoServer, + server: server, + k8sClient: k8sClientset, + fakeHelmClient: fakeHelmClient, stopCh: stopCh, tmpDir: cfg.TmpDir, @@ -180,7 +193,7 @@ func (ts *testSuite) StartControllers(docsEnabled bool) { mgr := controller.SetupAndStartController(ts.restConfig, &config.ControllerConfig{ DevelopMode: true, // DevelopMode allows "http" urls - ClusterServiceBrokerName: "helm-broker", + ClusterServiceBrokerName: clusterBrokerName, TmpDir: os.TempDir(), DocumentationEnabled: docsEnabled, }, ":8001", ts.storageFactory, uploadClient, ts.logger.WithField("svc", "broker")) @@ -206,11 +219,12 @@ func newOSBClient(url string) (osb.Client, error) { } type testSuite struct { - t *testing.T - server *httptest.Server - repoServer *httptest.Server - gitRepository *gitRepo - minio *minioServer + t *testing.T + server *httptest.Server + repoServer *httptest.Server + gitRepository *gitRepo + minio *minioServer + fakeHelmClient *helm2.FakeClient osbClient osb.Client dynamicClient client.Client @@ -222,6 +236,9 @@ type testSuite struct { storageFactory storage.Factory logger logrus.FieldLogger + + planID string + serviceID string } func (ts *testSuite) tearDown() { @@ -236,6 +253,29 @@ func (ts *testSuite) tearDown() { } } +func (ts *testSuite) waitForNumberOfReleases(n int) { + timeoutCh := time.After(3 * time.Second) + for { + if len(ts.fakeHelmClient.Rels) == n { + return + } + + select { + case <-timeoutCh: + assert.Fail(ts.t, "The timeout exceeded while waiting for the expected number of Helm releases") + return + default: + time.Sleep(pollingInterval) + } + } +} + +func (ts *testSuite) listReleases() { + for _, r := range ts.fakeHelmClient.Rels { + fmt.Println(r.Name) + } +} + func (ts *testSuite) initMinioServer() { minioServer, err := runMinioServer(ts.t, ts.tmpDir) require.NoError(ts.t, err) @@ -243,6 +283,122 @@ func (ts *testSuite) initMinioServer() { ts.minio = minioServer } +func (ts *testSuite) provisionInstanceFromServiceClass(prefix, namespace string) { + osbClient, err := newOSBClient(fmt.Sprintf("%s/%s", ts.server.URL, prefix)) + require.NoError(ts.t, err) + resp, err := osbClient.GetCatalog() + ts.planID = resp.Services[0].Plans[0].ID + ts.serviceID = resp.Services[0].ID + require.NoError(ts.t, err) + + _, err = osbClient.ProvisionInstance(&osb.ProvisionRequest{ + PlanID: ts.planID, + ServiceID: ts.serviceID, + InstanceID: instanceID, + OrganizationGUID: "org-guid", + SpaceGUID: "spaceGUID", + AcceptsIncomplete: true, + }) + require.NoError(ts.t, err) + + // The controller checks if there is any Service Instance (managed by Service Catalog). + // The following code simulates Service Catalog actions + err = ts.dynamicClient.Create(context.TODO(), &v1beta1.ServiceClass{ + ObjectMeta: v1.ObjectMeta{ + Name: "a-class", + Namespace: namespace, + }, + Spec: v1beta1.ServiceClassSpec{ + ServiceBrokerName: broker2.NamespacedBrokerName, + }, + }) + require.NoError(ts.t, err) + err = ts.dynamicClient.Create(context.TODO(), &v1beta1.ServiceInstance{ + ObjectMeta: v1.ObjectMeta{ + Name: "instance-001", + Namespace: namespace, + }, + Spec: v1beta1.ServiceInstanceSpec{ + ServiceClassRef: &v1beta1.LocalObjectReference{ + Name: "a-class", + }, + }, + }) + require.NoError(ts.t, err) +} + +func (ts *testSuite) provisionInstanceFromClusterServiceClass(prefix, namespace string) { + osbClient, err := newOSBClient(fmt.Sprintf("%s/%s", ts.server.URL, prefix)) + require.NoError(ts.t, err) + resp, err := osbClient.GetCatalog() + ts.planID = resp.Services[0].Plans[0].ID + ts.serviceID = resp.Services[0].ID + require.NoError(ts.t, err) + + _, err = osbClient.ProvisionInstance(&osb.ProvisionRequest{ + PlanID: ts.planID, + ServiceID: ts.serviceID, + InstanceID: instanceID, + OrganizationGUID: "org-guid", + SpaceGUID: "spaceGUID", + AcceptsIncomplete: true, + }) + require.NoError(ts.t, err) + + // The controller checks if there is any Service Instance (managed by Service Catalog). + // The following code simulates Service Catalog actions + err = ts.dynamicClient.Create(context.TODO(), &v1beta1.ClusterServiceClass{ + ObjectMeta: v1.ObjectMeta{ + Name: "some-class", + }, + Spec: v1beta1.ClusterServiceClassSpec{ + ClusterServiceBrokerName: clusterBrokerName, + }, + }) + require.NoError(ts.t, err) + err = ts.dynamicClient.Create(context.TODO(), &v1beta1.ServiceInstance{ + ObjectMeta: v1.ObjectMeta{ + Name: "instance-001", + Namespace: namespace, + }, + Spec: v1beta1.ServiceInstanceSpec{ + ClusterServiceClassRef: &v1beta1.ClusterObjectReference{ + Name: "some-class", + }, + }, + }) + require.NoError(ts.t, err) +} + +func (ts *testSuite) deprovisionInstance(prefix, namespace string) { + osbClient, err := newOSBClient(fmt.Sprintf("%s/%s", ts.server.URL, prefix)) + require.NoError(ts.t, err) + require.NoError(ts.t, err) + + _, err = osbClient.DeprovisionInstance(&osb.DeprovisionRequest{ + PlanID: ts.planID, + ServiceID: ts.serviceID, + InstanceID: instanceID, + AcceptsIncomplete: true, + }) + require.NoError(ts.t, err) + + // The controller checks if there is any Service Instance (managed by Service Catalog). + // The following code simulates Service Catalog actions + err = ts.dynamicClient.Delete(context.TODO(), &v1beta1.ServiceInstance{ + ObjectMeta: v1.ObjectMeta{ + Name: "instance-001", + Namespace: namespace, + }, + Spec: v1beta1.ServiceInstanceSpec{ + ServiceClassRef: &v1beta1.LocalObjectReference{ + Name: "a-class", + }, + }, + }) + require.NoError(ts.t, err) +} + func (ts *testSuite) assertNoServicesInCatalogEndpoint(prefix string) { osbClient, err := newOSBClient(fmt.Sprintf("%s/%s", ts.server.URL, prefix)) require.NoError(ts.t, err) @@ -346,6 +502,78 @@ func (ts *testSuite) waitForPhase(obj runtime.Object, status *v1alpha1.CommonAdd } } +func (ts *testSuite) waitForServiceBrokerRegistered(namespace string) { + timeoutCh := time.After(3 * time.Second) + for { + var obj v1beta1.ServiceBroker + err := ts.dynamicClient.Get(context.TODO(), types.NamespacedName{Namespace: namespace, Name: broker2.NamespacedBrokerName}, &obj) + if err == nil { + return + } + select { + case <-timeoutCh: + assert.Fail(ts.t, "The timeout exceeded while waiting for the ServiceBroker", err) + return + default: + time.Sleep(100 * time.Millisecond) + } + } +} + +func (ts *testSuite) waitForClusterServiceBrokerRegistered() { + timeoutCh := time.After(3 * time.Second) + for { + var obj v1beta1.ClusterServiceBroker + err := ts.dynamicClient.Get(context.TODO(), types.NamespacedName{Name: clusterBrokerName}, &obj) + if err == nil { + return + } + select { + case <-timeoutCh: + assert.Fail(ts.t, "The timeout exceeded while waiting for the ClusterServiceBroker", err) + return + default: + time.Sleep(100 * time.Millisecond) + } + } +} + +func (ts *testSuite) waitForServiceBrokerNotRegistered(namespace string) { + timeoutCh := time.After(3 * time.Second) + for { + var obj v1beta1.ServiceBroker + err := ts.dynamicClient.Get(context.TODO(), types.NamespacedName{Namespace: namespace, Name: broker2.NamespacedBrokerName}, &obj) + if apierrors.IsNotFound(err) { + return + } + select { + case <-timeoutCh: + assert.Fail(ts.t, "The timeout exceeded while waiting for the ServiceBroker unregistration") + return + default: + time.Sleep(100 * time.Millisecond) + } + } +} + +func (ts *testSuite) waitForClusterServiceBrokerNotRegistered() { + timeoutCh := time.After(3 * time.Second) + for { + var obj v1beta1.ClusterServiceBroker + err := ts.dynamicClient.Get(context.TODO(), types.NamespacedName{Name: clusterBrokerName}, &obj) + if apierrors.IsNotFound(err) { + return + } + select { + case <-timeoutCh: + assert.Fail(ts.t, "The timeout exceeded while waiting for the ClusterServiceBroker", err) + return + default: + time.Sleep(100 * time.Millisecond) + } + } +} + func (ts *testSuite) deleteAddonsConfiguration(namespace, name string) { require.NoError(ts.t, ts.dynamicClient.Delete(context.TODO(), &v1alpha1.AddonsConfiguration{ ObjectMeta: v1.ObjectMeta{