summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--qa/tasks/thrashosds-health.yaml2
-rwxr-xr-xqa/workunits/cephtool/test.sh16
-rwxr-xr-xsrc/cephadm/cephadm.py4
-rw-r--r--src/osdc/Journaler.cc16
-rw-r--r--src/pybind/mgr/cephadm/module.py5
-rw-r--r--src/pybind/mgr/cephadm/serve.py2
-rw-r--r--src/pybind/mgr/cephadm/services/cephadmservice.py2
-rw-r--r--src/pybind/mgr/cephadm/services/nfs.py2
-rw-r--r--src/pybind/mgr/smb/fs.py118
-rw-r--r--src/pybind/mgr/smb/handler.py2
-rw-r--r--src/pybind/mgr/smb/module.py2
-rw-r--r--src/pybind/mgr/smb/tests/test_fs.py108
-rw-r--r--src/pybind/mgr/smb/tests/test_smb.py55
13 files changed, 308 insertions, 26 deletions
diff --git a/qa/tasks/thrashosds-health.yaml b/qa/tasks/thrashosds-health.yaml
index f06fd68e9be..7399022abad 100644
--- a/qa/tasks/thrashosds-health.yaml
+++ b/qa/tasks/thrashosds-health.yaml
@@ -28,3 +28,5 @@ overrides:
- mons down
- mon down
- out of quorum
+ - noscrub
+ - nodeep-scrub
diff --git a/qa/workunits/cephtool/test.sh b/qa/workunits/cephtool/test.sh
index ee8b6029a52..89fdcaf0593 100755
--- a/qa/workunits/cephtool/test.sh
+++ b/qa/workunits/cephtool/test.sh
@@ -888,7 +888,7 @@ function test_tell_output_file()
# only one line of json
expect_true sed '2q1' < /tmp/foo > /dev/null
expect_true jq -e '.version | length > 0' < /tmp/foo
- rm -f /tmp/foo
+ sudo rm -f /tmp/foo
J=$(ceph tell --format=json-pretty --daemon-output-file=/tmp/foo "$name" version)
expect_true jq -e '.path == "/tmp/foo"' <<<"$J"
@@ -896,24 +896,24 @@ function test_tell_output_file()
# more than one line of json
expect_false sed '2q1' < /tmp/foo > /dev/null
expect_true jq -e '.version | length > 0' < /tmp/foo
- rm -f /tmp/foo
+ sudo rm -f /tmp/foo
# Test --daemon-output-file=:tmp:
J=$(ceph tell --format=json --daemon-output-file=":tmp:" "$name" version)
path=$(jq -r .path <<<"$J")
expect_true test -e "$path"
# only one line of json
- expect_true sed '2q1' < "$path" > /dev/null
- expect_true jq -e '.version | length > 0' < "$path"
- rm -f "$path"
+ expect_true sudo sh -c "sed '2q1' < \"$path\" > /dev/null"
+ expect_true sudo sudo sh -c "jq -e '.version | length > 0' < \"$path\""
+ sudo rm -f "$path"
J=$(ceph tell --format=json-pretty --daemon-output-file=":tmp:" "$name" version)
path=$(jq -r .path <<<"$J")
expect_true test -e "$path"
# only one line of json
- expect_false sed '2q1' < "$path" > /dev/null
- expect_true jq -e '.version | length > 0' < "$path"
- rm -f "$path"
+ expect_false sudo sh -c "sed '2q1' < \"$path\" > /dev/null"
+ expect_true sudo sh -c "jq -e '.version | length > 0' < \"$path\""
+ sudo rm -f "$path"
}
function test_mds_tell()
diff --git a/src/cephadm/cephadm.py b/src/cephadm/cephadm.py
index 95169358f3a..5deaec55949 100755
--- a/src/cephadm/cephadm.py
+++ b/src/cephadm/cephadm.py
@@ -2892,6 +2892,10 @@ def command_bootstrap(ctx):
cli(['config', 'set', 'mgr', 'mgr/cephadm/container_init', str(ctx.container_init), '--force'])
+ if ctx.no_cgroups_split:
+ logger.info('Setting mgr/cephadm/cgroups_split to false')
+ cli(['config', 'set', 'mgr', 'mgr/cephadm/cgroups_split', 'false', '--force'])
+
if not ctx.skip_dashboard:
prepare_dashboard(ctx, uid, gid, cli, wait_for_mgr_restart)
diff --git a/src/osdc/Journaler.cc b/src/osdc/Journaler.cc
index 04b90fb5952..c1c1178bd7f 100644
--- a/src/osdc/Journaler.cc
+++ b/src/osdc/Journaler.cc
@@ -493,7 +493,21 @@ void Journaler::_finish_write_head(int r, Header &wrote,
}
ceph_assert(!readonly);
ldout(cct, 10) << "_finish_write_head " << wrote << dendl;
- last_committed = wrote;
+ if (wrote.write_pos < last_committed.write_pos ||
+ wrote.expire_pos < last_committed.expire_pos ||
+ wrote.trimmed_pos < last_committed.trimmed_pos) {
+ lderr(cct) << __func__ << ": not updating last_committed: "
+ << "(wrote.write_pos/last_committed.write_pos="
+ << wrote.write_pos << "," << last_committed.write_pos << "), "
+ << "(wrote.expire_pos/last_committed.expire_pos="
+ << wrote.expire_pos << "," << last_committed.expire_pos << "), "
+ << "(wrote.trimmed_pos/last_committed.trimmed_pos="
+ << wrote.trimmed_pos << "," << last_committed.trimmed_pos << ")"
+ << dendl;
+ ceph_abort();
+ } else {
+ last_committed = wrote;
+ }
if (oncommit) {
oncommit->complete(r);
}
diff --git a/src/pybind/mgr/cephadm/module.py b/src/pybind/mgr/cephadm/module.py
index b28332bb6e6..c34f77a740c 100644
--- a/src/pybind/mgr/cephadm/module.py
+++ b/src/pybind/mgr/cephadm/module.py
@@ -3321,6 +3321,9 @@ Then run the following:
'data': self._preview_osdspecs(osdspecs=[cast(DriveGroupSpec, spec)])}
svc = self.cephadm_services[spec.service_type]
+ rank_map = None
+ if svc.ranked(spec):
+ rank_map = self.spec_store[spec.service_name()].rank_map
ha = HostAssignment(
spec=spec,
hosts=self.cache.get_schedulable_hosts(),
@@ -3329,7 +3332,7 @@ Then run the following:
networks=self.cache.networks,
daemons=self.cache.get_daemons_by_service(spec.service_name()),
allow_colo=svc.allow_colo(),
- rank_map=self.spec_store[spec.service_name()].rank_map if svc.ranked() else None
+ rank_map=rank_map
)
ha.validate()
hosts, to_add, to_remove = ha.place()
diff --git a/src/pybind/mgr/cephadm/serve.py b/src/pybind/mgr/cephadm/serve.py
index 59e06bbd024..eaaf4386f62 100644
--- a/src/pybind/mgr/cephadm/serve.py
+++ b/src/pybind/mgr/cephadm/serve.py
@@ -780,7 +780,7 @@ class CephadmServe:
}
rank_map = None
- if svc.ranked():
+ if svc.ranked(spec):
rank_map = self.mgr.spec_store[spec.service_name()].rank_map or {}
ha = HostAssignment(
spec=spec,
diff --git a/src/pybind/mgr/cephadm/services/cephadmservice.py b/src/pybind/mgr/cephadm/services/cephadmservice.py
index ec9df98413a..72e6177bc1d 100644
--- a/src/pybind/mgr/cephadm/services/cephadmservice.py
+++ b/src/pybind/mgr/cephadm/services/cephadmservice.py
@@ -288,7 +288,7 @@ class CephadmService(metaclass=ABCMeta):
"""
return None
- def ranked(self) -> bool:
+ def ranked(self, spec: ServiceSpec) -> bool:
"""
If True, we will assign a stable rank (0, 1, ...) and monotonically increasing
generation (0, 1, ...) to each daemon we create/deploy.
diff --git a/src/pybind/mgr/cephadm/services/nfs.py b/src/pybind/mgr/cephadm/services/nfs.py
index f46f65b084b..a0d7da9bb7e 100644
--- a/src/pybind/mgr/cephadm/services/nfs.py
+++ b/src/pybind/mgr/cephadm/services/nfs.py
@@ -23,7 +23,7 @@ logger = logging.getLogger(__name__)
class NFSService(CephService):
TYPE = 'nfs'
- def ranked(self) -> bool:
+ def ranked(self, spec: ServiceSpec) -> bool:
return True
def fence(self, daemon_id: str) -> None:
diff --git a/src/pybind/mgr/smb/fs.py b/src/pybind/mgr/smb/fs.py
index 8aaa8bcde06..dc9613f21fc 100644
--- a/src/pybind/mgr/smb/fs.py
+++ b/src/pybind/mgr/smb/fs.py
@@ -1,8 +1,9 @@
-from typing import List, Optional
+from typing import Dict, List, Optional, Tuple
import logging
import posixpath
import stat
+import time
import cephfs
from mgr_util import CephfsClient, Module_T, open_filesystem
@@ -60,16 +61,13 @@ class CephFSPathResolver:
self._mgr = mgr
self._cephfs_client = client or CephfsClient(mgr)
- def resolve(
- self, volume: str, subvolumegroup: str, subvolume: str, path: str
+ def resolve_subvolume_path(
+ self, volume: str, subvolumegroup: str, subvolume: str
) -> str:
- """Given a volume, subvolumegroup, subvolume, and path, return the real
- path within the file system. subvolumegroup and subvolume may be empty
- strings when no subvolume is being used.
+ """Given a volume, subvolumegroup, and subvolume, return the real path
+ within the file system. subvolumegroup and subvolume may be empty strings
+ when no subvolume is being used.
"""
- path = path.lstrip('/')
- if not (subvolumegroup or subvolume):
- return f'/{path}'
cmd = {
'prefix': 'fs subvolume getpath',
'vol_name': volume,
@@ -81,8 +79,23 @@ class CephFSPathResolver:
ret, data, status = self._mgr.mon_command(cmd)
if ret != 0:
raise CephFSSubvolumeResolutionError(status)
- log.debug('Mapped subvolume to path: %r', data)
- return posixpath.join(data.strip(), path)
+ log.info('Mapped subvolume to path: %r', data)
+ return data.strip()
+
+ def resolve(
+ self, volume: str, subvolumegroup: str, subvolume: str, path: str
+ ) -> str:
+ """Given a volume, subvolumegroup, subvolume, and path, return the real
+ path within the file system. subvolumegroup and subvolume may be empty
+ strings when no subvolume is being used.
+ """
+ path = path.lstrip('/')
+ if not (subvolumegroup or subvolume):
+ return f'/{path}'
+ subvolume_path = self.resolve_subvolume_path(
+ volume, subvolumegroup, subvolume
+ )
+ return posixpath.join(subvolume_path, path)
def resolve_exists(
self, volume: str, subvolumegroup: str, subvolume: str, path: str
@@ -108,3 +121,86 @@ class CephFSPathResolver:
raise NotADirectoryError(volpath)
log.debug('Verified that %r exists in %r', volpath, volume)
return volpath
+
+
+class _TTLCache:
+ def __init__(self, maxsize: int = 512, ttl: float = 300.0) -> None:
+ self.cache: Dict[Tuple[str, str, str], Tuple[str, float]] = {}
+ self.maxsize: int = maxsize
+ self.ttl: float = ttl
+
+ def _evict(self) -> None:
+ """Evicts items that have expired or if cache size exceeds maxsize."""
+ current_time: float = time.monotonic()
+ keys_to_evict: list[Tuple[str, str, str]] = [
+ key
+ for key, (_, timestamp) in self.cache.items()
+ if current_time - timestamp > self.ttl
+ ]
+ for key in keys_to_evict:
+ del self.cache[key]
+
+ # Further evict if cache size exceeds maxsize
+ if len(self.cache) > self.maxsize:
+ for key in list(self.cache.keys())[
+ : len(self.cache) - self.maxsize
+ ]:
+ del self.cache[key]
+
+ def get(self, key: Tuple[str, str, str]) -> Optional[str]:
+ """Retrieve item from cache if it exists and is not expired."""
+ self._evict() # Ensure expired items are removed
+ if key in self.cache:
+ value, _ = self.cache[key]
+ return value
+ return None
+
+ def set(self, key: Tuple[str, str, str], value: str) -> None:
+ """Set item in cache, evicting expired or excess items."""
+ self._evict() # Ensure expired items are removed
+ self.cache[key] = (value, time.monotonic())
+
+ def clear(self) -> None:
+ """Clear all items in the cache."""
+ self.cache.clear()
+
+ def __len__(self) -> int:
+ """Return the number of items currently in the cache."""
+ return len(self.cache)
+
+
+class CachingCephFSPathResolver(CephFSPathResolver):
+ """
+ A subclass of CephFSPathResolver that adds caching to the resolve method
+ to improve performance by reducing redundant path resolutions.
+
+ This implementation uses a TTL (Time-To-Live) cache rather than an LRU (Least
+ Recently Used) cache. The TTL cache is preferred in this scenario because
+ the validity of cached paths is time-sensitive, and we want to ensure that
+ paths are refreshed after a certain period regardless of access frequency.
+ Rlock can be used to synchronize access to the cache, but that is something
+ not required for now & can be later tested.
+ """
+
+ def __init__(
+ self, mgr: Module_T, *, client: Optional[CephfsClient] = None
+ ) -> None:
+ super().__init__(mgr, client=client)
+ # Initialize a TTL cache.
+ self._cache = _TTLCache(maxsize=512, ttl=5)
+
+ def resolve_subvolume_path(
+ self, volume: str, subvolumegroup: str, subvolume: str
+ ) -> str:
+ cache_key = (volume, subvolumegroup, subvolume)
+ cached_path = self._cache.get(cache_key)
+ if cached_path:
+ log.debug("Cache hit for key: %r", cache_key)
+ return cached_path
+
+ log.debug("Cache miss for key: %r", cache_key)
+ resolved_path = super().resolve_subvolume_path(
+ volume, subvolumegroup, subvolume
+ )
+ self._cache.set(cache_key, resolved_path)
+ return resolved_path
diff --git a/src/pybind/mgr/smb/handler.py b/src/pybind/mgr/smb/handler.py
index 84702e72f78..f230d7952d7 100644
--- a/src/pybind/mgr/smb/handler.py
+++ b/src/pybind/mgr/smb/handler.py
@@ -411,7 +411,7 @@ class ClusterConfigHandler:
for cluster_id in self.cluster_ids():
if (resources.Cluster, cluster_id) in matcher:
out.append(self._cluster_entry(cluster_id).get_cluster())
- for share_id in cluster_shares[cluster_id]:
+ for share_id in cluster_shares.get(cluster_id, []):
if (resources.Share, cluster_id, share_id) in matcher:
out.append(
self._share_entry(
diff --git a/src/pybind/mgr/smb/module.py b/src/pybind/mgr/smb/module.py
index 70c97d1a975..43ad681769a 100644
--- a/src/pybind/mgr/smb/module.py
+++ b/src/pybind/mgr/smb/module.py
@@ -52,7 +52,7 @@ class Module(orchestrator.OrchestratorClientMixin, MgrModule):
self._public_store = (
public_store or rados_store.RADOSConfigStore.init(self)
)
- path_resolver = path_resolver or fs.CephFSPathResolver(self)
+ path_resolver = path_resolver or fs.CachingCephFSPathResolver(self)
# Why the honk is the cast needed but path_resolver doesn't need it??
# Sometimes mypy drives me batty.
authorizer = cast(
diff --git a/src/pybind/mgr/smb/tests/test_fs.py b/src/pybind/mgr/smb/tests/test_fs.py
index 7af20eb6265..5653ccfd081 100644
--- a/src/pybind/mgr/smb/tests/test_fs.py
+++ b/src/pybind/mgr/smb/tests/test_fs.py
@@ -1,8 +1,11 @@
+import time
+import unittest
from unittest import mock
import pytest
import smb.fs
+from smb.fs import _TTLCache
def test_mocked_fs_authorizer():
@@ -65,3 +68,108 @@ def test_mocked_fs_path_resolver(monkeypatch):
)
with pytest.raises(FileNotFoundError):
fspr.resolve_exists('cephfs', 'alpha', 'beta', '/zowie')
+
+
+class TestTTLCache(unittest.TestCase):
+ def setUp(self):
+ self.cache = _TTLCache(
+ ttl=1, maxsize=3
+ ) # Short TTL and small size for testing
+
+ def test_cache_set_and_get(self):
+ self.cache.set(('key1', 'key2', 'key3'), ('value1', 'val', 'test'))
+ self.assertEqual(
+ self.cache.get(('key1', 'key2', 'key3')),
+ ('value1', 'val', 'test'),
+ )
+
+ def test_cache_expiry(self):
+ self.cache.set(('key1', 'key2', 'key3'), ('value1', 'val', 'test'))
+ time.sleep(1.5) # Wait for the TTL to expire
+ self.assertIsNone(self.cache.get(('key1', 'key2', 'key3')))
+
+ def test_cache_eviction(self):
+ # Fill the cache to maxsize
+ self.cache.set(('key1', 'key2', 'key3'), ('value1', 'val', 'test'))
+ self.cache.set(('key4', 'key5', 'key6'), ('value2', 'val', 'test'))
+ self.cache.set(('key7', 'key8', 'key9'), ('value3', 'val', 'test'))
+
+ # Add another entry to trigger eviction of the oldest
+ self.cache.set(('key10', 'key11', 'key12'), ('value4', 'val', 'test'))
+
+ # Ensure oldest entry is evicted
+ self.assertIsNone(self.cache.get(('key1', 'key2', 'key3')))
+
+ # Ensure other entries are present
+ self.assertEqual(
+ self.cache.get(('key4', 'key5', 'key6')),
+ ('value2', 'val', 'test'),
+ )
+ self.assertEqual(
+ self.cache.get(('key7', 'key8', 'key9')),
+ ('value3', 'val', 'test'),
+ )
+ self.assertEqual(
+ self.cache.get(('key10', 'key11', 'key12')),
+ ('value4', 'val', 'test'),
+ )
+
+ def test_cache_clear(self):
+ self.cache.set(('key1', 'key2', 'key3'), ('value1', 'val', 'test'))
+ self.cache.clear()
+ self.assertIsNone(self.cache.get(('key1', 'key2', 'key3')))
+
+
+def test_caching_fs_path_resolver(monkeypatch):
+ monkeypatch.setattr('cephfs.ObjectNotFound', KeyError)
+
+ def mmcmd(cmd):
+ if cmd['prefix'] == 'fs subvolume getpath':
+ if (
+ cmd['vol_name'] == 'cached_cephfs'
+ and cmd['sub_name'] == 'cached_beta'
+ ):
+ return 0, '/volumes/cool/path/f00d-600d', ''
+ return -5, '', 'cached_eek'
+
+ m = mock.MagicMock()
+ m.mon_command.side_effect = mmcmd
+
+ fspr = smb.fs.CachingCephFSPathResolver(m, client=m)
+
+ # Resolve a path (cache miss)
+ path = fspr.resolve(
+ 'cached_cephfs', 'cached_alpha', 'cached_beta', '/zowie'
+ )
+ assert path == '/volumes/cool/path/f00d-600d/zowie'
+ assert len(fspr._cache) == 1
+ assert m.mon_command.call_count == 1
+
+ # Resolve the same path again (cache hit)
+ path = fspr.resolve(
+ 'cached_cephfs', 'cached_alpha', 'cached_beta', '/zowie'
+ )
+ assert path == '/volumes/cool/path/f00d-600d/zowie'
+
+ # Ensure cache size remains the same
+ assert len(fspr._cache) == 1
+ assert m.mon_command.call_count == 1
+
+ path = fspr.resolve('cached_cephfs', '', '', '/zowie')
+ assert path == '/zowie'
+
+ # If subvolume is empty cache size should remain the same
+ assert len(fspr._cache) == 1
+ assert m.mon_command.call_count == 1
+
+ # Clear cache and validate
+ fspr._cache.clear()
+ assert len(fspr._cache) == 0
+
+ # Re-resolve to repopulate cache
+ path = fspr.resolve(
+ 'cached_cephfs', 'cached_alpha', 'cached_beta', '/zowie'
+ )
+ assert path == '/volumes/cool/path/f00d-600d/zowie'
+ assert len(fspr._cache) == 1
+ assert m.mon_command.call_count == 2
diff --git a/src/pybind/mgr/smb/tests/test_smb.py b/src/pybind/mgr/smb/tests/test_smb.py
index 4ee55e0aa90..8943123f9d1 100644
--- a/src/pybind/mgr/smb/tests/test_smb.py
+++ b/src/pybind/mgr/smb/tests/test_smb.py
@@ -737,3 +737,58 @@ def test_show_invalid_input(tmodule):
_example_cfg_1(tmodule)
with pytest.raises(smb.cli.InvalidInputValue):
tmodule.show(['ceph.smb.export'])
+
+
+def test_show_cluster_without_shares(tmodule):
+ # this cluster will have no shares associated with it
+ tmodule._internal_store.overwrite(
+ {
+ 'clusters.foo': {
+ 'resource_type': 'ceph.smb.cluster',
+ 'cluster_id': 'foo',
+ 'auth_mode': 'active-directory',
+ 'intent': 'present',
+ 'domain_settings': {
+ 'realm': 'dom1.example.com',
+ 'join_sources': [
+ {
+ 'source_type': 'resource',
+ 'ref': 'foo',
+ }
+ ],
+ },
+ },
+ 'join_auths.foo': {
+ 'resource_type': 'ceph.smb.join.auth',
+ 'auth_id': 'foo',
+ 'intent': 'present',
+ 'auth': {
+ 'username': 'testadmin',
+ 'password': 'Passw0rd',
+ },
+ },
+ }
+ )
+
+ res, body, status = tmodule.show.command(['ceph.smb.cluster.foo'])
+ assert res == 0
+ assert (
+ body.strip()
+ == """
+{
+ "resource_type": "ceph.smb.cluster",
+ "cluster_id": "foo",
+ "auth_mode": "active-directory",
+ "intent": "present",
+ "domain_settings": {
+ "realm": "dom1.example.com",
+ "join_sources": [
+ {
+ "source_type": "resource",
+ "ref": "foo"
+ }
+ ]
+ }
+}
+ """.strip()
+ )