Remove "dying" units from planned units (#936)

This PR addresses an issue that happens during a scale-down event,
where planned_units includes units with the status marked as dying.

This becomes visible when say the _storage_detaching hook errors based
on certain preconditions preventing a unit from immediately disappearing.

This PR essentially filters out those dying units to have an accurate count
of live units.

ops/model.py

@@ -2931,16 +2931,21 @@ def get_pebble(self, socket_path: str) -> 'Client':
     def planned_units(self) -> int:
         """Count of "planned" units that will run this application.
 
-        Includes the current unit in the count.
+        This will include the current unit, any units that are alive, units that are in the process
+        of being started, but will not include units that are being shut down.
 
         """
         # The goal-state tool will return the information that we need. Goal state as a general
         # concept is being deprecated, however, in favor of approaches such as the one that we use
         # here.
         app_state = self._run('goal-state', return_output=True, use_json=True)
-        app_state = typing.cast(Dict[str, List[str]], app_state)
+        app_state = typing.cast(Dict[str, Dict[str, Any]], app_state)
+
         # Planned units can be zero. We don't need to do error checking here.
-        return len(app_state.get('units', []))
+        # But we need to filter out dying units as they may be reported before being deleted
+        units = app_state.get('units', {})
+        num_alive = sum(1 for unit in units.values() if unit['status'] != 'dying')
+        return num_alive
 
     def update_relation_data(self, relation_id: int, _entity: 'UnitOrApplication',
                              key: str, value: str):

test/test_model.py

@@ -2712,6 +2712,35 @@ def test_relation_remote_app_name_script_errors(self):
             ['relation-list', '-r', '6', '--app', '--format=json'],
         ])
 
+    def test_planned_units(self):
+        # no units
+        fake_script(self, 'goal-state', """
+echo '{"units":{}, "relations":{}}'
+""")
+        self.assertEqual(self.backend.planned_units(), 0)
+
+        # only active units
+        fake_script(self, 'goal-state', """
+echo '{
+    "units":{
+        "app/0": {"status":"active","since":"2023-05-23 17:05:05Z"},
+        "app/1": {"status":"active","since":"2023-05-23 17:57:05Z"}
+    },
+    "relations": {}
+}'""")
+        self.assertEqual(self.backend.planned_units(), 2)
+
+        # active and dying units
+        fake_script(self, 'goal-state', """
+echo '{
+    "units":{
+        "app/0": {"status":"active","since":"2023-05-23 17:05:05Z"},
+        "app/1": {"status":"dying","since":"2023-05-23 17:57:05Z"}
+    },
+    "relations": {}
+}'""")
+        self.assertEqual(self.backend.planned_units(), 1)
+
 
 class TestLazyMapping(unittest.TestCase):