-
Notifications
You must be signed in to change notification settings - Fork 47
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
Switch public address to get public address and add fallback for unit.get_public_address() #474
Merged
lourot
merged 7 commits into
openstack-charmers:master
from
ajkavanagh:switch-public-address-to-get-public-address
Jan 14, 2022
Merged
Switch public address to get public address and add fallback for unit.get_public_address() #474
lourot
merged 7 commits into
openstack-charmers:master
from
ajkavanagh:switch-public-address-to-get-public-address
Jan 14, 2022
Conversation
This file contains 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
Due to the bug [1] on OpenStack providers, unit.public_address doesn't actually work reliably. The fix [2] is only for the async function unit.get_public_address(). Sadly, zaza relied on unit.public_address and so it needs this patch for juju 2.9 support on OpenStack providers. [1]: juju/python-libjuju#551 [2]: juju/python-libjuju#600
Bug [1] indicated a fault in the logic for get_app_ips() where the code went sync -> async -> sync -> async and thus ended up trying to reuse the same event loop.
* Added a regression test for the get_app_ips() function for associated error [1]. * Added third.yaml bundle with just ubuntu apps for public IP tests * Add pins for py35 support (oslo.config and kubernetes) [1]: openstack-charmers#470
Due to bug [1], it seems that there is a issue with libjuju communicating with the Juju controller which causes a permanent model disconnect that libjuju doesn't resolve. Thus, for zaza, this patch wraps unit.get_public.address() with a wrapper that can choose, based on the environment variable `ZAZA_FEATURE_BUG472` to use a subprocess shell call to the `juju status` command to get the public address. This should always succeed. The feature environment variable `ZAZA_FEATURE_BUG472` was added so that the library can switch between the native libjuju function and the fallback wrapper to enable testing of the issue as libjuju continues to evolve. By default, the wrapper function is used, to enable zaza to interoperate with libjuju with Juju 2.9 on OpenStack providers. The implementation is slightly complicated because an async version of the wrapper `get_unit_public_address()` is needed as it is called from async code. [1]: juju/python-libjuju#615
Codecov Report
@@ Coverage Diff @@
## master #474 +/- ##
==========================================
+ Coverage 87.11% 88.73% +1.62%
==========================================
Files 49 44 -5
Lines 4362 4324 -38
==========================================
+ Hits 3800 3837 +37
+ Misses 562 487 -75
Continue to review full report at Codecov.
|
These are tests and thus shouldn't be analysed by code coverage tools.
lourot
approved these changes
Jan 14, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Due to bug 1, it seems that there is a issue with libjuju
communicating with the Juju controller which causes a permanent
model disconnect that libjuju doesn't resolve. Thus, for zaza, this
patch wraps unit.get_public.address() with a wrapper that can choose,
based on the environment variable
ZAZA_FEATURE_BUG472
to use asubprocess shell call to the
juju status
command to get the publicaddress. This should always succeed.
The feature environment variable
ZAZA_FEATURE_BUG472
was added so thatthe library can switch between the native libjuju function and the
fallback wrapper to enable testing of the issue as libjuju continues to
evolve.
By default, the wrapper function is used, to enable zaza to interoperate
with libjuju with Juju 2.9 on OpenStack providers.
The implementation is slightly complicated because an async version of
the wrapper
get_unit_public_address()
is needed as it is called fromasync code.