Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create a containerize environment for development workflow #86

Merged
merged 42 commits into from
May 22, 2023

Conversation

ctmbl
Copy link
Contributor

@ctmbl ctmbl commented Mar 30, 2023

Close #64

This PR adds Dockerfiles and docker-compose file as well as a nginx conf template for development mode.
The main differences with the prod mode are:

  • shared volume for the source code, it is not embedded in the image
  • no certbot/no HTTPS because we don't need it => a different nginx config
  • not compiled frontend (then less efficient but should refresh itself on code changes)
  • that's it I think?

What solutions I chose

  • node_modules are in the image, so they don't overlap with one that could exists on home system, also we don't share the whole frontend and backend folder so local node_modules don't overlap with containerized one
  • separated files for dev and prod mode (for Docker, compose, and nginx) --> see next section

What still needs to be done/doesn't satisfy me

  • the nginx conf simply doesn't work at the time I would greatly appreciate the help of @atxr --> it works see below tks to 00d0912
  • the backend fails to reach mongoDB I get the error (1) (see Appendices section)
  • the frontend seems to fail to reach the backend I get (2)
  • I didn't tested that the website refreshes well on code chnages but I don't doubt that it does because the volumes are shared (cf the docker-compose-dev.yml) --> tested well on fc1c601
  • the call MODE=$MODE ./run_nginx.sh in nginx/Dockerfile isn't really beautiful I'd say
  • different Dockerfiles or an only one with multiple targets? --> don't care for now

Appendices

(1)

iscscfr-node-app-dev-1   | MongooseServerSelectionError: Could not connect to any servers in your MongoDB Atlas cluster. One common reason is that you're trying to access the database from an IP that isn't whitelisted. Make sure your current IP address is on your Atlas cluster's IP whitelist: https://www.mongodb.com/docs/atlas/security-whitelist/
iscscfr-node-app-dev-1   |     at Connection.openUri (/backend/node_modules/mongoose/lib/connection.js:825:32)
iscscfr-node-app-dev-1   |     at /backend/node_modules/mongoose/lib/index.js:414:10
iscscfr-node-app-dev-1   |     at /backend/node_modules/mongoose/lib/helpers/promiseOrCallback.js:41:5
iscscfr-node-app-dev-1   |     at new Promise (<anonymous>)
iscscfr-node-app-dev-1   |     at promiseOrCallback (/backend/node_modules/mongoose/lib/helpers/promiseOrCallback.js:40:10)
iscscfr-node-app-dev-1   |     at Mongoose._promiseOrCallback (/backend/node_modules/mongoose/lib/index.js:1288:10)
iscscfr-node-app-dev-1   |     at Mongoose.connect (/backend/node_modules/mongoose/lib/index.js:413:20)
iscscfr-node-app-dev-1   |     at Object.<anonymous> (/backend/app.js:23:4)
iscscfr-node-app-dev-1   |     at Module._compile (node:internal/modules/cjs/loader:1254:14)
iscscfr-node-app-dev-1   |     at Module._extensions..js (node:internal/modules/cjs/loader:1308:10) {
iscscfr-node-app-dev-1   |   reason: TopologyDescription {
iscscfr-node-app-dev-1   |     type: 'ReplicaSetNoPrimary',
iscscfr-node-app-dev-1   |     servers: Map(3) {
iscscfr-node-app-dev-1   |       'ac-a2ywkbi-shard-00-01.a11re32.mongodb.net:27017' => [ServerDescription],
iscscfr-node-app-dev-1   |       'ac-a2ywkbi-shard-00-02.a11re32.mongodb.net:27017' => [ServerDescription],
iscscfr-node-app-dev-1   |       'ac-a2ywkbi-shard-00-00.a11re32.mongodb.net:27017' => [ServerDescription]
iscscfr-node-app-dev-1   |     },
iscscfr-node-app-dev-1   |     stale: false,
iscscfr-node-app-dev-1   |     compatible: true,
iscscfr-node-app-dev-1   |     heartbeatFrequencyMS: 10000,
iscscfr-node-app-dev-1   |     localThresholdMS: 15,
iscscfr-node-app-dev-1   |     setName: 'atlas-s2e0y3-shard-0',
iscscfr-node-app-dev-1   |     maxElectionId: null,
iscscfr-node-app-dev-1   |     maxSetVersion: null,
iscscfr-node-app-dev-1   |     commonWireVersion: 0,
iscscfr-node-app-dev-1   |     logicalSessionTimeoutMinutes: null
iscscfr-node-app-dev-1   |   },
iscscfr-node-app-dev-1   |   code: undefined
iscscfr-node-app-dev-1   | }

(2)

iscscfr-react-app-dev-1  | Proxy error: Could not proxy request /api/articles from localhost:3000 to http://localhost:3001.
iscscfr-react-app-dev-1  | See https://nodejs.org/api/errors.html#errors_common_system_errors for more information (ECONNREFUSED).

@ctmbl ctmbl added help wanted Extra attention is needed Priority: High The Issue must be addressed as soon as possible labels Mar 30, 2023
@ctmbl ctmbl added this to the iScsc blog v0.2.0 milestone Mar 30, 2023
@ctmbl ctmbl self-assigned this Mar 30, 2023
@ctmbl ctmbl marked this pull request as draft March 30, 2023 13:45
@ctmbl
Copy link
Contributor Author

ctmbl commented Mar 30, 2023

I was getting with nginx container the following error:

iscscfr-nginx-1          | 2023/03/30 13:57:46 [emerg] 9#9: host not found in upstream "react-app" in /etc/nginx/nginx.conf:40

00d0912 have fixed it but still the backend doesn't reach mongo DB

@ctmbl ctmbl mentioned this pull request Mar 31, 2023
Copy link
Member

@amtoine amtoine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good for now.

solving the issues that make docker compose not run properly and i'll approve this 👌

.env.example Show resolved Hide resolved
frontend/package.json Show resolved Hide resolved
nginx/nginx.conf.prod.template Outdated Show resolved Hide resolved
nginx/nginx.conf.dev.template Show resolved Hide resolved
Copy link
Contributor

@atxr atxr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Last commit introduces two errors 🤔

nginx/nginx.conf.dev.template Outdated Show resolved Hide resolved
nginx/nginx.conf.dev.template Outdated Show resolved Hide resolved
@ctmbl
Copy link
Contributor Author

ctmbl commented Apr 12, 2023

This PR will stay idle for a while because we'll be working on two major changes that'll affect it:

  • migrate the DB to a local docker container, that should solve on the bug we currently experience
  • migrate the backend to python

atxr and others added 5 commits April 15, 2023 12:29
This merge is needed to bring recent DB local containerization iScsc#92 to
this branch
I simply refactor each service config attributes order
the order I chose is arbitrary but seems logical
 - [A|B] means A OR B, because they are mutually exclusive
```yml
services:
  <service name>:
    depends_on:
    [build|image]:
    networks:
    restart:
    env_file:
    ports:
    volumes:
```
@atxr
Copy link
Contributor

atxr commented May 16, 2023

Will do this evening 🚒

…format

This log_format named 'main' isn't used anyway so it is useless, I keep those lines commented anyway for a later PR where I'll properly set the logging in dev and prod mode
@ctmbl
Copy link
Contributor Author

ctmbl commented May 16, 2023

@atxr tks!

@atxr
Copy link
Contributor

atxr commented May 16, 2023

Thanks to b0ab27d, everything is working on my side!
Let's merge this 🏑

atxr
atxr previously approved these changes May 16, 2023
docker-compose.yml Outdated Show resolved Hide resolved
frontend/package.json Show resolved Hide resolved
.env.example Show resolved Hide resolved
ctmbl added a commit to ctmbl/iscsc.fr that referenced this pull request May 16, 2023
Remove env-cmd call in npm 'start' script --> handle by docker compose

Add nginx dev config (see commit body)

Took the one before [https](8087def) was introduced
It was introduced with [dev and prod mode](f20a338)

Add backend dev Dockerfile

Add frontend dev Dockerfile

Add dev docker compose

Separate dev and prod mode for the nginx template

created 2 separate template conf and pass an env var MODE
from .env to docker to run.sh for the nginx config

Update shared volumes in docker compose dev to only share source code

Fix nginx container error because targets not found

name of react and not containers was wrong

Remove commented dead config from nginx conf dev template

Add explicit logging config to nginx conf dev template

Adapt docker-compose-dev.yml to local containerized DB

Refactor docker-compose-dev.yml to be easier to understand

I simply refactor each service config attributes order
the order I chose is arbitrary but seems logical
 - [A|B] means A OR B, because they are mutually exclusive
```yml
services:
  <service name>:
    depends_on:
    [build|image]:
    networks:
    restart:
    env_file:
    ports:
    volumes:
```

Remove a useless tabulation in docker-compose-dev.yml

Refactor docker-compose.yml the same way than docker-compose-dev.yml

I simply refactor each service config attributes order
the order I chose is arbitrary but seems logical
 - [A|B] means A OR B, because they are mutually exclusive
```yml
services:
  <service name>:
    depends_on:
    [build|image]:
    networks:
    restart:
    env_file:
    ports:
    volumes:
```

Set the frontend's `proxy` with env var instead of hardcoded value

remove the hardcoded value of `proxy` in package.json

the frontend fails to reach the backend in container because
thanks to docker networks the backend exists at
`node-app[-dev]:$NODE_PORT` from the frontend container,
not at `localhost:$NODE_PORT`

add http-proxy-middleware to package.json and create a
./frontend/src/setupProxy.js to configure the proxy
following [react tutorial](https://create-react-app.dev/docs/proxying-api-requests-in-development/#configuring-the-proxy-manually)

PS: the changes in `package-lock.json` seems to come from the fact
that the newly installed package shares dependencies with other
and they ahve different needs over these shared dependencies

Update .env.example

Fix nginx container crash because it can't access to logs folder

git addugit addu I keep these lines commented because I want to add logging withb it iScsc#10

Configure MongoDB port through additional flags to pass to mongod in conatiner

Fix non persistent DB by mounting the right volume in the container

Run prettier

Specify read-write permissions on mounted volumes

Remove containers restart option in dev mode

Fix database container (iScsc#95)

* Fix non persistent DB by mounting the right volume in the container

* Configure MongoDB port through additional flags to pass to mongod in conatiner

* Run prettier

* Add mongodb folder to gitignore

* Set bitnami/mongodb version tag to latest

pros and cons have been discussed here: iScsc#95 (comment)

Improve DB_PORT passing to MongoDB

I just better read the doc https://hub.docker.com/r/bitnami/mongodb/ search for MONGODB_PORT_NUMBER

Write a first version of README about dev mode containerized

Add setup-db-folder.sh script

Update README with DB folder setup and clean it

Add an important waning about mongodb folder permissions to README

Fix setup-db-folder.sh script

Update bitnami mongoDB used image to latest

Remove restart attribute in docker compose dev, useless in dev mode

Update GH Action to exclude mongodb folder from prettier checking

Run prettier

Try a prettier GH Action fix

Armor nginx run script against unexpected MODE value leading to conf template not found

Fix variables wrongly substituted in nginx dev conf and comment logs format

This log_format named 'main' isn't used anyway so it is useless, I keep those lines commented anyway for a later PR where I'll properly set the logging in dev and prod mode

Improve setup database script
@gbrivady
Copy link
Member

gbrivady commented May 17, 2023

Had to try twice to make it work, but with the help of @ctmbl to clean and re-install everything it is working, so I'm going to suppose it was an error on my side.

Anyway thanks for the scripts and the updated readme, much better and simpler that way to test everything!

Good to go for me then 🚀

gbrivady
gbrivady previously approved these changes May 17, 2023
ctmbl added a commit that referenced this pull request May 19, 2023
* Add Node development environment (#86 still not merged)

Remove env-cmd call in npm 'start' script --> handle by docker compose

Add nginx dev config (see commit body)

Took the one before [https](8087def) was introduced
It was introduced with [dev and prod mode](f20a338)

Add backend dev Dockerfile

Add frontend dev Dockerfile

Add dev docker compose

Separate dev and prod mode for the nginx template

created 2 separate template conf and pass an env var MODE
from .env to docker to run.sh for the nginx config

Update shared volumes in docker compose dev to only share source code

Fix nginx container error because targets not found

name of react and not containers was wrong

Remove commented dead config from nginx conf dev template

Add explicit logging config to nginx conf dev template

Adapt docker-compose-dev.yml to local containerized DB

Refactor docker-compose-dev.yml to be easier to understand

I simply refactor each service config attributes order
the order I chose is arbitrary but seems logical
 - [A|B] means A OR B, because they are mutually exclusive
```yml
services:
  <service name>:
    depends_on:
    [build|image]:
    networks:
    restart:
    env_file:
    ports:
    volumes:
```

Remove a useless tabulation in docker-compose-dev.yml

Refactor docker-compose.yml the same way than docker-compose-dev.yml

I simply refactor each service config attributes order
the order I chose is arbitrary but seems logical
 - [A|B] means A OR B, because they are mutually exclusive
```yml
services:
  <service name>:
    depends_on:
    [build|image]:
    networks:
    restart:
    env_file:
    ports:
    volumes:
```

Set the frontend's `proxy` with env var instead of hardcoded value

remove the hardcoded value of `proxy` in package.json

the frontend fails to reach the backend in container because
thanks to docker networks the backend exists at
`node-app[-dev]:$NODE_PORT` from the frontend container,
not at `localhost:$NODE_PORT`

add http-proxy-middleware to package.json and create a
./frontend/src/setupProxy.js to configure the proxy
following [react tutorial](https://create-react-app.dev/docs/proxying-api-requests-in-development/#configuring-the-proxy-manually)

PS: the changes in `package-lock.json` seems to come from the fact
that the newly installed package shares dependencies with other
and they ahve different needs over these shared dependencies

Update .env.example

Fix nginx container crash because it can't access to logs folder

git addugit addu I keep these lines commented because I want to add logging withb it #10

Configure MongoDB port through additional flags to pass to mongod in conatiner

Fix non persistent DB by mounting the right volume in the container

Run prettier

Specify read-write permissions on mounted volumes

Remove containers restart option in dev mode

Fix database container (#95)

* Fix non persistent DB by mounting the right volume in the container

* Configure MongoDB port through additional flags to pass to mongod in conatiner

* Run prettier

* Add mongodb folder to gitignore

* Set bitnami/mongodb version tag to latest

pros and cons have been discussed here: #95 (comment)

Improve DB_PORT passing to MongoDB

I just better read the doc https://hub.docker.com/r/bitnami/mongodb/ search for MONGODB_PORT_NUMBER

Write a first version of README about dev mode containerized

Add setup-db-folder.sh script

Update README with DB folder setup and clean it

Add an important waning about mongodb folder permissions to README

Fix setup-db-folder.sh script

Update bitnami mongoDB used image to latest

Remove restart attribute in docker compose dev, useless in dev mode

Update GH Action to exclude mongodb folder from prettier checking

Run prettier

Try a prettier GH Action fix

Armor nginx run script against unexpected MODE value leading to conf template not found

Fix variables wrongly substituted in nginx dev conf and comment logs format

This log_format named 'main' isn't used anyway so it is useless, I keep those lines commented anyway for a later PR where I'll properly set the logging in dev and prod mode

Improve setup database script

* Update docker-compose-dev.yml for a flask backend

* Update backend/Dockerfile.dev to a python image running flask

* Fix FLASK_RUN_PORT env var name

* Update nginx conf to proxy to the flask app

* Fix flaks container not accessible from outside the container

* Update lasting wrong NODE_PORT var in .env.example

Co-authored-by: Alexandre Tullot <alexandre.tullot@protonmail.com>

---------

Co-authored-by: Alexandre Tullot <alexandre.tullot@protonmail.com>
Solves README.md conflicts
@ctmbl ctmbl dismissed stale reviews from gbrivady and atxr via d3cc944 May 19, 2023 14:29
atxr
atxr previously approved these changes May 19, 2023
gbrivady
gbrivady previously approved these changes May 19, 2023
Copy link

@J3y0 J3y0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am honnestly not qualified and have no idea of how to configure a docker-compose.yml file. I have no idea how it is constructed and as it has to be merged quickly, I have a plain confidence in what you have written ! Same thing for the nginx-related files.

For the Dockerfile, this looks good to me ! Same for bash scripts, I just have one or two questions.

Finally, the README.md is very clear. I just add a suggestion because I was wondering myself how to check the status for a second and I would have love not to think about it and blindly copy the command right from the README.md !

Thanks a lot for the amazing PR. That will ease a lot the deployment !

README.md Outdated Show resolved Hide resolved
scripts/setup-db-folder.sh Show resolved Hide resolved
scripts/setup-db-folder.sh Show resolved Hide resolved
@ctmbl ctmbl dismissed stale reviews from gbrivady and atxr via 7b4d4bd May 22, 2023 17:07
@ctmbl ctmbl requested review from gbrivady, atxr and J3y0 May 22, 2023 17:07
Copy link
Member

@gbrivady gbrivady left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✔️

@ctmbl ctmbl merged commit 17c1126 into iScsc:main May 22, 2023
@ctmbl ctmbl deleted the dev-container branch May 22, 2023 17:47
@ctmbl ctmbl linked an issue Jun 2, 2023 that may be closed by this pull request
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed Priority: High The Issue must be addressed as soon as possible
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Automation and enhancement of the dev workflow Add containerization in development mode
6 participants