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

Make nonroot default runtime #8563

Merged
merged 3 commits into from
Oct 27, 2022
Merged

Make nonroot default runtime #8563

merged 3 commits into from
Oct 27, 2022

Conversation

xpivarc
Copy link
Member

@xpivarc xpivarc commented Oct 4, 2022

What this PR does / why we need it:

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

Special notes for your reviewer:

Release note:

Kubevirt now runs with nonroot user by default

@kubevirt-bot
Copy link
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@kubevirt-bot kubevirt-bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note Denotes a PR that will be considered when it comes time to generate release notes. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. labels Oct 4, 2022
@kubevirt-bot kubevirt-bot requested a review from maiqueb October 4, 2022 12:07
@xpivarc
Copy link
Member Author

xpivarc commented Oct 4, 2022

/retest

@xpivarc
Copy link
Member Author

xpivarc commented Oct 6, 2022

/retest

1 similar comment
@vladikr
Copy link
Member

vladikr commented Oct 6, 2022

/retest

@xpivarc
Copy link
Member Author

xpivarc commented Oct 7, 2022

/test pull-kubevirt-e2e-k8s-1.22-sig-compute-realtime
Expected failure

@vladikr
Copy link
Member

vladikr commented Oct 14, 2022

/retest

xpivarc
xpivarc commented Oct 19, 2022
View reviewed changes
pkg/virt-api/webhooks/validating-webhook/admitters/vmi-create-admitter.go
if spec.Domain.CPU != nil && spec.Domain.CPU.Realtime != nil {
if nonroot {
Copy link
Member Author

Choose a reason for hiding this comment

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

@vladikr Maybe adding the sys_nice for time being is better option

Copy link
Member

Choose a reason for hiding this comment

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

I think we should add sys_nice back for the time being - I agree

@xpivarc
Copy link
Member Author

xpivarc commented Oct 19, 2022

/retest

@xpivarc xpivarc marked this pull request as ready for review October 20, 2022 06:33
@kubevirt-bot kubevirt-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 20, 2022
@xpivarc
Copy link
Member Author

xpivarc commented Oct 20, 2022

/retest

@acardace
Copy link
Member

/lgtm

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Oct 21, 2022
@acardace acardace mentioned this pull request Oct 21, 2022
Merged
@vladikr
Copy link
Member

vladikr commented Oct 24, 2022

/approve

@kubevirt-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: vladikr

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

The pull request process is described 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

@kubevirt-bot kubevirt-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 24, 2022
@acardace
Copy link
Member

/test pull-kubevirt-e2e-kind-1.22-sriov

@kubevirt-bot kubevirt-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 25, 2022
@xpivarc
Make nonroot default runtime
097521a 
Signed-off-by: L. Pivarc <[email protected]>
@xpivarc
Fix nonroot tests
6932d31 
Signed-off-by: L. Pivarc <[email protected]>
@xpivarc
Block RT VM with nonroot
c53bed2 
Currently RT VMs require SYS_NICE
capability.

Signed-off-by: L. Pivarc <[email protected]>
@kubevirt-bot kubevirt-bot removed lgtm Indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Oct 26, 2022
@xpivarc
Copy link
Member Author

xpivarc commented Oct 26, 2022

@acardace PTAL

@acardace
Copy link
Member

/lgtm

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Oct 26, 2022
@kubevirt-bot
Copy link
Contributor

kubevirt-bot commented Oct 26, 2022 •
edited
Loading

@xpivarc: The following tests 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-kubevirt-e2e-k8s-1.22-sig-compute-realtime 3986c45 link false /test pull-kubevirt-e2e-k8s-1.22-sig-compute-realtime
pull-kubevirt-check-tests-for-flakes c53bed2 link false /test pull-kubevirt-check-tests-for-flakes
pull-kubevirt-e2e-kind-1.23-vgpu c53bed2 link true /test pull-kubevirt-e2e-kind-1.23-vgpu

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. I understand the commands that are listed here.

@acardace
Copy link
Member

/retest-required

@kubevirt-bot kubevirt-bot merged commit 9c6bccf into kubevirt:main Oct 27, 2022
@rmohr
Copy link
Member

rmohr commented Oct 27, 2022 •
edited
Loading

Hi Lubo. Great change. Does this mean that we test now in the nonroot lanes the same as in the other CI lanes? And as such not testing root anymore?

@xpivarc
Copy link
Member Author

xpivarc commented Oct 27, 2022

Hi Lubo. Great change. Does this mean that we test now in the nonroot lanes the same as in the other CI lanes? And as such not testing root anymore?

For a brief time yes, kubevirt/project-infra#2418 should fix it. We should talk about how long we want to do this in the next community meeting.

@tiraboschi tiraboschi mentioned this pull request Mar 3, 2023
Merged
11 tasks
tiraboschi added a commit to tiraboschi/hyperconverged-cluster-operator that referenced this pull request Mar 3, 2023
@tiraboschi
Workaround for nonRoot FG on Kubevirt
ef247d0 
With kubevirt/kubevirt#8563
Kubevirt introduced a new FG named `Root`,
the `nonRoot` FG we are using is still there but
declared as deprecated.

Unfortunately due to bug on Kubevirt the now
deprecated `nonRoot` FG we rely on is
completely ignored so, as a workaround, we will
start internally translating nonRoot -> Root
on Kubevirt with negative logic.

In a future PR the nonRoot FG will be properly deprecated
also here and a new Root FG will
be introduced with a proper conversion logic on upgrades.

Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=2175171
JIRA-ticket: https://issues.redhat.com/browse/CNV-26406

Signed-off-by: stirabos <[email protected]>
kubevirt-bot pushed a commit to kubevirt/hyperconverged-cluster-operator that referenced this pull request Mar 5, 2023
@tiraboschi
Workaround for nonRoot FG on Kubevirt (#2273)
d4ac03c 
With kubevirt/kubevirt#8563
Kubevirt introduced a new FG named `Root`,
the `nonRoot` FG we are using is still there but
declared as deprecated.

Unfortunately due to bug on Kubevirt the now
deprecated `nonRoot` FG we rely on is
completely ignored so, as a workaround, we will
start internally translating nonRoot -> Root
on Kubevirt with negative logic.

In a future PR the nonRoot FG will be properly deprecated
also here and a new Root FG will
be introduced with a proper conversion logic on upgrades.

Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=2175171
JIRA-ticket: https://issues.redhat.com/browse/CNV-26406

Signed-off-by: stirabos <[email protected]>
@orelmisan orelmisan mentioned this pull request Oct 17, 2023
Merged
8 tasks
orelmisan added a commit to orelmisan/kubevirt that referenced this pull request Oct 17, 2023
@orelmisan
Remove unused NonRootExperimental feature gate
5ce20e1 
The NonRootExperimental feature gate was deprecated in
PR kubevirt#8563.

There is no code that is affected by this feature gate.

Signed-off-by: Orel Misan <[email protected]>
orelmisan added a commit to orelmisan/kubevirt that referenced this pull request Oct 18, 2023
@orelmisan
Remove leftover NonRootExperimental feature gate
a890070 
The NonRootExperimental feature gate was deprecated in
PR kubevirt#8563.

The presence or absence of this feature gate
does not change KubeVirt's behavior.

Signed-off-by: Orel Misan <[email protected]>
orelmisan added a commit to orelmisan/kubevirt that referenced this pull request Oct 23, 2023
@orelmisan
Remove leftover NonRoot feature gate
0987f00 
The `NonRoot` feature gate was deprecated in
PR kubevirt#8563.

The presence or absence of this feature gate
does not change KubeVirt's behavior.

Signed-off-by: Orel Misan <[email protected]>
This was referenced Oct 23, 2023
Merged
Open
awels pushed a commit to awels/kubevirt that referenced this pull request Oct 26, 2023
@orelmisan @awels
Remove leftover NonRootExperimental feature gate
966c6b6 
The NonRootExperimental feature gate was deprecated in
PR kubevirt#8563.

The presence or absence of this feature gate
does not change KubeVirt's behavior.

Signed-off-by: Orel Misan <[email protected]>
VirrageS pushed a commit to VirrageS/kubevirt that referenced this pull request Nov 17, 2023
@orelmisan @VirrageS
Remove leftover NonRootExperimental feature gate
d0307d0 
The NonRootExperimental feature gate was deprecated in
PR kubevirt#8563.

The presence or absence of this feature gate
does not change KubeVirt's behavior.

Signed-off-by: Orel Misan <[email protected]>
@xpivarc xpivarc mentioned this pull request Apr 24, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vladikr vladikr vladikr left review comments

@maiqueb maiqueb Awaiting requested review from maiqueb

@vasiliy-ul vasiliy-ul Awaiting requested review from vasiliy-ul

@kbidarkar kbidarkar Awaiting requested review from kbidarkar

@stu-gott stu-gott Awaiting requested review from stu-gott

Assignees

@acardace acardace

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@xpivarc @kubevirt-bot @vladikr @acardace @rmohr