Friday, 2017-12-22

*** sameo has quit IRC00:36
sboeufbergwolf: hey!01:21
*** bergwolf_ has joined #kata-dev01:22
sboeufbergwolf_:  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 IRC01:24
bergwolf_sboeuf, yup, in runv shim, it waits for SIGUSR1 if it finds out itself is an init process shim01:24
bergwolf_@sboeuf, https://github.com/hyperhq/runv/blob/master/cli/shim.go#L5901:25
sboeufbergwolf_: 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-shim01:25
sboeufbergwolf_: 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 runtime01:26
bergwolf_sboeuf, we can use container_id for the init process. what do you think?01:28
sboeufbergwolf_: 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 process01:28
sboeufbergwolf_: well container ID can be the solution I guess01:29
bergwolf_sboeuf, yes a boolean argument can work as well IMO01:29
bergwolf_sboeuf, but for agent, it seems container_id is better01:29
bergwolf_as you said for the reaped PID mapping issue01: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 comment01:31
sboeufso if we start a shim with containerID == processID, then the shim will know this is a container01:31
sboeufand then we need a signal to signal the shim it can start waiting for the process because it has been started01:32
bergwolf_yes, that's the idea01:32
sboeufok that seems to work. And I agree you should comment on the github to make sure all of us agree01:32
bergwolf_cool thanks for the feedback!01:33
sboeufbergwolf_: 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 started01:34
sboeufI 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
sboeufbergwolf_: 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 IRC01:38
*** liujiong has joined #kata-dev01:38
bergwolf_sboeuf, I think the runtime should call start container before sending SIGUSR1 to the shim01:39
bergwolf_then the shim will never wait on a created by not started container01:39
bergwolf_s/by/but01:39
sboeufbergwolf_: 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
sboeufI think waiting on a started container (aka process) is better because this sounds logical !01:41
*** liujiong has quit IRC01:41
sboeufbergwolf_: do you mind if I do the changes to the agent ? you should do those the changes to the shim01:41
bergwolf_sboeuf, yup I totally agree. there is nothing to wait if container is not started01:41
bergwolf_sboeuf, I'm fine with it01:42
*** liujiong has joined #kata-dev01:42
sboeufbergwolf_: ok cool. Let me try to summarize the conversation on the github ticket then and we can move forward with the implementation01: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 id01: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
sboeufbergwolf_: I think this is nice, except that I wonder if we should not be fully declarative about StartCOntainer() by providing a process ID too01:46
sboeufI mean we can leave the knowledge of container_id == task_id to the runtime01:46
bergwolf_sboeuf, in that case we must require it to be equal to container ID01:47
sboeufbut let's the agent be more generic01:47
sboeufbergwolf_: well that's the point, I am not sure we need the agent to be aware about this01: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 can01:49
sboeufbergwolf_: 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 ID01:49
sboeufbergwolf_: yes it can, we just store that somewhere in the agent01:50
bergwolf_yup, I agree with you. It makes sense to keep agent common in this case01:50
sboeufI didn't want to tie the entire protocol to this specific containerID == processID, even if we're gonna use it01:50
sboeufbergwolf_: cool :)01:50
sboeufbergwolf_: 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 runtime01:51
sboeufI think Samuel wanted to make sure there is no confusion with real PID01:51
bergwolf_sboeuf, I think exec_id is better but I can settle with task_id if you insist01:52
sboeufbergwolf: oh no I am fine with exec_id too01:52
bergwolf_cool01:52
sboeufso let's go with exec_id01:52
bergwolf_yup01:52
sboeufabout 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_id01:53
sboeufbergwolf_: are you gonna update the github issue ?01:59
bergwolf_yup, just sent it02:00
bergwolf_sboeuf, please see if it's OK02:01
sboeufbergwolf_: 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
sboeufbergwolf_: no problem ;) just trying to make sure everybody's on the same page02:03
bergwolf_@sboeuf, hmmm, no, actually I do mean CreateContainer()02:05
bergwolf_that is where the exec info is passed to the agent02:05
bergwolf_I think it's better to name it there as well02:05
sboeufbergwolf_: 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 already02:07
sboeufI know, I was thinking if we really want to make everything very explicit, we have to pass it every time02:07
sboeufThis is called StartContainer, you're right, the container_id should be enough in that case02:08
bergwolf_hmmm, StartContainer() is to start the container init process. I don't see why we need an exec_id there02:09
bergwolf_yup02:09
*** kratec has quit IRC02:11
sboeufbergwolf_: ok then this looks good, you can even merge 2. and 5., since this is related to CreateContainer(02:11
*** kratec has joined #kata-dev02:12
bergwolf_sboeuf, I seperated them because 5 can have more disscussion ;)02:14
sboeufbergwolf_: fair enough!02:15
sboeufbergwolf_: BTW, I have an unrelated question, do you know how you determine the number of CPUs for the VM in runv02:15
bergwolf_sboeuf, btw, will you take vacations for Christmas?02:15
sboeufbergwolf_: yes I will be in vacation next week, just checking emails02:15
sboeufbergwolf_: but I am going to implement the agent side of this PR tonight, this will be ready tomorrow02:16
bergwolf_sboeuf, by calculating through the spec. I remember there is something related there02: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
sboeufbergwolf_: so you mean relying on the cgroups ?02:17
bergwolf_if you can get it ready soon that'll be really great02:17
sboeufbergwolf_: sure no worries ;)02:17
bergwolf_let me check runv02:17
*** liujiong has quit IRC02:18
sboeufare 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/86ade7632ef7253336950f24ace3418f17f7872902: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 year02:19
*** liujiong has joined #kata-dev02:20
sboeufbergwolf_: oh okay this sounds nice :)02:20
bergwolf_and yes we have Chinese new year vacation that is about in Feb02:20
sboeufOk!02:20
sboeufbergwolf_: 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
sboeufbergwolf_: do you constraint this container inside the VM ? So that you can respect the expected amount of CPU for each container ?02:23
sboeufI mean with cgroups inside the VM ?02:23
bergwolf_no, hyperstart does not have cgroups support02:25
sboeufbergwolf_: ok got it. Kata agent will have it yay !02:25
bergwolf_but it's good to have cgroups since libcontainer supports it already02:26
bergwolf_yup!02:26
sboeufok now last question, how do you manage unconstrained containers ?02:26
bergwolf_no at all :)02:27
sboeufif you don't get any info from the spec about that, are you allocating 128 cores to your VM if the server is 128 cores02:27
sboeufbergwolf_: what do you mean ???02:27
sboeufbergwolf_: you don't want to handle this case ?02:27
bergwolf_oh, I thought you were asking if we manage unconstrained inside sandbox02:29
*** atuvenie has joined #kata-dev02:29
bergwolf_we have a default cpu/memory set for every sandbox02:30
bergwolf_if the spec does not specify it, use the default one02:30
bergwolf_I thought cc has a default value as well, right?02:30
sboeufbergwolf_: what is the number ? I think we are using 2 CPUs with 2G of ram02:30
bergwolf_oh, you have many machines :) We use 1 CPU and 128MB ram02:31
sboeufok sounds good02:31
sboeufthis is actually the default value you use to from your template VM, right ?02:32
sboeufand then if it is specified some constraints, you hotplug more CPUs and Memory, if not, you do nothing I guess02:32
bergwolf_I'm not very certain but I think yes it is02:32
sboeufbergwolf_: 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
sboeufyes we do!02:34
*** atuvenie has quit IRC02:34
sboeufI think Graham (from CC) was saying than VM with 2 CPUs start faster than VM with 102:35
sboeufbut that 3 or 4 CPUs was not worth it02:35
sboeufneed to check with him on that02:35
sboeufbergwolf_: 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 IRC02:48
*** coolsvap has joined #kata-dev03:26
*** liujiong has quit IRC03:34
*** liujiong has joined #kata-dev03:37
*** diga has joined #kata-dev03:41
*** liwei has joined #kata-dev03:46
*** atuvenie has joined #kata-dev04:29
*** atuvenie has quit IRC04:34
*** kratec has quit IRC06:15
*** kratec has joined #kata-dev06:16
*** kratec has joined #kata-dev06:16
*** liujiong has quit IRC07:02
*** liujiong has joined #kata-dev07:02
*** liujiong has quit IRC07:02
*** kratec has quit IRC07:09
*** kratec has joined #kata-dev07:10
*** atuvenie has joined #kata-dev08:36
*** atuvenie has quit IRC08:41
*** sameo has joined #kata-dev08:47
*** atuvenie has joined #kata-dev08:56
*** atuvenie has quit IRC09:01
*** atuvenie has joined #kata-dev09:04
*** jodh has joined #kata-dev09:11
*** jodh has quit IRC09:11
*** jodh has joined #kata-dev09:11
*** atuvenie has quit IRC09:12
*** atuvenie has joined #kata-dev09:18
*** atuvenie has quit IRC09:23
*** gwhaley has joined #kata-dev09:27
*** sameo has quit IRC09:53
*** kratec has quit IRC10:06
*** kratec has joined #kata-dev10:07
*** atuvenie has joined #kata-dev10:12
*** sameo has joined #kata-dev10:14
*** cdent has joined #kata-dev10:46
*** sameo has quit IRC11:39
*** gwhaley has quit IRC12:06
*** jodh has quit IRC12:55
*** coolsvap has quit IRC13:06
*** atuvenie has quit IRC13:10
*** gwhaley has joined #kata-dev13:26
*** jodh has joined #kata-dev13:30
*** jodh has quit IRC13:30
*** jodh has joined #kata-dev13:30
*** devimc has joined #kata-dev13:32
*** kratec has quit IRC13:34
*** kratec has joined #kata-dev13:36
*** sameo has joined #kata-dev14:17
*** cdent has quit IRC14:24
*** devimc has quit IRC14:50
*** gwhaley has quit IRC14:56
*** sameo has quit IRC15:00
*** marst has joined #kata-dev15:17
*** devimc has joined #kata-dev15:19
*** devimc has joined #kata-dev15:20
*** jodh has quit IRC15:36
*** rcw has joined #kata-dev15:39
*** diga has quit IRC17:15
*** kratec has quit IRC17:28
*** kratec has joined #kata-dev17:28
sboeufbergwolf: ping18:03
*** jgriffith is now known as jgriffith-merryC18:34
*** jgriffith-merryC is now known as jgriffith18:34
*** devimc has quit IRC21:08
*** rcw has quit IRC21:18
*** sameo has joined #kata-dev22:32
*** marst has quit IRC23:48

Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!