mgr/cephadm: fix rgw spec migration with simple specs #52301
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
1 failure, 3 dead jobs
Nothing to block merging |
|
jenkins retest this please |
|
jenkins test api |
1 similar comment
|
jenkins test api |
|
from api tests failure doesn't look related to this PR imo |
|
jenkins test api |
The rgw spec migration code, intended to formalize the rgw_frontend_type spec option, doesn't work with simple specs i.e. service_type: rgw service_id: rgw.1 service_name: rgw.rgw.1 placement: label: rgw because the migration code assumes there will always be a "spec" section inside the spec. This is the case for more involved rgw specs such as service_type: rgw service_id: foo placement: label: rgw count_per_host: 2 spec: rgw_realm: myrealm rgw_zone: myzone rgw_frontend_type: "beast" rgw_frontend_port: 5000 which is what the migration is actually concerned about (verification of the rgw_frontend_type in these specs). In the case where the spec is more simple, we should just leave the spec alone and move on. Unfortunately the current code assumes the field will always be there and hits an unhandled KeyError when trying to migrate the more simple specs. This causes the cephadm module to crash shortly after starting an upgrade to a version that includes this migration and it's very difficult to find the root cause. This can be worked around by adding fields to the rgw spec before upgrade so the "spec" field exists in the spec and the migration works as intended. This commit fixes the migration in the simple case as well as adding testing for that case to both the unit tests and orch/cephadm teuthology upgrade tests Fixes: https://tracker.ceph.com/issues/61889 Signed-off-by: Adam King <adking@redhat.com>
adk3798
force-pushed
the
cephadm-fix-rgw-migration
branch
from
42e4a03
to
1860ef8
Compare
|
rebased to see if that would help the failures somehow |
|
jenkins test api |
1 similar comment
|
jenkins test api |
14 tasks
|
jenkins test api |
14 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The rgw spec migration code, intended to formalize the rgw_frontend_type spec option, doesn't work with simple specs i.e.
because the migration code assumes there will always be a "spec" section inside the spec. This is the case for more involved rgw specs such as
which is what the migration is actually concerned about (verification of the rgw_frontend_type in these specs).
In the case where the spec is more simple, we should
just leave the spec alone and move on. Unfortunately
the current code assumes the field will always be
there and hits an unhandled KeyError when trying to
migrate the more simple specs. This causes the
cephadm module to crash shortly after starting an
upgrade to a version that includes this migration
and it's very difficult to find the root cause. This
can be worked around by adding fields to the rgw
spec before upgrade so the "spec" field exists in
the spec and the migration works as intended.
This commit fixes the migration in the simple
case as well as adding testing for that case to
both the unit tests and orch/cephadm teuthology
upgrade tests
Fixes: https://tracker.ceph.com/issues/61889
Contribution Guidelines
To sign and title your commits, please refer to Submitting Patches to Ceph.
If you are submitting a fix for a stable branch (e.g. "pacific"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.
Checklist
Show available Jenkins commands
jenkins retest this pleasejenkins test classic perfjenkins test crimson perfjenkins test signedjenkins test make checkjenkins test make check arm64jenkins test submodulesjenkins test dashboardjenkins test dashboard cephadmjenkins test apijenkins test docsjenkins render docsjenkins test ceph-volume alljenkins test ceph-volume toxjenkins test windows