cmd/juju/user: implement controller login #6964

Merged
merged 1 commit into from Feb 16, 2017

Conversation

Projects
None yet
4 participants
Owner

rogpeppe commented Feb 10, 2017

This is the first stage of a process that will eventually
deprecate juju register in favour of juju login.

Much of the new code in juju login is copied from
juju register.

To QA, try "juju login --host some-public-controller-address",
which should do the same thing as "juju register some-public-controller-address".
You could also try changing the table in getKnownControllerDomains
and doing "juju login some-alias".

@rogpeppe rogpeppe changed the base branch from develop to user-domain Feb 10, 2017

LGTM, with a +1 from a core developer.

cmd/juju/user/login.go
@@ -118,6 +177,7 @@ func (c *loginCommand) Run(ctx *cmd.Context) error {
// CodeNoCreds was returned, which means that external
// users are not supported. Fall back to prompting the
// user for their username and password.
+ } else {

Thank you for the level of comments of this code, I find it beautifully commented in a very useful way.

cmd/juju/user/login.go
+--host flag is specified the user is logged into
+the controller associated with the given domain and the controller is registered
+using the domain name (the -c flag can be used to choose a different controller
+name). It a different controller is already registered with the same name,
cmd/juju/user/login.go
+func (c *loginCommand) controllerLogin(ctx *cmd.Context, store jujuclient.ClientStore) error {
+ knownDomains, err := c.getKnownControllerDomains()
+ if err != nil {
+ // TODO perhaps this shouldn't be fatal.
@perrito666

perrito666 Feb 14, 2017

Contributor

own the TODO so we know who to ask when working on this please.

Owner

rogpeppe commented Feb 15, 2017

$$merge$$

Contributor

jujubot commented Feb 15, 2017

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

Contributor

jujubot commented Feb 15, 2017

Build failed: Generating tarball failed
build url: http://juju-ci.vapour.ws:8080/job/github-merge-juju/10301

Owner

rogpeppe commented Feb 15, 2017

$$merge$$

Contributor

jujubot commented Feb 15, 2017

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

Contributor

jujubot commented Feb 15, 2017

Build failed: Tests failed
build url: http://juju-ci.vapour.ws:8080/job/github-merge-juju/10302

cmd/juju/user: implement controller login
This is the first stage of a process that will eventually
deprecate juju register in favour of juju login.

Much of the new code in juju login is copied from
juju register.
Owner

rogpeppe commented Feb 15, 2017

$$merge$$

Contributor

jujubot commented Feb 15, 2017

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

Contributor

jujubot commented Feb 15, 2017

Build failed: Tests failed
build url: http://juju-ci.vapour.ws:8080/job/github-merge-juju/10304

Owner

rogpeppe commented Feb 16, 2017

$$merge$$

Contributor

jujubot commented Feb 16, 2017

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

@jujubot jujubot merged commit 2042d3b into juju:user-domain Feb 16, 2017

1 check failed

github-check-merge-juju Built PR, ran unit tests, and tested LXD deploy. Use !!.*!! to request another build. IE, !!build!!, !!retry!!
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment