Skip to content

Conversation

@portante
Copy link
Member

Fixes #2142.

We need to support a minimum version of an RPM in addition to an exact version. We use the rpmdev-vercmp from the rpmdevtools RPM in order to perform the matching.

@portante portante added bug Agent fio pbench-fio benchmark related uperf pbench-uperf benchmark related tools Of and related to the operation and behavior of various tools (iostat, sar, etc.) packaging Issues related to software packaging labels Apr 10, 2021
@portante portante added this to the v0.71 milestone Apr 10, 2021
@portante portante self-assigned this Apr 10, 2021
@portante portante requested review from ndokos and webbnh April 12, 2021 14:47
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.

Dmnt, apparently I failed to push "submit" on this a few hours ago.

Now I gotta go review the updates.

@webbnh
Copy link
Member

webbnh commented Apr 12, 2021

OK, apparently I was confused -- there are no updates.

So, my principal concern is with how these changes will work when the arguments to require-rpm are empty or missing. If you're comfortable with those details (and the rest of the nits I picked out), then I can change to "Approved".

@portante portante requested review from ndokos and webbnh April 12, 2021 22:13
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.

It all looks good except for the quotation problem with the command substitution invocation of require-rpm that we discussed above.

ndokos
ndokos previously approved these changes Apr 13, 2021
Copy link
Member Author

@portante portante left a comment

Choose a reason for hiding this comment

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

Lots of test file changes, but this now seems complete and ready for a file review.

webbnh
webbnh previously approved these changes Apr 15, 2021
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.

I found a few small things, and I have a couple of suggestions, but I'll leave them up to you.

webbnh
webbnh previously approved these changes Apr 16, 2021
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.

Good to go, more or less.

I have a bunch of concerns about the rpmdev-vercmp mock, but I'm hoping that they stem from my misunderstanding what it is supposed to do...in which case, maybe it needs better documentation.

ndokos
ndokos previously approved these changes Apr 16, 2021
@portante portante dismissed stale reviews from ndokos and webbnh via cc5957a April 16, 2021 23:09
@portante portante force-pushed the allow-ver-cmp branch 2 times, most recently from cc5957a to 42a84e1 Compare April 16, 2021 23:14
Copy link
Member Author

@portante portante left a comment

Choose a reason for hiding this comment

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

Okay, it was silly to include a mock of rpmdev-vercmp ...

@portante portante requested review from ndokos and webbnh April 16, 2021 23:16
@portante
Copy link
Member Author

Bugger all ... I have to rebuild the pbench-devel image with rpmdevtools ... updating shortly.

@portante
Copy link
Member Author

Ran afoul of #2226.

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.

Assuming that agent/bench-scripts/pbench-fio's days are numbered, this is good to go, although I've got a pointed question and a couple of nits for your consideration.

@portante portante merged commit eca8e44 into distributed-system-analysis:main Apr 20, 2021
@portante
Copy link
Member Author

Bugger all, failed to squash before merging, will address in another PR. Bummer.

@portante
Copy link
Member Author

Replaced by #2234.

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 packaging Issues related to software packaging tools Of and related to the operation and behavior of various tools (iostat, sar, etc.) uperf pbench-uperf benchmark related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Need to allow pbench-fio to match newer versions rather than exact

3 participants