-
Notifications
You must be signed in to change notification settings - Fork 83
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
spiderpool-agent: support to configure the sysctl config for node #3772
spiderpool-agent: support to configure the sysctl config for node #3772
Conversation
7b169b1
to
ba63403
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3772 +/- ##
==========================================
- Coverage 81.21% 81.16% -0.05%
==========================================
Files 50 50
Lines 4391 4391
==========================================
- Hits 3566 3564 -2
- Misses 669 670 +1
- Partials 156 157 +1
Flags with carried forward coverage won't be shown. Click here to find out more. |
ba63403
to
264826f
Compare
2eef7b8
to
f1b44f1
Compare
fdd4301
to
0375792
Compare
charts/spiderpool/values.yaml
Outdated
@@ -454,6 +454,9 @@ spiderpoolAgent: | |||
securityContext: {} | |||
# runAsUser: 0 | |||
|
|||
## @param spiderpoolAgent.sysctlConfigs the sysctl configs of spiderpoolAgent pod |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
我在想,我们是否不提供该接口,spiderpool 不负责 sysctl 的其他职责,只是确保自身正常工作
只需要提供一个 disable tune sysctl 的参数即可,避免我们的默认值 不符合预期。
其它的需求,用户自己去设置 主机的 sysctl 写配置文件
spiderpool 不承担 其它 sysctl 值的 调优职责,也避免 用户 通过该 接口 设置了一些 sysctl,把 spiderpool 的相关代码 跑挂了,我们是 退出好,还是 忽略好 ,都挺麻烦
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
我们可以保留这个接口,但不推荐用户使用此方式来来改任何 sysctl, 我们只是用它来做为我们自己的后门,方便以后要是出问题的时候,我们可以通过它快速配一下,避免需要重新编写代码
pkg/networking/sysctl/sysctl.go
Outdated
Value: "8192", | ||
}, | ||
{ | ||
Name: "net.ipv6.neigh.default.gc_thresh3", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
上一行 comment,什么内核 或 高内核 才有该 ipv6 的值。内核不存在该值 时,会自动忽略
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
没搜到内核版本支持?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
net.ipv6.neigh.default.gc_thresh3 (since Linux 2.2)
但是什么版本被拿掉了,要看下
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
就是没找到,内核代码没明显看出来
pkg/networking/sysctl/sysctl.go
Outdated
// DefaultSysctlConfig is the default sysctl config for the node | ||
var DefaultSysctlConfig = []struct { | ||
Name string | ||
Value string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
再加一些 字段 ?
requiredKernelVersion bool 一些低内核下,一些值是不存在的,不需要设置,忽略它们是否设置成功
setOnIpv4 : true Spiderpool 工作在 单双栈等场景下,只设置自己关心的值
setOnipv6: true
或者看其他项目是如何实践的
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
kube-proxy 会判断kernel 版本,这个有点复杂了,calico 会通过 是否启用 ipv6 来决定配置 ipv6 的参数
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这里如果需要判断 kernel 版本 和 是ipv4 或 ipv6,就有点繁琐了,这里应该是尽量去设置,忽略可接受的失败,否则如 ipv6-only 下还要去判断 ipv4 的 sysctl 不能去设置
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
spidepool helm 安装选项 中 有 是否启用 ipv4 还是 ipv6 的 环境变量 传递进来
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
有的
de11d60
to
8879bda
Compare
edc0d43
to
efacd86
Compare
2601752
to
32b8947
Compare
| `ipam.spidersubnet.enable` | SpiderSubnet feature. | `true` | | ||
| `ipam.spidersubnet.autoPool.enable` | SpiderSubnet Auto IPPool feature. | `true` | | ||
| `ipam.spidersubnet.autoPool.defaultRedundantIPNumber` | the default redundant IP number of SpiderSubnet feature auto-created IPPools | `1` | | ||
| `ipam.spiderSubnet.enable` | SpiderSubnet feature. | `true` | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
虽然是带货一些小错误,但要动 这些 字段,注意 doc 下一些 文档 要纠正
charts/spiderpool/README.md
Outdated
@@ -124,6 +124,7 @@ helm install spiderpool spiderpool/spiderpool --wait --namespace kube-system \ | |||
| `global.ipamUNIXSocketHostPath` | the host path of unix domain socket for ipam plugin | `/var/run/spidernet/spiderpool.sock` | | |||
| `global.configName` | the configmap name | `spiderpool-conf` | | |||
| `global.ciliumConfigMap` | the cilium's configMap, default is kube-system/cilium-config | `kube-system/cilium-config` | | |||
| `global.tuneSysctlConfig` | enable set specify sysctl configs for each node. | `false` | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个就是 spdieragent.tuneSysctlConfig 毕竟合适,它不生效到 其它组件
@@ -245,15 +245,11 @@ spec: | |||
{{- with .Values.spiderpoolAgent.extraEnv }} | |||
{{- toYaml . | nindent 8 }} | |||
{{- end }} | |||
{{- if or .Values.dra.enabled .Values.spiderpoolAgent.securityContext }} | |||
securityContext: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个默认是 强制的? 难免有一些 安全风险 和 背锅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
哦,这个应该要恢复
docs/reference/spiderpool-agent.md
Outdated
|
||
| sysctl config | value | description | | ||
| -------------| ------| ------------| | ||
| net.ipv4.neigh.default.gc_thresh3 | 8196 | This is the hard maximum number of entries to keep in the ARP cache. The garbage collector will always run if there are more than this number of entries in the cache. for ipv4 | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
8196 -> 20480
省得又不够
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
文档需要更新
02403bd
to
53dfba9
Compare
charts/spiderpool/values.yaml
Outdated
@@ -450,6 +450,9 @@ spiderpoolAgent: | |||
## @param spiderpoolAgent.resources.requests.memory the memory requests of spiderpoolAgent pod | |||
memory: 128Mi | |||
|
|||
## @param spiderpoolAgent.tuneSysctlConfig enable set specify sysctl configs for each node. | |||
tuneSysctlConfig: false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个默认是 true 把 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
我怕打开会覆盖客户的默认值?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
? tuneSysctlConfig: true
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这些事 underlay 必要条件,应该是默认的
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
charts/spiderpool/README.md
Outdated
@@ -282,6 +282,7 @@ helm install spiderpool spiderpool/spiderpool --wait --namespace kube-system \ | |||
| `spiderpoolAgent.resources.limits.memory` | the memory limit of spiderpoolAgent pod | `1024Mi` | | |||
| `spiderpoolAgent.resources.requests.cpu` | the cpu requests of spiderpoolAgent pod | `100m` | | |||
| `spiderpoolAgent.resources.requests.memory` | the memory requests of spiderpoolAgent pod | `128Mi` | | |||
| `spiderpoolAgent.tuneSysctlConfig` | enable set specify sysctl configs for each node. | `false` | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
enable to set required sysctl on each node to run spiderpool. refer to http:/.....doc... for details
@@ -75,6 +76,15 @@ func DaemonMain() { | |||
} | |||
logger.Sugar().Infof("Spiderpool-agent config: %+v", agentContext.Cfg) | |||
|
|||
// setup sysctls | |||
if agentContext.Cfg.TuneSysctlConfig { | |||
if err := sysctlConfig(agentContext.Cfg.EnableIPv4, agentContext.Cfg.EnableIPv6); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
作为 info 级别的日志,打开时,看不到一行 " set syctl " 日志
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个日志是有的
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if _, err := sysctl.Sysctl(sysConfig, value); err != nil { | ||
return err | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
需要一行 info 级别日志: sysctl 什么 key 设置了 什么值
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
53dfba9
to
31f52f7
Compare
8bae9e9
to
0ed157f
Compare
Signed-off-by: cyclinder <[email protected]>
0ed157f
to
8e64eee
Compare
spiderpool-agent: support to configure the sysctl config for node Signed-off-by: robot <[email protected]>
…pt_sysctl spiderpool-agent: support to configure the sysctl config for node
Thanks for contributing!
What type of PR is this?
What this PR does / why we need it:
support to configure the sysctl config for node
Which issue(s) this PR fixes:
Fixes #3587
Special notes for your reviewer: