tlm: ![](https://github.trello.services/images/mini-trello-icon.png) [python lib bug with is unit leader](https://trello.com/c/XGtikBMe/1008-python-lib-bug-with-is-unit-leader)
tlm: @hpidcock take a look. Haven't handled the case of being a subordinate more than once. Is this something we need to cover ?
hpidcock: $$merge$$
hpidcock: @SimonRichardson did you figure out why the merge didn't work last time?
diff --git a/juju/unit.py b/juju/unit.py
index 6cbde8d..9d8e27f 100644
--- a/juju/unit.py
+++ b/juju/unit.py
@@ -256,12 +256,13 @@ class Unit(model.ModelEntity):
         method.
 
         """
-        app = self.name.split("/")[0]
+        unit_parts = self.name.split("/")
+        app = unit_parts[0]
+        unit_index = unit_parts[1]
 
-        c = client.ClientFacade.from_connection(self.connection)
-
-        status = await c.FullStatus(patterns=None)
+        client_facade = client.ClientFacade.from_connection(self.connection)
 
+        status = await client_facade.FullStatus(patterns=None)
         # FullStatus may be more up to date than our model, and the
         # unit may have gone away, or we may be doing something silly,
         # like trying to fetch leadership for a subordinate, which
@@ -271,14 +272,30 @@ class Unit(model.ModelEntity):
         if not status.applications.get(app):
             return False
 
-        if not status.applications[app].get('units'):
-            return False
+        # We will attempt to look in two places for a leader property based on
+        # if the unit is subordinate or not. These variables allow for more
+        # generic non discriminate checks
+        app_data = status.applications.get(app)
+        target_unit = self.name
+
+        # Is the application a subordinate? If so change our data variables to
+        # the parent
+        if status.applications[app].get('subordinate-to'):
+            parent = status.applications[app]['subordinate-to'][0]
+            app_data = status.applications[parent]
+            target_unit = parent + "/" + unit_index
 
-        if not status.applications[app]['units'].get(self.name):
+        if not app_data.get('units') or not app_data['units'].get(target_unit):
             return False
 
-        return status.applications[app]['units'][self.name].get('leader',
-                                                                False)
+        unit = app_data['units'][target_unit]
+        if target_unit == self.name:
+            return unit.get('leader', False)
+
+        if unit.get('subordinates') and unit['subordinates'].get(self.name):
+            return unit['subordinates'][self.name].get('leader', False)
+
+        return False
 
     async def get_metrics(self):
         """Get metrics for the unit.

`unit.is_leader_from_status` reports False for all units of a subordinate charm
**Context**
python-libjuju v2.6.3
juju --version => 2.6.10-bionic-amd64 (snap)

This affects the migration of the functional test of openstack charms to the zaza testing framework, as some of them are deployed as subordinates (neutron-openvswitch and others in https://bugs.launchpad.net/charm-aodh/+bug/1828424)

**How to reproduce:**

Start with a deployed bundle including subordinate charms (e.g. neutron-openvswitch below). `juju status` reports a leader: neutron-openvswitch/0*

![image](https://user-images.githubusercontent.com/1916322/70766853-45770180-1db3-11ea-9fc3-6023bd0c38c2.png)

However `unit.is_leader_from_status` (https://github.com/juju/python-libjuju/blob/master/juju/unit.py#L246) reports False for all units. There is a comment in the code about returning False for subordinates on purpose. However as shown above, `juju status` does report a leader. `juju run --unit neutron-openvswitch/leader <cmd>` does work as expected as well.

Code to reproduce the issue:

```python
from juju import loop
from juju.model import Model

async def subordinate_has_no_leader():
    model = Model()
    await model.connect()

    app_name = 'neutron-openvswitch'
    app_ovs = model.applications[app_name]
    print("app: {}, units: {}".format(app_name, app_ovs.units))

    for unit in app_ovs.units:
        print("unit: {}, leader?: {}".format(unit.entity_id,
            await unit.is_leader_from_status()))

    await model.disconnect()

if __name__ == "__main__":
    loop.run(subordinate_has_no_leader())
```

which outputs the following for the model above (works fine for a principle charm):

```
$ python reproduce.py
app: neutron-openvswitch, units: [<Unit entity_id="neutron-openvswitch/1">, <Unit entity_id="neutron-openvswitch/0">]
unit: neutron-openvswitch/1, leader?: False
unit: neutron-openvswitch/0, leader?: False
```
diff --git a/tests/unit/test_unit.py b/tests/unit/test_unit.py
new file mode 100644
index 0000000..9029a01
--- /dev/null
+++ b/tests/unit/test_unit.py
@@ -0,0 +1,83 @@
+import asynctest
+import mock
+import pytest
+
+from juju.model import Model
+from juju.unit import Unit
+
+from .. import base
+
+
+@asynctest.patch('juju.client.client.ClientFacade')
+@pytest.mark.asyncio
+async def test_unit_is_leader(mock_cf):
+    tests = [
+        {
+            'applications': {
+                'test': {
+                    'units': {
+                        'test/0': {
+                            'subordinates': {
+                                'test-sub/0': {
+                                    'leader': True
+                                }
+                            }
+                        }
+                    }
+                },
+                'test-sub': {
+                    'subordinate-to': [
+                        'test'
+                    ]
+                }
+            },
+            'description': "Tests that subordinate units report is leader correctly",
+            'unit': 'test-sub/0',
+            'rval': True
+        },
+        {
+            'applications': {
+                'test': {
+                    'units': {
+                        'test/0': {
+                            'leader': True
+                        }
+                    }
+                }
+            },
+            'description': "Tests that unit reports is leader correctly",
+            'unit': 'test/0',
+            'rval': True
+        },
+        {
+            'applications': {},
+            'description': "Tests that non existent apps return False as leader",
+            'unit': 'test/0',
+            'rval': False
+        },
+        {
+            'applications': {
+                'test': {
+                    'units': {}
+                }
+            },
+            'description': "Tests that apps with no units report False as leader",
+            'unit': 'test/0',
+            'rval': False
+        },
+    ]
+
+    model = Model()
+    model._connector = mock.MagicMock()
+
+    for test in tests:
+        status = mock.Mock()
+        status.applications = test['applications']
+        client_facade = mock_cf.from_connection()
+        client_facade.FullStatus = base.AsyncMock(return_value=status)
+
+        unit = Unit("test", model)
+        unit.name = test['unit']
+
+        rval = await unit.is_leader_from_status()
+        assert rval == test['rval']

pip install -e .
parse_log_pytest
pytest
asynctest ipdb mock pytest pytest-asyncio pytest-xdist
apt-get update apt-get install -y python3-all debhelper sbuild schroot ubuntu-dev-tools snapd libsodium-dev
3.7
pytest --no-header -rA --tb=line --color=no -p no:cacheprovider -W ignore::DeprecationWarning
asynctest==0.13.0
attrs @ file:///croot/attrs_1668696182826/work
backcall==0.2.0
bcrypt==4.2.1
certifi @ file:///croot/certifi_1671487769961/work/certifi
cffi==1.15.1
charset-normalizer==3.4.1
cryptography==44.0.2
decorator==5.1.1
execnet==2.0.2
flit_core @ file:///opt/conda/conda-bld/flit-core_1644941570762/work/source/flit_core
idna==3.10
importlib-metadata @ file:///tmp/build/80754af9/importlib-metadata_1648562407465/work
iniconfig @ file:///home/linux1/recipes/ci/iniconfig_1610983019677/work
ipdb==0.13.13
ipython==7.34.0
jedi==0.19.2
-e git+https://github.com/juju/python-libjuju.git@0c7ca041f918b012f4da2038afada4cc275ddf34#egg=juju
jujubundlelib==0.5.7
macaroonbakery==1.3.4
matplotlib-inline==0.1.6
mock==5.2.0
packaging @ file:///croot/packaging_1671697413597/work
paramiko==2.12.0
parso==0.8.4
pexpect==4.9.0
pickleshare==0.7.5
pluggy @ file:///tmp/build/80754af9/pluggy_1648042572264/work
prompt_toolkit==3.0.48
protobuf==4.24.4
ptyprocess==0.7.0
py @ file:///opt/conda/conda-bld/py_1644396412707/work
pyasn1==0.5.1
pycparser==2.21
Pygments==2.17.2
pymacaroons==0.13.0
PyNaCl==1.5.0
pyRFC3339==1.1
pytest==7.1.2
pytest-asyncio==0.21.2
pytest-xdist==3.5.0
pytz==2025.2
PyYAML==6.0
requests==2.31.0
six==1.17.0
theblues==0.5.2
tomli @ file:///opt/conda/conda-bld/tomli_1657175507142/work
toposort==1.10
traitlets==5.9.0
typing_extensions @ file:///croot/typing_extensions_1669924550328/work
urllib3==2.0.7
wcwidth==0.2.13
websockets==7.0
zipp @ file:///croot/zipp_1672387121353/work

name: python-libjuju
channels:
  - defaults
  - https://repo.anaconda.com/pkgs/main
  - https://repo.anaconda.com/pkgs/r
  - conda-forge
dependencies:
  - _libgcc_mutex=0.1=main
  - _openmp_mutex=5.1=1_gnu
  - attrs=22.1.0=py37h06a4308_0
  - ca-certificates=2025.2.25=h06a4308_0
  - certifi=2022.12.7=py37h06a4308_0
  - flit-core=3.6.0=pyhd3eb1b0_0
  - importlib-metadata=4.11.3=py37h06a4308_0
  - importlib_metadata=4.11.3=hd3eb1b0_0
  - iniconfig=1.1.1=pyhd3eb1b0_0
  - ld_impl_linux-64=2.40=h12ee557_0
  - libffi=3.4.4=h6a678d5_1
  - libgcc-ng=11.2.0=h1234567_1
  - libgomp=11.2.0=h1234567_1
  - libstdcxx-ng=11.2.0=h1234567_1
  - ncurses=6.4=h6a678d5_0
  - openssl=1.1.1w=h7f8727e_0
  - packaging=22.0=py37h06a4308_0
  - pip=22.3.1=py37h06a4308_0
  - pluggy=1.0.0=py37h06a4308_1
  - py=1.11.0=pyhd3eb1b0_0
  - pytest=7.1.2=py37h06a4308_0
  - python=3.7.16=h7a1cb2a_0
  - readline=8.2=h5eee18b_0
  - setuptools=65.6.3=py37h06a4308_0
  - sqlite=3.45.3=h5eee18b_0
  - tk=8.6.14=h39e8969_0
  - tomli=2.0.1=py37h06a4308_0
  - typing_extensions=4.4.0=py37h06a4308_0
  - wheel=0.38.4=py37h06a4308_0
  - xz=5.6.4=h5eee18b_1
  - zipp=3.11.0=py37h06a4308_0
  - zlib=1.2.13=h5eee18b_1
  - pip:
      - asynctest==0.13.0
      - backcall==0.2.0
      - bcrypt==4.2.1
      - cffi==1.15.1
      - charset-normalizer==3.4.1
      - cryptography==44.0.2
      - decorator==5.1.1
      - execnet==2.0.2
      - idna==3.10
      - ipdb==0.13.13
      - ipython==7.34.0
      - jedi==0.19.2
      - jujubundlelib==0.5.7
      - macaroonbakery==1.3.4
      - matplotlib-inline==0.1.6
      - mock==5.2.0
      - paramiko==2.12.0
      - parso==0.8.4
      - pexpect==4.9.0
      - pickleshare==0.7.5
      - prompt-toolkit==3.0.48
      - protobuf==4.24.4
      - ptyprocess==0.7.0
      - pyasn1==0.5.1
      - pycparser==2.21
      - pygments==2.17.2
      - pymacaroons==0.13.0
      - pynacl==1.5.0
      - pyrfc3339==1.1
      - pytest-asyncio==0.21.2
      - pytest-xdist==3.5.0
      - pytz==2025.2
      - pyyaml==6.0
      - requests==2.31.0
      - six==1.17.0
      - theblues==0.5.2
      - toposort==1.10
      - traitlets==5.9.0
      - urllib3==2.0.7
      - wcwidth==0.2.13
      - websockets==7.0
prefix: /opt/conda/envs/python-libjuju

tests/unit/test_unit.py::test_unit_is_leader
juju/unit.py
juju/unit.py