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.

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

mgr/cephadm: add info to 'ceph orch upgrade status' in cephadm #39880

Merged
merged 1 commit into from
Mar 17, 2021

Conversation

adk3798
Copy link
Contributor

@adk3798 adk3798 commented Mar 5, 2021

properly fills in 'service_complete' field, adds info messages to 'message' field
such as what daemon type is being upgraded or if we're pulling an image and adds
'progress' field that shows how many daemons have been upgraded so far.

Fixes: https://tracker.ceph.com/issues/49235

Signed-off-by: Adam King adking@redhat.com

"services_complete" field was already present in the output of the command but we never filled it in in cephadm.

"message" field also already existed but was only used for errors. Adding in extra info to say what the upgrade is currently doing. Either what daemon type is being upgraded or that we're pulling an image.

"progress" field is new. It prints out "x/y ceph daemons upgraded" where x is how many daemons we've upgraded so far and y is the total number of daemons we plan to upgrade (daemons whose type is in CEPH_UPGRADE_ORDER). There is already a progress bar the user can see using the "ceph -s" command but I've found it to be inconsistent (https://pastebin.com/raw/JsXkGPkq) so I felt like the orchestrator could use its own.

The idea is to give the user a single command that provides good information on the current state of the upgrade.

Sample output (2 mgr, 3 mon, 3 crash, 6 osd cluster):

image
image

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

@adk3798
Copy link
Contributor Author

adk3798 commented Mar 5, 2021

NOTE: I noticed when testing this, sometimes right after the mon daemons are upgraded, the container_image_name, container_image_digests and version fields will not be set in the DaemonDescription of some daemons yet when they're checked to see if the daemon needs to be upgraded. In that case the logs will show something like

2021-03-05T21:47:09.196885+0000 mgr.vm-01.cjaxne [INF] Upgrade: Checking mgr daemons
2021-03-05T21:47:09.197038+0000 mgr.vm-01.cjaxne [DBG] daemon mgr.vm-00.qnfsfo not correct (None, None, None)
2021-03-05T21:47:09.197150+0000 mgr.vm-01.cjaxne [DBG] daemon mgr.vm-01.cjaxne not correct (None, None, None)
2021-03-05T21:47:09.197236+0000 mgr.vm-01.cjaxne [INF] Upgrade: Need to upgrade myself (mgr.vm-01.cjaxne)
2021-03-05T21:47:09.215117+0000 mgr.vm-01.cjaxne [INF] Upgrade: It is presumed safe to stop ['mgr.vm-00.qnfsfo']

even though the mgr daemons have already been upgraded. In that case you could get output like
image

even though the mgr and mon daemons have already been upgraded. I'm not sure if the fields not being set is related to this change. It's not something I've looked for before. I'll look into it more later.

src/pybind/mgr/cephadm/upgrade.py Outdated Show resolved Hide resolved
@@ -25,6 +25,9 @@ def __init__(self,
target_version: Optional[str] = None,
error: Optional[str] = None,
paused: Optional[bool] = None,
services_complete: Optional[List[str]] = None,
progress_str: str = '',
Copy link
Contributor

Choose a reason for hiding this comment

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

we should query this from the progress module.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would this just give us the % complete being used for the output of 'ceph -s' right now?

Copy link
Contributor

Choose a reason for hiding this comment

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

this is already there!

def _update_upgrade_progress(self, progress: float) -> None:
if not self.upgrade_state:
assert False, 'No upgrade in progress'
if not self.upgrade_state.progress_id:
self.upgrade_state.progress_id = str(uuid.uuid4())
self._save_upgrade_state()
self.mgr.remote('progress', 'update', self.upgrade_state.progress_id,
ev_msg='Upgrade to %s' % self.target_image,
ev_progress=progress)

src/pybind/mgr/cephadm/upgrade.py Outdated Show resolved Hide resolved
@adk3798
Copy link
Contributor Author

adk3798 commented Mar 9, 2021

removed all the new fields from the UpgradeStae object. Calculating completed services and number of daemons upgraded when upgrade status is called instead.

@sebastian-philipp
Copy link
Contributor

can you add Fixes: https://tracker.ceph.com/issues/49235 to the commit and PR messge?

properly fills in 'services_complete' field, adds info messages to 'message' field
such as what daemon type is being upgraded or if we're pulling an image and adds
'progress' field that shows how many daemons have been upgraded so far.

Fixes: https://tracker.ceph.com/issues/49235

Signed-off-by: Adam King <adking@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants