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

docs: proposal koordlet support cgroups v2 #720

Conversation

saintube
Copy link
Member

Signed-off-by: saintube saintube@foxmail.com

Ⅰ. Describe what this PR does

Add a proposal that clarifies how to support cgroups v2 in the koordlet.

Ⅱ. Does this pull request fix one issue?

Ⅲ. Describe how to verify it

Ⅳ. Special notes for reviews

V. Checklist

  • I have written necessary docs and comments
  • I have added necessary unit tests and integration tests
  • All checks passed in make test

Copy link
Member

@FillZpp FillZpp left a comment

Choose a reason for hiding this comment

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

A few nits, and we shouldn't change any code in a proposal PR.

@saintube saintube force-pushed the propose-koordlet-support-cgroups-v2 branch from 62f129b to 515cc3e Compare October 20, 2022 07:50
@koordinator-bot koordinator-bot bot added size/XL and removed size/L labels Oct 20, 2022
@codecov
Copy link

codecov bot commented Oct 20, 2022

Codecov Report

Base: 68.15% // Head: 68.15% // No change to project coverage 👍

Coverage data is based on head (00e3c4d) compared to base (f653e7e).
Patch has no changes to coverable lines.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #720   +/-   ##
=======================================
  Coverage   68.15%   68.15%           
=======================================
  Files         210      210           
  Lines       24102    24102           
=======================================
  Hits        16427    16427           
- Misses       6535     6536    +1     
+ Partials     1140     1139    -1     
Flag Coverage Δ
unittests 68.15% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pkg/koordlet/statesinformer/states_pods.go 54.60% <0.00%> (-2.13%) ⬇️
pkg/util/httputil/reverseproxy.go 85.10% <0.00%> (+0.79%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@saintube saintube changed the title WIP: docs: proposal koordlet support cgroups v2 docs: proposal koordlet support cgroups v2 Oct 20, 2022
@saintube
Copy link
Member Author

A few nits, and we shouldn't change any code in a proposal PR.

OK. I will split this PR.

@saintube saintube force-pushed the propose-koordlet-support-cgroups-v2 branch from 515cc3e to e26f9c1 Compare October 20, 2022 08:54
@saintube saintube requested review from FillZpp and removed request for jasonliu747 October 20, 2022 08:56
@saintube
Copy link
Member Author

@saintube saintube force-pushed the propose-koordlet-support-cgroups-v2 branch 2 times, most recently from 2c2005d to 9fe457c Compare October 27, 2022 09:33
@saintube saintube requested review from zwzhang0107 and removed request for FillZpp, eahydra, jasonliu747 and zwzhang0107 October 27, 2022 09:41
@saintube saintube requested review from FillZpp and zwzhang0107 and removed request for hormes and FillZpp November 3, 2022 04:34
Copy link
Member

@jasonliu747 jasonliu747 left a comment

Choose a reason for hiding this comment

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

/lgtm

@saintube
Copy link
Member Author

saintube commented Nov 7, 2022

/cc @stormgbs

@saintube saintube force-pushed the propose-koordlet-support-cgroups-v2 branch from 6819185 to 491df9b Compare November 8, 2022 03:18
@koordinator-bot koordinator-bot bot removed the lgtm label Nov 8, 2022
@saintube saintube force-pushed the propose-koordlet-support-cgroups-v2 branch from 491df9b to cb791db Compare November 8, 2022 09:07
@koordinator-bot koordinator-bot bot added size/XXL and removed size/XL labels Nov 8, 2022
@saintube saintube force-pushed the propose-koordlet-support-cgroups-v2 branch 2 times, most recently from 2929106 to b2071b5 Compare November 8, 2022 09:27
Signed-off-by: saintube <saintube@foxmail.com>
@saintube saintube force-pushed the propose-koordlet-support-cgroups-v2 branch from b2071b5 to 00e3c4d Compare November 8, 2022 11:59
@zwzhang0107
Copy link
Contributor

/lgtm

@koordinator-bot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by:

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

@koordinator-bot koordinator-bot bot merged commit c31e4a3 into koordinator-sh:main Nov 9, 2022
The `IsSupported()` method is defined for checking the compatibility of system resources when trying to modify the files.

```go
type ResourceType string
Copy link
Member

Choose a reason for hiding this comment

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

(FYI) I think ResourceName maybe a better choice :)

@saintube saintube mentioned this pull request Nov 24, 2022
3 tasks
@saintube saintube mentioned this pull request Dec 6, 2022
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants