-
Notifications
You must be signed in to change notification settings - Fork 491
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
2.6 fix intermittent login failure #10463
2.6 fix intermittent login failure #10463
Conversation
…indicate initialization.
Address already in use !!build!! |
@@ -29,8 +29,14 @@ type BackingWatcher interface { | |||
Stop() error | |||
} | |||
|
|||
// Unlocker is used to indicate that the model cache is ready to be used. | |||
type Unlocker interface { | |||
Unlock() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can Unlock()
fail?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No.
// Config describes the necessary fields for NewWorker. | ||
type Config struct { | ||
InitializedGate Unlocker |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will tend to stutter as variables are called gate
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually none of the variables are called gate
. But initialized
or unlocker
. The later of which is a bit odd IMO.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the layering is fine. modelcache just takes an Unlocker which it will call when it is ready, and other things can depend on the gate.
juju/testing/conn.go
Outdated
// want to do things with those models where the actions may touch | ||
// the model cache. | ||
func (s *JujuConnSuite) EnsureCachedModel(c *gc.C, uuid string) { | ||
start := time.Now() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this clearer as a
stop := time.After(testing.LongWait)
retry := time.After(0)
for {
select {
case <-retry:
_, err := s.Controller.Model(uuid)
if err == nil {
return
}
timeout = time.After(testing.ShortWait)
case <-stop:
c.Errorf("model %v not seen in cache after %v", uuid, testing.LongWait)
return
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I think something with a select is better.
|
This branch fixes two different intermittent failures due to the model not being found in the model cache.
This happens when the apiserver code is creating the facade context.
The fix is to ensure that the model exists in the cache before continuing with the test. A helper method has been added to the JujuConnSuite as this is the one that triggers it most.
Additionally the apiserver is now not created in the engine until the model cache has been initialized. This will help avoid login issues to the apiserver if there are many models.
QA steps
Documentation changes
No user facing impact.