🌱 Improve CAPD load balancer #11430
🌱 Improve CAPD load balancer #11430
Conversation
k8s-ci-robot
added
do-not-merge/work-in-progress
k8s-ci-robot
added
the
size/L
|
@fabriziopandini: The
The following commands are available to trigger optional jobs:
Use
In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/test ? |
|
@fabriziopandini: The following commands are available to trigger required jobs:
The following commands are available to trigger optional jobs:
Use
In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/test pull-cluster-api-e2e-main |
fabriziopandini
force-pushed
the
improve-capd-loadbalancer
branch
from
e23f7a0 to
2b40d21
Compare
|
/test pull-cluster-api-e2e-main |
fabriziopandini
changed the title
k8s-ci-robot
removed
the
do-not-merge/work-in-progress
|
/test pull-cluster-api-e2e-main |
| }, | ||
| { | ||
| ListenAddress: listenAddress, | ||
| HostPort: port, |
There was a problem hiding this comment.
| HostPort: port, | |
| HostPort: 0, |
Let's always pick a port. passing in two times the same port != 0 should lead to issues.
fabriziopandini
force-pushed
the
improve-capd-loadbalancer
branch
from
2b40d21 to
88b3662
Compare
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
LGTM label has been added. Git tree hash: d77e0152edae2809a2ea4313b423b7d75cc93b40
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: chrischdi The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
- Breakind change: kubernetes-sigs/cluster-api#11430 Signed-off-by: Danil-Grigorev <danil.grigorev@suse.com>
- Breakind change: kubernetes-sigs/cluster-api#11430 Signed-off-by: Danil-Grigorev <danil.grigorev@suse.com>
- Breakind change: kubernetes-sigs/cluster-api#11430 Signed-off-by: Danil-Grigorev <danil.grigorev@suse.com>
- Breakind change: kubernetes-sigs/cluster-api#11430 Signed-off-by: Danil-Grigorev <danil.grigorev@suse.com>
- Breakind change: kubernetes-sigs/cluster-api#11430 Signed-off-by: Danil-Grigorev <danil.grigorev@suse.com>
- Breakind change: kubernetes-sigs/cluster-api#11430 Signed-off-by: Danil-Grigorev <danil.grigorev@suse.com>
- Breakind change: kubernetes-sigs/cluster-api#11430 Signed-off-by: Danil-Grigorev <danil.grigorev@suse.com>
| for _, m := range controlPlaneMachineList.Items { | ||
| containerName := docker.MachineContainerName(cluster.Name, m.Name) | ||
| controlPlaneWeight[containerName] = 100 | ||
| if !m.DeletionTimestamp.IsZero() && len(controlPlaneMachineList.Items) > 1 { |
There was a problem hiding this comment.
Is this logic intentionally different from the one in reconcileNormal (l.255)?
There was a problem hiding this comment.
Yes. In line 255 we have to trigger reconcileLoadBalancerConfiguration when machine deletion start, but we have to make this only once; the difference (both the annotation and the check on docker machine not being deleted) is about ensuring "once".
| if dockerMachine.Annotations == nil { | ||
| dockerMachine.Annotations = map[string]string{} | ||
| } | ||
| dockerMachine.Annotations["dockermachine.infrastructure.cluster.x-k8s.io/weight"] = "0" |
There was a problem hiding this comment.
Is this annotation only used to decide if we have to call reconcileLoadBalancerConfiguration here?
There was a problem hiding this comment.
to call reconcileLoadBalancerConfiguration and to reconfigure it in the right way.
Plus: next reconciliations also use that weight information.
There was a problem hiding this comment.
FYI ATM we are not using weight from the annotation, which exists mostly to ensure the operation in L257 is executed once
There was a problem hiding this comment.
FYI ATM we are not using weight from the annotation
Yeah that's what confused me. That we never read this value. I was just wondering if we forgot to read it somewhere
There was a problem hiding this comment.
Let's just keep in mind that just because the annotation says the weight is 0. This doesn't always mean that the weight is actually 0 in haproxy (the last CP Machine)
What this PR does / why we need it:
Make CAPD drain load balancers backends when machine deletion starts, add haproxy stats
/area provider/infrastructure-docker