[RFC] Improve init script: specifically manage salt configurations rather than arbitrary salt processes#32025
[RFC] Improve init script: specifically manage salt configurations rather than arbitrary salt processes#32025plastikos wants to merge 41 commits intosaltstack:developfrom
Conversation
…han arbitrary salt processes
Unfortunately SysV init scripts tend to rummage through PIDs filtering for
appropriate processes to manage. Unfortunately the filters are usually weak
and don't account for similar processes run by other users, PIDs of dead
processes being re-used for completely different executables, etc.. These
weaknesses can result in killing unrelated processes with potentially serious
results.
These improvements to the SysV init script is a complete rewrite with the
following improvements:
* Specifically manage individual salt configurations rather than looking for
salt minion-like processes.
* Obtain salt minion information from the salt configuration - use the
information to manage the specifically configured process.
* Drop all of the platform-specific helper functions that allow the
previously-mentioned weaknesses.
+ Unfortunately this means that the output information may not match the
specific platform (this could easily be corrected).
* Now can manage multiple salt processes started by different users
+ Unfortunately starts/stops/restarts as a group and is unable to manage
them both as a group or as individual processes (this could easily be
corrected)
plastikos
changed the title
|
Don't merge this yet - I need to investigate a couple things and write an integration test. |
|
Looks good to me so far. Let me know when you're ready to try and get this in. |
* Correct algorithm parsing netsat output which can have variable fields depending on how many program args it can fit. * Use sed to parse rather than awk * Send stderr to /dev/null for netstat and salt-call when DEBUG is not set.
* Allow installation paths to be overridded for integration tests * Detect if netstat accepts "--notrim" option * Remove some unused cruft * Explicitly use "local" for local variables in functions * Drop bashism string translation * Correct management of timeout in start() and stop() * Add "kill -KILL" as a final attempt in stop() * Use exitstatus values from LSB * Put status() in function * Add try-restart from LSB
* Requires new salt-testing:salttesting.case:ShellTestCase.run_prog() * Only runs on Linux (although it could be used on other POSIX platforms :NOTE: MinionTest.test_linux_initscript() unfortunately kills the initial salt minion started for the test suite. While it leaves a new minion running it may not be handled properly by the test suite shut-down.
|
@cachedout, this is getting close to being ready. One thing that I could use help with is how to handle the minions that need to be started/stopped/restarted by the integration test. Unfortunately it doesn't work well with the already-spun-up minion and it's unclear to me the best way to get |
|
@cachedout, I need to figure out a good way of restarting the test minion for this test as well as testing |
|
Once this is cleaned up and pulled I'll do the same thing for |
|
@plastikos Sounds good. Please keep me posted. |
* Ensure that salt-minion is started using same user as test suite * Use classes out of integration.utils.testprogram * Set PATH in environment to find script front-ends * Create an ephemeral directory for testing using setUp()/tearDown()
* Use the ``.pid`` file to check for an actual running process * Use the returned PID from calling ``status`` to check against the ``.pid`` file
|
@jtand is your man for that. @jtand can you please contact @plastikos privately and get him set up with one? |
|
@plastikos if this fails again, let me know and I can get you a test VM. |
|
Yes, please! |
|
@plastikos Ok, How do you want me to send you the login info? |
|
@jtand, I just sent you an email. |
|
@jtand Thanks very much. @plastikos Keep me posted. Thanks! |
|
@cachedout , I've been sending emails to @jtand. At this point I am unable to reproduce the failures of the automated testing even in the VM @jtrand provided running the same test. The failure logged in the automated testing is quite odd and indicates that the started The overall status is that I'm stumped for the moment and call shenanigans on the automated testing. Yes, that's my technical diagnosis. |
|
@cachedout, I don't think that the VM is running (all of) the code from my branch - although I'm not certain what it's running. I point out the line in the Notice the last line: Notice the Clearly the tests are running a portion of my code from my I'm puzzled. |
|
@cachedout, I think I know what's going on. I might have a fix . . .. |
|
@cachedout, I fixed it. It looks like the test suite adds I'm kicking myself that it took me this long to figure out. I'll see about cleaning this up, ripping out the breadcrumbs and unnecessary debugging, quash some commits and get it re-submitted. |
|
@cachedout, how many places are subprocesses created that don't have
I think the above should be changed to add Thoughts? |
|
@plastikos I'm fine with those changes if @s0undt3ch is. |
3 participants
What does this PR do?
Prevents SysV init script from killing non-salt processes.
What issues does this PR fix or reference?
None
Previous Behavior
Current SysV init script kills unrelated processes.
New Behavior
Manage a specific salt minion configuration rather than anything that seems to be a salt minion.
Tests written?
Unfortunately SysV init scripts tend to rummage through PIDs filtering for
appropriate processes to manage. Unfortunately the filters are usually weak
and don't account for similar processes run by other users, PIDs of dead
processes being re-used for completely different executables, etc.. These
weaknesses can result in killing unrelated processes with potentially serious
results.
These improvements to the SysV init script is a complete rewrite with the
following improvements:
salt minion-like processes.
information to manage the specifically configured process.
previously-mentioned weaknesses.
specific platform (this could easily be corrected).
them both as a group or as individual processes (this could easily be
corrected)