From 9a7f7a2697608ebb61486431736a1a25f1f1288a Mon Sep 17 00:00:00 2001 From: Steve Wagner Date: Thu, 30 Mar 2023 13:02:31 -0700 Subject: [PATCH 01/12] BorderClient basics --- internal/application/border_client.go | 42 ++++++++++++++++++++++ internal/application/border_client_test.go | 41 +++++++++++++++++++++ internal/application/http_border_client.go | 16 +++++++++ internal/application/tcp_border_client.go | 16 +++++++++ 4 files changed, 115 insertions(+) create mode 100644 internal/application/border_client.go create mode 100644 internal/application/border_client_test.go create mode 100644 internal/application/http_border_client.go create mode 100644 internal/application/tcp_border_client.go diff --git a/internal/application/border_client.go b/internal/application/border_client.go new file mode 100644 index 0000000..2f1fc42 --- /dev/null +++ b/internal/application/border_client.go @@ -0,0 +1,42 @@ +/* + * Copyright 2023 F5 Inc. All rights reserved. + * Use of this source code is governed by the Apache License that can be found in the LICENSE file. + */ + +package application + +import ( + "fmt" + nginxClient "github.com/nginxinc/nginx-plus-go-client/client" + "github.com/sirupsen/logrus" +) + +type Interface interface { + Update() + Delete() +} + +type BorderClient struct { + NginxPlusClient *nginxClient.NginxClient +} + +func NewBorderClient(whichType string, nginxClient *nginxClient.NginxClient) (Interface, error) { + logrus.Debugf(`NewBorderClient for type: %s`, whichType) + + switch whichType { + case "tcp": + return &TcpBorderClient{ + BorderClient: BorderClient{ + NginxPlusClient: nginxClient, + }, + }, nil + case "http": + return &HttpBorderClient{ + BorderClient: BorderClient{ + NginxPlusClient: nginxClient, + }, + }, nil + default: + return nil, fmt.Errorf(`unknown border client type: %s`, whichType) + } +} diff --git a/internal/application/border_client_test.go b/internal/application/border_client_test.go new file mode 100644 index 0000000..b2a1aa9 --- /dev/null +++ b/internal/application/border_client_test.go @@ -0,0 +1,41 @@ +/* + * Copyright 2023 F5 Inc. All rights reserved. + * Use of this source code is governed by the Apache License that can be found in the LICENSE file. + */ + +package application + +import "testing" + +func TestBorderClient_CreatesHttpBorderClient(t *testing.T) { + client, err := NewBorderClient("http", nil) + if err != nil { + t.Errorf(`error creating border client: %v`, err) + } + + if _, ok := client.(*HttpBorderClient); !ok { + t.Errorf(`expected client to be of type HttpBorderClient`) + } +} + +func TestBorderClient_CreatesTcpBorderClient(t *testing.T) { + client, err := NewBorderClient("tcp", nil) + if err != nil { + t.Errorf(`error creating border client: %v`, err) + } + + if _, ok := client.(*TcpBorderClient); !ok { + t.Errorf(`expected client to be of type TcpBorderClient`) + } +} + +func TestBorderClient_UnknownClientType(t *testing.T) { + _, err := NewBorderClient("unknown", nil) + if err == nil { + t.Errorf(`expected error creating border client`) + } + + if err.Error() != `unknown border client type: unknown` { + t.Errorf(`expected error to be 'unknown border client type: unknown', got: %v`, err) + } +} diff --git a/internal/application/http_border_client.go b/internal/application/http_border_client.go new file mode 100644 index 0000000..27345f1 --- /dev/null +++ b/internal/application/http_border_client.go @@ -0,0 +1,16 @@ +/* + * Copyright 2023 F5 Inc. All rights reserved. + * Use of this source code is governed by the Apache License that can be found in the LICENSE file. + */ + +package application + +type HttpBorderClient struct { + BorderClient +} + +func (client *HttpBorderClient) Update() { +} + +func (client *HttpBorderClient) Delete() { +} diff --git a/internal/application/tcp_border_client.go b/internal/application/tcp_border_client.go new file mode 100644 index 0000000..c792adc --- /dev/null +++ b/internal/application/tcp_border_client.go @@ -0,0 +1,16 @@ +/* + * Copyright 2023 F5 Inc. All rights reserved. + * Use of this source code is governed by the Apache License that can be found in the LICENSE file. + */ + +package application + +type TcpBorderClient struct { + BorderClient +} + +func (client *TcpBorderClient) Update() { +} + +func (client *TcpBorderClient) Delete() { +} From 8d3b6245932267a723715546be97e18411acc445 Mon Sep 17 00:00:00 2001 From: Steve Wagner Date: Thu, 30 Mar 2023 13:38:31 -0700 Subject: [PATCH 02/12] Documentation updates --- DESIGN.md | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/DESIGN.md b/DESIGN.md index 7d1a09e..bf97b92 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -17,10 +17,11 @@ stateDiagram-v2 Handler --> Translator Translator --> Handler Handler --> Synchronizer : "nkl-synchronizer queue" - Synchronizer --> NGINXPlusLB1 - Synchronizer --> NGINXPlusLB2 - Synchronizer --> NGINXPlusLB... - Synchronizer --> NGINXPlusLBn + Synchronizer --> BorderClient : "HttpBorderClient | TcpBorderClient" + BorderClient --> NGINXPlusLB1 + BorderClient --> NGINXPlusLB2 + BorderClient --> NGINXPlusLB... + BorderClient --> NGINXPlusLBn ``` ### Settings From 0b53e1ccd5cdb1fe58aaa36f32b83d24624886e0 Mon Sep 17 00:00:00 2001 From: Steve Wagner Date: Fri, 31 Mar 2023 10:55:24 -0700 Subject: [PATCH 03/12] About to apply a broad rename --- internal/application/border_client.go | 20 ++--- internal/application/border_client_test.go | 8 +- internal/application/doc.go | 19 +++++ internal/application/http_border_client.go | 33 +++++++- .../application/http_border_client_test.go | 75 +++++++++++++++++++ .../application/nginx_client_interface.go | 16 ++++ internal/application/tcp_border_client.go | 8 +- .../application/tcp_border_client_test.go | 6 ++ internal/core/events.go | 1 + test/mocks/mock_nginx_plus_client.go | 38 ++++++++++ 10 files changed, 205 insertions(+), 19 deletions(-) create mode 100644 internal/application/doc.go create mode 100644 internal/application/http_border_client_test.go create mode 100644 internal/application/nginx_client_interface.go create mode 100644 internal/application/tcp_border_client_test.go create mode 100644 test/mocks/mock_nginx_plus_client.go diff --git a/internal/application/border_client.go b/internal/application/border_client.go index 2f1fc42..c641ae2 100644 --- a/internal/application/border_client.go +++ b/internal/application/border_client.go @@ -7,35 +7,29 @@ package application import ( "fmt" - nginxClient "github.com/nginxinc/nginx-plus-go-client/client" + "github.com/nginxinc/kubernetes-nginx-ingress/internal/core" "github.com/sirupsen/logrus" ) type Interface interface { - Update() - Delete() + Update(core.ServerUpdateEvent) error + Delete(core.ServerUpdateEvent) error } type BorderClient struct { - NginxPlusClient *nginxClient.NginxClient } -func NewBorderClient(whichType string, nginxClient *nginxClient.NginxClient) (Interface, error) { +func NewBorderClient(whichType string, borderClient interface{}) (Interface, error) { logrus.Debugf(`NewBorderClient for type: %s`, whichType) switch whichType { case "tcp": return &TcpBorderClient{ - BorderClient: BorderClient{ - NginxPlusClient: nginxClient, - }, + BorderClient: BorderClient{}, }, nil case "http": - return &HttpBorderClient{ - BorderClient: BorderClient{ - NginxPlusClient: nginxClient, - }, - }, nil + return NewHttpBorderClient(borderClient) + default: return nil, fmt.Errorf(`unknown border client type: %s`, whichType) } diff --git a/internal/application/border_client_test.go b/internal/application/border_client_test.go index b2a1aa9..8a197bb 100644 --- a/internal/application/border_client_test.go +++ b/internal/application/border_client_test.go @@ -5,10 +5,14 @@ package application -import "testing" +import ( + "github.com/nginxinc/kubernetes-nginx-ingress/test/mocks" + "testing" +) func TestBorderClient_CreatesHttpBorderClient(t *testing.T) { - client, err := NewBorderClient("http", nil) + borderClient := mocks.MockNginxClient{} + client, err := NewBorderClient("http", borderClient) if err != nil { t.Errorf(`error creating border client: %v`, err) } diff --git a/internal/application/doc.go b/internal/application/doc.go new file mode 100644 index 0000000..f0865c4 --- /dev/null +++ b/internal/application/doc.go @@ -0,0 +1,19 @@ +/* + * Copyright 2023 F5 Inc. All rights reserved. + * Use of this source code is governed by the Apache License that can be found in the LICENSE file. + */ + +/* +Package application includes support for applying updates to the Border servers. + +"Border Servers" are the servers that are exposed to the outside world and direct traffic into the cluster. +At this time the only supported Border Servers are NGINX Plus servers. The BorderClient module defines +an interface that can be implemented to support other Border Server types. + +- HttpBorderClient: updates NGINX Plus servers using HTTP Upstream methods on the NGINX Plus API. +- TcpBorderClient: updates NGINX Plus servers using Stream Upstream methods on the NGINX Plus API. + +Selection of the appropriate client is based on the Annotations present on the NodePort Service definition. +*/ + +package application diff --git a/internal/application/http_border_client.go b/internal/application/http_border_client.go index 27345f1..7c2b777 100644 --- a/internal/application/http_border_client.go +++ b/internal/application/http_border_client.go @@ -5,12 +5,41 @@ package application +import ( + "fmt" + "github.com/nginxinc/kubernetes-nginx-ingress/internal/core" +) + type HttpBorderClient struct { BorderClient + nginxClient NginxClientInterface +} + +func NewHttpBorderClient(client interface{}) (Interface, error) { + nginxClient, ok := client.(NginxClientInterface) + if !ok { + return nil, fmt.Errorf(`expected a NginxClientInterface, got a %v`, client) + } + + return &HttpBorderClient{ + nginxClient: nginxClient, + }, nil } -func (client *HttpBorderClient) Update() { +func (hbc *HttpBorderClient) Update(event core.ServerUpdateEvent) error { + _, _, _, err := hbc.nginxClient.UpdateHTTPServers(event.NginxHost, nil) + if err != nil { + return fmt.Errorf(`error occurred updating the nginx+ upstream server: %w`, err) + } + + return nil } -func (client *HttpBorderClient) Delete() { +func (hbc *HttpBorderClient) Delete(event core.ServerUpdateEvent) error { + err := hbc.nginxClient.DeleteHTTPServer(event.NginxHost, event.Servers[0].Server) + if err != nil { + return fmt.Errorf(`error occurred deleting the nginx+ upstream server: %w`, err) + } + + return nil } diff --git a/internal/application/http_border_client_test.go b/internal/application/http_border_client_test.go new file mode 100644 index 0000000..1336f52 --- /dev/null +++ b/internal/application/http_border_client_test.go @@ -0,0 +1,75 @@ +/* + * Copyright 2023 F5 Inc. All rights reserved. + * Use of this source code is governed by the Apache License that can be found in the LICENSE file. + */ + +package application + +import ( + "github.com/nginxinc/kubernetes-nginx-ingress/internal/core" + "github.com/nginxinc/kubernetes-nginx-ingress/test/mocks" + nginxClient2 "github.com/nginxinc/nginx-plus-go-client/client" + "testing" +) + +const ( + clientType = "http" + deletedEventType = core.Deleted + createEventType = core.Created + upstreamName = "upstreamName" + server = "server" +) + +func TestHttpBorderClient_Delete(t *testing.T) { + servers := []nginxClient2.StreamUpstreamServer{ + { + Server: server, + }, + } + event := core.NewServerUpdateEvent(deletedEventType, upstreamName, servers) + nginxClient := mocks.NewMockNginxClient() + borderClient, err := NewBorderClient(clientType, nginxClient) + if err != nil { + t.Fatalf(`error occurred creating a new border client: %v`, err) + } + + err = borderClient.Delete(*event) + if err != nil { + t.Fatalf(`error occurred deleting the nginx+ upstream server: %v`, err) + } + + if !nginxClient.CalledFunctions["DeleteHTTPServer"] { + t.Fatalf(`expected DeleteHTTPServer to be called`) + } +} + +func TestHttpBorderClient_Update(t *testing.T) { + servers := []nginxClient2.StreamUpstreamServer{ + { + Server: server, + }, + } + event := core.NewServerUpdateEvent(deletedEventType, upstreamName, servers) + nginxClient := mocks.NewMockNginxClient() + borderClient, err := NewBorderClient(clientType, nginxClient) + if err != nil { + t.Fatalf(`error occurred creating a new border client: %v`, err) + } + + err = borderClient.Update(*event) + if err != nil { + t.Fatalf(`error occurred deleting the nginx+ upstream server: %v`, err) + } + + if !nginxClient.CalledFunctions["UpdateHTTPServers"] { + t.Fatalf(`expected UpdateHTTPServers to be called`) + } +} + +func TestHttpBorderClient_BadNginxClient(t *testing.T) { + var emptyInterface interface{} + _, err := NewBorderClient(clientType, emptyInterface) + if err == nil { + t.Fatalf(`expected an error to occur when creating a new border client`) + } +} diff --git a/internal/application/nginx_client_interface.go b/internal/application/nginx_client_interface.go new file mode 100644 index 0000000..5ca986d --- /dev/null +++ b/internal/application/nginx_client_interface.go @@ -0,0 +1,16 @@ +/* + * Copyright (c) 2023 F5 Inc. All rights reserved. + * Use of this source code is governed by the Apache License that can be found in the LICENSE file. + */ + +package application + +import nginxClient "github.com/nginxinc/nginx-plus-go-client/client" + +type NginxClientInterface interface { + DeleteStreamServer(upstream string, server string) error + UpdateStreamServers(upstream string, servers []nginxClient.StreamUpstreamServer) ([]nginxClient.StreamUpstreamServer, []nginxClient.StreamUpstreamServer, []nginxClient.StreamUpstreamServer, error) + + DeleteHTTPServer(upstream string, server string) error + UpdateHTTPServers(upstream string, servers []nginxClient.UpstreamServer) ([]nginxClient.UpstreamServer, []nginxClient.UpstreamServer, []nginxClient.UpstreamServer, error) +} diff --git a/internal/application/tcp_border_client.go b/internal/application/tcp_border_client.go index c792adc..1005c42 100644 --- a/internal/application/tcp_border_client.go +++ b/internal/application/tcp_border_client.go @@ -5,12 +5,16 @@ package application +import "github.com/nginxinc/kubernetes-nginx-ingress/internal/core" + type TcpBorderClient struct { BorderClient } -func (client *TcpBorderClient) Update() { +func (client *TcpBorderClient) Update(_ core.ServerUpdateEvent) error { + return nil } -func (client *TcpBorderClient) Delete() { +func (client *TcpBorderClient) Delete(_ core.ServerUpdateEvent) error { + return nil } diff --git a/internal/application/tcp_border_client_test.go b/internal/application/tcp_border_client_test.go new file mode 100644 index 0000000..77741b7 --- /dev/null +++ b/internal/application/tcp_border_client_test.go @@ -0,0 +1,6 @@ +/* + * Copyright 2023 F5 Inc. All rights reserved. + * Use of this source code is governed by the Apache License that can be found in the LICENSE file. + */ + +package application diff --git a/internal/core/events.go b/internal/core/events.go index ecfbbc5..fea1b09 100644 --- a/internal/core/events.go +++ b/internal/core/events.go @@ -31,6 +31,7 @@ type ServerUpdateEvent struct { Type EventType UpstreamName string Servers []nginxClient.StreamUpstreamServer + HttpServers []nginxClient.UpstreamServer } type ServerUpdateEvents = []*ServerUpdateEvent diff --git a/test/mocks/mock_nginx_plus_client.go b/test/mocks/mock_nginx_plus_client.go new file mode 100644 index 0000000..7529ff7 --- /dev/null +++ b/test/mocks/mock_nginx_plus_client.go @@ -0,0 +1,38 @@ +/* + * Copyright (c) 2023 F5 Inc. All rights reserved. + * Use of this source code is governed by the Apache License that can be found in the LICENSE file. + */ + +package mocks + +import nginxClient "github.com/nginxinc/nginx-plus-go-client/client" + +type MockNginxClient struct { + CalledFunctions map[string]bool +} + +func NewMockNginxClient() *MockNginxClient { + return &MockNginxClient{ + CalledFunctions: make(map[string]bool), + } +} + +func (m MockNginxClient) DeleteStreamServer(upstream string, server string) error { + m.CalledFunctions["DeleteStreamServer"] = true + return nil +} + +func (m MockNginxClient) UpdateStreamServers(upstream string, servers []nginxClient.StreamUpstreamServer) ([]nginxClient.StreamUpstreamServer, []nginxClient.StreamUpstreamServer, []nginxClient.StreamUpstreamServer, error) { + m.CalledFunctions["UpdateStreamServers"] = true + return nil, nil, nil, nil +} + +func (m MockNginxClient) DeleteHTTPServer(upstream string, server string) error { + m.CalledFunctions["DeleteHTTPServer"] = true + return nil +} + +func (m MockNginxClient) UpdateHTTPServers(upstream string, servers []nginxClient.UpstreamServer) ([]nginxClient.UpstreamServer, []nginxClient.UpstreamServer, []nginxClient.UpstreamServer, error) { + m.CalledFunctions["UpdateHTTPServers"] = true + return nil, nil, nil, nil +} From 1ae7e2ca21ab3da6ba81242b3e6c3447a46810b3 Mon Sep 17 00:00:00 2001 From: Steve Wagner Date: Fri, 31 Mar 2023 11:14:06 -0700 Subject: [PATCH 04/12] Rename to support library differences --- internal/application/doc.go | 4 ++-- internal/application/http_border_client.go | 2 +- internal/core/events.go | 6 +++--- internal/synchronization/synchronizer.go | 4 ++-- internal/synchronization/synchronizer_test.go | 4 ++-- internal/translation/translator_test.go | 2 +- 6 files changed, 11 insertions(+), 11 deletions(-) diff --git a/internal/application/doc.go b/internal/application/doc.go index f0865c4..3254255 100644 --- a/internal/application/doc.go +++ b/internal/application/doc.go @@ -6,8 +6,8 @@ /* Package application includes support for applying updates to the Border servers. -"Border Servers" are the servers that are exposed to the outside world and direct traffic into the cluster. -At this time the only supported Border Servers are NGINX Plus servers. The BorderClient module defines +"Border TcpServers" are the servers that are exposed to the outside world and direct traffic into the cluster. +At this time the only supported Border TcpServers are NGINX Plus servers. The BorderClient module defines an interface that can be implemented to support other Border Server types. - HttpBorderClient: updates NGINX Plus servers using HTTP Upstream methods on the NGINX Plus API. diff --git a/internal/application/http_border_client.go b/internal/application/http_border_client.go index 7c2b777..0c7c067 100644 --- a/internal/application/http_border_client.go +++ b/internal/application/http_border_client.go @@ -36,7 +36,7 @@ func (hbc *HttpBorderClient) Update(event core.ServerUpdateEvent) error { } func (hbc *HttpBorderClient) Delete(event core.ServerUpdateEvent) error { - err := hbc.nginxClient.DeleteHTTPServer(event.NginxHost, event.Servers[0].Server) + err := hbc.nginxClient.DeleteHTTPServer(event.NginxHost, event.TcpServers[0].Server) // TODO: SW: This needs to be HttpServers not TcpServers if err != nil { return fmt.Errorf(`error occurred deleting the nginx+ upstream server: %w`, err) } diff --git a/internal/core/events.go b/internal/core/events.go index fea1b09..4906d52 100644 --- a/internal/core/events.go +++ b/internal/core/events.go @@ -30,7 +30,7 @@ type ServerUpdateEvent struct { NginxHost string Type EventType UpstreamName string - Servers []nginxClient.StreamUpstreamServer + TcpServers []nginxClient.StreamUpstreamServer HttpServers []nginxClient.UpstreamServer } @@ -49,7 +49,7 @@ func NewServerUpdateEvent(eventType EventType, upstreamName string, servers []ng return &ServerUpdateEvent{ Type: eventType, UpstreamName: upstreamName, - Servers: servers, + TcpServers: servers, } } @@ -59,7 +59,7 @@ func ServerUpdateEventWithIdAndHost(event *ServerUpdateEvent, id string, nginxHo NginxHost: nginxHost, Type: event.Type, UpstreamName: event.UpstreamName, - Servers: event.Servers, + TcpServers: event.TcpServers, } } diff --git a/internal/synchronization/synchronizer.go b/internal/synchronization/synchronizer.go index ecb0a62..5dc8451 100644 --- a/internal/synchronization/synchronizer.go +++ b/internal/synchronization/synchronizer.go @@ -150,7 +150,7 @@ func (s *Synchronizer) handleCreatedUpdatedEvent(serverUpdateEvent *core.ServerU return fmt.Errorf(`error occurred building the nginx+ client: %w`, err) } - _, _, _, err = client.UpdateStreamServers(serverUpdateEvent.UpstreamName, serverUpdateEvent.Servers) + _, _, _, err = client.UpdateStreamServers(serverUpdateEvent.UpstreamName, serverUpdateEvent.TcpServers) if err != nil { return fmt.Errorf(`error occurred updating the nginx+ upstream servers: %w`, err) } @@ -168,7 +168,7 @@ func (s *Synchronizer) handleDeletedEvent(serverUpdateEvent *core.ServerUpdateEv return fmt.Errorf(`error occurred building the nginx+ client: %w`, err) } - err = client.DeleteStreamServer(serverUpdateEvent.UpstreamName, serverUpdateEvent.Servers[0].Server) + err = client.DeleteStreamServer(serverUpdateEvent.UpstreamName, serverUpdateEvent.TcpServers[0].Server) if err != nil { return fmt.Errorf(`error occurred deleting the nginx+ upstream server: %w`, err) } diff --git a/internal/synchronization/synchronizer_test.go b/internal/synchronization/synchronizer_test.go index 4c1911e..a69b148 100644 --- a/internal/synchronization/synchronizer_test.go +++ b/internal/synchronization/synchronizer_test.go @@ -36,7 +36,7 @@ func TestSynchronizer_AddEventNoHosts(t *testing.T) { NginxHost: "", Type: 0, UpstreamName: "", - Servers: nil, + TcpServers: nil, } settings, err := configuration.NewSettings(context.Background(), nil) rateLimiter := &mocks.MockRateLimiter{} @@ -192,7 +192,7 @@ func buildEvents(count int) core.ServerUpdateEvents { NginxHost: "https://localhost:8080", Type: 0, UpstreamName: "", - Servers: nil, + TcpServers: nil, } } return events diff --git a/internal/translation/translator_test.go b/internal/translation/translator_test.go index bdf13b6..4409cff 100644 --- a/internal/translation/translator_test.go +++ b/internal/translation/translator_test.go @@ -594,7 +594,7 @@ func TestDeletedTranslateManyMixedPortsAndManyNodes(t *testing.T) { func assertExpectedServerCount(t *testing.T, expectedCount int, events core.ServerUpdateEvents) { for _, translatedEvent := range events { - serverCount := len(translatedEvent.Servers) + serverCount := len(translatedEvent.TcpServers) if serverCount != expectedCount { t.Fatalf("expected %d servers, got %d", expectedCount, serverCount) } From 0458c6bd75d057df2a1a5303afbd716e796c96e8 Mon Sep 17 00:00:00 2001 From: Steve Wagner Date: Fri, 31 Mar 2023 12:32:29 -0700 Subject: [PATCH 05/12] Add HTTP Border Client --- internal/application/http_border_client.go | 2 +- .../application/http_border_client_test.go | 78 ++++++++++++++----- internal/core/events.go | 6 +- internal/core/events_test.go | 13 ++-- internal/translation/translator.go | 23 ++++-- test/mocks/mock_nginx_plus_client.go | 37 ++++++++- 6 files changed, 123 insertions(+), 36 deletions(-) diff --git a/internal/application/http_border_client.go b/internal/application/http_border_client.go index 0c7c067..b1f1b04 100644 --- a/internal/application/http_border_client.go +++ b/internal/application/http_border_client.go @@ -36,7 +36,7 @@ func (hbc *HttpBorderClient) Update(event core.ServerUpdateEvent) error { } func (hbc *HttpBorderClient) Delete(event core.ServerUpdateEvent) error { - err := hbc.nginxClient.DeleteHTTPServer(event.NginxHost, event.TcpServers[0].Server) // TODO: SW: This needs to be HttpServers not TcpServers + err := hbc.nginxClient.DeleteHTTPServer(event.NginxHost, event.HttpServers[0].Server) if err != nil { return fmt.Errorf(`error occurred deleting the nginx+ upstream server: %w`, err) } diff --git a/internal/application/http_border_client_test.go b/internal/application/http_border_client_test.go index 1336f52..237b866 100644 --- a/internal/application/http_border_client_test.go +++ b/internal/application/http_border_client_test.go @@ -6,6 +6,7 @@ package application import ( + "errors" "github.com/nginxinc/kubernetes-nginx-ingress/internal/core" "github.com/nginxinc/kubernetes-nginx-ingress/test/mocks" nginxClient2 "github.com/nginxinc/nginx-plus-go-client/client" @@ -20,20 +21,16 @@ const ( server = "server" ) +var emptyStreamServers = []nginxClient2.StreamUpstreamServer{} + func TestHttpBorderClient_Delete(t *testing.T) { - servers := []nginxClient2.StreamUpstreamServer{ - { - Server: server, - }, - } - event := core.NewServerUpdateEvent(deletedEventType, upstreamName, servers) - nginxClient := mocks.NewMockNginxClient() - borderClient, err := NewBorderClient(clientType, nginxClient) + event := buildServerUpdateEvent(deletedEventType) + borderClient, nginxClient, err := buildBorderClient() if err != nil { t.Fatalf(`error occurred creating a new border client: %v`, err) } - err = borderClient.Delete(*event) + err = borderClient.Delete(event) if err != nil { t.Fatalf(`error occurred deleting the nginx+ upstream server: %v`, err) } @@ -44,19 +41,13 @@ func TestHttpBorderClient_Delete(t *testing.T) { } func TestHttpBorderClient_Update(t *testing.T) { - servers := []nginxClient2.StreamUpstreamServer{ - { - Server: server, - }, - } - event := core.NewServerUpdateEvent(deletedEventType, upstreamName, servers) - nginxClient := mocks.NewMockNginxClient() - borderClient, err := NewBorderClient(clientType, nginxClient) + event := buildServerUpdateEvent(createEventType) + borderClient, nginxClient, err := buildBorderClient() if err != nil { t.Fatalf(`error occurred creating a new border client: %v`, err) } - err = borderClient.Update(*event) + err = borderClient.Update(event) if err != nil { t.Fatalf(`error occurred deleting the nginx+ upstream server: %v`, err) } @@ -73,3 +64,54 @@ func TestHttpBorderClient_BadNginxClient(t *testing.T) { t.Fatalf(`expected an error to occur when creating a new border client`) } } + +func TestHttpBorderClient_DeleteReturnsError(t *testing.T) { + event := buildServerUpdateEvent(deletedEventType) + borderClient, _, err := buildTerrorizingBorderClient() + if err != nil { + t.Fatalf(`error occurred creating a new border client: %v`, err) + } + + err = borderClient.Delete(event) + + if err == nil { + t.Fatalf(`expected an error to occur when deleting the nginx+ upstream server`) + } +} + +func TestHttpBorderClient_UpdateReturnsError(t *testing.T) { + event := buildServerUpdateEvent(createEventType) + borderClient, _, err := buildTerrorizingBorderClient() + if err != nil { + t.Fatalf(`error occurred creating a new border client: %v`, err) + } + + err = borderClient.Update(event) + + if err == nil { + t.Fatalf(`expected an error to occur when deleting the nginx+ upstream server`) + } +} + +func buildTerrorizingBorderClient() (Interface, *mocks.MockNginxClient, error) { + nginxClient := mocks.NewErroringMockClient(errors.New(`something went horribly horribly wrong`)) + bc, err := NewBorderClient(clientType, nginxClient) + + return bc, nginxClient, err +} + +func buildBorderClient() (Interface, *mocks.MockNginxClient, error) { + nginxClient := mocks.NewMockNginxClient() + bc, err := NewBorderClient(clientType, nginxClient) + + return bc, nginxClient, err +} + +func buildServerUpdateEvent(eventType core.EventType) core.ServerUpdateEvent { + servers := []nginxClient2.UpstreamServer{ + { + Server: server, + }, + } + return *core.NewServerUpdateEvent(eventType, upstreamName, emptyStreamServers, servers) +} diff --git a/internal/core/events.go b/internal/core/events.go index 4906d52..7e3b9ee 100644 --- a/internal/core/events.go +++ b/internal/core/events.go @@ -45,11 +45,12 @@ func NewEvent(eventType EventType, service *v1.Service, previousService *v1.Serv } } -func NewServerUpdateEvent(eventType EventType, upstreamName string, servers []nginxClient.StreamUpstreamServer) *ServerUpdateEvent { +func NewServerUpdateEvent(eventType EventType, upstreamName string, tcpServers []nginxClient.StreamUpstreamServer, httpServers []nginxClient.UpstreamServer) *ServerUpdateEvent { return &ServerUpdateEvent{ Type: eventType, UpstreamName: upstreamName, - TcpServers: servers, + TcpServers: tcpServers, + HttpServers: httpServers, } } @@ -60,6 +61,7 @@ func ServerUpdateEventWithIdAndHost(event *ServerUpdateEvent, id string, nginxHo Type: event.Type, UpstreamName: event.UpstreamName, TcpServers: event.TcpServers, + HttpServers: event.HttpServers, } } diff --git a/internal/core/events_test.go b/internal/core/events_test.go index d46118f..c808e99 100644 --- a/internal/core/events_test.go +++ b/internal/core/events_test.go @@ -10,8 +10,11 @@ import ( "testing" ) +var emptyStreamServers []nginxClient.StreamUpstreamServer +var emptyHttpServers []nginxClient.UpstreamServer + func TestServerUpdateEventWithIdAndHost(t *testing.T) { - event := NewServerUpdateEvent(Created, "upstream", []nginxClient.StreamUpstreamServer{}) + event := NewServerUpdateEvent(Created, "upstream", emptyStreamServers, emptyHttpServers) if event.Id != "" { t.Errorf("expected empty Id, got %s", event.Id) @@ -33,7 +36,7 @@ func TestServerUpdateEventWithIdAndHost(t *testing.T) { } func TestTypeNameCreated(t *testing.T) { - event := NewServerUpdateEvent(Created, "upstream", []nginxClient.StreamUpstreamServer{}) + event := NewServerUpdateEvent(Created, "upstream", emptyStreamServers, emptyHttpServers) if event.TypeName() != "Created" { t.Errorf("expected 'Created', got %s", event.TypeName()) @@ -41,7 +44,7 @@ func TestTypeNameCreated(t *testing.T) { } func TestTypeNameUpdated(t *testing.T) { - event := NewServerUpdateEvent(Updated, "upstream", []nginxClient.StreamUpstreamServer{}) + event := NewServerUpdateEvent(Updated, "upstream", emptyStreamServers, emptyHttpServers) if event.TypeName() != "Updated" { t.Errorf("expected 'Updated', got %s", event.TypeName()) @@ -49,7 +52,7 @@ func TestTypeNameUpdated(t *testing.T) { } func TestTypeNameDeleted(t *testing.T) { - event := NewServerUpdateEvent(Deleted, "upstream", []nginxClient.StreamUpstreamServer{}) + event := NewServerUpdateEvent(Deleted, "upstream", emptyStreamServers, emptyHttpServers) if event.TypeName() != "Deleted" { t.Errorf("expected 'Deleted', got %s", event.TypeName()) @@ -57,7 +60,7 @@ func TestTypeNameDeleted(t *testing.T) { } func TestTypeNameUnknown(t *testing.T) { - event := NewServerUpdateEvent(EventType(100), "upstream", []nginxClient.StreamUpstreamServer{}) + event := NewServerUpdateEvent(EventType(100), "upstream", emptyStreamServers, emptyHttpServers) if event.TypeName() != "Unknown" { t.Errorf("expected 'Unknown', got %s", event.TypeName()) diff --git a/internal/translation/translator.go b/internal/translation/translator.go index 0bd0e02..13f7c4a 100644 --- a/internal/translation/translator.go +++ b/internal/translation/translator.go @@ -45,17 +45,24 @@ func buildServerUpdateEvents(ports []v1.ServicePort, event *core.Event) (core.Se events := core.ServerUpdateEvents{} for _, port := range ports { ingressName := fixIngressName(port.Name) - servers, _ := buildServers(event.NodeIps, port) + tcpServers, _ := buildTcpServers(event.NodeIps, port) + httpServers, _ := buildHttpServers(event.NodeIps, port) switch event.Type { case core.Created: fallthrough + case core.Updated: - events = append(events, core.NewServerUpdateEvent(event.Type, ingressName, servers)) - case core.Deleted: - for _, server := range servers { - events = append(events, core.NewServerUpdateEvent(event.Type, ingressName, []nginxClient.StreamUpstreamServer{server})) + events = append(events, core.NewServerUpdateEvent(event.Type, ingressName, tcpServers, httpServers)) + + case core.Deleted: // TODO: SW: This will be interesting, need to distinguish between a TCP and HTTP target + for _, server := range tcpServers { + events = append(events, core.NewServerUpdateEvent(event.Type, ingressName, []nginxClient.StreamUpstreamServer{server}, httpServers)) + } + for _, server := range httpServers { + events = append(events, core.NewServerUpdateEvent(event.Type, ingressName, tcpServers, []nginxClient.UpstreamServer{server})) } + default: logrus.Warnf(`Translator::buildServerUpdateEvents: unknown event type: %d`, event.Type) } @@ -65,7 +72,11 @@ func buildServerUpdateEvents(ports []v1.ServicePort, event *core.Event) (core.Se return events, nil } -func buildServers(nodeIps []string, port v1.ServicePort) ([]nginxClient.StreamUpstreamServer, error) { +func buildHttpServers(_ []string, _ v1.ServicePort) ([]nginxClient.UpstreamServer, error) { + return []nginxClient.UpstreamServer{}, nil +} + +func buildTcpServers(nodeIps []string, port v1.ServicePort) ([]nginxClient.StreamUpstreamServer, error) { var servers []nginxClient.StreamUpstreamServer for _, nodeIp := range nodeIps { diff --git a/test/mocks/mock_nginx_plus_client.go b/test/mocks/mock_nginx_plus_client.go index 7529ff7..2c16e12 100644 --- a/test/mocks/mock_nginx_plus_client.go +++ b/test/mocks/mock_nginx_plus_client.go @@ -9,30 +9,59 @@ import nginxClient "github.com/nginxinc/nginx-plus-go-client/client" type MockNginxClient struct { CalledFunctions map[string]bool + Error error } func NewMockNginxClient() *MockNginxClient { return &MockNginxClient{ CalledFunctions: make(map[string]bool), + Error: nil, } } -func (m MockNginxClient) DeleteStreamServer(upstream string, server string) error { +func NewErroringMockClient(err error) *MockNginxClient { + return &MockNginxClient{ + CalledFunctions: make(map[string]bool), + Error: err, + } +} + +func (m MockNginxClient) DeleteStreamServer(_ string, _ string) error { m.CalledFunctions["DeleteStreamServer"] = true + + if m.Error != nil { + return m.Error + } + return nil } -func (m MockNginxClient) UpdateStreamServers(upstream string, servers []nginxClient.StreamUpstreamServer) ([]nginxClient.StreamUpstreamServer, []nginxClient.StreamUpstreamServer, []nginxClient.StreamUpstreamServer, error) { +func (m MockNginxClient) UpdateStreamServers(_ string, _ []nginxClient.StreamUpstreamServer) ([]nginxClient.StreamUpstreamServer, []nginxClient.StreamUpstreamServer, []nginxClient.StreamUpstreamServer, error) { m.CalledFunctions["UpdateStreamServers"] = true + + if m.Error != nil { + return nil, nil, nil, m.Error + } + return nil, nil, nil, nil } -func (m MockNginxClient) DeleteHTTPServer(upstream string, server string) error { +func (m MockNginxClient) DeleteHTTPServer(_ string, _ string) error { m.CalledFunctions["DeleteHTTPServer"] = true + + if m.Error != nil { + return m.Error + } + return nil } -func (m MockNginxClient) UpdateHTTPServers(upstream string, servers []nginxClient.UpstreamServer) ([]nginxClient.UpstreamServer, []nginxClient.UpstreamServer, []nginxClient.UpstreamServer, error) { +func (m MockNginxClient) UpdateHTTPServers(_ string, _ []nginxClient.UpstreamServer) ([]nginxClient.UpstreamServer, []nginxClient.UpstreamServer, []nginxClient.UpstreamServer, error) { m.CalledFunctions["UpdateHTTPServers"] = true + + if m.Error != nil { + return nil, nil, nil, m.Error + } + return nil, nil, nil, nil } From bb2656c5b1532115e43c4aef8ad081eaa0bd2df3 Mon Sep 17 00:00:00 2001 From: Steve Wagner Date: Fri, 31 Mar 2023 13:13:10 -0700 Subject: [PATCH 06/12] Add TCP Border Client --- .../application/application_common_test.go | 51 +++++++++++++ internal/application/border_client.go | 5 +- internal/application/border_client_test.go | 7 +- .../application/http_border_client_test.go | 47 ++---------- internal/application/tcp_border_client.go | 31 +++++++- .../application/tcp_border_client_test.go | 74 +++++++++++++++++++ 6 files changed, 165 insertions(+), 50 deletions(-) create mode 100644 internal/application/application_common_test.go diff --git a/internal/application/application_common_test.go b/internal/application/application_common_test.go new file mode 100644 index 0000000..c55f903 --- /dev/null +++ b/internal/application/application_common_test.go @@ -0,0 +1,51 @@ +/* + * Copyright (c) 2023 F5 Inc. All rights reserved. + * Use of this source code is governed by the Apache License that can be found in the LICENSE file. + */ + +package application + +import ( + "errors" + "github.com/nginxinc/kubernetes-nginx-ingress/internal/core" + "github.com/nginxinc/kubernetes-nginx-ingress/test/mocks" + nginxClient2 "github.com/nginxinc/nginx-plus-go-client/client" +) + +const ( + clientTypeTcp = "tcp" + clientTypeHttp = "http" + deletedEventType = core.Deleted + createEventType = core.Created + upstreamName = "upstreamName" + server = "server" +) + +func buildTerrorizingBorderClient(clientType string) (Interface, *mocks.MockNginxClient, error) { + nginxClient := mocks.NewErroringMockClient(errors.New(`something went horribly horribly wrong`)) + bc, err := NewBorderClient(clientType, nginxClient) + + return bc, nginxClient, err +} + +func buildBorderClient(clientType string) (Interface, *mocks.MockNginxClient, error) { + nginxClient := mocks.NewMockNginxClient() + bc, err := NewBorderClient(clientType, nginxClient) + + return bc, nginxClient, err +} + +func buildServerUpdateEvent(eventType core.EventType) core.ServerUpdateEvent { + httpServers := []nginxClient2.UpstreamServer{ + { + Server: server, + }, + } + + tcpServers := []nginxClient2.StreamUpstreamServer{ + { + Server: server, + }, + } + return *core.NewServerUpdateEvent(eventType, upstreamName, tcpServers, httpServers) +} diff --git a/internal/application/border_client.go b/internal/application/border_client.go index c641ae2..d79beef 100644 --- a/internal/application/border_client.go +++ b/internal/application/border_client.go @@ -24,9 +24,8 @@ func NewBorderClient(whichType string, borderClient interface{}) (Interface, err switch whichType { case "tcp": - return &TcpBorderClient{ - BorderClient: BorderClient{}, - }, nil + return NewTcpBorderClient(borderClient) + case "http": return NewHttpBorderClient(borderClient) diff --git a/internal/application/border_client_test.go b/internal/application/border_client_test.go index 8a197bb..c7e6149 100644 --- a/internal/application/border_client_test.go +++ b/internal/application/border_client_test.go @@ -23,7 +23,8 @@ func TestBorderClient_CreatesHttpBorderClient(t *testing.T) { } func TestBorderClient_CreatesTcpBorderClient(t *testing.T) { - client, err := NewBorderClient("tcp", nil) + borderClient := mocks.MockNginxClient{} + client, err := NewBorderClient("tcp", borderClient) if err != nil { t.Errorf(`error creating border client: %v`, err) } @@ -34,7 +35,9 @@ func TestBorderClient_CreatesTcpBorderClient(t *testing.T) { } func TestBorderClient_UnknownClientType(t *testing.T) { - _, err := NewBorderClient("unknown", nil) + unknownClientType := "unknown" + borderClient := mocks.MockNginxClient{} + _, err := NewBorderClient(unknownClientType, borderClient) if err == nil { t.Errorf(`expected error creating border client`) } diff --git a/internal/application/http_border_client_test.go b/internal/application/http_border_client_test.go index 237b866..7164bf7 100644 --- a/internal/application/http_border_client_test.go +++ b/internal/application/http_border_client_test.go @@ -6,26 +6,12 @@ package application import ( - "errors" - "github.com/nginxinc/kubernetes-nginx-ingress/internal/core" - "github.com/nginxinc/kubernetes-nginx-ingress/test/mocks" - nginxClient2 "github.com/nginxinc/nginx-plus-go-client/client" "testing" ) -const ( - clientType = "http" - deletedEventType = core.Deleted - createEventType = core.Created - upstreamName = "upstreamName" - server = "server" -) - -var emptyStreamServers = []nginxClient2.StreamUpstreamServer{} - func TestHttpBorderClient_Delete(t *testing.T) { event := buildServerUpdateEvent(deletedEventType) - borderClient, nginxClient, err := buildBorderClient() + borderClient, nginxClient, err := buildBorderClient(clientTypeHttp) if err != nil { t.Fatalf(`error occurred creating a new border client: %v`, err) } @@ -42,7 +28,7 @@ func TestHttpBorderClient_Delete(t *testing.T) { func TestHttpBorderClient_Update(t *testing.T) { event := buildServerUpdateEvent(createEventType) - borderClient, nginxClient, err := buildBorderClient() + borderClient, nginxClient, err := buildBorderClient(clientTypeHttp) if err != nil { t.Fatalf(`error occurred creating a new border client: %v`, err) } @@ -59,7 +45,7 @@ func TestHttpBorderClient_Update(t *testing.T) { func TestHttpBorderClient_BadNginxClient(t *testing.T) { var emptyInterface interface{} - _, err := NewBorderClient(clientType, emptyInterface) + _, err := NewBorderClient(clientTypeHttp, emptyInterface) if err == nil { t.Fatalf(`expected an error to occur when creating a new border client`) } @@ -67,7 +53,7 @@ func TestHttpBorderClient_BadNginxClient(t *testing.T) { func TestHttpBorderClient_DeleteReturnsError(t *testing.T) { event := buildServerUpdateEvent(deletedEventType) - borderClient, _, err := buildTerrorizingBorderClient() + borderClient, _, err := buildTerrorizingBorderClient(clientTypeHttp) if err != nil { t.Fatalf(`error occurred creating a new border client: %v`, err) } @@ -81,7 +67,7 @@ func TestHttpBorderClient_DeleteReturnsError(t *testing.T) { func TestHttpBorderClient_UpdateReturnsError(t *testing.T) { event := buildServerUpdateEvent(createEventType) - borderClient, _, err := buildTerrorizingBorderClient() + borderClient, _, err := buildTerrorizingBorderClient(clientTypeHttp) if err != nil { t.Fatalf(`error occurred creating a new border client: %v`, err) } @@ -92,26 +78,3 @@ func TestHttpBorderClient_UpdateReturnsError(t *testing.T) { t.Fatalf(`expected an error to occur when deleting the nginx+ upstream server`) } } - -func buildTerrorizingBorderClient() (Interface, *mocks.MockNginxClient, error) { - nginxClient := mocks.NewErroringMockClient(errors.New(`something went horribly horribly wrong`)) - bc, err := NewBorderClient(clientType, nginxClient) - - return bc, nginxClient, err -} - -func buildBorderClient() (Interface, *mocks.MockNginxClient, error) { - nginxClient := mocks.NewMockNginxClient() - bc, err := NewBorderClient(clientType, nginxClient) - - return bc, nginxClient, err -} - -func buildServerUpdateEvent(eventType core.EventType) core.ServerUpdateEvent { - servers := []nginxClient2.UpstreamServer{ - { - Server: server, - }, - } - return *core.NewServerUpdateEvent(eventType, upstreamName, emptyStreamServers, servers) -} diff --git a/internal/application/tcp_border_client.go b/internal/application/tcp_border_client.go index 1005c42..4f8fed8 100644 --- a/internal/application/tcp_border_client.go +++ b/internal/application/tcp_border_client.go @@ -5,16 +5,41 @@ package application -import "github.com/nginxinc/kubernetes-nginx-ingress/internal/core" +import ( + "fmt" + "github.com/nginxinc/kubernetes-nginx-ingress/internal/core" +) type TcpBorderClient struct { BorderClient + nginxClient NginxClientInterface } -func (client *TcpBorderClient) Update(_ core.ServerUpdateEvent) error { +func NewTcpBorderClient(client interface{}) (Interface, error) { + nginxClient, ok := client.(NginxClientInterface) + if !ok { + return nil, fmt.Errorf(`expected a NginxClientInterface, got a %v`, client) + } + + return &TcpBorderClient{ + nginxClient: nginxClient, + }, nil +} + +func (tbc *TcpBorderClient) Update(event core.ServerUpdateEvent) error { + _, _, _, err := tbc.nginxClient.UpdateHTTPServers(event.NginxHost, nil) + if err != nil { + return fmt.Errorf(`error occurred updating the nginx+ upstream server: %w`, err) + } + return nil } -func (client *TcpBorderClient) Delete(_ core.ServerUpdateEvent) error { +func (tbc *TcpBorderClient) Delete(event core.ServerUpdateEvent) error { + err := tbc.nginxClient.DeleteStreamServer(event.NginxHost, event.TcpServers[0].Server) + if err != nil { + return fmt.Errorf(`error occurred deleting the nginx+ upstream server: %w`, err) + } + return nil } diff --git a/internal/application/tcp_border_client_test.go b/internal/application/tcp_border_client_test.go index 77741b7..8f3099c 100644 --- a/internal/application/tcp_border_client_test.go +++ b/internal/application/tcp_border_client_test.go @@ -4,3 +4,77 @@ */ package application + +import ( + "testing" +) + +func TestTcpBorderClient_Delete(t *testing.T) { + event := buildServerUpdateEvent(deletedEventType) + borderClient, nginxClient, err := buildBorderClient(clientTypeTcp) + if err != nil { + t.Fatalf(`error occurred creating a new border client: %v`, err) + } + + err = borderClient.Delete(event) + if err != nil { + t.Fatalf(`error occurred deleting the nginx+ upstream server: %v`, err) + } + + if !nginxClient.CalledFunctions["DeleteStreamServer"] { + t.Fatalf(`expected DeleteStreamServer to be called`) + } +} + +func TestTcpBorderClient_Update(t *testing.T) { + event := buildServerUpdateEvent(createEventType) + borderClient, nginxClient, err := buildBorderClient(clientTypeTcp) + if err != nil { + t.Fatalf(`error occurred creating a new border client: %v`, err) + } + + err = borderClient.Update(event) + if err != nil { + t.Fatalf(`error occurred deleting the nginx+ upstream server: %v`, err) + } + + if !nginxClient.CalledFunctions["UpdateHTTPServers"] { + t.Fatalf(`expected UpdateHTTPServers to be called`) + } +} + +func TestTcpBorderClient_BadNginxClient(t *testing.T) { + var emptyInterface interface{} + _, err := NewBorderClient(clientTypeTcp, emptyInterface) + if err == nil { + t.Fatalf(`expected an error to occur when creating a new border client`) + } +} + +func TestTcpBorderClient_DeleteReturnsError(t *testing.T) { + event := buildServerUpdateEvent(deletedEventType) + borderClient, _, err := buildTerrorizingBorderClient(clientTypeTcp) + if err != nil { + t.Fatalf(`error occurred creating a new border client: %v`, err) + } + + err = borderClient.Delete(event) + + if err == nil { + t.Fatalf(`expected an error to occur when deleting the nginx+ upstream server`) + } +} + +func TestTcpBorderClient_UpdateReturnsError(t *testing.T) { + event := buildServerUpdateEvent(createEventType) + borderClient, _, err := buildTerrorizingBorderClient(clientTypeTcp) + if err != nil { + t.Fatalf(`error occurred creating a new border client: %v`, err) + } + + err = borderClient.Update(event) + + if err == nil { + t.Fatalf(`expected an error to occur when deleting the nginx+ upstream server`) + } +} From d73769396ad9a66241c28d95806da9d33524eaf4 Mon Sep 17 00:00:00 2001 From: Steve Wagner Date: Fri, 31 Mar 2023 13:36:48 -0700 Subject: [PATCH 07/12] Use pointers --- internal/application/application_common_test.go | 4 ++-- internal/application/border_client.go | 4 ++-- internal/application/http_border_client.go | 4 ++-- internal/application/tcp_border_client.go | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/internal/application/application_common_test.go b/internal/application/application_common_test.go index c55f903..769b440 100644 --- a/internal/application/application_common_test.go +++ b/internal/application/application_common_test.go @@ -35,7 +35,7 @@ func buildBorderClient(clientType string) (Interface, *mocks.MockNginxClient, er return bc, nginxClient, err } -func buildServerUpdateEvent(eventType core.EventType) core.ServerUpdateEvent { +func buildServerUpdateEvent(eventType core.EventType) *core.ServerUpdateEvent { httpServers := []nginxClient2.UpstreamServer{ { Server: server, @@ -47,5 +47,5 @@ func buildServerUpdateEvent(eventType core.EventType) core.ServerUpdateEvent { Server: server, }, } - return *core.NewServerUpdateEvent(eventType, upstreamName, tcpServers, httpServers) + return core.NewServerUpdateEvent(eventType, upstreamName, tcpServers, httpServers) } diff --git a/internal/application/border_client.go b/internal/application/border_client.go index d79beef..8520c27 100644 --- a/internal/application/border_client.go +++ b/internal/application/border_client.go @@ -12,8 +12,8 @@ import ( ) type Interface interface { - Update(core.ServerUpdateEvent) error - Delete(core.ServerUpdateEvent) error + Update(*core.ServerUpdateEvent) error + Delete(*core.ServerUpdateEvent) error } type BorderClient struct { diff --git a/internal/application/http_border_client.go b/internal/application/http_border_client.go index b1f1b04..f6df293 100644 --- a/internal/application/http_border_client.go +++ b/internal/application/http_border_client.go @@ -26,7 +26,7 @@ func NewHttpBorderClient(client interface{}) (Interface, error) { }, nil } -func (hbc *HttpBorderClient) Update(event core.ServerUpdateEvent) error { +func (hbc *HttpBorderClient) Update(event *core.ServerUpdateEvent) error { _, _, _, err := hbc.nginxClient.UpdateHTTPServers(event.NginxHost, nil) if err != nil { return fmt.Errorf(`error occurred updating the nginx+ upstream server: %w`, err) @@ -35,7 +35,7 @@ func (hbc *HttpBorderClient) Update(event core.ServerUpdateEvent) error { return nil } -func (hbc *HttpBorderClient) Delete(event core.ServerUpdateEvent) error { +func (hbc *HttpBorderClient) Delete(event *core.ServerUpdateEvent) error { err := hbc.nginxClient.DeleteHTTPServer(event.NginxHost, event.HttpServers[0].Server) if err != nil { return fmt.Errorf(`error occurred deleting the nginx+ upstream server: %w`, err) diff --git a/internal/application/tcp_border_client.go b/internal/application/tcp_border_client.go index 4f8fed8..063d162 100644 --- a/internal/application/tcp_border_client.go +++ b/internal/application/tcp_border_client.go @@ -26,7 +26,7 @@ func NewTcpBorderClient(client interface{}) (Interface, error) { }, nil } -func (tbc *TcpBorderClient) Update(event core.ServerUpdateEvent) error { +func (tbc *TcpBorderClient) Update(event *core.ServerUpdateEvent) error { _, _, _, err := tbc.nginxClient.UpdateHTTPServers(event.NginxHost, nil) if err != nil { return fmt.Errorf(`error occurred updating the nginx+ upstream server: %w`, err) @@ -35,7 +35,7 @@ func (tbc *TcpBorderClient) Update(event core.ServerUpdateEvent) error { return nil } -func (tbc *TcpBorderClient) Delete(event core.ServerUpdateEvent) error { +func (tbc *TcpBorderClient) Delete(event *core.ServerUpdateEvent) error { err := tbc.nginxClient.DeleteStreamServer(event.NginxHost, event.TcpServers[0].Server) if err != nil { return fmt.Errorf(`error occurred deleting the nginx+ upstream server: %w`, err) From e00f92d0c3b7c2cfb43ce485706a6741f2308edd Mon Sep 17 00:00:00 2001 From: Steve Wagner Date: Fri, 31 Mar 2023 14:19:35 -0700 Subject: [PATCH 08/12] Synchronizer uses BorderClient --- internal/application/border_client.go | 10 ++++--- internal/application/border_client_test.go | 6 ++++- internal/application/null_border_client.go | 23 ++++++++++++++++ .../application/null_border_client_test.go | 24 +++++++++++++++++ internal/core/events.go | 3 +++ internal/synchronization/synchronizer.go | 27 +++++++++---------- 6 files changed, 74 insertions(+), 19 deletions(-) create mode 100644 internal/application/null_border_client.go create mode 100644 internal/application/null_border_client_test.go diff --git a/internal/application/border_client.go b/internal/application/border_client.go index 8520c27..ed971b2 100644 --- a/internal/application/border_client.go +++ b/internal/application/border_client.go @@ -19,10 +19,11 @@ type Interface interface { type BorderClient struct { } -func NewBorderClient(whichType string, borderClient interface{}) (Interface, error) { - logrus.Debugf(`NewBorderClient for type: %s`, whichType) +// NewBorderClient Returns a NullBorderClient if the type is unknown, this avoids panics due to nil pointer dereferences. +func NewBorderClient(clientType string, borderClient interface{}) (Interface, error) { + logrus.Debugf(`NewBorderClient for type: %s`, clientType) - switch whichType { + switch clientType { case "tcp": return NewTcpBorderClient(borderClient) @@ -30,6 +31,7 @@ func NewBorderClient(whichType string, borderClient interface{}) (Interface, err return NewHttpBorderClient(borderClient) default: - return nil, fmt.Errorf(`unknown border client type: %s`, whichType) + borderClient, _ := NewNullBorderClient() + return borderClient, fmt.Errorf(`unknown border client type: %s`, clientType) } } diff --git a/internal/application/border_client_test.go b/internal/application/border_client_test.go index c7e6149..6d5ab50 100644 --- a/internal/application/border_client_test.go +++ b/internal/application/border_client_test.go @@ -37,7 +37,7 @@ func TestBorderClient_CreatesTcpBorderClient(t *testing.T) { func TestBorderClient_UnknownClientType(t *testing.T) { unknownClientType := "unknown" borderClient := mocks.MockNginxClient{} - _, err := NewBorderClient(unknownClientType, borderClient) + client, err := NewBorderClient(unknownClientType, borderClient) if err == nil { t.Errorf(`expected error creating border client`) } @@ -45,4 +45,8 @@ func TestBorderClient_UnknownClientType(t *testing.T) { if err.Error() != `unknown border client type: unknown` { t.Errorf(`expected error to be 'unknown border client type: unknown', got: %v`, err) } + + if _, ok := client.(*NullBorderClient); !ok { + t.Errorf(`expected client to be of type NullBorderClient`) + } } diff --git a/internal/application/null_border_client.go b/internal/application/null_border_client.go new file mode 100644 index 0000000..f9a7a3a --- /dev/null +++ b/internal/application/null_border_client.go @@ -0,0 +1,23 @@ +/* + * Copyright (c) 2023 F5 Inc. All rights reserved. + * Use of this source code is governed by the Apache License that can be found in the LICENSE file. + */ + +package application + +import "github.com/nginxinc/kubernetes-nginx-ingress/internal/core" + +type NullBorderClient struct { +} + +func NewNullBorderClient() (Interface, error) { + return &NullBorderClient{}, nil +} + +func (nbc *NullBorderClient) Update(_ *core.ServerUpdateEvent) error { + return nil +} + +func (nbc *NullBorderClient) Delete(_ *core.ServerUpdateEvent) error { + return nil +} diff --git a/internal/application/null_border_client_test.go b/internal/application/null_border_client_test.go new file mode 100644 index 0000000..42e9dfb --- /dev/null +++ b/internal/application/null_border_client_test.go @@ -0,0 +1,24 @@ +/* + * Copyright (c) 2023 F5 Inc. All rights reserved. + * Use of this source code is governed by the Apache License that can be found in the LICENSE file. + */ + +package application + +import "testing" + +func TestNullBorderClient_Delete(t *testing.T) { + client := NullBorderClient{} + err := client.Delete(nil) + if err != nil { + t.Errorf(`expected no error deleting border client, got: %v`, err) + } +} + +func TestNullBorderClient_Update(t *testing.T) { + client := NullBorderClient{} + err := client.Update(nil) + if err != nil { + t.Errorf(`expected no error updating border client, got: %v`, err) + } +} diff --git a/internal/core/events.go b/internal/core/events.go index 7e3b9ee..926f4ca 100644 --- a/internal/core/events.go +++ b/internal/core/events.go @@ -26,6 +26,7 @@ type Event struct { } type ServerUpdateEvent struct { + ClientType string Id string NginxHost string Type EventType @@ -47,6 +48,7 @@ func NewEvent(eventType EventType, service *v1.Service, previousService *v1.Serv func NewServerUpdateEvent(eventType EventType, upstreamName string, tcpServers []nginxClient.StreamUpstreamServer, httpServers []nginxClient.UpstreamServer) *ServerUpdateEvent { return &ServerUpdateEvent{ + ClientType: "http", Type: eventType, UpstreamName: upstreamName, TcpServers: tcpServers, @@ -56,6 +58,7 @@ func NewServerUpdateEvent(eventType EventType, upstreamName string, tcpServers [ func ServerUpdateEventWithIdAndHost(event *ServerUpdateEvent, id string, nginxHost string) *ServerUpdateEvent { return &ServerUpdateEvent{ + ClientType: event.ClientType, Id: id, NginxHost: nginxHost, Type: event.Type, diff --git a/internal/synchronization/synchronizer.go b/internal/synchronization/synchronizer.go index 5dc8451..2b78a3d 100644 --- a/internal/synchronization/synchronizer.go +++ b/internal/synchronization/synchronizer.go @@ -7,6 +7,7 @@ package synchronization import ( "fmt" + "github.com/nginxinc/kubernetes-nginx-ingress/internal/application" "github.com/nginxinc/kubernetes-nginx-ingress/internal/communication" "github.com/nginxinc/kubernetes-nginx-ingress/internal/configuration" "github.com/nginxinc/kubernetes-nginx-ingress/internal/core" @@ -79,8 +80,8 @@ func (s *Synchronizer) ShutDown() { s.eventQueue.ShutDownWithDrain() } -func (s *Synchronizer) buildNginxPlusClient(nginxHost string) (*nginxClient.NginxClient, error) { - logrus.Debugf(`Synchronizer::buildNginxPlusClient for host: %s`, nginxHost) +func (s *Synchronizer) buildBorderClient(event *core.ServerUpdateEvent) (application.Interface, error) { + logrus.Debugf(`Synchronizer::buildBorderClient`) var err error @@ -89,12 +90,12 @@ func (s *Synchronizer) buildNginxPlusClient(nginxHost string) (*nginxClient.Ngin return nil, fmt.Errorf(`error creating HTTP client: %v`, err) } - client, err := nginxClient.NewNginxClient(httpClient, nginxHost) + ngxClient, err := nginxClient.NewNginxClient(httpClient, event.NginxHost) if err != nil { return nil, fmt.Errorf(`error creating Nginx Plus client: %v`, err) } - return client, nil + return application.NewBorderClient(event.ClientType, ngxClient) } func (s *Synchronizer) fanOutEventToHosts(event core.ServerUpdateEvents) core.ServerUpdateEvents { @@ -145,14 +146,13 @@ func (s *Synchronizer) handleCreatedUpdatedEvent(serverUpdateEvent *core.ServerU var err error - client, err := s.buildNginxPlusClient(serverUpdateEvent.NginxHost) + borderClient, err := s.buildBorderClient(serverUpdateEvent) if err != nil { - return fmt.Errorf(`error occurred building the nginx+ client: %w`, err) + return fmt.Errorf(`error occurred creating the border client: %w`, err) } - _, _, _, err = client.UpdateStreamServers(serverUpdateEvent.UpstreamName, serverUpdateEvent.TcpServers) - if err != nil { - return fmt.Errorf(`error occurred updating the nginx+ upstream servers: %w`, err) + if err = borderClient.Update(serverUpdateEvent); err != nil { + return fmt.Errorf(`error occurred updating the %s upstream servers: %w`, serverUpdateEvent.ClientType, err) } return nil @@ -163,14 +163,13 @@ func (s *Synchronizer) handleDeletedEvent(serverUpdateEvent *core.ServerUpdateEv var err error - client, err := s.buildNginxPlusClient(serverUpdateEvent.NginxHost) + borderClient, err := s.buildBorderClient(serverUpdateEvent) if err != nil { - return fmt.Errorf(`error occurred building the nginx+ client: %w`, err) + return fmt.Errorf(`error occurred creating the border client: %w`, err) } - err = client.DeleteStreamServer(serverUpdateEvent.UpstreamName, serverUpdateEvent.TcpServers[0].Server) - if err != nil { - return fmt.Errorf(`error occurred deleting the nginx+ upstream server: %w`, err) + if err = borderClient.Delete(serverUpdateEvent); err != nil { + return fmt.Errorf(`error occurred deleting the %s upstream servers: %w`, serverUpdateEvent.ClientType, err) } return nil From 9c53ad998738df769a4588c85abf24f9ae7f228b Mon Sep 17 00:00:00 2001 From: Steve Wagner Date: Fri, 31 Mar 2023 15:08:25 -0700 Subject: [PATCH 09/12] Translator extracts client type from Annotations, default when not found --- .../application/application_common_test.go | 6 +-- internal/application/application_constants.go | 11 ++++++ .../application/http_border_client_test.go | 18 ++++----- .../application/tcp_border_client_test.go | 18 ++++----- internal/core/events.go | 4 +- internal/core/events_test.go | 16 +++++--- internal/translation/translator.go | 37 +++++++++++++++---- 7 files changed, 73 insertions(+), 37 deletions(-) create mode 100644 internal/application/application_constants.go diff --git a/internal/application/application_common_test.go b/internal/application/application_common_test.go index 769b440..12ac65e 100644 --- a/internal/application/application_common_test.go +++ b/internal/application/application_common_test.go @@ -13,8 +13,6 @@ import ( ) const ( - clientTypeTcp = "tcp" - clientTypeHttp = "http" deletedEventType = core.Deleted createEventType = core.Created upstreamName = "upstreamName" @@ -35,7 +33,7 @@ func buildBorderClient(clientType string) (Interface, *mocks.MockNginxClient, er return bc, nginxClient, err } -func buildServerUpdateEvent(eventType core.EventType) *core.ServerUpdateEvent { +func buildServerUpdateEvent(eventType core.EventType, clientType string) *core.ServerUpdateEvent { httpServers := []nginxClient2.UpstreamServer{ { Server: server, @@ -47,5 +45,5 @@ func buildServerUpdateEvent(eventType core.EventType) *core.ServerUpdateEvent { Server: server, }, } - return core.NewServerUpdateEvent(eventType, upstreamName, tcpServers, httpServers) + return core.NewServerUpdateEvent(eventType, upstreamName, clientType, tcpServers, httpServers) } diff --git a/internal/application/application_constants.go b/internal/application/application_constants.go new file mode 100644 index 0000000..fa5cd42 --- /dev/null +++ b/internal/application/application_constants.go @@ -0,0 +1,11 @@ +/* + * Copyright (c) 2023 F5 Inc. All rights reserved. + * Use of this source code is governed by the Apache License that can be found in the LICENSE file. + */ + +package application + +const ( + ClientTypeTcp = "tcp" + ClientTypeHttp = "http" +) diff --git a/internal/application/http_border_client_test.go b/internal/application/http_border_client_test.go index 7164bf7..40a5fe1 100644 --- a/internal/application/http_border_client_test.go +++ b/internal/application/http_border_client_test.go @@ -10,8 +10,8 @@ import ( ) func TestHttpBorderClient_Delete(t *testing.T) { - event := buildServerUpdateEvent(deletedEventType) - borderClient, nginxClient, err := buildBorderClient(clientTypeHttp) + event := buildServerUpdateEvent(deletedEventType, ClientTypeHttp) + borderClient, nginxClient, err := buildBorderClient(ClientTypeHttp) if err != nil { t.Fatalf(`error occurred creating a new border client: %v`, err) } @@ -27,8 +27,8 @@ func TestHttpBorderClient_Delete(t *testing.T) { } func TestHttpBorderClient_Update(t *testing.T) { - event := buildServerUpdateEvent(createEventType) - borderClient, nginxClient, err := buildBorderClient(clientTypeHttp) + event := buildServerUpdateEvent(createEventType, ClientTypeHttp) + borderClient, nginxClient, err := buildBorderClient(ClientTypeHttp) if err != nil { t.Fatalf(`error occurred creating a new border client: %v`, err) } @@ -45,15 +45,15 @@ func TestHttpBorderClient_Update(t *testing.T) { func TestHttpBorderClient_BadNginxClient(t *testing.T) { var emptyInterface interface{} - _, err := NewBorderClient(clientTypeHttp, emptyInterface) + _, err := NewBorderClient(ClientTypeHttp, emptyInterface) if err == nil { t.Fatalf(`expected an error to occur when creating a new border client`) } } func TestHttpBorderClient_DeleteReturnsError(t *testing.T) { - event := buildServerUpdateEvent(deletedEventType) - borderClient, _, err := buildTerrorizingBorderClient(clientTypeHttp) + event := buildServerUpdateEvent(deletedEventType, ClientTypeHttp) + borderClient, _, err := buildTerrorizingBorderClient(ClientTypeHttp) if err != nil { t.Fatalf(`error occurred creating a new border client: %v`, err) } @@ -66,8 +66,8 @@ func TestHttpBorderClient_DeleteReturnsError(t *testing.T) { } func TestHttpBorderClient_UpdateReturnsError(t *testing.T) { - event := buildServerUpdateEvent(createEventType) - borderClient, _, err := buildTerrorizingBorderClient(clientTypeHttp) + event := buildServerUpdateEvent(createEventType, ClientTypeHttp) + borderClient, _, err := buildTerrorizingBorderClient(ClientTypeHttp) if err != nil { t.Fatalf(`error occurred creating a new border client: %v`, err) } diff --git a/internal/application/tcp_border_client_test.go b/internal/application/tcp_border_client_test.go index 8f3099c..9074ecc 100644 --- a/internal/application/tcp_border_client_test.go +++ b/internal/application/tcp_border_client_test.go @@ -10,8 +10,8 @@ import ( ) func TestTcpBorderClient_Delete(t *testing.T) { - event := buildServerUpdateEvent(deletedEventType) - borderClient, nginxClient, err := buildBorderClient(clientTypeTcp) + event := buildServerUpdateEvent(deletedEventType, ClientTypeTcp) + borderClient, nginxClient, err := buildBorderClient(ClientTypeTcp) if err != nil { t.Fatalf(`error occurred creating a new border client: %v`, err) } @@ -27,8 +27,8 @@ func TestTcpBorderClient_Delete(t *testing.T) { } func TestTcpBorderClient_Update(t *testing.T) { - event := buildServerUpdateEvent(createEventType) - borderClient, nginxClient, err := buildBorderClient(clientTypeTcp) + event := buildServerUpdateEvent(createEventType, ClientTypeTcp) + borderClient, nginxClient, err := buildBorderClient(ClientTypeTcp) if err != nil { t.Fatalf(`error occurred creating a new border client: %v`, err) } @@ -45,15 +45,15 @@ func TestTcpBorderClient_Update(t *testing.T) { func TestTcpBorderClient_BadNginxClient(t *testing.T) { var emptyInterface interface{} - _, err := NewBorderClient(clientTypeTcp, emptyInterface) + _, err := NewBorderClient(ClientTypeTcp, emptyInterface) if err == nil { t.Fatalf(`expected an error to occur when creating a new border client`) } } func TestTcpBorderClient_DeleteReturnsError(t *testing.T) { - event := buildServerUpdateEvent(deletedEventType) - borderClient, _, err := buildTerrorizingBorderClient(clientTypeTcp) + event := buildServerUpdateEvent(deletedEventType, ClientTypeTcp) + borderClient, _, err := buildTerrorizingBorderClient(ClientTypeTcp) if err != nil { t.Fatalf(`error occurred creating a new border client: %v`, err) } @@ -66,8 +66,8 @@ func TestTcpBorderClient_DeleteReturnsError(t *testing.T) { } func TestTcpBorderClient_UpdateReturnsError(t *testing.T) { - event := buildServerUpdateEvent(createEventType) - borderClient, _, err := buildTerrorizingBorderClient(clientTypeTcp) + event := buildServerUpdateEvent(createEventType, ClientTypeTcp) + borderClient, _, err := buildTerrorizingBorderClient(ClientTypeTcp) if err != nil { t.Fatalf(`error occurred creating a new border client: %v`, err) } diff --git a/internal/core/events.go b/internal/core/events.go index 926f4ca..86b7bf8 100644 --- a/internal/core/events.go +++ b/internal/core/events.go @@ -46,9 +46,9 @@ func NewEvent(eventType EventType, service *v1.Service, previousService *v1.Serv } } -func NewServerUpdateEvent(eventType EventType, upstreamName string, tcpServers []nginxClient.StreamUpstreamServer, httpServers []nginxClient.UpstreamServer) *ServerUpdateEvent { +func NewServerUpdateEvent(eventType EventType, upstreamName string, clientType string, tcpServers []nginxClient.StreamUpstreamServer, httpServers []nginxClient.UpstreamServer) *ServerUpdateEvent { return &ServerUpdateEvent{ - ClientType: "http", + ClientType: clientType, Type: eventType, UpstreamName: upstreamName, TcpServers: tcpServers, diff --git a/internal/core/events_test.go b/internal/core/events_test.go index c808e99..360abe7 100644 --- a/internal/core/events_test.go +++ b/internal/core/events_test.go @@ -10,11 +10,13 @@ import ( "testing" ) +const clientType = "clientType" + var emptyStreamServers []nginxClient.StreamUpstreamServer var emptyHttpServers []nginxClient.UpstreamServer func TestServerUpdateEventWithIdAndHost(t *testing.T) { - event := NewServerUpdateEvent(Created, "upstream", emptyStreamServers, emptyHttpServers) + event := NewServerUpdateEvent(Created, "upstream", clientType, emptyStreamServers, emptyHttpServers) if event.Id != "" { t.Errorf("expected empty Id, got %s", event.Id) @@ -33,10 +35,14 @@ func TestServerUpdateEventWithIdAndHost(t *testing.T) { if eventWithIdAndHost.NginxHost != "host" { t.Errorf("expected NginxHost to be 'host', got %s", eventWithIdAndHost.NginxHost) } + + if eventWithIdAndHost.ClientType != clientType { + t.Errorf("expected ClientType to be '%s', got %s", clientType, eventWithIdAndHost.ClientType) + } } func TestTypeNameCreated(t *testing.T) { - event := NewServerUpdateEvent(Created, "upstream", emptyStreamServers, emptyHttpServers) + event := NewServerUpdateEvent(Created, "upstream", clientType, emptyStreamServers, emptyHttpServers) if event.TypeName() != "Created" { t.Errorf("expected 'Created', got %s", event.TypeName()) @@ -44,7 +50,7 @@ func TestTypeNameCreated(t *testing.T) { } func TestTypeNameUpdated(t *testing.T) { - event := NewServerUpdateEvent(Updated, "upstream", emptyStreamServers, emptyHttpServers) + event := NewServerUpdateEvent(Updated, "upstream", clientType, emptyStreamServers, emptyHttpServers) if event.TypeName() != "Updated" { t.Errorf("expected 'Updated', got %s", event.TypeName()) @@ -52,7 +58,7 @@ func TestTypeNameUpdated(t *testing.T) { } func TestTypeNameDeleted(t *testing.T) { - event := NewServerUpdateEvent(Deleted, "upstream", emptyStreamServers, emptyHttpServers) + event := NewServerUpdateEvent(Deleted, "upstream", clientType, emptyStreamServers, emptyHttpServers) if event.TypeName() != "Deleted" { t.Errorf("expected 'Deleted', got %s", event.TypeName()) @@ -60,7 +66,7 @@ func TestTypeNameDeleted(t *testing.T) { } func TestTypeNameUnknown(t *testing.T) { - event := NewServerUpdateEvent(EventType(100), "upstream", emptyStreamServers, emptyHttpServers) + event := NewServerUpdateEvent(EventType(100), "upstream", clientType, emptyStreamServers, emptyHttpServers) if event.TypeName() != "Unknown" { t.Errorf("expected 'Unknown', got %s", event.TypeName()) diff --git a/internal/translation/translator.go b/internal/translation/translator.go index 13f7c4a..82dc07c 100644 --- a/internal/translation/translator.go +++ b/internal/translation/translator.go @@ -47,21 +47,23 @@ func buildServerUpdateEvents(ports []v1.ServicePort, event *core.Event) (core.Se ingressName := fixIngressName(port.Name) tcpServers, _ := buildTcpServers(event.NodeIps, port) httpServers, _ := buildHttpServers(event.NodeIps, port) + clientType := getClientType(port.Name, event.Service.Annotations) switch event.Type { case core.Created: fallthrough case core.Updated: - events = append(events, core.NewServerUpdateEvent(event.Type, ingressName, tcpServers, httpServers)) + events = append(events, core.NewServerUpdateEvent(event.Type, ingressName, clientType, tcpServers, httpServers)) - case core.Deleted: // TODO: SW: This will be interesting, need to distinguish between a TCP and HTTP target + case core.Deleted: for _, server := range tcpServers { - events = append(events, core.NewServerUpdateEvent(event.Type, ingressName, []nginxClient.StreamUpstreamServer{server}, httpServers)) - } - for _, server := range httpServers { - events = append(events, core.NewServerUpdateEvent(event.Type, ingressName, tcpServers, []nginxClient.UpstreamServer{server})) + events = append(events, core.NewServerUpdateEvent(event.Type, ingressName, clientType, []nginxClient.StreamUpstreamServer{server}, httpServers)) } + // TODO: SW: This will be interesting, need to distinguish between a TCP and HTTP target + //for _, server := range httpServers { + // events = append(events, core.NewServerUpdateEvent(event.Type, ingressName, clientType, tcpServers, []nginxClient.UpstreamServer{server})) + //} default: logrus.Warnf(`Translator::buildServerUpdateEvents: unknown event type: %d`, event.Type) @@ -72,8 +74,17 @@ func buildServerUpdateEvents(ports []v1.ServicePort, event *core.Event) (core.Se return events, nil } -func buildHttpServers(_ []string, _ v1.ServicePort) ([]nginxClient.UpstreamServer, error) { - return []nginxClient.UpstreamServer{}, nil +func buildHttpServers(nodeIps []string, port v1.ServicePort) ([]nginxClient.UpstreamServer, error) { + var servers []nginxClient.UpstreamServer + + for _, nodeIp := range nodeIps { + server := nginxClient.UpstreamServer{ + Server: fmt.Sprintf("%s:%d", nodeIp, port.NodePort), + } + servers = append(servers, server) + } + + return servers, nil } func buildTcpServers(nodeIps []string, port v1.ServicePort) ([]nginxClient.StreamUpstreamServer, error) { @@ -92,3 +103,13 @@ func buildTcpServers(nodeIps []string, port v1.ServicePort) ([]nginxClient.Strea func fixIngressName(name string) string { return name[4:] } + +func getClientType(portName string, annotations map[string]string) string { + if annotations != nil { + if clientType, ok := annotations[portName]; ok { + return clientType + } + } + + return "http" +} From 6734ad9bb3edbd0be5d919ea5b9bd7a5320bb54c Mon Sep 17 00:00:00 2001 From: Steve Wagner Date: Fri, 31 Mar 2023 15:27:45 -0700 Subject: [PATCH 10/12] Copy Paste issue, TCP Border Client need to use the *Stream methods --- internal/application/tcp_border_client.go | 2 +- internal/application/tcp_border_client_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/application/tcp_border_client.go b/internal/application/tcp_border_client.go index 063d162..db023c4 100644 --- a/internal/application/tcp_border_client.go +++ b/internal/application/tcp_border_client.go @@ -27,7 +27,7 @@ func NewTcpBorderClient(client interface{}) (Interface, error) { } func (tbc *TcpBorderClient) Update(event *core.ServerUpdateEvent) error { - _, _, _, err := tbc.nginxClient.UpdateHTTPServers(event.NginxHost, nil) + _, _, _, err := tbc.nginxClient.UpdateStreamServers(event.NginxHost, nil) if err != nil { return fmt.Errorf(`error occurred updating the nginx+ upstream server: %w`, err) } diff --git a/internal/application/tcp_border_client_test.go b/internal/application/tcp_border_client_test.go index 9074ecc..69087e5 100644 --- a/internal/application/tcp_border_client_test.go +++ b/internal/application/tcp_border_client_test.go @@ -38,8 +38,8 @@ func TestTcpBorderClient_Update(t *testing.T) { t.Fatalf(`error occurred deleting the nginx+ upstream server: %v`, err) } - if !nginxClient.CalledFunctions["UpdateHTTPServers"] { - t.Fatalf(`expected UpdateHTTPServers to be called`) + if !nginxClient.CalledFunctions["UpdateStreamServers"] { + t.Fatalf(`expected UpdateStreamServers to be called`) } } From b3370181d81cfa4cfa69f9d7dbc0a9324f3183c7 Mon Sep 17 00:00:00 2001 From: Steve Wagner Date: Wed, 5 Apr 2023 15:43:06 -0700 Subject: [PATCH 11/12] Fix stupid mistake in the refactor --- internal/application/http_border_client.go | 4 ++-- internal/application/null_border_client.go | 7 ++++++- internal/application/tcp_border_client.go | 4 ++-- internal/configuration/settings.go | 7 ++++--- internal/translation/translator.go | 20 +++++++++++++------- 5 files changed, 27 insertions(+), 15 deletions(-) diff --git a/internal/application/http_border_client.go b/internal/application/http_border_client.go index f6df293..1c6397d 100644 --- a/internal/application/http_border_client.go +++ b/internal/application/http_border_client.go @@ -27,7 +27,7 @@ func NewHttpBorderClient(client interface{}) (Interface, error) { } func (hbc *HttpBorderClient) Update(event *core.ServerUpdateEvent) error { - _, _, _, err := hbc.nginxClient.UpdateHTTPServers(event.NginxHost, nil) + _, _, _, err := hbc.nginxClient.UpdateHTTPServers(event.UpstreamName, event.HttpServers) if err != nil { return fmt.Errorf(`error occurred updating the nginx+ upstream server: %w`, err) } @@ -36,7 +36,7 @@ func (hbc *HttpBorderClient) Update(event *core.ServerUpdateEvent) error { } func (hbc *HttpBorderClient) Delete(event *core.ServerUpdateEvent) error { - err := hbc.nginxClient.DeleteHTTPServer(event.NginxHost, event.HttpServers[0].Server) + err := hbc.nginxClient.DeleteHTTPServer(event.UpstreamName, event.HttpServers[0].Server) if err != nil { return fmt.Errorf(`error occurred deleting the nginx+ upstream server: %w`, err) } diff --git a/internal/application/null_border_client.go b/internal/application/null_border_client.go index f9a7a3a..f118068 100644 --- a/internal/application/null_border_client.go +++ b/internal/application/null_border_client.go @@ -5,7 +5,10 @@ package application -import "github.com/nginxinc/kubernetes-nginx-ingress/internal/core" +import ( + "github.com/nginxinc/kubernetes-nginx-ingress/internal/core" + "github.com/sirupsen/logrus" +) type NullBorderClient struct { } @@ -15,9 +18,11 @@ func NewNullBorderClient() (Interface, error) { } func (nbc *NullBorderClient) Update(_ *core.ServerUpdateEvent) error { + logrus.Warn("NullBorderClient.Update called") return nil } func (nbc *NullBorderClient) Delete(_ *core.ServerUpdateEvent) error { + logrus.Warn("NullBorderClient.Delete called") return nil } diff --git a/internal/application/tcp_border_client.go b/internal/application/tcp_border_client.go index db023c4..cc4c173 100644 --- a/internal/application/tcp_border_client.go +++ b/internal/application/tcp_border_client.go @@ -27,7 +27,7 @@ func NewTcpBorderClient(client interface{}) (Interface, error) { } func (tbc *TcpBorderClient) Update(event *core.ServerUpdateEvent) error { - _, _, _, err := tbc.nginxClient.UpdateStreamServers(event.NginxHost, nil) + _, _, _, err := tbc.nginxClient.UpdateStreamServers(event.UpstreamName, event.TcpServers) if err != nil { return fmt.Errorf(`error occurred updating the nginx+ upstream server: %w`, err) } @@ -36,7 +36,7 @@ func (tbc *TcpBorderClient) Update(event *core.ServerUpdateEvent) error { } func (tbc *TcpBorderClient) Delete(event *core.ServerUpdateEvent) error { - err := tbc.nginxClient.DeleteStreamServer(event.NginxHost, event.TcpServers[0].Server) + err := tbc.nginxClient.DeleteStreamServer(event.UpstreamName, event.TcpServers[0].Server) if err != nil { return fmt.Errorf(`error occurred deleting the nginx+ upstream server: %w`, err) } diff --git a/internal/configuration/settings.go b/internal/configuration/settings.go index bc1dc11..c394232 100644 --- a/internal/configuration/settings.go +++ b/internal/configuration/settings.go @@ -19,9 +19,10 @@ import ( ) const ( - ConfigMapsNamespace = "nkl" - ResyncPeriod = 0 - NklPrefix = ConfigMapsNamespace + "-" + ConfigMapsNamespace = "nkl" + ResyncPeriod = 0 + NklPrefix = ConfigMapsNamespace + "-" + PortAnnotationPrefix = "nginxinc.io" ) type WorkQueueSettings struct { diff --git a/internal/translation/translator.go b/internal/translation/translator.go index 82dc07c..7d08509 100644 --- a/internal/translation/translator.go +++ b/internal/translation/translator.go @@ -57,13 +57,17 @@ func buildServerUpdateEvents(ports []v1.ServicePort, event *core.Event) (core.Se events = append(events, core.NewServerUpdateEvent(event.Type, ingressName, clientType, tcpServers, httpServers)) case core.Deleted: - for _, server := range tcpServers { - events = append(events, core.NewServerUpdateEvent(event.Type, ingressName, clientType, []nginxClient.StreamUpstreamServer{server}, httpServers)) + // SW: This is kind of icky, look at making this better + switch clientType { + case "http": + for _, server := range httpServers { + events = append(events, core.NewServerUpdateEvent(event.Type, ingressName, clientType, tcpServers, []nginxClient.UpstreamServer{server})) + } + case "tcp": + for _, server := range tcpServers { + events = append(events, core.NewServerUpdateEvent(event.Type, ingressName, clientType, []nginxClient.StreamUpstreamServer{server}, httpServers)) + } } - // TODO: SW: This will be interesting, need to distinguish between a TCP and HTTP target - //for _, server := range httpServers { - // events = append(events, core.NewServerUpdateEvent(event.Type, ingressName, clientType, tcpServers, []nginxClient.UpstreamServer{server})) - //} default: logrus.Warnf(`Translator::buildServerUpdateEvents: unknown event type: %d`, event.Type) @@ -105,8 +109,10 @@ func fixIngressName(name string) string { } func getClientType(portName string, annotations map[string]string) string { + key := fmt.Sprintf("%s/%s", configuration.PortAnnotationPrefix, portName) + logrus.Infof("getClientType: key=%s", key) if annotations != nil { - if clientType, ok := annotations[portName]; ok { + if clientType, ok := annotations[key]; ok { return clientType } } From 3e026c70bd37059312384731526fde75395a18ae Mon Sep 17 00:00:00 2001 From: Steve Wagner Date: Thu, 6 Apr 2023 15:55:19 -0700 Subject: [PATCH 12/12] Put an interface between NKL and the NGINX Plus client Upstream objects --- .../application/application_common_test.go | 12 +-- internal/application/http_border_client.go | 26 +++++- internal/application/tcp_border_client.go | 26 +++++- internal/core/event.go | 32 ++++++++ internal/core/events.go | 82 ------------------- internal/core/server_update_event.go | 50 +++++++++++ ...ts_test.go => server_update_event_test.go} | 14 ++-- internal/core/upstream_server.go | 18 ++++ internal/synchronization/synchronizer_test.go | 20 ++--- internal/translation/translator.go | 40 ++------- internal/translation/translator_test.go | 2 +- 11 files changed, 172 insertions(+), 150 deletions(-) create mode 100644 internal/core/event.go delete mode 100644 internal/core/events.go create mode 100644 internal/core/server_update_event.go rename internal/core/{events_test.go => server_update_event_test.go} (83%) create mode 100644 internal/core/upstream_server.go diff --git a/internal/application/application_common_test.go b/internal/application/application_common_test.go index 12ac65e..e963d03 100644 --- a/internal/application/application_common_test.go +++ b/internal/application/application_common_test.go @@ -9,7 +9,6 @@ import ( "errors" "github.com/nginxinc/kubernetes-nginx-ingress/internal/core" "github.com/nginxinc/kubernetes-nginx-ingress/test/mocks" - nginxClient2 "github.com/nginxinc/nginx-plus-go-client/client" ) const ( @@ -34,16 +33,11 @@ func buildBorderClient(clientType string) (Interface, *mocks.MockNginxClient, er } func buildServerUpdateEvent(eventType core.EventType, clientType string) *core.ServerUpdateEvent { - httpServers := []nginxClient2.UpstreamServer{ + upstreamServers := core.UpstreamServers{ { - Server: server, + Host: server, }, } - tcpServers := []nginxClient2.StreamUpstreamServer{ - { - Server: server, - }, - } - return core.NewServerUpdateEvent(eventType, upstreamName, clientType, tcpServers, httpServers) + return core.NewServerUpdateEvent(eventType, upstreamName, clientType, upstreamServers) } diff --git a/internal/application/http_border_client.go b/internal/application/http_border_client.go index 1c6397d..dc7d00a 100644 --- a/internal/application/http_border_client.go +++ b/internal/application/http_border_client.go @@ -8,6 +8,7 @@ package application import ( "fmt" "github.com/nginxinc/kubernetes-nginx-ingress/internal/core" + nginxClient "github.com/nginxinc/nginx-plus-go-client/client" ) type HttpBorderClient struct { @@ -16,18 +17,19 @@ type HttpBorderClient struct { } func NewHttpBorderClient(client interface{}) (Interface, error) { - nginxClient, ok := client.(NginxClientInterface) + ngxClient, ok := client.(NginxClientInterface) if !ok { return nil, fmt.Errorf(`expected a NginxClientInterface, got a %v`, client) } return &HttpBorderClient{ - nginxClient: nginxClient, + nginxClient: ngxClient, }, nil } func (hbc *HttpBorderClient) Update(event *core.ServerUpdateEvent) error { - _, _, _, err := hbc.nginxClient.UpdateHTTPServers(event.UpstreamName, event.HttpServers) + httpUpstreamServers := asNginxHttpUpstreamServers(event.UpstreamServers) + _, _, _, err := hbc.nginxClient.UpdateHTTPServers(event.UpstreamName, httpUpstreamServers) if err != nil { return fmt.Errorf(`error occurred updating the nginx+ upstream server: %w`, err) } @@ -36,10 +38,26 @@ func (hbc *HttpBorderClient) Update(event *core.ServerUpdateEvent) error { } func (hbc *HttpBorderClient) Delete(event *core.ServerUpdateEvent) error { - err := hbc.nginxClient.DeleteHTTPServer(event.UpstreamName, event.HttpServers[0].Server) + err := hbc.nginxClient.DeleteHTTPServer(event.UpstreamName, event.UpstreamServers[0].Host) if err != nil { return fmt.Errorf(`error occurred deleting the nginx+ upstream server: %w`, err) } return nil } + +func asNginxHttpUpstreamServer(server *core.UpstreamServer) nginxClient.UpstreamServer { + return nginxClient.UpstreamServer{ + Server: server.Host, + } +} + +func asNginxHttpUpstreamServers(servers core.UpstreamServers) []nginxClient.UpstreamServer { + var upstreamServers []nginxClient.UpstreamServer + + for _, server := range servers { + upstreamServers = append(upstreamServers, asNginxHttpUpstreamServer(server)) + } + + return upstreamServers +} diff --git a/internal/application/tcp_border_client.go b/internal/application/tcp_border_client.go index cc4c173..bef0bc6 100644 --- a/internal/application/tcp_border_client.go +++ b/internal/application/tcp_border_client.go @@ -8,6 +8,7 @@ package application import ( "fmt" "github.com/nginxinc/kubernetes-nginx-ingress/internal/core" + nginxClient "github.com/nginxinc/nginx-plus-go-client/client" ) type TcpBorderClient struct { @@ -16,18 +17,19 @@ type TcpBorderClient struct { } func NewTcpBorderClient(client interface{}) (Interface, error) { - nginxClient, ok := client.(NginxClientInterface) + ngxClient, ok := client.(NginxClientInterface) if !ok { return nil, fmt.Errorf(`expected a NginxClientInterface, got a %v`, client) } return &TcpBorderClient{ - nginxClient: nginxClient, + nginxClient: ngxClient, }, nil } func (tbc *TcpBorderClient) Update(event *core.ServerUpdateEvent) error { - _, _, _, err := tbc.nginxClient.UpdateStreamServers(event.UpstreamName, event.TcpServers) + streamUpstreamServers := asNginxStreamUpstreamServers(event.UpstreamServers) + _, _, _, err := tbc.nginxClient.UpdateStreamServers(event.UpstreamName, streamUpstreamServers) if err != nil { return fmt.Errorf(`error occurred updating the nginx+ upstream server: %w`, err) } @@ -36,10 +38,26 @@ func (tbc *TcpBorderClient) Update(event *core.ServerUpdateEvent) error { } func (tbc *TcpBorderClient) Delete(event *core.ServerUpdateEvent) error { - err := tbc.nginxClient.DeleteStreamServer(event.UpstreamName, event.TcpServers[0].Server) + err := tbc.nginxClient.DeleteStreamServer(event.UpstreamName, event.UpstreamServers[0].Host) if err != nil { return fmt.Errorf(`error occurred deleting the nginx+ upstream server: %w`, err) } return nil } + +func asNginxStreamUpstreamServer(server *core.UpstreamServer) nginxClient.StreamUpstreamServer { + return nginxClient.StreamUpstreamServer{ + Server: server.Host, + } +} + +func asNginxStreamUpstreamServers(servers core.UpstreamServers) []nginxClient.StreamUpstreamServer { + var upstreamServers []nginxClient.StreamUpstreamServer + + for _, server := range servers { + upstreamServers = append(upstreamServers, asNginxStreamUpstreamServer(server)) + } + + return upstreamServers +} diff --git a/internal/core/event.go b/internal/core/event.go new file mode 100644 index 0000000..b5609b1 --- /dev/null +++ b/internal/core/event.go @@ -0,0 +1,32 @@ +/* + * Copyright (c) 2023 F5 Inc. All rights reserved. + * Use of this source code is governed by the Apache License that can be found in the LICENSE file. + */ + +package core + +import v1 "k8s.io/api/core/v1" + +type EventType int + +const ( + Created EventType = iota + Updated + Deleted +) + +type Event struct { + Type EventType + Service *v1.Service + PreviousService *v1.Service + NodeIps []string +} + +func NewEvent(eventType EventType, service *v1.Service, previousService *v1.Service, nodeIps []string) Event { + return Event{ + Type: eventType, + Service: service, + PreviousService: previousService, + NodeIps: nodeIps, + } +} diff --git a/internal/core/events.go b/internal/core/events.go deleted file mode 100644 index 86b7bf8..0000000 --- a/internal/core/events.go +++ /dev/null @@ -1,82 +0,0 @@ -/* - * Copyright 2023 F5 Inc. All rights reserved. - * Use of this source code is governed by the Apache License that can be found in the LICENSE file. - */ - -package core - -import ( - nginxClient "github.com/nginxinc/nginx-plus-go-client/client" - v1 "k8s.io/api/core/v1" -) - -type EventType int - -const ( - Created EventType = iota - Updated - Deleted -) - -type Event struct { - Type EventType - Service *v1.Service - PreviousService *v1.Service - NodeIps []string -} - -type ServerUpdateEvent struct { - ClientType string - Id string - NginxHost string - Type EventType - UpstreamName string - TcpServers []nginxClient.StreamUpstreamServer - HttpServers []nginxClient.UpstreamServer -} - -type ServerUpdateEvents = []*ServerUpdateEvent - -func NewEvent(eventType EventType, service *v1.Service, previousService *v1.Service, nodeIps []string) Event { - return Event{ - Type: eventType, - Service: service, - PreviousService: previousService, - NodeIps: nodeIps, - } -} - -func NewServerUpdateEvent(eventType EventType, upstreamName string, clientType string, tcpServers []nginxClient.StreamUpstreamServer, httpServers []nginxClient.UpstreamServer) *ServerUpdateEvent { - return &ServerUpdateEvent{ - ClientType: clientType, - Type: eventType, - UpstreamName: upstreamName, - TcpServers: tcpServers, - HttpServers: httpServers, - } -} - -func ServerUpdateEventWithIdAndHost(event *ServerUpdateEvent, id string, nginxHost string) *ServerUpdateEvent { - return &ServerUpdateEvent{ - ClientType: event.ClientType, - Id: id, - NginxHost: nginxHost, - Type: event.Type, - UpstreamName: event.UpstreamName, - TcpServers: event.TcpServers, - HttpServers: event.HttpServers, - } -} - -func (e *ServerUpdateEvent) TypeName() string { - switch e.Type { - case Created: - return "Created" - case Updated: - return "Updated" - case Deleted: - return "Deleted" - default: - return "Unknown" - } -} diff --git a/internal/core/server_update_event.go b/internal/core/server_update_event.go new file mode 100644 index 0000000..dd02127 --- /dev/null +++ b/internal/core/server_update_event.go @@ -0,0 +1,50 @@ +/* + * Copyright 2023 F5 Inc. All rights reserved. + * Use of this source code is governed by the Apache License that can be found in the LICENSE file. + */ + +package core + +type ServerUpdateEvent struct { + ClientType string + Id string + NginxHost string + Type EventType + UpstreamName string + UpstreamServers UpstreamServers +} + +type ServerUpdateEvents = []*ServerUpdateEvent + +func NewServerUpdateEvent(eventType EventType, upstreamName string, clientType string, upstreamServers UpstreamServers) *ServerUpdateEvent { + return &ServerUpdateEvent{ + ClientType: clientType, + Type: eventType, + UpstreamName: upstreamName, + UpstreamServers: upstreamServers, + } +} + +func ServerUpdateEventWithIdAndHost(event *ServerUpdateEvent, id string, nginxHost string) *ServerUpdateEvent { + return &ServerUpdateEvent{ + ClientType: event.ClientType, + Id: id, + NginxHost: nginxHost, + Type: event.Type, + UpstreamName: event.UpstreamName, + UpstreamServers: event.UpstreamServers, + } +} + +func (e *ServerUpdateEvent) TypeName() string { + switch e.Type { + case Created: + return "Created" + case Updated: + return "Updated" + case Deleted: + return "Deleted" + default: + return "Unknown" + } +} diff --git a/internal/core/events_test.go b/internal/core/server_update_event_test.go similarity index 83% rename from internal/core/events_test.go rename to internal/core/server_update_event_test.go index 360abe7..a891e23 100644 --- a/internal/core/events_test.go +++ b/internal/core/server_update_event_test.go @@ -6,17 +6,15 @@ package core import ( - nginxClient "github.com/nginxinc/nginx-plus-go-client/client" "testing" ) const clientType = "clientType" -var emptyStreamServers []nginxClient.StreamUpstreamServer -var emptyHttpServers []nginxClient.UpstreamServer +var emptyUpstreamServers UpstreamServers func TestServerUpdateEventWithIdAndHost(t *testing.T) { - event := NewServerUpdateEvent(Created, "upstream", clientType, emptyStreamServers, emptyHttpServers) + event := NewServerUpdateEvent(Created, "upstream", clientType, emptyUpstreamServers) if event.Id != "" { t.Errorf("expected empty Id, got %s", event.Id) @@ -42,7 +40,7 @@ func TestServerUpdateEventWithIdAndHost(t *testing.T) { } func TestTypeNameCreated(t *testing.T) { - event := NewServerUpdateEvent(Created, "upstream", clientType, emptyStreamServers, emptyHttpServers) + event := NewServerUpdateEvent(Created, "upstream", clientType, emptyUpstreamServers) if event.TypeName() != "Created" { t.Errorf("expected 'Created', got %s", event.TypeName()) @@ -50,7 +48,7 @@ func TestTypeNameCreated(t *testing.T) { } func TestTypeNameUpdated(t *testing.T) { - event := NewServerUpdateEvent(Updated, "upstream", clientType, emptyStreamServers, emptyHttpServers) + event := NewServerUpdateEvent(Updated, "upstream", clientType, emptyUpstreamServers) if event.TypeName() != "Updated" { t.Errorf("expected 'Updated', got %s", event.TypeName()) @@ -58,7 +56,7 @@ func TestTypeNameUpdated(t *testing.T) { } func TestTypeNameDeleted(t *testing.T) { - event := NewServerUpdateEvent(Deleted, "upstream", clientType, emptyStreamServers, emptyHttpServers) + event := NewServerUpdateEvent(Deleted, "upstream", clientType, emptyUpstreamServers) if event.TypeName() != "Deleted" { t.Errorf("expected 'Deleted', got %s", event.TypeName()) @@ -66,7 +64,7 @@ func TestTypeNameDeleted(t *testing.T) { } func TestTypeNameUnknown(t *testing.T) { - event := NewServerUpdateEvent(EventType(100), "upstream", clientType, emptyStreamServers, emptyHttpServers) + event := NewServerUpdateEvent(EventType(100), "upstream", clientType, emptyUpstreamServers) if event.TypeName() != "Unknown" { t.Errorf("expected 'Unknown', got %s", event.TypeName()) diff --git a/internal/core/upstream_server.go b/internal/core/upstream_server.go new file mode 100644 index 0000000..210b001 --- /dev/null +++ b/internal/core/upstream_server.go @@ -0,0 +1,18 @@ +/* + * Copyright (c) 2023 F5 Inc. All rights reserved. + * Use of this source code is governed by the Apache License that can be found in the LICENSE file. + */ + +package core + +type UpstreamServer struct { + Host string +} + +type UpstreamServers = []*UpstreamServer + +func NewUpstreamServer(host string) *UpstreamServer { + return &UpstreamServer{ + Host: host, + } +} diff --git a/internal/synchronization/synchronizer_test.go b/internal/synchronization/synchronizer_test.go index a69b148..315def7 100644 --- a/internal/synchronization/synchronizer_test.go +++ b/internal/synchronization/synchronizer_test.go @@ -32,11 +32,11 @@ func TestSynchronizer_NewSynchronizer(t *testing.T) { func TestSynchronizer_AddEventNoHosts(t *testing.T) { const expectedEventCount = 0 event := &core.ServerUpdateEvent{ - Id: "", - NginxHost: "", - Type: 0, - UpstreamName: "", - TcpServers: nil, + Id: "", + NginxHost: "", + Type: 0, + UpstreamName: "", + UpstreamServers: nil, } settings, err := configuration.NewSettings(context.Background(), nil) rateLimiter := &mocks.MockRateLimiter{} @@ -188,11 +188,11 @@ func buildEvents(count int) core.ServerUpdateEvents { events := make(core.ServerUpdateEvents, count) for i := 0; i < count; i++ { events[i] = &core.ServerUpdateEvent{ - Id: fmt.Sprintf("id-%v", i), - NginxHost: "https://localhost:8080", - Type: 0, - UpstreamName: "", - TcpServers: nil, + Id: fmt.Sprintf("id-%v", i), + NginxHost: "https://localhost:8080", + Type: 0, + UpstreamName: "", + UpstreamServers: nil, } } return events diff --git a/internal/translation/translator.go b/internal/translation/translator.go index 7d08509..2169251 100644 --- a/internal/translation/translator.go +++ b/internal/translation/translator.go @@ -9,7 +9,6 @@ import ( "fmt" "github.com/nginxinc/kubernetes-nginx-ingress/internal/configuration" "github.com/nginxinc/kubernetes-nginx-ingress/internal/core" - nginxClient "github.com/nginxinc/nginx-plus-go-client/client" "github.com/sirupsen/logrus" v1 "k8s.io/api/core/v1" "strings" @@ -45,8 +44,7 @@ func buildServerUpdateEvents(ports []v1.ServicePort, event *core.Event) (core.Se events := core.ServerUpdateEvents{} for _, port := range ports { ingressName := fixIngressName(port.Name) - tcpServers, _ := buildTcpServers(event.NodeIps, port) - httpServers, _ := buildHttpServers(event.NodeIps, port) + upstreamServers, _ := buildUpstreamServers(event.NodeIps, port) clientType := getClientType(port.Name, event.Service.Annotations) switch event.Type { @@ -54,19 +52,11 @@ func buildServerUpdateEvents(ports []v1.ServicePort, event *core.Event) (core.Se fallthrough case core.Updated: - events = append(events, core.NewServerUpdateEvent(event.Type, ingressName, clientType, tcpServers, httpServers)) + events = append(events, core.NewServerUpdateEvent(event.Type, ingressName, clientType, upstreamServers)) case core.Deleted: - // SW: This is kind of icky, look at making this better - switch clientType { - case "http": - for _, server := range httpServers { - events = append(events, core.NewServerUpdateEvent(event.Type, ingressName, clientType, tcpServers, []nginxClient.UpstreamServer{server})) - } - case "tcp": - for _, server := range tcpServers { - events = append(events, core.NewServerUpdateEvent(event.Type, ingressName, clientType, []nginxClient.StreamUpstreamServer{server}, httpServers)) - } + for _, server := range upstreamServers { + events = append(events, core.NewServerUpdateEvent(event.Type, ingressName, clientType, core.UpstreamServers{server})) } default: @@ -78,26 +68,12 @@ func buildServerUpdateEvents(ports []v1.ServicePort, event *core.Event) (core.Se return events, nil } -func buildHttpServers(nodeIps []string, port v1.ServicePort) ([]nginxClient.UpstreamServer, error) { - var servers []nginxClient.UpstreamServer +func buildUpstreamServers(nodeIps []string, port v1.ServicePort) (core.UpstreamServers, error) { + var servers core.UpstreamServers for _, nodeIp := range nodeIps { - server := nginxClient.UpstreamServer{ - Server: fmt.Sprintf("%s:%d", nodeIp, port.NodePort), - } - servers = append(servers, server) - } - - return servers, nil -} - -func buildTcpServers(nodeIps []string, port v1.ServicePort) ([]nginxClient.StreamUpstreamServer, error) { - var servers []nginxClient.StreamUpstreamServer - - for _, nodeIp := range nodeIps { - server := nginxClient.StreamUpstreamServer{ - Server: fmt.Sprintf("%s:%d", nodeIp, port.NodePort), - } + host := fmt.Sprintf("%s:%d", nodeIp, port.NodePort) + server := core.NewUpstreamServer(host) servers = append(servers, server) } diff --git a/internal/translation/translator_test.go b/internal/translation/translator_test.go index 4409cff..4e635d6 100644 --- a/internal/translation/translator_test.go +++ b/internal/translation/translator_test.go @@ -594,7 +594,7 @@ func TestDeletedTranslateManyMixedPortsAndManyNodes(t *testing.T) { func assertExpectedServerCount(t *testing.T, expectedCount int, events core.ServerUpdateEvents) { for _, translatedEvent := range events { - serverCount := len(translatedEvent.TcpServers) + serverCount := len(translatedEvent.UpstreamServers) if serverCount != expectedCount { t.Fatalf("expected %d servers, got %d", expectedCount, serverCount) }