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

feature: support infiniband with ib-sriov and ipoib cni #2815

Merged
merged 35 commits into from
Dec 14, 2023
Merged

Conversation

weizhoublue
Copy link
Collaborator

@weizhoublue weizhoublue commented Dec 11, 2023

Copy link

codecov bot commented Dec 11, 2023

Codecov Report

Merging #2815 (48022ca) into main (510b670) will decrease coverage by 0.51%.
Report is 4 commits behind head on main.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2815      +/-   ##
==========================================
- Coverage   81.60%   81.09%   -0.51%     
==========================================
  Files          49       49              
  Lines        5333     5333              
==========================================
- Hits         4352     4325      -27     
- Misses        821      852      +31     
+ Partials      160      156       -4     
Flag Coverage Δ
unittests 81.09% <ø> (-0.51%) ⬇️

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

see 1 file with indirect coverage changes

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/utils/pointer"
"os"
Copy link
Collaborator

Choose a reason for hiding this comment

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

sort 一下 imports

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

IDE 自动排的,save 就这样

Copy link
Collaborator

Choose a reason for hiding this comment

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

goland 的自动排序没 vscode 好使...

namespace: kube-system
spec:
cniType: ib-sriov
sriov:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
sriov:
ibsriov:

@@ -28,7 +28,7 @@ type SpiderMultusConfigList struct {
// MultusCNIConfigSpec defines the desired state of SpiderMultusConfig.
type MultusCNIConfigSpec struct {
// +kubebuilder:validation:Required
// +kubebuilder:validation:Enum=macvlan;ipvlan;sriov;ovs;custom
// +kubebuilder:validation:Enum=macvlan;ipvlan;sriov;ovs;ib-sriov;custom
Copy link
Collaborator

Choose a reason for hiding this comment

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

这里是 ib-sriov, 47行 json tag 是 ibsriov, 不一致

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

cni bin 叫 ib-sriov, 为了 CNItype 和 最终multus 中的 type 等一致,golang中避免2个变量,这里一致 引用 ib-sriov
而 spec.ibsriov 又不合适为 ib-sriov

type IBSRIOVNetConf struct {
Type string `json:"type"`
Pkey *string `json:"pkey,omitempty"`
LinkState *string `json:"link_state,omitempty"`
Copy link
Collaborator

Choose a reason for hiding this comment

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

json tag 统一下,都为 驼峰法

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

这个是 CNI 最终的 config,没法改

Copy link
Collaborator

Choose a reason for hiding this comment

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

他们官方怎么又又下划线,又有驼峰

@weizhoublue weizhoublue added pr/not-ready not ready for merging release/feature-new release note for new feature labels Dec 12, 2023
@weizhoublue weizhoublue changed the title suport ib-sriov cni new feature: suport infiniband with ib-sriov and ipoib cni Dec 12, 2023
@weizhoublue weizhoublue changed the title new feature: suport infiniband with ib-sriov and ipoib cni feature: support infiniband with ib-sriov and ipoib cni Dec 12, 2023
@weizhoublue weizhoublue removed the pr/not-ready not ready for merging label Dec 12, 2023
@weizhoublue weizhoublue temporarily deployed to release-base-images December 12, 2023 13:01 — with GitHub Actions Inactive
@weizhoublue weizhoublue temporarily deployed to release-base-images December 12, 2023 13:43 — with GitHub Actions Inactive
@weizhoublue weizhoublue temporarily deployed to release-base-images December 14, 2023 06:16 — with GitHub Actions Inactive
@weizhoublue weizhoublue temporarily deployed to release-base-images December 14, 2023 07:21 — with GitHub Actions Inactive
@weizhoublue weizhoublue temporarily deployed to release-base-images December 14, 2023 08:00 — with GitHub Actions Inactive

2. 基于 [IPoIB CNI](https://github.com/Mellanox/ipoib-cni) 给 POD 提供 IPoIB 的网卡,它并不提供 RDMA 网卡通信能力,适用于需要 TCP/IP 通信的常规应用,因为它不需要提供 SRIOV 资源,因此能让主机上运行更多 POD

另外,在 RDMA 通信场景下,对于基于 clusterIP 进行通信的应用,为了让 RDMA 流量通过 underlay 网卡转发,可在容器网络命名空间内基于 cgroup eBPF 实现 clusterIP 解析,具体可参考 [cgroup eBPF 解析 clusterIP](./underlay_cni_service-zh_CN.md)
Copy link
Collaborator

Choose a reason for hiding this comment

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

所以这种模式下,不需要 coordinator?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

coordinator 依然可解决 POD 健康检查 等问题

name: ib-sriov
namespace: kube-system
spec:
cniType: ib-sriov
Copy link
Collaborator

Choose a reason for hiding this comment

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

这种模式下仍然需要 coordinator?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

存在 不妨碍 ,可保底

@@ -43,6 +43,12 @@ type MultusCNIConfigSpec struct {
// +kubebuilder:validation:Optional
OvsConfig *SpiderOvsCniConfig `json:"ovs,omitempty"`

// +kubebuilder:validation:Optional
IbSriovConfig *SpiderIBSriovCniConfig `json:"ibsriov,omitempty"`
Copy link
Collaborator

Choose a reason for hiding this comment

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

为了与上面保持统一,tag 改为 ib-sriov? 更和谐一些,golang 本身也支持 "-"

https://stackoverflow.com/questions/57478256/how-to-unmarshal-a-json-string-with-a-hyphen-in-key-to-a-struct

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

没有项目 的 spec 中的 key 是带 - 号的 把 ,非常奇怪

type IBSRIOVNetConf struct {
Type string `json:"type"`
Pkey *string `json:"pkey,omitempty"`
LinkState *string `json:"link_state,omitempty"`
Copy link
Collaborator

Choose a reason for hiding this comment

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

他们官方怎么又又下划线,又有驼峰

@weizhoublue weizhoublue merged commit 00156ed into main Dec 14, 2023
41 checks passed
@weizhoublue weizhoublue deleted the pr/welan/asdf branch December 14, 2023 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release/feature-new release note for new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants