diff --git a/controllers/watcherapi_controller.go b/controllers/watcherapi_controller.go index 0bac4e6d..ee080201 100644 --- a/controllers/watcherapi_controller.go +++ b/controllers/watcherapi_controller.go @@ -232,7 +232,13 @@ func (r *WatcherAPIReconciler) Reconcile(ctx context.Context, req ctrl.Request) // but we have to return while waiting for the service to be exposed return ctrl.Result{}, err } - _ = apiEndpoints // we'll use the apiEndpoints later + + result, err = r.ensureKeystoneEndpoint(ctx, helper, instance, apiEndpoints) + if (err != nil || result != ctrl.Result{}) { + // We can ignore RequeueAfter as we are watching the KeystoneEndpoint + // resource + return result, err + } // We reached the end of the Reconcile, update the Ready condition based on // the sub conditions @@ -444,10 +450,82 @@ func (r *WatcherAPIReconciler) ensureServiceExposed( return apiEndpoints, ctrl.Result{}, nil } +func (r *WatcherAPIReconciler) ensureKeystoneEndpoint( + ctx context.Context, + helper *helper.Helper, + instance *watcherv1beta1.WatcherAPI, + apiEndpoints map[string]string, +) (ctrl.Result, error) { + Log := r.GetLogger(ctx) + Log.Info(fmt.Sprintf("Defining WatcherAPI KeystoneEndpoint '%s'", instance.Name)) + + endpointSpec := keystonev1.KeystoneEndpointSpec{ + ServiceName: watcher.ServiceName, + Endpoints: apiEndpoints, + } + endpoint := keystonev1.NewKeystoneEndpoint( + watcher.ServiceName, + instance.Namespace, + endpointSpec, + getAPIServiceLabels(), + r.RequeueTimeout, + ) + ctrlResult, err := endpoint.CreateOrPatch(ctx, helper) + if err != nil { + return ctrlResult, err + } + + c := endpoint.GetConditions().Mirror(condition.KeystoneEndpointReadyCondition) + if c != nil { + instance.Status.Conditions.Set(c) + } + + return ctrlResult, nil +} + +func (r *WatcherAPIReconciler) ensureKeystoneEndpointDeletion( + ctx context.Context, + helper *helper.Helper, + instance *watcherv1beta1.WatcherAPI, +) error { + // Remove the finalizer from our KeystoneEndpoint CR + // This is oddly added automatically when we created KeystoneEndpoint but + // we need to remove it manually + Log := r.GetLogger(ctx) + + endpoint, err := keystonev1.GetKeystoneEndpointWithName(ctx, helper, watcher.ServiceName, instance.Namespace) + if err != nil && !k8s_errors.IsNotFound(err) { + return err + } + + if k8s_errors.IsNotFound(err) { + // Nothing to do as it was never created + return nil + } + + updated := controllerutil.RemoveFinalizer(endpoint, helper.GetFinalizer()) + if !updated { + // No finalizer to remove + return nil + } + + if err = helper.GetClient().Update(ctx, endpoint); err != nil && !k8s_errors.IsNotFound(err) { + return err + } + Log.Info("Removed finalizer from WatcherAPI KeystoneEndpoint") + + return nil +} + func (r *WatcherAPIReconciler) reconcileDelete(ctx context.Context, instance *watcherv1beta1.WatcherAPI, helper *helper.Helper) (ctrl.Result, error) { Log := r.GetLogger(ctx) Log.Info(fmt.Sprintf("Reconcile Service '%s' delete started", instance.Name)) + err := r.ensureKeystoneEndpointDeletion(ctx, helper, instance) + if err != nil { + return ctrl.Result{}, err + } + // Remove our finalizer from Memcached memcached, err := memcachedv1.GetMemcachedByName(ctx, helper, instance.Spec.MemcachedInstance, instance.Namespace) if err != nil && !k8s_errors.IsNotFound(err) { @@ -480,6 +558,7 @@ func (r *WatcherAPIReconciler) initStatus(instance *watcherv1beta1.WatcherAPI) e condition.UnknownCondition(condition.MemcachedReadyCondition, condition.InitReason, condition.MemcachedReadyInitMessage), condition.UnknownCondition(condition.DeploymentReadyCondition, condition.InitReason, condition.DeploymentReadyInitMessage), condition.UnknownCondition(condition.ExposeServiceReadyCondition, condition.InitReason, condition.ExposeServiceReadyInitMessage), + condition.UnknownCondition(condition.KeystoneEndpointReadyCondition, condition.InitReason, "KeystoneEndpoint not created"), ) instance.Status.Conditions.Init(&cl) @@ -514,6 +593,7 @@ func (r *WatcherAPIReconciler) SetupWithManager(mgr ctrl.Manager) error { Owns(&appsv1.Deployment{}). Owns(&corev1.Service{}). Owns(&routev1.Route{}). + Owns(&keystonev1.KeystoneEndpoint{}). Watches( &corev1.Secret{}, handler.EnqueueRequestsFromMapFunc(r.findObjectsForSrc), diff --git a/tests/functional/watcher_controller_test.go b/tests/functional/watcher_controller_test.go index fe5e0346..ffcf7dba 100644 --- a/tests/functional/watcher_controller_test.go +++ b/tests/functional/watcher_controller_test.go @@ -253,6 +253,9 @@ var _ = Describe("Watcher controller", func() { // We validate the full Watcher CR readiness status here // DB Ready + // Simulate KeystoneEndpoint success + keystone.SimulateKeystoneEndpointReady(watcherTest.WatcherKeystoneEndpointName) + // Simulate WatcherAPI deployment th.SimulateDeploymentReplicaReady(watcherTest.WatcherAPIDeployment) th.ExpectCondition( @@ -641,6 +644,9 @@ var _ = Describe("Watcher controller", func() { // Simulate dbsync success th.SimulateJobSuccess(watcherTest.WatcherDBSync) + // Simulate KeystoneEndpoint success + keystone.SimulateKeystoneEndpointReady(watcherTest.WatcherKeystoneEndpointName) + // Simulate WatcherAPI deployment th.SimulateDeploymentReplicaReady(watcherTest.WatcherAPIDeployment) diff --git a/tests/functional/watcher_test_data.go b/tests/functional/watcher_test_data.go index b201e5c7..c0ca1d70 100644 --- a/tests/functional/watcher_test_data.go +++ b/tests/functional/watcher_test_data.go @@ -51,6 +51,7 @@ type WatcherTestData struct { WatcherInternalServiceName types.NamespacedName WatcherRouteName types.NamespacedName WatcherInternalRouteName types.NamespacedName + WatcherKeystoneEndpointName types.NamespacedName } // GetWatcherTestData is a function that initialize the WatcherTestData @@ -135,5 +136,9 @@ func GetWatcherTestData(watcherName types.NamespacedName) WatcherTestData { Namespace: watcherName.Namespace, Name: "watcher-internal", }, + WatcherKeystoneEndpointName: types.NamespacedName{ + Namespace: watcherName.Namespace, + Name: "watcher", + }, } } diff --git a/tests/functional/watcherapi_controller_test.go b/tests/functional/watcherapi_controller_test.go index 52dd7fe6..6e4244ce 100644 --- a/tests/functional/watcherapi_controller_test.go +++ b/tests/functional/watcherapi_controller_test.go @@ -181,11 +181,30 @@ var _ = Describe("WatcherAPI controller", func() { condition.ExposeServiceReadyCondition, corev1.ConditionTrue, ) + th.AssertRouteExists(watcherTest.WatcherRouteName) public := th.GetService(watcherTest.WatcherPublicServiceName) Expect(public.Labels["service"]).To(Equal("watcher-api")) + Expect(public.Labels["public"]).To(Equal("true")) internal := th.GetService(watcherTest.WatcherInternalServiceName) Expect(internal.Labels["service"]).To(Equal("watcher-api")) - th.AssertRouteExists(watcherTest.WatcherRouteName) + Expect(internal.Labels["internal"]).To(Equal("true")) + }) + It("created the keystone endpoint for the watcher-api service", func() { + keystone.SimulateKeystoneEndpointReady(watcherTest.WatcherKeystoneEndpointName) + // it registers the endpointURL as the public endpoint and svc + // for the internal + keystoneEndpoint := keystone.GetKeystoneEndpoint(watcherTest.WatcherKeystoneEndpointName) + endpoints := keystoneEndpoint.Spec.Endpoints + // jgilaber: the public endpoint returned by the exposeEndpoint + // function of lib-common has an empty hostname + Expect(endpoints).To(HaveKeyWithValue("public", "http://")) + Expect(endpoints).To(HaveKeyWithValue("internal", "http://watcher-internal."+watcherTest.WatcherAPI.Namespace+".svc:9322")) + th.ExpectCondition( + watcherTest.WatcherAPI, + ConditionGetterFunc(WatcherAPIConditionGetter), + condition.KeystoneEndpointReadyCondition, + corev1.ConditionTrue, + ) }) }) When("the secret is created but missing fields", func() { @@ -356,6 +375,9 @@ var _ = Describe("WatcherAPI controller", func() { }) It("creates MetalLB service", func() { th.SimulateDeploymentReplicaReady(watcherTest.WatcherAPIDeployment) + // simulate that the internal service got a LoadBalancerIP + // assigned + th.SimulateLoadBalancerServiceIP(watcherTest.WatcherInternalServiceName) // As the public endpoint is not mentioned in the service override // a generic Service and a Route is created @@ -363,6 +385,8 @@ var _ = Describe("WatcherAPI controller", func() { Expect(public.Annotations).NotTo(HaveKey("metallb.universe.tf/address-pool")) Expect(public.Annotations).NotTo(HaveKey("metallb.universe.tf/allow-shared-ip")) Expect(public.Annotations).NotTo(HaveKey("metallb.universe.tf/loadBalancerIPs")) + Expect(public.Labels["service"]).To(Equal("watcher-api")) + Expect(public.Labels["public"]).To(Equal("true")) th.AssertRouteExists(watcherTest.WatcherRouteName) // As the internal endpoint is configure in the service override it @@ -372,12 +396,12 @@ var _ = Describe("WatcherAPI controller", func() { Expect(internal.Annotations).To(HaveKeyWithValue("metallb.universe.tf/address-pool", "osp-internalapi")) Expect(internal.Annotations).To(HaveKeyWithValue("metallb.universe.tf/allow-shared-ip", "osp-internalapi")) Expect(internal.Annotations).To(HaveKeyWithValue("metallb.universe.tf/loadBalancerIPs", "internal-lb-ip-1,internal-lb-ip-2")) + Expect(internal.Labels["service"]).To(Equal("watcher-api")) + Expect(internal.Labels["internal"]).To(Equal("true")) th.AssertRouteNotExists(watcherTest.WatcherInternalRouteName) - // simulate that the internal service got a LoadBalancerIP - // assigned - th.SimulateLoadBalancerServiceIP(watcherTest.WatcherInternalServiceName) - + // simulate the keystone endpoint + keystone.SimulateKeystoneEndpointReady(watcherTest.WatcherKeystoneEndpointName) th.ExpectCondition( watcherTest.WatcherAPI, ConditionGetterFunc(WatcherAPIConditionGetter), diff --git a/tests/kuttl/test-suites/default/watcher-api-service-override/05-assert.yaml b/tests/kuttl/test-suites/default/common/cleanup-assert.yaml similarity index 100% rename from tests/kuttl/test-suites/default/watcher-api-service-override/05-assert.yaml rename to tests/kuttl/test-suites/default/common/cleanup-assert.yaml diff --git a/tests/kuttl/test-suites/default/watcher-api-service-override/05-errors.yaml b/tests/kuttl/test-suites/default/common/cleanup-errors.yaml similarity index 94% rename from tests/kuttl/test-suites/default/watcher-api-service-override/05-errors.yaml rename to tests/kuttl/test-suites/default/common/cleanup-errors.yaml index 633d9dd9..ff8083af 100644 --- a/tests/kuttl/test-suites/default/watcher-api-service-override/05-errors.yaml +++ b/tests/kuttl/test-suites/default/common/cleanup-errors.yaml @@ -91,3 +91,8 @@ metadata: public: "true" service: watcher-api name: watcher-public +--- +apiVersion: keystone.openstack.org/v1beta1 +kind: KeystoneEndpoint +metadata: + name: watcher diff --git a/tests/kuttl/test-suites/default/deps/infra.yaml b/tests/kuttl/test-suites/default/deps/infra.yaml index 31b56f9c..f8acb222 100644 --- a/tests/kuttl/test-suites/default/deps/infra.yaml +++ b/tests/kuttl/test-suites/default/deps/infra.yaml @@ -32,3 +32,8 @@ spec: enabled: false telemetry: enabled: false + tls: + ingress: + enabled: false + podLevel: + enabled: false diff --git a/tests/kuttl/test-suites/default/watcher-api-service-override/01-assert.yaml b/tests/kuttl/test-suites/default/watcher-api-service-override/01-assert.yaml index f3e5b95b..e85d1d9a 100644 --- a/tests/kuttl/test-suites/default/watcher-api-service-override/01-assert.yaml +++ b/tests/kuttl/test-suites/default/watcher-api-service-override/01-assert.yaml @@ -141,6 +141,7 @@ metadata: finalizers: - openstack.org/watcher - openstack.org/keystoneservice + - openstack.org/keystoneendpoint-watcher spec: enabled: true passwordSelector: WatcherPassword @@ -216,6 +217,10 @@ status: reason: Ready status: "True" type: InputReady + - message: Setup complete + reason: Ready + status: "True" + type: KeystoneEndpointReady - message: " Memcached instance has been provisioned" reason: Ready status: "True" @@ -288,10 +293,6 @@ spec: selector: service: watcher-api type: LoadBalancer -status: - loadBalancer: - ingress: - - ip: 172.17.0.82 --- apiVersion: route.openshift.io/v1 kind: Route @@ -307,6 +308,13 @@ spec: kind: Service name: watcher-public --- +apiVersion: keystone.openstack.org/v1beta1 +kind: KeystoneEndpoint +metadata: + name: watcher +spec: + serviceName: watcher +--- apiVersion: kuttl.dev/v1beta1 kind: TestAssert namespaced: true diff --git a/tests/kuttl/test-suites/default/watcher-api-service-override/01-deploy-with-defaults.yaml b/tests/kuttl/test-suites/default/watcher-api-service-override/01-deploy-with-defaults.yaml index bcd84992..91792d57 100644 --- a/tests/kuttl/test-suites/default/watcher-api-service-override/01-deploy-with-defaults.yaml +++ b/tests/kuttl/test-suites/default/watcher-api-service-override/01-deploy-with-defaults.yaml @@ -10,5 +10,4 @@ spec: service: internal: ipAddressPool: "internalapi" - loadBalancerIPs: - - 172.17.0.82 + loadBalancerIPs: [] diff --git a/tests/kuttl/test-suites/default/watcher-api-service-override/02-assert.yaml b/tests/kuttl/test-suites/default/watcher-api-service-override/02-assert.yaml new file mode 120000 index 00000000..2bebf0ef --- /dev/null +++ b/tests/kuttl/test-suites/default/watcher-api-service-override/02-assert.yaml @@ -0,0 +1 @@ +../common/cleanup-assert.yaml \ No newline at end of file diff --git a/tests/kuttl/test-suites/default/watcher-api-service-override/05-cleanup-watcher.yaml b/tests/kuttl/test-suites/default/watcher-api-service-override/02-cleanup-watcher.yaml similarity index 100% rename from tests/kuttl/test-suites/default/watcher-api-service-override/05-cleanup-watcher.yaml rename to tests/kuttl/test-suites/default/watcher-api-service-override/02-cleanup-watcher.yaml diff --git a/tests/kuttl/test-suites/default/watcher-api-service-override/02-errors.yaml b/tests/kuttl/test-suites/default/watcher-api-service-override/02-errors.yaml new file mode 120000 index 00000000..a632ed81 --- /dev/null +++ b/tests/kuttl/test-suites/default/watcher-api-service-override/02-errors.yaml @@ -0,0 +1 @@ +../common/cleanup-errors.yaml \ No newline at end of file diff --git a/tests/kuttl/test-suites/default/watcher/01-assert.yaml b/tests/kuttl/test-suites/default/watcher/01-assert.yaml index 9ae80f76..617bf4b6 100644 --- a/tests/kuttl/test-suites/default/watcher/01-assert.yaml +++ b/tests/kuttl/test-suites/default/watcher/01-assert.yaml @@ -141,6 +141,7 @@ metadata: finalizers: - openstack.org/watcher - openstack.org/keystoneservice + - openstack.org/keystoneendpoint-watcher spec: enabled: true passwordSelector: WatcherPassword @@ -216,6 +217,10 @@ status: reason: Ready status: "True" type: InputReady + - message: Setup complete + reason: Ready + status: "True" + type: KeystoneEndpointReady - message: " Memcached instance has been provisioned" reason: Ready status: "True" @@ -302,6 +307,13 @@ spec: kind: Service name: watcher-public --- +apiVersion: keystone.openstack.org/v1beta1 +kind: KeystoneEndpoint +metadata: + name: watcher +spec: + serviceName: watcher +--- apiVersion: kuttl.dev/v1beta1 kind: TestAssert namespaced: true diff --git a/tests/kuttl/test-suites/default/watcher/04-assert.yaml b/tests/kuttl/test-suites/default/watcher/04-assert.yaml index 6bd0fd51..eb208ea1 100644 --- a/tests/kuttl/test-suites/default/watcher/04-assert.yaml +++ b/tests/kuttl/test-suites/default/watcher/04-assert.yaml @@ -141,6 +141,7 @@ metadata: finalizers: - openstack.org/watcher - openstack.org/keystoneservice + - openstack.org/keystoneendpoint-watcher spec: enabled: true passwordSelector: WatcherPassword @@ -197,6 +198,10 @@ status: reason: Ready status: "True" type: InputReady + - message: Setup complete + reason: Ready + status: "True" + type: KeystoneEndpointReady - message: " Memcached instance has been provisioned" reason: Ready status: "True" diff --git a/tests/kuttl/test-suites/default/watcher/05-assert.yaml b/tests/kuttl/test-suites/default/watcher/05-assert.yaml deleted file mode 100644 index 89371130..00000000 --- a/tests/kuttl/test-suites/default/watcher/05-assert.yaml +++ /dev/null @@ -1,7 +0,0 @@ -apiVersion: kuttl.dev/v1beta1 -kind: TestAssert -namespaced: true -commands: - - script: | - set -ex - oc exec -n watcher-kuttl-default openstackclient -- openstack service list -f value -c Name -c Type | [ $(grep -c ^watcher) == 0 ] diff --git a/tests/kuttl/test-suites/default/watcher/05-assert.yaml b/tests/kuttl/test-suites/default/watcher/05-assert.yaml new file mode 120000 index 00000000..2bebf0ef --- /dev/null +++ b/tests/kuttl/test-suites/default/watcher/05-assert.yaml @@ -0,0 +1 @@ +../common/cleanup-assert.yaml \ No newline at end of file diff --git a/tests/kuttl/test-suites/default/watcher/05-errors.yaml b/tests/kuttl/test-suites/default/watcher/05-errors.yaml deleted file mode 100644 index 633d9dd9..00000000 --- a/tests/kuttl/test-suites/default/watcher/05-errors.yaml +++ /dev/null @@ -1,93 +0,0 @@ -apiVersion: v1 -kind: ServiceAccount -metadata: - name: watcher-watcher-kuttl ---- -apiVersion: keystone.openstack.org/v1beta1 -kind: KeystoneService -metadata: - name: watcher ---- -apiVersion: v1 -kind: Secret -metadata: - name: rabbitmq-transport-url-watcher-kuttl-watcher-transport ---- -apiVersion: v1 -kind: Secret -metadata: - name: watcher-kuttl ---- -apiVersion: rabbitmq.openstack.org/v1beta1 -kind: TransportURL -metadata: - name: watcher-kuttl-watcher-transport ---- -apiVersion: mariadb.openstack.org/v1beta1 -kind: MariaDBDatabase -metadata: - name: watcher ---- -apiVersion: v1 -kind: Secret -metadata: - name: watcher-db-secret ---- -apiVersion: batch/v1 -kind: Job -metadata: - labels: - service: watcher - name: watcher-kuttl-db-sync ---- -apiVersion: v1 -kind: Secret -metadata: - name: watcher-kuttl-config-data ---- -apiVersion: watcher.openstack.org/v1beta1 -kind: Watcher -metadata: - name: watcher-kuttl ---- -apiVersion: watcher.openstack.org/v1beta1 -kind: WatcherAPI -metadata: - finalizers: - - openstack.org/watcherapi - name: watcher-kuttl-api ---- -apiVersion: apps/v1 -kind: Deployment -metadata: - name: watcher-kuttl-api ---- -apiVersion: v1 -kind: Pod -metadata: - labels: - service: watcher-api ---- -apiVersion: v1 -kind: Service -metadata: - labels: - internal: "true" - service: watcher-api - name: watcher-internal ---- -apiVersion: v1 -kind: Service -metadata: - labels: - public: "true" - service: watcher-api - name: watcher-public ---- -apiVersion: route.openshift.io/v1 -kind: Route -metadata: - labels: - public: "true" - service: watcher-api - name: watcher-public diff --git a/tests/kuttl/test-suites/default/watcher/05-errors.yaml b/tests/kuttl/test-suites/default/watcher/05-errors.yaml new file mode 120000 index 00000000..a632ed81 --- /dev/null +++ b/tests/kuttl/test-suites/default/watcher/05-errors.yaml @@ -0,0 +1 @@ +../common/cleanup-errors.yaml \ No newline at end of file