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-16585 tests: Fix NLT handling of __fxstat detection #15150

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions ci/unit/required_packages.sh
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ pkgs="argobots \
pmix \
protobuf-c \
spdk-devel \
strace \
valgrind-devel"

# output with trailing newline suppressed
Expand Down
21 changes: 19 additions & 2 deletions utils/node_local_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -1521,8 +1521,8 @@ def il_cmd(self, cmd, check_read=True, check_write=True, check_fstat=True):
print(f'Logged il to {log_name}')
print(ret)

if self.caching:
check_fstat = False
check_fstat = check_fstat and not self.caching and \
look_for_library_call(self.conf, cmd, '__fxstat')
Comment on lines +1524 to +1525
Copy link
Contributor

Choose a reason for hiding this comment

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

This would be called for every il_cmd invocation and there are probably dozens so it could/should be saved in conf

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought about that and wanted to not assume different executables ended up using the same libraries. And, I don't think it's going to affect overall runtime to just do this every time. Do you feel differently?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd missed that it was running the actual command rather than just a generic unix command here. This means the command has to be idempotent and perform the same operations on subsequent invocations but looking at the places where this is called it seems it probably is.


try:
log_test(self.conf, log_name, check_read=check_read, check_write=check_write,
Expand Down Expand Up @@ -6327,6 +6327,23 @@ def server_fi(args):
server.set_fi(probability=0)


def look_for_library_call(conf, cmd, library_str):
# pylint: disable=wrong-spelling-in-docstring
"""Look for library_str in the strace call stack of running cmd."""
with tempfile.NamedTemporaryFile(mode='r',
prefix='dnt_assess_',
suffix='.strace',
dir=conf.tmp_dir) as tmpfile:
strace_cmd = ['strace', '--stack-traces', '-v', '-o', tmpfile.name] + cmd
subprocess.run(strace_cmd, check=True)
lines = tmpfile.readlines()
for line in lines:
if re.search(library_str, line):
return True

return False


def run(wf, args):
"""Main entry point"""
# pylint: disable=too-many-branches
Expand Down
1 change: 1 addition & 0 deletions utils/scripts/install-el8.sh
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ dnf --nodocs install \
python3-devel \
python3-pip \
sg3_utils \
strace \
sudo \
systemd \
valgrind-devel \
Expand Down
1 change: 1 addition & 0 deletions utils/scripts/install-el9.sh
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ dnf --nodocs install \
python3-devel \
python3-pip \
sg3_utils \
strace \
sudo \
valgrind-devel \
which \
Expand Down
1 change: 1 addition & 0 deletions utils/scripts/install-leap15.sh
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ dnf --nodocs install \
python3-devel \
scons \
sg3_utils \
strace \
sudo \
valgrind-devel \
which \
Expand Down
8 changes: 8 additions & 0 deletions utils/scripts/install-ubuntu.sh
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,13 @@ set -e

arch=$(uname -i)

# hack to install 24.04's golang-go on 22.04:
apt-get install -y software-properties-common
add-apt-repository "deb http://archive.ubuntu.com/ubuntu noble main"
apt-get update
apt-get install -y golang-go
add-apt-repository -r "deb http://archive.ubuntu.com/ubuntu noble main"

Comment on lines +17 to +23
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a part of the fix or something else?

Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a part of the fix or something else?

there was a recent change that made it such that made go 1.22 a requirement and ubuntu 22 has 1.18. Not sure why the builds didn't fail on that patch.

Copy link
Contributor

Choose a reason for hiding this comment

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

This would probably be better landing as part of #15174 where the go developers can review it.

That said, this script normally just installs packages, https://github.com/daos-stack/daos/blob/master/utils/docker/Dockerfile.ubuntu would be a better place for this code, or perhaps a utils/scripts/helpers/repo-helper-debian.sh script to match what rocky does.

apt-get install \
autoconf \
build-essential \
Expand Down Expand Up @@ -52,6 +59,7 @@ apt-get install \
pkg-config \
python3-dev \
python3-venv \
strace \
uuid-dev \
valgrind \
yasm
Expand Down
Loading