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

cephfs: implement getMetadataPool() with go-ceph #1578

Merged
merged 3 commits into from Oct 20, 2020

Conversation

nixpanic
Copy link
Member

@nixpanic nixpanic commented Oct 14, 2020

Reduce the number of ceph fs command executions, and use go-ceph with a cached/shared connection.

Fixes: #1554
Do-Not-Merge: depends on #1545, #1581 and #1577, which need to get merged first

@nixpanic nixpanic added enhancement New feature or request DNM DO NOT MERGE component/cephfs Issues related to CephFS labels Oct 14, 2020
@nixpanic nixpanic added this to In progress in Use go-ceph for volume management operations via automation Oct 14, 2020
@nixpanic nixpanic force-pushed the cephfs/go-ceph/getMetadataPool branch 2 times, most recently from e34c60d to 055c75b Compare October 15, 2020 14:35
@nixpanic nixpanic removed the DNM DO NOT MERGE label Oct 15, 2020
@nixpanic nixpanic marked this pull request as ready for review October 15, 2020 14:36
@mergify
Copy link
Contributor

mergify bot commented Oct 19, 2020

This pull request now has conflicts with the target branch. Could you please resolve conflicts and force push the corrected changes? 🙏

@nixpanic nixpanic force-pushed the cephfs/go-ceph/getMetadataPool branch from 055c75b to 219c37d Compare October 19, 2020 13:17
@nixpanic
Copy link
Member Author

/retest ci/centos/upgrade-tests-cephfs

@nixpanic
Copy link
Member Author

@Madhu-1 @humblec this is ready for review, please have a look

@nixpanic nixpanic requested a review from humblec October 19, 2020 14:21
@nixpanic nixpanic force-pushed the cephfs/go-ceph/getMetadataPool branch from 219c37d to e012394 Compare October 20, 2020 08:38
@nixpanic nixpanic requested a review from Madhu-1 October 20, 2020 08:39
@nixpanic
Copy link
Member Author

/retest ci/centos/upgrade-tests-cephfs

@humblec
Copy link
Collaborator

humblec commented Oct 20, 2020

Reduce the numbed of ceph fs -> @nixpanic numbed to number in PR description.

if err != nil {
util.ErrorLog(ctx, "could not list filesystems, can not fetch metadata pool for %s:", vo.FsName, err)
return "", err
Copy link
Collaborator

Choose a reason for hiding this comment

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

@nixpanic dont we have to defer fsa here?

Copy link
Member Author

Choose a reason for hiding this comment

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

no, there is no destructor for FSAdmin

Copy link
Collaborator

Choose a reason for hiding this comment

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

oh..ok .. its good to have.. We dont have to leave the connection behind.

@nixpanic nixpanic requested a review from humblec October 20, 2020 09:34
@humblec humblec added this to the release-3.2.0 milestone Oct 20, 2020
@nixpanic
Copy link
Member Author

/retest ci/centos/mini-e2e-helm/k8s-1.19.2

Signed-off-by: Niels de Vos <ndevos@redhat.com>
Fixes: ceph#1554
Signed-off-by: Niels de Vos <ndevos@redhat.com>
golang-ci suddenly complains about the following issue

    internal/cephfs/util.go:41:1: directive `// nolint:unparam //  todo:program values has to be revisited later` is unused for linter unparam (nolintlint)
    // nolint:unparam //  todo:program values has to be revisited later
    ^

Dropping the comment completely seems to fix it. Ideally
execCommandJSON() will get removed once the migration to go-ceph is
complete.

Signed-off-by: Niels de Vos <ndevos@redhat.com>
@nixpanic
Copy link
Member Author

/retest ci/centos/mini-e2e-helm/k8s-1.18.9

@nixpanic
Copy link
Member Author

/retest ci/centos/upgrade-tests-rbd

@mergify mergify bot merged commit 5a1e370 into ceph:master Oct 20, 2020
Use go-ceph for volume management operations automation moved this from In progress to Done Oct 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/cephfs Issues related to CephFS enhancement New feature or request
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

cephfs: replace getMetadataPool() with a go-ceph implementation
3 participants