Skip to content

Conversation

@portante
Copy link
Member

@portante portante commented Nov 4, 2021

All three existing benchmark scripts which manipulate network firewalls have been changed to ignore firewalls entirely.

For pbench-fio, this means it no longer attempts to add open ports when clients are used.

For pbench-uperf and pbench-netperf, those scripts no longer disable firewalld explicitly.

Any one using pbench-fio in client mode, or either pbench-uperf or pbench-netperf, must be responsible for properly enabling access on the expected ports.

The documentation for pbench-uperf and pbench-fio have been updated to reflect this new behavior.

Fixed #1217.

@portante portante added bug Agent fio pbench-fio benchmark related uperf pbench-uperf benchmark related labels Nov 4, 2021
@portante portante added this to the v0.71 milestone Nov 4, 2021
@portante portante self-assigned this Nov 4, 2021
@portante portante force-pushed the remove-firewall-cmd branch 2 times, most recently from a489f9e to c6afaaf Compare November 4, 2021 05:41
@portante portante linked an issue Nov 4, 2021 that may be closed by this pull request
dbutenhof
dbutenhof previously approved these changes Nov 4, 2021
Copy link
Member

@dbutenhof dbutenhof left a comment

Choose a reason for hiding this comment

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

I think the indentation is funky in one place; but otherwise just a few side comments. Although given the assorted fixes across the layers of commit here, I do wonder about whether you should consider changing the PR title...

Copy link
Member

@webbnh webbnh left a comment

Choose a reason for hiding this comment

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

This could use some polishing.

portante added a commit to portante/pbench that referenced this pull request Nov 5, 2021
Originally part of PR distributed-system-analysis#2524, we correct the usage output to be emitted
on `stderr` in error conditions, and to `stdout` for the one non-error
condition, `--help`.

Further we remove the annoying trailing semi-colons, and remove the
extra `shift` statement while processing options.

Lastly, unrecognized parameters now cause an error.
@portante
Copy link
Member Author

portante commented Nov 5, 2021

FWIW, I created PR #2526 to track the pbench-fio usage changes in a separate commit along with some related cleanup.

@portante portante force-pushed the remove-firewall-cmd branch from c6afaaf to 81576a5 Compare November 5, 2021 12:01
@portante portante marked this pull request as draft November 5, 2021 12:12
@portante
Copy link
Member Author

portante commented Nov 5, 2021

I've decided to convert this to a Draft so that I can break this PR out a bit. The first break-out is PR #2526. Once that is accepted and merged, I'll post another break-out PR.

dbutenhof
dbutenhof previously approved these changes Nov 5, 2021
@portante portante removed the uperf pbench-uperf benchmark related label Nov 5, 2021
webbnh pushed a commit that referenced this pull request Nov 5, 2021
Originally part of PR #2524, we correct the usage output to be emitted
on `stderr` in error conditions, and to `stdout` for the one non-error
condition, `--help`.

Further we remove the annoying trailing semi-colons, and remove the
extra `shift` statement while processing options.

Lastly, unrecognized parameters now cause an error.
@portante portante force-pushed the remove-firewall-cmd branch 2 times, most recently from 7cc3b26 to e446b74 Compare November 5, 2021 21:06
@portante portante marked this pull request as ready for review November 5, 2021 21:06
@portante portante requested review from dbutenhof and webbnh November 5, 2021 21:11
dbutenhof
dbutenhof previously approved these changes Nov 5, 2021
All three existing benchmark scripts which manipulate network firewalls
have been changed to ignore firewalls entirely.

For `pbench-fio`, this means it no longer attempts to add open ports
when clients are used.

For `pbench-uperf` and `pbench-netperf`, those scripts no longer disable
`firewalld` explicitly.

Any one using `pbench-fio` in client mode, or either `pbench-uperf` or
`pbench-netperf`, must be responsible for properly enabling access on
the expected ports.

The documentation for `pbench-uperf` and `pbench-fio` have been updated
to reflect this new behavior.
Copy link
Member

@webbnh webbnh left a comment

Choose a reason for hiding this comment

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

GTG

@portante portante merged commit f6be81f into distributed-system-analysis:main Nov 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Agent bug fio pbench-fio benchmark related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Stop using firewall-cmd in pbench-fio

3 participants