Skip to content

Commit

Permalink
Refactor upstream implementation in state package
Browse files Browse the repository at this point in the history
- Adds new component relationship.Capturer to capture and report on
  relationships between resources.
- Replaces ServiceStore component with a new component ServiceResolver.
- Adds the "kubernetes.io/service-name" as an index field to the EndpointSlice
  resolver. This allows for a quick lookup on EndpointSlices given a Service
name.
- Adds a ServicePortsChangedPredicate function as an event filter for Service
  events. This filters out Service updates that don't involve port changes.
Also, fixed bug where we weren't updating nginx config on a Service port update.
- Moves captureResourceChanges methods from ChangeProcessor to the store.
- Moves storeChanged boolean to the store.
- Removes helpers.getNamespacedName in favor of third party function.
- Plumbs context from EventHandler through ServiceResolver.Resolver() function.
  • Loading branch information
kate-osborn committed Sep 15, 2022
1 parent 2e0ae98 commit bda6dcf
Show file tree
Hide file tree
Showing 33 changed files with 2,085 additions and 1,176 deletions.
6 changes: 1 addition & 5 deletions internal/events/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@ type EventHandler interface {
type EventHandlerConfig struct {
// Processor is the state ChangeProcessor.
Processor state.ChangeProcessor
// ServiceStore is the state ServiceStore.
ServiceStore state.ServiceStore
// SecretStore is the state SecretStore.
SecretStore state.SecretStore
// SecretMemoryManager is the state SecretMemoryManager.
Expand Down Expand Up @@ -74,7 +72,7 @@ func (h *EventHandlerImpl) HandleEventBatch(ctx context.Context, batch EventBatc
}
}

changed, conf, statuses := h.cfg.Processor.Process()
changed, conf, statuses := h.cfg.Processor.Process(ctx)
if !changed {
h.cfg.Logger.Info("Handling events didn't result into NGINX configuration changes")
return
Expand Down Expand Up @@ -121,7 +119,6 @@ func (h *EventHandlerImpl) propagateUpsert(e *UpsertEvent) {
case *v1beta1.HTTPRoute:
h.cfg.Processor.CaptureUpsertChange(r)
case *apiv1.Service:
h.cfg.ServiceStore.Upsert(r)
h.cfg.Processor.CaptureUpsertChange(r)
case *apiv1.Secret:
// FIXME(kate-osborn): need to handle certificate rotation
Expand All @@ -142,7 +139,6 @@ func (h *EventHandlerImpl) propagateDelete(e *DeleteEvent) {
case *v1beta1.HTTPRoute:
h.cfg.Processor.CaptureDeleteChange(e.Type, e.NamespacedName)
case *apiv1.Service:
h.cfg.ServiceStore.Delete(e.NamespacedName)
h.cfg.Processor.CaptureDeleteChange(e.Type, e.NamespacedName)
case *apiv1.Secret:
// FIXME(kate-osborn): make sure that affected servers are updated
Expand Down
10 changes: 0 additions & 10 deletions internal/events/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ var _ = Describe("EventHandler", func() {
var (
handler *events.EventHandlerImpl
fakeProcessor *statefakes.FakeChangeProcessor
fakeServiceStore *statefakes.FakeServiceStore
fakeSecretStore *statefakes.FakeSecretStore
fakeSecretMemoryManager *statefakes.FakeSecretDiskMemoryManager
fakeGenerator *configfakes.FakeGenerator
Expand Down Expand Up @@ -69,7 +68,6 @@ var _ = Describe("EventHandler", func() {

BeforeEach(func() {
fakeProcessor = &statefakes.FakeChangeProcessor{}
fakeServiceStore = &statefakes.FakeServiceStore{}
fakeSecretMemoryManager = &statefakes.FakeSecretDiskMemoryManager{}
fakeSecretStore = &statefakes.FakeSecretStore{}
fakeGenerator = &configfakes.FakeGenerator{}
Expand All @@ -79,7 +77,6 @@ var _ = Describe("EventHandler", func() {

handler = events.NewEventHandlerImpl(events.EventHandlerConfig{
Processor: fakeProcessor,
ServiceStore: fakeServiceStore,
SecretStore: fakeSecretStore,
SecretMemoryManager: fakeSecretMemoryManager,
Generator: fakeGenerator,
Expand Down Expand Up @@ -234,13 +231,6 @@ var _ = Describe("EventHandler", func() {
Expect(passedNsName).Should(Equal(d.NamespacedName))
}

// Check Service-related expectations
Expect(fakeServiceStore.UpsertCallCount()).Should(Equal(1))
Expect(fakeServiceStore.UpsertArgsForCall(0)).Should(Equal(svc))

Expect(fakeServiceStore.DeleteCallCount()).Should(Equal(1))
Expect(fakeServiceStore.DeleteArgsForCall(0)).Should(Equal(svcNsName))

// Check Secret-related expectations
Expect(fakeSecretStore.UpsertCallCount()).Should(Equal(1))
Expect(fakeSecretStore.UpsertArgsForCall(0)).Should(Equal(secret))
Expand Down
5 changes: 2 additions & 3 deletions internal/manager/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"github.com/nginxinc/nginx-kubernetes-gateway/internal/nginx/file"
ngxruntime "github.com/nginxinc/nginx-kubernetes-gateway/internal/nginx/runtime"
"github.com/nginxinc/nginx-kubernetes-gateway/internal/state"
"github.com/nginxinc/nginx-kubernetes-gateway/internal/state/resolver"
"github.com/nginxinc/nginx-kubernetes-gateway/internal/status"
"github.com/nginxinc/nginx-kubernetes-gateway/pkg/sdk"
)
Expand Down Expand Up @@ -91,12 +92,11 @@ func Start(cfg config.Config) error {
secretStore := state.NewSecretStore()
secretMemoryMgr := state.NewSecretDiskMemoryManager(secretsFolder, secretStore)

serviceStore := state.NewServiceStore(mgr.GetClient())
processor := state.NewChangeProcessorImpl(state.ChangeProcessorConfig{
GatewayCtlrName: cfg.GatewayCtlrName,
GatewayClassName: cfg.GatewayClassName,
SecretMemoryManager: secretMemoryMgr,
ServiceStore: serviceStore,
ServiceResolver: resolver.NewServiceResolverImpl(mgr.GetClient()),
Logger: cfg.Logger.WithName("changeProcessor"),
})

Expand All @@ -116,7 +116,6 @@ func Start(cfg config.Config) error {

eventHandler := events.NewEventHandlerImpl(events.EventHandlerConfig{
Processor: processor,
ServiceStore: serviceStore,
SecretStore: secretStore,
SecretMemoryManager: secretMemoryMgr,
Generator: configGenerator,
Expand Down
11 changes: 6 additions & 5 deletions internal/nginx/config/generator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

"github.com/nginxinc/nginx-kubernetes-gateway/internal/helpers"
"github.com/nginxinc/nginx-kubernetes-gateway/internal/state"
"github.com/nginxinc/nginx-kubernetes-gateway/internal/state/resolver"
)

func TestGenerate(t *testing.T) {
Expand Down Expand Up @@ -386,7 +387,7 @@ func TestGenerateHTTPUpstreams(t *testing.T) {
stateUpstreams := []state.Upstream{
{
Name: "up1",
Endpoints: []state.Endpoint{
Endpoints: []resolver.Endpoint{
{
Address: "10.0.0.0",
Port: 80,
Expand All @@ -403,7 +404,7 @@ func TestGenerateHTTPUpstreams(t *testing.T) {
},
{
Name: "up2",
Endpoints: []state.Endpoint{
Endpoints: []resolver.Endpoint{
{
Address: "11.0.0.0",
Port: 80,
Expand All @@ -412,7 +413,7 @@ func TestGenerateHTTPUpstreams(t *testing.T) {
},
{
Name: "up3",
Endpoints: []state.Endpoint{},
Endpoints: []resolver.Endpoint{},
},
}

Expand Down Expand Up @@ -482,15 +483,15 @@ func TestGenerateUpstream(t *testing.T) {
{
stateUpstream: state.Upstream{
Name: "no-endpoints",
Endpoints: []state.Endpoint{},
Endpoints: []resolver.Endpoint{},
},
expectedUpstream: upstream{Name: "no-endpoints", Servers: []upstreamServer{{Address: nginx502Server}}},
msg: "no endpoints",
},
{
stateUpstream: state.Upstream{
Name: "multiple-endpoints",
Endpoints: []state.Endpoint{
Endpoints: []resolver.Endpoint{
{
Address: "10.0.0.1",
Port: 80,
Expand Down
6 changes: 3 additions & 3 deletions internal/nginx/config/upstreams_template.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ package config
var httpUpstreamsTemplate = `{{ range $u := .Upstreams }}
upstream {{ $u.Name }} {
random two least_conn;
{{ range $server := $u.Servers }}
server {{ $server.Address }};
{{ end }}
{{ range $server := $u.Servers }}
server {{ $server.Address }};
{{ end }}
}
{{ end }}`
Loading

0 comments on commit bda6dcf

Please sign in to comment.