api: update enableRdma/rdmaIsolation field for spiderMultusConfig #4301
Conversation
cyclinder
requested review from
weizhoublue,
ty-dc,
bzsuni and
windsonsea
as code owners
cyclinder
force-pushed
the
spidermultusconfig/api_update
branch
3 times, most recently
from
1e0f7cd
to
4dee59d
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4301 +/- ##
=======================================
Coverage 79.55% 79.56%
=======================================
Files 53 53
Lines 6252 6255 +3
=======================================
+ Hits 4974 4977 +3
Misses 1085 1085
Partials 193 193
Flags with carried forward coverage won't be shown. Click here to find out more.
|
cyclinder
force-pushed
the
spidermultusconfig/api_update
branch
from
4dee59d
to
fd7b567
Compare
| @@ -229,10 +246,6 @@ spec: | |||
| - mode | |||
| - name | |||
| type: object | |||
| enableRdma: | |||
There was a problem hiding this comment.
这个 是 破坏性 的 版本 变更，只能说 deprecated ？
There was a problem hiding this comment.
我也是考虑了一下，目前没有任何地方在引用这个代码，所以感觉可以直接移除？
There was a problem hiding this comment.
不是代码引用的问题。
升级 流程上 是否能 会问题，存量实例 的转换 怎么办
There was a problem hiding this comment.
升级指的是老 CRD 用新镜像，因为新镜像不会用到 enableRdma 这个字段，所以不会影响升级。手动测试过都是正常的，我也是考虑到没地方引用这个字段，所以想一步到位
There was a problem hiding this comment.
那不现实的，只换镜像，那不叫升级，不处理 CRD 的升级 和转换。那未来 任何新功能，crd 新的定义，存量环境都享受不到，结果它们升级的意义 只是为了 修复 bug
There was a problem hiding this comment.
我的意思是即使未升级 CRD只升级镜像测试都没问题，正常升级流程CRD和镜像都要升级，所以更没问题, crd 升级后，crs 中该字段自动移除
There was a problem hiding this comment.
是否要一步到位, 你决定就好，我都OK @weizhoublue
There was a problem hiding this comment.
我的意思是即使未升级 CRD只升级镜像测试都没问题--- 不是一个升级服务的流程，不考虑
正常升级流程CRD和镜像都要升级 --- crd 升级后 删除字段，相关实例 是否能正常，是否有验证
| @@ -280,10 +303,6 @@ spec: | |||
| - mode | |||
| - name | |||
| type: object | |||
| enableRdma: | |||
Signed-off-by: Cyclinder Kuo <[email protected]>
cyclinder
force-pushed
the
spidermultusconfig/api_update
branch
from
fd7b567
to
6ea762a
Compare
Thanks for contributing!
Notice:
"release/none"
"release/bug"
"release/feature"
What issue(s) does this PR fix:
Fixes #4267
Special notes for your reviewer:
Remove enableRdma field for macvlan and ipvlan CNI
Make the enableRdma field DEPRECATED for sriov-cni, and use rdmaIsolation instead. which keep consistant with ibsriov-cni.
Update the comments of the spiderMultusConfig fields and docs.