Add Supplementary group tests #63

spuiuk · 2024-01-31T19:46:11Z

*** EDIT - Step 1 is now handled in PR #73. Please use this PR only for the supplementary group tests ***
Step 1:
Change the way we describe exports in the test-info.yml file.

Introduce exported_shares option.

exported_shares:
  - {"name": "export2"}
  - {"name": "export3", "mntdir": "/mnt-cephfs", "fs": "cephfs"}

This allows us to describe both plain exportes smbshares plus premounted shares and the backing filesystem.

remove section premounted_shares.
These aren't used in the automated testing yet and can be described through exported_shares section.

Step 2:
Add supplementary group test.

The motivation for making this changes is to cater to a new type of tests which require access to the underlying filesystem outside of SMB/samba. Changes to test-info.yml allow an easy way to describe an exported share where I also give the location of the direct mount so that we can perform these filesystem operations.

The new test added is a Supplementary group test where the following requirements are needed.

user defined in test-info.yml should be part of the sg supplementary group.
Access to the backend fs outside of SMB is needed - We need to setup a directory to which the supplementary group has write access. We test if the user who is part of the supplementary group has write access.
In this case, we cannot write to the backend using vfs_cephfs module but the write on a filesystem mounted using the ceph-fuse module works fine.

Depends on #73

Shwetha-Acharya · 2024-02-08T06:04:34Z

test-info.yml.example

@@ -9,12 +9,16 @@ public_interfaces:
 exported_sharenames:
  - "gluster-vol"

+# List of dict of exported shares
+# name: share_name
+# mntdir: If this is a premounted share, the mount directory


we can rephrase it as mount directory in case of a premounted share

Shwetha-Acharya · 2024-02-08T06:20:25Z

testhelper/testhelper.py

+    if "shares" in test_info:
+        return test_info["shares"]
+
+    default_fs = test_info.get("test_backend", None)


Is it necessary to use None here as default value when a key is not present in dict is already None

Shwetha-Acharya · 2024-02-08T06:21:51Z

test-info.yml.example

@@ -9,12 +9,16 @@ public_interfaces:
 exported_sharenames:
  - "gluster-vol"

+# List of dict of exported shares


Should this be exported_shares: list of dict of exported shares

Shwetha-Acharya · 2024-02-08T06:21:54Z

test-info.yml.example

@@ -9,12 +9,16 @@ public_interfaces:
 exported_sharenames:
  - "gluster-vol"

+# List of dict of exported shares
+# name: share_name
+# mntdir: If this is a premounted share, the mount directory


mntdir: mount directory in a premounted share would sound better

Shwetha-Acharya · 2024-02-08T06:23:20Z

testhelper/testhelper.py

+    """
+    Is the share dict a premounted share
+
+    Paramters:


typo: parameters

Shwetha-Acharya · 2024-02-08T06:23:59Z

testhelper/testhelper.py

+
+def is_premounted_share(share: dict) -> bool:
+    """
+    Is the share dict a premounted share


check if the share is premounted would sound better

Shwetha-Acharya

May be you can describe this change in more detail in the commit message (misc: Add test for supplementary groups for some of us to know better about the need of test for supplementary groups

spuiuk · 2024-02-22T16:07:04Z

May be you can describe this change in more detail in the commit message (misc: Add test for supplementary groups for some of us to know better about the need of test for supplementary groups

Instead of the commit message, I've added it to the python file directly. This allows users to get the information directly from the test files.

Shwetha-Acharya

Added few comments.
Rest of the implementation looks good to me.

Shwetha-Acharya · 2024-02-27T11:19:56Z

test-info.yml.example

@@ -9,12 +9,16 @@ public_interfaces:
 exported_sharenames:
  - "gluster-vol"

+# exported_shares: List of dict of exported shares


Using multi line comment here similar to testheper.py would be better as it would appear more readable

This is an example file for yaml content. Yaml doesn't allow multi line comments like in Python.

Shwetha-Acharya · 2024-02-27T11:50:50Z

test-info.yml.example

@@ -9,12 +9,16 @@ public_interfaces:
 exported_sharenames:
  - "gluster-vol"

+# exported_shares: List of dict of exported shares
+# name: share_name


just curious why we would need a separate exported_sharenames and again a name exported_shares.

Let me know if they are any different or there is any special use case? If not could we just use share name from exported_shares?

They have 2 different formats in the way they are expressed in the yaml. exported_sharenames is how it is currently reported but it only lists the shares exported from that sharename which is inadequate for a complex test. This is currently used by sit-environment to report the shares but in the future we want to move to providing additional information in exported_shares which the current exported_sharename field doesn't provide.

Since we have multiple projects, we have a chicken and egg problem. We can't make changes to one which will break the testing on the other. So for now, the changes ensure that both exported_sharenames and exported_shares are supported. Once these changes go in, I will put in a PR to move sit-environment to use exported_shares instead of exported_sharenames. Once changes go into both, we can consider removing support for exported_sharenames.

xhernandez

Since SIT environment is becoming more configurable now, and it could provide more complex configurations in the near future, I think we should also make the test-info.yml configuration more flexible.

I would nest all configuration below each share, so that different share can have different configurations.

For example:

shares:
  share-cephfs-vfs:
    # If present, it means share is pre-mounted
    path: /mnt/share/share-cephfs-vfs
    backend:
      name: cephfs
      variant: vfs
      # If present, this provides direct access to the backend, bypassing Samba
      path: /mnt/backend/share
    users:
      test1: x
      test2: x
    addresses:
      - 192.168.123.10
      - 192.168.123.11

spuiuk · 2024-02-28T10:58:04Z

Thanks, As discussed on chat, we will also add options to add default options which will make it easier to write the test-info.yml files manually.

spuiuk · 2024-03-07T13:13:45Z

I have uploaded the latest changes. The supplementary tests will not be automatically run in the sit-environment since we have not setup backend filesystems and the sit-environment will have to be modified to use the new format for test-info.yml before we can run the supplementary tests.

However the changes introduced here changes the way we layout the environment in test-info.yml.

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]>

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]>

Add share["backend"]["path"] to indicate where direct access to the share is available. Add helper function get_shares_with_directmnt() to locate shares with direct access to the exported path available Signed-off-by: Sachin Prabhu <[email protected]>

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

spuiuk · 2024-03-23T01:57:23Z

Depends on #73

spuiuk · 2024-03-23T01:58:14Z

Split this PR with the changes to test-info.yml now handled in #73. Please use this PR to comment on supplementary tests.

dpulls · 2024-03-26T21:00:00Z

🎉 All dependencies have been resolved !

spuiuk · 2024-05-02T09:11:13Z

Closing this issue in favour of PR #79.

There are several changes made to the underlying code which necessitates re-writing a large part of the testing infrastructure. This includes the changes to the way test-info.yaml is specified as well as using the pysmb module to connect to the remote smb server.

spuiuk marked this pull request as draft January 31, 2024 19:47

spuiuk force-pushed the exports_fix branch 2 times, most recently from 29a4f47 to 4a4789c Compare February 1, 2024 15:19

spuiuk changed the title ~~Exports fix~~ Add Supplementary group tests Feb 1, 2024

spuiuk requested review from gd, anoopcs9, synarete, Shwetha-Acharya and phlogistonjohn February 1, 2024 16:37

spuiuk force-pushed the exports_fix branch 2 times, most recently from 8f845a8 to 37c6e1a Compare February 7, 2024 17:33

spuiuk marked this pull request as ready for review February 7, 2024 17:33

spuiuk force-pushed the exports_fix branch 2 times, most recently from 76d7f7d to 08863bc Compare February 7, 2024 17:38

Shwetha-Acharya reviewed Feb 8, 2024

View reviewed changes

spuiuk force-pushed the exports_fix branch 2 times, most recently from 9aa05e0 to 8026a1d Compare February 22, 2024 16:05

Shwetha-Acharya reviewed Feb 27, 2024

View reviewed changes

xhernandez reviewed Feb 27, 2024

View reviewed changes

spuiuk force-pushed the exports_fix branch 3 times, most recently from deb1772 to bbc0173 Compare March 5, 2024 22:40

spuiuk added 3 commits March 23, 2024 01:41

test-info.yml: remove private_interfaces from examples

c0d99df

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]>

testcases: introduce get_exported_shares() helper function

205468d

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 mentioned this pull request Mar 23, 2024

test-info.yml changes #73

Merged

spuiuk added 2 commits March 23, 2024 01:55

misc: Add test for supplementary groups

469175b

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

spuiuk force-pushed the exports_fix branch from bbc0173 to 469175b Compare March 23, 2024 01:55

anoopcs9 mentioned this pull request May 2, 2024

Supplementary tests #79

Open

spuiuk closed this May 2, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Supplementary group tests #63

Add Supplementary group tests #63

spuiuk commented Jan 31, 2024 •

edited

Loading

Shwetha-Acharya Feb 8, 2024

spuiuk Feb 22, 2024

Shwetha-Acharya Feb 8, 2024

spuiuk Feb 22, 2024

Shwetha-Acharya Feb 8, 2024

spuiuk Feb 22, 2024

Shwetha-Acharya Feb 8, 2024

spuiuk Feb 22, 2024

Shwetha-Acharya Feb 8, 2024

spuiuk Feb 22, 2024

Shwetha-Acharya Feb 8, 2024

spuiuk Feb 22, 2024

Shwetha-Acharya left a comment •

edited

Loading

spuiuk commented Feb 22, 2024

Shwetha-Acharya left a comment

Shwetha-Acharya Feb 27, 2024

spuiuk Feb 27, 2024

Shwetha-Acharya Feb 27, 2024

spuiuk Feb 27, 2024

xhernandez left a comment •

edited

Loading

spuiuk commented Feb 28, 2024

spuiuk commented Mar 7, 2024

spuiuk commented Mar 23, 2024 •

edited

Loading

spuiuk commented Mar 23, 2024

dpulls bot commented Mar 26, 2024

spuiuk commented May 2, 2024

Add Supplementary group tests #63

Add Supplementary group tests #63

Conversation

spuiuk commented Jan 31, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Shwetha-Acharya left a comment • edited Loading

Choose a reason for hiding this comment

spuiuk commented Feb 22, 2024

Shwetha-Acharya left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xhernandez left a comment • edited Loading

Choose a reason for hiding this comment

spuiuk commented Feb 28, 2024

spuiuk commented Mar 7, 2024

spuiuk commented Mar 23, 2024 • edited Loading

spuiuk commented Mar 23, 2024

dpulls bot commented Mar 26, 2024

spuiuk commented May 2, 2024

spuiuk commented Jan 31, 2024 •

edited

Loading

Shwetha-Acharya left a comment •

edited

Loading

xhernandez left a comment •

edited

Loading

spuiuk commented Mar 23, 2024 •

edited

Loading