*** sameo has quit IRC | 00:36 | |
sboeuf | bergwolf: hey! | 01:21 |
---|---|---|
*** bergwolf_ has joined #kata-dev | 01:22 | |
sboeuf | bergwolf_: what do you mean by "it waits for a signal" ? Do you have a way to communicate between the runtime and the shim ? | 01:22 |
*** devimc has quit IRC | 01:24 | |
bergwolf_ | sboeuf, yup, in runv shim, it waits for SIGUSR1 if it finds out itself is an init process shim | 01:24 |
bergwolf_ | @sboeuf, https://github.com/hyperhq/runv/blob/master/cli/shim.go#L59 | 01:25 |
sboeuf | bergwolf_: and how it knows about the process ID then ? It has been started with the process ID (already known at this time) ? | 01:25 |
bergwolf_ | we can do the same in kata-shim | 01:25 |
sboeuf | bergwolf_: no the issue with using "init" is that at some point I need a very unique ID so that I can map the reaped PID to the process ID chosen by the runtime | 01:26 |
bergwolf_ | sboeuf, we can use container_id for the init process. what do you think? | 01:28 |
sboeuf | bergwolf_: but we can start the shim with the expected ID and use a boolean as you were mentioning to say to the shim this is a container process | 01:28 |
sboeuf | bergwolf_: well container ID can be the solution I guess | 01:29 |
bergwolf_ | sboeuf, yes a boolean argument can work as well IMO | 01:29 |
bergwolf_ | sboeuf, but for agent, it seems container_id is better | 01:29 |
bergwolf_ | as you said for the reaped PID mapping issue | 01:30 |
bergwolf_ | so to conclude, we are OK to proceed with a runtime generated process ID and use container_id as init process ID, correct? | 01:30 |
bergwolf_ | let's put this on github so that lai and sameo can comment | 01:31 |
sboeuf | so if we start a shim with containerID == processID, then the shim will know this is a container | 01:31 |
sboeuf | and then we need a signal to signal the shim it can start waiting for the process because it has been started | 01:32 |
bergwolf_ | yes, that's the idea | 01:32 |
sboeuf | ok that seems to work. And I agree you should comment on the github to make sure all of us agree | 01:32 |
bergwolf_ | cool thanks for the feedback! | 01:33 |
sboeuf | bergwolf_: no problem! Let me think if I think of some drawbacks here. So the assumption here is that we expect the shim to always call WaitProcess() and ReadStdout() when the process is started | 01:34 |
sboeuf | I like that because I was thinking about handling the case where the shim was connecting before the process was started, and it was making thing a bit more complicated from the agent. | 01:35 |
sboeuf | bergwolf_: I have a question though, do you think it is possible to wait onto a "created" container process, but not "started" ? | 01:36 |
*** marst has quit IRC | 01:38 | |
*** liujiong has joined #kata-dev | 01:38 | |
bergwolf_ | sboeuf, I think the runtime should call start container before sending SIGUSR1 to the shim | 01:39 |
bergwolf_ | then the shim will never wait on a created by not started container | 01:39 |
bergwolf_ | s/by/but | 01:39 |
sboeuf | bergwolf_: no no, don't miss my point, I agree with that :). I was just wondering if it was possible to wait for a "created" container (from a libcontainer perspective) | 01:40 |
sboeuf | I think waiting on a started container (aka process) is better because this sounds logical ! | 01:41 |
*** liujiong has quit IRC | 01:41 | |
sboeuf | bergwolf_: do you mind if I do the changes to the agent ? you should do those the changes to the shim | 01:41 |
bergwolf_ | sboeuf, yup I totally agree. there is nothing to wait if container is not started | 01:41 |
bergwolf_ | sboeuf, I'm fine with it | 01:42 |
*** liujiong has joined #kata-dev | 01:42 | |
sboeuf | bergwolf_: ok cool. Let me try to summarize the conversation on the github ticket then and we can move forward with the implementation | 01:43 |
bergwolf_ | I just wanted to push the change quickly. We've been going back and forth on this for too long :) | 01:43 |
bergwolf_ | I have it here: | 01:43 |
bergwolf_ | 1. the runtime creates process ID and pass it through `ExecProcessRequest` | 01:43 |
bergwolf_ | 2. for container init process, use `ContainerId` in `CreateContainerRequest` as its process id | 01:43 |
bergwolf_ | 3. kata shim will wait for SIGUSR1 before proceeding if it finds out itself an init process shim (by comparing containerid and processid arguments) | 01:43 |
bergwolf_ | 4. the runtime should send SIGUSR1 to shim after calling `StartContainer` | 01:43 |
bergwolf_ | what do you think? | 01:43 |
sboeuf | bergwolf_: I think this is nice, except that I wonder if we should not be fully declarative about StartCOntainer() by providing a process ID too | 01:46 |
sboeuf | I mean we can leave the knowledge of container_id == task_id to the runtime | 01:46 |
bergwolf_ | sboeuf, in that case we must require it to be equal to container ID | 01:47 |
sboeuf | but let's the agent be more generic | 01:47 |
sboeuf | bergwolf_: well that's the point, I am not sure we need the agent to be aware about this | 01:47 |
bergwolf_ | well, I think the agent needs to know which is the init process as well. Can it get the knowledge without using the init process name? | 01:49 |
bergwolf_ | hmmm, I think it can | 01:49 |
sboeuf | bergwolf_: that's no big deal but I wanted to let the opportunity to provide anything. Let's say you have another shim who knows that he is a container process because of a boolean, in that case, the process ID might not match the container ID | 01:49 |
sboeuf | bergwolf_: yes it can, we just store that somewhere in the agent | 01:50 |
bergwolf_ | yup, I agree with you. It makes sense to keep agent common in this case | 01:50 |
sboeuf | I didn't want to tie the entire protocol to this specific containerID == processID, even if we're gonna use it | 01:50 |
sboeuf | bergwolf_: cool :) | 01:50 |
sboeuf | bergwolf_: last question then, you said you're fine with task_id ? | 01:51 |
bergwolf_ | so we add a point 5, add process ID to `CreateContainerRequest` that must equal to containerID for kata runtime | 01:51 |
sboeuf | I think Samuel wanted to make sure there is no confusion with real PID | 01:51 |
bergwolf_ | sboeuf, I think exec_id is better but I can settle with task_id if you insist | 01:52 |
sboeuf | bergwolf: oh no I am fine with exec_id too | 01:52 |
bergwolf_ | cool | 01:52 |
sboeuf | so let's go with exec_id | 01:52 |
bergwolf_ | yup | 01:52 |
sboeuf | about the point 5, yes please write this down. We still have the option to use a boolean if Samuel or Lai don't like container_id == process_id | 01:53 |
sboeuf | bergwolf_: are you gonna update the github issue ? | 01:59 |
bergwolf_ | yup, just sent it | 02:00 |
bergwolf_ | sboeuf, please see if it's OK | 02:01 |
sboeuf | bergwolf_: Everytime you've written CreateContainer() I think it should be StartContainer(), right ? | 02:02 |
bergwolf_ | oh, my bad! | 02:02 |
bergwolf_ | thanks for spotting it! | 02:03 |
sboeuf | bergwolf_: no problem ;) just trying to make sure everybody's on the same page | 02:03 |
bergwolf_ | @sboeuf, hmmm, no, actually I do mean CreateContainer() | 02:05 |
bergwolf_ | that is where the exec info is passed to the agent | 02:05 |
bergwolf_ | I think it's better to name it there as well | 02:05 |
sboeuf | bergwolf_: well yes I understand, so basically we need exec_id in both CreateContainer() StartContainer() and ExecProcess(), is that right ? | 02:06 |
bergwolf_ | StartContainer() doesn't need it, no? | 02:07 |
bergwolf_ | The agent has the exec_id for init process already | 02:07 |
sboeuf | I know, I was thinking if we really want to make everything very explicit, we have to pass it every time | 02:07 |
sboeuf | This is called StartContainer, you're right, the container_id should be enough in that case | 02:08 |
bergwolf_ | hmmm, StartContainer() is to start the container init process. I don't see why we need an exec_id there | 02:09 |
bergwolf_ | yup | 02:09 |
*** kratec has quit IRC | 02:11 | |
sboeuf | bergwolf_: ok then this looks good, you can even merge 2. and 5., since this is related to CreateContainer( | 02:11 |
*** kratec has joined #kata-dev | 02:12 | |
bergwolf_ | sboeuf, I seperated them because 5 can have more disscussion ;) | 02:14 |
sboeuf | bergwolf_: fair enough! | 02:15 |
sboeuf | bergwolf_: BTW, I have an unrelated question, do you know how you determine the number of CPUs for the VM in runv | 02:15 |
bergwolf_ | sboeuf, btw, will you take vacations for Christmas? | 02:15 |
sboeuf | bergwolf_: yes I will be in vacation next week, just checking emails | 02:15 |
sboeuf | bergwolf_: but I am going to implement the agent side of this PR tonight, this will be ready tomorrow | 02:16 |
bergwolf_ | sboeuf, by calculating through the spec. I remember there is something related there | 02:16 |
bergwolf_ | sboeuf, cool, I was just wondering if you are OOO I can take it over so that we don't wait too long :) | 02:16 |
sboeuf | bergwolf_: so you mean relying on the cgroups ? | 02:17 |
bergwolf_ | if you can get it ready soon that'll be really great | 02:17 |
sboeuf | bergwolf_: sure no worries ;) | 02:17 |
bergwolf_ | let me check runv | 02:17 |
*** liujiong has quit IRC | 02:18 | |
sboeuf | are you taking vacation for Christmas, or is it different in China ? (I guess it is since Chinese new year is a different time of year) | 02:18 |
bergwolf_ | sboeuf, https://github.com/hyperhq/runv/commit/86ade7632ef7253336950f24ace3418f17f78729 | 02:18 |
bergwolf_ | no we don't have Chrismas holiday :( | 02:19 |
bergwolf_ | but the team is leaving for team building after Chrismas for the New year | 02:19 |
*** liujiong has joined #kata-dev | 02:20 | |
sboeuf | bergwolf_: oh okay this sounds nice :) | 02:20 |
bergwolf_ | and yes we have Chinese new year vacation that is about in Feb | 02:20 |
sboeuf | Ok! | 02:20 |
sboeuf | bergwolf_: ok so you rely on the spec resources, that seems pretty straightforward. And every time you have a new container on the same pod, you hotplug the right number of CPU expected by the container to the VM ? | 02:22 |
sboeuf | bergwolf_: do you constraint this container inside the VM ? So that you can respect the expected amount of CPU for each container ? | 02:23 |
sboeuf | I mean with cgroups inside the VM ? | 02:23 |
bergwolf_ | no, hyperstart does not have cgroups support | 02:25 |
sboeuf | bergwolf_: ok got it. Kata agent will have it yay ! | 02:25 |
bergwolf_ | but it's good to have cgroups since libcontainer supports it already | 02:26 |
bergwolf_ | yup! | 02:26 |
sboeuf | ok now last question, how do you manage unconstrained containers ? | 02:26 |
bergwolf_ | no at all :) | 02:27 |
sboeuf | if you don't get any info from the spec about that, are you allocating 128 cores to your VM if the server is 128 cores | 02:27 |
sboeuf | bergwolf_: what do you mean ??? | 02:27 |
sboeuf | bergwolf_: you don't want to handle this case ? | 02:27 |
bergwolf_ | oh, I thought you were asking if we manage unconstrained inside sandbox | 02:29 |
*** atuvenie has joined #kata-dev | 02:29 | |
bergwolf_ | we have a default cpu/memory set for every sandbox | 02:30 |
bergwolf_ | if the spec does not specify it, use the default one | 02:30 |
bergwolf_ | I thought cc has a default value as well, right? | 02:30 |
sboeuf | bergwolf_: what is the number ? I think we are using 2 CPUs with 2G of ram | 02:30 |
bergwolf_ | oh, you have many machines :) We use 1 CPU and 128MB ram | 02:31 |
sboeuf | ok sounds good | 02:31 |
sboeuf | this is actually the default value you use to from your template VM, right ? | 02:32 |
sboeuf | and then if it is specified some constraints, you hotplug more CPUs and Memory, if not, you do nothing I guess | 02:32 |
bergwolf_ | I'm not very certain but I think yes it is | 02:32 |
sboeuf | bergwolf_: ok thx I wanted to know how you were handling this. I am sure we'll have this discussion in a few weeks about Kata :) | 02:33 |
bergwolf_ | yup, we'll need default values for kata as well ;) | 02:34 |
sboeuf | yes we do! | 02:34 |
*** atuvenie has quit IRC | 02:34 | |
sboeuf | I think Graham (from CC) was saying than VM with 2 CPUs start faster than VM with 1 | 02:35 |
sboeuf | but that 3 or 4 CPUs was not worth it | 02:35 |
sboeuf | need to check with him on that | 02:35 |
sboeuf | bergwolf_: thx for the discussion ! I gotta go, I'll implement what we talked about later tonight ! | 02:39 |
bergwolf_ | cool, ttyl! | 02:41 |
*** bergwolf_ has quit IRC | 02:48 | |
*** coolsvap has joined #kata-dev | 03:26 | |
*** liujiong has quit IRC | 03:34 | |
*** liujiong has joined #kata-dev | 03:37 | |
*** diga has joined #kata-dev | 03:41 | |
*** liwei has joined #kata-dev | 03:46 | |
*** atuvenie has joined #kata-dev | 04:29 | |
*** atuvenie has quit IRC | 04:34 | |
*** kratec has quit IRC | 06:15 | |
*** kratec has joined #kata-dev | 06:16 | |
*** kratec has joined #kata-dev | 06:16 | |
*** liujiong has quit IRC | 07:02 | |
*** liujiong has joined #kata-dev | 07:02 | |
*** liujiong has quit IRC | 07:02 | |
*** kratec has quit IRC | 07:09 | |
*** kratec has joined #kata-dev | 07:10 | |
*** atuvenie has joined #kata-dev | 08:36 | |
*** atuvenie has quit IRC | 08:41 | |
*** sameo has joined #kata-dev | 08:47 | |
*** atuvenie has joined #kata-dev | 08:56 | |
*** atuvenie has quit IRC | 09:01 | |
*** atuvenie has joined #kata-dev | 09:04 | |
*** jodh has joined #kata-dev | 09:11 | |
*** jodh has quit IRC | 09:11 | |
*** jodh has joined #kata-dev | 09:11 | |
*** atuvenie has quit IRC | 09:12 | |
*** atuvenie has joined #kata-dev | 09:18 | |
*** atuvenie has quit IRC | 09:23 | |
*** gwhaley has joined #kata-dev | 09:27 | |
*** sameo has quit IRC | 09:53 | |
*** kratec has quit IRC | 10:06 | |
*** kratec has joined #kata-dev | 10:07 | |
*** atuvenie has joined #kata-dev | 10:12 | |
*** sameo has joined #kata-dev | 10:14 | |
*** cdent has joined #kata-dev | 10:46 | |
*** sameo has quit IRC | 11:39 | |
*** gwhaley has quit IRC | 12:06 | |
*** jodh has quit IRC | 12:55 | |
*** coolsvap has quit IRC | 13:06 | |
*** atuvenie has quit IRC | 13:10 | |
*** gwhaley has joined #kata-dev | 13:26 | |
*** jodh has joined #kata-dev | 13:30 | |
*** jodh has quit IRC | 13:30 | |
*** jodh has joined #kata-dev | 13:30 | |
*** devimc has joined #kata-dev | 13:32 | |
*** kratec has quit IRC | 13:34 | |
*** kratec has joined #kata-dev | 13:36 | |
*** sameo has joined #kata-dev | 14:17 | |
*** cdent has quit IRC | 14:24 | |
*** devimc has quit IRC | 14:50 | |
*** gwhaley has quit IRC | 14:56 | |
*** sameo has quit IRC | 15:00 | |
*** marst has joined #kata-dev | 15:17 | |
*** devimc has joined #kata-dev | 15:19 | |
*** devimc has joined #kata-dev | 15:20 | |
*** jodh has quit IRC | 15:36 | |
*** rcw has joined #kata-dev | 15:39 | |
*** diga has quit IRC | 17:15 | |
*** kratec has quit IRC | 17:28 | |
*** kratec has joined #kata-dev | 17:28 | |
sboeuf | bergwolf: ping | 18:03 |
*** jgriffith is now known as jgriffith-merryC | 18:34 | |
*** jgriffith-merryC is now known as jgriffith | 18:34 | |
*** devimc has quit IRC | 21:08 | |
*** rcw has quit IRC | 21:18 | |
*** sameo has joined #kata-dev | 22:32 | |
*** marst has quit IRC | 23:48 |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!