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

[JUJU-320] Unit public address #600

Merged
merged 1 commit into from
Dec 9, 2021

Conversation

SimonRichardson
Copy link
Member

In order to get the correct public address from the unit, you have to do
a query to the application facade to attempt to retrieve it. In order to
support backward compatibility with older instances of Juju that didn't
have the new facade method, we fall back to the previous implementation.

Fixes: #551
Bug: https://bugs.launchpad.net/juju/+bug/1950799

In order to get the correct public address from the unit, you have to do
a query to the application facade to attempt to retrieve it. In order to
support backwards compatibility with older instances of Juju that didn't
have the new facade method, we fall back to the previous implementation.
@SimonRichardson SimonRichardson changed the title Unit public address [JUJU-320] Unit public address Dec 8, 2021
Copy link
Member

@manadart manadart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As we discussed, this is the right way.

@SimonRichardson
Copy link
Member Author

$$merge$$

@jujubot jujubot merged commit dd382ec into juju:master Dec 9, 2021
@wolsen
Copy link
Contributor

wolsen commented Dec 21, 2021

FWIW, I tested this today with the recreate scenario in #551 (from this comment) and I can still see the problem. I deployed 10 units of ch:ubuntu (series focal) on juju version 2.9.22 and all looked fine. I then removed those units and added 10 more units of ch:ubuntu (series jammy) on version 2.9.22 and latest python-libjuju from tip and that reproduced the problem:

$ juju status
Model  Controller   Cloud/Region             Version  SLA          Timestamp
jammy  serverstack  serverstack/serverstack  2.9.22   unsupported  21:48:54Z

App     Version  Status   Scale  Charm   Store     Channel  Rev  OS      Message
ubuntu           unknown     10  ubuntu  charmhub  stable    10  ubuntu  

Unit        Workload  Agent  Machine  Public address  Ports  Message
ubuntu/10*  unknown   idle   10       10.5.3.141             
ubuntu/11   unknown   idle   11       10.5.2.148             
ubuntu/12   unknown   idle   12       10.5.3.9               
ubuntu/13   unknown   idle   13       10.5.0.46              
ubuntu/14   unknown   idle   14       10.5.3.228             
ubuntu/15   unknown   idle   15       10.5.2.175             
ubuntu/16   unknown   idle   16       10.5.0.61              
ubuntu/17   unknown   idle   17       10.5.3.131             
ubuntu/18   unknown   idle   18       10.5.2.86              
ubuntu/19   unknown   idle   19       10.5.1.131             

Machine  State    DNS         Inst id                               Series  AZ    Message
10       started  10.5.3.141  5f0617d0-1c86-4abb-822f-01648e4a674e  jammy   nova  ACTIVE
11       started  10.5.2.148  3a9d8f7f-461e-4a6e-b9ce-ca2420122990  jammy   nova  ACTIVE
12       started  10.5.3.9    1ac4c66c-0118-4979-b16c-a825a63b319e  jammy   nova  ACTIVE
13       started  10.5.0.46   552528bf-9e18-4ec9-b910-81e6ff0f0c36  jammy   nova  ACTIVE
14       started  10.5.3.228  67274d25-19ac-40d8-85a7-623f95b6c5f1  jammy   nova  ACTIVE
15       started  10.5.2.175  cc9752b1-700d-4595-822f-8c3187d4b0a7  jammy   nova  ACTIVE
16       started  10.5.0.61   c9571b3d-3be5-4ceb-ad69-b925bf61d5bc  jammy   nova  ACTIVE
17       started  10.5.3.131  2fde9628-25a8-4f1c-af66-121c815d9bfb  jammy   nova  ACTIVE
18       started  10.5.2.86   680722f3-d933-4c32-bbef-d648a1d7587b  jammy   nova  ACTIVE
19       started  10.5.1.131  e6bfbd1b-155a-466a-a549-bc9b730f9844  jammy   nova  ACTIVE

Which produced the following output reliably:

$ python3 libjuju-test.py 
ubuntu/10 Address: 10.5.3.141
ubuntu/12 Address: 10.5.3.9
ubuntu/13 Address: None
ubuntu/16 Address: None
ubuntu/15 Address: None
ubuntu/18 Address: None
ubuntu/19 Address: None
ubuntu/11 Address: None
ubuntu/17 Address: 10.5.3.131
ubuntu/14 Address: 10.5.3.228

wolsen added a commit to wolsen/python-libjuju that referenced this pull request Dec 21, 2021
PR juju#600 updated the get_public_address method for units to look up the
public address in the ApplicationFacade's UnitsInfo object. However,
the change used the get(str) method of the Type class which performs a
lookup in the json dictionary using the hyphenated key rather than the
underscore key, thus always returns None since this key is not in the
JSON dictionary. Changing it to return the lookup of 'public-address'
instead of 'public_address' results in the unit's public address being
returned.

Fixes juju#550

Signed-off-by: Billy Olsen <[email protected]>
@wolsen
Copy link
Contributor

wolsen commented Dec 21, 2021

The change in PR #610 reliably works for me

wolsen added a commit to wolsen/python-libjuju that referenced this pull request Dec 21, 2021
PR juju#600 updated the get_public_address method for units to look up the
public address in the ApplicationFacade's UnitsInfo object. However,
the change used the get(str) method of the Type class which performs a
lookup in the json dictionary using the hyphenated key rather than the
underscore key, thus always returns None since this key is not in the
JSON dictionary. Changing it to return the lookup of 'public-address'
instead of 'public_address' results in the unit's public address being
returned.

Fixes juju#551

Signed-off-by: Billy Olsen <[email protected]>
jujubot added a commit that referenced this pull request Jan 4, 2022
#610

PR #600 updated the get_public_address method for units to look up the
public address in the ApplicationFacade's UnitsInfo object. However,
the change used the get(str) method of the Type class which performs a
lookup in the json dictionary using the hyphenated key rather than the
underscore key, thus always returns None since this key is not in the
JSON dictionary. Changing it to return the lookup of 'public-address'
instead of 'public_address' results in the unit's public address being
returned.

Fixes #551 

Signed-off-by: Billy Olsen <[email protected]>
ajkavanagh added a commit to ajkavanagh/zaza that referenced this pull request Jan 7, 2022
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
ajkavanagh added a commit to ajkavanagh/zaza-openstack-tests that referenced this pull request Jan 7, 2022
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.

This patch relies on an associated patch in zaza [3]; thus this will
fails its tests until that passes.

[1]: juju/python-libjuju#551
[2]: juju/python-libjuju#600
[3]: openstack-charmers/zaza#468
ChrisMacNaughton pushed a commit to openstack-charmers/zaza that referenced this pull request Jan 10, 2022
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
ajkavanagh added a commit to ajkavanagh/zaza that referenced this pull request Jan 13, 2022
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
ajkavanagh added a commit to ajkavanagh/zaza-openstack-tests that referenced this pull request Jan 14, 2022
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.

This patch relies on an associated patch in zaza [3]; thus this will
fails its tests until that passes.

[1]: juju/python-libjuju#551
[2]: juju/python-libjuju#600
[3]: openstack-charmers/zaza#468
ajkavanagh added a commit to ajkavanagh/zaza that referenced this pull request Jan 19, 2022
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
ajkavanagh added a commit to ajkavanagh/zaza-openstack-tests that referenced this pull request Jan 19, 2022
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.

This patch relies on an associated patch in zaza [3]; thus this will
fails its tests until that passes.

[1]: juju/python-libjuju#551
[2]: juju/python-libjuju#600
[3]: openstack-charmers/zaza#468
jujubot added a commit that referenced this pull request Jan 31, 2022
#619

## What's Changed
* [JUJU-320] Unit public address by @SimonRichardson in #600
* [JUJU-244] Add attach-resource by @cderici in #601
* [JUJU-140] Model.wait_for_idle -- for apps with no units yet by @cderici in #575
* [JUJU-367] Improve `get_charm_series` to check the model for series for a local charm by @cderici in #607
* [JUJU-366] Utility for connecting directly to existing connection by @cderici in #605
* Use public-address key instead of public_address by @wolsen in #610
* [JUJU-376] `wait_for_idle` to support scale down by @cderici in #613
* [JUJU-378] Utility for block_until-ing with a custom coroutine by @cderici in #614
* Fallback to 'local-fan' by @dparv in #612
* Minor comments on docs for block_until related functions. by @juanmanuel-tirado in #617
* Additional checks in print status. by @juanmanuel-tirado in #622

## New Contributors
* @wolsen made their first contribution in #610
* @dparv made their first contribution in #612


[JUJU-320]: https://warthogs.atlassian.net/browse/JUJU-320?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
[JUJU-244]: https://warthogs.atlassian.net/browse/JUJU-244?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
[JUJU-140]: https://warthogs.atlassian.net/browse/JUJU-140?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
[JUJU-367]: https://warthogs.atlassian.net/browse/JUJU-367?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
[JUJU-366]: https://warthogs.atlassian.net/browse/JUJU-366?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
[JUJU-376]: https://warthogs.atlassian.net/browse/JUJU-376?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
[JUJU-378]: https://warthogs.atlassian.net/browse/JUJU-378?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
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.

Getting a unit's public_address is unreliable in Juju 2.9
4 participants