Skip to content

feat: Add support for fetching cephfs mirroring status and lists to replication framework#601

Merged
UtkarshBhatthere merged 10 commits intomainfrom
feature/squid/StatusCephfsMirroring
Sep 9, 2025
Merged

feat: Add support for fetching cephfs mirroring status and lists to replication framework#601
UtkarshBhatthere merged 10 commits intomainfrom
feature/squid/StatusCephfsMirroring

Conversation

@UtkarshBhatthere
Copy link
Copy Markdown
Contributor

@UtkarshBhatthere UtkarshBhatthere commented Aug 7, 2025 •
edited
Loading 

root@primary:~# microceph replication list cephfs ist cephfs
+--------+-------------------------+-----------+
| VOLUME | RESOURCE                | PATH      |
+--------+-------------------------+-----------+
| vol    | /one                    | directory |
| vol    | /two                    | directory |
| vol    | /volumes/_nogroup/hpc   | subvolume |
| vol    | /volumes/openstack/ceph | subvolume |
+--------+-------------------------+-----------+
root@primary:~# microceph replication status cephfs vol
+--------------------------+
|          SUMMARY         |
+----------------+---------+
| Volume         | vol     |
| Resource Count | 4       |
| Peer Count     | 1       |
+----------------+---------+

+-------------+-------------------------+-------+--------------+---------------+---------------+
| REMOTE NAME | RESOURCE PATH           | STATE | SNAPS SYNCED | SNAPS DELETED | SNAPS RENAMED |
+-------------+-------------------------+-------+--------------+---------------+---------------+
| secondary   | /one                    | idle  |            3 |               0               |
| secondary   | /two                    | idle  |            1 |               0               |
| secondary   | /volumes/_nogroup/hpc   | idle  |            1 |               0               |
| secondary   | /volumes/openstack/ceph | idle  |            1 |               0               |
+-------------+-------------------------+-------+--------------+-------------------------------+

@UtkarshBhatthere UtkarshBhatthere marked this pull request as draft August 7, 2025 10:47
@UtkarshBhatthere UtkarshBhatthere force-pushed the feature/squid/StatusCephfsMirroring branch 6 times, most recently from b664a39 to ae53214 Compare August 14, 2025 12:58
@UtkarshBhatthere UtkarshBhatthere force-pushed the feature/squid/StatusCephfsMirroring branch 4 times, most recently from 6731721 to cb556db Compare August 20, 2025 07:25
@UtkarshBhatthere UtkarshBhatthere force-pushed the feature/squid/StatusCephfsMirroring branch from cb556db to 2fa806d Compare August 22, 2025 13:18
@UtkarshBhatthere UtkarshBhatthere changed the title feat: Add support for fetching cephfs mirroring status to replication framework feat: Add support for fetching cephfs mirroring status and lists to replication framework Aug 22, 2025
@UtkarshBhatthere
feat: Add support for fetching CephFS mirroring status
8fa1623 
Signed-off-by: utkarsh bhatt <utkarsh.bhatt@canonical.com>
@UtkarshBhatthere UtkarshBhatthere force-pushed the feature/squid/StatusCephfsMirroring branch from 2fa806d to 14e0ed9 Compare August 25, 2025 09:03
@UtkarshBhatthere
feat: Added Cephfs replication handler
b3924e9 
Signed-off-by: utkarsh bhatt <utkarsh.bhatt@canonical.com>
@UtkarshBhatthere
temp: Added info logs
207d1fa 
Signed-off-by: Utkarsh Bhatt <utkarsh.bhatt@canonical.com>
@UtkarshBhatthere UtkarshBhatthere force-pushed the feature/squid/StatusCephfsMirroring branch 3 times, most recently from 70788e7 to 60d24de Compare August 26, 2025 07:29
@UtkarshBhatthere UtkarshBhatthere marked this pull request as ready for review August 26, 2025 11:03
sabaini
sabaini requested changes Aug 27, 2025
View reviewed changes
Copy link
Copy Markdown
Collaborator

@sabaini sabaini left a comment

Choose a reason for hiding this comment

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

Hey hey, first run through with some comments / requests -- thanks @UtkarshBhatthere

Before we can merge we'd also need unit tests for this

Comment thread microceph/api/ops_replication.go
Comment thread microceph/api/ops_replication.go Outdated
Comment thread microceph/ceph/cephfs_mirror.go Outdated
Comment thread microceph/ceph/cephfs_mirror.go Outdated
Comment thread microceph/ceph/cephfs_mirror.go
}

full_sock_path := filepath.Join(run_path, socket.Name())
err = CheckFsMirrorHelperCommands(full_sock_path)
Copy link
Copy Markdown
Collaborator

@sabaini sabaini Aug 27, 2025

Choose a reason for hiding this comment

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

Hm, probing the admin sockets feels brittle -- upstream could change cmd output eg. Also it's relatively heavy in terms of resource usage. Would relying on the socket name pattern ceph-client.cephfs-mirror.{hostname}.asok work? I feel that's a relatively stable convention

Copy link
Copy Markdown
Contributor Author

@UtkarshBhatthere UtkarshBhatthere Aug 27, 2025

Choose a reason for hiding this comment

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

So there is an inconsistency there that cephfs-mirror daemon creates random named socket files each time, it is also not cleaned so they keep on growing....

Copy link
Copy Markdown
Contributor Author

@UtkarshBhatthere UtkarshBhatthere Aug 27, 2025

Choose a reason for hiding this comment

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

refer: https://ceph-storage.slack.com/archives/C1HFU4JK1/p1756295649714109

Copy link
Copy Markdown
Contributor Author

@UtkarshBhatthere UtkarshBhatthere Aug 27, 2025

Choose a reason for hiding this comment

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

i will modify this to find the most recently created socket.

Copy link
Copy Markdown
Contributor Author

@UtkarshBhatthere UtkarshBhatthere Aug 29, 2025

Choose a reason for hiding this comment

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

so apparently the most recent one is not a sure shot solution. We still end up iterating over a bunch of them.

Copy link
Copy Markdown
Collaborator

@sabaini sabaini Sep 4, 2025

Choose a reason for hiding this comment

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

Ack, if we end up needing to do the probe dance let's go with that for now lacking a better solution. Aiui upstream does plan to provide support for cli commands what don't need the admin socket access is that right? If so would be good to leave a TODO comment so we can revisit this once this support lands.

Copy link
Copy Markdown
Contributor Author

@UtkarshBhatthere UtkarshBhatthere Sep 9, 2025

Choose a reason for hiding this comment

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

ack, will add a todo for the same.

Copy link
Copy Markdown
Collaborator

@sabaini sabaini Sep 9, 2025

Choose a reason for hiding this comment

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

Hm, did you add that TODO comment?

Comment thread microceph/ceph/cephfs_volume.go Outdated
Comment thread microceph/ceph/cephfs_volume.go
Comment thread microceph/ceph/replication_cephfs.go Outdated
Comment thread microceph/cmd/microceph/replication_list_cephfs.go Outdated
Comment thread microceph/cmd/microcephd/main.go Outdated
@UtkarshBhatthere UtkarshBhatthere force-pushed the feature/squid/StatusCephfsMirroring branch from cf8a64a to 72e6bcf Compare September 2, 2025 14:07
sabaini
sabaini approved these changes Sep 9, 2025
View reviewed changes
Copy link
Copy Markdown
Collaborator

@sabaini sabaini left a comment

Choose a reason for hiding this comment

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

Hey @UtkarshBhatthere lgtm in general, only thing missing is that TODO comment :-)

Comment thread microceph/ceph/cephfs_mirror.go
}

full_sock_path := filepath.Join(run_path, socket.Name())
err = CheckFsMirrorHelperCommands(full_sock_path)
Copy link
Copy Markdown
Collaborator

@sabaini sabaini Sep 9, 2025

Choose a reason for hiding this comment

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

Hm, did you add that TODO comment?

@UtkarshBhatthere UtkarshBhatthere force-pushed the feature/squid/StatusCephfsMirroring branch from 5457ad2 to 01a94a1 Compare September 9, 2025 10:53
@UtkarshBhatthere
fix: Adds a patch for mirroring module from upstream
5cfa721 
This patch is merged in squid and expected to land in the 19.2.4 point
release. Thus it is being added a temporary patch to microceph repo.

Signed-off-by: Utkarsh Bhatt <utkarsh.bhatt@canonical.com>
@UtkarshBhatthere
test: Modified functional tests to verify new commands
5e256cf 
Signed-off-by: Utkarsh Bhatt <utkarsh.bhatt@canonical.com>
@UtkarshBhatthere
fix: resolve review comments
9d43d4b 
Signed-off-by: Utkarsh Bhatt <utkarsh.bhatt@canonical.com>
@UtkarshBhatthere
fix: comment resolutions
fdbc352 
Signed-off-by: Utkarsh Bhatt <utkarsh.bhatt@canonical.com>
@UtkarshBhatthere
fix: added timeout for list status
c0c1336 
Signed-off-by: Utkarsh Bhatt <utkarsh.bhatt@canonical.com>
@UtkarshBhatthere
fix: Fix volume fetcher for missing ungrouped subvolumes
dc931e9 
Signed-off-by: Utkarsh Bhatt <utkarsh.bhatt@canonical.com>
@UtkarshBhatthere
chore: Added a TODO for future
0dec2e2 
Added todo for future migration to ceph cli against the local admin sock
available from cephfs-mirror daemon.

Signed-off-by: Utkarsh Bhatt <utkarsh.bhatt@canonical.com>
@UtkarshBhatthere UtkarshBhatthere force-pushed the feature/squid/StatusCephfsMirroring branch from 01a94a1 to 0dec2e2 Compare September 9, 2025 10:53
@UtkarshBhatthere UtkarshBhatthere merged commit 9d13f4a into main Sep 9, 2025
51 of 52 checks passed
@UtkarshBhatthere UtkarshBhatthere deleted the feature/squid/StatusCephfsMirroring branch April 28, 2026 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sabaini sabaini sabaini approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@UtkarshBhatthere @sabaini