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 custom runtime "contrast-cc" #344

Merged
merged 7 commits into from
Apr 22, 2024
Merged

use custom runtime "contrast-cc" #344

merged 7 commits into from
Apr 22, 2024

Conversation

malt3
Copy link
Contributor

@malt3 malt3 commented Apr 17, 2024

This PR switches from the kata-cc-isolation runtime to contrast-cc-<VERSIONHASH>.
I need to include a bunch of somewhat unrelated changes in this PR to make sure it is a clean switch.
Failing e2e tests are due to the test not using the new ContrastTest framework. We need to decide what to do about that. I will probably open a PR in parallel to move the remaining tests.

Test cases:

# just will now install the runtime for all default targets
$ just

Planned follow ups (to limit the scope of this PR):

  • Enforce launch digest in aTLS / manifest
  • Automatically replace runtimeClassName: contrast-cc with runtimeClassName: contrast-cc-<VERSIONHASH> in generate step

@malt3 malt3 requested a review from katexochen as a code owner April 17, 2024 13:33
@malt3 malt3 marked this pull request as draft April 17, 2024 13:33
@malt3 malt3 force-pushed the feat/custom-runtime branch 3 times, most recently from 3716db7 to fbd6ad7 Compare April 18, 2024 07:57
Copy link

github-actions bot commented Apr 18, 2024

PR Preview Action v1.4.7
Preview removed because the pull request was closed.
2024-04-22 15:44 UTC

@malt3 malt3 added the feature Shiny new feature for our users label Apr 18, 2024
@malt3 malt3 force-pushed the feat/custom-runtime branch 2 times, most recently from 09a0eab to c0c7e08 Compare April 18, 2024 08:39
@malt3 malt3 requested a review from burgerdev April 18, 2024 08:59
@malt3 malt3 marked this pull request as ready for review April 18, 2024 08:59
Comment on lines +203 to +208
nix shell .#contrast --command resourcegen runtime workspace/runtime.yml
nix run .#kypatch images -- workspace/runtime.yml \
--replace "ghcr.io/edgelesssys/contrast/node-installer:latest" "$nodeInstallerImgTagged"
Copy link
Contributor

Choose a reason for hiding this comment

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

cc @wirungu: another use case for image patching incoming.

Comment on lines -171 to +169
paths = filterNonCoCoRuntime("kata-cc-isolation", paths, logger)
paths = filterNonCoCoRuntime("contrast-cc", paths, logger)
Copy link
Contributor

Choose a reason for hiding this comment

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

This reminds me that we'll likely need to do something here if kata-containers/kata-containers#8571 would eventually be fixed.

Copy link
Contributor Author

@malt3 malt3 Apr 18, 2024

Choose a reason for hiding this comment

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

Agreed. I do expect that resourcegen will give us a flag to specify the runtimeClassName to select, so this should be a trivial fix.

cli/cmd/policyhash.go Outdated Show resolved Hide resolved
e2e/internal/contrasttest/contrasttest.go Outdated Show resolved Hide resolved
e2e/internal/contrasttest/contrasttest.go Outdated Show resolved Hide resolved
e2e/internal/contrasttest/contrasttest.go Show resolved Hide resolved
node-installer/node-installer.go Show resolved Hide resolved
node-installer/node-installer.go Show resolved Hide resolved
@malt3 malt3 force-pushed the feat/custom-runtime branch 2 times, most recently from 0bfc39a to 91f3a53 Compare April 18, 2024 15:12
@@ -66,6 +66,7 @@ buildGoModule rec {
"-s"
"-w"
"-X main.version=v${version}"
"-X github.com/edgelesssys/contrast/cli/cmd.runtimeHandler=${runtimeHandler}"
Copy link
Member

Choose a reason for hiding this comment

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

Why create a second var when we already have one that contains runtimeHandler?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The other var is in the internal e2e package.

e2e/internal/contrasttest/contrasttest.go Show resolved Hide resolved
@malt3 malt3 force-pushed the feat/custom-runtime branch from 91f3a53 to e4f9222 Compare April 19, 2024 10:02
@malt3 malt3 force-pushed the feat/custom-runtime branch from e4f9222 to 89538ed Compare April 22, 2024 09:16
@malt3 malt3 requested review from burgerdev and katexochen April 22, 2024 11:59
Copy link
Contributor

@burgerdev burgerdev left a comment

Choose a reason for hiding this comment

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

lgtm

node-installer/node-installer.go Show resolved Hide resolved
@malt3 malt3 changed the title custom runtime "contrast-cc" use custom runtime "contrast-cc" Apr 22, 2024
@malt3 malt3 merged commit 4afe3af into main Apr 22, 2024
9 checks passed
@malt3 malt3 deleted the feat/custom-runtime branch April 22, 2024 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Shiny new feature for our users
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants