Skip to content
This repository has been archived by the owner on Oct 22, 2021. It is now read-only.

Create pidfile and "bpm pid" command #11

Merged
merged 4 commits into from
May 20, 2021
Merged

Create pidfile and "bpm pid" command #11

merged 4 commits into from
May 20, 2021

Conversation

jandubois
Copy link
Member

@jandubois jandubois commented May 18, 2021

Some drain scripts send signals directly to the process via the PID from the pidfile (because bpm doesn't have a mechanism to send e.g. a USR1 signal). This obviously only works when the drain runs inside the same container as the process itself.

This commit maintains a pid file at the /var/vcap/sys/run/bpm/$JOB/$PROCESS.pid location and also implements the bpm pid command to access it. This is an extension to the changes in #10 and should enable some of the drain scripts mentioned in cloudfoundry-incubator/kubecf#1728 to work without any local patches.

Manual testing

Start the new container-run as before (main.sh from #10):

# ./container-run --job-name main --process-name main $PWD/main.sh; echo $?
Start
Loop 1
[...]
Loop 10
End

Then run from a different terminal:

# bpm start main; sleep 3; bpm pid main; kill -quit $(bpm pid main); sleep 3; kill $(bpm pid main)
20968
# sleep 3; bpm pid main
Process is not running

And verify it has the expected effect:

[...]
End
Start
Loop 1
Loop 2
SIGQUIT
Loop 3
SIGTERM
Exiting because already received SIGQUIT

Some drain scripts send signals directly to the process via the
PID from the pidfile (because bpm doesn't have a mechanism to
send e.g. a USR1 signal). This obviously only works when the drain
runs inside the same container as the process itself.

This commit maintains a pid file at the /var/vcap/sys/run/bpm/$JOB/$PORC.pid
location and also implements the `bpm pid` command to access it.

Signed-off-by: Jan Dubois <[email protected]>
The pid is unused by the rest of the tests, so for now just set
it to 0.

Signed-off-by: Jan Dubois <[email protected]>
@andrew-edgar
Copy link
Contributor

I have looked at most of the tests that were failing (other than the path issues) and we need to change the tests to "Expect" Pid function to called. Most of the tests have Expects for "Wait" but it will also need "Expects" for "Pid" to be called. I can add that and I know how and where to add it but none of the tests will work until the hard coding of the paths is fixed

@jandubois jandubois requested a review from manno May 19, 2021 18:32
Copy link
Member

@manno manno left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@manno manno merged commit 6c0b007 into master May 20, 2021
@manno manno deleted the pidfile branch May 20, 2021 06:57
manno pushed a commit to cloudfoundry-incubator/quarks-operator that referenced this pull request May 20, 2021
Also fix the version of container-run and use newer Go version to build
it.

References cloudfoundry-incubator/quarks-container-run#11
manno pushed a commit to cloudfoundry-incubator/quarks-operator that referenced this pull request May 21, 2021
Also fix the version of container-run and use newer Go version to build
it.

References cloudfoundry-incubator/quarks-container-run#11
manno pushed a commit to cloudfoundry-incubator/quarks-operator that referenced this pull request May 21, 2021
Also fix the version of container-run and use newer Go version to build
it.

References cloudfoundry-incubator/quarks-container-run#11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants