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.

Sign up for GitHub

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

Jump to bottom

mgr/cephadm: staggered upgrade #45786

Merged
merged 8 commits into from May 20, 2022
Merged

mgr/cephadm: staggered upgrade #45786

merged 8 commits into from May 20, 2022

Conversation

adk3798
Copy link
Contributor

@adk3798 adk3798 commented Apr 5, 2022 •
edited

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

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

Still needs docs and tests.

Additionally, an open question is how far do we want to enforce upgrade ordering? The whole list? Only up until mds? Just mgr then mon? I'm not sure, really.

For reference, upgrade order is: mgr -> mon -> crash -> osd -> mds -> rgw -> rbd-mirror -> cephfs-mirror -> iscsi -> nfs

Checklist

  • Tracker (select at least one)
    • References tracker ticket
    • Very recent bug; references commit where it was introduced
    • New feature (ticket optional)
    • Doc update (no ticket needed)
    • Code cleanup (no ticket needed)
  • Component impact
    • Affects Dashboard, opened tracker ticket
    • Affects Orchestrator, opened tracker ticket
    • No impact that needs to be tracked
  • Documentation (select at least one)
    • Updates relevant documentation
    • No doc update is appropriate
  • Tests (select at least one)
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
  • jenkins test windows

@adk3798 adk3798 requested a review from a team as a code owner April 5, 2022 21:50
@vumrao
Copy link
Contributor

vumrao commented Apr 11, 2022

YAY 👍

@github-actions
Copy link

This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved

@adk3798
Copy link
Contributor Author

adk3798 commented Apr 13, 2022 •
edited

Added unit test for staggered upgrade param validation and a teuthology test for staggered upgrade. Docs still to do.

@adk3798 adk3798 requested a review from a team as a code owner April 19, 2022 17:22
@adk3798
Copy link
Contributor Author

adk3798 commented Apr 19, 2022

jenkins render docs

@adk3798
Copy link
Contributor Author

adk3798 commented Apr 19, 2022

jenkins render docs

@adk3798 adk3798 force-pushed the staggered-upgrade branch 3 times, most recently from 6937904 to d190af7 Compare April 21, 2022 17:28
anthonyeleven
anthonyeleven requested changes Apr 21, 2022
View reviewed changes
Copy link
Contributor

@anthonyeleven anthonyeleven left a comment

Choose a reason for hiding this comment

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

Nice. Upgrades and command line syntax are areas where it's really easy to be confused, so I love seeing detailed descriptions and examples like this. Here are some nitpicky doc suggestions.

doc/cephadm/upgrade.rst Outdated Show resolved Hide resolved
doc/cephadm/upgrade.rst Outdated Show resolved Hide resolved
doc/cephadm/upgrade.rst Outdated Show resolved Hide resolved
doc/cephadm/upgrade.rst Show resolved Hide resolved
doc/cephadm/upgrade.rst Show resolved Hide resolved
doc/cephadm/upgrade.rst Outdated Show resolved Hide resolved
doc/cephadm/upgrade.rst Outdated Show resolved Hide resolved
doc/cephadm/upgrade.rst Show resolved Hide resolved
doc/cephadm/upgrade.rst Show resolved Hide resolved
doc/cephadm/upgrade.rst Outdated Show resolved Hide resolved
@adk3798 adk3798 force-pushed the staggered-upgrade branch 2 times, most recently from 66df615 to 226108c Compare April 22, 2022 03:38
sebastian-philipp
sebastian-philipp reviewed Apr 26, 2022
View reviewed changes
src/pybind/mgr/cephadm/upgrade.py
Comment on lines +340 to +380
# what we need to do here is build a list of daemons that must already be upgraded
# in order for the user's selection of daemons to upgrade to be valid. for example,
# if they say --daemon-types 'osd,mds' but mons have not been upgraded, we block.
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️

@github-actions
Copy link

This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved

@adk3798 adk3798 force-pushed the staggered-upgrade branch 2 times, most recently from 903d9c5 to 406dffb Compare May 14, 2022 23:06
@adk3798
Copy link
Contributor Author

adk3798 commented May 14, 2022

@rkachach I think I've responded to or addressed all of your review comments. Let me know if there's anything else or questions on what I said,

@adk3798 adk3798 requested a review from rkachach May 14, 2022 23:07
@adk3798
Copy link
Contributor Author

adk3798 commented May 15, 2022

jenkins test dashboard cephadm

@adk3798
Copy link
Contributor Author

adk3798 commented May 17, 2022

jenkins test dashboard cephadm

@adk3798
Copy link
Contributor Author

adk3798 commented May 17, 2022

ping @anthonyeleven I think this PR is getting close to being fully ready if you have a chance to re-review the docs

@anthonyeleven
Copy link
Contributor

LGTM for docs, with the above minor suggestions.

@adk3798
mgr/cephadm: split _do_upgrade into sub functions
7b83c51 
This function was around 500 lines and difficult to work
with. Splitting it into sub functions should hopefully make
it a bit easier to understand and make changes to.

Signed-off-by: Adam King <adking@redhat.com>
@adk3798
mgr/cephadm: add new args and validation for staggered upgrade
e6b0fe0 
Signed-off-by: Adam King <adking@redhat.com>
@adk3798
mgr/cephadm: make UpgradeState from_json a bit safer
aeaa0b5 
This way, for downgrades to whatever versions
this lands in onward, having added new parameters to
UpgradeState shouldn't break anything. Can't do much
about downgrades to older versions from this one
but this should help in the future.

Signed-off-by: Adam King <adking@redhat.com>
@adk3798
mgr/cephadm: make use of new upgrade control parameters
c1f3497 
Fixes: https://tracker.ceph.com/issues/54135

Signed-off-by: Adam King <adking@redhat.com>
@adk3798
mgr/cephadm: autopep8 changes
62a06c4 
Signed-off-by: Adam King <adking@redhat.com>
@adk3798
qa/suites/orch/cephadm: staggered upgrade test
791e1d2 
Signed-off-by: Adam King <adking@redhat.com>
@adk3798
mgr/cephadm: unit test for staggered upgrade param validation
0a46fcb 
Signed-off-by: Adam King <adking@redhat.com>
@adk3798
doc/cephadm: staggered upgrade docs
6a68def 
Signed-off-by: Adam King <adking@redhat.com>
@adk3798
Copy link
Contributor Author

adk3798 commented May 20, 2022

jenkins test dashboard cephadm

@adk3798
Copy link
Contributor Author

adk3798 commented May 20, 2022 •
edited

18 runs of new test for staggered upgrade and 2 runs of simple upgrade, all passed

https://pulpito.ceph.com/adking-2022-05-20_04:04:42-orch:cephadm:upgrade-wip-adk-testing-2022-05-19-2005-distro-basic-smithi/
https://pulpito.ceph.com/adking-2022-05-20_05:14:33-orch:cephadm:upgrade-wip-adk-testing-2022-05-19-2005-distro-basic-smithi/

Additional full orch run including a few more instances of upgrade tests, all passed

https://pulpito.ceph.com/adking-2022-05-20_15:32:09-orch:cephadm-wip-adk-testing-2022-05-19-2005-distro-basic-smithi/

@adk3798 adk3798 merged commit 54cdc1d into ceph:master May 20, 2022
This was referenced May 21, 2022
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sebastian-philipp sebastian-philipp sebastian-philipp left review comments

@anthonyeleven anthonyeleven anthonyeleven approved these changes

@rkachach rkachach rkachach approved these changes

Assignees
No one assigned
Labels
cephadm documentation monitoring orchestrator pybind wip-adk-testing
Projects
None yet
Milestone
No milestone
5 participants
@adk3798 @vumrao @anthonyeleven @sebastian-philipp @rkachach