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

Update the pbench_demo script to add --server #3508

Merged
merged 1 commit into from
Jul 28, 2023

Conversation

webbnh
Copy link
Member

@webbnh webbnh commented Jul 28, 2023

This PR is a follow-on to #3494, which removed the use of PB_AGENT_SERVER_LOC from the containerized Agent pbench wrapper script (it turns out that the demo was depending on it...). Now that we have the --server option on pbench-results-move, we use that in the demo script (instead of dynamically creating an Agent config file) to specify the server location.

While making the above addition, I also dusted off the demo file:

  • I reworked the header comment to clarify the purpose of the file (i.e., it's more documentation than tool) and to explicitly list the required input environment variables.
  • I removed the explicit override of the container image -- that's a feature of the pbench script, and if the user wants it s/he can define it in the environment when running the demo -- and reworked the comment which mentions it.
  • I removed the feature of specifying the API Key token as a command line argument -- I'm not aware of anyone using it, and it therefore imposes useless complication. Providing it via the environment is adequate (and can be done as part of the command line anyway, so the difference is just syntactic sugar).
  • The script now makes sure during setup that the Server location and API Key environment variables are defined and that the Server location is properly formatted, so that there are fewer surprises after the workload run is complete when we try to upload the result.
  • I repositioned the code which creates the FIO work directory and added an explanatory comment for it.
  • I changed the script execution to echo only the "interesting" commands, instead of the whole script including the setup.

NOTE: since @pravins wants this change for use over the weekend, and since this is a change to the contrib area, and since there aren't any reviewers around today, I'm going to go ahead and merge this once the tests pass. If someone has comments, we can iterate on them.

@webbnh webbnh added Agent Contrib Containerization Of and relating to the process of setting up and maintaining container images labels Jul 28, 2023
@webbnh webbnh added this to the v0.73 milestone Jul 28, 2023
@webbnh webbnh requested review from ndokos and dbutenhof July 28, 2023 17:40
@webbnh webbnh self-assigned this Jul 28, 2023
@webbnh webbnh merged commit bc7e692 into distributed-system-analysis:main Jul 28, 2023
3 checks passed
@webbnh webbnh deleted the pbench_demo_update branch July 28, 2023 18:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Agent Containerization Of and relating to the process of setting up and maintaining container images Contrib
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

1 participant