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

feat(qrm): introduce network QRM plugin and support packet tagging #14

Merged
merged 5 commits into from
Apr 25, 2023
Merged

feat(qrm): introduce network QRM plugin and support packet tagging #14

merged 5 commits into from
Apr 25, 2023

Conversation

caohe
Copy link
Member

@caohe caohe commented Apr 13, 2023

What type of PR is this?

Features

What this PR does / why we need it:

  • Introduce a network QRM plugin with the following capabilities:
    • Packet tagging
      • In cgroups v1 environments, configure the net_cls cgroup through the Cgroup Manager to enable outbound traffic tagging for containers.
      • In cgroups v2 environments, use eBPF through the External Manager to support traffic tagging.
    • Network bandwidth control (will be implemented in future PRs)
  • Refactor external manager to support multiple operating systems.

Which issue(s) this PR fixes:

Fix #5

Special notes for your reviewer:

Related PR: kubewharf/katalyst-api#2

@caohe caohe added the enhancement New feature or request label Apr 13, 2023
@caohe caohe added this to the v0.2 milestone Apr 13, 2023
@caohe caohe requested review from csfldf and waynepeking348 April 13, 2023 09:37
@caohe caohe marked this pull request as draft April 13, 2023 09:37
@caohe caohe mentioned this pull request Apr 13, 2023
Open
9 tasks
@codecov
Copy link

codecov bot commented Apr 13, 2023 •
edited
Loading

Codecov Report

Patch coverage: 55.68% and project coverage change: -0.18 ⚠️

Comparison is base (f3ce63d) 50.92% compared to head (4912069) 50.74%.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main      #14      +/-   ##
==========================================
- Coverage   50.92%   50.74%   -0.18%     
==========================================
  Files         311      315       +4     
  Lines       31785    32073     +288     
==========================================
+ Hits        16185    16274      +89     
- Misses      13682    13876     +194     
- Partials     1918     1923       +5
Flag Coverage Δ
unittest 50.74% <55.68%> (-0.18%) ⬇️

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

Impacted Files Coverage Δ
...r/plugin/qosaware/reporter/manager/resource/cpu.go 100.00% <ø> (ø)
...lugin/qosaware/reporter/manager/resource/memory.go 100.00% <ø> (ø)
.../sysadvisor/qosaware/reporter/headroom_reporter.go 100.00% <ø> (ø)
cmd/katalyst-agent/app/options/qrm/qrm_base.go 46.51% <42.85%> (-3.49%) ⬇️
.../agent/qrm-plugins/network/dynamicpolicy/policy.go 43.63% <43.63%> (ø)
...d/katalyst-agent/app/options/qrm/network_plugin.go 48.48% <48.48%> (ø)
.../sysadvisor/qosaware/reporter/headroom_reporter.go 70.83% <66.66%> (ø)
pkg/util/qos/net_enhancement.go 72.72% <72.72%> (ø)
pkg/metaserver/external/cgroupid/manager_linux.go 61.44% <75.00%> (ø)
pkg/util/cgroup/common/path.go 85.41% <83.33%> (+6.84%) ⬆️
... and 7 more

... and 11 files with indirect coverage changes

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 in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@caohe caohe force-pushed the network-qrm-plugin branch from bb224ee to 6adb46e Compare April 19, 2023 09:14
@caohe caohe added the workflow/draft draft: no need to review label Apr 20, 2023
caohe
caohe commented Apr 21, 2023
View reviewed changes
pkg/config/agent/qrm/network_plugin.go
type NetworkQRMPluginConfig struct {
// PolicyName is used to switch between several strategies
PolicyName string
NetClass NetClassConfig
Copy link
Member Author

Choose a reason for hiding this comment

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

The filed NetClass is a dynamic configurable field, currently located in the config/qrm directory. cc @luomingmeng

@caohe caohe force-pushed the network-qrm-plugin branch 2 times, most recently from 185b5ad to 98c428a Compare April 21, 2023 08:32
@caohe caohe added workflow/need-review review: test succeeded, need to review workflow/merge-hold merge-hold: code is ready but still has dependency and removed workflow/draft draft: no need to review labels Apr 21, 2023
@caohe caohe marked this pull request as ready for review April 21, 2023 08:32
@caohe
Copy link
Member Author

caohe commented Apr 21, 2023

@csfldf @waynepeking348 PTAL. I will add more unit tests on Sunday.

@caohe caohe force-pushed the network-qrm-plugin branch from 98c428a to ebba5ab Compare April 21, 2023 11:50
@waynepeking348 waynepeking348 mentioned this pull request Apr 23, 2023
Merged
waynepeking348
waynepeking348 reviewed Apr 23, 2023
View reviewed changes
go.mod Outdated Show resolved Hide resolved
pkg/util/external/rdt/manager.go Show resolved Hide resolved
pkg/util/qos/net_enhancement.go Outdated Show resolved Hide resolved
pkg/metaserver/external/manager.go Outdated Show resolved Hide resolved
pkg/metaserver/external/manager_unsupported.go Show resolved Hide resolved
pkg/agent/qrm-plugins/network/dynamicpolicy/policy.go Outdated Show resolved Hide resolved
pkg/agent/qrm-plugins/network/dynamicpolicy/policy.go Outdated Show resolved Hide resolved
pkg/agent/qrm-plugins/network/dynamicpolicy/policy.go Show resolved Hide resolved
pkg/agent/qrm-plugins/network/dynamicpolicy/policy.go Show resolved Hide resolved
@caohe caohe force-pushed the network-qrm-plugin branch from ebba5ab to 710b808 Compare April 23, 2023 07:41
@waynepeking348
Copy link
Collaborator

add some comments here, need to be resolved before approve

@caohe caohe force-pushed the network-qrm-plugin branch 2 times, most recently from f1b7f7e to 0296e85 Compare April 24, 2023 02:42
@caohe
Copy link
Member Author

caohe commented Apr 24, 2023

@csfldf @waynepeking348 I added some unit tests, and the business code didn't change. Could u PTAL again?

@waynepeking348
Copy link
Collaborator

@csfldf @waynepeking348 I added some unit tests, and the business code didn't change. Could u PTAL again?

sure, but we're now pending on this pr kubewharf/katalyst-api#6 before this one can be merged

caohe added 2 commits April 24, 2023 10:54
@caohe
fix(external-manager): refactor external manager to support multiple …
02fcde1 
…systems

Signed-off-by: caohe <caohe9603@gmail.com>
@caohe
feat(qrm): introduce a network qrm plugin
9ec1d3d 
Signed-off-by: caohe <caohe9603@gmail.com>
@caohe caohe force-pushed the network-qrm-plugin branch from 0296e85 to 5d16393 Compare April 24, 2023 02:54
@caohe caohe force-pushed the network-qrm-plugin branch from 5d16393 to 6821446 Compare April 24, 2023 03:17
@caohe
Copy link
Member Author

caohe commented Apr 24, 2023

/hold

caohe added 2 commits April 24, 2023 11:30
@caohe
feat(qrm): implement packet tagging
2b8de15 
Signed-off-by: caohe <caohe9603@gmail.com>
@caohe
test(qrm): add tests for network qrm plugin
e6d399c 
Signed-off-by: caohe <caohe9603@gmail.com>
@caohe caohe force-pushed the network-qrm-plugin branch 2 times, most recently from 727c207 to 68743bd Compare April 24, 2023 04:06
@caohe caohe added workflow/merge-ready merge-ready: code is ready and can be merged and removed workflow/merge-hold merge-hold: code is ready but still has dependency labels Apr 24, 2023
@caohe caohe requested review from csfldf and waynepeking348 April 24, 2023 04:25
@caohe caohe force-pushed the network-qrm-plugin branch from 68743bd to cc45b83 Compare April 24, 2023 06:22
@caohe
test(qrm): add tests for network qrm plugin
4912069 
Signed-off-by: caohe <caohe9603@gmail.com>
@caohe caohe force-pushed the network-qrm-plugin branch from cc45b83 to 4912069 Compare April 24, 2023 06:39
@waynepeking348 waynepeking348 merged commit 438dc9c into kubewharf:main Apr 25, 2023
@caohe caohe deleted the network-qrm-plugin branch April 25, 2023 06:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@waynepeking348 waynepeking348 waynepeking348 approved these changes

@csfldf csfldf csfldf approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request workflow/merge-ready merge-ready: code is ready and can be merged workflow/need-review review: test succeeded, need to review
Projects
Katalyst Development Dashboard
Status: ✅ Done
Milestone
v0.2
3 participants
@caohe @waynepeking348 @csfldf