search

voxpupuli / puppet-mongodb

mongodb installation
Apache License 2.0
93 stars 451 forks source link

Fix replset and sharding integration tests #743

Closed h-haaks closed 7 months ago

h-haaks commented 7 months ago

Fixing integration test that haven't been run for a very long time. To be able to run integration test that require two hosts with gha I have created a custom ci workflow and set it to unmanaged in modulesync.

h-haaks commented 7 months ago

Finally got this working locally! I added some tests to verify replset setup as part of mongodb::server as well.

Still don't know if it's possible to run these tests in the gha test matrix ...

h-haaks commented 7 months ago

@stevenpost Could you have a look at this?

h-haaks commented 7 months ago

Finally got all integration tests to run. There seems to be some instability in the replset tests with mongodb 7.0 and auth enabled. Rerunning the failing tests a few times fixes it.

stevenpost commented 7 months ago

Finally got all integration tests to run. There seems to be some instability in the replset tests with mongodb 7.0 and auth enabled. Rerunning the failing tests a few times fixes it.

I noticed the CentOS 8 tests failing a lot due to timeouts, nothing to do with the mongodb puppet module, but with the servers. I had a colleague trigger those again to make the pipeline pass.

bastelfreak commented 7 months ago

We pull the centos 8 image from quay.io and that seems to trigger random timeouts :(

h-haaks commented 7 months ago

@stevenpost, I'm not talking about the quay.io issues. I'm aware of that :) If you look at https://github.com/voxpupuli/puppet-mongodb/actions/runs/8719641032/job/23921963409?pr=743 it has 3 re-runs. See each of the runs ( dropdown button to the right ) and you'll see what I'm talking about.

stevenpost commented 7 months ago

@stevenpost, I'm not talking about the quay.io issues. I'm aware of that :) If you look at https://github.com/voxpupuli/puppet-mongodb/actions/runs/8719641032/job/23921963409?pr=743 it has 3 re-runs. See each of the runs ( dropdown button to the right ) and you'll see what I'm talking about.

I see what you mean, and it isn't limited to MongoDB 7.0. The members of the set are defined at line 214 (https://github.com/voxpupuli/puppet-mongodb/blob/76858968bab1a981639e8925a15346992fafcd12/spec/acceptance/replset_spec.rb#L214C11-L214C70), Do you know what the values are for x? They should map to the facts networking.fqdn, otherwise you get the error:

Notice: /Stage[main]/Main/Mongodb_replset[test]/ensure: created
Warning: Host rocky9-64-2-puppet7.example.com:27017 is available, but you are unauthorized because of authentication is enabled: true
Error: /Stage[main]/Main/Mongodb_replset[test]: Could not evaluate: rs.initiate() failed for replicaset test

Followed by 

Warning: User info is available only from master host

Even though at this point, both nodes are standalone. You are then hitting this if statement in the provider: https://github.com/voxpupuli/puppet-mongodb/blob/76858968bab1a981639e8925a15346992fafcd12/lib/puppet/provider/mongodb_replset/mongo.rb#L179 going into the else branch, which won't work with authentication enabled.

h-haaks commented 7 months ago

My last commit changed the outcome of the integration tests quite a bit. Now there is no job failing on rs.initiate() any more.

The problem here seems to be that when rs.initiate() is invoked with more than one member its up to mongodb election to decide which server should be the primary(master). The test code assumes that the host with master role ( the first host ) allways is elected as primary, which is not always the case.

When the second host is elected primary, the idempotency check on host 1 now fails auth to host 2 ...

h-haaks commented 7 months ago

Personally I'm not sure I like the overall design of the module when it comes to setting up replication. I don't think I have seen puppet implementations that have to cross connect between multiple servers to get its job done ...

If I was to reimplement this I think I would somehow use exported resources for replicaset members and let puppet on the currently elected primary realize the exported the hosts trough the localhost connection.

stevenpost commented 7 months ago

Personally I'm not sure I like the overall design of the module when it comes to setting up replication. I don't think I have seen puppet implementations that have to cross connect between multiple servers to get its job done ...

If I was to reimplement this I think I would somehow use exported resources for replicaset members and let puppet on the currently elected primary realize the exported the hosts trough the localhost connection.

I think most modules should strive to be independent of PuppetDB, not everyone has that luxury. That being said, I agree that the current approach is suboptimal.

When the second host is elected primary, the idempotency check on host 1 now fails auth to host 2 ...

Then the check is wrong? I haven't looked at this PR it in detail yet, but some things come to mind:

h-haaks commented 7 months ago

Finally I got this stabilized :) To summarize: 7f36dacfc80483da9911b1eb0834efe3039ddb1e is needed to make sure the fist host is always elected as primary ( master ) 24bb055dc8a44e98590fb141e4232e0f81ab1306 is needed to prevent misleading errors saying rs.initiate() failed for replicaset #{name} when initiation actually is successful but another host is elected as primary.

I still see lots of issues with how these replset parts are implemented in the module. I'll try to put this up for discussion in a separate issue.

h-haaks commented 7 months ago

Regarding f9957b3bf23079813ca32db0c32c5ff4fd0ba38c, I really want to wait for https://github.com/voxpupuli/puppet_metadata/pull/124 and https://github.com/voxpupuli/gha-puppet/pull/53 so that I can remove it before this is merged.