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

Add support for samba setup using ceph smb manager module #113

Merged
merged 3 commits into from
Aug 5, 2024

Conversation

anoopcs9
Copy link
Collaborator

  • Enable and create samba shares with ceph smb mgr module.
  • Explicit creation of samba users via users.setup is skipped(achieved through smb mgr module).
  • Client access directed at node running smb service.

Finally, we have a new backend - cephfs.mgr.vfs 😀 .

@anoopcs9
Copy link
Collaborator Author

EXTRA_VARS="backend=cephfs.mgr.vfs test_sanity_only=1" make test

Above command successfully completed.

phlogistonjohn
phlogistonjohn previously approved these changes Jul 26, 2024
Copy link
Collaborator

@phlogistonjohn phlogistonjohn left a comment

Choose a reason for hiding this comment

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

Changes to add smb mgr module support look correct to me. I have some questions around how to possibly use the module more efficiently but I'd be fine with merging this as it is too.

playbooks/ansible/roles/sit.cephfs/tasks/main.yml Outdated Show resolved Hide resolved
playbooks/ansible/roles/sit.cephfs/tasks/main.yml Outdated Show resolved Hide resolved
playbooks/ansible/roles/sit.cephfs/tasks/main.yml Outdated Show resolved Hide resolved
xhernandez
xhernandez previously approved these changes Jul 29, 2024
spuiuk
spuiuk previously approved these changes Jul 31, 2024
Copy link
Collaborator

@spuiuk spuiuk left a comment

Choose a reason for hiding this comment

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

ACK

@anoopcs9
Copy link
Collaborator Author

I've added the do-not-merge label to include the suggestions from @phlogistonjohn and @xhernandez in the current PR itself. I'll update shortly.

@anoopcs9
Copy link
Collaborator Author

anoopcs9 commented Jul 31, 2024

Error ENOTSUP: Warning: due to ceph-mgr restart, some PG states may not be up to date\nModule 'smb' is not enabled/loaded (required by command 'smb apply'): use ceph mgr module enable smb to enable it

@phlogistonjohn
I happen to see this error on CI cluster(not locally for me) while trying to create smb cluster immediately after smb mgr module is enabled. May be we wait for mgr to be running after ceph mgr module enable smb?

@phlogistonjohn
Copy link
Collaborator

I happen to see this error on CI cluster(not locally for me) while trying to create smb cluster immediately after smb mgr module is enabled. May be we wait for mgr to be running after ceph mgr module enable smb?

Yes, try waiting or waiting longer. Enabling the module triggers a mgr restart and it's not the fastest.

phlogistonjohn
phlogistonjohn previously approved these changes Jul 31, 2024
Copy link
Collaborator

@phlogistonjohn phlogistonjohn left a comment

Choose a reason for hiding this comment

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

updated code lgtm (modulo the restart issue you are seeing)

@anoopcs9
Copy link
Collaborator Author

anoopcs9 commented Aug 1, 2024

updated code lgtm (modulo the restart issue you are seeing)

Added a task to wait for mgr restart to complete and tests successfully passed 😊 .

Copy link
Collaborator

@phlogistonjohn phlogistonjohn left a comment

Choose a reason for hiding this comment

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

looks ok to me.

@anoopcs9
Copy link
Collaborator Author

anoopcs9 commented Aug 2, 2024

updated code lgtm (modulo the restart issue you are seeing)

Added a task to wait for mgr restart to complete and tests successfully passed 😊 .

I spoke too soon. Full test suite run has some smbtorture failures.

Signed-off-by: Anoop C S <anoopcs@cryptolab.net>
Signed-off-by: Anoop C S <anoopcs@cryptolab.net>
Signed-off-by: Anoop C S <anoopcs@cryptolab.net>
@anoopcs9
Copy link
Collaborator Author

anoopcs9 commented Aug 5, 2024

updated code lgtm (modulo the restart issue you are seeing)

Added a task to wait for mgr restart to complete and tests successfully passed 😊 .

I spoke too soon. Full test suite run has some smbtorture failures.

I think its good to start with the CI runs and eventually debug and provide fixes. Removing the do-not-merge label.

@anoopcs9 anoopcs9 merged commit b0e5af7 into samba-in-kubernetes:main Aug 5, 2024
9 checks passed
@anoopcs9 anoopcs9 deleted the add-smb-mgr-setup branch August 5, 2024 08:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants