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

DAOS-14826 test: Add a dfuse/pil4dfs test for bash fd usage. #13681

Merged
merged 6 commits into from
Feb 1, 2024

Conversation

ashleypittman
Copy link
Contributor

Add a test for pil4dfs that tries to replicate what configure
scripts do with file descriptors.

Required-githooks: true
Test-tag: FdTest
Skip-init-tests: true

Signed-off-by: Ashley Pittman [email protected]

Copy link

github-actions bot commented Jan 29, 2024

Bug-tracker data:
Ticket title is 'hang in configure with libpil4dfs.so loaded'
Status is 'Resolved'
Labels: 'intercept_lib'
https://daosio.atlassian.net/browse/DAOS-14826

Add a test for pil4dfs that tries to replicate what configure
scripts do with file descriptors.

Required-githooks: true
Test-tag: FdTest
Skip-init-tests: true

Signed-off-by: Ashley Pittman <[email protected]>
@daosbuild1
Copy link
Collaborator

Test stage Build RPM on EL 9 completed with status UNSTABLE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-13681/2/execution/node/358/log

@daosbuild1
Copy link
Collaborator

Test stage Build RPM on EL 8 completed with status UNSTABLE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-13681/2/execution/node/361/log

@daosbuild1
Copy link
Collaborator

Test stage Build RPM on Leap 15.5 completed with status UNSTABLE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-13681/2/execution/node/278/log

@daosbuild1
Copy link
Collaborator

Test stage Build DEB on Ubuntu 20.04 completed with status UNSTABLE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-13681/2/execution/node/346/log

@daosbuild1
Copy link
Collaborator

Test stage Unit Test on EL 8.8 completed with status UNSTABLE. https://build.hpdd.intel.com/job/daos-stack/job/daos//view/change-requests/job/PR-13681/2/testReport/

Required-githooks: true
Test-tag: FdTest
Skip-unit-tests: true
Signed-off-by: Ashley Pittman <[email protected]>
@daosbuild1
Copy link
Collaborator

Test stage Functional on EL 8.8 completed with status UNSTABLE. https://build.hpdd.intel.com/job/daos-stack/job/daos//view/change-requests/job/PR-13681/3/testReport/

@ashleypittman ashleypittman marked this pull request as ready for review January 29, 2024 13:59
@ashleypittman ashleypittman requested a review from a team as a code owner January 29, 2024 13:59
Required-githooks: true

Signed-off-by: Ashley Pittman <[email protected]>
@ashleypittman
Copy link
Contributor Author

Results are at https://build.hpdd.intel.com/job/daos-stack/job/daos//view/change-requests/job/PR-13681/3/testReport/FTEST_dfuse/

Test passed for dfuse and ioil, failed for pil4dfs as expected. The test is weekly so could probably land before #13532

@daosbuild1
Copy link
Collaborator

Test stage Unit Test on EL 8.8 completed with status UNSTABLE. https://build.hpdd.intel.com/job/daos-stack/job/daos//view/change-requests/job/PR-13681/4/testReport/

wiliamhuang
wiliamhuang previously approved these changes Jan 29, 2024
@wiliamhuang
Copy link
Contributor

The new test works fine in my local test.

@mchaarawi
Copy link
Contributor

please rebase with latest master and the test-tag

Comment on lines 86 to 90
def run_bashfd(self, il_lib=None):
"""Run a shell script which opens and writes to files.

This attempts to replicate the way that configure scripts manipulate fds in bash.
"""
Copy link
Contributor

Choose a reason for hiding this comment

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

We try to document all arguments

Suggested change
def run_bashfd(self, il_lib=None):
"""Run a shell script which opens and writes to files.
This attempts to replicate the way that configure scripts manipulate fds in bash.
"""
def run_bashfd(self, il_lib=None):
"""Run a shell script which opens and writes to files.
This attempts to replicate the way that configure scripts manipulate fds in bash.
Args:
il_lib (str, optional): interception library to run with. Defaults to None
"""

Comment on lines 98 to 101
self.add_pool(connect=False)
self.add_container(self.pool)
mount_dir = f"/tmp/{self.pool.identifier}_daos_dfuse"
self.start_dfuse(self.hostlist_clients, self.pool, self.container, mount_dir=mount_dir)
Copy link
Contributor

Choose a reason for hiding this comment

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

We're trying to move away from these "magic" pool and container functions and instead use explicit variables. Also, you could let the framework generic a mount_dir for you.

Suggested change
self.add_pool(connect=False)
self.add_container(self.pool)
mount_dir = f"/tmp/{self.pool.identifier}_daos_dfuse"
self.start_dfuse(self.hostlist_clients, self.pool, self.container, mount_dir=mount_dir)
pool = self.get_pool(connect=False)
container = self.get_container(pool)
self.start_dfuse(self.hostlist_clients, pool, container)

Comment on lines +105 to +119
with open(os.path.join(fuse_root_dir, "bash_fd_inner.sh"), "w") as fd:
fd.write(INNER)

os.chmod(os.path.join(fuse_root_dir, "bash_fd_inner.sh"), stat.S_IXUSR | stat.S_IRUSR)

with open(os.path.join(fuse_root_dir, "bash_fd_outer.sh"), "w") as fd:
fd.write(OUTER)

os.chmod(os.path.join(fuse_root_dir, "bash_fd_outer.sh"), stat.S_IXUSR | stat.S_IRUSR)

cmd = f"cd {fuse_root_dir}; ./bash_fd_outer.sh"

result = run_remote(self.log, self.hostlist_clients, env_str + cmd)
if not result.passed:
self.fail(f'"{cmd}" failed on {result.failed_hosts}')
Copy link
Contributor

Choose a reason for hiding this comment

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

Just pointing out that this creates local files and then issues a remote command to execute them. This only works because the test runner is also the only client. That's okay for now, but eventually (not sure when) we'd like to not rely on the test runner being a client.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there a "write remote file" option in ftest yet? There's no other way to write fuse tests now, and there never has been.

Copy link
Contributor

Choose a reason for hiding this comment

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

No, there isn't. But it's something we'll likely need in the future. For now, I think this approach is okay.

Comment on lines 121 to 126
# stop dfuse
self.stop_dfuse()
# destroy container
self.container.destroy()
# destroy pool
self.pool.destroy()
Copy link
Contributor

Choose a reason for hiding this comment

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

This generally isn't necessary. It will all be handled in teardown

Test Description:
Test a typical I/O pattern for bash based configure scripts.

:avocado: tags=all,pr,full_regression
Copy link
Contributor

Choose a reason for hiding this comment

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

pr and weekly? Is that intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Forgot to update this one, no that's not intentional.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Re-pushed. This was the only change.

Required-githooks: true
Signed-off-by: Ashley Pittman <[email protected]>
Test-tag: DFuseFdTest
Required-githooks: true
daltonbohning
daltonbohning previously approved these changes Jan 30, 2024
wiliamhuang
wiliamhuang previously approved these changes Jan 31, 2024
Test-tag: DFuseFdTest
Required-githooks: true
Signed-off-by: Ashley Pittman <[email protected]>
@ashleypittman ashleypittman requested a review from a team February 1, 2024 18:41
@daltonbohning daltonbohning merged commit 0c3ebbd into master Feb 1, 2024
46 checks passed
@daltonbohning daltonbohning deleted the amd/bash-fd-test branch February 1, 2024 18:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants