Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

mgr/cephadm: fix rgw spec migration with simple specs #52301

Merged
merged 1 commit into from
Jul 7, 2023
+65 −32
Merged

mgr/cephadm: fix rgw spec migration with simple specs #52301

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions qa/suites/orch/cephadm/upgrade/3-upgrade/simple.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ tasks:
- radosgw-admin zone create --rgw-zonegroup=default --rgw-zone=z --master --default
- radosgw-admin period update --rgw-realm=r --commit
- ceph orch apply rgw foo --realm r --zone z --placement=2 --port=8000
# simple rgw spec (will have no "spec" field) to make sure that works with rgw spec migration
- ceph orch apply rgw smpl
# setup iscsi
- ceph osd pool create foo
- rbd pool init foo
Expand Down
5 changes: 5 additions & 0 deletions src/pybind/mgr/cephadm/migrations.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,11 @@ def migrate_rgw_spec(self, spec: Dict[Any, Any]) -> Optional[RGWSpec]:
return RGWSpec.from_json(new_spec)

def rgw_spec_needs_migration(self, spec: Dict[Any, Any]) -> bool:
if 'spec' not in spec:
# if users allowed cephadm to set up most of the
# attributes, it's possible there is no "spec" section
# inside the spec. In that case, no migration is needed
return False
return 'rgw_frontend_type' in spec['spec'] \
and spec['spec']['rgw_frontend_type'] is not None \
and spec['spec']['rgw_frontend_type'].strip() not in ['beast', 'civetweb']
Expand Down
90 changes: 58 additions & 32 deletions src/pybind/mgr/cephadm/tests/test_migration.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import json
import pytest

from ceph.deployment.service_spec import PlacementSpec, ServiceSpec, HostPlacementSpec
from ceph.utils import datetime_to_str, datetime_now
Expand Down Expand Up @@ -259,26 +260,43 @@ def test_migrate_set_sane_value(cephadm_module: CephadmOrchestrator):
assert cephadm_module.migration_current == 0


@pytest.mark.parametrize(
"rgw_spec_store_entry, should_migrate",
[
({
'spec': {
'service_type': 'rgw',
'service_name': 'rgw.foo',
'service_id': 'foo',
'placement': {
'hosts': ['host1']
},
'spec': {
'rgw_frontend_type': 'beast tcp_nodelay=1 request_timeout_ms=65000 rgw_thread_pool_size=512',
'rgw_frontend_port': '5000',
},
},
'created': datetime_to_str(datetime_now()),
}, True),
({
'spec': {
'service_type': 'rgw',
'service_name': 'rgw.foo',
'service_id': 'foo',
'placement': {
'hosts': ['host1']
},
},
'created': datetime_to_str(datetime_now()),
}, False),
]
)
@mock.patch("cephadm.serve.CephadmServe._run_cephadm", _run_cephadm('[]'))
def test_migrate_rgw_spec(cephadm_module: CephadmOrchestrator):
def test_migrate_rgw_spec(cephadm_module: CephadmOrchestrator, rgw_spec_store_entry, should_migrate):
with with_host(cephadm_module, 'host1'):
cephadm_module.set_store(
SPEC_STORE_PREFIX + 'rgw',
json.dumps({
'spec': {
'service_type': 'rgw',
'service_name': 'rgw.foo',
'service_id': 'foo',
'placement': {
'hosts': ['host1']
},
'spec': {
'rgw_frontend_type': 'beast tcp_nodelay=1 request_timeout_ms=65000 rgw_thread_pool_size=512',
'rgw_frontend_port': '5000',
},
},
'created': datetime_to_str(datetime_now()),
}, sort_keys=True),
json.dumps(rgw_spec_store_entry, sort_keys=True),
)

# make sure rgw_migration_queue is populated accordingly
Expand All @@ -296,19 +314,27 @@ def test_migrate_rgw_spec(cephadm_module: CephadmOrchestrator):
cephadm_module.migration.migrate()
assert cephadm_module.migration_current == LAST_MIGRATION

# make sure the spec has been migrated and the the param=value entries
# that were part of the rgw_frontend_type are now in the new
# 'rgw_frontend_extra_args' list
assert 'rgw.foo' in cephadm_module.spec_store.all_specs
rgw_spec = cephadm_module.spec_store.all_specs['rgw.foo']
assert dict(rgw_spec.to_json()) == {'service_type': 'rgw',
'service_id': 'foo',
'service_name': 'rgw.foo',
'placement': {'hosts': ['host1']},
'spec': {
'rgw_frontend_extra_args': ['tcp_nodelay=1',
'request_timeout_ms=65000',
'rgw_thread_pool_size=512'],
'rgw_frontend_port': '5000',
'rgw_frontend_type': 'beast',
}}
if should_migrate:
# make sure the spec has been migrated and the the param=value entries
# that were part of the rgw_frontend_type are now in the new
# 'rgw_frontend_extra_args' list
assert 'rgw.foo' in cephadm_module.spec_store.all_specs
rgw_spec = cephadm_module.spec_store.all_specs['rgw.foo']
assert dict(rgw_spec.to_json()) == {'service_type': 'rgw',
'service_id': 'foo',
'service_name': 'rgw.foo',
'placement': {'hosts': ['host1']},
'spec': {
'rgw_frontend_extra_args': ['tcp_nodelay=1',
'request_timeout_ms=65000',
'rgw_thread_pool_size=512'],
'rgw_frontend_port': '5000',
'rgw_frontend_type': 'beast',
}}
else:
# in a real environment, we still expect the spec to be there,
# just untouched by the migration. For this test specifically
# though, the spec will only have ended up in the spec store
# if it was migrated, so we can use this to test the spec
# was untouched
assert 'rgw.foo' not in cephadm_module.spec_store.all_specs