Release notes, checklist + ansible fixes that are in Galaxy #3453
Conversation
docs/release/checklist.txt
Outdated
|
|
||
| - Rebuild RPM in the COPR repo for the release (e.g. pbench-0.72) and redo steps 4-5 (the smoke test in particular). | ||
|
|
||
| - Build tagged containers. |
There was a problem hiding this comment.
This harks back to your comments in scrum: it might not be a bad idea to add actual command syntax to some of these bullet points while they're fresh in your mind as an aid to future developers who tread this path.
There was a problem hiding this comment.
Will do - actually, I'll make it into a draft for now and work on it after the more immediate problem. It would be nice to get this in eventually, but there is no hurry.
| - Installation on Fedora 36 succeeds but you will not be able to do | ||
| anything with it: Python 3.10 changed the way that pip install modules | ||
| (it inserts a "local" element in the path), so the PYTHONPATH settings | ||
| are wrong and modules cannot be found. This is a larger problem and we | ||
| are working on that, but the upshot is that you won't be able to use | ||
| Fedora 36 with this release. |
There was a problem hiding this comment.
Do we even care about Fedora 36 anymore? Should we?
There was a problem hiding this comment.
Yes, this is the straight v0.71 announcement. I should just make it more boilerplate.
- v0.72.0 release notes - v0.72.1.release notes - release checklist (greatly expanded but still WIP) - sample announcement (TBD) PBENCH-1129
|
I have added the updated release notes for v0.72.0 and v0.72.1, and an expanded (but still incomplete) checklist; the sample announcement is unmodified and needs work as well. Still in draft mode until I complete these changes. |
There was a problem hiding this comment.
I realize that this is a draft PR and that some of these files are still very much works in progress. I'm hoping that some of my comments are timely inputs to those works. I also realize that the contents of at least one of these files has already gone out, so take my comments on that for what they are worth.
| @@ -0,0 +1,5 @@ | |||
| This release is a point release for v0.72.0. It fixes one problem that was found just as we were releasing v0.72.0. The problem was that `pbench-postprocess-tools` was not handling labeled hosts correctly (labels may be attached to hosts during tool registration). See issue #3454 and PR #3456 for a fuller description. | |||
There was a problem hiding this comment.
Do we want to provide actual hyperlinks for the issue and/or PR?
| @@ -0,0 +1,141 @@ | |||
| #+TITLE: Release checklist | |||
|
|
|||
| * Clone the release branch | |||
There was a problem hiding this comment.
I cannot help but laugh at this instruction. (Yes, it should be obvious, but we know from experience that it is not, and so it is entirely appropriate that it should be here!!) 😀
However, are you sure you want to use "Clone", as opposed to, say, "Check out"?
| git checkout upstream/b0.72 | ||
| git branch -u upsream/b0.72 |
There was a problem hiding this comment.
Do you really want the branch -u command? (Does it actually do anything, given that you've just checked the branch out from that location?)
And, is the lack of a t in the upstream ref intended? (It appears again at line 9.) If so, perhaps another name would be better, since this one looks like a typo.
| That will allow the =make= invocation below to create properly named RPMs (e.g. =pbench-agent-pbench-agent-0.72.0-1gXXXXXXX) | ||
| in the appropriate repo (=pbench-0.72-test= in this case) |
There was a problem hiding this comment.
You're missing the closing = in line 15.
| cd agent/rpm | ||
| make copr-test |
There was a problem hiding this comment.
We don't need to specify any Make variables on the make command line? (E.g., perhaps not CHROOTS, but COPR_USER?)
Do we need to say anything about having an account on COPR or configuring the "project" correctly (e.g., with the correct chroots)?
(I.e., if the user doesn't specify COPR_USER, it'll use their username, and if they don't have an account or an appropriate project on COPR, they won't be happy....)
If you make a change here, make sure the "rebuild" step (line 115) matches appropriately.
| The demo script (which is to be thought of more as "executable documentation" than anything else at this point) uses the `pbench` command to execute a series of commands inside a container. The first time that the `pbench` command runs, it realizes that there is no container yet, so it downloads the `pbench-agent-all-fedora-36:b0.72` image from `quay.io` and starts the container. It then executes the first command that it was given *inside* that container. Subsequent invocations of the `pbench` command execute their arguments inside that container, first registering tools, then listing the tools, then running a simple `fio` benchmark under `pbench-user-benchmark` and finally pushing the results to the configured Pbench server. Although this is a very simple set of commands, it indicates how things would go in a more complicated invocation. | ||
|
|
There was a problem hiding this comment.
Rather than "executable documentation", I would use phrasing like, "which contains a sequence of commands that a user might execute to run a benchmark workload using the containerized Pbench Agent."
The first time that the
pbenchcommand runs, it realizes that there is no container yet, so it downloads thepbench-agent-all-fedora-36:b0.72image fromquay.ioand starts the container.
I would either put the credit/blame where it is due (on Podman) or use the passive voice: "The first time that the pbench command script runs, the container image is downloaded from quay.io and started." I don't know that we want to give the full container name/tag in this text.
I'm thinking that this paragraph would be better if we described the pbench script first and then mentioned the pbench-demo script (which needs hardly more than the description I suggested above).
I would say that the pbench script executes its arguments as a command line inside the container, downloading the container image if it is not present on the local host.
It's not obvious to me that we need to describe what the demo script does. That is, if the user already knows how to use Pbench, then the description is redundant; if the reader is new to Pbench, I'm not sure that the demo script should be the first place they learn from. 🙂 I think the salient point to make is that the pertinent state inside the container is persisted in a volume mapped in from the user's file system on the host, so that sequences of commands like registering tools and running benchmark workloads work together even though they are performed in separate invocations of the container.
| There are a couple of **significant caveats**: this version of the demo script does *NOT* use `pbench-move-results` to send the results to the server (although it be could modified to do so). Instead it passes an authentication token to the `pbench-results-move` command (see below) to push the results. That token is generated by the `pbench-generate-token` script, which is invoked at the very beginning of the demo script: that script asks for a user ID and a password and then generates and stores that token in a file (the file is stored in a directory which is mapped into the container from the outside, so the token persists beyond the run of the demo script). That means you have to have a user ID and a password on the Pbench server before generating the token. | ||
|
|
There was a problem hiding this comment.
I feel like this description implies that users should expect to run the demo script, and, while it's true that one can, that's not really the point of it. The expectation should be that users would use the pbench command, and build their own tooling which uses it.
Moreover, the structure of this section ties together the concepts of the containerized Agent and the pbench-generate-token tool which really should be kept separate. Yes, this evil demo script is the source of this tie-in, but I expect the containerized Agent to live on while the token generation devolves to the Dashboard and SSO. So, keeping them separate here would probably be a very good idea.
| The trouble is that this is a *very* temporary arrangement: we expect that very soon, you will be able to use Red Hat SSO for logging in and generating the document. The accounts created as above will go away, as will the `pbench-generate-token` script which is already deprecated. Any datasets submitted through this mechanism will therefore be orphaned, hence the imprecation to use this to kick the tires, not for storing "real" results that you don't want to lose. We are *NOT* planning to migrate any such results. | ||
|
|
There was a problem hiding this comment.
Any datasets submitted through this mechanism will therefore be orphaned, hence the imprecation to use this to kick the tires, not for storing "real" results that you don't want to lose. We are NOT planning to migrate any such results.
I think perhaps you buried the lead....
| But we are not quite there yet: we may need to move to HTTPS in the Integration Lab and we need to finish setting up the authentication underpinnings in order to hook into the SSO infrastructure. Until that point, we encourage you to continue with pbench-move-results and the legacy interface to the server. If you have questions about any of this, ping us on GChat. | ||
|
|
There was a problem hiding this comment.
We will need to move to HTTPS (and, have done so...although not yet in production) for access to the Pbench Server (and, does anyone but us care whether it's in the Integration Lab?).
Presumably "pbench-move-results" should be in mono-space font.
| ==== | ||
| For the full Changelog, see the link near the top of these notes. |
There was a problem hiding this comment.
Rather than referring the reader to the top of the notes, wouldn't it be just as easy to hyperlink it from here, as well? (If you're concerned about replicating the link, I would refer the reader from the top to here....)
Recommended by dependabot due to a CVE (which doesn't really affect us). On the whole, the changes caused by this new version seem trivial and innocuous (like adding blank lines after file docstrings) or minor improvements in readability (the new wrapping of compound dict references and broken ternary expressions). This supercedes dependabot's distributed-system-analysis#3453.
Recommended by dependabot due to a CVE (which doesn't really affect us). On the whole, the changes caused by this new version seem trivial and innocuous (like adding blank lines after file docstrings) or minor improvements in readability (the new wrapping of compound dict references and broken ternary expressions). This supercedes dependabot's #3453.
This PR contains two commits:
the second commit contains a few fixes to the ansible roles: this version of the collection has already been uploaded to Galaxy. This commit will need to be forward-ported tomain.PBENCH-1129