config files to use kubernetes container pool and invoker-agent #155
Conversation
dgrove-oss
force-pushed
the
kube-container-pool
branch
from
c145114 to
6429468
Compare
dgrove-oss
changed the title
dgrove-oss
force-pushed
the
kube-container-pool
branch
2 times, most recently
from
54a44ae to
4ad1636
Compare
dgrove-oss
force-pushed
the
kube-container-pool
branch
from
2355265 to
613c670
Compare
dgrove-oss
changed the title
dgrove-oss
force-pushed
the
kube-container-pool
branch
2 times, most recently
from
15a091a to
ba9b2b1
Compare
dgrove-oss
force-pushed
the
kube-container-pool
branch
from
be4a5c2 to
9c9a68a
Compare
|
@dubeejw -- would you have time to look at the Go piece of this PR? This is my first Go program, so I'm looking for style feedback and someone to catch newbie mistakes. |
|
Would it make sense to (eventually) use CRI instead of direct interaction with the docker daemon? |
|
Would it make sense to have invoker-agent in a different repo? It doesn't seem to be too related to kube deployment - at least at the moment.. |
dgrove-oss
force-pushed
the
kube-container-pool
branch
4 times, most recently
from
7a92eff to
15f6e8c
Compare
1. Deployment files for invoker statefulset using KubernetesContainerPool. 2. Update invoker configuration to describe the two container factory options 3. Implement simple Go invoker-agent to proxy pause/unpause operations and log consolidation for a remote invoker instance.
dgrove-oss
force-pushed
the
kube-container-pool
branch
from
15f6e8c to
5aa4721
Compare
docker/invoker-agent/main.go
Outdated
| } | ||
|
|
||
| // handler for /resume route | ||
| func suspendUserAction(w http.ResponseWriter, r *http.Request) { |
There was a problem hiding this comment.
can you add to the comment/function docs what the expected contract is? ie receive container value as a parameter.
should this have some defensive logic in case of a missing container value?
docker/invoker-agent/main.go
Outdated
| w.WriteHeader(500) | ||
| fmt.Fprintf(w, "Pausing %s failed with error: %v\n", container, err) | ||
| } else if resp.StatusCode < 200 || resp.StatusCode > 299 { | ||
| w.WriteHeader(resp.StatusCode) |
There was a problem hiding this comment.
post status of 500? i'd think the contract to the caller should be success or failure.
docker/invoker-agent/main.go
Outdated
| */ | ||
|
|
||
| // handler for /resume route | ||
| func resumeUserAction(w http.ResponseWriter, r *http.Request) { |
There was a problem hiding this comment.
similar comments to suspend below.
docker/invoker-agent/main.go
Outdated
| } | ||
|
|
||
| // Request handler for /logs route | ||
| func forwardLogsFromUserAction(w http.ResponseWriter, r *http.Request) { |
There was a problem hiding this comment.
it would be nice to test this logic - perhaps a slight refactoring to avoid having to mock the http call would make a unit test easier to write.
| } | ||
|
|
||
| vars := mux.Vars(r) | ||
| container := vars["container"] |
There was a problem hiding this comment.
defensively error out if container is not set?
| container := vars["container"] | ||
|
|
||
| var lfi LogForwardInfo | ||
| b, err := ioutil.ReadAll(r.Body) |
There was a problem hiding this comment.
what's expected in the body (looks like last seek position) - can you document in the function docs?
|
added basic documentation to the entrypoint functions & opened an issue to add unit tests for invokerAgent later. |
|
@rabbah -- I added a little documentation on the key methods and opened an issue to document need for unit testing of log extraction of invokerAgent. Good enough for now? |
config files for deploying an invoker that uses the k8s container pool of apache/openwhisk#3219.
Also adds an invoker-agent that can perform node-level operations like pause/unpause and log extraction on the worker node on behalf of a potentially remote invoker pod.
Fixes #110.