Tear down deltaproxy sub-proxies on shutdown (#70221)

changelog/70221.fixed.md

@@ -0,0 +1 @@
+Made a deltaproxy tear down its sub-proxies when it stops. Sub-proxies live in ``deltaproxy_objs`` and each owns its own ``req_channel``, schedule, beacons and periodic callbacks, but nothing in the shutdown path ever touched them, so every stop or restart abandoned them.

salt/minion.py

@@ -6333,6 +6333,27 @@ def tune_in(self, start=True):
         mp_call = _metaproxy_call(self.opts, "tune_in")
         return mp_call(self, start)
 
+    def destroy(self):
+        """
+        Tear down the proxy minion.
+
+        A deltaproxy holds its sub-proxies in ``deltaproxy_objs``, and each of
+        them owns its own ``req_channel``, schedule, beacons and periodic
+        callbacks.  Nothing used to tear those down, so every stop or restart
+        abandoned them.  Destroy the sub-proxies first, then this minion.
+        """
+        subproxies = getattr(self, "deltaproxy_objs", None) or {}
+        for minion_id, _minion in list(subproxies.items()):
+            try:
+                _minion.destroy()
+            except Exception:  # pylint: disable=broad-except
+                log.warning(
+                    "Unable to tear down sub proxy %s", minion_id, exc_info=True
+                )
+        if subproxies:
+            subproxies.clear()
+        super().destroy()
+
     def _target_load(self, load):
         """
         Verify that the publication is valid and applies to this minion

tests/pytests/unit/test_minion.py

@@ -2441,3 +2441,61 @@ async def _instant_sleep(_):
         # code path; the .destroy() call would try to tear down channels
         # we never created. A best-effort close is enough.
         pass
+
+
+def test_proxy_minion_destroy_tears_down_subproxies(minion_opts):
+    """
+    A deltaproxy holds its sub-proxies in ``deltaproxy_objs`` and each of them
+    owns its own ``req_channel``, schedule, beacons and periodic callbacks.
+    Nothing tore those down, so every stop or restart abandoned them.
+    """
+    minion_opts["metaproxy"] = "deltaproxy"
+    proxy = salt.minion.ProxyMinion.__new__(salt.minion.ProxyMinion)
+    proxy.opts = minion_opts
+
+    sub1 = MagicMock()
+    sub2 = MagicMock()
+    proxy.deltaproxy_objs = {"minion1": sub1, "minion2": sub2}
+
+    with patch.object(salt.minion.Minion, "destroy") as parent_destroy:
+        proxy.destroy()
+
+    assert sub1.destroy.called
+    assert sub2.destroy.called
+    # The control proxy itself is still torn down afterwards.
+    assert parent_destroy.called
+
+
+def test_proxy_minion_destroy_survives_a_bad_subproxy(minion_opts):
+    """
+    One sub-proxy failing to tear down must not stop the others, nor the
+    control proxy's own teardown.
+    """
+    minion_opts["metaproxy"] = "deltaproxy"
+    proxy = salt.minion.ProxyMinion.__new__(salt.minion.ProxyMinion)
+    proxy.opts = minion_opts
+
+    bad = MagicMock()
+    bad.destroy.side_effect = RuntimeError("device gone")
+    good = MagicMock()
+    proxy.deltaproxy_objs = {"minion1": bad, "minion2": good}
+
+    with patch.object(salt.minion.Minion, "destroy") as parent_destroy:
+        proxy.destroy()
+
+    assert good.destroy.called
+    assert parent_destroy.called
+
+
+def test_proxy_minion_destroy_without_subproxies(minion_opts):
+    """
+    Inverse: a single (non-delta) proxy has no ``deltaproxy_objs`` at all and
+    must still tear itself down normally.
+    """
+    proxy = salt.minion.ProxyMinion.__new__(salt.minion.ProxyMinion)
+    proxy.opts = minion_opts
+
+    with patch.object(salt.minion.Minion, "destroy") as parent_destroy:
+        proxy.destroy()
+
+    assert parent_destroy.called