chore: improve CI workflow #307

sebtiz13 · 2023-06-01T15:18:32Z

What does this PR do ?

Update and improve CI workflows, refactor actions directly in workflows steps and add check-types step to check types error before try to build.

Other changes

Improve docker-compose

Open redis and elastic port for local development
Add healthcheck on services to automatically restart when crash
Use node 16 version of kuzzle container
Add a volume to keep data of elasticsearch
Add a volume for the node_modules of kuzzle backend (prevent error with packages installed in local, ex: GLIBC error)

Boyscout

Update kuzzle dependency
Update kuzzle package from 2.23 to 2.25.
This is useful to update uWebSockets.js package from 20.0 to 20.7 to support node version 18

rolljee · 2023-06-01T15:33:37Z

.github/workflows/push_dev.workflow.yml

+        run: |
+          docker-compose up -d
+          # Start backend in background (& is mandatory to run process in background otherwise the task never continue)
+          npm run dev &


Isn't the docker compose suppose to launch the whole task, why nom run dev is needed ?

Docker only run redis and elasticsearch, kuzzle is run out of docker to improve the log of errors in CI.
Also maybe it's better to run npm run prod instead development mode for better tests.

This is not a good idea, kuzzle should be ran inside the docker compose, because when it will be deployed in production it will be used inside a docker container, so when you test it you should run it in the same environment.

If you seem to be lacking of logs, you should add a docker-compose logs kuzzle when the script fails in the wait-nuzzle-start.sh

The problem is not especially for wait-kuzzle.sh but when functional tests cause errors.
IMHO it's better like that, but no problem, I can restore into docker.

I understand why you want it this way, but in this way we diverge too much from the final environment in which a kuzzle is deployed and this creates more risk of trouble. Maybe we can talk about it on discord a bit to see if we can still have a docker environment, and more log that suit the debugging of the stack when it runs functional test

wdyt ?

kuzzle-device-manager/.github/workflows/pull_request.workflow.yml

Lines 41 to 53 in d5e85cc

- name: docker pull

run: docker compose --profile backend pull

- name: docker install dependencies

run: docker compose run --no-deps kuzzle /bin/bash -c "npm ci"

- name: docker backend

run: |

# Continue when error

set +e

export BACKEND_COMMAND="npm run build && npm run prod";

docker compose --profile backend up -d --wait

exitcode="$?"

[[ "$exitcode" == "0" ]] || docker compose logs kuzzle

exit "$exitcode"

I'm not sure if this is the best, but I had to split the docker setup into 3 steps to avoid the ci timeout

rolljee · 2023-06-01T15:33:47Z

.github/workflows/push_master.workflow.yml

+        run: |
+          docker-compose up -d
+          # Start backend in background (& is mandatory to run process in background otherwise the task never continue)
+          npm run dev &


Same comment

rolljee · 2023-06-01T15:35:10Z

docker-compose.yml

-    depends_on:
-      - redis
+    healthcheck:
+      test: ['CMD', 'curl', '-f', 'http://kuzzle:7512/_healthCheck']


Since you are using Kuzzle 2.25, you can use "http://localhost:7512/_healthcheck" new public route

I think there is mistake because, I have used, this route for healthcheck on kuzzle service.

http://kuzzle:7512/_healthCheck does exists indeed but it will be deprecated

Hum sorry, I Don't really understand the difference between "http://localhost:7512/_healthcheck" and "http://kuzzle:7512/_healthCheck" for the test.
The only difference is the DNS and I found it's more accurate to use the service name instead localhost which is less clear.

Could you explain to me the difference ?

Sorry, I tested locally then forgot to update with the service name indeed

I meant http://kuzzle:7512/_healthcheck

The route you are using http://kuzzle:7512/_healthCheck is not a public route, it will be working as is until your kuzzle instance allow the anonymous the use this controller server.

{ -- verb: "get", path: "/_healthCheck", controller: "server", action: "healthCheck", }

Whereas the http://kuzzle:7512/_healthcheck and will always return a 200 since it is a public route

Ok my bad, I just saw the difference in typo between _healthCheck and _healthcheck

alexandrebouthinon · 2023-06-09T07:45:24Z

docker-compose.yml

+    healthcheck:
+      test: ['CMD', 'curl', '-f', 'http://kuzzle:7512/_healthcheck']
+      timeout: 1s
+      interval: 2s
+      retries: 10
+      start_period: 30s


I think we should not set a Docker Compose healthcheck on Kuzzle since functional test for example could lead to permissions issues on this route

Or at least you should handle 403/401 status code to be allowed

The healthcheck route are not forced to be public, or in minimal case, always allow call from localhost ?
Also, I don't see how to handle 403/401 status code here

The only way to do that is to create a custom healthcheck (like a Node.js or bash script for example)but for now ignore my comment

This health check method is public now 👌🏼 we should not have any issue see kuzzleio/kuzzle#2455

* chore: improve docker-compose config * ci: improve ci workflows * chore: remove useless postinstall * refactor: improve build packages types * chore: update kuzzle dependency * chore: update healthcheck endpoint * fix: restore run backend in docker * fix: correct docker setup in ci

sebtiz13 added 5 commits June 1, 2023 14:19

chore: improve docker-compose config

cba3594

ci: improve ci workflows

1a4555e

chore: remove useless postinstall

3387154

refactor: improve build packages types

5914dfe

chore: update kuzzle dependency

8058ae3

sebtiz13 added the changelog:exclude label Jun 1, 2023

sebtiz13 requested review from alexandrebouthinon and rolljee June 1, 2023 15:18

sebtiz13 self-assigned this Jun 1, 2023

sebtiz13 mentioned this pull request Jun 1, 2023

feat: add groups for assets #306

Merged

rolljee requested changes Jun 1, 2023

View reviewed changes

sebtiz13 added 2 commits June 2, 2023 12:05

chore: update healthcheck endpoint

34f115d

fix: restore run backend in docker

b9d950a

sebtiz13 force-pushed the chore/improve-workflow branch 17 times, most recently from e549f93 to 55fe83c Compare June 2, 2023 13:53

sebtiz13 force-pushed the chore/improve-workflow branch from 55fe83c to 3406a80 Compare June 2, 2023 14:05

fix: correct docker setup in ci

d5e85cc

sebtiz13 force-pushed the chore/improve-workflow branch from 3406a80 to d5e85cc Compare June 2, 2023 14:11

sebtiz13 requested a review from rolljee June 2, 2023 14:25

alexandrebouthinon reviewed Jun 9, 2023

View reviewed changes

alexandrebouthinon approved these changes Jun 9, 2023

View reviewed changes

sebtiz13 merged commit 0849ea0 into 2-dev Jun 13, 2023

sebtiz13 deleted the chore/improve-workflow branch June 13, 2023 10:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: improve CI workflow #307

chore: improve CI workflow #307

sebtiz13 commented Jun 1, 2023

rolljee Jun 1, 2023

sebtiz13 Jun 1, 2023

rolljee Jun 2, 2023 •

edited

Loading

sebtiz13 Jun 2, 2023

rolljee Jun 2, 2023

sebtiz13 Jun 2, 2023

rolljee Jun 1, 2023

rolljee Jun 1, 2023

sebtiz13 Jun 1, 2023

rolljee Jun 2, 2023

sebtiz13 Jun 2, 2023

rolljee Jun 2, 2023

sebtiz13 Jun 2, 2023

sebtiz13 Jun 2, 2023

alexandrebouthinon Jun 9, 2023

alexandrebouthinon Jun 9, 2023

sebtiz13 Jun 9, 2023

alexandrebouthinon Jun 9, 2023

rolljee Jun 9, 2023

	- name: docker pull
	run: docker compose --profile backend pull
	- name: docker install dependencies
	run: docker compose run --no-deps kuzzle /bin/bash -c "npm ci"
	- name: docker backend
	run: \|
	# Continue when error
	set +e
	export BACKEND_COMMAND="npm run build && npm run prod";
	docker compose --profile backend up -d --wait
	exitcode="$?"
	[[ "$exitcode" == "0" ]] \|\| docker compose logs kuzzle
	exit "$exitcode"

chore: improve CI workflow #307

chore: improve CI workflow #307

Conversation

sebtiz13 commented Jun 1, 2023

What does this PR do ?

Other changes

Boyscout

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rolljee Jun 2, 2023 • 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rolljee Jun 2, 2023 •

edited

Loading