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.

Sign up for GitHub

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

Jump to bottom

ISSUE-27: Fix docker mongo setup #28

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

ISSUE-27: Fix docker mongo setup #28

wants to merge 5 commits into from

Conversation

carlan
Copy link

@carlan carlan commented Jun 27, 2020

This is the fix for the mongodb initialization. I haven't tested on MacOS and Windows, it's working properly on *nix based OS's.

What was done:

  • Updated server docker-compose.yml to add container name and new volume
  • Updated main README.md to add a note on MONGO_HOST configuration
  • Added mongo-init.sh to initialize mongodb container

Fixes #27 💚

Type of change

  • Bug fix

PR Checklist:

  • Merged the current development branch before submit a PR
  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings

carlan added 2 commits June 27, 2020 22:24
@carlan
ISSUE-27: Fix docker mongo setup
1022cf1 
- Updated server docker-compose.yml to add container name and new volume
- Updated main README.md to add a note on MONGO_HOST configuration
- Added mongo-init.sh to initialize mongodb container

Fixes CodingGarden#27 💚
@carlan
Merge branch 'master' into bugfix/issue-27
525f623
w3cj
w3cj requested changes Jul 12, 2020
View reviewed changes
Copy link
Member

@w3cj w3cj left a comment

Choose a reason for hiding this comment

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

See comment.

server/mongo-init.sh Outdated
@@ -0,0 +1,9 @@
#!/bin/bash
echo '========================================================================='
Copy link
Member

Choose a reason for hiding this comment

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

This is not required and happens by default... I removed all containers / images and db-data and was able to start up everything without this script.

The main fix that is needed is this:

  • An init script on the API container that waits to see the message waiting for connections on port 27017 before starting itself.

Copy link

Choose a reason for hiding this comment

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

For the waiting connections I made a PR #39

Copy link
Author

Choose a reason for hiding this comment

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

The main fix that is needed is this:

  • An init script on the API container that waits to see the message waiting for connections on port 27017 before starting itself.

See my last comment below.

carlan added 2 commits July 18, 2020 01:05
@carlan
ISSUE-27: Fix docker mongo setup
50f4f60 
- Rolled back previous changes after feedback
- Updated docker-compose.yml
- Updated gitignore
- Added initialization scripts for db and api
- Added placeholder file

Fixes CodingGarden#27 💚
@carlan
Merge branch 'master' into bugfix/issue-27
13dedb7
@carlan
Copy link
Author

carlan commented Jul 18, 2020 •
edited
Loading

👋 I had some time to work on this today.

I updated this PR to undo the changes I've made previously and introduce some new files to solve the issue. It's working fine on *nix. I'm curious to see it running on Mac.

One downside, the placeholder file docker-data/mongod.log will need to be present before hand otherwise docker will create it as a directory and we need it as a file. I had to update the gitignore and check in the docker-data and the placeholder file. EDIT: this isn't relevant with the latest changes.

Let me know your thoughts on it.

EDIT: I saw you had a bit of hard time understanding/remembering (no offence intended) what's going on. I should have added a comment explaining what I've done. Sorry about that.
The solution I've applied for this is rudimentar simple, I believe as you wanted/mentioned on previous stream. On db container I'm exporting the mongo logs in a volume, without it the server api wouldn't have a way to check if the database is ready or not based on the string. On server api container, I'm reading the log from the same volume and waiting for the string before continue.

@carlan carlan requested a review from w3cj July 18, 2020 10:01
@carlan
ISSUE-27: Fix docker mongo setup
e1d4d2e 
- Rolled back previous changes on gitignore
- Updated docker-compose.yml
- Updated db and api initialization scripts
- Deleted placeholder file

Fixes CodingGarden#27 💚
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brilvio brilvio brilvio left review comments

@w3cj w3cj Awaiting requested review from w3cj

Assignees
No one assigned
Labels
Changes Requested
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Fix docker mongo setup
3 participants
@carlan @brilvio @w3cj