ateom: detect actor process exits and expose GetWorkloadHealth - #1202
ateom: detect actor process exits and expose GetWorkloadHealth#1202Zoe Zhao (zoez7) wants to merge 3 commits into
Conversation
sfunkenhauser
left a comment
sfunkenhauser
left a comment
There was a problem hiding this comment.
Couple small review comments.
A larger overall question though. Do we need to re-implement this logic, or can we instead use the pod informer to get the container status?
| // wait on during graceful shutdown. The sandbox runs one workload at a time. | ||
| type workloadSession struct { | ||
| rcmd *runsc | ||
| prober containerProber |
There was a problem hiding this comment.
It looks like containerProber just exposes a subset of runsc methods. Is that correct? Can we just use the existing rcmd variable instead? If the reason for the interface was to enable testing, let's still combine them and create an interface for all runsc methods.
There was a problem hiding this comment.
I was using the interface for testing. Created interface for all runsc methods.
| var containersToDelete []string | ||
| defer func() { | ||
| if retErr != nil { | ||
| s.activeActor.Store(nil) |
There was a problem hiding this comment.
Seems a little weird that we have multiple defer func() that both unset s.activeActor. Are they both necessary?
There was a problem hiding this comment.
Yes both are needed.
- If we remove the first defer, any error between line 759 and 786 does not register the cleanup of
s.activeActor. - If we remove the second one, during
deactivateActorNetworkingetc, the activeActor would not be unset yet (due to LIFO of defer functions)
There was a problem hiding this comment.
We shouldn't push the counter binary. Maybe we are missing this from .gitignore?
There was a problem hiding this comment.
Looks like our .gitignore already ignores all bin/, I updated my GOBIN env var locally to point there.
2 participants
Part of #292
GetWorkloadHealth(actor_uid)in ateom.proto so atelet can probe each ateom to get the status.