-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Expose bpf-lb-sock-hostns-only in cilium status #24570
Expose bpf-lb-sock-hostns-only in cilium status #24570
Conversation
It seems the CI is failing due the changes made into this PR https://github.com/cilium/cilium/actions/runs/4526983771/jobs/7972485978?pr=24570 |
3f6ec94
to
09dec34
Compare
@@ -43,6 +43,9 @@ type KubeProxyReplacement struct { | |||
// mode | |||
// Enum: [Disabled Strict Probe Partial] | |||
Mode string `json:"mode,omitempty"` | |||
|
|||
// flag bpf-lb-sock-hostns-only | |||
BPFSocketLBHostnsOnly bool `json:"bpfSocketLBHostnsOnly,omitempty"` |
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.
This should go under KubeProxyReplacementFeatures
(see above L 41). Please take a look at how other flags are exposed.
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.
Thanks. I've added the flag to the openapi
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.
This is still not addressed. You've added the flag under KubeProxyReplacement
, and not KubeProxyReplacementFeatures
. Was there an issue in adding it under the latter?
09dec34
to
06f79f6
Compare
Commit 06f79f622a1383ed41adf14a2149625716bf3885 does not contain "Signed-off-by". Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin |
06f79f6
to
b66913f
Compare
Commit b66913ff24048ebc0d24fb2c8637d2658a1c4b8b does not contain "Signed-off-by". Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin |
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.
Thanks for your contribution. LGTM... This is ready for merge once you add "Signed-off-by:" in the commit message.
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.
b66913f
to
10565d2
Compare
10565d2
to
9ba0d85
Compare
BPFSocketLBHostnsOnly is outputted in the Kube Proxy Replacement section in cilium status --verbose: $ cilium status --verbose [...] KubeProxyReplacement Details: [...] Socket LB Coverage: Hostns-only [...] Fixes: cilium#24160 Signed-off-by: Roman Ptitcyn <romanspb@yahoo.com>
9ba0d85
to
98a2046
Compare
I missed it (((. |
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.
Thanks!
Integration test failure is same as - #24653.
The PR is ready to merge.
BPFSocketLBHostnsOnly is outputted in the Kube Proxy Replacement section in cilium status --verbose:
$ cilium status --verbose
[...]
KubeProxyReplacement Details:
[...]
BPFSocketLBHostnsOnly: Enabled
[...]
Fixes: #24160
Please ensure your pull request adheres to the following guidelines:
description and a
Fixes: #XXX
line if the commit addresses a particularGitHub issue.
Fixes: <commit-id>
tag, thenplease add the commit author[s] as reviewer[s] to this issue.
Fixes: #issue-number