fix: mark the work as failed if exact command runner does not exist after receptor restart #716

kurokobo · 2022-12-24T22:40:55Z

Description

Closes #711

During Restart() of command work, this PR makes the receptor to check if command runner for the unit exists or not, and if not exists, mark the unit as failed.

Check if PID exists; throw process does not exist error if negative
If PID found, check if the process is receptor; throw is not command runner error if negative
If the process is receptor, check if the args for the process contains unitdir=<expected unitdir>; throw is command runner for different unit error if negative
UPDATE: In any error pattern, as well as mark it failed, wipe stored PID for the unit. Because if the PID remains in status, the SIGINT sent to the PID that may already used different process during release/cancel after Receptor restart.

Using shirou/gopsutil.

I don't know if this is the best way but works anyway.
Please feel free to reject this if there is alternative better way 😃

Tests

Configuration file

---
- local-only:
- node:
    id: foo
- control-service:
    service: control
    filename: /tmp/receptor.sock
- work-command:
    worktype: sleep
    command: sh
    params: -c "sleep 60; echo WORKER DONE"

Case 01: Resumed correctly

Start Receptor
```
./receptor -c foo.yml &
```

Start command work

$ receptorctl --socket /tmp/receptor.sock work submit --no-payload sleep
Result: Job Started
Unit ID: Fm7fQoAs

Kill and restart Receptor main process

pkill -f "./receptor -c foo.yml" && ./receptor -c foo.yml &

Monitor work unit and ensure work state will be updated from Running to Succeeded

$ receptorctl --socket /tmp/receptor.sock work list
{
    "Fm7fQoAs": {
        "Detail": "exit status 0",
        "ExtraData": {
            "Params": "-c \"sleep 60; echo WORKER DONE\"",
            "Pid": 939252
        },
        "State": 2,
        "StateName": "Succeeded",
        "StdoutSize": 12,
        "WorkType": "sleep"
    }
}

Release units and stop receptor

receptorctl --socket /tmp/receptor.sock work release --all
pkill -f "./receptor -c foo.yml"

Case 02: Missing command runner (process does not exist)

Start Receptor
```
./receptor -c foo.yml &
```

Start command work

$ receptorctl --socket /tmp/receptor.sock work submit --no-payload sleep
Result: Job Started
Unit ID: EqMr5iww

Send SIGKILL both Receptor main process and command runner

pkill -9 -f "./receptor -c foo.yml"
kill -9 $(pgrep -f EqMr5iww)

Ensure the status file for the unit is in Running state

$ cat /tmp/receptor/foo/EqMr5iww/status
{"State":1,"Detail":"Running: PID 939879","StdoutSize":0,"WorkType":"sleep","ExtraData":{"Pid":939871,"Params":"-c \"sleep 60; echo WORKER DONE\""}}

Restart Receptor main process
```
./receptor -c foo.yml &
```

Ensure work state has been updated as Failed with Orphaned: process does not exist, and PID has been wiped as 0

$ receptorctl --socket /tmp/receptor.sock work list
{
    "EqMr5iww": {
        "Detail": "Orphaned: process does not exist",
        "ExtraData": {
            "Params": "-c \"sleep 60; echo WORKER DONE\"",
            "Pid": 0
        },
        "State": 3,
        "StateName": "Failed",
        "StdoutSize": 0,
        "WorkType": "sleep"
    }
}

Release units and stop receptor

receptorctl --socket /tmp/receptor.sock work release --all
pkill -f "./receptor -c foo.yml"

Case 03: Missing command runner (process exists but not receptor)

Start Receptor
```
./receptor -c foo.yml &
```

Start command work

$ receptorctl --socket /tmp/receptor.sock work submit --no-payload sleep
Result: Job Started
Unit ID: JuYT8el0

Send SIGKILL both Receptor main process and command runner

pkill -9 -f "./receptor -c foo.yml"
kill -9 $(pgrep -f JuYT8el0)

Ensure the status file for the unit is in Running state

$ cat /tmp/receptor/foo/JuYT8el0/status
{"State":1,"Detail":"Running: PID 940207","StdoutSize":0,"WorkType":"sleep","ExtraData":{"Pid":940197,"Params":"-c \"sleep 60; echo WORKER DONE\""}}

Fake PID in status file to invalid process 1 (systemd)

$ sed -i 's/"Pid"\:[0-9]*/"Pid"\:1/g' /tmp/receptor/foo/JuYT8el0/status
$ cat /tmp/receptor/foo/JuYT8el0/status
{"State":1,"Detail":"Running: PID 940207","StdoutSize":0,"WorkType":"sleep","ExtraData":{"Pid":1,"Params":"-c \"sleep 60; echo WORKER DONE\""}}

Restart Receptor main process
```
./receptor -c foo.yml &
```

Ensure work state has been updated as Failed with Orphaned: pid X is not command runner

$ receptorctl --socket /tmp/receptor.sock work list
{
    "JuYT8el0": {
        "Detail": "Orphaned: pid 1 is not command runner",
        "ExtraData": {
            "Params": "-c \"sleep 60; echo WORKER DONE\"",
            "Pid": 0
        },
        "State": 3,
        "StateName": "Failed",
        "StdoutSize": 12,
        "WorkType": "sleep"
    }
}

Release units and stop receptor. Ensure work is releasable since SIGINT never sent to PID 1 (If PID was not wiped, SIGINT was sent to 1 and we got Operation not permitted error)
```
receptorctl --socket /tmp/receptor.sock work release --all
pkill -f "./receptor -c foo.yml"
```

Case 04: Missing command runner (process exists but is not for exact unit)

Start Receptor
```
./receptor -c foo.yml &
```

Start two command works

$ receptorctl --socket /tmp/receptor.sock work submit --no-payload sleep
Result: Job Started
Unit ID: EqMr5iww

$ receptorctl --socket /tmp/receptor.sock work submit --no-payload sleep
Result: Job Started
Unit ID: FYyQ6Ifn

Send SIGKILL both Receptor main process and one of the command runner
```
pkill -9 -f "./receptor -c foo.yml"
kill -9 $(pgrep -f FYyQ6Ifn)
```

Ensure the status file for the unit is in Running state

$ cat /tmp/receptor/foo/FYyQ6Ifn/status
{"State":1,"Detail":"Running: PID 941175","StdoutSize":0,"WorkType":"sleep","ExtraData":{"Pid":941166,"Params":"-c \"sleep 60; echo WORKER DONE\""}}

Fake PID in status file to pid for another runner

$ pgrep -f EqMr5iww
941083
$ sed -i 's/"Pid"\:[0-9]*/"Pid"\:941083/g' /tmp/receptor/foo/FYyQ6Ifn/status
$ cat /tmp/receptor/foo/FYyQ6Ifn/status
{"State":1,"Detail":"Running: PID 941175","StdoutSize":0,"WorkType":"sleep","ExtraData":{"Pid":941083,"Params":"-c \"sleep 60; echo WORKER DONE\""}}

Restart Receptor main process
```
./receptor -c foo.yml &
```

Ensure work state has been updated as Failed with Orphaned: pid X is command runner for different unit

$ receptorctl --socket /tmp/receptor.sock work list
{
    "EqMr5iww": {
        "Detail": "exit status 0",
        "ExtraData": {
            "Params": "-c \"sleep 60; echo WORKER DONE\"",
            "Pid": 941083
        },
        "State": 2,
        "StateName": "Succeeded",
        "StdoutSize": 12,
        "WorkType": "sleep"
    },
    "FYyQ6Ifn": {
        "Detail": "Orphaned: pid 941083 is command runner for different unit",
        "ExtraData": {
            "Params": "-c \"sleep 60; echo WORKER DONE\"",
            "Pid": 0
        },
        "State": 3,
        "StateName": "Failed",
        "StdoutSize": 0,
        "WorkType": "sleep"
    }
}

Release units and stop receptor

receptorctl --socket /tmp/receptor.sock work release --all
pkill -f "./receptor -c foo.yml"

Case 05: Node down

Invoke completely the same test as described in #711.
At the last step, I can get Failed state for the work.

$ docker compose exec foo receptorctl work list
{
    "rN7KMwTD": {
        "Detail": "Orphaned: process does not exist",
        "ExtraData": {
            "Expiration": "0001-01-01T00:00:00Z",
            "LocalCancelled": false,
            "LocalReleased": false,
            "RemoteNode": "bar",
            "RemoteParams": {},
            "RemoteStarted": true,
            "RemoteUnitID": "8ve1Hdiz",
            "RemoteWorkType": "sleep",
            "SignWork": false,
            "TLSClient": ""
        },
        "State": 3,
        "StateName": "Failed",
        "StdoutSize": 0,
        "WorkType": "remote"
    }
}

$ docker compose exec bar cat /tmp/receptor/bar/8ve1Hdiz/status
{"State":3,"Detail":"Orphaned: process does not exist","StdoutSize":0,"WorkType":"sleep","ExtraData":{"Pid":0,"Params":"-c \"sleep 60; echo WORKER DONE\""}}

$ docker compose exec foo receptorctl work release --all
Released:
(rN7KMwTD, released)

…fter receptor restart

AaronH88 · 2023-03-14T14:34:28Z

pkg/workceptor/command.go

@@ -16,6 +16,7 @@ import (
 	"github.com/ansible/receptor/pkg/logger"
 	"github.com/ghjm/cmdline"
 	"github.com/google/shlex"
+	"github.com/shirou/gopsutil/v3/process"


Do you know what user elevation is needed for this library to work as expected?

AaronH88 · 2023-03-14T14:36:34Z

pkg/workceptor/command.go

+
+	cmd, err := p.CmdlineSlice()
+	if err != nil {
+		return true, err


Im worried here that receptor may not have privilege to view the cmd line slice, do you know if their are any restrictions on which type of user can see this data?

e.g. do we need sudo? do we need to be in a user group etc.

AaronH88 · 2023-03-14T14:41:08Z

pkg/workceptor/command.go

+	p, err := process.NewProcess(int32(pid))
+	if err != nil {
+		// process does not exist
+		return true, err


Im finding the name of the function a little confusing here. If the process dosent exist, how can it also be an orphaned process?

Perhaps changing the name to something like "isPidValid()" and reversing the bools returned will make a little more sense? What do you think?

kurokobo · 2023-03-15T16:27:07Z

@AaronH88
Thanks for your review here too, will look into this!

AaronH88 · 2023-03-15T16:30:44Z

Thanks @kurokobo !
If you can find some way to automate the tests you outlined in the comments here, that would be amazing also!

kurokobo · 2023-03-15T16:33:51Z

I agree, when I was testing by hand as described in my first comment, I thought these tests should be automated :P

fix: mark the work as failed if exact command runner does not exist a…

821a05b

…fter receptor restart

kurokobo force-pushed the command branch from 2357713 to 821a05b Compare December 24, 2022 23:02

fix: wipe pid for orphaned unit

3005c2a

AaronH88 reviewed Mar 14, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: mark the work as failed if exact command runner does not exist after receptor restart #716

fix: mark the work as failed if exact command runner does not exist after receptor restart #716

kurokobo commented Dec 24, 2022 •

edited

Loading

AaronH88 Mar 14, 2023

AaronH88 Mar 14, 2023 •

edited

Loading

AaronH88 Mar 14, 2023

AaronH88 Mar 14, 2023

kurokobo commented Mar 15, 2023

AaronH88 commented Mar 15, 2023

kurokobo commented Mar 15, 2023

fix: mark the work as failed if exact command runner does not exist after receptor restart #716

Are you sure you want to change the base?

fix: mark the work as failed if exact command runner does not exist after receptor restart #716

Conversation

kurokobo commented Dec 24, 2022 • edited Loading

Description

Tests

Configuration file

Case 01: Resumed correctly

Case 02: Missing command runner (process does not exist)

Case 03: Missing command runner (process exists but not receptor)

Case 04: Missing command runner (process exists but is not for exact unit)

Case 05: Node down

AaronH88 Mar 14, 2023

Choose a reason for hiding this comment

AaronH88 Mar 14, 2023 • edited Loading

Choose a reason for hiding this comment

AaronH88 Mar 14, 2023

Choose a reason for hiding this comment

AaronH88 Mar 14, 2023

Choose a reason for hiding this comment

kurokobo commented Mar 15, 2023

AaronH88 commented Mar 15, 2023

kurokobo commented Mar 15, 2023

kurokobo commented Dec 24, 2022 •

edited

Loading

AaronH88 Mar 14, 2023 •

edited

Loading