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

cephadm: quick fix in upgrade.py #40529

Closed
wants to merge 1 commit into from

Conversation

guits
Copy link
Contributor

@guits guits commented Mar 31, 2021

I went to an issue when trying to upgrade in a RHCS/downstream context
where target_release.split('.') returns something like: 16.1.0-1084.el8cp

there's a difference between upstream and dowstream release of Ceph
since upstream it would return a pattern with 3 digits like :
16.1.0-1325-geb5d7a86

With a downstream release, the upgrade process fails and throws the following error:

Mar 31 08:44:41 ceph-vasi-node2-mon-mgr conmon[346502]:   File "/usr/share/ceph/mgr/cephadm/module.py", line 462, in serve
Mar 31 08:44:41 ceph-vasi-node2-mon-mgr conmon[346502]:     serve.serve()
Mar 31 08:44:41 ceph-vasi-node2-mon-mgr conmon[346502]:   File "/usr/share/ceph/mgr/cephadm/serve.py", line 92, in serve
Mar 31 08:44:41 ceph-vasi-node2-mon-mgr conmon[346502]:     if self.mgr.upgrade.continue_upgrade():
Mar 31 08:44:41 ceph-vasi-node2-mon-mgr conmon[346502]:   File "/usr/share/ceph/mgr/cephadm/upgrade.py", line 239, in continue_upgrade
Mar 31 08:44:41 ceph-vasi-node2-mon-mgr conmon[346502]:     self._do_upgrade()
Mar 31 08:44:41 ceph-vasi-node2-mon-mgr conmon[346502]:   File "/usr/share/ceph/mgr/cephadm/upgrade.py", line 438, in _do_upgrade
Mar 31 08:44:41 ceph-vasi-node2-mon-mgr conmon[346502]:     target_major, target_minor, target_patch = target_version.split('.')
Mar 31 08:44:41 ceph-vasi-node2-mon-mgr conmon[346502]: ValueError: too many values to unpack (expected 3)

The current code assumes target_version.split('.') will always return
a list of 3 elements, which is false in a downstream deployment.

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1945260
Fixes: https://tracker.ceph.com/issues/50085

Signed-off-by: Guillaume Abrioux gabrioux@redhat.com

I went to an issue when trying to upgrade in a RHCS/downstream context
where `target_release.split('.')` returns something like: `16.1.0-1084.el8cp`

there's a difference between upstream and dowstream release of Ceph
since upstream it would return a pattern with 3 digits like :
`16.1.0-1325-geb5d7a86`

With a downstream release, the upgrade process fails and throws the following error:

```
Mar 31 08:44:41 ceph-vasi-node2-mon-mgr conmon[346502]:   File "/usr/share/ceph/mgr/cephadm/module.py", line 462, in serve
Mar 31 08:44:41 ceph-vasi-node2-mon-mgr conmon[346502]:     serve.serve()
Mar 31 08:44:41 ceph-vasi-node2-mon-mgr conmon[346502]:   File "/usr/share/ceph/mgr/cephadm/serve.py", line 92, in serve
Mar 31 08:44:41 ceph-vasi-node2-mon-mgr conmon[346502]:     if self.mgr.upgrade.continue_upgrade():
Mar 31 08:44:41 ceph-vasi-node2-mon-mgr conmon[346502]:   File "/usr/share/ceph/mgr/cephadm/upgrade.py", line 239, in continue_upgrade
Mar 31 08:44:41 ceph-vasi-node2-mon-mgr conmon[346502]:     self._do_upgrade()
Mar 31 08:44:41 ceph-vasi-node2-mon-mgr conmon[346502]:   File "/usr/share/ceph/mgr/cephadm/upgrade.py", line 438, in _do_upgrade
Mar 31 08:44:41 ceph-vasi-node2-mon-mgr conmon[346502]:     target_major, target_minor, target_patch = target_version.split('.')
Mar 31 08:44:41 ceph-vasi-node2-mon-mgr conmon[346502]: ValueError: too many values to unpack (expected 3)
```

The current code assumes `target_version.split('.')` will always return
a list of 3 elements, which is false in a downstream deployment.

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1945260
Fixes: https://tracker.ceph.com/issues/50085

Signed-off-by: Guillaume Abrioux <gabrioux@redhat.com>
@guits guits requested a review from a team as a code owner March 31, 2021 14:56
@adk3798
Copy link
Contributor

adk3798 commented Mar 31, 2021

@guits I think this is the same thing being fixed by #40478
https://tracker.ceph.com/issues/50043
https://bugzilla.redhat.com/show_bug.cgi?id=1944978

If you agree maybe you can leave a review there on whether you like that fix or prefer this one. Also, as you can see in the linked PR, I think the check in _check_target_version also needs adjustment and will subsequently cause upgrade to fail after the check you're fixing here works.

try:
(major, minor, patch) = version.split('.')
assert int(minor) >= 0
# patch might be a number or {number}-g{sha1}
except ValueError:
return 'version must be in the form X.Y.Z (e.g., 15.2.3)'

@guits
Copy link
Contributor Author

guits commented Mar 31, 2021

hi @adk3798

thanks for pointing out this, it looks like I've missed this in existing BZs/trackers 🤷‍♂️

feel free to close this one in favor of #40478 if you think it's a better approach.

Thanks!

@adk3798 adk3798 closed this Mar 31, 2021
@guits guits deleted the guits-fix_cephadm_upgrade branch March 31, 2021 19:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants