Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set Gateway Programmed condition #658

Merged
merged 4 commits into from
Jun 1, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 39 additions & 35 deletions docs/gateway-api-compatibility.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,11 @@ of the [static-mode](./cli-help.md#static-mode) command.

Fields:
* `spec`
* `controllerName` - supported.
* `parametersRef` - not supported.
* `description` - supported.
* `controllerName` - supported.
* `parametersRef` - not supported.
* `description` - supported.
* `status`
* `conditions` - partially supported.
* `conditions` - partially supported.

### Gateway

Expand All @@ -54,18 +54,18 @@ See [static-mode](./cli-help.md#static-mode) command for more info.

Fields:
* `spec`
* `gatewayClassName` - supported.
* `listeners`
* `name` - supported.
* `hostname` - partially supported. Wildcard hostnames like `*.example.com` are not yet supported.
* `port` - partially supported. Allowed values: `80` for HTTP listeners and `443` for HTTPS listeners.
* `protocol` - partially supported. Allowed values: `HTTP`, `HTTPS`.
* `tls`
* `mode` - partially supported. Allowed value: `Terminate`.
* `certificateRefs` - partially supported. The TLS certificate and key must be stored in a Secret resource of type `kubernetes.io/tls` in the same namespace as the Gateway resource. Only a single reference is supported. You must deploy the Secret before the Gateway resource. Secret rotation (watching for updates) is not supported.
* `options` - not supported.
* `allowedRoutes` - not supported.
* `addresses` - not supported.
* `gatewayClassName` - supported.
* `listeners`
* `name` - supported.
* `hostname` - partially supported. Wildcard hostnames like `*.example.com` are not yet supported.
* `port` - partially supported. Allowed values: `80` for HTTP listeners and `443` for HTTPS listeners.
* `protocol` - partially supported. Allowed values: `HTTP`, `HTTPS`.
* `tls`
* `mode` - partially supported. Allowed value: `Terminate`.
* `certificateRefs` - partially supported. The TLS certificate and key must be stored in a Secret resource of type `kubernetes.io/tls` in the same namespace as the Gateway resource. Only a single reference is supported. You must deploy the Secret before the Gateway resource. Secret rotation (watching for updates) is not supported.
* `options` - not supported.
* `allowedRoutes` - not supported.
* `addresses` - not supported.
* `status`
* `addresses` - Pod IPAddress supported.
* `conditions` - Supported (Condition/Status/Reason):
Expand All @@ -75,11 +75,14 @@ Fields:
* `Accepted/False/Invalid`
* `Accepted/False/UnsupportedValue`: Custom reason for when a value of a field in a Gateway is invalid or not supported.
* `Accepted/False/GatewayConflict`: Custom reason for when the Gateway is ignored due to a conflicting Gateway. NKG only supports a single Gateway.
* `Programmed/True/Programmed`
* `Programmed/False/Invalid`
* `Programmed/False/GatewayConflict`: Custom reason for when the Gateway is ignored due to a conflicting Gateway. NKG only supports a single Gateway.
* `listeners`
* `name` - supported.
* `supportedKinds` - not supported.
* `attachedRoutes` - supported.
* `conditions` - Supported (Condition/Status/Reason):
* `name` - supported.
* `supportedKinds` - not supported.
* `attachedRoutes` - supported.
* `conditions` - Supported (Condition/Status/Reason):
* `Accepted/True/Accepted`
* `Accepted/False/UnsupportedProtocol`
* `Accepted/False/InvalidCertificateRef`
Expand All @@ -101,26 +104,27 @@ Fields:
* `parentRefs` - partially supported. Port not supported.
* `hostnames` - partially supported. Wildcard binding is not supported: a hostname like `example.com` will not bind to a listener with the hostname `*.example.com`. However, `example.com` will bind to a listener with the empty hostname.
* `rules`
* `matches`
* `path` - partially supported. Only `PathPrefix` and `Exact` types.
* `headers` - partially supported. Only `Exact` type.
* `queryParams` - partially supported. Only `Exact` type.
* `method` - supported.
* `filters`
* `type` - supported.
* `requestRedirect` - supported except for the experimental `path` field. If multiple filters with `requestRedirect` are configured, NGINX Kubernetes Gateway will choose the first one and ignore the rest.
* `requestHeaderModifier`, `requestMirror`, `urlRewrite`, `extensionRef` - not supported.
* `backendRefs` - partially supported. Backend ref `filters` are not supported.
* `matches`
* `path` - partially supported. Only `PathPrefix` and `Exact` types.
* `headers` - partially supported. Only `Exact` type.
* `queryParams` - partially supported. Only `Exact` type.
* `method` - supported.
* `filters`
* `type` - supported.
* `requestRedirect` - supported except for the experimental `path` field. If multiple filters with `requestRedirect` are configured, NGINX Kubernetes Gateway will choose the first one and ignore the rest.
* `requestHeaderModifier`, `requestMirror`, `urlRewrite`, `extensionRef` - not supported.
* `backendRefs` - partially supported. Backend ref `filters` are not supported.
* `status`
* `parents`
* `parentRef` - supported.
* `controllerName` - supported.
* `conditions` - partially supported. Supported (Condition/Status/Reason):
* `Accepted/True/Accepted`
* `Accepted/False/NoMatchingListenerHostname`
* `parentRef` - supported.
* `controllerName` - supported.
* `conditions` - partially supported. Supported (Condition/Status/Reason):
* `Accepted/True/Accepted`
* `Accepted/False/NoMatchingListenerHostname`
* `Accepted/False/NoMatchingParent`
* `Accepted/False/UnsupportedValue`: Custom reason for when the HTTPRoute includes an invalid or unsupported value.
* `Accepted/False/InvalidListener`: Custom reason for when the HTTPRoute references an invalid listener.
* `Accepted/False/GatewayNotProgrammed`: Custom reason for when the Gateway is not Programmed. HTTPRoute may be valid and configured, but will maintain this status as long as the Gateway is not Programmed.
* `ResolvedRefs/True/ResolvedRefs`
* `ResolvedRefs/False/InvalidKind`
* `ResolvedRefs/False/RefNotPermitted`
Expand Down
11 changes: 8 additions & 3 deletions internal/events/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"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/dataplane"
"github.com/nginxinc/nginx-kubernetes-gateway/internal/state/resolver"
"github.com/nginxinc/nginx-kubernetes-gateway/internal/state/secrets"
"github.com/nginxinc/nginx-kubernetes-gateway/internal/status"
)
Expand All @@ -35,6 +36,8 @@ type EventHandlerConfig struct {
SecretStore secrets.SecretStore
// SecretMemoryManager is the state SecretMemoryManager.
SecretMemoryManager secrets.SecretDiskMemoryManager
// ServiceResolver resolves Services to Endpoints.
ServiceResolver resolver.ServiceResolver
// Generator is the nginx config Generator.
Generator config.Generator
// NginxFileMgr is the file Manager for nginx.
Expand Down Expand Up @@ -74,20 +77,22 @@ func (h *EventHandlerImpl) HandleEventBatch(ctx context.Context, batch EventBatc
}
}

changed, conf, statuses := h.cfg.Processor.Process(ctx)
changed, graph := h.cfg.Processor.Process()
if !changed {
h.cfg.Logger.Info("Handling events didn't result into NGINX configuration changes")
return
}

err := h.updateNginx(ctx, conf)
var nginxReloadRes status.NginxReloadResult
err := h.updateNginx(ctx, dataplane.BuildConfiguration(ctx, graph, h.cfg.ServiceResolver))
if err != nil {
h.cfg.Logger.Error(err, "Failed to update NGINX configuration")
nginxReloadRes.Error = err
} else {
h.cfg.Logger.Info("NGINX configuration was successfully updated")
}

h.cfg.StatusUpdater.Update(ctx, statuses)
h.cfg.StatusUpdater.Update(ctx, status.BuildStatuses(graph, nginxReloadRes))
}

func (h *EventHandlerImpl) updateNginx(ctx context.Context, conf dataplane.Configuration) error {
Expand Down
18 changes: 6 additions & 12 deletions internal/events/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ import (
"github.com/nginxinc/nginx-kubernetes-gateway/internal/nginx/config/configfakes"
"github.com/nginxinc/nginx-kubernetes-gateway/internal/nginx/file/filefakes"
"github.com/nginxinc/nginx-kubernetes-gateway/internal/nginx/runtime/runtimefakes"
"github.com/nginxinc/nginx-kubernetes-gateway/internal/state"
"github.com/nginxinc/nginx-kubernetes-gateway/internal/state/dataplane"
"github.com/nginxinc/nginx-kubernetes-gateway/internal/state/graph"
"github.com/nginxinc/nginx-kubernetes-gateway/internal/state/secrets/secretsfakes"
"github.com/nginxinc/nginx-kubernetes-gateway/internal/state/statefakes"
"github.com/nginxinc/nginx-kubernetes-gateway/internal/status/statusfakes"
Expand Down Expand Up @@ -50,7 +50,7 @@ var _ = Describe("EventHandler", func() {
fakeStatusUpdater *statusfakes.FakeUpdater
)

expectReconfig := func(expectedConf dataplane.Configuration, expectedCfg []byte, expectedStatuses state.Statuses) {
expectReconfig := func(expectedConf dataplane.Configuration, expectedCfg []byte) {
Expect(fakeProcessor.ProcessCallCount()).Should(Equal(1))

Expect(fakeGenerator.GenerateCallCount()).Should(Equal(1))
Expand All @@ -64,8 +64,6 @@ var _ = Describe("EventHandler", func() {
Expect(fakeNginxRuntimeMgr.ReloadCallCount()).Should(Equal(1))

Expect(fakeStatusUpdater.UpdateCallCount()).Should(Equal(1))
_, statuses := fakeStatusUpdater.UpdateArgsForCall(0)
Expect(statuses).Should(Equal(expectedStatuses))
}

BeforeEach(func() {
Expand Down Expand Up @@ -93,10 +91,8 @@ var _ = Describe("EventHandler", func() {
DescribeTable(
"A batch with one event",
func(e interface{}) {
fakeConf := dataplane.Configuration{}
fakeStatuses := state.Statuses{}
changed := true
fakeProcessor.ProcessReturns(changed, fakeConf, fakeStatuses)
fakeProcessor.ProcessReturns(changed, &graph.Graph{})

fakeCfg := []byte("fake")
fakeGenerator.GenerateReturns(fakeCfg)
Expand All @@ -120,7 +116,7 @@ var _ = Describe("EventHandler", func() {
}

// Check that a reconfig happened
expectReconfig(fakeConf, fakeCfg, fakeStatuses)
expectReconfig(dataplane.Configuration{}, fakeCfg)
},
Entry(
"HTTPRoute upsert",
Expand Down Expand Up @@ -258,10 +254,8 @@ var _ = Describe("EventHandler", func() {
batch = append(batch, upserts...)
batch = append(batch, deletes...)

fakeConf := dataplane.Configuration{}
changed := true
fakeStatuses := state.Statuses{}
fakeProcessor.ProcessReturns(changed, fakeConf, fakeStatuses)
fakeProcessor.ProcessReturns(changed, &graph.Graph{})

fakeCfg := []byte("fake")
fakeGenerator.GenerateReturns(fakeCfg)
Expand Down Expand Up @@ -294,7 +288,7 @@ var _ = Describe("EventHandler", func() {
Expect(fakeSecretStore.DeleteArgsForCall(0)).Should(Equal(secretNsName))

// Check that a reconfig happened
expectReconfig(fakeConf, fakeCfg, fakeStatuses)
expectReconfig(dataplane.Configuration{}, fakeCfg)
})

Describe("Edge cases", func() {
Expand Down
2 changes: 1 addition & 1 deletion internal/manager/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,6 @@ func Start(cfg config.Config) error {
GatewayCtlrName: cfg.GatewayCtlrName,
GatewayClassName: cfg.GatewayClassName,
SecretMemoryManager: secretMemoryMgr,
ServiceResolver: resolver.NewServiceResolverImpl(mgr.GetClient()),
RelationshipCapturer: relationship.NewCapturerImpl(),
Logger: cfg.Logger.WithName("changeProcessor"),
Validators: validation.Validators{
Expand All @@ -160,6 +159,7 @@ func Start(cfg config.Config) error {
Processor: processor,
SecretStore: secretStore,
SecretMemoryManager: secretMemoryMgr,
ServiceResolver: resolver.NewServiceResolverImpl(mgr.GetClient()),
Generator: configGenerator,
Logger: cfg.Logger.WithName("eventHandler"),
NginxFileMgr: nginxFileMgr,
Expand Down
1 change: 0 additions & 1 deletion internal/nginx/runtime/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ func (m *ManagerImpl) Reload(ctx context.Context) error {

// FIXME(pleshakov)
// (1) ensure the reload actually happens.
// (2) ensure that in case of an error, the error message can be seen by the admins.
// https://github.com/nginxinc/nginx-kubernetes-gateway/issues/664

// for now, to prevent a subsequent reload starting before the in-flight reload finishes, we simply sleep.
Expand Down
26 changes: 7 additions & 19 deletions internal/state/change_processor.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package state

import (
"context"
"fmt"
"sync"

Expand All @@ -18,10 +17,8 @@ import (

gwapivalidation "sigs.k8s.io/gateway-api/apis/v1beta1/validation"

"github.com/nginxinc/nginx-kubernetes-gateway/internal/state/dataplane"
"github.com/nginxinc/nginx-kubernetes-gateway/internal/state/graph"
"github.com/nginxinc/nginx-kubernetes-gateway/internal/state/relationship"
"github.com/nginxinc/nginx-kubernetes-gateway/internal/state/resolver"
"github.com/nginxinc/nginx-kubernetes-gateway/internal/state/secrets"
"github.com/nginxinc/nginx-kubernetes-gateway/internal/state/validation"
)
Expand All @@ -46,19 +43,15 @@ type ChangeProcessor interface {
// The method panics if the resource is of unsupported type or if the passed Gateway is different from the one
// this ChangeProcessor was created for.
CaptureDeleteChange(resourceType client.Object, nsname types.NamespacedName)
// Process processes any captured changes and produces an internal representation of the Gateway configuration and
// the status information about the processed resources.
// If no changes were captured, the changed return argument will be false and both the configuration and statuses
// will be empty.
Process(ctx context.Context) (changed bool, conf dataplane.Configuration, statuses Statuses)
// Process produces a Graph-like representation of GatewayAPI resources.
// If no changes were captured, the changed return argument will be false and graph will be empty.
Process() (changed bool, graphCfg *graph.Graph)
sjberman marked this conversation as resolved.
Show resolved Hide resolved
}

// ChangeProcessorConfig holds configuration parameters for ChangeProcessorImpl.
type ChangeProcessorConfig struct {
// SecretMemoryManager is the secret memory manager.
SecretMemoryManager secrets.SecretDiskMemoryManager
// ServiceResolver resolves Services to Endpoints.
ServiceResolver resolver.ServiceResolver
// RelationshipCapturer captures relationships between Kubernetes API resources and Gateway API resources.
RelationshipCapturer relationship.Capturer
// Validators validate resources according to data-plane specific rules.
Expand Down Expand Up @@ -197,26 +190,21 @@ func (c *ChangeProcessorImpl) CaptureDeleteChange(resourceType client.Object, ns
c.updater.Delete(resourceType, nsname)
}

func (c *ChangeProcessorImpl) Process(
ctx context.Context,
) (changed bool, conf dataplane.Configuration, statuses Statuses) {
func (c *ChangeProcessorImpl) Process() (bool, *graph.Graph) {
c.lock.Lock()
defer c.lock.Unlock()

if !c.getAndResetClusterStateChanged() {
return false, conf, statuses
return false, nil
}

g := graph.BuildGraph(
graphCfg := graph.BuildGraph(
c.clusterState,
c.cfg.GatewayCtlrName,
c.cfg.GatewayClassName,
c.cfg.SecretMemoryManager,
c.cfg.Validators,
)

conf = dataplane.BuildConfiguration(ctx, g, c.cfg.ServiceResolver)
statuses = buildStatuses(g)

return true, conf, statuses
return true, graphCfg
}
Loading