Skip to content

mgr/cephadm: Also make ssh._reset_con async#42919

Merged
tchaikov merged 4 commits intoceph:masterfrom
sebastian-philipp:cephadm-async-close-conn
Sep 6, 2021
Merged

mgr/cephadm: Also make ssh._reset_con async#42919
tchaikov merged 4 commits intoceph:masterfrom
sebastian-philipp:cephadm-async-close-conn

Conversation

@sebastian-philipp
Copy link
Contributor

Otherwise conn.close() might raise: RuntimeError: Non-thread-safe operation invoked on an event loop other than the current one

Fixes: https://tracker.ceph.com/issues/52407
Signed-off-by: Sebastian Wagner sewagner@redhat.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 cephadm
  • jenkins test api
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox

Otherwise `conn.close()` might raise: `RuntimeError:
Non-thread-safe operation invoked on an event loop
other than the current one`

Fixes: https://tracker.ceph.com/issues/52407
Signed-off-by: Sebastian Wagner <sewagner@redhat.com>
Also increase mypy coverage by avoiding kwargs

Signed-off-by: Sebastian Wagner <sewagner@redhat.com>
Signed-off-by: Sebastian Wagner <sewagner@redhat.com>
Signed-off-by: Sebastian Wagner <sewagner@redhat.com>
Copy link
Member

@nizamial09 nizamial09 left a comment

Choose a reason for hiding this comment

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

Thank you @sebastian-philipp for the quick fix

def reset_con(self, host: str) -> None:
self.mgr.event_loop.get_result(self._reset_con(host))

def _reset_cons(self) -> None:
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can probably do the same thing to _reset_cons and _reconfig_ssh. It looks like if _reset_cons is called and there are hosts to remove, it will result in the same error as _reset_con was causing before

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.

6 participants