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

Add an explicit ENTRYPOINT to the Agent "all" containers #3320

Merged

Conversation

webbnh
Copy link
Member

@webbnh webbnh commented Mar 7, 2023

The Agent commands require the environment which the agent/profile script sets up. This script is normally run automatically when a user logs in by virtue of a symbolic link in /etc/profile.d; however, when a user invokes containerized Agent commands, although the profile script exists inside the container, there is no login performed, and so the script isn't run...and it is cumbersome to cause it to run inside the container environment in conjunction with running a pbench command (the easiest approach is to create a script (see contrib/containerized-pbench/README.md), but the user has to map that into the container in order to use it; otherwise, the user has to run an interactive shell inside the container, which is not much better).

This change provides a little script which source's the Agent profile script and then exec's the requested command inside the container, and makes use of it as the default entrypoint for the Agent "all" containers. This enables users to use a command line something like

$ podman run <cid> pbench-results-push

This PR makes the following changes:

  • Adds a utility script for use as the containers entrypoint.
  • Modifies the Agent container build to specify the new script as the container entrypoint if the container "type" is "all".

This PR also includes two new files in contrib/containerized-pbench:

  • a pbench command script which wraps the container invocation and provides some helpful defaults; and,
  • a "demo" script which shows how a user might run a benchmark using the containerized Agent commands.

These last two files don't need to be merged into the repo -- they are mostly provided as "functional documentation" showing how this change is useful. However, they do make it easy to run Agent commands, and they work without having to install the Agent.

@webbnh webbnh added Agent Code Infrastructure Containerization Of and relating to the process of setting up and maintaining container images labels Mar 7, 2023
@webbnh webbnh added this to the v0.72 milestone Mar 7, 2023
@webbnh webbnh self-assigned this Mar 7, 2023
@webbnh webbnh force-pushed the containerize-agent-commands branch 6 times, most recently from 082583d to 7b79be7 Compare March 9, 2023 22:59
@webbnh webbnh requested review from portante, ndokos and pravins March 9, 2023 23:08
@webbnh webbnh marked this pull request as ready for review March 9, 2023 23:08
@webbnh
Copy link
Member Author

webbnh commented Mar 9, 2023

I think the wrinkles are all smoothed out and this PR is ready for review.

dbutenhof
dbutenhof previously approved these changes Mar 10, 2023
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.

Looks OK, even really nice, but simply looking at the files doesn't make it immediately obvious how to use them in combination. It'd be nice to update (or maybe even replace) contrib/containerized-pbench/README.md ...

And does the simpler example script ecosystem Pete set up in there still make sense? It'd be nice for the documentation to set some context.

@webbnh
Copy link
Member Author

webbnh commented Mar 10, 2023

does the simpler example script ecosystem Pete set up in there still make sense?

This PR is very much an example of "run it up the flag pole and see who salutes".

I would like to see the agent/ changes merged into main. However, those changes alone are kind of abstruse without an explanation of how to use them, and using them repeatedly is easier with a wrapper script, so I added the pbench script, but I put it in contrib/, since it's not anything we plan on "shipping" at this point. And, then, rather than explaining how to use that script, it seemed easier and clearer to just provide a demonstration of it...hence the addition of the pbench_demo script in contrib/.

As far as I'm concerned, we don't need to merge the pbench_demo script (and, before we do, we should remove the definition of PB_AGENT_IMAGE_NAME, since, going forward, it shouldn't be pulling the image from this PR...), but we can merge it too if we like it as an example.

If people like the contrib/.../pbench script, etc., then I think we can remove the "example script ecosystem`, as what I'm submitting here should do all that in a more compact and accessible way...I think.

I can update the README.md if we decide to include either of the contrib/ files when this PR is merged.

@dbutenhof
Copy link
Member

Well, I'd like to see this contrib stuff merged, because I think it makes the agent container workflow simpler, and this is something we all expect (are expected) to be doing over the next week. 😁

@webbnh
Copy link
Member Author

webbnh commented Mar 10, 2023

this is something we all expect (are expected) to be doing over the next week.

My thoughts exactly. 😇 (This is why the demo_pbench script has that funky definition for PB_AGENT_IMAGE_NAME in it right now.... 😉)

agent/profile Outdated Show resolved Hide resolved
contrib/containerized-pbench/pbench Show resolved Hide resolved
@webbnh webbnh force-pushed the containerize-agent-commands branch from 7b79be7 to 63ee73e Compare March 11, 2023 02:25
@webbnh webbnh marked this pull request as draft March 11, 2023 02:25
@webbnh
Copy link
Member Author

webbnh commented Mar 11, 2023

Converting to draft while I work the wrinkles out of the new approach.

@webbnh webbnh changed the title Use the Agent profile script as the containers entrypoints Add an explicit ENTRYPOINT to the Agent "all" containers Mar 11, 2023
@webbnh webbnh force-pushed the containerize-agent-commands branch from 63ee73e to 541aea8 Compare March 11, 2023 02:46
@webbnh webbnh force-pushed the containerize-agent-commands branch from 541aea8 to 90f985d Compare March 11, 2023 03:27
@webbnh webbnh force-pushed the containerize-agent-commands branch from 90f985d to 19ed28e Compare March 11, 2023 04:01
@webbnh webbnh marked this pull request as ready for review March 11, 2023 04:02
@webbnh
Copy link
Member Author

webbnh commented Mar 11, 2023

OK, I've reworked this change (and updated the PR): agent/profile is now untouched, and the container entrypoint is a new, little, wrapper script.

@webbnh webbnh requested review from dbutenhof and portante March 11, 2023 04:03
portante
portante previously approved these changes Mar 11, 2023
dbutenhof
dbutenhof previously approved these changes Mar 11, 2023
@webbnh webbnh dismissed stale reviews from dbutenhof and portante via 2e77fac March 13, 2023 15:00
@webbnh
Copy link
Member Author

webbnh commented Mar 13, 2023

OK, this is now ready to merge...if I could get one more round of approvals, please.

@webbnh webbnh requested review from portante and dbutenhof March 13, 2023 15:00
@webbnh webbnh merged commit 6713d5f into distributed-system-analysis:main Mar 13, 2023
@webbnh webbnh deleted the containerize-agent-commands branch March 13, 2023 15:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Agent Code Infrastructure Containerization Of and relating to the process of setting up and maintaining container images
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants