supercharge / mongodb-github-action

Use MongoDB in GitHub Actions
MIT License
224 stars 46 forks source link

add replica set option #3

Closed StefanoDeVuono closed 4 years ago

StefanoDeVuono commented 4 years ago

This PR is to allow users to add replica sets to Mongo in GitHub Actions.

We use the INPUT environment variable to name a replica set if there is one and then start docker with a replica set and initialize it.

TODO:

marcuspoehls commented 4 years ago

@StefanoDeVuono Hey Stefano, thank you for the PR! I'll look over the changes and reply back. I'll probably be able to put my thoughts into this by tomorrow.

marcuspoehls commented 4 years ago

Hey Stefano. Thank you for your patience!

I'm currently busy with work. I'm sure I'll have time to get back to this PR and your work in the next days. Probably on Wednesday.

StefanoDeVuono commented 4 years ago

@marcuspoehls I'm still testing it out. I'll squash and let you know what I find

StefanoDeVuono commented 4 years ago

@marcuspoehls The code should be good now.

I created an automated test. It's in node.js, but lemme know if you want me to add it.

marcuspoehls commented 4 years ago

@StefanoDeVuono Hey Stefano, thank you for your comments on my questions. I can dedicate some time today to get this replica set feature added to this GitHub Action.

Can you please add your Node.js test to this PR? I think it’s a good idea to have an extra test for the replica set.

In the meantime, I’ll read about MongoDB replica sets and how we can ensure the functionality is working as expected. You did a great job extending the existing code with only a few lines to support replica sets. The best start I can get :) Thank you!

StefanoDeVuono commented 4 years ago

I added tests!

And I added the sleep back in. it looks like the server won't be ready for the rs.initiate() command otherwise. See here: https://github.com/StefanoDeVuono/mongodb-github-action/pull/2/checks?check_run_id=538467496

And with sleep: https://github.com/StefanoDeVuono/mongodb-github-action/pull/2/checks?sha=64910570108c1cf469c729acebf1037fbd37d68d

marcuspoehls commented 4 years ago

@StefanoDeVuono Thank you for the test.

I think your replica set isn't testing against the replica set. You're not passing down the options containing the replica set name when connecting Mongoose to MongoDB.

I was toying with replica set support in the replica-sets branch. I wasn't able to make a connection to the MongoDB replica set in the docker container.

Maybe you have an idea. Once we get that fixed, I'll merge this PR.

Thank you for your help!

StefanoDeVuono commented 4 years ago

@marcuspoehls I got it!

The replica sets have a members.name field which is the hostname. Output of rs.status() is

"set": "rsTest",
"...": "...",
"members" : [
    {
        "_id" : 0,
        "name" : "a7f90e76379b:27017",
        "...": "...",

on a dockerized replica set initialized with rs.initiate() where a7f90e76379b is the host, 27017 is the port, a set is the name of the replica set.

So, I pass in a basic replica set config

mongo --eval "rs.initiate({ \"_id\": \"$MONGO_REPL_SET\", \"members\": [{ \"_id\": 0, \"host\": \"localhost\" }] })"

and it seems to work

marcuspoehls commented 4 years ago

@StefanoDeVuono Thank you for your work Stefano! I really appreciate your help. I’ll release a new version by the end of today. I’m currently on vacation and will squeeze in some time to push a new release. Again, thank you for your patience and walking through all the issues related to replica sets 🙌