Load MongoDB creds from agent.conf if no username is passed #14
Conversation
babbageclunk
force-pushed
the
get-creds-from-agent-conf
branch
from
f0464f7
to
47da335
Compare
cmd/restore.go
Outdated
| if c.username == "" { | ||
| username, password, err = c.loadCreds() | ||
| if err != nil { | ||
| return errors.Annotate(err, "reading credentials from agent.conf") |
There was a problem hiding this comment.
if the underyling error is
"couldn't find an agent.conf - please specify username and password"
this annotation will read a bit icky.
Maybe make all errors from ReadCredsFromAgentConf well worded and avoid the annotation
There was a problem hiding this comment.
How about I change the annotation to "loading credentials"? Then the annotated error would be "loading credentials: couldn't find an agent.conf - please specify username and password", which reads ok.
| if len(matches) == 0 { | ||
| return "", "", errors.Errorf("couldn't find an agent.conf - please specify username and password") | ||
| } | ||
| conf := matches[0] |
There was a problem hiding this comment.
Yeah, that was my thinking - we'd only expect to find one but it might be machine 0,1,2...
| @@ -8,11 +8,10 @@ PRIMARY or SECONDARY state. | |||
| The expected usage is to copy the juju-restore binary and the backup | |||
| file to the primary controller machine and then run it: | |||
|
|
|||
| ./juju-restore --username machine-n --password mongo-password /path/to/backup/file | |||
| ./juju-restore /path/to/backup/file | |||
| from the machine agent's config file: | ||
| `/var/lib/juju/agents/machine-<n>/agent.conf` | ||
| Username and password will be collected automatically from the machine | ||
| agent's config file: `/var/lib/juju/agents/machine-<n>/agent.conf` |
There was a problem hiding this comment.
Hmm... worth remembering that when we split the controller out, we'll need to support getting the password from there instead. However since that isn't yet fully defined, we can't even try to do things in preparation.
| // XXXXXXXXXXXXX Remove ME | ||
| // To be used as an option during development to enable an easier | ||
| // way to re-start all agents in HA federation. XXXXXXXXXXXXX | ||
| // Remove ME | ||
| restart bool |
There was a problem hiding this comment.
What about just putting that arg behind a feature flag?
There was a problem hiding this comment.
Yeah, good idea - I'll do that.
babbageclunk
force-pushed
the
get-creds-from-agent-conf
branch
from
47da335
to
5b626ad
Compare
It's only really useful while developing juju-restore - to enable it set JUJU_RESTORE_DEV_MODE=on in the environment.
|
|
Description of change
Rather than requiring the user to look up the username and password from the agent.conf we grab them automatically if possible. Still allow specifying them manually if we need to override that for some reason.
QA steps
Restore a backup without setting
--username- the right credentials will be used from the agent.conf.