Skip to content

DAOS-15603: Support of FIO with ansible ftest script - #14138

Merged
daltonbohning merged 22 commits into
masterfrom
ckochhof/dev/master/daos-15603
May 3, 2024
Merged

DAOS-15603: Support of FIO with ansible ftest script#14138
daltonbohning merged 22 commits into
masterfrom
ckochhof/dev/master/daos-15603

Conversation

@knard38

@knard38 knard38 commented Apr 9, 2024

Copy link
Copy Markdown
Contributor

Description

Miscellaneous improvements allowing to run fio functional tests, and to use the shared cluster.

Required-githooks: true

Before requesting gatekeeper:

  • Two review approvals and any prior change requests have been resolved.
  • Testing is complete and all tests passed or there is a reason documented in the PR why it should be force landed and forced-landing tag is set.
  • Features: (or Test-tag*) commit pragma was used or there is a reason documented that there are no appropriate tags for this PR.
  • Commit messages follows the guidelines outlined here.
  • Any tests skipped by the ticket being addressed have been run and passed in the PR.

Gatekeeper:

  • You are the appropriate gatekeeper to be landing the patch.
  • The PR has 2 reviews by people familiar with the code, including appropriate owners.
  • Githooks were used. If not, request that user install them and check copyright dates.
  • Checkpatch issues are resolved. Pay particular attention to ones that will show up on future PRs.
  • All builds have passed. Check non-required builds for any new compiler warnings.
  • Sufficient testing is done. Check feature pragmas and test tags and that tests skipped for the ticket are run and now pass with the changes.
  • If applicable, the PR has addressed any potential version compatibility issues.
  • Check the target branch. If it is master branch, should the PR go to a feature branch? If it is a release branch, does it have merge approval in the JIRA ticket.
  • Extra checks if forced landing is requested
    • Review comments are sufficiently resolved, particularly by prior reviewers that requested changes.
    • No new NLT or valgrind warnings. Check the classic view.
    • Quick-build or Quick-functional is not used.
  • Fix the commit message upon landing. Check the standard here. Edit it to create a single commit. If necessary, ask submitter for a new summary.

@github-actions

github-actions Bot commented Apr 9, 2024

Copy link
Copy Markdown

Errors are component not formatted correctly,Ticket number suffix is not a number. See https://daosio.atlassian.net/wiki/spaces/DC/pages/11133911069/Commit+Comments,Unable to load ticket data
https://daosio.atlassian.net/browse/DAOS-15603:

@knard38 knard38 self-assigned this Apr 9, 2024
@daosbuild1

Copy link
Copy Markdown
Collaborator

Test stage Functional on EL 8.8 completed with status FAILURE. https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-14138/1/display/redirect

@knard38
knard38 force-pushed the ckochhof/dev/master/daos-15603 branch from bfd9960 to 89df007 Compare April 9, 2024 20:18
Miscellaneous improvements allowing to run fio functional tests, and to
use the shared cluster.

Doc-only: true
Required-githooks: true

Signed-off-by: Cedric Koch-Hofer <cedric.koch-hofer@intel.com>
@knard38
knard38 force-pushed the ckochhof/dev/master/daos-15603 branch from 89df007 to 22f74c8 Compare April 10, 2024 07:21
@knard38
knard38 marked this pull request as ready for review April 10, 2024 07:26
@knard38
knard38 requested a review from a team as a code owner April 10, 2024 07:26
Comment thread utils/ansible/ftest/file/enable-coredumps.sh
all:
vars:
daos_runtime_dir: /home/foo/daos
# daos_ofi_provider: ofi+tcp;ofi_rxm

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If you comment it out for now and plan to use it later, you might want to add some explanation.

@knard38 knard38 Apr 12, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This comment present and example of an optional parameters.
if it is confusing, I can remove it or add something like this

Suggested change
# daos_ofi_provider: ofi+tcp;ofi_rxm
# Optional parameter
# daos_ofi_provider: ofi+tcp;ofi_rxm

According to you preference, I will do the same thing for line 11 and 13 of this inventory example.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For me, I think it's better to have the comment as you added.

@knard38 knard38 Apr 12, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

  • Add default parameter explanation

@knard38 knard38 Apr 12, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

  • Add default parameter explanation

Fixed with commit 07b4777, d9fa641 and 712254e

shimizukko
shimizukko previously approved these changes Apr 12, 2024
Comment thread utils/ansible/ftest/templates/daos-make.sh.j2 Outdated
Comment thread utils/ansible/ftest/templates/daos-make.sh.j2 Outdated
Integrate reviewers comments:
- Replace scons-3 with scons
- Add default parameter explanation
- Miscellaneous fixes and improvements

Doc-only: true
Required-githooks: true

Signed-off-by: Cedric Koch-Hofer <cedric.koch-hofer@intel.com>
Fix code spell

Doc-only: true
Required-githooks: true

Signed-off-by: Cedric Koch-Hofer <cedric.koch-hofer@intel.com>
Fix yaml lint check

Doc-only: true
Required-githooks: true

Signed-off-by: Cedric Koch-Hofer <cedric.koch-hofer@intel.com>
Fix yaml lint check

Doc-only: true
Required-githooks: true

Signed-off-by: Cedric Koch-Hofer <cedric.koch-hofer@intel.com>
Comment thread utils/ansible/ftest/templates/daos-launch.sh.j2 Outdated
Integrate reviewers comments:
- Remove useless set +u before activate

Doc-only: true
Required-githooks: true

Signed-off-by: Cedric Koch-Hofer <cedric.koch-hofer@intel.com>
shimizukko
shimizukko previously approved these changes Apr 12, 2024
Comment thread utils/ansible/ftest/templates/daos-make.sh.j2 Outdated
Comment thread utils/ansible/ftest/templates/daos-make.sh.j2 Outdated
Comment thread utils/ansible/ftest/templates/daos-make.sh.j2 Outdated
Comment thread utils/ansible/ftest/templates/daos-make.sh.j2 Outdated
Integrate reviewers comments:
- Remove useless python related packages
- Remove useless option and add a comment
- Remove useless opts

Doc-only: true
Required-githooks: true

Signed-off-by: Cedric Koch-Hofer <cedric.koch-hofer@intel.com>
@knard38
knard38 requested a review from ashleypittman April 25, 2024 15:11
shimizukko
shimizukko previously approved these changes Apr 26, 2024
ashleypittman
ashleypittman previously approved these changes Apr 26, 2024

@ashleypittman ashleypittman left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Build/setup changes look goot to me.

Comment thread utils/ansible/ftest/vars/Rocky8.yml Outdated
Comment thread utils/ansible/ftest/vars/Rocky8.yml
Fix regression with force install

Doc-only: true
Required-githooks: true

Signed-off-by: Cedric Koch-Hofer <cedric.koch-hofer@intel.com>
Integrate reviewers comments:
- Remove useless dependencies

Doc-only: true
Required-githooks: true

Signed-off-by: Cedric Koch-Hofer <cedric.koch-hofer@intel.com>
@knard38
knard38 dismissed stale reviews from ashleypittman and shimizukko via 03ae132 April 26, 2024 08:52
ashleypittman
ashleypittman previously approved these changes Apr 26, 2024
shimizukko
shimizukko previously approved these changes Apr 26, 2024
@knard38
knard38 requested a review from a team April 26, 2024 09:49
@knard38

knard38 commented Apr 26, 2024

Copy link
Copy Markdown
Contributor Author

@daos-stack/daos-gatekeeper please could you lend this PR with the following commit message:
Title: DAOS-15603 test: Support of FIO with ansible ftest script
Message:

Add support to build and run fio functional tests.
Add support of the shared cluster nodes.
Add support of MOFED libraries.
Miscellaneous improvements of the build process:
- improvement of scons usage
- use of virtualenv with pip
- etc...

Comment on lines +6 to +8
if [[ $(id -u) -ne 0 ]] ; then
echo "[ERROR] Could only be used by root"
fi

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should this exit 1?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Arghhhh, thanks for your attention.

@knard38 knard38 May 2, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

  • Fix invalid missing exit 1

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

  • Fix invalid missing exit 1

Fixed wit commit 7d876bc

Comment thread utils/ansible/ftest/file/enable-coredumps.sh
kanard38 added 2 commits May 2, 2024 13:26
Integrate reviewers comments:
- Fix invalid missing exit 1

Doc-only: true
Required-githooks: true

Signed-off-by: Cedric Koch-Hofer <cedric.koch-hofer@intel.com>
Build and install FIO man pages

Doc-only: true
Required-githooks: true

Signed-off-by: Cedric Koch-Hofer <cedric.koch-hofer@intel.com>
@knard38
knard38 requested a review from a team May 3, 2024 05:56
@daltonbohning
daltonbohning merged commit 98e8b81 into master May 3, 2024
@daltonbohning
daltonbohning deleted the ckochhof/dev/master/daos-15603 branch May 3, 2024 14:16
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.

6 participants