search

jvm-profiling-tools / perf-map-agent

A java agent to generate method mappings to use with the linux `perf` tool
GNU General Public License v2.0
1.65k stars 260 forks source link

Dont call sudo from the scripts. #18

Open mbruggmann opened 8 years ago

mbruggmann commented 8 years ago

The reason we'd like to avoid using sudo is that we are executing this inside a docker container that doesn't have it installed.

mbruggmann commented 8 years ago

ping @danielnorberg @dflemstr

danielnorberg commented 8 years ago

:+1:

ktoso commented 8 years ago

Maybe it could be "check if sudo available, if so, then use it, if not - must be root"? The sudo way of using it is pretty useful when integrating these scripts with other tools., I'd rather sudo there than run as root.

jrudolph commented 8 years ago

Thanks @mbruggmann for the PR. Sorry, for keeping you hanging for a few weeks.

@ktoso that's what I would prefer as well. AFAICS there's no way to allow non-root users to run perf with full features, so I wouldn't like to penalize the interactive users for scripted usage.

@mbruggmann do you know of a best-practice how to solve such a situation? We could either add a flag whether to sudo or not, or make the SUDO command itself configurable through an environment variable.

ktoso commented 8 years ago

Another solution could be to document how to add NOPASSWD mode to sudoers for perf. Then yeah sudo is used, however one would not be asked for pass (I use it like this).

dflemstr commented 8 years ago

@ktoso @jrudolph We are running the scripts inside a Docker container where:

  1. sudo is not installed (so we get "no such command" or something like that).
  2. We are running as root so sudoers policies don't apply.

Installing sudo is tedious because these Docker containers are spawned from images owned by other people and asking them to all install sudo would take a lot of time.

Of course checking if sudo exists and recursively launching the script with sudo is an option but it sounds much simpler and is more obvious to the users of the script if they just have to say sudo create-java-perf-map.sh

jrudolph commented 8 years ago

@ktoso is this really related to this change? The question is how to handle the situation that sudo is not available at all.

@dflemstr yes, we understood that your use case is different from ours but as running as root is mandatory for perf I'd like to make it as convenient as possible for interactive users. Couldn't we just introduce another check that would allow you to specify the sudo binary as an environment variable which would default to sudo but could be overridden to a no-op command (like /bin/sh). WDYT?

ktoso commented 8 years ago

@jrudolph right, I mistook it to my use case.

The proposal you just explained sounds good for all cases I guess.

mbruggmann commented 8 years ago

For reference, I don't have a strong opinion on this one (nor best-practice), so I'm happy to go with whatever as long as it enables our use-case.

tekumara commented 5 years ago

I've created https://github.com/jvm-profiling-tools/perf-map-agent/pull/76 which hopefully addresses some of the concerns above.