catch errors based on missing authentication to allow creation of admin user on replicaset setup #479
Conversation
…in user on replicaset setup
lib/puppet/provider/mongodb.rb
Outdated
| if mongorc_file | ||
| res = mongo_cmd(db, conn_string, mongorc_file + cmd_ismaster).to_s.chomp | ||
| end | ||
| if res.match(/Authentication failed/) or not mongorc_file |
There was a problem hiding this comment.
Perhaps you should switch these around because res will not be defined if mongorc_file is false.
| new(name: db['name'], | ||
| ensure: :present) | ||
| end | ||
| rescue |
There was a problem hiding this comment.
Should we log anything about why it failed? Otherwise debugging might be very hard.
| ensure: :present) | ||
| end | ||
| rescue | ||
| {} |
|
I adapted the code as requested and fixed some issue. Do you need anything else to merge? |
lib/puppet/provider/mongodb.rb
Outdated
| cmd_ismaster | ||
| end | ||
| res = mongo_cmd(db, conn_string, full_command).to_s.chomp | ||
| if res =~ %r{Authentication failed} |
There was a problem hiding this comment.
You retry the same command twice if there's an authentication failure (previous iteration didn't have this). You probably want to change this to if !mongorc_file && res =~ %r{Authentication failed} to get the same result.
|
Should work now |
lib/puppet/provider/mongodb.rb
Outdated
| res = mongo_cmd(db, conn_string, full_command).to_s.chomp | ||
|
|
||
| # Retry command without authentication when mongorc_file is set and authentication failed | ||
| if mongorc_file && res =~ %r{Authentication failed} |
There was a problem hiding this comment.
Rubocop doesn't like the double space here
|
The issue happens when setting up a new replicaset with authentication enabled. The self.instances get an authentication error, because neither the admin database nor the administrative user have been created, yet. So the whole catalog fails. |
| @@ -6,11 +6,16 @@ | |||
|
|
|||
| def self.instances | |||
| require 'json' | |||
| dbs = JSON.parse mongo_eval('printjson(db.getMongo().getDBs())') | |||
| begin | |||
There was a problem hiding this comment.
No need for this indenting. You can write
def self.instances
# code
rescue
# code
end
|
All fixes were to ensure a puppet run on a bare system to setup a new replicaset. |
|
I'm working on a big update that enables spec tests again (#491). Currently I'm at the point that I've verified the problem but after a full day my brain is done with it. Tomorrow I'm going to take a stab at integrating this into it. |
|
Could you rebase this on the current master? I put in a lot of effort to at least get acceptance tests working on CentOS7 (mongodb 2.6). With this fix I think we should also be able to get tests working on Debian and Ubuntu (mongodb 3.x). |
|
Dear @Henning-B, thanks for the PR! This is pccibot, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase? You can find my sourcecode at voxpupuli/vox-pupuli-tasks |
1 similar comment
|
Dear @Henning-B, thanks for the PR! This is pccibot, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase? You can find my sourcecode at voxpupuli/vox-pupuli-tasks |
|
Dear @Henning-B, thanks for the PR! This is pccibot, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase? You can find my sourcecode at voxpupuli/vox-pupuli-tasks |
|
Dear @Henning-B, thanks for the PR! This is pccibot, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase? You can find my sourcecode at voxpupuli/vox-pupuli-tasks |
|
This should be handled by #703... We can close this. |
Pull Request (PR) description
When setting up a new replica set with authentication enabled the puppet run fails because some exceptions were not caught properly, when getting the user and database instances. When checking if the mongodb is a primary node a recheck without authentication is done to be able to create the admin user.
This Pull Request (PR) fixes the following issues