Skip to content

mgr/cephadm: get_daemons_by_service should use dd.service_name#36388

Merged
sebastian-philipp merged 2 commits intoceph:masterfrom
sebastian-philipp:cephadm-inventory-get_daemons_by_service
Aug 4, 2020
Merged

mgr/cephadm: get_daemons_by_service should use dd.service_name#36388
sebastian-philipp merged 2 commits intoceph:masterfrom
sebastian-philipp:cephadm-inventory-get_daemons_by_service

Conversation

@sebastian-philipp
Copy link
Contributor

@sebastian-philipp sebastian-philipp commented Jul 31, 2020

daemon_name does not necessarily starts with the service_name.

Especially not for OSDs. See #35945

Signed-off-by: Sebastian Wagner sebastian.wagner@suse.com

Checklist

  • References tracker ticket
  • Updates documentation if necessary
  • Includes tests for new functionality or reproducer for bug

Show available Jenkins commands
  • jenkins retest this please
  • jenkins test classic perf
  • jenkins test crimson perf
  • jenkins test signed
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test dashboard backend
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox

daemon_name does not necessarily starts with the service_name.

Especially not for OSDs.

Signed-off-by: Sebastian Wagner <sebastian.wagner@suse.com>
Signed-off-by: Sebastian Wagner <sebastian.wagner@suse.com>
@sebastian-philipp
Copy link
Contributor Author

This btw also relates to

if d.matches_service(service_name):

which also still uses

def matches_service(self, service_name):
# type: (Optional[str]) -> bool
if service_name:
return self.name().startswith(service_name + '.')
return False

which is broken for OSDs: If you run ceph orch restart osd.myosdspec, I would expect this to restart all OSDs that was related to the OSD spec.

@sebastian-philipp sebastian-philipp added the wip-swagner-testing My Teuthology tests label Aug 4, 2020
@sebastian-philipp
Copy link
Contributor Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants