Add timeout for juju controllers refresh#286
Open
guoqiao wants to merge 1 commit intoopenstack-charmers:masterfrom
Open
Add timeout for juju controllers refresh#286guoqiao wants to merge 1 commit intoopenstack-charmers:masterfrom
guoqiao wants to merge 1 commit intoopenstack-charmers:masterfrom
Conversation
If you have a dead controller, which is already deleted but still has a
local record in juju, it will cause a few problems without hints:
1. juju destroy-model will freeze forever
2. juju kill-model will fail with unuseful message
3. juju controllers --refresh will freeze forever
The only way to fix so far is `juju unregister`.
A related bug has been reported:
https://bugs.launchpad.net/juju-core/+bug/1593506
`functest-prepare` will also freeze forever because of 3, without any
message. To avoid wasting time on waiting, add timeout and help message
for people to find the problem earlier.
Signed-off-by: Joe Guo <guoqiao@gmail.com>
ajkavanagh
requested changes
Jun 29, 2022
Collaborator
ajkavanagh
left a comment
ajkavanagh
left a comment
There was a problem hiding this comment.
Please use async IO functions in async function definitions as otherwise the async loop will stall.
Comment on lines
+36
to
+42
|
|
||
| # if you have a dead controller, this cmd will wait forever. | ||
| # add timeout to avoid waiting. | ||
| # use `juju unregister` to remove your dead controller and try again | ||
| # related bug: https://bugs.launchpad.net/juju-core/+bug/1593506 | ||
| subprocess.check_call(['juju', 'list-controllers', '--refresh'], | ||
| timeout=60) |
Collaborator
There was a problem hiding this comment.
This is a nice change, but there is a major fault in the original code, which is that there is a non-async subprocess.check_call(...) in an async function. This will stall the async loop which basically forces it to sync. This should be changed to use the asyncio equivalent.
ajkavanagh
added
the
stale
Collaborator
|
Marking as stale as nothing has happened with the PR in over 2 years. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
If you have a dead controller, which is already deleted but still has a
local record in juju, it will cause a few problems without hints:
The only way to fix so far is
juju unregister.A related bug has been reported:
functest-preparewill also freeze forever because of 3, without anymessage. To avoid wasting time on waiting, add timeout and help message
for people to find the problem earlier.
Signed-off-by: Joe Guo guoqiao@gmail.com