From: Shweta Bhosale Date: Mon, 11 May 2026 05:03:48 +0000 (+0530) Subject: cephadm: added remove-file command for client file cleanup X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=c40c6296a1cf3221e9da4ce7666255c017d7e202;p=ceph.git cephadm: added remove-file command for client file cleanup Add a cephadm remove-file subcommand that deletes only regular files. Have the mgr call it from CephadmServe._write_client_files when pruning stale client keyrings instead of running ssh rm, so removal goes through the same cephadm/invoker path as other host operations. Fixes: https://tracker.ceph.com/issues/74045 Signed-off-by: Shweta Bhosale --- diff --git a/doc/man/8/cephadm.rst b/doc/man/8/cephadm.rst index c290bf7712d..1d352ded429 100644 --- a/doc/man/8/cephadm.rst +++ b/doc/man/8/cephadm.rst @@ -13,7 +13,7 @@ Synopsis | [--log-dir LOG_DIR] [--logrotate-dir LOGROTATE_DIR] | [--unit-dir UNIT_DIR] [--verbose] [--timeout TIMEOUT] | [--retry RETRY] [--no-container-init] -| {version,pull,inspect-image,ls,list-networks,list-rdma,adopt,rm-daemon,rm-cluster,run,shell,enter,ceph-volume,unit,logs,bootstrap,deploy,check-host,check-online,prepare-host,prepare-host-sudo-hardening,setup-ssh-user,add-repo,rm-repo,install,list-images,update-osd-service} +| {version,pull,inspect-image,ls,list-networks,list-rdma,adopt,rm-daemon,rm-cluster,remove-file,run,shell,enter,ceph-volume,unit,logs,bootstrap,deploy,check-host,check-online,prepare-host,prepare-host-sudo-hardening,setup-ssh-user,add-repo,rm-repo,install,list-images,update-osd-service} | ... @@ -92,6 +92,8 @@ Synopsis | **cephadm** **check-online** +| **cephadm** **remove-file** [-h] [--fsid FSID] --path PATH + | **cephadm** **prepare-host** | **cephadm** **add-repo** [-h] [--release RELEASE] [--version VERSION] @@ -300,6 +302,19 @@ This command is primarily intended for cephadm internals (for example, the offline host watcher), rather than direct operator workflows. +remove-file +----------- + +Remove a regular file on the local host. Missing paths are ignored. +Refuses directories and symbolic links, only plain files are +removed. + +Arguments: + +* [--fsid FSID] cluster FSID (passed automatically when invoked by the orchestrator) +* --path PATH absolute path of the file to remove (required) + + deploy ------ diff --git a/src/cephadm/cephadm.py b/src/cephadm/cephadm.py index 2365054f42d..d49e96ea45b 100755 --- a/src/cephadm/cephadm.py +++ b/src/cephadm/cephadm.py @@ -4643,6 +4643,29 @@ def command_gather_facts(ctx: CephadmContext) -> None: print(host.dump()) +def command_remove_file(ctx: CephadmContext) -> int: + """Remove a regular file on the host + """ + norm = Path(os.path.normpath(str(Path(ctx.remove_file_path).expanduser()))) + + if not norm.is_absolute(): + raise Error(f'Can not remove non-absolute path: {norm}') + try: + if not norm.exists(): + return 0 + # Refuse symlinks explicitly because is_file() follows them + if norm.is_symlink() or not norm.is_file(): + raise Error(f'Can not remove non-regular file: {norm}') + + norm.unlink() + + except FileNotFoundError: + return 0 + except OSError as e: + raise Error(f'failed to remove {norm}: {e}') + return 0 + + ################################## @@ -5697,6 +5720,18 @@ def _get_parser(): 'gather-facts', help='gather and return host related information (JSON format)') parser_gather_facts.set_defaults(func=command_gather_facts) + parser_remove_file = subparsers.add_parser( + 'remove-file', help='remove a file on the host') + parser_remove_file.set_defaults(func=command_remove_file) + parser_remove_file.add_argument( + '--fsid', + help='cluster FSID') + parser_remove_file.add_argument( + '--path', + required=True, + dest='remove_file_path', + help='absolute path of the file to remove') + parser_maintenance = subparsers.add_parser( 'host-maintenance', help='Manage the maintenance state of a host') parser_maintenance.add_argument( @@ -5845,7 +5880,7 @@ def main() -> None: command_add_repo, command_rm_repo, command_install, - command_bootstrap + command_bootstrap, ]: check_container_engine(ctx) # command handler diff --git a/src/cephadm/tests/test_cephadm.py b/src/cephadm/tests/test_cephadm.py index 7d2402d8b9b..efba1cf777e 100644 --- a/src/cephadm/tests/test_cephadm.py +++ b/src/cephadm/tests/test_cephadm.py @@ -106,6 +106,44 @@ class TestCephAdm(object): _attempt_bind.side_effect = os_error assert port_in_use(empty_ctx, _cephadm.EndPoint('0.0.0.0', 9100)) == False + def test_command_remove_file(self, cephadm_fs): + rm_path = '/tmp/cephadm-remove-file-test' + cephadm_fs.create_file(rm_path, contents='x') + with with_cephadm_ctx( + ['remove-file', '--fsid', '00000000-0000-0000-0000-0000deadbeef', '--path', rm_path] + ) as ctx: + assert _cephadm.command_remove_file(ctx) == 0 + assert not cephadm_fs.exists(rm_path) + + def test_command_remove_file_missing_ok(self, cephadm_fs): + missing = '/tmp/cephadm-remove-file-missing' + with with_cephadm_ctx( + ['remove-file', '--fsid', '00000000-0000-0000-0000-0000deadbeef', '--path', missing] + ) as ctx: + assert _cephadm.command_remove_file(ctx) == 0 + + def test_command_remove_file_refuses_directory(self, cephadm_fs): + dpath = '/tmp/cephadm-remove-file-isdir' + cephadm_fs.create_dir(dpath) + with with_cephadm_ctx( + ['remove-file', '--fsid', '00000000-0000-0000-0000-0000deadbeef', '--path', dpath] + ) as ctx: + with pytest.raises(_cephadm.Error, match='Can not remove non-regular file'): + _cephadm.command_remove_file(ctx) + assert cephadm_fs.exists(dpath) + + def test_command_remove_file_refuses_symlink(self, cephadm_fs): + target = '/tmp/cephadm-remove-file-symtarget' + link = '/tmp/cephadm-remove-file-symlink' + cephadm_fs.create_file(target, contents='x') + cephadm_fs.create_symlink(link, target) + with with_cephadm_ctx( + ['remove-file', '--fsid', '00000000-0000-0000-0000-0000deadbeef', '--path', link] + ) as ctx: + with pytest.raises(_cephadm.Error, match='Can not remove non-regular file'): + _cephadm.command_remove_file(ctx) + assert cephadm_fs.exists(link) + @mock.patch('cephadm.socket.socket.bind') @mock.patch('cephadm.logger') def test_port_in_use_special_cases(self, _logger, _bind): diff --git a/src/pybind/mgr/cephadm/serve.py b/src/pybind/mgr/cephadm/serve.py index e45ec846df2..7d992260717 100644 --- a/src/pybind/mgr/cephadm/serve.py +++ b/src/pybind/mgr/cephadm/serve.py @@ -1399,8 +1399,9 @@ class CephadmServe: if path == '/etc/ceph/ceph.conf': continue self.log.info(f'Removing {host}:{path}') - cmd = ssh.RemoteCommand(ssh.Executables.RM, ['-f', path]) - self.mgr.ssh.check_execute_command(host, cmd) + with self.mgr.async_timeout_handler(host, f'cephadm remove-file ({path})'): + self.mgr.wait_async(self._run_cephadm( + host, cephadmNoImage, 'remove-file', ['--path', path])) updated_files = True self.mgr.cache.removed_client_file(host, path) if updated_files: diff --git a/src/pybind/mgr/cephadm/tests/test_cephadm.py b/src/pybind/mgr/cephadm/tests/test_cephadm.py index b97b7f175f2..bb5933152a7 100644 --- a/src/pybind/mgr/cephadm/tests/test_cephadm.py +++ b/src/pybind/mgr/cephadm/tests/test_cephadm.py @@ -15,7 +15,7 @@ from cephadm.inventory import ( ) from cephadm.services.osd import OSD, OSDRemovalQueue, OsdIdClaims from cephadm.services.nvmeof import NvmeofService -from cephadm.utils import SpecialHostLabels +from cephadm.utils import SpecialHostLabels, cephadmNoImage try: from typing import List @@ -2346,6 +2346,20 @@ class TestCephadm(object): CephadmServe(cephadm_module)._write_client_files({}, 'host2') CephadmServe(cephadm_module)._write_client_files({}, 'host3') + @mock.patch('cephadm.serve.CephadmServe._run_cephadm', new_callable=mock.AsyncMock) + def test_write_client_files_remove_calls_cephadm_remove_file(self, _run_cephadm, cephadm_module): + _run_cephadm.return_value = ([''], [''], 0) + cephadm_module.inventory.add_host(HostSpec('host1', '10.0.0.1')) + cephadm_module.cache.prime_empty_host('host1') + stale = '/var/lib/ceph/fsid/config/foo.keyring' + cephadm_module.cache.update_client_file('host1', stale, 'digest', 0o600, 0, 0) + CephadmServe(cephadm_module)._write_client_files({'host1': {}}, 'host1') + _run_cephadm.assert_called_once() + pos_args = _run_cephadm.call_args[0] + # Bound method mock: call_args do not include self. + assert pos_args[0:4] == ('host1', cephadmNoImage, 'remove-file', ['--path', stale]) + assert stale not in cephadm_module.cache.get_host_client_files('host1') + @mock.patch('cephadm.CephadmOrchestrator.mon_command') @mock.patch("cephadm.inventory.HostCache.get_host_client_files") def test_dont_write_etc_ceph_client_files_when_turned_off(self, _get_client_files, _mon_command, cephadm_module):