Fixes LP#1514874: login failures with valid credentials. #5506

Merged
merged 4 commits into from Jun 1, 2016

Conversation

Projects
None yet
3 participants
Member

anastasiamac commented Jun 1, 2016

When we create agent configuration for juju entities, we do not specify password since we want to generate a new one. However, we do have access to the old password sent over wire.

Prior to this PR, we would try to login 3 times:

  1. with no password and, as expected, we would fail with authentication issues;
  2. with old password and will connect;
  3. with newly generated/updated password.

This PR ensures that we do not try to login with no password \o/

I am still running tests suites but am proposing the actual code change to get early feedback.

(Review request: http://reviews.vapour.ws/r/4951/)

worker/apicaller/open.go
+ // we can connect with old password.
+ info.Password = agentConfig.OldPassword()
+ }
+
@axw

axw Jun 1, 2016

Member

this feels a bit odd, why not just check for info.Password == "" in openAPIStateUsingInfo?

@anastasiamac

anastasiamac Jun 1, 2016

Member

Good idea. I'll move the logic.

worker/apicaller/open.go
if err != nil {
return nil, err
}
- // Reconnect to the API with the new password.
+ // Before we reconnect to the API with the new password,
@axw

axw Jun 1, 2016

Member

stating the obvious, please drop

Member

anastasiamac commented Jun 1, 2016

$$fixes-1514874$$

Contributor

jujubot commented Jun 1, 2016

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

@jujubot jujubot merged commit 77ec347 into juju:1.25 Jun 1, 2016

@anastasiamac anastasiamac deleted the anastasiamac:too-many-logins branch Jun 1, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment