adds: initial service architecture for Elastic search service port #1802

raygervais · 2021-02-20T17:16:10Z

Issue This PR Addresses

Type of Change

Bugfix: Change which fixes an issue
New Feature: Change which adds functionality
Documentation Update: Change which improves documentation
UI: Change which improves UI

Description

Adds support for search.docker.localhost, which ports the original ELK query controller to a dedicated service. The following is added as a warning to not work further on the original codebase for querying:

/*
 * NOTE: This has been ported to ~/src/api/search
 * Future updates will remove the codebase below
 * and point towards `service.docker.localhost` for querying.
 */

Checklist

Quality: This PR builds and passes our npm test and works locally
Tests: This PR includes thorough tests or an explanation of why it does not
Screenshots: This PR includes screenshots or GIFs of the changes made or an explanation of why it does not (if applicable)
Documentation: This PR includes updated/added documentation to user exposed functionality or configuration variables are added/changed or an explanation of why it does not(if applicable)

src/api/search/server.js

humphd

Left some initial feedback, and let's get CI to work (prettier fixes, eslint fixes).

Also, this needs tests

src/api/docker-compose-api-production.yml

src/api/search/package.json

humphd · 2021-02-20T19:25:08Z

src/api/search/server.js

+
+const query = require('./src/routes/query');
+
+const service = new Satellite();


You can also do:

const service = new Satellite({ router: query });

I tried this, and it didn't seem to find the /query route with the above implementation.

With 1.5.1 or 1.5.2?

I've shipped 1.6.0, fyi.

I had tested with 1.5.2, latest at the time.
Will update to 1.6.0

src/api/search/src/bin/search.js

src/api/search/src/bin/elastic.js

src/api/search/src/routes/health.js

humphd · 2021-03-14T23:58:19Z

I notice you pushed a bunch of commits, but also that it's quite a bit out of date. Let me know when you want me to review this again.

raygervais · 2021-03-15T00:08:01Z

Will do @humphd, right now just getting the env setup to write tests. Currently running into this issue:

raygervais@MacOS ~/D/R/telescope> docker-compose --env-file config/env.development up -d
                                  
Creating elasticsearch ... 
Creating redis         ... 
Creating elasticsearch ... done
Creating redis         ... done
ERROR: for traefik  Cannot start service traefik: Ports are not available: listen tcp 0.0.0.0:80: bi

humphd · 2021-03-15T00:09:13Z

What's running on 80 for you?

raygervais · 2021-03-15T00:14:24Z

httpd       111           root    4u  IPv6 0xb9c406840149f42d      0t0    TCP *:80 (LISTEN)
httpd       674           _www    4u  IPv6 0xb9c406840149f42d      0t0    TCP *:80 (LISTEN)

Welp. Looks like I really need to clean this system given I'm fairly confident this is a Apache/PHP setup still running.

raygervais · 2021-03-15T00:17:08Z

Yup it was, back to writing tests now.

humphd · 2021-03-15T00:19:08Z

Cool. I know you are well versed on docker and such, but if any of this needs explaining, let me know. I had to iterate and change a lot of it to make it all work.

raygervais · 2021-03-15T00:56:27Z

One question which I cannot seem to figure out, according to the README and env.development files, it implies I should be able to access the API via api.telescope.localhost/v1, yet I cannot seem to connect to anything but Traefik itself via localhost:8080. See docker ps output:

raygervais@MacOS ~/D/R/telescope> docker-compose --env-file config/env.development ps -a
     Name                   Command               State                                                           Ports                                                        
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
apm              /usr/local/bin/docker-entr ...   Up      0.0.0.0:55097->8200/tcp                                                                                              
auth             docker-entrypoint.sh dumb- ...   Up      0.0.0.0:55092->7777/tcp                                                                                              
elasticsearch    /tini -- /usr/local/bin/do ...   Up      0.0.0.0:9200->9200/tcp,0.0.0.0:55091->9200/tcp, 9300/tcp                                                             
feed-discovery   docker-entrypoint.sh dumb- ...   Up      0.0.0.0:55095->7777/tcp                                                                                              
firebase         docker-entrypoint.sh fireb ...   Up      0.0.0.0:55099->4000/tcp, 5000/tcp, 5001/tcp, 8080/tcp, 8085/tcp, 0.0.0.0:8088->8088/tcp, 9000/tcp, 9005/tcp, 9099/tcp
image            docker-entrypoint.sh dumb- ...   Up      0.0.0.0:55093->4444/tcp                                                                                              
login            docker-php-entrypoint apac ...   Up      80/tcp, 0.0.0.0:55098->8080/tcp, 8443/tcp                                                                            
posts            docker-entrypoint.sh dumb- ...   Up      0.0.0.0:55094->5555/tcp                                                                                              
redis            docker-entrypoint.sh redis ...   Up      0.0.0.0:6379->6379/tcp,0.0.0.0:55090->6379/tcp                                                                       
search           docker-entrypoint.sh dumb- ...   Up      0.0.0.0:55096->4445/tcp                                                                                              
traefik          /entrypoint.sh --log.level ...   Up      0.0.0.0:80->80/tcp, 0.0.0.0:8080->8080/tcp

And traefik:

humphd · 2021-03-15T02:27:50Z

@raygervais we found out today that Docker on Windows can't do this custom domain name resolution via its DNS, so we switched to use localhost:80/v1/image etc. It's not as pretty as api.telescope.localhost but it lets our Windows friends play along.

I'm fixing it in #1944, which you can review if you want.

…backend fixes: missing variables from port adds: documentation updates and code cleanup adds: corrections based on feedback and improvements adds: port of original tests (failing WIP) fixes: rebase issues and updates docker env

humphd · 2021-03-16T01:54:20Z

docker/docker-compose.yml

@@ -157,3 +185,22 @@ services:
        hard: -1
    ports:
      - '9200'
+    labels:


NOTE: @manekenpix recently changed this to expose 9200 in development, since we can't do fancy domain names for localhost. Check what's on master.

@humphd @manekenpix I don't see the Labels part on master - should it be removed?

Follow the way things are on master yes. NOTE: we define things in different places now: the docker-compose.yml is our base file, and development.yml are overrides/extras for development/CI; production.yml for production. So look in all three places.

humphd · 2021-03-16T01:54:31Z

docker/docker-compose.yml

+    environment:
+      - ELASTIC_APM_SERVICE_NAME=search
+      - SEARCH_PORT
+


Remove this

@humphd Can you please clarify what should be removed?

Remove the blank line

humphd · 2021-03-16T01:54:55Z

docker/docker-compose.yml

+      # Specify the redis port
+      - 'traefik.http.services.elasticsearch.loadbalancer.server.port=9200'
+
+  apm:


This belongs in production.yml

According to This ToDo, I will move the APM part to production.yml.

@humphd @manekenpix Also, why don't we have all of this on master, but just referenced in a ToDo?

We haven't done the Elastic/ELK stuff yet for logging, etc. We started to, but will follow-up with it later.

humphd · 2021-03-16T01:55:46Z

src/api/search/Dockerfile

+
+COPY --chown=node:node . .
+
+RUN npm install ci --only=production


RUN npm install --only=production --no-package-lock

humphd · 2021-03-16T01:56:10Z

src/api/search/package.json

+  "version": "1.0.0",
+  "description": "A service for optimizing images",
+  "scripts": {
+    "dev": "nodemon server.js | pino-pretty -c -t",


pino pretty isn't needed now, we do it for you in Satellite.

humphd · 2021-03-16T01:56:48Z

src/api/search/package.json

+  "scripts": {
+    "dev": "nodemon server.js | pino-pretty -c -t",
+    "start": "node server.js",
+    "test": "jest test.js"


Test config and execution all happens from the root now. Setup your tests like we do in image, feed-discovery, posts, etc. https://github.com/Seneca-CDOT/telescope/blob/master/src/api/image/jest.config.js

humphd · 2021-03-16T01:57:19Z

src/api/search/package.json

+    "node": ">=12.0.0"
+  },
+  "devDependencies": {
+    "del-cli": "^3.0.1",


Remove everything in here except supertest and nodemon.

humphd · 2021-03-16T01:58:25Z

src/api/search/src/routes/query.js

+    const { text, filter, page, perPage } = req.query;
+    res.send(await search(text, filter, page, perPage));
+  } catch (error) {
+    res.status(500).send(`There was an error while executing your query: ${error}`);


For the sake of using APM, you might want to call next(err) instead, and let it go down through to the error handler, where we'll log and deal with it in APM.

humphd · 2021-03-16T01:59:05Z

src/api/search/test.js

@@ -0,0 +1,118 @@
+const request = require('supertest');
+process.env.ELASTICSEARCH_HOSTS = 'http://elasticsearch:9200';


Your tests can read in config/env.development (see how auth's jest tests do it).

izhuravlev · 2021-03-26T15:11:59Z

@raygervais Do you need help on this one? Do you want me to participate?

raygervais · 2021-03-26T15:14:02Z

If you @izhuravlev or @HyperTHD want to take this over, it would be greatly appreciated. I'm bogged down with work and other items right now.

izhuravlev · 2021-03-26T15:16:20Z

@raygervais I'll take that.

@HyperTHD your help and advice will be greatly appreciated!

humphd · 2021-04-08T19:05:06Z

@izhuravlev What's the status on this?

izhuravlev · 2021-04-08T19:07:34Z

@izhuravlev What's the status on this?

@humphd added some changes and added a couple of questions above for the parts that are unclear.

humphd · 2021-04-09T17:50:53Z

Moving this work to #2106, thanks for getting it started.

vercel bot deployed to Preview February 20, 2021 17:16 View deployment

humphd reviewed Feb 20, 2021

View reviewed changes

src/api/search/server.js Show resolved Hide resolved

src/api/search/server.js Outdated Show resolved Hide resolved

vercel bot deployed to Preview February 20, 2021 17:55 View deployment

raygervais force-pushed the feature/search-microservice branch from 4e2e85c to c3b74da Compare February 20, 2021 18:06

vercel bot deployed to Preview February 20, 2021 18:06 View deployment

vercel bot deployed to Preview February 20, 2021 18:20 View deployment

raygervais force-pushed the feature/search-microservice branch from bd46a68 to 3eb9587 Compare February 20, 2021 18:23

vercel bot deployed to Preview February 20, 2021 18:23 View deployment

raygervais marked this pull request as ready for review February 20, 2021 18:24

humphd requested changes Feb 20, 2021

View reviewed changes

raygervais force-pushed the feature/search-microservice branch from 3eb9587 to bc2b7c8 Compare February 21, 2021 17:45

vercel bot deployed to Preview February 21, 2021 17:46 View deployment

vercel bot deployed to Preview February 21, 2021 18:29 View deployment

humphd mentioned this pull request Feb 21, 2021

Change traefik to use /v1/service-name URLs vs. subdomains #1812

Merged

izhuravlev assigned raygervais Feb 23, 2021

izhuravlev requested review from chrispinkney, HyperTHD and Metropass February 23, 2021 15:07

izhuravlev added this to the 1.8 Release milestone Feb 23, 2021

izhuravlev added the area: microservices label Feb 23, 2021

rjayroso modified the milestones: 1.8 Release, 1.9 Release Mar 9, 2021

rjayroso requested a review from birtony March 9, 2021 15:18

humphd mentioned this pull request Mar 12, 2021

Fix #1706: Add Microservices docker-compose to production startup #1900

Merged

raygervais force-pushed the feature/search-microservice branch from 3e5c882 to 11c9ca9 Compare March 14, 2021 21:39

vercel bot deployed to Preview March 14, 2021 21:39 View deployment

raygervais force-pushed the feature/search-microservice branch from 11c9ca9 to 2cc7341 Compare March 15, 2021 00:04

vercel bot deployed to Preview March 15, 2021 00:04 View deployment

vercel bot deployed to Preview March 15, 2021 00:59 View deployment

raygervais force-pushed the feature/search-microservice branch from 1a83b14 to a222715 Compare March 16, 2021 00:42

vercel bot deployed to Preview March 16, 2021 00:42 View deployment

humphd requested changes Mar 16, 2021

View reviewed changes

humphd assigned HyperTHD Mar 19, 2021

HyperTHD assigned izhuravlev Mar 23, 2021

This was referenced Mar 23, 2021

Fixes SearchResults not displaying posts when searching #2012

Closed

Have search service work in staging and production #2014

Closed

humphd modified the milestones: 1.9 Release, 2.0-alpha Release Mar 26, 2021

manekenpix mentioned this pull request Apr 4, 2021

Fix search page for vercel. #2075

Merged

8 tasks

izhuravlev mentioned this pull request Apr 9, 2021

Feature/search microservice #2106

Merged

8 tasks

humphd closed this Apr 9, 2021


		const query = require('./src/routes/query');

		const service = new Satellite();


		COPY --chown=node:node . .

		RUN npm install ci --only=production

		@@ -0,0 +1,118 @@
		const request = require('supertest');
		process.env.ELASTICSEARCH_HOSTS = 'http://elasticsearch:9200';

adds: initial service architecture for Elastic search service port #1802

adds: initial service architecture for Elastic search service port #1802

Conversation

raygervais commented Feb 20, 2021 • edited Loading

Issue This PR Addresses

Type of Change

Description

Checklist

humphd left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

humphd commented Mar 14, 2021

raygervais commented Mar 15, 2021

humphd commented Mar 15, 2021

raygervais commented Mar 15, 2021

raygervais commented Mar 15, 2021

humphd commented Mar 15, 2021

raygervais commented Mar 15, 2021

humphd commented Mar 15, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

izhuravlev Apr 8, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

izhuravlev commented Mar 26, 2021

raygervais commented Mar 26, 2021

izhuravlev commented Mar 26, 2021

humphd commented Apr 8, 2021

izhuravlev commented Apr 8, 2021

humphd commented Apr 9, 2021

raygervais commented Feb 20, 2021 •

edited

Loading

izhuravlev Apr 8, 2021 •

edited

Loading