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

test-info.yml changes #73

Merged
merged 4 commits into from
Mar 26, 2024
Merged

Conversation

spuiuk
Copy link
Collaborator

@spuiuk spuiuk commented Mar 23, 2024

Splitting this set of changes out of PR #63.

These commits change the way we describe shares in test-info.yml file.

These changes will allow for us to set server, backend fs type and users for each individual share. This means that shares across multiple smb shares and filesystems can be tested at the same time.

# Server hostname to be tested
server: server_name

# Users to use for authentication
users:
  user1: user1password

# Backend filesystem of the exported shares
backend: glusterfs

# shares: List of dict of exported shares
shares:
  # share name export1
  export1:
    # If present, it means the share is pre-mounted
    path: /mnt/share/export1-cephfs-vfs
    backend:
      # If present backend filesystem
      name: cephfs.vfs
    # If present, username to perform the tests with on this share
    users:
      test2: x
    # If present, the hostname to use to connect to the test server
    server: hostname1
  # share name export2
  export2:
    # Use default values set for this share

With these changes, we can still continue using the old format to describe test-info.yml but they will be deprecated and removed once we have made the changes to the sit-environment to use the new format.

Copy link
Collaborator

@synarete synarete left a comment

Choose a reason for hiding this comment

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

The overall logic is fine, but needs a bit of (Python style) cleanups.

testhelper/testhelper.py Outdated Show resolved Hide resolved
testhelper/testhelper.py Outdated Show resolved Hide resolved
testhelper/testhelper.py Outdated Show resolved Hide resolved
testhelper/testhelper.py Outdated Show resolved Hide resolved
We do not use this section in any of our current tests and I don't
forsee the need for this in the near future.

Signed-off-by: Sachin Prabhu <[email protected]>
Avoid directly accessing the test_info dict and use helper function
instead. This will allow us to modify the test_info.yml with minimal
disruption.

Signed-off-by: Sachin Prabhu <[email protected]>
@spuiuk spuiuk force-pushed the test-info.changes branch 3 times, most recently from 5290051 to 4942b83 Compare March 24, 2024 18:27
testinfo = testhelper.read_yaml("test-info1.yml")
arr = []
for share in testhelper.get_exported_shares(testinfo):
s = testhelper.get_share(testinfo, share)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am not very comfortable with one letter variable name, I would vote for something more descriptive like share_info

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have made the changes.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for the update, I still see some occurrences of 's' in testhelper/testhelper.py
Please see if all such occurrences can be updated as well.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes. but not in this set of changes.

Copy link
Collaborator

@synarete synarete left a comment

Choose a reason for hiding this comment

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

Looks fine, but few style issues.

testhelper/testhelper.py Outdated Show resolved Hide resolved
testhelper/testhelper.py Outdated Show resolved Hide resolved
Change the way we list shares in test-info.yml
This will be used to add additional functionality to tests.

eg:
shares:
  # share name export1
  export1:
    # If present, it means the share is pre-mounted
    path: /mnt/share/export1-cephfs-vfs
    backend:
      # If present backend filesystem
      name: cephfs.vfs
    # If present, username to perform the tests with on this share
    users:
      test2: x
    # If present, the hostname to use to connect to the test server
    server: hostname1
  export2:

This commit still supports the old format of test-info.yml. The new
changes deprecates the old test-info.yml format which can be removed
once the sit-environment has been updated.

Signed-off-by: Sachin Prabhu <[email protected]>
With the latest updates to testhelper, the complexity of the helper code
has increased and there is now a good case for adding selftests to
ensure that the helper code works as expected.

We start by creating a few simple tests for the code in
testhelper/testhelper.py.

Signed-off-by: Sachin Prabhu <[email protected]>
Copy link
Collaborator

@synarete synarete left a comment

Choose a reason for hiding this comment

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

lgtm

@spuiuk spuiuk merged commit d1d28f1 into samba-in-kubernetes:main Mar 26, 2024
8 of 10 checks passed
@spuiuk spuiuk deleted the test-info.changes branch March 26, 2024 21:00
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.

3 participants