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

Group possible backend share configurations under same environment #117

Merged
merged 8 commits into from
Aug 28, 2024

Conversation

anoopcs9
Copy link
Collaborator

@anoopcs9 anoopcs9 commented Aug 9, 2024

Key considerations:

  • cephfs and cephfs.vfs are combined together under cephfs.
    • cephfs.mgr.vfs becomes cephfs.mgr when shares are configured using ceph smb mgr module.
  • A new methods field to indicate how variations of each share section are to be configured within smb.conf.
    • Special cases:
      • kernel client approach is not yet supported by ceph mgr module. Accordingly we skip this variation.
      • gpfs scale variant configuration is always based on vfs. Thus we skip iterating over available methods.

fixes #101

@anoopcs9 anoopcs9 force-pushed the group-backend-shares branch 3 times, most recently from 5a9628f to b84556a Compare August 10, 2024 11:05
@anoopcs9 anoopcs9 marked this pull request as ready for review August 11, 2024 05:27
@anoopcs9
Copy link
Collaborator Author

I have another branch(group-backend-shares2) which reduces the complexity of changes from test-info.yml.j2 by defining the strategies within global settings.yml.

@anoopcs9
Copy link
Collaborator Author

anoopcs9 commented Aug 14, 2024

/retest centos-ci/gpfs.scale

@spuiuk
Copy link
Collaborator

spuiuk commented Aug 14, 2024

Notes: Shares created

cephfs:
share-cephfs-default-vfs
share-cephfs-default-klcient

cephmgr:
share-cephfs-mgr-vfs

spuiuk
spuiuk previously approved these changes Aug 14, 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.

Looks good. ACK

anoopcs9 added a commit to anoopcs9/samba-centosci that referenced this pull request Aug 15, 2024
With [1] all possible share configurations are grouped after their
respective backend file system. So those duplicate entries can be
removed reducing the number of jobs in the matrix.

[1] samba-in-kubernetes/sit-environment#117
anoopcs9 added a commit to anoopcs9/samba-centosci that referenced this pull request Aug 15, 2024
With [1] suffix 'vfs' is irrelevant in 'cephfs.mgr.vfs' as multiple
share configurations are included within mgr variant.

[1] samba-in-kubernetes/sit-environment#117
Copy link
Collaborator

@xhernandez xhernandez left a comment

Choose a reason for hiding this comment

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

I think that the meaning of variant and method (or strategy) is a bit mixed. Why do we need variant if we already have another list that contains all the variants we want to use ?

playbooks/settings.yml Outdated Show resolved Hide resolved
Copy link
Collaborator Author

@anoopcs9 anoopcs9 left a comment

Choose a reason for hiding this comment

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

I think that the meaning of variant and method (or strategy) is a bit mixed. Why do we need variant if we already have another list that contains all the variants we want to use ?

I intent to change the meaning of variant to indicate how(or who) Samba and/or CTDB configurations are handled during cluster setup. With this we will have just two variants namely, default(independent setup style) and mgr(using ceph mgr module). Each of these variants can have different methods through which shares are configured for client access.

playbooks/settings.yml Outdated Show resolved Hide resolved
@xhernandez
Copy link
Collaborator

I intent to change the meaning of variant to indicate how(or who) Samba and/or CTDB configurations are handled during cluster setup. With this we will have just two variants namely, default(independent setup style) and mgr(using ceph mgr module). Each of these variants can have different methods through which shares are configured for client access.

This means that the variant only makes sense for the ceph backend. All other backends won't have variants, which is a bit weird.

I think that we should move the definition of backends inside settings.yml (see #119). Then we could easily extend the definition like this (just a possible way, not thought much about it):

backends:
  cephfs:
    provider: mgr
    settings:
        <optional mgr specific options>
    shares:
      kclient:
        <optional kclient specific options>
      vfs:
        <optional vfs specific options>

This way we could easily create more than one backend, with multiple shares, in a single configuration (additional work required to avoid using the same devices, but that's another problem).

In any case, this is not related to this patch. We can discuss this in #119.

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

I intent to change the meaning of variant to indicate how(or who) Samba and/or CTDB configurations are handled during cluster setup. With this we will have just two variants namely, default(independent setup style) and mgr(using ceph mgr module). Each of these variants can have different methods through which shares are configured for client access.

This means that the variant only makes sense for the ceph backend. All other backends won't have variants, which is a bit weird.

I have considered scale as a variant for GPFS.

I think that we should move the definition of backends inside settings.yml (see #119). Then we could easily extend the definition like this (just a possible way, not thought much about it):

backends:
  cephfs:
    provider: mgr
    settings:
        <optional mgr specific options>
    shares:
      kclient:
        <optional kclient specific options>
      vfs:
        <optional vfs specific options>

This way we could easily create more than one backend, with multiple shares, in a single configuration (additional work required to avoid using the same devices, but that's another problem).

In any case, this is not related to this patch. We can discuss this in #119.

Sure.

For now I've updated to include the suggestion to use only methods avoiding the strategy keyword.

anoopcs9 added a commit to anoopcs9/sit-test-cases that referenced this pull request Aug 28, 2024
With samba-in-kubernetes/sit-environment#117,
'variant' no longer indicates the method through which a particular
share is configured for client access. Instead determine the need for
VFS specific flapping file using the share name itself.

Signed-off-by: Anoop C S <anoopcs@cryptolab.net>
anoopcs9 added a commit to anoopcs9/sit-test-cases that referenced this pull request Aug 28, 2024
With samba-in-kubernetes/sit-environment#117,
'variant' no longer indicates the method through which a particular
share is configured for client access. Instead determine the need for
VFS specific flapping file using the share name itself.

Signed-off-by: Anoop C S <anoopcs@cryptolab.net>
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 anoopcs9 merged commit 8d260b5 into samba-in-kubernetes:main Aug 28, 2024
10 checks passed
@anoopcs9 anoopcs9 deleted the group-backend-shares branch August 28, 2024 09:00
anoopcs9 added a commit to samba-in-kubernetes/samba-centosci that referenced this pull request Aug 28, 2024
With [1] all possible share configurations are grouped after their
respective backend file system. So those duplicate entries can be
removed reducing the number of jobs in the matrix.

[1] samba-in-kubernetes/sit-environment#117
anoopcs9 added a commit to samba-in-kubernetes/samba-centosci that referenced this pull request Aug 28, 2024
With [1] suffix 'vfs' is irrelevant in 'cephfs.mgr.vfs' as multiple
share configurations are included within mgr variant.

[1] samba-in-kubernetes/sit-environment#117
anoopcs9 added a commit to samba-in-kubernetes/samba-centosci that referenced this pull request Aug 28, 2024
With [1] all possible share configurations are grouped after their
respective backend file system. So those duplicate entries can be
removed reducing the number of jobs in the matrix.

[1] samba-in-kubernetes/sit-environment#117
anoopcs9 added a commit to samba-in-kubernetes/samba-centosci that referenced this pull request Aug 28, 2024
With [1] suffix 'vfs' is irrelevant in 'cephfs.mgr.vfs' as multiple
share configurations are included within mgr variant.

[1] samba-in-kubernetes/sit-environment#117
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.

Combine multiple variants within same environment
3 participants