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
k8s: Fix unnecessary update with MetalLB #23210
Conversation
62c4674
to
46b56ef
Compare
46b56ef
to
54a5a71
Compare
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.
Looks good to me
/test Job 'Cilium-PR-K8s-1.24-kernel-5.4' failed: Click to show.Test Name
Failure Output
If it is a flake and a GitHub issue doesn't already exist to track it, comment |
Yeah, we've had quite a few CI issues at the end of last week/beginning of this week. Probably best to rebase for the CI to pass. |
54a5a71
to
45c9501
Compare
ConvertToK8sV1LoadBalancerIngress sets an empty ports array even if the correspoinding ports field is nil. Because of this, MetalLB updates svc.Status.LoadBalancer.Ingres even though it has already allocated an external IP. Metal detects a diff as follows, and updates the lb svc unnecessarily. v1.ServiceStatus{ LoadBalancer: v1.LoadBalancerStatus{ Ingress: []v1.LoadBalancerIngress{ { IP: "10.72.32.4", Hostname: "", - Ports: []v1.PortStatus{}, + Ports: nil, },},},Conditions: nil,} Fixes: cilium#23107 Signed-off-by: Yusuke Suzuki <yusuke-suzuki@cybozu.co.jp>
/test Job 'Cilium-PR-K8s-1.24-kernel-5.4' failed: Click to show.Test Name
Failure Output
If it is a flake and a GitHub issue doesn't already exist to track it, comment |
Cilium-PR-K8s-1.24-kernel-5.4' failed
The endpoint regeneration was canceled while cilium-agent was restoring an endpoint for kube-system/coredns-8c79ffd8b-jbl2s because it was being deleted. Another one coredns-8c79ffd8b-bkw2z was scheduled on another node k8s2. I don't see any problem with it.
Full Log
|
I don't think this change can affect |
This is noting to do with changes make here. I believe this would be flake #22578. Reviews are in, all tests are green otherwise, marking ready-to-merge. |
ConvertToK8sV1LoadBalancerIngress sets an empty ports array even if the correspoinding ports field is nil. Because of this, MetalLB updates svc.Status.LoadBalancer.Ingres even though it has already allocated an external IP.
Metal detects a diff as follows, and updates the lb svc unnecessarily.
Fixes: #23107
Signed-off-by: Yusuke Suzuki yusuke-suzuki@cybozu.co.jp
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.