Add a pod mutating webhook to auto inject the pod network resources #4179

cyclinder · 2024-10-17T11:01:55Z

Thanks for contributing!

Notice:

unite test or E2E test
do not forget essential code comment and log
document for the PR
release note label
"release/none"
"release/bug"
"release/feature"
read about Contribution notice: https://spidernet-io.github.io/spiderpool/latest/develop/contributing/

What issue(s) does this PR fix:

Fixes #4177

Special notes for your reviewer:

codecov · 2024-10-18T11:25:46Z

Codecov Report

Attention: Patch coverage is 54.96183% with 118 lines in your changes missing coverage. Please review.

Project coverage is 79.21%. Comparing base (b63d30e) to head (c997b6c).
Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/podmanager/utils.go	66.97%	65 Missing and 6 partials ⚠️
pkg/podmanager/pod_webhook.go	0.00%	47 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4179      +/-   ##
==========================================
- Coverage   80.34%   79.21%   -1.13%     
==========================================
  Files          51       52       +1     
  Lines        5637     5899     +262     
==========================================
+ Hits         4529     4673     +144     
- Misses        941     1053     +112     
- Partials      167      173       +6

Flag	Coverage Δ
unittests	`79.21% <54.96%> (-1.13%)`	⬇️

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

Files with missing lines	Coverage Δ
pkg/podmanager/pod_manager.go	`94.38% <ø> (ø)`
pkg/podmanager/pod_webhook.go	`0.00% <0.00%> (ø)`
pkg/podmanager/utils.go	`65.98% <66.97%> (+7.36%)`	⬆️

weizhoublue · 2024-10-22T02:37:45Z

docs/usage/install/ai/get-started-macvlan-zh_CN.md

@@ -214,7 +214,9 @@

 3. 创建 CNI 配置和对应的 ippool 资源


1 不建议修改保底方案，而是开一个小节单独说明新的方式
2 没看到 helm 安装要开开关

没看到 webhook 的 namespace 级别的安装或者配置选项？

希望把过滤 namespace 列表作为可配选项么？

肯定是需要的，支持白名单或黑名单，通过 helm value 和 configmap 暴露，可调

weizhoublue · 2024-10-22T02:45:14Z

pkg/constant/k8s.go

@@ -51,7 +51,8 @@ const (
 )

 const (
-	AnnotationPre = "ipam.spidernet.io"
+	AnnotationPre        = "ipam.spidernet.io"


key prefix 是否定义太多了，或者尽量复用
spidernet.io/...... 就够了，前缀是一个组织名，一个后缀足以区分出不同的功能

只是觉得 ipam.spidernet.io 不太恰当，就 spidernet.io 比较合适

其实 spidernet.io/**** 就完事了，基于目前已经发出去的版本，可以只保留两个

ipam.spidernet.io/***
cni.spidernet.io/***

以前老的保留，以后新的都统一为：spidernet.io

weizhoublue · 2024-10-22T02:45:46Z

pkg/constant/k8s.go

+
+	// webhook
+	PodMutatingWebhookName = "pods.spiderpool.spidernet.io"
+	AnnoMutatingPodWebhook = AnnotationSpinderPre + "/multus-config-label"


multus-config-label -》 resource-inject-key

weizhoublue · 2024-10-22T03:01:45Z

docs/usage/install/ai/get-started-macvlan-zh_CN.md

    ---
    apiVersion: spiderpool.spidernet.io/v2beta1
    kind: SpiderMultusConfig
    metadata:
      name: gpu1-macvlan
      namespace: spiderpool
+      labels:
+        shared-rdma-gpu: ""


这个 label 不应该是这么打的。从其它项目的实践来说，它应该是

pod 打
netresources.spidernet.io/resource-inject-key: myKeyName

SpiderMultusConfig 打
netresources.spidernet.io/resource-inject-key: myKeyName

为什么呢？应该双方应该都应该用 netresources.spidernet.io/resource-inject-key 作为key 名，这样这不至于和其它项目相互污染，也才看那名字知道这个 annotation 是什么用途

weizhoublue · 2024-10-22T03:15:57Z

docs/usage/install/ai/get-started-macvlan-zh_CN.md

-            spidernet.io/shared_cx5_gpu7: 1
-            spidernet.io/shared_cx5_gpu8: 1
-            #nvidia.com/gpu: 1
+      netresources.spidernet.io/multus-config-label: shared-rdma-gpu


可以考虑2 个

netresources.spidernet.io/resource-inject-key: myKeyName
netresources.spidernet.io/resource-inject-amount: 1 （default，默认可不写）

resource-inject-amount: 1 是网卡数量还是 resources 的值？感觉是后者，但似乎没必要？什么场景会有大于 1 的需求？

是否是之前谈到的 gpu 和 nic 亲和性的实现时可能需要

cc @weizhoublue

weizhoublue · 2024-10-30T01:33:45Z

如果 multusconfig 配置中没有 rdma resource，是否也能正常工作？

Signed-off-by: cyclinder <[email protected]>

weizhoublue · 2024-10-30T09:44:01Z

docs/usage/install/ai/get-started-sriov-zh_CN.md

+        spidernet.io/gpu6sriov: 1
+        spidernet.io/gpu7sriov: 1
+        spidernet.io/gpu8sriov: 1
+        #nvidia.com/gpu: 1


这行不应该出现

以后PR 改

cyclinder requested a review from weizhoublue as a code owner October 17, 2024 11:01

cyclinder marked this pull request as draft October 17, 2024 11:02

cyclinder force-pushed the webhook/auto_smc branch from bd40d6a to 5eb5a2a Compare October 18, 2024 11:22

cyclinder marked this pull request as ready for review October 18, 2024 11:31

cyclinder requested review from ty-dc, bzsuni and windsonsea as code owners October 18, 2024 11:32

cyclinder force-pushed the webhook/auto_smc branch 2 times, most recently from f0c5d90 to 5effe3c Compare October 21, 2024 07:30

cyclinder temporarily deployed to release-base-images October 21, 2024 07:30 — with GitHub Actions Inactive

cyclinder force-pushed the webhook/auto_smc branch from 5effe3c to b421dd1 Compare October 21, 2024 08:55

cyclinder temporarily deployed to release-base-images October 21, 2024 08:56 — with GitHub Actions Inactive

cyclinder force-pushed the webhook/auto_smc branch from b421dd1 to c1bcc2d Compare October 21, 2024 12:43

cyclinder temporarily deployed to release-base-images October 21, 2024 12:43 — with GitHub Actions Inactive

cyclinder force-pushed the webhook/auto_smc branch from c1bcc2d to a4ca744 Compare October 21, 2024 12:44

cyclinder temporarily deployed to release-base-images October 21, 2024 12:44 — with GitHub Actions Inactive

cyclinder force-pushed the webhook/auto_smc branch from a4ca744 to d5119ca Compare October 21, 2024 14:37

cyclinder temporarily deployed to release-base-images October 21, 2024 14:37 — with GitHub Actions Inactive

cyclinder added the release/feature-new release note for new feature label Oct 22, 2024

weizhoublue reviewed Oct 22, 2024

View reviewed changes

cyclinder force-pushed the webhook/auto_smc branch from d5119ca to 44052c3 Compare October 22, 2024 06:02

cyclinder temporarily deployed to release-base-images October 22, 2024 06:02 — with GitHub Actions Inactive

cyclinder force-pushed the webhook/auto_smc branch from 44052c3 to b1dd917 Compare October 22, 2024 06:06

cyclinder temporarily deployed to release-base-images October 22, 2024 06:06 — with GitHub Actions Inactive

cyclinder force-pushed the webhook/auto_smc branch from b1dd917 to 2781a85 Compare October 22, 2024 06:38

cyclinder temporarily deployed to release-base-images October 29, 2024 12:37 — with GitHub Actions Inactive

cyclinder force-pushed the webhook/auto_smc branch from 52fa66c to 575918c Compare October 29, 2024 12:55

cyclinder temporarily deployed to release-base-images October 29, 2024 12:55 — with GitHub Actions Inactive

cyclinder force-pushed the webhook/auto_smc branch from 575918c to 31ecc38 Compare October 29, 2024 13:55

cyclinder temporarily deployed to release-base-images October 29, 2024 13:55 — with GitHub Actions Inactive

cyclinder force-pushed the webhook/auto_smc branch from 31ecc38 to 07e167f Compare October 30, 2024 03:13

cyclinder temporarily deployed to release-base-images October 30, 2024 03:13 — with GitHub Actions Inactive

cyclinder force-pushed the webhook/auto_smc branch from 07e167f to da165ea Compare October 30, 2024 03:24

cyclinder temporarily deployed to release-base-images October 30, 2024 03:24 — with GitHub Actions Inactive

cyclinder force-pushed the webhook/auto_smc branch from da165ea to 1b7ab43 Compare October 30, 2024 03:35

cyclinder temporarily deployed to release-base-images October 30, 2024 03:35 — with GitHub Actions Inactive

cyclinder force-pushed the webhook/auto_smc branch from 1b7ab43 to 59f02fd Compare October 30, 2024 03:49

cyclinder temporarily deployed to release-base-images October 30, 2024 03:49 — with GitHub Actions Inactive

cyclinder force-pushed the webhook/auto_smc branch from 59f02fd to d1e6936 Compare October 30, 2024 03:52

cyclinder temporarily deployed to release-base-images October 30, 2024 03:52 — with GitHub Actions Inactive

cyclinder force-pushed the webhook/auto_smc branch from d1e6936 to 425bde6 Compare October 30, 2024 04:27

cyclinder temporarily deployed to release-base-images October 30, 2024 04:27 — with GitHub Actions Inactive

cyclinder force-pushed the webhook/auto_smc branch from 425bde6 to 394c80c Compare October 30, 2024 04:32

cyclinder temporarily deployed to release-base-images October 30, 2024 04:32 — with GitHub Actions Inactive

cyclinder force-pushed the webhook/auto_smc branch from 394c80c to c183d54 Compare October 30, 2024 06:08

cyclinder temporarily deployed to release-base-images October 30, 2024 06:13 — with GitHub Actions Inactive

Add a pod mutating webhook to auto inject the pod network resources

c997b6c

Signed-off-by: cyclinder <[email protected]>

cyclinder force-pushed the webhook/auto_smc branch from c183d54 to c997b6c Compare October 30, 2024 08:38

cyclinder temporarily deployed to release-base-images October 30, 2024 08:38 — with GitHub Actions Inactive

weizhoublue reviewed Oct 30, 2024

View reviewed changes

weizhoublue approved these changes Oct 30, 2024

View reviewed changes

ty-dc approved these changes Oct 30, 2024

View reviewed changes

cyclinder merged commit 0a14405 into spidernet-io:main Oct 30, 2024
61 of 62 checks passed

cyclinder deleted the webhook/auto_smc branch October 30, 2024 09:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a pod mutating webhook to auto inject the pod network resources #4179

Add a pod mutating webhook to auto inject the pod network resources #4179

cyclinder commented Oct 17, 2024 •

edited

Loading

codecov bot commented Oct 18, 2024 •

edited

Loading

weizhoublue Oct 22, 2024 •

edited

Loading

weizhoublue Oct 24, 2024

cyclinder Oct 24, 2024

weizhoublue Oct 25, 2024

weizhoublue Oct 22, 2024 •

edited

Loading

cyclinder Oct 22, 2024

weizhoublue Oct 24, 2024

cyclinder Oct 24, 2024

weizhoublue Oct 22, 2024

cyclinder Oct 22, 2024

weizhoublue Oct 22, 2024

weizhoublue Oct 22, 2024 •

edited

Loading

cyclinder Oct 22, 2024

cyclinder Oct 22, 2024

cyclinder Oct 24, 2024

weizhoublue commented Oct 30, 2024

weizhoublue Oct 30, 2024

cyclinder Oct 30, 2024

		@@ -214,7 +214,9 @@

		3. 创建 CNI 配置和对应的 ippool 资源

Add a pod mutating webhook to auto inject the pod network resources #4179

Add a pod mutating webhook to auto inject the pod network resources #4179

Conversation

cyclinder commented Oct 17, 2024 • edited Loading

Thanks for contributing!

codecov bot commented Oct 18, 2024 • edited Loading

Codecov Report

weizhoublue Oct 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

weizhoublue Oct 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

weizhoublue Oct 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

weizhoublue commented Oct 30, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cyclinder commented Oct 17, 2024 •

edited

Loading

codecov bot commented Oct 18, 2024 •

edited

Loading

weizhoublue Oct 22, 2024 •

edited

Loading

weizhoublue Oct 22, 2024 •

edited

Loading

weizhoublue Oct 22, 2024 •

edited

Loading