Skip to content
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

Conversation

ajkavanagh
Copy link
Collaborator

@ajkavanagh ajkavanagh commented Jan 13, 2022

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.

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
Copy link

codecov bot commented Jan 13, 2022

Codecov Report

Merging #474 (41b736a) into master (e15cc85) will increase coverage by 1.62%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            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     
Impacted Files Coverage Δ
zaza/model.py 85.77% <100.00%> (+0.38%) ⬆️
zaza/utilities/generic.py 82.74% <100.00%> (+5.49%) ⬆️
zaza/utilities/juju.py 92.20% <100.00%> (ø)
zaza/charm_tests/conncheck/__init__.py
zaza/charm_tests/noop/tests.py
zaza/charm_tests/noop/__init__.py
zaza/charm_tests/conncheck/tests.py
zaza/charm_tests/noop/setup.py

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e15cc85...41b736a. Read the comment docs.

These are tests and thus shouldn't be analysed by code coverage tools.
@lourot lourot merged commit 81416cb into openstack-charmers:master Jan 14, 2022
@ajkavanagh ajkavanagh deleted the switch-public-address-to-get-public-address branch January 14, 2022 15:47
@SPFZ
Copy link

SPFZ commented Jan 19, 2022

Link 1 in first comment does not work for me. I believe it was meant to link to #472

@ajkavanagh
Copy link
Collaborator Author

Link 1 in first comment does not work for me. I believe it was meant to link to #472

Updated. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants