Merge pull request ceph#60587 from phlogistonjohn/jjm-more-py312-fixes

various python 3.12 fixes

Reviewed-by: Adam King <[email protected]>

src/cephadm/cephadm.py

@@ -4489,8 +4489,9 @@ def _rm_cluster(ctx: CephadmContext, keep_logs: bool, zap_osds: bool) -> None:
 ##################################
 
 
-def check_time_sync(ctx, enabler=None):
-    # type: (CephadmContext, Optional[Packager]) -> bool
+def check_time_sync(
+    ctx: CephadmContext, enabler: Optional[Packager] = None
+) -> bool:
     units = [
         'chrony.service',  # 18.04 (at least)
         'chronyd.service',  # el / opensuse

src/cephadm/cephadmlib/call_wrappers.py

@@ -311,14 +311,14 @@ def call_throws(
     return out, err, ret
 
 
-def call_timeout(ctx, command, timeout):
-    # type: (CephadmContext, List[str], int) -> int
+def call_timeout(
+    ctx: CephadmContext, command: List[str], timeout: int
+) -> int:
     logger.debug(
         'Running command (timeout=%s): %s' % (timeout, ' '.join(command))
     )
 
-    def raise_timeout(command, timeout):
-        # type: (List[str], int) -> NoReturn
+    def raise_timeout(command: List[str], timeout: int) -> NoReturn:
         msg = 'Command `%s` timed out after %s seconds' % (command, timeout)
         logger.debug(msg)
         raise TimeoutExpired(msg)

src/cephadm/cephadmlib/daemon_identity.py

@@ -157,7 +157,7 @@ def sidecar_service_name(self) -> str:
         )
 
     def sidecar_script(self, base_data_dir: Union[str, os.PathLike]) -> str:
-        sname = f'sidecar-{ self.subcomponent }.run'
+        sname = f'sidecar-{self.subcomponent}.run'
         return str(pathlib.Path(self.data_dir(base_data_dir)) / sname)
 
     @property

src/cephadm/cephadmlib/daemons/ingress.py

@@ -82,8 +82,7 @@ def create_daemon_dirs(self, data_dir: str, uid: int, gid: int) -> None:
     def get_daemon_args(self) -> List[str]:
         return ['haproxy', '-f', '/var/lib/haproxy/haproxy.cfg']
 
-    def validate(self):
-        # type: () -> None
+    def validate(self) -> None:
         if not is_fsid(self.fsid):
             raise Error('not an fsid: %s' % self.fsid)
         if not self.daemon_id:
@@ -99,12 +98,10 @@ def validate(self):
                         'required file missing from config-json: %s' % fname
                     )
 
-    def get_daemon_name(self):
-        # type: () -> str
+    def get_daemon_name(self) -> str:
         return '%s.%s' % (self.daemon_type, self.daemon_id)
 
-    def get_container_name(self, desc=None):
-        # type: (Optional[str]) -> str
+    def get_container_name(self, desc: Optional[str] = None) -> str:
         cname = 'ceph-%s-%s' % (self.fsid, self.get_daemon_name())
         if desc:
             cname = '%s-%s' % (cname, desc)
@@ -212,8 +209,7 @@ def create_daemon_dirs(self, data_dir: str, uid: int, gid: int) -> None:
         # populate files from the config-json
         populate_files(data_dir, self.files, uid, gid)
 
-    def validate(self):
-        # type: () -> None
+    def validate(self) -> None:
         if not is_fsid(self.fsid):
             raise Error('not an fsid: %s' % self.fsid)
         if not self.daemon_id:
@@ -229,20 +225,17 @@ def validate(self):
                         'required file missing from config-json: %s' % fname
                     )
 
-    def get_daemon_name(self):
-        # type: () -> str
+    def get_daemon_name(self) -> str:
         return '%s.%s' % (self.daemon_type, self.daemon_id)
 
-    def get_container_name(self, desc=None):
-        # type: (Optional[str]) -> str
+    def get_container_name(self, desc: Optional[str] = None) -> str:
         cname = 'ceph-%s-%s' % (self.fsid, self.get_daemon_name())
         if desc:
             cname = '%s-%s' % (cname, desc)
         return cname
 
     @staticmethod
-    def get_container_envs():
-        # type: () -> List[str]
+    def get_container_envs() -> List[str]:
         envs = [
             'KEEPALIVED_AUTOCONF=false',
             'KEEPALIVED_CONF=/etc/keepalived/keepalived.conf',

src/cephadm/cephadmlib/daemons/nfs.py

@@ -42,9 +42,13 @@ def for_daemon_type(cls, daemon_type: str) -> bool:
         return cls.daemon_type == daemon_type
 
     def __init__(
-        self, ctx, fsid, daemon_id, config_json, image=DEFAULT_IMAGE
-    ):
-        # type: (CephadmContext, str, Union[int, str], Dict, str) -> None
+        self,
+        ctx: CephadmContext,
+        fsid: str,
+        daemon_id: Union[int, str],
+        config_json: Dict,
+        image: str = DEFAULT_IMAGE,
+    ) -> None:
         self.ctx = ctx
         self.fsid = fsid
         self.daemon_id = daemon_id
@@ -62,8 +66,9 @@ def __init__(
         self.validate()
 
     @classmethod
-    def init(cls, ctx, fsid, daemon_id):
-        # type: (CephadmContext, str, Union[int, str]) -> NFSGanesha
+    def init(
+        cls, ctx: CephadmContext, fsid: str, daemon_id: Union[int, str]
+    ) -> 'NFSGanesha':
         return cls(ctx, fsid, daemon_id, fetch_configs(ctx), ctx.image)
 
     @classmethod

src/cephadm/cephadmlib/daemons/nvmeof.py

@@ -33,9 +33,13 @@ def for_daemon_type(cls, daemon_type: str) -> bool:
         return cls.daemon_type == daemon_type
 
     def __init__(
-        self, ctx, fsid, daemon_id, config_json, image=DEFAULT_NVMEOF_IMAGE
-    ):
-        # type: (CephadmContext, str, Union[int, str], Dict, str) -> None
+        self,
+        ctx: CephadmContext,
+        fsid: str,
+        daemon_id: Union[int, str],
+        config_json: Dict,
+        image: str = DEFAULT_NVMEOF_IMAGE,
+    ) -> None:
         self.ctx = ctx
         self.fsid = fsid
         self.daemon_id = daemon_id
@@ -48,8 +52,9 @@ def __init__(
         self.validate()
 
     @classmethod
-    def init(cls, ctx, fsid, daemon_id):
-        # type: (CephadmContext, str, Union[int, str]) -> CephNvmeof
+    def init(
+        cls, ctx: CephadmContext, fsid: str, daemon_id: Union[int, str]
+    ) -> 'CephNvmeof':
         return cls(ctx, fsid, daemon_id, fetch_configs(ctx), ctx.image)
 
     @classmethod

src/cephadm/cephadmlib/data_utils.py

@@ -189,8 +189,9 @@ def normalize_image_digest(digest: str) -> str:
     return digest
 
 
-def get_legacy_config_fsid(cluster, legacy_dir=None):
-    # type: (str, Optional[str]) -> Optional[str]
+def get_legacy_config_fsid(
+    cluster: str, legacy_dir: Optional[str] = None
+) -> Optional[str]:
     config_file = '/etc/ceph/%s.conf' % cluster
     if legacy_dir is not None:
         config_file = os.path.abspath(legacy_dir + config_file)

src/cephadm/cephadmlib/file_utils.py

@@ -52,8 +52,9 @@ def write_new(
     os.rename(tempname, destination)
 
 
-def populate_files(config_dir, config_files, uid, gid):
-    # type: (str, Dict, int, int) -> None
+def populate_files(
+    config_dir: str, config_files: Dict, uid: int, gid: int
+) -> None:
     """create config files for different services"""
     for fname in config_files:
         config_file = os.path.join(config_dir, fname)
@@ -71,8 +72,7 @@ def touch(
         os.chown(file_path, uid, gid)
 
 
-def write_tmp(s, uid, gid):
-    # type: (str, int, int) -> IO[str]
+def write_tmp(s: str, uid: int, gid: int) -> IO[str]:
     tmp_f = tempfile.NamedTemporaryFile(mode='w', prefix='ceph-tmp')
     os.fchown(tmp_f.fileno(), uid, gid)
     tmp_f.write(s)
@@ -97,8 +97,7 @@ def recursive_chown(path: str, uid: int, gid: int) -> None:
             os.chown(os.path.join(dirpath, filename), uid, gid)
 
 
-def read_file(path_list, file_name=''):
-    # type: (List[str], str) -> str
+def read_file(path_list: List[str], file_name: str = '') -> str:
     """Returns the content of the first file found within the `path_list`
 
     :param path_list: list of file paths to search
@@ -123,14 +122,12 @@ def read_file(path_list, file_name=''):
     return 'Unknown'
 
 
-def pathify(p):
-    # type: (str) -> str
+def pathify(p: str) -> str:
     p = os.path.expanduser(p)
     return os.path.abspath(p)
 
 
-def get_file_timestamp(fn):
-    # type: (str) -> Optional[str]
+def get_file_timestamp(fn: str) -> Optional[str]:
     try:
         mt = os.path.getmtime(fn)
         return datetime.datetime.fromtimestamp(

src/cephadm/cephadmlib/systemd.py

@@ -11,8 +11,7 @@
 logger = logging.getLogger()
 
 
-def check_unit(ctx, unit_name):
-    # type: (CephadmContext, str) -> Tuple[bool, str, bool]
+def check_unit(ctx: CephadmContext, unit_name: str) -> Tuple[bool, str, bool]:
     # NOTE: we ignore the exit code here because systemctl outputs
     # various exit codes based on the state of the service, but the
     # string result is more explicit (and sufficient).
@@ -56,8 +55,9 @@ def check_unit(ctx, unit_name):
     return (enabled, state, installed)
 
 
-def check_units(ctx, units, enabler=None):
-    # type: (CephadmContext, List[str], Optional[Packager]) -> bool
+def check_units(
+    ctx: CephadmContext, units: List[str], enabler: Optional[Packager] = None
+) -> bool:
     for u in units:
         (enabled, state, installed) = check_unit(ctx, u)
         if enabled and state == 'running':

src/cephadm/tests/test_agent.py

@@ -668,7 +668,7 @@ def recv(self, len: Optional[int] = None):
         agent.mgr_listener.run()
 
         # verify payload was correctly extracted
-        assert _handle_json_payload.called_with(json.loads(payload))
+        _handle_json_payload.assert_called_with(json.loads(payload))
         FakeConn.send.assert_called_once_with(b'ACK')
 
         # second run, with bad json data received

src/cephadm/tests/test_cephadm.py

@@ -1,5 +1,6 @@
 # type: ignore
 
+import contextlib
 import copy
 import errno
 import json
@@ -38,6 +39,13 @@ def get_ceph_conf(
         mon_host = {mon_host}
 '''
 
+@contextlib.contextmanager
+def bootstrap_test_ctx(*args, **kwargs):
+    with with_cephadm_ctx(*args, **kwargs) as ctx:
+        ctx.no_cleanup_on_failure = True
+        yield ctx
+
+
 class TestCephAdm(object):
 
     @mock.patch('cephadm.logger')
@@ -1432,21 +1440,21 @@ def test_config(self, cephadm_fs, funkypatch):
             '--config', conf_file,
         )
 
-        with with_cephadm_ctx(cmd) as ctx:
+        with bootstrap_test_ctx(cmd) as ctx:
             msg = r'No such file or directory'
             with pytest.raises(_cephadm.Error, match=msg):
                 _cephadm.command_bootstrap(ctx)
 
         cephadm_fs.create_file(conf_file)
-        with with_cephadm_ctx(cmd) as ctx:
+        with bootstrap_test_ctx(cmd) as ctx:
             retval = _cephadm.command_bootstrap(ctx)
             assert retval == 0
 
     def test_no_mon_addr(self, cephadm_fs, funkypatch):
         funkypatch.patch('cephadmlib.systemd.call')
 
         cmd = self._get_cmd()
-        with with_cephadm_ctx(cmd) as ctx:
+        with bootstrap_test_ctx(cmd) as ctx:
             msg = r'must specify --mon-ip or --mon-addrv'
             with pytest.raises(_cephadm.Error, match=msg):
                 _cephadm.command_bootstrap(ctx)
@@ -1455,13 +1463,13 @@ def test_skip_mon_network(self, cephadm_fs, funkypatch):
         funkypatch.patch('cephadmlib.systemd.call')
         cmd = self._get_cmd('--mon-ip', '192.168.1.1')
 
-        with with_cephadm_ctx(cmd, list_networks={}) as ctx:
+        with bootstrap_test_ctx(cmd, list_networks={}) as ctx:
             msg = r'--skip-mon-network'
             with pytest.raises(_cephadm.Error, match=msg):
                 _cephadm.command_bootstrap(ctx)
 
         cmd += ['--skip-mon-network']
-        with with_cephadm_ctx(cmd, list_networks={}) as ctx:
+        with bootstrap_test_ctx(cmd, list_networks={}) as ctx:
             retval = _cephadm.command_bootstrap(ctx)
             assert retval == 0
 
@@ -1540,12 +1548,12 @@ def test_mon_ip(self, mon_ip, list_networks, result, cephadm_fs, funkypatch):
 
         cmd = self._get_cmd('--mon-ip', mon_ip)
         if not result:
-            with with_cephadm_ctx(cmd, list_networks=list_networks) as ctx:
+            with bootstrap_test_ctx(cmd, list_networks=list_networks) as ctx:
                 msg = r'--skip-mon-network'
                 with pytest.raises(_cephadm.Error, match=msg):
                     _cephadm.command_bootstrap(ctx)
         else:
-            with with_cephadm_ctx(cmd, list_networks=list_networks) as ctx:
+            with bootstrap_test_ctx(cmd, list_networks=list_networks) as ctx:
                 retval = _cephadm.command_bootstrap(ctx)
                 assert retval == 0
 
@@ -1604,11 +1612,11 @@ def test_mon_addrv(self, mon_addrv, list_networks, err, cephadm_fs, funkypatch):
 
         cmd = self._get_cmd('--mon-addrv', mon_addrv)
         if err:
-            with with_cephadm_ctx(cmd, list_networks=list_networks) as ctx:
+            with bootstrap_test_ctx(cmd, list_networks=list_networks) as ctx:
                 with pytest.raises(_cephadm.Error, match=err):
                     _cephadm.command_bootstrap(ctx)
         else:
-            with with_cephadm_ctx(cmd, list_networks=list_networks) as ctx:
+            with bootstrap_test_ctx(cmd, list_networks=list_networks) as ctx:
                 retval = _cephadm.command_bootstrap(ctx)
                 assert retval == 0
 
@@ -1621,13 +1629,13 @@ def test_allow_fqdn_hostname(self, cephadm_fs, funkypatch):
             '--skip-mon-network',
         )
 
-        with with_cephadm_ctx(cmd, hostname=hostname) as ctx:
+        with bootstrap_test_ctx(cmd, hostname=hostname) as ctx:
             msg = r'--allow-fqdn-hostname'
             with pytest.raises(_cephadm.Error, match=msg):
                 _cephadm.command_bootstrap(ctx)
 
         cmd += ['--allow-fqdn-hostname']
-        with with_cephadm_ctx(cmd, hostname=hostname) as ctx:
+        with bootstrap_test_ctx(cmd, hostname=hostname) as ctx:
             retval = _cephadm.command_bootstrap(ctx)
             assert retval == 0
 
@@ -1646,7 +1654,7 @@ def test_fsid(self, fsid, err, cephadm_fs, funkypatch):
             '--fsid', fsid,
         )
 
-        with with_cephadm_ctx(cmd) as ctx:
+        with bootstrap_test_ctx(cmd) as ctx:
             if err:
                 with pytest.raises(_cephadm.Error, match=err):
                     _cephadm.command_bootstrap(ctx)
@@ -1661,7 +1669,7 @@ def test_fsid(self, cephadm_fs):
         fsid = '00000000-0000-0000-0000-0000deadbeef'
 
         cmd = ['shell', '--fsid', fsid]
-        with with_cephadm_ctx(cmd) as ctx:
+        with bootstrap_test_ctx(cmd) as ctx:
             retval = _cephadm.command_shell(ctx)
             assert retval == 0
             assert ctx.fsid == fsid

src/cephadm/tests/test_deploy.py

@@ -495,6 +495,7 @@ def test_deploy_ceph_exporter_container(cephadm_fs, funkypatch):
 def test_deploy_and_rm_iscsi(cephadm_fs, funkypatch):
     # Test that the deploy and remove paths for iscsi (which has sidecar container)
     # create and remove the correct unit files.
+    funkypatch.patch('shutil.rmtree')  # fakefs + shutil.rmtree breaks on py3.12
     mocks = _common_patches(funkypatch)
     _firewalld = mocks['Firewalld']
     fsid = 'b01dbeef-701d-9abe-0000-e1e5a47004a7'