Thanks Jim This makes sense. I’ll work on integrating these changes.
Tyson > On Aug 23, 2017, at 10:46 AM, Jim Crossley <[email protected]> wrote: > > One other thing, Tyson, related to that cleanup() function. You'll note > that it refers to the invoker's InstanceId to find its containers. So I > think we'll want to add InstanceId to getContainerFactory's parameter list. > We use this in the kube impl to attach a label for each created container > denoting its invoker. This makes it trivial to delete them all in one call. > > Jim > > On Tue, Aug 22, 2017 at 6:44 PM, Jim Crossley <[email protected]> wrote: > >> Hi Tyson, >> >> Thanks for spearheading this SPI effort. Much appreciated! >> >> As a "Kube folk" who has taken a stab at a ContainerFactory SPI [1], I >> think we also need to encapsulate the cleanup() function defined in >> InvokerReactive within the ContainerFactory interface [2]. The factory >> impls are going to know best how to delete all the containers they create, >> and those docker calls currently used in IR.cleanup() won't work on all >> kube-like platforms, e.g. OpenShift. >> >> Thanks, >> Jim >> >> [1] >> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fprojectodd%2Fincubator-openwhisk%2Fblob%2Fkube-container-&data=02%7C01%7C%7C53feedd5c8eb40129c1508d4ea4ee2a7%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636391071916251489&sdata=wx3xizOpClqQYOqec%2F3JGf5UyamuTBR%2Fp62FFgcFHKQ%3D&reserved=0 >> openshift/core/invoker/src/main/scala/whisk/core/containerpool/ >> ContainerProvider.scala >> [2] >> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fprojectodd%2Fincubator-openwhisk%2Fblob%2Fkube-container-&data=02%7C01%7C%7C53feedd5c8eb40129c1508d4ea4ee2a7%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636391071916251489&sdata=wx3xizOpClqQYOqec%2F3JGf5UyamuTBR%2Fp62FFgcFHKQ%3D&reserved=0 >> openshift/core/invoker/src/main/scala/whisk/core/invoker/ >> InvokerReactive.scala#L69-L73 >> >> On Tue, Aug 22, 2017 at 1:40 PM, Tyson Norris <[email protected]> >> wrote: >> >>> Hi - >>> I wanted to give a nudge for comments on these PRs, as well as provide >>> some context: >>> >>> #2584 - LoadBalancer - to allow alternative load balancers (e.g. >>> not-kafka, concurrent, multiple executors, etc) >>> >>> #2656 - ActivationExecutor - (builds on 2584) adds an abstraction *below* >>> load balancer for enabling multiple execution workflows; an example >>> provided is “kind based”, such that different action kinds can be executed >>> by particular ActivationExecutor (with a fallback to the default >>> Kafka/invoker based executor which still can handle everything). Existing >>> LoadBalancerService implements both LoadBalancer and ActivationExecutor >>> >>> #2659 - ContainerFactory - this one is simply providing an approach for >>> defining how containers are managed, which can be used to enable >>> mesos/kube/etc container launching/killing/etc. (Kube folks, I’m looking at >>> you for feedback on this) >>> >>> Let me know if you have comments either via GitHub for individual PR >>> questions, or let’s discuss on list if there is some general overall >>> feedback to these (design, etc), since they are somewhat all related to the >>> flow of activation execution. >>> >>> Upcoming additional related items: >>> - concurrent ActivationExecutor (does container pool, but with concurrent >>> requests and without message queues) >>> - logging (to allow alternate log collection, storage, and retrieval - >>> e.g. we want to decouple log collection from OW processing jvms and >>> delegate to an separate API for retrieving logs on demand) >>> - authentication (not related to execution paths directly, but this is >>> floating on our radar) >>> >>> Thanks for any feedback >>> Tyson >> >> >>
