Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use host systemctl #242

Merged
merged 5 commits into from
Aug 14, 2023
Merged

Conversation

jepio
Copy link
Member

@jepio jepio commented Aug 9, 2023

This adapts the operator to changes that happened in kata-deploy in kata-containers/kata-containers#7520, which switched to using nsenter to interact with systemd.

@jepio
Copy link
Member Author

jepio commented Aug 9, 2023

FYI: i haven't tested this in any way, i hope we have sufficient CI coverage....

@jepio
Copy link
Member Author

jepio commented Aug 9, 2023

/test

@jepio
Copy link
Member Author

jepio commented Aug 9, 2023

/test

@fidencio
Copy link
Member

fidencio commented Aug 9, 2023

@jepio, nice that you have updated the pre-install payload to do the same, but you'll also need to update https://github.com/confidential-containers/operator/blob/main/install/pre-install-payload/scripts/container-engine-for-cc-deploy.sh

fidencio added a commit to fidencio/enclave-cc that referenced this pull request Aug 9, 2023
The operator started using HostPID=true, allowing us to use nsenter to
interact with systemd in the host. We do the same for kata payload
installation with kata-deploy.

There's a little bit of chicken-egg problem here, as this PR depends on
confidential-containers/operator#242, and that
PR depends on this one.

Signed-off-by: Fabiano Fidêncio <[email protected]>
@mythi
Copy link
Contributor

mythi commented Aug 10, 2023

To get enclave-cc properly tested, let's use vanilla kind node image and update ccruntime-enclave-cc.yaml

@jepio
Copy link
Member Author

jepio commented Aug 10, 2023

trying it right now

@jepio
Copy link
Member Author

jepio commented Aug 10, 2023

/test

jepio added 5 commits August 10, 2023 15:20
Kata-deploy in kata now uses the host's systemctl through nsenter, which
requires running the pod with HostPID=true and allows dropping mounts for
systemd/dbus sockets.

Fixes: #7520
Signed-off-by: Jeremi Piotrowski <[email protected]>
Now that we run with HostPID=true, we can use nsenter to interact with systemd
in the host. We do the same for kata payload installation with kata-deploy.

Fixes: #7520
Signed-off-by: Jeremi Piotrowski <[email protected]>
Since we switched to using host systemctl through nsenter, we no longer need to
install systemd. We also don't need to worry about host/container systemd
compatibility so we can update the pre-install image to the latest Ubuntu LTS.

Signed-off-by: Jeremi Piotrowski <[email protected]>
This bloats the image size and the tarball sits unused.

Signed-off-by: Jeremi Piotrowski <[email protected]>
These are not needed because enclave-cc's deploy script moved to using host
systemctl via nsenter. The other mounts lower in the file are still needed
because the containerd deploy script still relies on them (in the specified
container image version).

Signed-off-by: Jeremi Piotrowski <[email protected]>
@fidencio
Copy link
Member

I've rebased the branch (using GitHub WebUI) as the PR that this one was depending on was merged.

Let's get the tests running now.

@fidencio
Copy link
Member

/test

@fidencio
Copy link
Member

/test-tdx

@fidencio
Copy link
Member

TDX tests are not passing, but I'm 100% sure this is not related to this PR, this I'm taking the bullet and merging it.

Copy link
Member

@fidencio fidencio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, thanks @jepio!

@fidencio fidencio merged commit 88dda3e into confidential-containers:main Aug 14, 2023
@jepio jepio deleted the host-systemctl branch August 14, 2023 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants