From cb053341441d8d3bb33721f5ee3f61f608e1b78b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20Krienb=C3=BChl?= Date: Tue, 13 Feb 2024 16:42:14 +0100 Subject: [PATCH] Use a more flexible port to pool mapping Previously loadbalancer pools were named as a combination of the service port protocol and outward bound port: - `service.spec.ports[].protocol` - `service.spec.ports[].port` Since a name mis-match causes a pool to be recreated - an operation that takes around 30-45 seconds - a change in the port number caused the pool and its members to be recreated. Since Kubernetes has the option to set a unique name for each service port, we can offer an approach that allows to change the port, without pool recreation in some cases: - If a service has only one port with no name and the port changes. - If a service uses names but keeps the names stable when changing ports. In such cases, only the listener has to be re-created, which can be done in 10-15 seconds. There's still a slight downtime that has to be expected, but it is much shorter. When the name changes, the pool still has to be recreated, which will be a bit unexpected, but for deployments that would like to add/remove/change a port with minimal downtime, there's now a way. --- examples/nginx-hello.yml | 2 +- pkg/cloudscale_ccm/reconcile.go | 16 +++++++++++++--- pkg/cloudscale_ccm/reconcile_test.go | 11 +++++++---- 3 files changed, 21 insertions(+), 8 deletions(-) diff --git a/examples/nginx-hello.yml b/examples/nginx-hello.yml index 345feea..8986f21 100644 --- a/examples/nginx-hello.yml +++ b/examples/nginx-hello.yml @@ -51,7 +51,7 @@ spec: - port: 80 protocol: TCP targetPort: 80 - name: primary + name: http selector: app: hello type: LoadBalancer diff --git a/pkg/cloudscale_ccm/reconcile.go b/pkg/cloudscale_ccm/reconcile.go index e13dd3f..02849e3 100644 --- a/pkg/cloudscale_ccm/reconcile.go +++ b/pkg/cloudscale_ccm/reconcile.go @@ -146,7 +146,7 @@ func desiredLbState( } pool := cloudscale.LoadBalancerPool{ - Name: poolName(port.Protocol, port.Port), + Name: poolName(port.Protocol, port.Name), Algorithm: algorithm, Protocol: protocol, } @@ -954,10 +954,20 @@ func (l *lbState) poolsByName() map[string]*cloudscale.LoadBalancerPool { // poolName produces the name of the pool for the given service port (the port // that is bound on the load balancer and reachable from outside of it). // +// We use the name of the port (may be empty, but is enforced to be unqiue +// for each service). +// // Warning: This named is used to compare desired pools to actual pools. // Any change to it causes pools to be rebuilt, which must be avoided! -func poolName(protocol v1.Protocol, port int32) string { - return strings.ToLower(fmt.Sprintf("%s/%d", protocol, port)) +func poolName(protocol v1.Protocol, name string) string { + p := strings.ToLower(string(protocol)) + + // By default, the port has no name (required with more than 1 port) + if name == "" { + return p + } + + return fmt.Sprintf("%s/%s", p, name) } // poolMemberName produces the name of the pool member for the given node diff --git a/pkg/cloudscale_ccm/reconcile_test.go b/pkg/cloudscale_ccm/reconcile_test.go index aaef175..1ee4960 100644 --- a/pkg/cloudscale_ccm/reconcile_test.go +++ b/pkg/cloudscale_ccm/reconcile_test.go @@ -13,8 +13,9 @@ import ( ) func TestPoolName(t *testing.T) { - assert.Equal(t, "tcp/80", poolName(v1.ProtocolTCP, 80)) - assert.Equal(t, "udp/443", poolName(v1.ProtocolUDP, 443)) + assert.Equal(t, "tcp", poolName(v1.ProtocolTCP, "")) + assert.Equal(t, "udp/foo", poolName(v1.ProtocolUDP, "foo")) + assert.Equal(t, "udp/FOO", poolName(v1.ProtocolUDP, "FOO")) } func TestPoolMemberName(t *testing.T) { @@ -126,11 +127,13 @@ func TestDesiredService(t *testing.T) { Protocol: "TCP", Port: 80, NodePort: 8080, + Name: "http", }, { Protocol: "TCP", Port: 443, NodePort: 8443, + Name: "https", }, } @@ -143,10 +146,10 @@ func TestDesiredService(t *testing.T) { // Have one pool per service port assert.Len(t, desired.pools, 2) - assert.Equal(t, desired.pools[0].Name, "tcp/80") + assert.Equal(t, desired.pools[0].Name, "tcp/http") assert.Equal(t, desired.pools[0].Protocol, "tcp") assert.Equal(t, desired.pools[0].Algorithm, "round_robin") - assert.Equal(t, desired.pools[1].Name, "tcp/443") + assert.Equal(t, desired.pools[1].Name, "tcp/https") assert.Equal(t, desired.pools[0].Protocol, "tcp") assert.Equal(t, desired.pools[0].Algorithm, "round_robin")