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

Support other flavour of CRI #1442

Open
wants to merge 23 commits into
base: main
Choose a base branch
from
Open

Support other flavour of CRI #1442

wants to merge 23 commits into from

Conversation

CiraciNicolo
Copy link
Contributor

Change description

This PR implements support for different CRI implementations, instead of the default containerd. At this moment the only other CRI implementation supported is CRI-O. By default containerd is still the default CRI, to start using CRI-O references the packer configuration called "qemu-ubuntu-2204-crio.json".

Currently the only operating systems supported are debian based and centos based. It should also work on rhel but not tested.

The implementation can be done following this steps:

  • create an ansible role for your CRI flavour
  • implement default packer variables
  • create builder target and makefile target

Both ansible role and default values should be called as your flavour, in this way you can set the cri_flavour build variable and all tasks/var should be included.

Be also careful to correctly default the variables in images/capi/packer/config/ansible-args.json and implement the goss specs.

Related issues

No issues are addressed by this PR

This PR also includes the following fixes:

  • Fix incorrect boolean evalutation for GVisor installation if enabled. Be careful that this fixes could change the expected behaviour if you are leveraging Gvisor

Additional context

This PR is a de facto continuation of #1437, but all my commits have been cherry picked to a dedicated branch.

@CiraciNicolo
feat(crio): support other CRI flavours
15d7068 
This first commit implements the following components:
- Ansible playbook to configure CRI-O
- Define packer build context for QEMU and Ubuntu 2204

This implementations is limited, more OS and infrastructure provider will be probably supported in the following commits. The default CRI flavour is containerd and therefore a new packer common variable has been introduced for providing a default value.

Signed-off-by: Nicolò Ciraci <[email protected]>
@CiraciNicolo
feat(crio): fixed crio installation and missing crictl
7bfcd66 
This commit also include documentation tweaks, the packer variables for a qemu build using crio and generalize the cri socket
@CiraciNicolo
feat(crio): implement goss spec for crio and edit docs
9f5aaa2
@CiraciNicolo
feat(crio-gvisor): This commit partially implements gvisor support
ea5b5b4 
Gvisor support is implemented by this commit but it is actually broken,
you can refer to google/gvisor#3283 to see the
progress. This commit just implements the heavy lifting needed in the
future.
@CiraciNicolo
fix(crio): remove default CNI for crio
5d34982
@CiraciNicolo
feat(banner): add banner
a04aa8f
@CiraciNicolo
fix(crio): Small fixes for gvisor compat, docs tweaks and CRI-O CNI
dd8a4ee
@CiraciNicolo
feat(crio): add support for centos (redhat)
d86cd65 
Signed-off-by: Nicolò Ciraci <[email protected]>
@CiraciNicolo
feat(crio): Fix documentation and formatting
017ed6c
@CiraciNicolo
feat(crio): json sort
c5200ff
@CiraciNicolo
fix(lint): fix linting errors
ccd10dd
@CiraciNicolo
feat(crio): linted ansible files
1b01f1f
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Apr 11, 2024
@k8s-ci-robot
Copy link
Contributor

Hi @CiraciNicolo. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Apr 11, 2024
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign kkeshavamurthy for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

This was referenced Apr 11, 2024
Closed
Open
@mboersma
Copy link
Contributor

/ok-to-test

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Apr 15, 2024
@mboersma
Copy link
Contributor

/retest

@CiraciNicolo
Copy link
Contributor Author

Hi! Am I misunderstanding, or are there problems in the pipeline?

Powering off VirtualMachine:vm-276295... Error: The attempted operation cannot be performed in the current state (Powered off).

@mboersma
Copy link
Contributor

/retest

I think that's a flake. We'll see.

@CiraciNicolo
Copy link
Contributor Author

Regarding this PR, we were discussing the feasibility of deprecating some variables. Before merging, can we continue with that? It would be pretty nice to unify the variables.

@mboersma mboersma added the kind/feature Categorizes issue or PR as related to a new feature. label May 1, 2024
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 3, 2024
@k8s-ci-robot
Copy link
Contributor

Adding label do-not-merge/contains-merge-commits because PR contains merge commits, which are not allowed in this repository.
Use git rebase to reapply your commits on top of the target branch. Detailed instructions for doing so can be found here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@k8s-ci-robot k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jun 5, 2024
@CiraciNicolo
chore: sort json
f0446a4 
Signed-off-by: Nicolò Ciraci <[email protected]>
@CiraciNicolo
chore: linted files
28b5b1b 
Signed-off-by: Nicolò Ciraci <[email protected]>
@CiraciNicolo
refactor: refactor some files, removed a block
dc669cd 
Signed-off-by: Nicolò Ciraci <[email protected]>
@k8s-ci-robot k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jun 5, 2024
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

  • Mark this PR as fresh with /remove-lifecycle stale
  • Close this PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 9, 2024
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 7, 2024
@mboersma
Copy link
Contributor

mboersma commented Oct 7, 2024

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Oct 7, 2024
@k8s-ci-robot
Copy link
Contributor

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@k8s-ci-robot
Copy link
Contributor

@CiraciNicolo: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
pull-gcp-all 0ab2bbc link true /test pull-gcp-all

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mboersma mboersma Awaiting requested review from mboersma

@rohityadavcloud rohityadavcloud Awaiting requested review from rohityadavcloud

Assignees
No one assigned
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/contains-merge-commits kind/feature Categorizes issue or PR as related to a new feature. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@CiraciNicolo @k8s-ci-robot @mboersma @k8s-triage-robot