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.

Sign up for GitHub

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

Jump to bottom

Improve cri-o support by using pull_type #376

Merged
merged 2 commits into from
Jul 11, 2024
Merged

Improve cri-o support by using pull_type #376

merged 2 commits into from
Jul 11, 2024

Conversation

beraldoleal
Copy link
Member

@beraldoleal beraldoleal commented May 3, 2024 •
edited
Loading

Kata containers is introducing a handler for PULL_TYPE variable to allow configuring CRI-O for guest-pull image pulling.
Lets adapt the operator to propagate this variable.

Most of the work here was made by @wainersm before leaving PTO, I just fixed a few things.

This is related to #365.

@beraldoleal beraldoleal marked this pull request as draft May 3, 2024 13:44
@bpradipt
Copy link
Member

@beraldoleal @wainersm any plans to have this PR merged for the upcoming 0.9.0 release ? It will be great to have support for deploying CoCo operator on crio based K8s clusters

@beraldoleal beraldoleal marked this pull request as ready for review June 21, 2024 18:34
@beraldoleal
Copy link
Member Author

beraldoleal commented Jun 21, 2024 •
edited
Loading

Since #365 was merged, I'm removing the draft flag here.

@beraldoleal beraldoleal changed the title Draft: Improve cri-o support by using pull_type Improve cri-o support by using pull_type Jun 21, 2024
@bpradipt bpradipt requested a review from wainersm June 22, 2024 05:02
ldoktor
ldoktor reviewed Jun 25, 2024
View reviewed changes
controllers/ccruntime_controller.go
{
Name: "PULL_TYPE_MAPPING",
Value: strings.Join(pull_type_mapping, ","),
},
Copy link
Contributor

Choose a reason for hiding this comment

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

This is outside my comfort zone, but it looks consistent with how snapshotter is treated.

@bpradipt bpradipt requested a review from ldoktor July 10, 2024 11:43
@wainersm @beraldoleal
pre-reqs: add support to cri-o
c4f2024 
Currently when deploying using cri-o, reqs-deploy dies with "cri-o is
not yet supported". Lets add cri-o support here.

Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
@beraldoleal beraldoleal force-pushed the crio-support-v1 branch 2 times, most recently from ff800ad to 7759b87 Compare July 10, 2024 14:58
@wainersm @beraldoleal
controller: add support to pull_type
b986227 
The same way we can customize the snapshotter to be used by the runtime
class, lets add a new property to customize the pulling image method.

Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
Signed-off-by: Beraldo Leal <bleal@redhat.com>
bpradipt
bpradipt approved these changes Jul 10, 2024
View reviewed changes
Copy link
Member

@bpradipt bpradipt left a comment

Choose a reason for hiding this comment

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

/lgtm

@bpradipt
Copy link
Member

Tested this successfully for peer-pods.

kubectl logs -n confidential-containers-system cc-operator-daemon-install-l7589
Environment variables passed to this script
* NODE_NAME: peer-pods-worker
* DEBUG: false
* SHIMS: remote
* DEFAULT_SHIM:
* CREATE_RUNTIMECLASSES: true
* CREATE_DEFAULT_RUNTIMECLASS: false
* ALLOWED_HYPERVISOR_ANNOTATIONS:
* SNAPSHOTTER_HANDLER_MAPPING: remote:nydus
* AGENT_HTTPS_PROXY:
* AGENT_NO_PROXY:
* PULL_TYPE_MAPPING: remote:guest-pull
copying kata artifacts onto host
Creating the runtime classes
Creating the kata-remote runtime class
runtimeclass.node.k8s.io/kata-remote created
Add Kata Containers as a supported runtime for CRIO:

[crio.runtime.runtimes.kata-remote]
	runtime_path = "/usr/local/bin/containerd-shim-kata-remote-v2"
	runtime_type = "vm"
	runtime_root = "/run/vc"
	runtime_config_path = "/opt/kata/share/defaults/kata-containers//configuration-remote.toml"
	privileged_without_host_devices = true
	runtime_pull_image = true
node/peer-pods-worker labeled

@bpradipt bpradipt requested a review from stevenhorsman July 10, 2024 17:09
@bpradipt bpradipt mentioned this pull request Jul 10, 2024
Merged
@beraldoleal
Copy link
Member Author

/test

1 similar comment
@wainersm
Copy link
Member

/test

@beraldoleal
Copy link
Member Author

@ldoktor thanks for the review, let me know if you have further comments.

stevenhorsman
stevenhorsman approved these changes Jul 11, 2024
View reviewed changes
Copy link
Member

@stevenhorsman stevenhorsman left a comment

Choose a reason for hiding this comment

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

The code looks fine to me. It would be good to have a cri-o flavour e2e job, but I guess that can come later.

@bpradipt bpradipt merged commit 6bba65f into confidential-containers:main Jul 11, 2024
11 of 12 checks passed
bpradipt added a commit to bpradipt/operator that referenced this pull request Jul 11, 2024
@bpradipt
crd: Fix manifest due to changes in the ccRuntime CRD
8711931 
PR confidential-containers#376 introduced
support for pull_type option, however the manifest update was missed.
This fixes it.

Signed-off-by: Pradipta Banerjee <pradipta.banerjee@gmail.com>
@bpradipt bpradipt mentioned this pull request Jul 11, 2024
Merged
@wainersm wainersm mentioned this pull request Aug 6, 2024
Closed
This was referenced Aug 6, 2024
Open
Closed
@beraldoleal beraldoleal mentioned this pull request Aug 16, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stevenhorsman stevenhorsman stevenhorsman approved these changes

@bpradipt bpradipt bpradipt approved these changes

@wainersm wainersm Awaiting requested review from wainersm

@ldoktor ldoktor Awaiting requested review from ldoktor

Assignees
No one assigned
Labels
ok-to-test
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@beraldoleal @bpradipt @wainersm @ldoktor @stevenhorsman