Skip to content

mgr/cephadm: add feature-based REDEPLOY logic for SMB services#69907

Open
Sodani wants to merge 1 commit into
ceph:mainfrom
Sodani:shsodani_ceph_rgw_new_module
Open

mgr/cephadm: add feature-based REDEPLOY logic for SMB services#69907
Sodani wants to merge 1 commit into
ceph:mainfrom
Sodani:shsodani_ceph_rgw_new_module

Conversation

@Sodani

@Sodani Sodani commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Implement intelligent action selection for SMB daemon updates by tracking feature changes as dependencies. When SMB service features change (e.g., adding cephfs-proxy, remote-control, keybridge), the system now triggers a REDEPLOY instead of RECONFIG, as these changes require container structure modifications.

Fixes: https://tracker.ceph.com/issues/77762

Checklist

  • Tracker (select at least one)
    • References tracker ticket
    • Very recent bug; references commit where it was introduced
    • New feature (ticket optional)
    • Doc update (no ticket needed)
    • Code cleanup (no ticket needed)
  • Component impact
    • Affects Dashboard, opened tracker ticket
    • Affects Orchestrator, opened tracker ticket
    • No impact that needs to be tracked
  • Documentation (select at least one)
    • Updates relevant documentation
    • No doc update is appropriate
  • Tests (select at least one)

avanthakkar
avanthakkar reviewed Jul 2, 2026
View reviewed changes
Comment thread src/pybind/mgr/cephadm/services/smb.py
Comment thread src/pybind/mgr/cephadm/services/smb.py Outdated
@avanthakkar avanthakkar changed the title mgr/cephadm/smb: Add feature-based REDEPLOY logic for SMB services mgr/cephadm: add feature-based REDEPLOY logic for SMB services Jul 2, 2026
@Sodani Sodani force-pushed the shsodani_ceph_rgw_new_module branch 2 times, most recently from 4f97911 to 59a355d Compare July 2, 2026 13:09
@Sodani Sodani marked this pull request as ready for review July 2, 2026 16:10
@Sodani Sodani requested review from a team as code owners July 2, 2026 16:10
phlogistonjohn
phlogistonjohn reviewed Jul 2, 2026
View reviewed changes

@phlogistonjohn phlogistonjohn left a comment •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Generally lgtm.
Avan's sambacc fix (for samba-in-kubernetes/sambacc#208) has been merged and I rebuilt samba-container. Please retest with the latest samba container and see if that resolves the issue loading the config we discussed earlier.

@Sodani Sodani force-pushed the shsodani_ceph_rgw_new_module branch from 59a355d to aac157b Compare July 3, 2026 05:29
@Sodani
mgr/cephadm/smb: Add feature-based REDEPLOY logic for SMB services
bcc6451 
Implement intelligent action selection for SMB daemon updates by tracking
feature changes as dependencies. When SMB service features change (e.g.,
adding cephfs-proxy, remote-control, keybridge), the system now triggers
a REDEPLOY instead of RECONFIG, as these changes require container
structure modifications.

Fixes: https://tracker.ceph.com/issues/77762

Signed-off-by: Shweta Sodani <[email protected]>
@Sodani Sodani force-pushed the shsodani_ceph_rgw_new_module branch from aac157b to bcc6451 Compare July 3, 2026 06:29
@avanthakkar

avanthakkar commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

jenkins test api

avanthakkar
avanthakkar approved these changes Jul 3, 2026
View reviewed changes

@avanthakkar avanthakkar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, thanks!

@avanthakkar avanthakkar mentioned this pull request Jul 3, 2026
Draft
14 tasks
@avanthakkar

avanthakkar commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Generally lgtm. Avan's sambacc fix (for samba-in-kubernetes/sambacc#208) has been merged and I rebuilt samba-container. Please retest with the latest samba container and see if that resolves the issue loading the config we discussed earlier.

The issue seems to be resolved now. I see correct config loaded in net conf list. @Sodani Can you verify?

@Sodani

Sodani commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

Generally lgtm. Avan's sambacc fix (for samba-in-kubernetes/sambacc#208) has been merged and I rebuilt samba-container. Please retest with the latest samba container and see if that resolves the issue loading the config we discussed earlier.

The issue seems to be resolved now. I see correct config loaded in net conf list. @Sodani Can you verify?

Yes I have verified. This fix is working with latest samba container build. Thanks.

phlogistonjohn
phlogistonjohn approved these changes Jul 3, 2026
View reviewed changes

@phlogistonjohn phlogistonjohn left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm

@phlogistonjohn

phlogistonjohn commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

@avanthakkar will you run teuthology with this PR?

@avanthakkar

avanthakkar commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

@avanthakkar will you run teuthology with this PR?

Yes infact I have triggered combined build of this PR and other which depends on this with similar changes for rgw_creds_uri

@avanthakkar

avanthakkar commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

@avanthakkar will you run teuthology with this PR?

Yes infact I have triggered combined build of this PR and other which depends on this with similar changes for rgw_creds_uri

Just started :
https://pulpito-ng.ceph.com/runs/avan-2026-07-03_13:35:10-orch:cephadm:smb-wip-athakkar2-testing-2026-07-03-1648-distro-default-trial

@avanthakkar

avanthakkar commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

I see 3 failure about CEPHADM_FAILED_DAEMON and this seems related. From what I checked in logs there seems to be race between re-deploy and smb cluster teardown which causes ceph health issues. I'm looking into this further to understand issue and provide a fix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@avanthakkar avanthakkar avanthakkar approved these changes
@phlogistonjohn phlogistonjohn phlogistonjohn approved these changes

Assignees

No one assigned

Labels

cephadm pybind wip-athakkar-testing

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Sodani @avanthakkar @phlogistonjohn