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

rgw: fix CephObjectStore failing with pre-existing pools #14772

Merged
merged 5 commits into from
Oct 3, 2024
Merged

rgw: fix CephObjectStore failing with pre-existing pools #14772

merged 5 commits into from
Oct 3, 2024

Conversation

jhoblitt
Copy link
Contributor

@jhoblitt jhoblitt commented Sep 25, 2024 •
edited
Loading

When attempting to use pre-existing (or cephBlockPool managed) pools and the cephObjectStore.spec.{metadataPool,dataPool} fields are not specified, rgw creation will fail with this error:

2024-09-25 21:00:17.744358 E | ceph-object-controller: failed to reconcile CephObjectStore "rook-ceph/test1". failed to create object store deployments: failed to create object pools: failed to create metadata pools: failed to create pool "test1.rgw.control": pool "test1.rgw.control" type is not defined as replicated or erasure coded

Issue resolved by this Pull Request:

Checklist:

  • Commit Message Formatting: Commit titles and messages follow guidelines in the developer guide.
  • Reviewed the developer guide on Submitting a Pull Request
  • Pending release notes updated with breaking and/or notable changes for the next minor release.
  • Documentation has been updated, if necessary.
  • Unit tests have been added, if necessary.
  • Integration tests have been added, if necessary.

@jhoblitt jhoblitt marked this pull request as draft September 25, 2024 21:32
@jhoblitt jhoblitt force-pushed the bugfix/cephobjectstore-existing-pools branch 6 times, most recently from 58ad2fe to 5198df1 Compare September 30, 2024 20:51
@jhoblitt jhoblitt marked this pull request as ready for review September 30, 2024 21:09
@jhoblitt
Copy link
Contributor Author

The canary test is only waiting for the CephObjectStore resource to become ready. My thinking is that is probably sufficient as rgw starting up with missing pool should probably be considered a bug in ceph/rgw and testing the rgw api isn't necessary.

The example has CephBlockPool.spec.parameters set which aren't necessary the canary test but I thought it might serve as a good example for humans as to why one might want to use CephBlockPools to directly manage the rgw pools. I have no objection to removing that field from the example.

travisn
travisn requested changes Oct 1, 2024
View reviewed changes
PendingReleaseNotes.md Outdated Show resolved Hide resolved
deploy/examples/rgw-with-cephblockpool.yaml Outdated Show resolved Hide resolved
deploy/examples/rgw-with-cephblockpool.yaml Outdated Show resolved Hide resolved
deploy/examples/rgw-with-cephblockpool.yaml Outdated Show resolved Hide resolved
deploy/examples/rgw-with-cephblockpool.yaml Outdated Show resolved Hide resolved
deploy/examples/rgw-with-cephblockpool.yaml Outdated Show resolved Hide resolved
@jhoblitt jhoblitt force-pushed the bugfix/cephobjectstore-existing-pools branch 13 times, most recently from a143058 to aae6436 Compare October 2, 2024 00:51
@jhoblitt
Copy link
Contributor Author

jhoblitt commented Oct 2, 2024

TestCephMultiClusterDeploySuite (v1.31.0) has failed 3 times in a row now but I don't understand changes to the canary test plumbing could have caused the integration test to start failing.

@jhoblitt jhoblitt requested review from BlaineEXE and travisn October 2, 2024 02:54
@jhoblitt
Copy link
Contributor Author

jhoblitt commented Oct 2, 2024

TestCephMultiClusterDeploySuite (v1.31.0) has failed 3 times in a row now but I don't understand changes to the canary test plumbing could have caused the integration test to start failing.

I was able to click retry until it passed. It failed in a different test each time.

@jhoblitt jhoblitt force-pushed the bugfix/cephobjectstore-existing-pools branch from aae6436 to cb63f42 Compare October 2, 2024 16:39
@jhoblitt
Copy link
Contributor Author

jhoblitt commented Oct 2, 2024

Repushed as I noticed that the -test suffix was missing in the "header" comments of the -test.yaml file.

BlaineEXE
BlaineEXE requested changes Oct 2, 2024
View reviewed changes
tests/scripts/github-action-helper.sh Outdated Show resolved Hide resolved
tests/scripts/github-action-helper.sh Outdated Show resolved Hide resolved
@jhoblitt jhoblitt force-pushed the bugfix/cephobjectstore-existing-pools branch 2 times, most recently from 161f4de to da84403 Compare October 2, 2024 17:21
@jhoblitt jhoblitt requested a review from BlaineEXE October 2, 2024 17:23
@jhoblitt jhoblitt changed the title rgw: fix cephObjectStore failing with pre-existing pools rgw: fix CephObjectStore failing with pre-existing pools Oct 2, 2024
travisn
travisn reviewed Oct 3, 2024
View reviewed changes
tests/scripts/github-action-helper.sh Outdated Show resolved Hide resolved
.github/workflows/canary-integration-test.yml Outdated Show resolved Hide resolved
@jhoblitt
test: add github-action-helper toolbox functions
cb1dd15 
Added these functions for running commands in the toolbox pod:

- toolbox()
- ceph()
- rbd()
- radosgw-admin()

Signed-off-by: Joshua Hoblitt <josh@hoblitt.com>
@jhoblitt
rgw: fix cephObjectStore failing with pre-existing pools
9b952e0 
When attempting to use pre-existing (or cephBlockPool managed) pools and
the cephObjectStore.spec.{metadataPool,dataPool} fields are not
specified, rgw creation will fail with this error:

    2024-09-25 21:00:17.744358 E | ceph-object-controller: failed to reconcile CephObjectStore "rook-ceph/test1". failed to create object store deployments: failed to create object pools: failed to create metadata pools: failed to create pool "test1.rgw.control": pool "test1.rgw.control" type is not defined as replicated or erasure coded

Signed-off-by: Joshua Hoblitt <josh@hoblitt.com>
@jhoblitt
test: add object-with-cephblockpool canary test
ed81560 
Signed-off-by: Joshua Hoblitt <josh@hoblitt.com>
@jhoblitt jhoblitt force-pushed the bugfix/cephobjectstore-existing-pools branch from da84403 to f633c30 Compare October 3, 2024 17:16
@jhoblitt
test: mv canary test specific resources out of deploy_cluster()
31d55b9 
Factor out most of the CRs used by various canary tests to a new
deploy_cluster_full_of_cruft_please_stop_using_this() function.

Signed-off-by: Joshua Hoblitt <josh@hoblitt.com>
@jhoblitt
test: convert all github-action-helper functs to $REPO_DIR
581fd5c 
This allows all functions to be called in any order without concern for
the CWD.

Signed-off-by: Joshua Hoblitt <josh@hoblitt.com>
@jhoblitt jhoblitt force-pushed the bugfix/cephobjectstore-existing-pools branch from f633c30 to 581fd5c Compare October 3, 2024 17:18
travisn
travisn reviewed Oct 3, 2024
View reviewed changes
Copy link
Member

@travisn travisn left a comment

Choose a reason for hiding this comment

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

LGTM, will approve after the CI passes, thanks!

@travisn travisn merged commit 0499534 into rook:master Oct 3, 2024
56 checks passed
@mergify mergify bot mentioned this pull request Oct 3, 2024
Merged
6 tasks
@jhoblitt jhoblitt deleted the bugfix/cephobjectstore-existing-pools branch October 3, 2024 17:53
@BlaineEXE BlaineEXE mentioned this pull request Oct 3, 2024
Merged
6 tasks
travisn added a commit that referenced this pull request Oct 3, 2024
@travisn
Merge pull request #14799 from rook/mergify/bp/release-1.15/pr-14772
26f1ba5 
rgw: fix CephObjectStore failing with pre-existing pools (backport #14772)
@travisn travisn mentioned this pull request Oct 11, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BlaineEXE BlaineEXE BlaineEXE approved these changes

@travisn travisn travisn approved these changes

Assignees
No one assigned
Labels
backport-release-1.15
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Allow setting options for CephObjectStore index pools
3 participants
@jhoblitt @travisn @BlaineEXE