-
Notifications
You must be signed in to change notification settings - Fork 376
Conversation
Interesting change. IIU, that is conceptually quite a change - effectively turning off systemd in our .img images (by skipping it as init)? So, questions:
I'm all for optimisation and boot/speed improvements - just want to check we've looked at all the knock-on consequences. /cc @egernst @mcastelino @WeiZhang555 @bergwolf for any thoughts. |
And the CIs are really not happy. I'm seeing all sorts of odd fails, like the runtime is maybe just bust? Can you check those out to see if it was spurious or a real issue pls?
|
Hi @devimc - I agree with @grahamwhaley, it would be great to get a bit more detail on this. fwics, the problem with this change is that it assumes either the runtime's I think what you need to do is create an osbuilder PR that will effectively run the following when building an image where
That will then create an image that does not require any systemd options to be specified in |
I think @grahamwhaley made a good question list 😄 I'm not very clear about the usage, if user want to use agent as init, they can also choose a custom image file, so what's the difference with that? |
Ping @devimc :) |
Codecov Report
@@ Coverage Diff @@
## master #768 +/- ##
=========================================
Coverage ? 65.29%
=========================================
Files ? 87
Lines ? 10604
Branches ? 0
=========================================
Hits ? 6924
Misses ? 2983
Partials ? 697 |
@devimc Any updates? |
@jodh-intel @amshinde sorry, nop, I haven't had time to run any metrics script, I'm working on the namespaces stuff |
@devimc ping :) |
@devimc nudge, any progress? 😄 |
bc9d88a
to
69aed29
Compare
/test |
69aed29
to
42d7352
Compare
/test |
42d7352
to
7340483
Compare
/test |
7340483
to
803a59a
Compare
/test |
# | ||
# NOTE: If you want to use systemd or any other init daemon, the `init` parameter | ||
# should be used here. For example for systemd you have to add following line: | ||
# init=/lib/systemd/systemd systemd.unit=@PROJECT_TAG@.target systemd.mask=systemd-networkd.service systemd.mask=systemd-networkd.socket |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to be a little careful about the wording here as we will be using systemd by default.
Also, since we're removing the systemd masks here, shouldn't they be added to the osbuilder PR (or maybe we no longer need them at all)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, since we're removing the systemd masks here, shouldn't they be added to the osbuilder PR (or maybe we no longer need them at all)?
we don't need them, these mask files are present if systemd is installed (AGENT_INIT=no in osbuilder)
@grahamwhaley do you know why metrics CI is not happy, was a regression detected? |
Heh heh, let's call it a 'change', and not a regression... it looks like:
all items (boot time and memory footprint) shrank. In conclusion - all the changes were in the 'good' direction - which is less of a worry than if they all went the other direction ;-) |
803a59a
to
ae5dd2d
Compare
@grahamwhaley thanks, I'll use the report generator |
ae5dd2d
to
e7b2447
Compare
@devimc please answer the questions you have been asked since it would help everybody to understand what is the purpose of this change. As mentioned by @WeiZhang555:
|
Sorry, I forgot that question
There are no difference, but currently we are tied to systemd because we have hardcoded it, we use systemd even when we don't know if the image has it func needSystemd(config vc.HypervisorConfig) bool {
return config.ImagePath != ""
} from my point of view and now that we can use cc @sboeuf |
Thanks @devimc, this makes sense to me. Several comments based on this:
|
sure thing
with kata-agent as init? I don't know, @jodh-intel any thoughts?
I will do it |
Let kernel start the default init daemon (/sbin/init) and rid of all related to systemd. Currently we are tied to systemd because we have hardcoded it, we use systemd even when we don't know if the image has it. Now that we can use kata-agent as init process in rootfs images (kata-containers/agent#376), we should use it, since we are responsible for any security flaw found in kata-containers, it doesn't matter if we wrote that code or not (systemd). The usage of kata-agent as init process brings performance improvements in boot time and memory footprint. Depends-on: github.com/kata-containers/tests#948 fixes kata-containers#767 Signed-off-by: Julio Montes <julio.montes@intel.com>
e7b2447
to
5430aa5
Compare
/test |
Well I think I might be confused, I was thinking we were pushing logs to the journal (inside the guest) from the agent, and those might not be available if systemd was not initializing the journal. |
@devimc If users want speed and density, why not just use initrd with agent as the init process instead? I'm asking because I thought rootfs+systemd aims to provide more features (such as logging and other VM-like services). In your metrics tests, could you please add comparison with intird+agent-as-init? |
Thanks for the graphs @devimc |
Whatever we decide, since we offer these different image types, we need something like the following to help users decide what is most appropriate option for them as it's starting to get confusing (bad! :):
|
Table of options and pros and cons, absolutely - we have been missing that for some time (probably since we added the factory as the 3rd option at least). I am of course going to suggest it gets added to the WIP User Guide |
@grahamwhaley - good call and done: https://github.com/kata-containers/documentation/wiki/UserGuide#images. @devimc (and in fact anyone else! ;) - feel free to add to that document! ;) |
yes, a laptop with full UI |
Boot timeMemory footprint |
Quick summary of comments from discussion in the Arch Council call just now:
There seemed to no dissent (I won't call it consensus since there wasn't affirmative assent either :) for the idea that if 3 could somehow be addressed, this is a good idea. One suggestion for how to tackle 3 is to leverage Agent's existing support for running and managing containers to... run and manage "init" services as static containers specified in the image. A strawman for how such a thing could be implemented:
This is borrowing heavily from the Kubernetes concept of Static Pods, although it goes one level higher in generality and lets the init behavior be a static set of RPCs. This wouldn't work if you wanted to do anything dynamic, although you could always Inception things and kick off a privileged bootstrap container that connects to the agent and dynamically issues RPCs. An alternative, if that level of generality isn't desirable, would be to populate the path with just OCI specs, at which point it really looks like Static Pods (or systemd services, for that matter). |
Great write-up @jon! Reading it has just reminded me that we in fact already have a very similar capability in the agent today - guest hooks: |
Hi @jon thanks for the summary 😄 |
Let kernel start the default init daemon (/sbin/init) and rid of
all related to systemd.
Currently we are tied to systemd because we have hardcoded it, we use
systemd even when we don't know if the image has it. Now that we can use
kata-agent as init process in rootfs images (kata-containers/agent#376), we
should use it, since we are responsible for any security flaw found in
kata-containers, it doesn't matter if we wrote that code or not (systemd).
The usage of kata-agent as init process brings performance improvements in
boot time and memory footprint.
Depends-on: github.com/kata-containers/tests#948
fixes #767
Signed-off-by: Julio Montes julio.montes@intel.com