Add initial CI/CD workflow #5

Merseleo · 2025-02-25T13:57:40Z

Closes #3

What I have made

added a basic CI/CD pipeline to the mock backend, including jobs for
- building the mock backend
- linting the mock backend (In order for this job to run successfully, the code had to be formatted beforehand)
- building and publishing a docker image of the mock backend to GitHub registry
- deploying the mock backend on the snowballr server
re-initialize docker for the mock backend (using docker init) for best practices
small improvements in the eslint and prettier configuration

…ted grpc files to prettierignore

…and compose.yml

…(inspired by job in snowballr-api)

…ker image

…bmodule is initialized in docker job

Merseleo · 2025-03-06T16:41:00Z

Probably we need to find another solution for the deployment, because if we regularly start new mock backends on the server without stopping and deleting the existing containers, we will run into space problems very quickly. I am open for suggestions.

luca-schlecker · 2025-03-06T19:01:55Z

.dockerignore

+**/docker-compose*
+**/compose.y*ml
+**/Dockerfile*


Didn't we just remove them in the last pr?

You are, as I said I generated this file and docker seems to be the opinion that these files should not be included, but as I said, I would integrate it. I will delete it.

luca-schlecker · 2025-03-06T19:02:22Z

.dockerignore

Seems to me like there is a lot in here that is not part of our project.

You are right, there is a lot of stuff not included in our project. I didnt create this .dockerignore on my own, but (as I wrote in the PR description) used docker init as @Slartibartfass2 suggested. This command generated this file and the compose file and Dockerfile etc. based on our files and additional best practices. So imo it is no problem, that this file ignore more files per default than we have, because if we add files later it is more unlikely that we get problems because someone (probably me) forgot to add the files to this .dockerignore

luca-schlecker · 2025-03-06T19:06:38Z

compose.yaml

Why go for a compose file if we're only working with one container?

I didn't create this compose file, it was completely generated by docker init

Imo, we should not delete it either, because it seems to me that on the one hand it is easier to start a container with a compose file and on the other hand it is not impossible that another container could come later or something and then you can simply extend it directly.

Basically, I understand that it seems pointless, but to be honest, I don't think this file needs to be maintained and therefore doesn't mean any effort, does it?

luca-schlecker · 2025-03-06T19:07:19Z

package.json

@@ -1,7 +1,7 @@
 {
    "name": "snowballr-mock-backend",
-    "version": "1.0.0",
-    "description": "A Mock Backend for the SnowballR Frontend Development",


Is title-case a no-go?

No it is not a no-go, in my experience it's just very unusual for descriptions, especially of packages. But if you want to go with this title-case I can change it back, but the version should stay 0.1, as everything can still change in the mock backend (e.g. dynamically load data at the start will be introduced soon).

luca-schlecker · 2025-03-06T19:11:35Z

src/logging-interceptor.ts

-export const LOGGING_INTERCEPTOR: ServerInterceptor = function (
-    methodDescriptor: ServerMethodDefinition<any, any>,
+export const LOGGING_INTERCEPTOR: ServerInterceptor = function <RequestT, ResponseT>(
+    methodDescriptor: ServerMethodDefinition<RequestT, ResponseT>,


Interesting choice. I am surprised it works, I didn't know you could just do that.

I searched for a solution of this problem of types for generic functions and the first solution was using Generics. Didnt know this either, but it works and seems a bit more elegant to. Or should I change it back and add a comment for the linter?

luca-schlecker · 2025-03-06T19:17:55Z

Dockerfile

+
+################################################################################
+# Create a stage for installing production dependecies.
+FROM base as deps


Docker warns about inconsistent casing of keywords. as should probably be capitalized everywhere.

Good catch, I corrected it.

luca-schlecker · 2025-03-06T19:22:25Z

.github/workflows/quality-check.yml

+              with:
+                  submodules: "recursive"


Do we explicitly want to pull all submodules for linting?

Actually we dont need to, but I pull the submodule to use the custom "setup" workflow (with always generate the grpc stuff and therefore need the submodule). If it is in your opinion not good practice, I can delete it and dont use the "setup" workflow.

luca-schlecker · 2025-03-06T19:26:49Z

.github/workflows/deploy.yml

+
+                    docker pull ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}
+
+                    docker run -d --name snowballr-mock-backend -p 3001:3001 ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}


What are your thoughts on exposing both viable ports? (the grpc-web one and the native one)

My first thought was, that it is not necessary, as this is the deployed mock backend and why should we use grpcui or grpcurl with this, but on the other hand, it shouldn't cause any problem to expose the other port, so I will add it.

luca-schlecker · 2025-03-06T19:27:08Z

.github/workflows/deploy.yml

+                  mkdir -p ~/.ssh
+                  echo "${{ secrets.SSH_PRIVATE_KEY }}" > ~/.ssh/id_rsa
+                  chmod 600 ~/.ssh/id_rsa
+                  echo "StrictHostKeyChecking no" >> ~/.ssh/config


Isn't this already part of the ssh command below?

Actually it is, you are right (I must admit, I only copied this code and forgot to check, whether there are duplicate "lines"). I delete this line.

luca-schlecker · 2025-03-06T19:37:20Z

.github/workflows/deploy.yml

+                  context: .
+                  tags: ${{ steps.meta.outputs.tags }}
+                  labels: ${{ steps.meta.outputs.labels }}


Is the missing push: true intentional?

No, i accidentally deleted this line.

luca-schlecker · 2025-03-06T19:39:37Z

Probably we need to find another solution for the deployment, because if we regularly start new mock backends on the server without stopping and deleting the existing containers, we will run into space problems very quickly. I am open for suggestions.

Couldn't we use the latest tag and update the existing container instead of pulling a new specific version?

…rnings in Dockerfile and delete Dockerfile and compose file from .dockerignore)

…000 and remove duplicate commands)

Merseleo

Thank you for the quick response. I guess you also didn't test the deploy workflow. Any ideas how we can properly test it or do you think it will work without any problems or we will see the errors when merging to main?

Merseleo · 2025-03-07T09:13:44Z

.dockerignore

You are right, there is a lot of stuff not included in our project. I didnt create this .dockerignore on my own, but (as I wrote in the PR description) used docker init as @Slartibartfass2 suggested. This command generated this file and the compose file and Dockerfile etc. based on our files and additional best practices. So imo it is no problem, that this file ignore more files per default than we have, because if we add files later it is more unlikely that we get problems because someone (probably me) forgot to add the files to this .dockerignore

Merseleo · 2025-03-07T09:14:37Z

.dockerignore

+**/docker-compose*
+**/compose.y*ml
+**/Dockerfile*


You are, as I said I generated this file and docker seems to be the opinion that these files should not be included, but as I said, I would integrate it. I will delete it.

Merseleo · 2025-03-07T09:20:49Z

Dockerfile

+
+################################################################################
+# Create a stage for installing production dependecies.
+FROM base as deps


Good catch, I corrected it.

Merseleo · 2025-03-07T09:25:37Z

compose.yaml

I didn't create this compose file, it was completely generated by docker init

Imo, we should not delete it either, because it seems to me that on the one hand it is easier to start a container with a compose file and on the other hand it is not impossible that another container could come later or something and then you can simply extend it directly.

Basically, I understand that it seems pointless, but to be honest, I don't think this file needs to be maintained and therefore doesn't mean any effort, does it?

Merseleo · 2025-03-07T09:29:02Z

package.json

@@ -1,7 +1,7 @@
 {
    "name": "snowballr-mock-backend",
-    "version": "1.0.0",
-    "description": "A Mock Backend for the SnowballR Frontend Development",


No it is not a no-go, in my experience it's just very unusual for descriptions, especially of packages. But if you want to go with this title-case I can change it back, but the version should stay 0.1, as everything can still change in the mock backend (e.g. dynamically load data at the start will be introduced soon).

Merseleo · 2025-03-07T09:32:03Z

src/logging-interceptor.ts

-export const LOGGING_INTERCEPTOR: ServerInterceptor = function (
-    methodDescriptor: ServerMethodDefinition<any, any>,
+export const LOGGING_INTERCEPTOR: ServerInterceptor = function <RequestT, ResponseT>(
+    methodDescriptor: ServerMethodDefinition<RequestT, ResponseT>,


I searched for a solution of this problem of types for generic functions and the first solution was using Generics. Didnt know this either, but it works and seems a bit more elegant to. Or should I change it back and add a comment for the linter?

Merseleo · 2025-03-07T09:34:31Z

.github/workflows/deploy.yml

+                  context: .
+                  tags: ${{ steps.meta.outputs.tags }}
+                  labels: ${{ steps.meta.outputs.labels }}


No, i accidentally deleted this line.

Merseleo · 2025-03-07T09:35:24Z

.github/workflows/quality-check.yml

+              with:
+                  submodules: "recursive"


Actually we dont need to, but I pull the submodule to use the custom "setup" workflow (with always generate the grpc stuff and therefore need the submodule). If it is in your opinion not good practice, I can delete it and dont use the "setup" workflow.

Merseleo · 2025-03-07T09:37:14Z

.github/workflows/deploy.yml

+
+                    docker pull ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}
+
+                    docker run -d --name snowballr-mock-backend -p 3001:3001 ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}


My first thought was, that it is not necessary, as this is the deployed mock backend and why should we use grpcui or grpcurl with this, but on the other hand, it shouldn't cause any problem to expose the other port, so I will add it.

Merseleo · 2025-03-07T09:38:52Z

.github/workflows/deploy.yml

+                  mkdir -p ~/.ssh
+                  echo "${{ secrets.SSH_PRIVATE_KEY }}" > ~/.ssh/id_rsa
+                  chmod 600 ~/.ssh/id_rsa
+                  echo "StrictHostKeyChecking no" >> ~/.ssh/config


Actually it is, you are right (I must admit, I only copied this code and forgot to check, whether there are duplicate "lines"). I delete this line.

Merseleo linked an issue Feb 25, 2025 that may be closed by this pull request

Add initial CI/CD workflow #3

Open

Merseleo requested a review from Slartibartfass2 February 25, 2025 13:57

Merseleo self-assigned this Feb 25, 2025

Merseleo removed the request for review from Slartibartfass2 February 25, 2025 13:58

Merseleo marked this pull request as draft February 25, 2025 13:58

Merseleo force-pushed the ci/3-add-initial-ci-cd-workflow branch 6 times, most recently from b2f988c to a39d677 Compare February 26, 2025 10:39

Merseleo added 8 commits March 6, 2025 09:27

ci: add build job to mock backend

90e6212

ci: add linting job to mock backend

80a1783

style: format code conforming to linter

c98d30e

chore: adapted eslint config to config from frontend and added genera…

a1a6e61

…ted grpc files to prettierignore

feat: initialize docker (using docker init) and configure Dockerfile …

767b157

…and compose.yml

ci: add job for building and publishing docker image of mock backend …

c49fb14

…(inspired by job in snowballr-api)

ci: add job for deploying the mock backend based of the published doc…

a41d3b8

…ker image

fix: expose grpc web proxy port 3001 in the compose.yml

4a3b87e

Merseleo force-pushed the ci/3-add-initial-ci-cd-workflow branch from 1c7d284 to 4a3b87e Compare March 6, 2025 09:22

Merseleo marked this pull request as ready for review March 6, 2025 09:23

Merseleo requested review from Slartibartfass2 and luca-schlecker March 6, 2025 09:23

Merseleo force-pushed the ci/3-add-initial-ci-cd-workflow branch 7 times, most recently from 39c03f5 to 3237bef Compare March 6, 2025 12:02

fix: checkout repository with submodules: recursive option, so API su…

380fba3

…bmodule is initialized in docker job

Merseleo force-pushed the ci/3-add-initial-ci-cd-workflow branch from 3237bef to 380fba3 Compare March 6, 2025 12:06

ci: expose port 3001 of the deployed docker container

81c9b34

luca-schlecker requested changes Mar 6, 2025

View reviewed changes

Merseleo removed the request for review from Slartibartfass2 March 7, 2025 09:07

refactor: fix some problems in autogenerated docker files (address wa…

5c9f1d4

…rnings in Dockerfile and delete Dockerfile and compose file from .dockerignore)

Merseleo force-pushed the ci/3-add-initial-ci-cd-workflow branch from 4b7a7e2 to b134277 Compare March 7, 2025 09:42

refactor: addressed feedback from PR in workflow files (expose port 3…

fbc2257

…000 and remove duplicate commands)

Merseleo force-pushed the ci/3-add-initial-ci-cd-workflow branch from b134277 to fbc2257 Compare March 7, 2025 09:45

Merseleo commented Mar 7, 2025

View reviewed changes

Merseleo requested a review from luca-schlecker March 7, 2025 14:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add initial CI/CD workflow #5

Add initial CI/CD workflow #5

Merseleo commented Feb 25, 2025 •

edited

Loading

Merseleo commented Mar 6, 2025

luca-schlecker Mar 6, 2025

Merseleo Mar 7, 2025

luca-schlecker Mar 6, 2025

Merseleo Mar 7, 2025

luca-schlecker Mar 6, 2025

Merseleo Mar 7, 2025

luca-schlecker Mar 6, 2025

Merseleo Mar 7, 2025

luca-schlecker Mar 6, 2025

Merseleo Mar 7, 2025

luca-schlecker Mar 6, 2025

Merseleo Mar 7, 2025

luca-schlecker Mar 6, 2025

Merseleo Mar 7, 2025

luca-schlecker Mar 6, 2025

Merseleo Mar 7, 2025

luca-schlecker Mar 6, 2025

Merseleo Mar 7, 2025

luca-schlecker Mar 6, 2025

Merseleo Mar 7, 2025

luca-schlecker commented Mar 6, 2025

Merseleo left a comment

Merseleo Mar 7, 2025

Merseleo Mar 7, 2025

Merseleo Mar 7, 2025

Merseleo Mar 7, 2025

Merseleo Mar 7, 2025

Merseleo Mar 7, 2025

Merseleo Mar 7, 2025

Merseleo Mar 7, 2025

Merseleo Mar 7, 2025

Merseleo Mar 7, 2025


		docker pull ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}

		docker run -d --name snowballr-mock-backend -p 3001:3001 ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}

Add initial CI/CD workflow #5

Are you sure you want to change the base?

Add initial CI/CD workflow #5

Conversation

Merseleo commented Feb 25, 2025 • edited Loading

What I have made

Merseleo commented Mar 6, 2025

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

luca-schlecker commented Mar 6, 2025

Merseleo 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

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

Merseleo commented Feb 25, 2025 •

edited

Loading