Skip to content

mgr/cephadm: guard _check_daemons#36349

Merged
sebastian-philipp merged 5 commits intoceph:masterfrom
sebastian-philipp:cephadm-check_deamons_except
Aug 5, 2020
Merged

mgr/cephadm: guard _check_daemons#36349
sebastian-philipp merged 5 commits intoceph:masterfrom
sebastian-philipp:cephadm-check_deamons_except

Conversation

@sebastian-philipp
Copy link
Contributor

Continue with other daemons, if one fails

Fixes: https://tracker.ceph.com/issues/46748
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

@sebastian-philipp sebastian-philipp requested a review from a team as a code owner July 29, 2020 12:59
@sebastian-philipp sebastian-philipp force-pushed the cephadm-check_deamons_except branch 2 times, most recently from 1e37a1a to 7f760d9 Compare July 30, 2020 12:30
@sebastian-philipp sebastian-philipp added the wip-swagner-testing My Teuthology tests label Jul 30, 2020
@sebastian-philipp
Copy link
Contributor Author

@sebastian-philipp sebastian-philipp force-pushed the cephadm-check_deamons_except branch from 7f760d9 to 90df6bf Compare July 31, 2020 11:20
@sebastian-philipp sebastian-philipp added needs-review and removed wip-swagner-testing My Teuthology tests labels Jul 31, 2020
@sebastian-philipp sebastian-philipp force-pushed the cephadm-check_deamons_except branch from 90df6bf to ef34571 Compare July 31, 2020 11:31
Copy link

@Devp00l Devp00l left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works great :)

@sebastian-philipp
Copy link
Contributor Author

@sebastian-philipp
Copy link
Contributor Author

jenkins test make check

@sebastian-philipp
Copy link
Contributor Author

=================================== FAILURES ===================================
______________________ TestCephadm.test_daemon_check_post ______________________

self = <cephadm.tests.test_cephadm.TestCephadm object at 0x7f0cc46c65c0>
cephadm_module = <cephadm.module.CephadmOrchestrator object at 0x7f0cc469f438>

    @mock.patch("cephadm.module.CephadmOrchestrator._run_cephadm", _run_cephadm('{}'))
    def test_daemon_check_post(self, cephadm_module: CephadmOrchestrator):
        with with_host(cephadm_module, 'test'):
            with with_service(cephadm_module, ServiceSpec(service_type='grafana'), CephadmOrchestrator.apply_grafana, 'test'):
    
                # Make sure, _check_daemons does a redeploy due to monmap change:
                cephadm_module._store['_ceph_get/mon_map'] = {
                    'modified': datetime.datetime.utcnow().strftime(CEPH_DATEFMT),
                    'fsid': 'foobar',
                }
                cephadm_module.notify('mon_map', None)
    
                with mock.patch("cephadm.module.CephadmOrchestrator.mon_command") as _mon_cmd:
    
                    cephadm_module._check_daemons()
>                   _mon_cmd.assert_any_call({'prefix': 'dashboard set-grafana-api-url', 'value': 'https://test:3000'})

cephadm/tests/test_cephadm.py:263: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <MagicMock name='mon_command' id='139692269904952'>
args = ({'prefix': 'dashboard set-grafana-api-url', 'value': 'https://test:3000'},)
kwargs = {}
expected = (({'prefix': 'dashboard set-grafana-api-url', 'value': 'https://test:3000'},), {})
actual = [], cause = None
expected_string = "mon_command({'prefix': 'dashboard set-grafana-api-url', 'value': 'https://test:3000'})"

    def assert_any_call(self, *args, **kwargs):
        """assert the mock has been called with the specified arguments.
    
        The assert passes if the mock has *ever* been called, unlike
        `assert_called_with` and `assert_called_once_with` that only pass if
        the call is the most recent one."""
        expected = self._call_matcher((args, kwargs))
        actual = [self._call_matcher(c) for c in self.call_args_list]
        if expected not in actual:
            cause = expected if isinstance(expected, Exception) else None
            expected_string = self._format_mock_call_signature(args, kwargs)
            raise AssertionError(
                '%s call not found' % expected_string
>           ) from cause
E           AssertionError: mon_command({'prefix': 'dashboard set-grafana-api-url', 'value': 'https://test:3000'}) call not found

/usr/lib/python3.6/unittest/mock.py:876: AssertionError

Continue with other daemons, if one fails

Fixes: https://tracker.ceph.com/issues/46748
Signed-off-by: Sebastian Wagner <sebastian.wagner@suse.com>
Into:

* `test_list_daemons`
* `test_daemon_action` (now without listing daemons)

Signed-off-by: Sebastian Wagner <sebastian.wagner@suse.com>
Signed-off-by: Sebastian Wagner <sebastian.wagner@suse.com>
We can't run daemon_check_post for the type, if a single daemon failed.
Mainly cause `daemon_check_post` is run by service type.

Signed-off-by: Sebastian Wagner <sebastian.wagner@suse.com>
@sebastian-philipp sebastian-philipp force-pushed the cephadm-check_deamons_except branch from ef34571 to afdc58b Compare August 5, 2020 10:59
@sebastian-philipp sebastian-philipp added wip-swagner-testing My Teuthology tests and removed needs-rebase labels Aug 5, 2020
Signed-off-by: Sebastian Wagner <sebastian.wagner@suse.com>
@sebastian-philipp sebastian-philipp force-pushed the cephadm-check_deamons_except branch from afdc58b to 7d3deb2 Compare August 5, 2020 13:31
@sebastian-philipp
Copy link
Contributor Author

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants