saltstack-formulas / mongodb-formula

http://docs.saltstack.com/en/latest/topics/development/conventions/formulas.html
Other
31 stars 121 forks source link

Split mongod and mongos, removed mongodb meta state #60

Closed sethcenterbar closed 4 years ago

sethcenterbar commented 5 years ago

references #56

So.. this is my first stab at it, attempting to follow your comments. I know it's not the most perfect solution, but it was the quickest way to separate them. I'm wondering if it would be best to leave the mongodb metastate as the installation of the server, and then have mongod and mongos simply configure and turn on those services. Thoughts @noelmcloughlin?

noelmcloughlin commented 5 years ago

It depends. One approach suggests mongodb and mongodb.clean metastate should handle everything (mongos, mongod, bi, roto3t) but the modularity you are introducing allows pick & choose. That is certainly consistent and perhaps solution of least surprise (a good thing).

The other approach suggests mongodb and mongodb.clean metastate handle the most common use case - namely mongod - on basis newbies/busy people get what they expected and nothing else

I would lean towards 1st option - the README is there for a reason.

I'll do some testing during the week. thanks.

sethcenterbar commented 5 years ago

Ok. You'll find in my pull request that I opted for there to be no mongodb.init, and assumed the user would just include the metastates they wanted. So, basically neither of the two things you just said 😆 Oops!

noelmcloughlin commented 5 years ago

No problem - If we want a mongodb.init it can be added as afterthought. Don't forget to update the README. I'll test next week and review the PR. thx

noelmcloughlin commented 5 years ago

Hi @sethcenterbar

I got some time to check the PR now - there are some issues. Here are a few suggestions.

  1. In mongodb/mongod directory change all occurrences of mongodb server to mongodb mongod.
  2. In mongodb/mongos directory change all occurrences of mongodb server to mongodb mongos.
  3. You could safely delete all directories named depreciated I think.
  4. Fix mongod/init.sls i.e. change server to mongod
  5. Fix mongos/init.sls i.e. change server to mongos.
  6. Maybe defaults.yaml should be tidied up similarly. The mongodb.server stuff could be moved to mongodb.system since "server" is defunct. something like this ...
    mongodb:
    system:
     ... no change here .. etc  ...
    package: mongodb-org      ##<<< mongodb.server.stuff moved here.
    version: 4.0
    use_repo: False
    use_archive: False
    use_schema: False
    disable_transparent_hugepages: true
    binpath: /usr
    user: mongodb
    group: mongodb
sethcenterbar commented 5 years ago

@noelmcloughlin , I really want to be able to help with this. I'm going to take some time to learn kitchen and try to understand the testing process better so I don't have to bounce every attempt off of you.

noelmcloughlin commented 5 years ago

I don't mind, learning kitchen is not necessary since you can just do testing in a local VM. This is your first stab at Salt so bounce off me all you want. There is saltstack community on Slack too where you can get advice. We're open!!

noelmcloughlin commented 5 years ago

Hi @sethcenterbar Did you get any further with this? I have two other PRs to get merged before/after yours.

sethcenterbar commented 5 years ago

I haven’t made any progress. Go ahead 🙃

On Mon, May 6, 2019 at 6:00 PM N notifications@github.com wrote:

Hi @sethcenterbar https://github.com/sethcenterbar Did you get any further with this? I have two other PRs to get merged before/after yours.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/saltstack-formulas/mongodb-formula/pull/60#issuecomment-489799072, or mute the thread https://github.com/notifications/unsubscribe-auth/AB54DXRXQ555RZ64VA3LBU3PUCS6ZANCNFSM4HASVIQA .

noelmcloughlin commented 5 years ago

It would be great to get this tested. The improvements in this PR are needed imho.

noelmcloughlin commented 4 years ago

replaced by https://github.com/saltstack-formulas/mongodb-formula/pull/83

pull-assistant[bot] commented 4 years ago
Score: 1.00

Best reviewed: commit by commit


Optimal code review plan

     Split mongod and mongos, removed mongodb meta state

Powered by Pull Assistant. Last update 410dec0 ... 410dec0. Read the comment docs.