mgr/cephadm: retry after JSONDecodeError in wait_for_mgr_restart()

[fultonj]

'ceph mgr dump' does not always return valid JSON so cephadm
will throw an exception sometimes when applying a spec as per
the issue this PR closes. Add a try/except to catch a possible
JSONDecodeError and retry after sleeping.

Fixes: https://tracker.ceph.com/issues/49870
Signed-off-by: John Fulton [email protected]

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 api
• jenkins test docs
• jenkins render docs
• jenkins test ceph-volume all
• jenkins test ceph-volume tox

[sebastian-philipp]

This piece of code has a history:

• cephadm: use (new) 'mgr stat' instead of 'mgr dump' to check mgrmap epoch #39028 introduced the mgr stat command to avoid the truncated json output
• cephadm: rewrite call() with asyncio #39035 and cephadm: rewrite call() to be more robust #39031 rewrote call()

See also:

• https://tracker.ceph.com/issues/48993
• conn_sock: do not fail on EAGAIN containers/conmon#237
• podman exec may truncate stdout/stderr containers/podman#9096

But I think this is a good workaround for octopus.

[sebastian-philipp]

for backporting, I'd suggest to limit the amount of changes to existing code

def retry(func: Callable): 
    for sleep_secs in [1, 4, 4]:
        try:    
            return func() 

(and sleeping 25 seconds is too long)

[sebastian-philipp]

Suggested change

	out = cli(['mgr', 'stat'])
	out = retry(lambda: cli(['mgr', 'stat']))

[fultonj]

Thanks for the code review and please find this updated patch.

As per your suggestion, I used a new retry() function in order to minimize changes to the existing wait_for_mgr_restart() function. Because json.loads() is throwing the unhandled exception, not cli(), though I need to re-call cli() if a JSON exception is thrown, I had the retry() function call json.loads().

[fultonj]

       return json.loads(cli_func())

Should I add the above on line 3931 so that we call one more time and if the exception is thrown at least the error is identifcal so as to not introduce any new behavior?

[liewegas]

cephadm:3928:13: E722 do not use bare 'except'
cephadm:3929:73: E222 multiple spaces after operator
1     E222 multiple spaces after operator
1     E722 do not use bare 'except'

[fultonj]

cephadm:3928:13: E722 do not use bare 'except'
cephadm:3929:73: E222 multiple spaces after operator
1     E222 multiple spaces after operator
1     E722 do not use bare 'except'

Thanks, I should have pep8'd it before submitting. Fixed now.