diff --git a/scaleway/loadbalancers.go b/scaleway/loadbalancers.go index 0eb58c5..c15337e 100644 --- a/scaleway/loadbalancers.go +++ b/scaleway/loadbalancers.go @@ -662,17 +662,6 @@ func (l *loadbalancers) updateLoadBalancer(ctx context.Context, loadbalancer *sc } } - // Remove extra backends - for _, b := range backendsOps.remove { - klog.V(3).Infof("deleting backend: %s port: %d loadbalancer: %s", b.ID, b.ForwardPort, loadbalancer.ID) - if err := l.api.DeleteBackend(&scwlb.ZonedAPIDeleteBackendRequest{ - Zone: loadbalancer.Zone, - BackendID: b.ID, - }); err != nil { - return fmt.Errorf("failed deleting backend: %s port: %d loadbalancer: %s err: %v", b.ID, b.ForwardPort, loadbalancer.ID, err) - } - } - for _, port := range service.Spec.Ports { var backend *scwlb.Backend // Update backend @@ -779,6 +768,18 @@ func (l *loadbalancers) updateLoadBalancer(ctx context.Context, loadbalancer *sc } } + // Remove extra backends. This must happen after frontends have been rewired above, + // since a backend cannot be deleted while still referenced by a frontend. + for _, b := range backendsOps.remove { + klog.V(3).Infof("deleting backend: %s port: %d loadbalancer: %s", b.ID, b.ForwardPort, loadbalancer.ID) + if err := l.api.DeleteBackend(&scwlb.ZonedAPIDeleteBackendRequest{ + Zone: loadbalancer.Zone, + BackendID: b.ID, + }); err != nil { + return fmt.Errorf("failed deleting backend: %s port: %d loadbalancer: %s err: %v", b.ID, b.ForwardPort, loadbalancer.ID, err) + } + } + if !lbExternallyManaged { loadBalancerType := getLoadBalancerType(service) if loadBalancerType != "" && strings.ToLower(loadbalancer.Type) != loadBalancerType { @@ -1183,8 +1184,11 @@ func servicePortToFrontend(service *v1.Service, loadbalancer *scwlb.LB, port v1. } return &scwlb.Frontend{ - Name: fmt.Sprintf("%s_%s_%d", string(service.UID), strings.ToLower(string(port.Protocol)), port.Port), - InboundPort: port.Port, + Name: fmt.Sprintf("%s_%s_%d", string(service.UID), strings.ToLower(string(port.Protocol)), port.Port), + InboundPort: port.Port, + // Backend is a stub carrying only the desired ForwardPort, used by frontendEquals + // to detect a frontend still wired to a stale backend (e.g. after a NodePort change). + Backend: &scwlb.Backend{ForwardPort: port.NodePort}, TimeoutClient: &timeoutClient, ConnectionRateLimit: connectionRateLimit, CertificateIDs: certificateIDs, @@ -1443,6 +1447,10 @@ func frontendEquals(got, want *scwlb.Frontend) bool { klog.V(3).Infof("frontend.InboundPort: %d - %d", got.InboundPort, want.InboundPort) return false } + if !backendForwardPortEqual(got.Backend, want.Backend) { + klog.V(3).Infof("frontend.Backend.ForwardPort: %v - %v", got.Backend, want.Backend) + return false + } if !durationPtrEqual(got.TimeoutClient, want.TimeoutClient) { klog.V(3).Infof("frontend.TimeoutClient: %s - %s", got.TimeoutClient, want.TimeoutClient) return false @@ -1898,6 +1906,18 @@ func uint32PtrEqual(got, want *uint32) bool { return *got == *want } +// backendForwardPortEqual returns true if both backends are nil, or if both are +// non-nil and reference the same ForwardPort +func backendForwardPortEqual(got, want *scwlb.Backend) bool { + if got == nil && want == nil { + return true + } + if got == nil || want == nil { + return false + } + return got.ForwardPort == want.ForwardPort +} + // chunkArray takes an array and split it in chunks of a given size func chunkArray(array []string, maxChunkSize int) [][]string { result := [][]string{} diff --git a/scaleway/loadbalancers_test.go b/scaleway/loadbalancers_test.go index 03c8d90..7fe4c31 100644 --- a/scaleway/loadbalancers_test.go +++ b/scaleway/loadbalancers_test.go @@ -399,6 +399,7 @@ func TestServicePortToFrontend(t *testing.T) { &scwlb.Frontend{ Name: "uid_tcp_1234", InboundPort: 1234, + Backend: &scwlb.Backend{ForwardPort: 0}, TimeoutClient: &defaultTimeout, CertificateIDs: []string{}, }, @@ -417,6 +418,7 @@ func TestServicePortToFrontend(t *testing.T) { &scwlb.Frontend{ Name: "uid_tcp_1234", InboundPort: 1234, + Backend: &scwlb.Backend{ForwardPort: 0}, TimeoutClient: &otherTimeout, CertificateIDs: []string{}, }, @@ -435,6 +437,7 @@ func TestServicePortToFrontend(t *testing.T) { &scwlb.Frontend{ Name: "uid_tcp_1234", InboundPort: 1234, + Backend: &scwlb.Backend{ForwardPort: 0}, TimeoutClient: &defaultTimeout, CertificateIDs: []string{"uid-1"}, }, @@ -453,6 +456,7 @@ func TestServicePortToFrontend(t *testing.T) { &scwlb.Frontend{ Name: "uid_tcp_1234", InboundPort: 1234, + Backend: &scwlb.Backend{ForwardPort: 0}, TimeoutClient: &defaultTimeout, CertificateIDs: []string{"uid-1"}, }, @@ -471,6 +475,7 @@ func TestServicePortToFrontend(t *testing.T) { &scwlb.Frontend{ Name: "uid_tcp_1234", InboundPort: 1234, + Backend: &scwlb.Backend{ForwardPort: 0}, TimeoutClient: &defaultTimeout, CertificateIDs: []string{"uid-1"}, }, @@ -489,6 +494,7 @@ func TestServicePortToFrontend(t *testing.T) { &scwlb.Frontend{ Name: "uid_tcp_1234", InboundPort: 1234, + Backend: &scwlb.Backend{ForwardPort: 0}, TimeoutClient: &defaultTimeout, CertificateIDs: []string{"uid-1", "uid-2"}, }, @@ -507,6 +513,7 @@ func TestServicePortToFrontend(t *testing.T) { &scwlb.Frontend{ Name: "uid_tcp_1234", InboundPort: 1234, + Backend: &scwlb.Backend{ForwardPort: 0}, TimeoutClient: &defaultTimeout, CertificateIDs: []string{"uid-1", "uid-2"}, }, @@ -525,6 +532,7 @@ func TestServicePortToFrontend(t *testing.T) { &scwlb.Frontend{ Name: "uid_tcp_1234", InboundPort: 1234, + Backend: &scwlb.Backend{ForwardPort: 0}, TimeoutClient: &defaultTimeout, CertificateIDs: []string{}, ConnectionRateLimit: func() *uint32 { v := uint32(100); return &v }(), @@ -544,6 +552,7 @@ func TestServicePortToFrontend(t *testing.T) { &scwlb.Frontend{ Name: "uid_tcp_1234", InboundPort: 1234, + Backend: &scwlb.Backend{ForwardPort: 0}, TimeoutClient: &defaultTimeout, CertificateIDs: []string{}, EnableAccessLogs: true, @@ -563,6 +572,7 @@ func TestServicePortToFrontend(t *testing.T) { &scwlb.Frontend{ Name: "uid_tcp_1234", InboundPort: 1234, + Backend: &scwlb.Backend{ForwardPort: 0}, TimeoutClient: &defaultTimeout, CertificateIDs: []string{}, EnableHTTP3: true, @@ -758,6 +768,28 @@ func TestFrontendEquals(t *testing.T) { }, false, }, + { + // A frontend still wired to a backend with a stale ForwardPort (e.g. after + // the service's NodePort changed) must not be considered equal, otherwise it + // never gets rewired before the stale backend is deleted. + "with a different backend forward port", + &scwlb.Frontend{ + Name: "uid_tcp_1234", + InboundPort: 1234, + Backend: &scwlb.Backend{ForwardPort: 30001}, + TimeoutClient: &otherTimeout, + CertificateIDs: []string{"uid-1", "uid-2"}, + }, + &scwlb.Frontend{ + ID: "uid-1", + Name: "uid_tcp_1234", + InboundPort: 1234, + Backend: &scwlb.Backend{ForwardPort: 30002}, + TimeoutClient: &otherTimeout, + CertificateIDs: []string{"uid-2", "uid-1"}, + }, + false, + }, { "with a different timeout", &scwlb.Frontend{