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

implement kube alpha features for 1.22 #5207

Merged
merged 6 commits into from
Oct 28, 2021
Merged

implement kube alpha features for 1.22 #5207

merged 6 commits into from
Oct 28, 2021

Conversation

haircommander
Copy link
Member

What type of PR is this?

/kind feature

What this PR does / why we need it:

add support for swap and unified fields

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Add support for swap CRI field, allowing a container's memory swap limit to be configured by the Kubelet.
Add support for unified CRI field, allowing a container's cgroupv2 specific files to be configured by the Kubelet

@openshift-ci openshift-ci bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/feature Categorizes issue or PR as related to a new feature. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. labels Aug 16, 2021
@openshift-ci openshift-ci bot requested review from giuseppe and nalind August 16, 2021 18:52
@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 16, 2021
@haircommander haircommander added this to the 1.22 milestone Aug 16, 2021
@codecov
Copy link

codecov bot commented Aug 16, 2021 •
edited
Loading

Codecov Report

Merging #5207 (8f1daef) into main (7163dbb) will decrease coverage by 0.02%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main    #5207      +/-   ##
==========================================
- Coverage   43.49%   43.46%   -0.03%     
==========================================
  Files         118      118              
  Lines       11786    11797      +11     
==========================================
+ Hits         5126     5128       +2     
- Misses       6168     6177       +9     
  Partials      492      492

TomSweeneyRedHat
TomSweeneyRedHat reviewed Aug 17, 2021
View reviewed changes
server/cri/types/types.go Outdated
CPUsetMems string
HugepageLimits []*HugepageLimit
Unified map[string]string
MemorySwapLimitInBytes int64
Copy link
Contributor

Choose a reason for hiding this comment

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

Unless there's a reason not to, my OCD would appreciate ordering the variables in alphanumeric order.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It makes git annotate harder to use when attempting to determine when a field or feature was added?

@TomSweeneyRedHat
Copy link
Contributor

Tests need further help, changes LGTM in general, one OCD request for consideration.

@haircommander haircommander force-pushed the features-kube-1.22 branch 2 times, most recently from 332c3d9 to ee7c7c1 Compare August 18, 2021 19:00
@haircommander
Copy link
Member Author

rebased on #5204

@haircommander
Copy link
Member Author

/retest

@haircommander haircommander force-pushed the features-kube-1.22 branch 3 times, most recently from e4094b7 to 9070f11 Compare August 24, 2021 19:01
@haircommander haircommander removed this from the 1.22 milestone Aug 24, 2021
@openshift-ci openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 29, 2021
@mrunalp mrunalp mentioned this pull request Sep 6, 2021
Merged
1 task
@openshift-ci openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 24, 2021
@haircommander
Copy link
Member Author

/retest

@haircommander
Copy link
Member Author

/retest
@cri-o/cri-o-maintainers PTAL

@haircommander
Copy link
Member Author

/retest

@haircommander
Copy link
Member Author

@cri-o/cri-o-maintainers PTAL

@openshift-ci openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 18, 2021
@haircommander
Copy link
Member Author

/test integration_rhel

@haircommander
Copy link
Member Author

/retest

@haircommander
Copy link
Member Author

/test integration_rhel

2 similar comments
@haircommander
Copy link
Member Author

/test integration_rhel

@haircommander
Copy link
Member Author

/test integration_rhel

@haircommander
scripts: pin cri-tools version
518fceb 
Signed-off-by: Peter Hunt <pehunt@redhat.com>
@haircommander
test: bump cri-tools version
aca331d 
Signed-off-by: Peter Hunt <pehunt@redhat.com>
@haircommander
server/cri: add support for 1.22 features
9ab385d 
Signed-off-by: Peter Hunt <pehunt@redhat.com>
@haircommander
server: implement swap support
cd8bc4c 
Signed-off-by: Peter Hunt <pehunt@redhat.com>
@haircommander
server: add support for CRI unified field
f253acb 
Signed-off-by: Peter Hunt <pehunt@redhat.com>
@haircommander
test: drop swap disable playbook
8f1daef 
Signed-off-by: Peter Hunt <pehunt@redhat.com>
@haircommander
Copy link
Member Author

/retest

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 27, 2021

@haircommander: 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
ci/openshift-jenkins/integration_crun_cgroupv2 8f1daef link false /test integration_cgroupv2
ci/openshift-jenkins/e2e_crun_cgroupv2 8f1daef link false /test e2e_cgroupv2

Full PR test history. Your PR dashboard.

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.

@haircommander
Copy link
Member Author

@cri-o/cri-o-maintainers PTAL

saschagrunert
saschagrunert approved these changes Oct 28, 2021
View reviewed changes
Copy link
Member

@saschagrunert saschagrunert left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 28, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 28, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: haircommander, saschagrunert

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:
  • OWNERS [haircommander,saschagrunert]

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

@openshift-bot
Copy link

/retest-required

Please review the full test history for this PR and help us cut down flakes.

7 similar comments
@openshift-bot
Copy link

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot openshift-merge-robot merged commit a05be1c into cri-o:main Oct 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TomSweeneyRedHat TomSweeneyRedHat TomSweeneyRedHat left review comments

@saschagrunert saschagrunert saschagrunert approved these changes

@mrunalp mrunalp Awaiting requested review from mrunalp mrunalp is a code owner

@runcom runcom Awaiting requested review from runcom

@giuseppe giuseppe Awaiting requested review from giuseppe

@nalind nalind Awaiting requested review from nalind

Assignees

@saschagrunert saschagrunert

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. kind/feature Categorizes issue or PR as related to a new feature. 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.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@haircommander @TomSweeneyRedHat @openshift-bot @saschagrunert @nalind @ehashman @openshift-merge-robot