mgr/smb: improvements to smb mgr module and docs #58518

anoopcs9 · 2024-07-10T18:41:55Z

Fix a wrong field name in error response while trying to remove a cluster with active shares.
Add a missing option to ceph smb cluster create in docs.

phlogistonjohn

LGTM, thanks! Good eye on the json error key name.

doc/mgr/smb.rst

zdover23 · 2024-07-10T20:28:29Z

I can't speak on the change to src/pybind/mgr/smb/handler.py, but if @phlogistonjohn has approved it, I'll merge this and generate the backports as soon as tests have passed.

phlogistonjohn · 2024-07-10T21:27:42Z

@zdover23 no backports please. AFAIR, they wouldn't even apply to squid because this doc file is only in main.

phlogistonjohn · 2024-07-10T21:28:35Z

Let's have @adk3798 merge it too after he's run teuthology tests. thanks!

zdover23 · 2024-07-10T21:28:46Z

@zdover23 no backports please. AFAIR, they wouldn't even apply to squid because this doc file is only in main.

@phlogistonjohn Roger that.

anoopcs9 · 2024-07-11T12:14:40Z

@phlogistonjohn @adk3798 @zdover23 Can I squeeze in the following additional documentation change as part of the PR? In that case I could address @anthonyeleven 's suggestion too.

index d48c638dbee..1fd27e53b7d 100644
--- a/doc/mgr/smb.rst
+++ b/doc/mgr/smb.rst
@@ -357,7 +357,7 @@ custom_dns
 placement
     Optional. A Ceph Orchestration :ref:`placement specifier
     <orchestrator-cli-placement-spec>`.  Defaults to one host if not provided
-custom_smb_share_options
+custom_smb_global_options
     Optional mapping. Specify key-value pairs that will be directly added to
     the global ``smb.conf`` options (or equivalent) of a Samba server.  Do
     *not* use this option unless you are prepared to debug the Samba instances

zdover23 · 2024-07-11T12:19:06Z

@phlogistonjohn @adk3798 @zdover23 Can I squeeze in the following additional documentation change as part of the PR? In that case I could address @anthonyeleven 's suggestion too.

index d48c638dbee..1fd27e53b7d 100644

--- a/doc/mgr/smb.rst

+++ b/doc/mgr/smb.rst

@@ -357,7 +357,7 @@ custom_dns

 placement

     Optional. A Ceph Orchestration :ref:`placement specifier

     <orchestrator-cli-placement-spec>`.  Defaults to one host if not provided

-custom_smb_share_options

+custom_smb_global_options

     Optional mapping. Specify key-value pairs that will be directly added to

     the global ``smb.conf`` options (or equivalent) of a Samba server.  Do

     *not* use this option unless you are prepared to debug the Samba instances

@anoopcs9, I'm sure you can, because this PR hasn't yet been merged. I don't know if you'll want to squash your commits, but if you do: https://docs.ceph.com/en/latest/start/documenting-ceph/#squash-extraneous-commits

In the following error response the list of active shares is incorrectly displayed with "clusters" field. { "resource": { "resource_type": "ceph.smb.cluster", "cluster_id": "smbcluster", "intent": "removed" }, "clusters": [ "smbshare" ], "msg": "cluster in use by shares", "success": false } Replace "clusters" with "shares" to avoid confusion. Signed-off-by: Anoop C S <anoopcs@cryptolab.net>

anoopcs9 · 2024-07-11T12:43:28Z

@phlogistonjohn @adk3798 @zdover23 Can I squeeze in the following additional documentation change as part of the PR? In that case I could address @anthonyeleven 's suggestion too.
index d48c638dbee..1fd27e53b7d 100644

--- a/doc/mgr/smb.rst

+++ b/doc/mgr/smb.rst

@@ -357,7 +357,7 @@ custom_dns

 placement

     Optional. A Ceph Orchestration :ref:`placement specifier

     <orchestrator-cli-placement-spec>`.  Defaults to one host if not provided

-custom_smb_share_options

+custom_smb_global_options

     Optional mapping. Specify key-value pairs that will be directly added to

     the global ``smb.conf`` options (or equivalent) of a Samba server.  Do

     *not* use this option unless you are prepared to debug the Samba instances
@anoopcs9, I'm sure you can, because this PR hasn't yet been merged. I don't know if you'll want to squash your commits, but if you do: https://docs.ceph.com/en/latest/start/documenting-ceph/#squash-extraneous-commits

I've updated to add an extra patch for the above mentioned change.

'--define-user-pass' allows us to specify the list of users, along with their passwords, permitted to access different shares within a cluster. But this option was missing from the corresponding docs. Signed-off-by: Anoop C S <anoopcs@cryptolab.net>

Custom options to the ceph.smb.cluster resource are expected under "custom_smb_global_options" and not "custom_smb_share_options". Signed-off-by: Anoop C S <anoopcs@cryptolab.net>

zdover23 · 2024-07-11T23:56:19Z

teuthology.exceptions.CommandFailedError: Command failed with status 28: '../src/vstart.sh -n --nolockdep'

zdover23 · 2024-07-11T23:56:27Z

jenkins test api

adk3798 · 2024-07-15T12:11:07Z

jenkins test api

adk3798 · 2024-07-15T12:28:07Z

https://pulpito.ceph.com/adking-2024-07-11_11:10:32-orch:cephadm-wip-adk-testing-2024-07-10-1548-distro-default-smithi/

Failures

3 mds_upgrade_sequence failures, known issue
2 staggered upgrade ceph orch ps vs. ceph versions mismatch, known issue
1 failure in test_cephadm task. This was actually caused by a PR in the run (have commented about it on that PR)
thrash and rgw_ingress tests timing out, known issue

Should only block merging for the one PR breaking the test_cephadm task test

adk3798 · 2024-07-15T22:14:20Z

jenkins test api

mgr/smb: improvements to smb mgr module and docs Reviewed-by: Anthony D'Atri <anthonyeleven@users.noreply.github.com> Reviewed-by: John Mulligan <jmulligan@redhat.com>

anoopcs9 requested a review from a team as a code owner July 10, 2024 18:41

github-actions bot added documentation mgr pybind labels Jul 10, 2024

phlogistonjohn approved these changes Jul 10, 2024

View reviewed changes

adk3798 added the wip-adk-testing label Jul 10, 2024

adk3798 changed the title ~~Few improvements to smb mgr module and docs~~ mgr/smb: improvements to smb mgr module and docs Jul 10, 2024

anthonyeleven approved these changes Jul 10, 2024

View reviewed changes

doc/mgr/smb.rst Outdated Show resolved Hide resolved

anoopcs9 force-pushed the smb-mgr-updates branch from d7a6ee1 to 7a904aa Compare July 11, 2024 12:42

anoopcs9 added 2 commits July 11, 2024 18:15

doc/mgr/smb: Fix a typo in options for cluster resource

97119ed

Custom options to the ceph.smb.cluster resource are expected under "custom_smb_global_options" and not "custom_smb_share_options". Signed-off-by: Anoop C S <anoopcs@cryptolab.net>

anoopcs9 force-pushed the smb-mgr-updates branch from 7a904aa to 97119ed Compare July 11, 2024 12:45

adk3798 merged commit d43d1fc into ceph:main Jul 18, 2024

anoopcs9 deleted the smb-mgr-updates branch July 26, 2024 04:48

mgr/smb: improvements to smb mgr module and docs #58518

mgr/smb: improvements to smb mgr module and docs #58518

Uh oh!

Conversation

anoopcs9 commented Jul 10, 2024

Uh oh!

phlogistonjohn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

zdover23 commented Jul 10, 2024

Uh oh!

phlogistonjohn commented Jul 10, 2024

Uh oh!

phlogistonjohn commented Jul 10, 2024

Uh oh!

zdover23 commented Jul 10, 2024

Uh oh!

anoopcs9 commented Jul 11, 2024

Uh oh!

zdover23 commented Jul 11, 2024

Uh oh!

anoopcs9 commented Jul 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zdover23 commented Jul 11, 2024

Uh oh!

zdover23 commented Jul 11, 2024

Uh oh!

adk3798 commented Jul 15, 2024

Uh oh!

adk3798 commented Jul 15, 2024

Uh oh!

adk3798 commented Jul 15, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

anoopcs9 commented Jul 11, 2024 •

edited

Loading