From 42707bed509b0135e5f4727fc46edc80c8152fe9 Mon Sep 17 00:00:00 2001 From: Saylor Berman Date: Thu, 3 Aug 2023 13:18:12 -0600 Subject: [PATCH 1/5] Dynamic control plane configuration support Problem: We want to be able to change control plane settings dynamically, to avoid having to restart NKG and lose valuable state. Solution: Introducing a new CRD that is initialized and created on startup, NginxGateway. This CRD can be updated by the user to dynamically change the state of the control plane. Right now we simply support changing the logging level. The controller will revert to using default values if the CRD is not detected. --- .github/workflows/ci.yml | 9 + Makefile | 7 +- apis/doc.go | 2 + apis/v1alpha1/doc.go | 6 + apis/v1alpha1/nginxgateway_types.go | 86 +++++++++ apis/v1alpha1/register.go | 41 +++++ apis/v1alpha1/zz_generated.deepcopy.go | 132 ++++++++++++++ cmd/gateway/commands.go | 28 ++- cmd/gateway/commands_test.go | 8 +- cmd/gateway/validation_test.go | 10 +- conformance/Makefile | 2 + conformance/provisioner/README.md | 2 +- conformance/provisioner/provisioner.yaml | 2 +- .../provisioner/static-deployment.yaml | 7 +- deploy/helm-chart/README.md | 20 ++- .../crds/gateway.nginx.org_nginxgateways.yaml | 130 ++++++++++++++ .../helm-chart/templates/control-config.yaml | 9 + deploy/helm-chart/templates/deployment.yaml | 5 + deploy/helm-chart/templates/rbac.yaml | 13 ++ deploy/helm-chart/values.yaml | 12 +- deploy/manifests/nginx-gateway.yaml | 169 +++++++++++++++++- docs/cli-help.md | 2 +- docs/control-plane-configuration.md | 37 ++++ docs/proposals/control-plane-config.md | 2 +- docs/resource-validation.md | 4 +- examples/advanced-routing/gateway.yaml | 2 +- examples/cafe-example/gateway.yaml | 2 +- examples/cross-namespace-routing/README.md | 4 +- examples/cross-namespace-routing/gateway.yaml | 2 +- examples/http-header-filter/gateway.yaml | 2 +- examples/https-termination/gateway.yaml | 2 +- examples/traffic-splitting/gateway.yaml | 2 +- go.mod | 7 +- go.sum | 17 ++ internal/framework/status/statuses.go | 17 +- internal/framework/status/updater.go | 16 ++ internal/framework/status/updater_test.go | 51 ++++++ internal/mode/static/config/config.go | 9 +- internal/mode/static/handler.go | 108 ++++++++++- internal/mode/static/handler_test.go | 91 +++++++++- internal/mode/static/manager.go | 35 +++- .../static/state/conditions/conditions.go | 21 +++ tools.go | 1 + 43 files changed, 1080 insertions(+), 54 deletions(-) create mode 100644 apis/doc.go create mode 100644 apis/v1alpha1/doc.go create mode 100644 apis/v1alpha1/nginxgateway_types.go create mode 100644 apis/v1alpha1/register.go create mode 100644 apis/v1alpha1/zz_generated.deepcopy.go create mode 100644 deploy/helm-chart/crds/gateway.nginx.org_nginxgateways.yaml create mode 100644 deploy/helm-chart/templates/control-config.yaml create mode 100644 docs/control-plane-configuration.md diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b89e62227e..53625deffe 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -55,6 +55,9 @@ jobs: - name: Check if generated go files are up to date run: make generate && git diff --exit-code + - name: Check if generated CRDs and types are up to date + run: make generate-crds && git diff --exit-code + - name: Check if generated manifests are up to date run: make generate-manifests && git diff --exit-code @@ -237,12 +240,18 @@ jobs: . --wait --create-namespace +<<<<<<< HEAD --set nginxGateway.image.repository=$(echo ${{ steps.nkg-meta.outputs.tags }} | cut -d ":" -f 1) --set nginxGateway.image.tag=$(echo ${{ steps.nkg-meta.outputs.tags }} | cut -d ":" -f 2) --set nginxGateway.image.pullPolicy=Never --set nginx.image.repository=$(echo ${{ steps.nginx-meta.outputs.tags }} | cut -d ":" -f 1) --set nginx.image.tag=$(echo ${{ steps.nginx-meta.outputs.tags }} | cut -d ":" -f 2) --set nginx.image.pullPolicy=Never +======= + --set nginxGateway.image.repository=$(echo ${{ steps.meta.outputs.tags }} | cut -d ":" -f 1) + --set nginxGateway.image.tag=$(echo ${{ steps.meta.outputs.tags }} | cut -d ":" -f 2) + --set nginxGateway.image.pullPolicy=Never +>>>>>>> 606489f (Dynamic control plane configuration support) --set service.type=NodePort -n nginx-gateway working-directory: ${{ github.workspace }}/deploy/helm-chart diff --git a/Makefile b/Makefile index e3ec3f8316..bfce0802fc 100644 --- a/Makefile +++ b/Makefile @@ -21,7 +21,7 @@ KIND_KUBE_CONFIG=$${HOME}/.kube/kind/config## The location of the kind kubeconfi OUT_DIR ?= $(shell pwd)/build/out## The folder where the binary will be stored ARCH ?= amd64## The architecture of the image and/or binary. For example: amd64 or arm64 override HELM_TEMPLATE_COMMON_ARGS += --set creator=template --set nameOverride=nginx-gateway## The common options for the Helm template command. -override HELM_TEMPLATE_EXTRA_ARGS_FOR_ALL_MANIFESTS_FILE += --set service.create=false## The options to be passed to the full Helm templating command only. +override HELM_TEMPLATE_EXTRA_ARGS_FOR_ALL_MANIFESTS_FILE += --include-crds --set service.create=false## The options to be passed to the full Helm templating command only. override DOCKER_BUILD_OPTIONS += --build-arg VERSION=$(VERSION) --build-arg GIT_COMMIT=$(GIT_COMMIT) --build-arg DATE=$(DATE)## The options for the docker build command. For example, --pull override NGINX_DOCKER_BUILD_OPTIONS += --build-arg NJS_DIR=$(NJS_DIR) --build-arg NGINX_CONF_DIR=$(NGINX_CONF_DIR) .DEFAULT_GOAL := help @@ -62,6 +62,11 @@ build-goreleaser: ## Build the binary using GoReleaser generate: ## Run go generate go generate ./... +.PHONY: generate-crds +generate-crds: ## Generate CRDs and Go types using kubebuilder + go run sigs.k8s.io/controller-tools/cmd/controller-gen crd paths=./apis/... output:crd:dir=deploy/helm-chart/crds + go run sigs.k8s.io/controller-tools/cmd/controller-gen object paths=./apis/... + .PHONY: clean clean: ## Clean the build -rm -r $(OUT_DIR) diff --git a/apis/doc.go b/apis/doc.go new file mode 100644 index 0000000000..7eefd6bf3b --- /dev/null +++ b/apis/doc.go @@ -0,0 +1,2 @@ +// Package apis stores the API definitions for NGINX Kubernetes Gateway configuration. +package apis diff --git a/apis/v1alpha1/doc.go b/apis/v1alpha1/doc.go new file mode 100644 index 0000000000..84eca4a984 --- /dev/null +++ b/apis/v1alpha1/doc.go @@ -0,0 +1,6 @@ +// Package v1alpha1 contains API Schema definitions for the +// gateway.nginx.org API group. +// +// +kubebuilder:object:generate=true +// +groupName=gateway.nginx.org +package v1alpha1 diff --git a/apis/v1alpha1/nginxgateway_types.go b/apis/v1alpha1/nginxgateway_types.go new file mode 100644 index 0000000000..fb452cc9d7 --- /dev/null +++ b/apis/v1alpha1/nginxgateway_types.go @@ -0,0 +1,86 @@ +package v1alpha1 + +import metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + +// +kubebuilder:object:root=true +// +kubebuilder:storageversion +// +kubebuilder:subresource:status + +// NginxGateway represents the dynamic configuration for an NGINX Kubernetes Gateway control plane. +type NginxGateway struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` + + // NginxGatewaySpec defines the desired state of the NginxGateway. + Spec NginxGatewaySpec `json:"spec"` + + // NginxGatewayStatus defines the state of the NginxGateway. + Status NginxGatewayStatus `json:"status,omitempty"` +} + +// +kubebuilder:object:root=true + +// NginxGatewayList contains a list of NginxGateways. +type NginxGatewayList struct { + metav1.TypeMeta `json:",inline"` + metav1.ListMeta `json:"metadata,omitempty"` + Items []NginxGateway `json:"items"` +} + +// NginxGatewaySpec defines the desired state of the NginxGateway. +type NginxGatewaySpec struct { + // Logging defines logging related settings for the control plane. + // + // +optional + Logging *Logging `json:"logging,omitempty"` +} + +// Logging defines logging related settings for the control plane. +type Logging struct { + // Level defines the logging level. + // + // +optional + // +kubebuilder:default=info + Level *ControllerLogLevel `json:"level,omitempty"` +} + +// ControllerLogLevel type defines the logging level for the control plane. +// +// +kubebuilder:validation:Enum=info;debug;error +type ControllerLogLevel string + +const ( + // Info level for control plane logging. + ControllerLogLevelInfo ControllerLogLevel = "info" + + // Debug level for control plane logging. + ControllerLogLevelDebug ControllerLogLevel = "debug" + + // Error level for control plane logging. + ControllerLogLevelError ControllerLogLevel = "error" +) + +// NginxGatewayStatus defines the state of the NginxGateway. +type NginxGatewayStatus struct { + // +optional + Conditions []metav1.Condition `json:"conditions,omitempty"` +} + +// NginxGatewayConditionType is a type of condition associated with an +// NginxGateway. This type should be used with the NginxGatewayStatus.Conditions field. +type NginxGatewayConditionType string + +// NginxGatewayConditionReason defines the set of reasons that explain why a +// particular NginxGateway condition type has been raised. +type NginxGatewayConditionReason string + +const ( + // This condition is true when the NginxGateway configuration is syntactically and semantically valid. + NginxGatewayConditionValid NginxGatewayConditionType = "Valid" + + // This reason is used with the "Valid" condition when the condition is True. + NginxGatewayReasonValid NginxGatewayConditionReason = "Valid" + + // This reason is used with the "Valid" condition when the condition is False. + NginxGatewayReasonInvalid NginxGatewayConditionReason = "Invalid" +) diff --git a/apis/v1alpha1/register.go b/apis/v1alpha1/register.go new file mode 100644 index 0000000000..f3f36d27da --- /dev/null +++ b/apis/v1alpha1/register.go @@ -0,0 +1,41 @@ +package v1alpha1 + +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" +) + +// GroupName specifies the group name used to register the objects. +const GroupName = "gateway.nginx.org" + +// SchemeGroupVersion is group version used to register these objects +var SchemeGroupVersion = schema.GroupVersion{Group: GroupName, Version: "v1alpha1"} + +// Resource takes an unqualified resource and returns a Group qualified GroupResource +func Resource(resource string) schema.GroupResource { + return SchemeGroupVersion.WithResource(resource).GroupResource() +} + +var ( + // SchemeBuilder collects functions that add things to a scheme. It's to allow + // code to compile without explicitly referencing generated types. You should + // declare one in each package that will have generated deep copy or conversion + // functions. + SchemeBuilder = runtime.NewSchemeBuilder(addKnownTypes) + + // AddToScheme applies all the stored functions to the scheme. A non-nil error + // indicates that one function failed and the attempt was abandoned. + AddToScheme = SchemeBuilder.AddToScheme +) + +// Adds the list of known types to Scheme. +func addKnownTypes(scheme *runtime.Scheme) error { + scheme.AddKnownTypes(SchemeGroupVersion, + &NginxGateway{}, + &NginxGatewayList{}, + ) + // AddToGroupVersion allows the serialization of client types like ListOptions. + metav1.AddToGroupVersion(scheme, SchemeGroupVersion) + return nil +} diff --git a/apis/v1alpha1/zz_generated.deepcopy.go b/apis/v1alpha1/zz_generated.deepcopy.go new file mode 100644 index 0000000000..150d4eb552 --- /dev/null +++ b/apis/v1alpha1/zz_generated.deepcopy.go @@ -0,0 +1,132 @@ +//go:build !ignore_autogenerated +// +build !ignore_autogenerated + +// Code generated by controller-gen. DO NOT EDIT. + +package v1alpha1 + +import ( + "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" +) + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *Logging) DeepCopyInto(out *Logging) { + *out = *in + if in.Level != nil { + in, out := &in.Level, &out.Level + *out = new(ControllerLogLevel) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Logging. +func (in *Logging) DeepCopy() *Logging { + if in == nil { + return nil + } + out := new(Logging) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *NginxGateway) DeepCopyInto(out *NginxGateway) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) + in.Spec.DeepCopyInto(&out.Spec) + in.Status.DeepCopyInto(&out.Status) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NginxGateway. +func (in *NginxGateway) DeepCopy() *NginxGateway { + if in == nil { + return nil + } + out := new(NginxGateway) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *NginxGateway) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *NginxGatewayList) DeepCopyInto(out *NginxGatewayList) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ListMeta.DeepCopyInto(&out.ListMeta) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]NginxGateway, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NginxGatewayList. +func (in *NginxGatewayList) DeepCopy() *NginxGatewayList { + if in == nil { + return nil + } + out := new(NginxGatewayList) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *NginxGatewayList) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *NginxGatewaySpec) DeepCopyInto(out *NginxGatewaySpec) { + *out = *in + if in.Logging != nil { + in, out := &in.Logging, &out.Logging + *out = new(Logging) + (*in).DeepCopyInto(*out) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NginxGatewaySpec. +func (in *NginxGatewaySpec) DeepCopy() *NginxGatewaySpec { + if in == nil { + return nil + } + out := new(NginxGatewaySpec) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *NginxGatewayStatus) DeepCopyInto(out *NginxGatewayStatus) { + *out = *in + if in.Conditions != nil { + in, out := &in.Conditions, &out.Conditions + *out = make([]v1.Condition, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NginxGatewayStatus. +func (in *NginxGatewayStatus) DeepCopy() *NginxGatewayStatus { + if in == nil { + return nil + } + out := new(NginxGatewayStatus) + in.DeepCopyInto(out) + return out +} diff --git a/cmd/gateway/commands.go b/cmd/gateway/commands.go index 38b7f66770..ad969f46c8 100644 --- a/cmd/gateway/commands.go +++ b/cmd/gateway/commands.go @@ -1,13 +1,15 @@ package main import ( + "errors" "fmt" "os" "github.com/spf13/cobra" + "go.uber.org/zap" "k8s.io/apimachinery/pkg/types" utilruntime "k8s.io/apimachinery/pkg/util/runtime" - "sigs.k8s.io/controller-runtime/pkg/log/zap" + ctlrZap "sigs.k8s.io/controller-runtime/pkg/log/zap" "github.com/nginxinc/nginx-kubernetes-gateway/internal/mode/provisioner" "github.com/nginxinc/nginx-kubernetes-gateway/internal/mode/static" @@ -15,7 +17,7 @@ import ( ) const ( - domain = "k8s-gateway.nginx.org" + domain = "gateway.nginx.org" gatewayClassFlag = "gatewayclass" gatewayClassNameUsage = `The name of the GatewayClass resource. ` + `Every NGINX Gateway must have a unique corresponding GatewayClass resource.` @@ -118,12 +120,15 @@ func createStaticModeCommand() *cobra.Command { // flag values gateway := namespacedNameValue{} var updateGCStatus bool + var configCRDName string cmd := &cobra.Command{ Use: "static-mode", Short: "Configure NGINX in the scope of a single Gateway resource", RunE: func(cmd *cobra.Command, args []string) error { - logger := zap.New() + atom := zap.NewAtomicLevel() + + logger := ctlrZap.New(ctlrZap.Level(atom)) logger.Info( "Starting NGINX Kubernetes Gateway in static mode", "version", version, @@ -136,6 +141,11 @@ func createStaticModeCommand() *cobra.Command { return fmt.Errorf("error validating POD_IP environment variable: %w", err) } + namespace := os.Getenv("MY_NAMESPACE") + if namespace == "" { + return errors.New("MY_NAMESPACE environment variable must be set") + } + var gwNsName *types.NamespacedName if cmd.Flags().Changed(gatewayFlag) { gwNsName = &gateway.value @@ -143,9 +153,12 @@ func createStaticModeCommand() *cobra.Command { conf := config.Config{ GatewayCtlrName: gatewayCtlrName.value, + ConfigCRDName: configCRDName, Logger: logger, + AtomicLevel: atom, GatewayClassName: gatewayClassName.value, PodIP: podIP, + Namespace: namespace, GatewayNsName: gwNsName, UpdateGatewayClassStatus: updateGCStatus, } @@ -168,6 +181,13 @@ func createStaticModeCommand() *cobra.Command { "equal, it will choose the resource that appears first in alphabetical order by {namespace}/{name}.", ) + cmd.Flags().StringVar( + &configCRDName, + "nginx-gateway-config-name", + "nginx-gateway-config", + `The name of the NginxGateway CRD to be used for this controller's dynamic configuration.`, + ) + cmd.Flags().BoolVar( &updateGCStatus, "update-gatewayclass-status", @@ -184,7 +204,7 @@ func createProvisionerModeCommand() *cobra.Command { Short: "Provision a static-mode NGINX Gateway Deployment per Gateway resource", Hidden: true, RunE: func(cmd *cobra.Command, args []string) error { - logger := zap.New() + logger := ctlrZap.New() logger.Info( "Starting NGINX Kubernetes Gateway Provisioner", "version", version, diff --git a/cmd/gateway/commands_test.go b/cmd/gateway/commands_test.go index 425f66a2e1..42e570535c 100644 --- a/cmd/gateway/commands_test.go +++ b/cmd/gateway/commands_test.go @@ -42,7 +42,7 @@ func TestRootCmdFlagValidation(t *testing.T) { { name: "valid flags", args: []string{ - "--gateway-ctlr-name=k8s-gateway.nginx.org/nginx-gateway", + "--gateway-ctlr-name=gateway.nginx.org/nginx-gateway", "--gatewayclass=nginx", }, wantErr: false, @@ -77,7 +77,7 @@ func TestRootCmdFlagValidation(t *testing.T) { { name: "gatewayclass is not set", args: []string{ - "--gateway-ctlr-name=k8s-gateway.nginx.org/nginx-gateway", + "--gateway-ctlr-name=gateway.nginx.org/nginx-gateway", }, wantErr: true, expectedErrPrefix: `required flag(s) "gatewayclass" not set`, @@ -85,7 +85,7 @@ func TestRootCmdFlagValidation(t *testing.T) { { name: "gatewayclass is set to empty string", args: []string{ - "--gateway-ctlr-name=k8s-gateway.nginx.org/nginx-gateway", + "--gateway-ctlr-name=gateway.nginx.org/nginx-gateway", "--gatewayclass=", }, wantErr: true, @@ -94,7 +94,7 @@ func TestRootCmdFlagValidation(t *testing.T) { { name: "gatewayclass is invalid", args: []string{ - "--gateway-ctlr-name=k8s-gateway.nginx.org/nginx-gateway", + "--gateway-ctlr-name=gateway.nginx.org/nginx-gateway", "--gatewayclass=@", }, wantErr: true, diff --git a/cmd/gateway/validation_test.go b/cmd/gateway/validation_test.go index 8836fa5b18..47231c73ec 100644 --- a/cmd/gateway/validation_test.go +++ b/cmd/gateway/validation_test.go @@ -15,17 +15,17 @@ func TestValidateGatewayControllerName(t *testing.T) { }{ { name: "valid", - value: "k8s-gateway.nginx.org/nginx-gateway", + value: "gateway.nginx.org/nginx-gateway", expErr: false, }, { name: "valid - with subpath", - value: "k8s-gateway.nginx.org/nginx-gateway/my-gateway", + value: "gateway.nginx.org/nginx-gateway/my-gateway", expErr: false, }, { name: "valid - with complex subpath", - value: "k8s-gateway.nginx.org/nginx-gateway/my-gateway/v1", + value: "gateway.nginx.org/nginx-gateway/my-gateway/v1", expErr: false, }, { @@ -35,12 +35,12 @@ func TestValidateGatewayControllerName(t *testing.T) { }, { name: "invalid - lacks path", - value: "k8s-gateway.nginx.org", + value: "gateway.nginx.org", expErr: true, }, { name: "invalid - lacks path, only slash is present", - value: "k8s-gateway.nginx.org/", + value: "gateway.nginx.org/", expErr: true, }, { diff --git a/conformance/Makefile b/conformance/Makefile index 841d575e12..4f929dbbb7 100644 --- a/conformance/Makefile +++ b/conformance/Makefile @@ -8,6 +8,7 @@ KIND_KUBE_CONFIG=$${HOME}/.kube/kind/config TAG = latest PREFIX = conformance-test-runner NKG_MANIFEST=../deploy/manifests/nginx-gateway.yaml +CRDS=../deploy/helm-chart/crds/ SERVICE_MANIFEST=../deploy/manifests/service/nodeport.yaml STATIC_MANIFEST=provisioner/static-deployment.yaml PROVISIONER_MANIFEST=provisioner/provisioner.yaml @@ -49,6 +50,7 @@ load-images: ## Load NKG and NGINX images on configured kind cluster prepare-nkg-dependencies: update-nkg-manifest ## Install NKG dependencies on configured kind cluster ./scripts/install-gateway.sh $(GW_API_VERSION) kubectl wait --for=condition=available --timeout=60s deployment gateway-api-admission-server -n gateway-system + kubectl apply -f $(CRDS) kubectl apply -f $(NKG_MANIFEST) kubectl apply -f $(SERVICE_MANIFEST) diff --git a/conformance/provisioner/README.md b/conformance/provisioner/README.md index 7610fd6f7f..58084e0ad5 100644 --- a/conformance/provisioner/README.md +++ b/conformance/provisioner/README.md @@ -11,7 +11,7 @@ Flags: -h, --help help for provisioner-mode Global Flags: - --gateway-ctlr-name string The name of the Gateway controller. The controller name must be of the form: DOMAIN/PATH. The controller's domain is 'k8s-gateway.nginx.org' (default "") + --gateway-ctlr-name string The name of the Gateway controller. The controller name must be of the form: DOMAIN/PATH. The controller's domain is 'gateway.nginx.org' (default "") --gatewayclass string The name of the GatewayClass resource. Every NGINX Gateway must have a unique corresponding GatewayClass resource. (default "") ``` diff --git a/conformance/provisioner/provisioner.yaml b/conformance/provisioner/provisioner.yaml index ddb4e53d06..e7585dc611 100644 --- a/conformance/provisioner/provisioner.yaml +++ b/conformance/provisioner/provisioner.yaml @@ -68,5 +68,5 @@ spec: runAsUser: 1001 args: - provisioner-mode - - --gateway-ctlr-name=k8s-gateway.nginx.org/nginx-gateway-controller + - --gateway-ctlr-name=gateway.nginx.org/nginx-gateway-controller - --gatewayclass=nginx diff --git a/conformance/provisioner/static-deployment.yaml b/conformance/provisioner/static-deployment.yaml index b311285182..efd14539f6 100644 --- a/conformance/provisioner/static-deployment.yaml +++ b/conformance/provisioner/static-deployment.yaml @@ -25,13 +25,18 @@ spec: containers: - args: - static-mode - - --gateway-ctlr-name=k8s-gateway.nginx.org/nginx-gateway-controller + - --gateway-ctlr-name=gateway.nginx.org/nginx-gateway-controller - --gatewayclass=nginx + - --nginx-gateway-config-name=nginx-gateway-config env: - name: POD_IP valueFrom: fieldRef: fieldPath: status.podIP + - name: MY_NAMESPACE + valueFrom: + fieldRef: + fieldPath: metadata.namespace image: ghcr.io/nginxinc/nginx-kubernetes-gateway:edge imagePullPolicy: Always name: nginx-gateway diff --git a/deploy/helm-chart/README.md b/deploy/helm-chart/README.md index 8bae0eab54..e5b5794966 100644 --- a/deploy/helm-chart/README.md +++ b/deploy/helm-chart/README.md @@ -39,7 +39,7 @@ helm install my-release oci://ghcr.io/nginxinc/charts/nginx-kubernetes-gateway - ```shell helm pull oci://ghcr.io/nginxinc/charts/nginx-kubernetes-gateway --untar --version 0.0.0-edge -cd nginx-gateway +cd nginx-kubernetes-gateway ``` #### Installing the Chart @@ -53,6 +53,7 @@ helm install my-release . --create-namespace --wait -n nginx-gateway ## Upgrading the Chart ### Upgrading the Gateway Resources + Before you upgrade a release, ensure the Gateway API resources are the correct version as supported by the NGINX Kubernetes Gateway - [see the Technical Specifications](../../README.md#technical-specifications).: @@ -62,7 +63,23 @@ To upgrade the Gateway resources from [the Gateway API repo](https://github.com/ kubectl apply -f https://github.com/kubernetes-sigs/gateway-api/releases/download/v0.7.1/standard-install.yaml ``` +### Upgrading the CRDs + +Helm does not upgrade the NGINX Kubernetes Gateway CRDs during a release upgrade. Before you upgrade a release, +you must [pull the chart](#pulling-the-chart) from GitHub and run the following command to upgrade the CRDs: + +```shell +kubectl apply -f crds/ +``` + +The following warning is expected and can be ignored: + +```text +Warning: kubectl apply should be used on resource created by either kubectl create --save-config or kubectl apply. +``` + ### Upgrading the Chart from the OCI Registry + To upgrade the release `my-release`, run: ```shell @@ -85,6 +102,7 @@ To uninstall/delete the release `my-release`: ```shell helm uninstall my-release -n nginx-gateway kubectl delete ns nginx-gateway +kubectl delete crd nginxgateways.gateway.nginx.org ``` These commands remove all the Kubernetes components associated with the release and deletes the release. diff --git a/deploy/helm-chart/crds/gateway.nginx.org_nginxgateways.yaml b/deploy/helm-chart/crds/gateway.nginx.org_nginxgateways.yaml new file mode 100644 index 0000000000..3bba63cdcf --- /dev/null +++ b/deploy/helm-chart/crds/gateway.nginx.org_nginxgateways.yaml @@ -0,0 +1,130 @@ +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.11.4 + name: nginxgateways.gateway.nginx.org +spec: + group: gateway.nginx.org + names: + kind: NginxGateway + listKind: NginxGatewayList + plural: nginxgateways + singular: nginxgateway + scope: Namespaced + versions: + - name: v1alpha1 + schema: + openAPIV3Schema: + description: NginxGateway represents the dynamic configuration for an NGINX + Kubernetes Gateway control plane. + properties: + apiVersion: + description: 'APIVersion defines the versioned schema of this representation + of an object. Servers should convert recognized schemas to the latest + internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources' + type: string + kind: + description: 'Kind is a string value representing the REST resource this + object represents. Servers may infer this from the endpoint the client + submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' + type: string + metadata: + type: object + spec: + description: NginxGatewaySpec defines the desired state of the NginxGateway. + properties: + logging: + description: Logging defines logging related settings for the control + plane. + properties: + level: + default: info + description: Level defines the logging level. + enum: + - info + - debug + - error + type: string + type: object + type: object + status: + description: NginxGatewayStatus defines the state of the NginxGateway. + properties: + conditions: + items: + description: "Condition contains details for one aspect of the current + state of this API Resource. --- This struct is intended for direct + use as an array at the field path .status.conditions. For example, + \n type FooStatus struct{ // Represents the observations of a + foo's current state. // Known .status.conditions.type are: \"Available\", + \"Progressing\", and \"Degraded\" // +patchMergeKey=type // +patchStrategy=merge + // +listType=map // +listMapKey=type Conditions []metav1.Condition + `json:\"conditions,omitempty\" patchStrategy:\"merge\" patchMergeKey:\"type\" + protobuf:\"bytes,1,rep,name=conditions\"` \n // other fields }" + properties: + lastTransitionTime: + description: lastTransitionTime is the last time the condition + transitioned from one status to another. This should be when + the underlying condition changed. If that is not known, then + using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: message is a human readable message indicating + details about the transition. This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: observedGeneration represents the .metadata.generation + that the condition was set based upon. For instance, if .metadata.generation + is currently 12, but the .status.conditions[x].observedGeneration + is 9, the condition is out of date with respect to the current + state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: reason contains a programmatic identifier indicating + the reason for the condition's last transition. Producers + of specific condition types may define expected values and + meanings for this field, and whether the values are considered + a guaranteed API. The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: type of condition in CamelCase or in foo.example.com/CamelCase. + --- Many .condition.type values are consistent across resources + like Available, but because arbitrary conditions can be useful + (see .node.status.conditions), the ability to deconflict is + important. The regex it matches is (dns1123SubdomainFmt/)?(qualifiedNameFmt) + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + type: array + type: object + required: + - spec + type: object + served: true + storage: true + subresources: + status: {} diff --git a/deploy/helm-chart/templates/control-config.yaml b/deploy/helm-chart/templates/control-config.yaml new file mode 100644 index 0000000000..19076115cb --- /dev/null +++ b/deploy/helm-chart/templates/control-config.yaml @@ -0,0 +1,9 @@ +apiVersion: gateway.nginx.org/v1alpha1 +kind: NginxGateway +metadata: + name: {{ .Values.nginxGateway.crdConfig.name }} + namespace: {{ .Release.Namespace }} + labels: + {{- include "nginx-gateway.labels" . | nindent 4 }} +spec: + {{- toYaml .Values.nginxGateway.crdConfig.spec | nindent 2 }} diff --git a/deploy/helm-chart/templates/deployment.yaml b/deploy/helm-chart/templates/deployment.yaml index abd3874dab..7129599e25 100644 --- a/deploy/helm-chart/templates/deployment.yaml +++ b/deploy/helm-chart/templates/deployment.yaml @@ -22,11 +22,16 @@ spec: - static-mode - --gateway-ctlr-name={{ .Values.nginxGateway.gatewayControllerName }} - --gatewayclass={{ .Values.nginxGateway.gatewayClassName }} + - --nginx-gateway-config-name={{ .Values.nginxGateway.crdConfig.name }} env: - name: POD_IP valueFrom: fieldRef: fieldPath: status.podIP + - name: MY_NAMESPACE + valueFrom: + fieldRef: + fieldPath: metadata.namespace image: {{ .Values.nginxGateway.image.repository }}:{{ .Values.nginxGateway.image.tag | default .Chart.AppVersion }} imagePullPolicy: {{ .Values.nginxGateway.image.pullPolicy }} name: nginx-gateway diff --git a/deploy/helm-chart/templates/rbac.yaml b/deploy/helm-chart/templates/rbac.yaml index 08119a2ac8..8306e90fca 100644 --- a/deploy/helm-chart/templates/rbac.yaml +++ b/deploy/helm-chart/templates/rbac.yaml @@ -63,6 +63,19 @@ rules: - gatewayclasses/status verbs: - update +- apiGroups: + - gateway.nginx.org + resources: + - nginxgateways + verbs: + - list + - watch +- apiGroups: + - gateway.nginx.org + resources: + - nginxgateways/status + verbs: + - update --- apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRoleBinding diff --git a/deploy/helm-chart/values.yaml b/deploy/helm-chart/values.yaml index e80a726cf3..ba4f6f42ff 100644 --- a/deploy/helm-chart/values.yaml +++ b/deploy/helm-chart/values.yaml @@ -6,8 +6,16 @@ nginxGateway: ## belong to its class - i.e. have the "gatewayClassName" field resource equal to the class. gatewayClassName: nginx ## The name of the Gateway controller. The controller name must be of the form: DOMAIN/PATH. The controller's domain - ## is k8s-gateway.nginx.org. - gatewayControllerName: k8s-gateway.nginx.org/nginx-gateway-controller + ## is gateway.nginx.org. + gatewayControllerName: gateway.nginx.org/nginx-gateway-controller + ## The dynamic configuration for the control plane that is contained in the NginxGateway CRD. + crdConfig: + name: nginx-gateway-config + spec: + logging: + ## Log level. Supported values "info", "debug", "error". + level: info + image: ## The NGINX Kubernetes Gateway image to use repository: ghcr.io/nginxinc/nginx-kubernetes-gateway diff --git a/deploy/manifests/nginx-gateway.yaml b/deploy/manifests/nginx-gateway.yaml index a419e9df58..f622d9374e 100644 --- a/deploy/manifests/nginx-gateway.yaml +++ b/deploy/manifests/nginx-gateway.yaml @@ -3,6 +3,139 @@ kind: Namespace metadata: name: nginx-gateway --- +# Source: crds/gateway.nginx.org_nginxgateways.yaml +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.11.4 + name: nginxgateways.gateway.nginx.org +spec: + group: gateway.nginx.org + names: + kind: NginxGateway + listKind: NginxGatewayList + plural: nginxgateways + singular: nginxgateway + scope: Namespaced + versions: + - name: v1alpha1 + schema: + openAPIV3Schema: + description: NginxGateway represents the dynamic configuration for an NGINX + Kubernetes Gateway control plane. + properties: + apiVersion: + description: 'APIVersion defines the versioned schema of this representation + of an object. Servers should convert recognized schemas to the latest + internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources' + type: string + kind: + description: 'Kind is a string value representing the REST resource this + object represents. Servers may infer this from the endpoint the client + submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' + type: string + metadata: + type: object + spec: + description: NginxGatewaySpec defines the desired state of the NginxGateway. + properties: + logging: + description: Logging defines logging related settings for the control + plane. + properties: + level: + default: info + description: Level defines the logging level. + enum: + - info + - debug + - error + type: string + type: object + type: object + status: + description: NginxGatewayStatus defines the state of the NginxGateway. + properties: + conditions: + items: + description: "Condition contains details for one aspect of the current + state of this API Resource. --- This struct is intended for direct + use as an array at the field path .status.conditions. For example, + \n type FooStatus struct{ // Represents the observations of a + foo's current state. // Known .status.conditions.type are: \"Available\", + \"Progressing\", and \"Degraded\" // +patchMergeKey=type // +patchStrategy=merge + // +listType=map // +listMapKey=type Conditions []metav1.Condition + `json:\"conditions,omitempty\" patchStrategy:\"merge\" patchMergeKey:\"type\" + protobuf:\"bytes,1,rep,name=conditions\"` \n // other fields }" + properties: + lastTransitionTime: + description: lastTransitionTime is the last time the condition + transitioned from one status to another. This should be when + the underlying condition changed. If that is not known, then + using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: message is a human readable message indicating + details about the transition. This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: observedGeneration represents the .metadata.generation + that the condition was set based upon. For instance, if .metadata.generation + is currently 12, but the .status.conditions[x].observedGeneration + is 9, the condition is out of date with respect to the current + state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: reason contains a programmatic identifier indicating + the reason for the condition's last transition. Producers + of specific condition types may define expected values and + meanings for this field, and whether the values are considered + a guaranteed API. The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: type of condition in CamelCase or in foo.example.com/CamelCase. + --- Many .condition.type values are consistent across resources + like Available, but because arbitrary conditions can be useful + (see .node.status.conditions), the ability to deconflict is + important. The regex it matches is (dns1123SubdomainFmt/)?(qualifiedNameFmt) + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + type: array + type: object + required: + - spec + type: object + served: true + storage: true + subresources: + status: {} + +--- # Source: nginx-kubernetes-gateway/templates/rbac.yaml apiVersion: v1 kind: ServiceAccount @@ -74,6 +207,19 @@ rules: - gatewayclasses/status verbs: - update +- apiGroups: + - gateway.nginx.org + resources: + - nginxgateways + verbs: + - list + - watch +- apiGroups: + - gateway.nginx.org + resources: + - nginxgateways/status + verbs: + - update --- # Source: nginx-kubernetes-gateway/templates/rbac.yaml apiVersion: rbac.authorization.k8s.io/v1 @@ -119,13 +265,18 @@ spec: containers: - args: - static-mode - - --gateway-ctlr-name=k8s-gateway.nginx.org/nginx-gateway-controller + - --gateway-ctlr-name=gateway.nginx.org/nginx-gateway-controller - --gatewayclass=nginx + - --nginx-gateway-config-name=nginx-gateway-config env: - name: POD_IP valueFrom: fieldRef: fieldPath: status.podIP + - name: MY_NAMESPACE + valueFrom: + fieldRef: + fieldPath: metadata.namespace image: ghcr.io/nginxinc/nginx-kubernetes-gateway:edge imagePullPolicy: Always name: nginx-gateway @@ -189,4 +340,18 @@ metadata: app.kubernetes.io/instance: nginx-gateway app.kubernetes.io/version: "edge" spec: - controllerName: k8s-gateway.nginx.org/nginx-gateway-controller + controllerName: gateway.nginx.org/nginx-gateway-controller +--- +# Source: nginx-kubernetes-gateway/templates/control-config.yaml +apiVersion: gateway.nginx.org/v1alpha1 +kind: NginxGateway +metadata: + name: nginx-gateway-config + namespace: nginx-gateway + labels: + app.kubernetes.io/name: nginx-gateway + app.kubernetes.io/instance: nginx-gateway + app.kubernetes.io/version: "edge" +spec: + logging: + level: info diff --git a/docs/cli-help.md b/docs/cli-help.md index 18fa7a300b..aa2e8f3ea8 100644 --- a/docs/cli-help.md +++ b/docs/cli-help.md @@ -16,7 +16,7 @@ Flags: | Name | Type | Description | |------------------------------|----------|---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| -| `gateway-ctlr-name` | `string` | The name of the Gateway controller. The controller name must be of the form: `DOMAIN/PATH`. The controller's domain is `k8s-gateway.nginx.org`. | +| `gateway-ctlr-name` | `string` | The name of the Gateway controller. The controller name must be of the form: `DOMAIN/PATH`. The controller's domain is `gateway.nginx.org`. | | `gatewayclass` | `string` | The name of the GatewayClass resource. Every NGINX Gateway must have a unique corresponding GatewayClass resource. | | `gateway` | `string` | The namespaced name of the Gateway resource to use. Must be of the form: `NAMESPACE/NAME`. If not specified, the control plane will process all Gateways for the configured GatewayClass. However, among them, it will choose the oldest resource by creation timestamp. If the timestamps are equal, it will choose the resource that appears first in alphabetical order by {namespace}/{name}. | | `update-gatewayclass-status` | `bool` | Update the status of the GatewayClass resource. (default true) | diff --git a/docs/control-plane-configuration.md b/docs/control-plane-configuration.md new file mode 100644 index 0000000000..6292b971ce --- /dev/null +++ b/docs/control-plane-configuration.md @@ -0,0 +1,37 @@ +# Control Plane Configuration + +This document describes how to dynamically update the NGINX Kubernetes Gateway control plane configuration. + +## Overview + +NGINX Kubernetes Gateway offers a way to update the control plane configuration dynamically without the need for a +restart. These configuration options include: + +| Option | Available values | Default value | +|---------------|--------------------|---------------| +| Logging Level | info, debug, error | info | + + +The control plane configuration is stored in the NginxGateway custom resource. This resource is created during the +installation of NGINX Kubernetes Gateway. The default name of the resource is `nginx-gateway-config` and is deployed +in the same Namespace as the controller (`nginx-gateway`). + +The control plane only watches this single instance of the custom resource. If the resource is deleted or invalid, an +error is emitted and the default values will be used by the control plane for its configuration. + +## Viewing and Updating the Configuration + +To view the current configuration: + +```shell +kubectl -n nginx-gateway get nginxgateways nginx-gateway-config -o yaml +``` + +To update the configuration: + +```shell +kubectl -n nginx-gateway edit nginxgateways nginx-gateway-config +``` + +This will open the configuration in your default editor. You can then update and save the configuration, which is +applied automatically to the control plane. diff --git a/docs/proposals/control-plane-config.md b/docs/proposals/control-plane-config.md index 954f2c41bc..fc781776f5 100644 --- a/docs/proposals/control-plane-config.md +++ b/docs/proposals/control-plane-config.md @@ -1,7 +1,7 @@ # Enhancement Proposal-928: Control Plane Dynamic Configuration - Issue: https://github.com/nginxinc/nginx-kubernetes-gateway/issues/928 -- Status: Implementable +- Status: Completed ## Summary diff --git a/docs/resource-validation.md b/docs/resource-validation.md index dc210121ff..ba4f57e143 100644 --- a/docs/resource-validation.md +++ b/docs/resource-validation.md @@ -34,7 +34,7 @@ Status: Reason: Accepted Status: True Type: Accepted - Controller Name: k8s-gateway.nginx.org/nginx-gateway-controller + Controller Name: gateway.nginx.org/nginx-gateway-controller Parent Ref: Group: gateway.networking.k8s.io Kind: Gateway @@ -138,7 +138,7 @@ Status: Reason: UnsupportedValue Status: False Type: Accepted - Controller Name: k8s-gateway.nginx.org/nginx-gateway-controller + Controller Name: gateway.nginx.org/nginx-gateway-controller Parent Ref: Group: gateway.networking.k8s.io Kind: Gateway diff --git a/examples/advanced-routing/gateway.yaml b/examples/advanced-routing/gateway.yaml index 9fb0ebd1af..e531e6e467 100644 --- a/examples/advanced-routing/gateway.yaml +++ b/examples/advanced-routing/gateway.yaml @@ -3,7 +3,7 @@ kind: Gateway metadata: name: gateway labels: - domain: k8s-gateway.nginx.org + domain: gateway.nginx.org spec: gatewayClassName: nginx listeners: diff --git a/examples/cafe-example/gateway.yaml b/examples/cafe-example/gateway.yaml index f9859bfa12..9e507147f1 100644 --- a/examples/cafe-example/gateway.yaml +++ b/examples/cafe-example/gateway.yaml @@ -3,7 +3,7 @@ kind: Gateway metadata: name: gateway labels: - domain: k8s-gateway.nginx.org + domain: gateway.nginx.org spec: gatewayClassName: nginx listeners: diff --git a/examples/cross-namespace-routing/README.md b/examples/cross-namespace-routing/README.md index 4f689a7c26..955e214801 100644 --- a/examples/cross-namespace-routing/README.md +++ b/examples/cross-namespace-routing/README.md @@ -128,7 +128,7 @@ Condtions: Reason: RefNotPermitted Status: False Type: ResolvedRefs - Controller Name: k8s-gateway.nginx.org/nginx-gateway-controller + Controller Name: gateway.nginx.org/nginx-gateway-controller ``` ```shell @@ -142,5 +142,5 @@ Condtions: Reason: RefNotPermitted Status: False Type: ResolvedRefs - Controller Name: k8s-gateway.nginx.org/nginx-gateway-controller + Controller Name: gateway.nginx.org/nginx-gateway-controller ``` diff --git a/examples/cross-namespace-routing/gateway.yaml b/examples/cross-namespace-routing/gateway.yaml index f9859bfa12..9e507147f1 100644 --- a/examples/cross-namespace-routing/gateway.yaml +++ b/examples/cross-namespace-routing/gateway.yaml @@ -3,7 +3,7 @@ kind: Gateway metadata: name: gateway labels: - domain: k8s-gateway.nginx.org + domain: gateway.nginx.org spec: gatewayClassName: nginx listeners: diff --git a/examples/http-header-filter/gateway.yaml b/examples/http-header-filter/gateway.yaml index 9fb0ebd1af..e531e6e467 100644 --- a/examples/http-header-filter/gateway.yaml +++ b/examples/http-header-filter/gateway.yaml @@ -3,7 +3,7 @@ kind: Gateway metadata: name: gateway labels: - domain: k8s-gateway.nginx.org + domain: gateway.nginx.org spec: gatewayClassName: nginx listeners: diff --git a/examples/https-termination/gateway.yaml b/examples/https-termination/gateway.yaml index 51b0758a6e..f81b6948ff 100644 --- a/examples/https-termination/gateway.yaml +++ b/examples/https-termination/gateway.yaml @@ -3,7 +3,7 @@ kind: Gateway metadata: name: gateway labels: - domain: k8s-gateway.nginx.org + domain: gateway.nginx.org spec: gatewayClassName: nginx listeners: diff --git a/examples/traffic-splitting/gateway.yaml b/examples/traffic-splitting/gateway.yaml index 9fb0ebd1af..e531e6e467 100644 --- a/examples/traffic-splitting/gateway.yaml +++ b/examples/traffic-splitting/gateway.yaml @@ -3,7 +3,7 @@ kind: Gateway metadata: name: gateway labels: - domain: k8s-gateway.nginx.org + domain: gateway.nginx.org spec: gatewayClassName: nginx listeners: diff --git a/go.mod b/go.mod index 8d5e317d77..1f9da643f6 100644 --- a/go.mod +++ b/go.mod @@ -12,10 +12,12 @@ require ( github.com/onsi/ginkgo/v2 v2.11.0 github.com/onsi/gomega v1.27.10 github.com/spf13/cobra v1.7.0 + go.uber.org/zap v1.24.0 k8s.io/api v0.27.4 k8s.io/apimachinery v0.27.4 k8s.io/client-go v0.27.4 sigs.k8s.io/controller-runtime v0.15.1 + sigs.k8s.io/controller-tools v0.11.4 sigs.k8s.io/gateway-api v0.7.1 ) @@ -26,12 +28,14 @@ require ( github.com/emicklei/go-restful/v3 v3.9.0 // indirect github.com/evanphx/json-patch v5.6.0+incompatible // indirect github.com/evanphx/json-patch/v5 v5.6.0 // indirect + github.com/fatih/color v1.13.0 // indirect github.com/fsnotify/fsnotify v1.6.0 // indirect github.com/go-logr/zapr v1.2.4 // indirect github.com/go-openapi/jsonpointer v0.19.6 // indirect github.com/go-openapi/jsonreference v0.20.1 // indirect github.com/go-openapi/swag v0.22.3 // indirect github.com/go-task/slim-sprig v0.0.0-20230315185526-52ccab3ef572 // indirect + github.com/gobuffalo/flect v0.3.0 // indirect github.com/gogo/protobuf v1.3.2 // indirect github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect github.com/golang/protobuf v1.5.3 // indirect @@ -44,6 +48,8 @@ require ( github.com/josharian/intern v1.0.0 // indirect github.com/json-iterator/go v1.1.12 // indirect github.com/mailru/easyjson v0.7.7 // indirect + github.com/mattn/go-colorable v0.1.9 // indirect + github.com/mattn/go-isatty v0.0.14 // indirect github.com/matttproud/golang_protobuf_extensions v1.0.4 // indirect github.com/moby/spdystream v0.2.0 // indirect github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect @@ -59,7 +65,6 @@ require ( github.com/stretchr/testify v1.8.2 // indirect go.uber.org/atomic v1.9.0 // indirect go.uber.org/multierr v1.7.0 // indirect - go.uber.org/zap v1.24.0 // indirect golang.org/x/mod v0.11.0 // indirect golang.org/x/net v0.12.0 // indirect golang.org/x/oauth2 v0.5.0 // indirect diff --git a/go.sum b/go.sum index 265bda7436..1e1e8de283 100644 --- a/go.sum +++ b/go.sum @@ -26,6 +26,8 @@ github.com/evanphx/json-patch v5.6.0+incompatible h1:jBYDEEiFBPxA0v50tFdvOzQQTCv github.com/evanphx/json-patch v5.6.0+incompatible/go.mod h1:50XU6AFN0ol/bzJsmQLiYLvXMP4fmwYFNcr97nuDLSk= github.com/evanphx/json-patch/v5 v5.6.0 h1:b91NhWfaz02IuVxO9faSllyAtNXHMPkC5J8sJCLunww= github.com/evanphx/json-patch/v5 v5.6.0/go.mod h1:G79N1coSVB93tBe7j6PhzjmR3/2VvlbKOFpnXhI9Bw4= +github.com/fatih/color v1.13.0 h1:8LOYc1KYPPmyKMuN8QV2DNRWNbLo6LZ0iLs8+mlH53w= +github.com/fatih/color v1.13.0/go.mod h1:kLAiJbzzSOZDVNGyDpeOxJ47H46qBXwg5ILebYFFOfk= github.com/fsnotify/fsnotify v1.6.0 h1:n+5WquG0fcWoWp6xPWfHdbskMCQaFnG6PfBrh1Ky4HY= github.com/fsnotify/fsnotify v1.6.0/go.mod h1:sl3t1tCWJFWoRz9R8WJCbQihKKwmorjAbSClcnxKAGw= github.com/go-logr/logr v1.2.0/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= @@ -41,6 +43,8 @@ github.com/go-openapi/swag v0.22.3 h1:yMBqmnQ0gyZvEb/+KzuWZOXgllrXT4SADYbvDaXHv/ github.com/go-openapi/swag v0.22.3/go.mod h1:UzaqsxGiab7freDnrUUra0MwWfN/q7tE4j+VcZ0yl14= github.com/go-task/slim-sprig v0.0.0-20230315185526-52ccab3ef572 h1:tfuBGBXKqDEevZMzYi5KSi8KkcZtzBcTgAUUtapy0OI= github.com/go-task/slim-sprig v0.0.0-20230315185526-52ccab3ef572/go.mod h1:9Pwr4B2jHnOSGXyyzV8ROjYa2ojvAY6HCGYYfMoC3Ls= +github.com/gobuffalo/flect v0.3.0 h1:erfPWM+K1rFNIQeRPdeEXxo8yFr/PO17lhRnS8FUrtk= +github.com/gobuffalo/flect v0.3.0/go.mod h1:5pf3aGnsvqvCj50AVni7mJJF8ICxGZ8HomberC3pXLE= github.com/gogo/protobuf v1.3.2 h1:Ov1cvc58UF3b5XjBnZv7+opcTcQFZebYjWzi34vdm4Q= github.com/gogo/protobuf v1.3.2/go.mod h1:P1XiOD3dCwIKUDQYPy72D8LYyHL2YPYrpS2s69NZV8Q= github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q= @@ -99,6 +103,11 @@ github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= github.com/mailru/easyjson v0.7.7 h1:UGYAvKxe3sBsEDzO8ZeWOSlIQfWFlxbzLZe7hwFURr0= github.com/mailru/easyjson v0.7.7/go.mod h1:xzfreul335JAWq5oZzymOObrkdz5UnU4kGfJJLY9Nlc= +github.com/mattn/go-colorable v0.1.9 h1:sqDoxXbdeALODt0DAeJCVp38ps9ZogZEAXjus69YV3U= +github.com/mattn/go-colorable v0.1.9/go.mod h1:u6P/XSegPjTcexA+o6vUJrdnUu04hMope9wVRipJSqc= +github.com/mattn/go-isatty v0.0.12/go.mod h1:cbi8OIDigv2wuxKPP5vlRcQ1OAZbq2CE4Kysco4FUpU= +github.com/mattn/go-isatty v0.0.14 h1:yVuAays6BHfxijgZPzw+3Zlu5yQgKGP2/hcQbHb7S9Y= +github.com/mattn/go-isatty v0.0.14/go.mod h1:7GGIvUiUoEMVVmxf/4nioHXj79iQHKdU27kJ6hsGG94= github.com/matttproud/golang_protobuf_extensions v1.0.4 h1:mmDVorXM7PCGKw94cs5zkfA9PSy5pEvNWRP0ET0TIVo= github.com/matttproud/golang_protobuf_extensions v1.0.4/go.mod h1:BSXmuO+STAnVfrANrmjBb36TMTDstsz7MSK+HVaYKv4= github.com/maxbrunsfeld/counterfeiter/v6 v6.6.2 h1:CEy7VRV/Vbm7YLuZo3pGKa5GlPX4zzric6dEubIJTx0= @@ -112,6 +121,8 @@ github.com/modern-go/reflect2 v1.0.2 h1:xBagoLtFs94CBntxluKeaWgTMpvLxC4ur3nMaC9G github.com/modern-go/reflect2 v1.0.2/go.mod h1:yWuevngMOJpCy52FWWMvUC8ws7m/LJsjYzDa0/r8luk= github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 h1:C3w9PqII01/Oq1c1nUAm88MOHcQC9l5mIlSMApZMrHA= github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ= +github.com/nxadm/tail v1.4.8 h1:nPr65rt6Y5JFSKQO7qToXr7pePgD6Gwiw05lkbyAQTE= +github.com/onsi/ginkgo v1.16.5 h1:8xi0RTUf59SOSfEtZMvwTvXYMzG4gV23XVHOZiXNtnE= github.com/onsi/ginkgo/v2 v2.11.0 h1:WgqUCUt/lT6yXoQ8Wef0fsNn5cAuMK7+KT9UFRz2tcU= github.com/onsi/ginkgo/v2 v2.11.0/go.mod h1:ZhrRA5XmEE3x3rhlzamx/JJvujdZoJ2uvgI7kR0iZvM= github.com/onsi/gomega v1.27.10 h1:naR28SdDFlqrG6kScpT8VWpu1xWY5nJRCF3XaYyBjhI= @@ -203,10 +214,13 @@ golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5h golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20191204072324-ce4227a45e2e/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20200116001909-b77594299b42/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20200223170610-d5e6a3e2c0ae/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210330210617-4fbd30eecc44/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210510120138-977fb7262007/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.0.0-20210630005230-0f9fa26af87c/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220908164124-27713097b956/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.10.0 h1:SqMFp9UcQJZa+pmYuAKjd9xq1f0j5rLcDIk0mj4qAsA= golang.org/x/sys v0.10.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= @@ -267,6 +281,7 @@ gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntN gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q= gopkg.in/inf.v0 v0.9.1 h1:73M5CoZyi3ZLMOyDlQh031Cx6N9NDJ2Vvfl76EDAgDc= gopkg.in/inf.v0 v0.9.1/go.mod h1:cWUDdTG/fYaXco+Dcufb5Vnc6Gp2YChqWtbxRZE0mXw= +gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7 h1:uRGJdciOHaEIrze2W8Q3AKkepLTh2hOroT7a+7czfdQ= gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.8/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.3.0/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= @@ -297,6 +312,8 @@ k8s.io/utils v0.0.0-20230209194617-a36077c30491 h1:r0BAOLElQnnFhE/ApUsg3iHdVYYPB k8s.io/utils v0.0.0-20230209194617-a36077c30491/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0= sigs.k8s.io/controller-runtime v0.15.1 h1:9UvgKD4ZJGcj24vefUFgZFP3xej/3igL9BsOUTb/+4c= sigs.k8s.io/controller-runtime v0.15.1/go.mod h1:7ngYvp1MLT+9GeZ+6lH3LOlcHkp/+tzA/fmHa4iq9kk= +sigs.k8s.io/controller-tools v0.11.4 h1:jqXJ/Xb6yBgbgcBbw1YoC3rC+Bt1XZWiLjj0ZHv/GrU= +sigs.k8s.io/controller-tools v0.11.4/go.mod h1:qcfX7jfcfYD/b7lAhvqAyTbt/px4GpvN88WKLFFv7p8= sigs.k8s.io/gateway-api v0.7.1 h1:Tts2jeepVkPA5rVG/iO+S43s9n7Vp7jCDhZDQYtPigQ= sigs.k8s.io/gateway-api v0.7.1/go.mod h1:Xv0+ZMxX0lu1nSSDIIPEfbVztgNZ+3cfiYrJsa2Ooso= sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd h1:EDPBXCAspyGV4jQlpZSudPeMmr1bNJefnuqLsRAsHZo= diff --git a/internal/framework/status/statuses.go b/internal/framework/status/statuses.go index 9def44228f..fe8d7d9530 100644 --- a/internal/framework/status/statuses.go +++ b/internal/framework/status/statuses.go @@ -19,11 +19,12 @@ type GatewayStatuses map[types.NamespacedName]GatewayStatus // GatewayClassStatuses holds the statuses of GatewayClasses where the key is the namespaced name of a GatewayClass. type GatewayClassStatuses map[types.NamespacedName]GatewayClassStatus -// Statuses holds the status-related information about Gateway API resources. +// Statuses holds the status-related information about resources. type Statuses struct { GatewayClassStatuses GatewayClassStatuses GatewayStatuses GatewayStatuses HTTPRouteStatuses HTTPRouteStatuses + NginxGatewayStatus NginxGatewayStatus } // GatewayStatus holds the status of the winning Gateway resource. @@ -66,6 +67,18 @@ type ParentStatus struct { // GatewayClassStatus holds status-related information about the GatewayClass resource. type GatewayClassStatus struct { - Conditions []conditions.Condition + // Conditions is the list of conditions for this GatewayClass. + Conditions []conditions.Condition + // ObservedGeneration is the generation of the resource that was processed. + ObservedGeneration int64 +} + +// NginxGatewayStatus holds status-related information about the NginxGateway resource. +type NginxGatewayStatus struct { + // NSName is the NamespacedName of the NginxGateway resource. + NSName types.NamespacedName + // Conditions is the list of conditions for this NginxGateway. + Conditions []conditions.Condition + // ObservedGeneration is the generation of the resource that was processed. ObservedGeneration int64 } diff --git a/internal/framework/status/updater.go b/internal/framework/status/updater.go index f9afdcd320..0020535a8f 100644 --- a/internal/framework/status/updater.go +++ b/internal/framework/status/updater.go @@ -8,6 +8,8 @@ import ( "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/gateway-api/apis/v1beta1" + + nkgAPI "github.com/nginxinc/nginx-kubernetes-gateway/apis/v1alpha1" ) //go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . Updater @@ -117,6 +119,20 @@ func (upd *updaterImpl) Update(ctx context.Context, statuses Statuses) { ) }) } + + ngStatus := statuses.NginxGatewayStatus + if len(ngStatus.Conditions) > 0 { + upd.update(ctx, ngStatus.NSName, &nkgAPI.NginxGateway{}, func(object client.Object) { + ng := object.(*nkgAPI.NginxGateway) + ng.Status = nkgAPI.NginxGatewayStatus{ + Conditions: convertConditions( + ngStatus.Conditions, + ngStatus.ObservedGeneration, + upd.cfg.Clock.Now(), + ), + } + }) + } } func (upd *updaterImpl) update( diff --git a/internal/framework/status/updater_test.go b/internal/framework/status/updater_test.go index 7791b1ff33..ea83e7687b 100644 --- a/internal/framework/status/updater_test.go +++ b/internal/framework/status/updater_test.go @@ -14,6 +14,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/log/zap" "sigs.k8s.io/gateway-api/apis/v1beta1" + nkgAPI "github.com/nginxinc/nginx-kubernetes-gateway/apis/v1alpha1" "github.com/nginxinc/nginx-kubernetes-gateway/internal/framework/helpers" "github.com/nginxinc/nginx-kubernetes-gateway/internal/framework/status" "github.com/nginxinc/nginx-kubernetes-gateway/internal/framework/status/statusfakes" @@ -34,6 +35,7 @@ var _ = Describe("Updater", func() { scheme := runtime.NewScheme() Expect(v1beta1.AddToScheme(scheme)).Should(Succeed()) + Expect(nkgAPI.AddToScheme(scheme)).Should(Succeed()) client = fake.NewClientBuilder(). WithScheme(scheme). @@ -41,6 +43,7 @@ var _ = Describe("Updater", func() { &v1beta1.GatewayClass{}, &v1beta1.Gateway{}, &v1beta1.HTTPRoute{}, + &nkgAPI.NginxGateway{}, ). Build() @@ -62,6 +65,7 @@ var _ = Describe("Updater", func() { gc *v1beta1.GatewayClass gw, ignoredGw *v1beta1.Gateway hr *v1beta1.HTTPRoute + ng *nkgAPI.NginxGateway ipAddrType = v1beta1.IPAddressType addr = v1beta1.GatewayAddress{ Type: &ipAddrType, @@ -105,6 +109,14 @@ var _ = Describe("Updater", func() { }, }, }, + NginxGatewayStatus: status.NginxGatewayStatus{ + NSName: types.NamespacedName{ + Namespace: "nginx-gateway", + Name: "nginx-gateway-config", + }, + ObservedGeneration: 0, + Conditions: status.CreateTestConditions("Test"), + }, } } @@ -261,6 +273,16 @@ var _ = Describe("Updater", func() { APIVersion: "gateway.networking.k8s.io/v1beta1", }, } + ng = &nkgAPI.NginxGateway{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "nginx-gateway", + Name: "nginx-gateway-config", + }, + TypeMeta: metav1.TypeMeta{ + Kind: "NginxGateway", + APIVersion: "gateway.nginx.org/v1alpha1", + }, + } }) It("should create resources in the API server", func() { @@ -268,6 +290,7 @@ var _ = Describe("Updater", func() { Expect(client.Create(context.Background(), gw)).Should(Succeed()) Expect(client.Create(context.Background(), ignoredGw)).Should(Succeed()) Expect(client.Create(context.Background(), hr)).Should(Succeed()) + Expect(client.Create(context.Background(), ng)).Should(Succeed()) }) It("should update statuses", func() { @@ -329,6 +352,34 @@ var _ = Describe("Updater", func() { Expect(helpers.Diff(expectedHR, latestHR)).To(BeEmpty()) }) + It("should have the updated status of NginxGateway in the API server", func() { + latestNG := &nkgAPI.NginxGateway{} + expectedNG := &nkgAPI.NginxGateway{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "nginx-gateway", + Name: "nginx-gateway-config", + }, + TypeMeta: metav1.TypeMeta{ + Kind: "NginxGateway", + APIVersion: "gateway.nginx.org/v1alpha1", + }, + Status: nkgAPI.NginxGatewayStatus{ + Conditions: status.CreateExpectedAPIConditions("Test", 0, fakeClockTime), + }, + } + + err := client.Get( + context.Background(), + types.NamespacedName{Namespace: "nginx-gateway", Name: "nginx-gateway-config"}, + latestNG, + ) + Expect(err).Should(Not(HaveOccurred())) + + expectedNG.ResourceVersion = latestNG.ResourceVersion + + Expect(helpers.Diff(expectedNG, latestNG)).To(BeEmpty()) + }) + It("should update statuses with canceled context - function normally returns", func() { ctx, cancel := context.WithCancel(context.Background()) cancel() diff --git a/internal/mode/static/config/config.go b/internal/mode/static/config/config.go index 0bcd1c09ff..696ce82c2e 100644 --- a/internal/mode/static/config/config.go +++ b/internal/mode/static/config/config.go @@ -2,12 +2,17 @@ package config import ( "github.com/go-logr/logr" + "go.uber.org/zap" "k8s.io/apimachinery/pkg/types" ) type Config struct { + // GatewayCtlrName is the name of this controller. GatewayCtlrName string - Logger logr.Logger + // ConfigCRDName is the name of the NginxControlConfig CRD for this controller. + ConfigCRDName string + Logger logr.Logger + AtomicLevel zap.AtomicLevel // GatewayNsName is the namespaced name of a Gateway resource that the Gateway will use. // The Gateway will ignore all other Gateway resources. GatewayNsName *types.NamespacedName @@ -15,6 +20,8 @@ type Config struct { GatewayClassName string // PodIP is the IP address of this Pod. PodIP string + // Namespace is the Namespace of this Pod. + Namespace string // UpdateGatewayClassStatus enables updating the status of the GatewayClass resource. UpdateGatewayClassStatus bool } diff --git a/internal/mode/static/handler.go b/internal/mode/static/handler.go index 2fea22e0b1..0a5a6408b1 100644 --- a/internal/mode/static/handler.go +++ b/internal/mode/static/handler.go @@ -2,16 +2,27 @@ package static import ( "context" + "encoding/json" "fmt" "github.com/go-logr/logr" + "go.uber.org/zap" + "go.uber.org/zap/zapcore" + apiv1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/tools/record" + "sigs.k8s.io/controller-runtime/pkg/client" + nkgAPI "github.com/nginxinc/nginx-kubernetes-gateway/apis/v1alpha1" + "github.com/nginxinc/nginx-kubernetes-gateway/internal/framework/conditions" "github.com/nginxinc/nginx-kubernetes-gateway/internal/framework/events" "github.com/nginxinc/nginx-kubernetes-gateway/internal/framework/status" "github.com/nginxinc/nginx-kubernetes-gateway/internal/mode/static/nginx/config" "github.com/nginxinc/nginx-kubernetes-gateway/internal/mode/static/nginx/file" "github.com/nginxinc/nginx-kubernetes-gateway/internal/mode/static/nginx/runtime" "github.com/nginxinc/nginx-kubernetes-gateway/internal/mode/static/state" + staticConds "github.com/nginxinc/nginx-kubernetes-gateway/internal/mode/static/state/conditions" "github.com/nginxinc/nginx-kubernetes-gateway/internal/mode/static/state/dataplane" "github.com/nginxinc/nginx-kubernetes-gateway/internal/mode/static/state/resolver" ) @@ -30,14 +41,21 @@ type eventHandlerConfig struct { nginxRuntimeMgr runtime.Manager // statusUpdater updates statuses on Kubernetes resources. statusUpdater status.Updater + // eventRecorder records events for Kubernetes resources. + eventRecorder record.EventRecorder + // atomicLevel is used for updating the logger's log level. + atomicLevel zap.AtomicLevel // logger is the logger to be used by the EventHandler. logger logr.Logger + // controlConfigNSName is the NamespacedName of the NginxGateway config for this controller. + controlConfigNSName types.NamespacedName } // eventHandlerImpl implements EventHandler. // eventHandlerImpl is responsible for: // (1) Reconciling the Gateway API and Kubernetes built-in resources with the NGINX configuration. // (2) Keeping the statuses of the Gateway API resources updated. +// (3) Updating control plane configuration. type eventHandlerImpl struct { cfg eventHandlerConfig } @@ -53,9 +71,17 @@ func (h *eventHandlerImpl) HandleEventBatch(ctx context.Context, batch events.Ev for _, event := range batch { switch e := event.(type) { case *events.UpsertEvent: - h.cfg.processor.CaptureUpsertChange(e.Resource) + if cfg, ok := e.Resource.(*nkgAPI.NginxGateway); ok { + h.updateControlPlane(ctx, cfg) + } else { + h.cfg.processor.CaptureUpsertChange(e.Resource) + } case *events.DeleteEvent: - h.cfg.processor.CaptureDeleteChange(e.Type, e.NamespacedName) + if _, ok := e.Type.(*nkgAPI.NginxGateway); ok { + h.updateControlPlane(ctx, nil) + } else { + h.cfg.processor.CaptureDeleteChange(e.Type, e.NamespacedName) + } default: panic(fmt.Errorf("unknown event type %T", e)) } @@ -92,3 +118,81 @@ func (h *eventHandlerImpl) updateNginx(ctx context.Context, conf dataplane.Confi return nil } + +// updateControlPlane updates the control plane configuration with the given user spec. +// If any fields are not set within the user spec, the default configuration values are used. +func (h *eventHandlerImpl) updateControlPlane(ctx context.Context, cfg *nkgAPI.NginxGateway) { + // build up default configuration + defaultLogLevel := nkgAPI.ControllerLogLevelInfo + controlConfig := nkgAPI.NginxGatewaySpec{ + Logging: &nkgAPI.Logging{ + Level: &defaultLogLevel, + }, + } + + updateControlPlane := func() error { + // by marshaling the user config and then unmarshaling on top of the default config, + // we ensure that any unset user values are set with the default values + if cfg != nil { + cfgBytes, err := json.Marshal(cfg.Spec) + if err != nil { + return fmt.Errorf("error marshaling control config: %w", err) + } + + if err := json.Unmarshal(cfgBytes, &controlConfig); err != nil { + return fmt.Errorf("error unmarshaling control config: %w", err) + } + } else { + msg := "NginxGateway configuration was deleted; using defaults" + h.cfg.logger.Error(nil, msg) + h.cfg.eventRecorder.Event( + &nkgAPI.NginxGateway{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: h.cfg.controlConfigNSName.Namespace, + Name: h.cfg.controlConfigNSName.Name, + }, + }, + apiv1.EventTypeWarning, + "ResourceDeleted", + msg, + ) + } + + // set the log level + level, err := zapcore.ParseLevel(string(*controlConfig.Logging.Level)) + if err != nil { + return fmt.Errorf("error parsing log level string: %w", err) + } + h.cfg.atomicLevel.SetLevel(level) + + return nil + } + + var cond []conditions.Condition + if err := updateControlPlane(); err != nil { + msg := "Failed to update control plane configuration" + h.cfg.logger.Error(err, msg) + h.cfg.eventRecorder.Eventf( + cfg, + apiv1.EventTypeWarning, + "FailedUpdate", + "%s; "+msg, + err.Error(), + ) + cond = []conditions.Condition{staticConds.NewNginxGatewayInvalid(fmt.Sprintf("%s: %v", msg, err))} + } else { + cond = []conditions.Condition{staticConds.NewNginxGatewayValid()} + } + + if cfg != nil { + statuses := status.Statuses{ + NginxGatewayStatus: status.NginxGatewayStatus{ + NSName: client.ObjectKeyFromObject(cfg), + Conditions: cond, + ObservedGeneration: cfg.Generation, + }, + } + + h.cfg.statusUpdater.Update(ctx, statuses) + } +} diff --git a/internal/mode/static/handler_test.go b/internal/mode/static/handler_test.go index 3d937262ea..3bdccfae58 100644 --- a/internal/mode/static/handler_test.go +++ b/internal/mode/static/handler_test.go @@ -5,16 +5,24 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "go.uber.org/zap" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" - "sigs.k8s.io/controller-runtime/pkg/log/zap" + "k8s.io/client-go/tools/record" + ctlrZap "sigs.k8s.io/controller-runtime/pkg/log/zap" "sigs.k8s.io/gateway-api/apis/v1beta1" + nkgAPI "github.com/nginxinc/nginx-kubernetes-gateway/apis/v1alpha1" + "github.com/nginxinc/nginx-kubernetes-gateway/internal/framework/conditions" "github.com/nginxinc/nginx-kubernetes-gateway/internal/framework/events" + "github.com/nginxinc/nginx-kubernetes-gateway/internal/framework/helpers" + "github.com/nginxinc/nginx-kubernetes-gateway/internal/framework/status" "github.com/nginxinc/nginx-kubernetes-gateway/internal/framework/status/statusfakes" "github.com/nginxinc/nginx-kubernetes-gateway/internal/mode/static/nginx/config/configfakes" "github.com/nginxinc/nginx-kubernetes-gateway/internal/mode/static/nginx/file" "github.com/nginxinc/nginx-kubernetes-gateway/internal/mode/static/nginx/file/filefakes" "github.com/nginxinc/nginx-kubernetes-gateway/internal/mode/static/nginx/runtime/runtimefakes" + staticConds "github.com/nginxinc/nginx-kubernetes-gateway/internal/mode/static/state/conditions" "github.com/nginxinc/nginx-kubernetes-gateway/internal/mode/static/state/dataplane" "github.com/nginxinc/nginx-kubernetes-gateway/internal/mode/static/state/graph" "github.com/nginxinc/nginx-kubernetes-gateway/internal/mode/static/state/statefakes" @@ -28,6 +36,9 @@ var _ = Describe("eventHandler", func() { fakeNginxFileMgr *filefakes.FakeManager fakeNginxRuntimeMgr *runtimefakes.FakeManager fakeStatusUpdater *statusfakes.FakeUpdater + fakeEventRecorder *record.FakeRecorder + namespace = "nginx-gateway" + configName = "nginx-gateway-config" ) expectReconfig := func(expectedConf dataplane.Configuration, expectedFiles []file.File) { @@ -51,14 +62,18 @@ var _ = Describe("eventHandler", func() { fakeNginxFileMgr = &filefakes.FakeManager{} fakeNginxRuntimeMgr = &runtimefakes.FakeManager{} fakeStatusUpdater = &statusfakes.FakeUpdater{} + fakeEventRecorder = record.NewFakeRecorder(1) handler = newEventHandlerImpl(eventHandlerConfig{ - processor: fakeProcessor, - generator: fakeGenerator, - logger: zap.New(), - nginxFileMgr: fakeNginxFileMgr, - nginxRuntimeMgr: fakeNginxRuntimeMgr, - statusUpdater: fakeStatusUpdater, + processor: fakeProcessor, + generator: fakeGenerator, + logger: ctlrZap.New(), + atomicLevel: zap.NewAtomicLevel(), + nginxFileMgr: fakeNginxFileMgr, + nginxRuntimeMgr: fakeNginxRuntimeMgr, + statusUpdater: fakeStatusUpdater, + eventRecorder: fakeEventRecorder, + controlConfigNSName: types.NamespacedName{Namespace: namespace, Name: configName}, }) }) @@ -132,6 +147,68 @@ var _ = Describe("eventHandler", func() { }) }) + When("receiving control plane configuration updates", func() { + cfg := func(level nkgAPI.ControllerLogLevel) *nkgAPI.NginxGateway { + return &nkgAPI.NginxGateway{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: namespace, + Name: configName, + }, + Spec: nkgAPI.NginxGatewaySpec{ + Logging: &nkgAPI.Logging{ + Level: helpers.GetPointer(level), + }, + }, + } + } + + expStatuses := func(cond conditions.Condition) status.Statuses { + return status.Statuses{ + NginxGatewayStatus: status.NginxGatewayStatus{ + NSName: types.NamespacedName{ + Namespace: namespace, + Name: configName, + }, + Conditions: []conditions.Condition{cond}, + ObservedGeneration: 0, + }, + } + } + + It("handles a valid config", func() { + batch := []interface{}{&events.UpsertEvent{Resource: cfg(nkgAPI.ControllerLogLevelDebug)}} + handler.HandleEventBatch(context.Background(), batch) + + Expect(fakeStatusUpdater.UpdateCallCount()).Should(Equal(1)) + _, statuses := fakeStatusUpdater.UpdateArgsForCall(0) + Expect(statuses).To(Equal(expStatuses(staticConds.NewNginxGatewayValid()))) + }) + + It("handles an invalid config", func() { + batch := []interface{}{&events.UpsertEvent{Resource: cfg(nkgAPI.ControllerLogLevel("invalid"))}} + handler.HandleEventBatch(context.Background(), batch) + + Expect(fakeStatusUpdater.UpdateCallCount()).Should(Equal(1)) + _, statuses := fakeStatusUpdater.UpdateArgsForCall(0) + cond := staticConds.NewNginxGatewayInvalid( + "Failed to update control plane configuration: error parsing log level string: unrecognized level: \"invalid\"") + Expect(statuses).To(Equal(expStatuses(cond))) + Expect(len(fakeEventRecorder.Events)).To(Equal(1)) + event := <-fakeEventRecorder.Events + Expect(event).To(Equal( + "Warning FailedUpdate error parsing log level string: " + + "unrecognized level: \"invalid\"; Failed to update control plane configuration")) + }) + + It("handles a deleted config", func() { + batch := []interface{}{&events.DeleteEvent{Type: &nkgAPI.NginxGateway{}}} + handler.HandleEventBatch(context.Background(), batch) + Expect(len(fakeEventRecorder.Events)).To(Equal(1)) + event := <-fakeEventRecorder.Events + Expect(event).To(Equal("Warning ResourceDeleted NginxGateway configuration was deleted; using defaults")) + }) + }) + It("should panic for an unknown event type", func() { e := &struct{}{} diff --git a/internal/mode/static/manager.go b/internal/mode/static/manager.go index 50cd5b078a..7de6698012 100644 --- a/internal/mode/static/manager.go +++ b/internal/mode/static/manager.go @@ -16,6 +16,7 @@ import ( k8spredicate "sigs.k8s.io/controller-runtime/pkg/predicate" gatewayv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1" + nkgAPI "github.com/nginxinc/nginx-kubernetes-gateway/apis/v1alpha1" "github.com/nginxinc/nginx-kubernetes-gateway/internal/framework/controller" "github.com/nginxinc/nginx-kubernetes-gateway/internal/framework/controller/filter" "github.com/nginxinc/nginx-kubernetes-gateway/internal/framework/controller/index" @@ -44,6 +45,7 @@ func init() { utilruntime.Must(gatewayv1beta1.AddToScheme(scheme)) utilruntime.Must(apiv1.AddToScheme(scheme)) utilruntime.Must(discoveryV1.AddToScheme(scheme)) + utilruntime.Must(nkgAPI.AddToScheme(scheme)) } func StartManager(cfg config.Config) error { @@ -67,6 +69,11 @@ func StartManager(cfg config.Config) error { return fmt.Errorf("cannot build runtime manager: %w", err) } + controlConfigNSName := types.NamespacedName{ + Namespace: cfg.Namespace, + Name: cfg.ConfigCRDName, + } + // Note: for any new object type or a change to the existing one, // make sure to also update prepareFirstEventBatchPreparerArgs() controllerRegCfgs := []struct { @@ -118,6 +125,17 @@ func StartManager(cfg config.Config) error { { objectType: &gatewayv1beta1.ReferenceGrant{}, }, + { + objectType: &nkgAPI.NginxGateway{}, + options: func() []controller.Option { + if cfg.ConfigCRDName != "" { + return []controller.Option{ + controller.WithNamespacedNameFilter(filter.CreateSingleResourceFilter(controlConfigNSName)), + } + } + return nil + }(), + }, } ctx := ctlr.SetupSignalHandler() @@ -169,13 +187,16 @@ func StartManager(cfg config.Config) error { }) eventHandler := newEventHandlerImpl(eventHandlerConfig{ - processor: processor, - serviceResolver: resolver.NewServiceResolverImpl(mgr.GetClient()), - generator: configGenerator, - logger: cfg.Logger.WithName("eventHandler"), - nginxFileMgr: nginxFileMgr, - nginxRuntimeMgr: nginxRuntimeMgr, - statusUpdater: statusUpdater, + processor: processor, + serviceResolver: resolver.NewServiceResolverImpl(mgr.GetClient()), + generator: configGenerator, + logger: cfg.Logger.WithName("eventHandler"), + atomicLevel: cfg.AtomicLevel, + nginxFileMgr: nginxFileMgr, + nginxRuntimeMgr: nginxRuntimeMgr, + statusUpdater: statusUpdater, + eventRecorder: recorder, + controlConfigNSName: controlConfigNSName, }) objects, objectLists := prepareFirstEventBatchPreparerArgs(cfg.GatewayClassName, cfg.GatewayNsName) diff --git a/internal/mode/static/state/conditions/conditions.go b/internal/mode/static/state/conditions/conditions.go index 72f95a8846..3d27571afa 100644 --- a/internal/mode/static/state/conditions/conditions.go +++ b/internal/mode/static/state/conditions/conditions.go @@ -6,6 +6,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/gateway-api/apis/v1beta1" + nkgAPI "github.com/nginxinc/nginx-kubernetes-gateway/apis/v1alpha1" "github.com/nginxinc/nginx-kubernetes-gateway/internal/framework/conditions" ) @@ -541,3 +542,23 @@ func NewGatewayConflictNotProgrammed() conditions.Condition { Message: GatewayMessageGatewayConflict, } } + +// NewNginxGatewayValid returns a Condition that indicates that the NginxGateway config is valid. +func NewNginxGatewayValid() conditions.Condition { + return conditions.Condition{ + Type: string(nkgAPI.NginxGatewayConditionValid), + Status: metav1.ConditionTrue, + Reason: string(nkgAPI.NginxGatewayReasonValid), + Message: "NginxGateway is valid", + } +} + +// NewNginxGatewayInvalid returns a Condition that indicates that the NginxGateway config is invalid. +func NewNginxGatewayInvalid(msg string) conditions.Condition { + return conditions.Condition{ + Type: string(nkgAPI.NginxGatewayConditionValid), + Status: metav1.ConditionFalse, + Reason: string(nkgAPI.NginxGatewayReasonInvalid), + Message: msg, + } +} diff --git a/tools.go b/tools.go index 97875e7239..da60ee9839 100644 --- a/tools.go +++ b/tools.go @@ -8,4 +8,5 @@ package tools import ( _ "github.com/maxbrunsfeld/counterfeiter/v6" + _ "sigs.k8s.io/controller-tools/cmd/controller-gen" ) From 041fea90d936ceef4455ed7104b108936b2b0a31 Mon Sep 17 00:00:00 2001 From: Saylor Berman Date: Thu, 10 Aug 2023 15:27:19 -0600 Subject: [PATCH 2/5] Address all of the code reviews --- apis/v1alpha1/nginxgateway_types.go | 10 +- cmd/gateway/commands.go | 17 +-- .../provisioner/static-deployment.yaml | 2 +- deploy/helm-chart/README.md | 3 +- .../crds/gateway.nginx.org_nginxgateways.yaml | 4 + deploy/helm-chart/templates/_helpers.tpl | 8 ++ .../helm-chart/templates/control-config.yaml | 4 +- deploy/helm-chart/templates/deployment.yaml | 2 +- deploy/helm-chart/templates/rbac.yaml | 1 + deploy/helm-chart/values.yaml | 12 +- deploy/manifests/nginx-gateway.yaml | 7 +- docs/cli-help.md | 7 +- docs/control-plane-configuration.md | 37 ++++-- examples/advanced-routing/gateway.yaml | 2 - examples/cafe-example/gateway.yaml | 2 - examples/cross-namespace-routing/gateway.yaml | 2 - examples/http-header-filter/gateway.yaml | 2 - examples/https-termination/gateway.yaml | 2 - examples/traffic-splitting/gateway.yaml | 2 - internal/framework/status/statuses.go | 6 +- internal/framework/status/updater.go | 4 +- internal/framework/status/updater_test.go | 8 +- internal/mode/static/config/config.go | 10 +- internal/mode/static/config_updater.go | 105 ++++++++++++++++++ internal/mode/static/handler.go | 79 +++---------- internal/mode/static/handler_test.go | 18 +-- internal/mode/static/manager.go | 71 ++++++++---- 27 files changed, 278 insertions(+), 149 deletions(-) create mode 100644 internal/mode/static/config_updater.go diff --git a/apis/v1alpha1/nginxgateway_types.go b/apis/v1alpha1/nginxgateway_types.go index fb452cc9d7..463d2ceb04 100644 --- a/apis/v1alpha1/nginxgateway_types.go +++ b/apis/v1alpha1/nginxgateway_types.go @@ -63,6 +63,9 @@ const ( // NginxGatewayStatus defines the state of the NginxGateway. type NginxGatewayStatus struct { // +optional + // +listType=map + // +listMapKey=type + // +kubebuilder:validation:MaxItems=8 Conditions []metav1.Condition `json:"conditions,omitempty"` } @@ -75,12 +78,13 @@ type NginxGatewayConditionType string type NginxGatewayConditionReason string const ( - // This condition is true when the NginxGateway configuration is syntactically and semantically valid. + // NginxGatewayConditionValid is a condition that is true when the NginxGateway + // configuration is syntactically and semantically valid. NginxGatewayConditionValid NginxGatewayConditionType = "Valid" - // This reason is used with the "Valid" condition when the condition is True. + // NginxGatewayReasonValid is a reason that is used with the "Valid" condition when the condition is True. NginxGatewayReasonValid NginxGatewayConditionReason = "Valid" - // This reason is used with the "Valid" condition when the condition is False. + // NginxGatewayReasonInvalid is a reason that is used with the "Valid" condition when the condition is False. NginxGatewayReasonInvalid NginxGatewayConditionReason = "Invalid" ) diff --git a/cmd/gateway/commands.go b/cmd/gateway/commands.go index ad969f46c8..7627ee5fdc 100644 --- a/cmd/gateway/commands.go +++ b/cmd/gateway/commands.go @@ -120,7 +120,9 @@ func createStaticModeCommand() *cobra.Command { // flag values gateway := namespacedNameValue{} var updateGCStatus bool - var configCRDName string + configName := stringValidatingValue{ + validator: validateResourceName, + } cmd := &cobra.Command{ Use: "static-mode", @@ -153,7 +155,7 @@ func createStaticModeCommand() *cobra.Command { conf := config.Config{ GatewayCtlrName: gatewayCtlrName.value, - ConfigCRDName: configCRDName, + ConfigName: configName.String(), Logger: logger, AtomicLevel: atom, GatewayClassName: gatewayClassName.value, @@ -181,11 +183,12 @@ func createStaticModeCommand() *cobra.Command { "equal, it will choose the resource that appears first in alphabetical order by {namespace}/{name}.", ) - cmd.Flags().StringVar( - &configCRDName, - "nginx-gateway-config-name", - "nginx-gateway-config", - `The name of the NginxGateway CRD to be used for this controller's dynamic configuration.`, + cmd.Flags().VarP( + &configName, + "config", + "c", + `The name of the NginxGateway resource to be used for this controller's dynamic configuration.`+ + ` Lives in the same Namespace as the controller.`, ) cmd.Flags().BoolVar( diff --git a/conformance/provisioner/static-deployment.yaml b/conformance/provisioner/static-deployment.yaml index efd14539f6..b5e25bc37b 100644 --- a/conformance/provisioner/static-deployment.yaml +++ b/conformance/provisioner/static-deployment.yaml @@ -27,7 +27,7 @@ spec: - static-mode - --gateway-ctlr-name=gateway.nginx.org/nginx-gateway-controller - --gatewayclass=nginx - - --nginx-gateway-config-name=nginx-gateway-config + - --config=nginx-gateway-config env: - name: POD_IP valueFrom: diff --git a/deploy/helm-chart/README.md b/deploy/helm-chart/README.md index e5b5794966..53f5b05a12 100644 --- a/deploy/helm-chart/README.md +++ b/deploy/helm-chart/README.md @@ -129,8 +129,9 @@ The following tables lists the configurable parameters of the NGINX Kubernetes G | `nginxGateway.image.tag` | The tag for the NGINX Kubernetes Gateway image. | edge | | `nginxGateway.image.pullPolicy` | The `imagePullPolicy` for the NGINX Kubernetes Gateway image. | Always | | `nginxGateway.gatewayClassName` | The name of the GatewayClass for the NGINX Kubernetes Gateway deployment. | nginx | -| `nginxGateway.gatewayControllerName` | The name of the Gateway controller. The controller name must be of the form: DOMAIN/PATH. The controller's domain is k8s-gateway.nginx.org. | k8s-gateway.nginx.org/nginx-gateway-controller | +| `nginxGateway.gatewayControllerName` | The name of the Gateway controller. The controller name must be of the form: DOMAIN/PATH. The controller's domain is gateway.nginx.org. | gateway.nginx.org/nginx-gateway-controller | | `nginxGateway.kind` | The kind of the NGINX Kubernetes Gateway installation - currently, only Deployment is supported. | deployment | +|`nginxGateway.config` | The dynamic configuration for the control plane that is contained in the NginxGateway resource | [See nginxGateway.config section](values.yaml) | | `nginx.image.repository` | The repository for the NGINX image. | ghcr.io/nginxinc/nginx-kubernetes-gateway/nginx | | `nginx.image.tag` | The tag for the NGINX image. | edge | | `nginx.image.pullPolicy` | The `imagePullPolicy` for the NGINX image. | Always | diff --git a/deploy/helm-chart/crds/gateway.nginx.org_nginxgateways.yaml b/deploy/helm-chart/crds/gateway.nginx.org_nginxgateways.yaml index 3bba63cdcf..2a3360ce0a 100644 --- a/deploy/helm-chart/crds/gateway.nginx.org_nginxgateways.yaml +++ b/deploy/helm-chart/crds/gateway.nginx.org_nginxgateways.yaml @@ -119,7 +119,11 @@ spec: - status - type type: object + maxItems: 8 type: array + x-kubernetes-list-map-keys: + - type + x-kubernetes-list-type: map type: object required: - spec diff --git a/deploy/helm-chart/templates/_helpers.tpl b/deploy/helm-chart/templates/_helpers.tpl index c067c5aa33..acdc0b16e9 100644 --- a/deploy/helm-chart/templates/_helpers.tpl +++ b/deploy/helm-chart/templates/_helpers.tpl @@ -23,6 +23,14 @@ If release name contains chart name it will be used as a full name. {{- end }} {{- end }} +{{/* +Create control plane config name. +*/}} +{{- define "nginx-gateway.config-name" -}} +{{- $name := default .Release.Name .Values.nameOverride }} +{{- printf "%s-config" $name | trunc 63 | trimSuffix "-" }} +{{- end }} + {{/* Create chart name and version as used by the chart label. */}} diff --git a/deploy/helm-chart/templates/control-config.yaml b/deploy/helm-chart/templates/control-config.yaml index 19076115cb..0385e44a02 100644 --- a/deploy/helm-chart/templates/control-config.yaml +++ b/deploy/helm-chart/templates/control-config.yaml @@ -1,9 +1,9 @@ apiVersion: gateway.nginx.org/v1alpha1 kind: NginxGateway metadata: - name: {{ .Values.nginxGateway.crdConfig.name }} + name: {{ include "nginx-gateway.config-name" . }} namespace: {{ .Release.Namespace }} labels: {{- include "nginx-gateway.labels" . | nindent 4 }} spec: - {{- toYaml .Values.nginxGateway.crdConfig.spec | nindent 2 }} + {{- toYaml .Values.nginxGateway.config | nindent 2 }} diff --git a/deploy/helm-chart/templates/deployment.yaml b/deploy/helm-chart/templates/deployment.yaml index 7129599e25..e89ba7bcb5 100644 --- a/deploy/helm-chart/templates/deployment.yaml +++ b/deploy/helm-chart/templates/deployment.yaml @@ -22,7 +22,7 @@ spec: - static-mode - --gateway-ctlr-name={{ .Values.nginxGateway.gatewayControllerName }} - --gatewayclass={{ .Values.nginxGateway.gatewayClassName }} - - --nginx-gateway-config-name={{ .Values.nginxGateway.crdConfig.name }} + - --config={{ include "nginx-gateway.config-name" . }} env: - name: POD_IP valueFrom: diff --git a/deploy/helm-chart/templates/rbac.yaml b/deploy/helm-chart/templates/rbac.yaml index 8306e90fca..6bb156898f 100644 --- a/deploy/helm-chart/templates/rbac.yaml +++ b/deploy/helm-chart/templates/rbac.yaml @@ -68,6 +68,7 @@ rules: resources: - nginxgateways verbs: + - get - list - watch - apiGroups: diff --git a/deploy/helm-chart/values.yaml b/deploy/helm-chart/values.yaml index ba4f6f42ff..d616a630cb 100644 --- a/deploy/helm-chart/values.yaml +++ b/deploy/helm-chart/values.yaml @@ -8,13 +8,11 @@ nginxGateway: ## The name of the Gateway controller. The controller name must be of the form: DOMAIN/PATH. The controller's domain ## is gateway.nginx.org. gatewayControllerName: gateway.nginx.org/nginx-gateway-controller - ## The dynamic configuration for the control plane that is contained in the NginxGateway CRD. - crdConfig: - name: nginx-gateway-config - spec: - logging: - ## Log level. Supported values "info", "debug", "error". - level: info + ## The dynamic configuration for the control plane that is contained in the NginxGateway resource. + config: + logging: + ## Log level. Supported values "info", "debug", "error". + level: info image: ## The NGINX Kubernetes Gateway image to use diff --git a/deploy/manifests/nginx-gateway.yaml b/deploy/manifests/nginx-gateway.yaml index f622d9374e..37977fc429 100644 --- a/deploy/manifests/nginx-gateway.yaml +++ b/deploy/manifests/nginx-gateway.yaml @@ -125,7 +125,11 @@ spec: - status - type type: object + maxItems: 8 type: array + x-kubernetes-list-map-keys: + - type + x-kubernetes-list-type: map type: object required: - spec @@ -212,6 +216,7 @@ rules: resources: - nginxgateways verbs: + - get - list - watch - apiGroups: @@ -267,7 +272,7 @@ spec: - static-mode - --gateway-ctlr-name=gateway.nginx.org/nginx-gateway-controller - --gatewayclass=nginx - - --nginx-gateway-config-name=nginx-gateway-config + - --config=nginx-gateway-config env: - name: POD_IP valueFrom: diff --git a/docs/cli-help.md b/docs/cli-help.md index aa2e8f3ea8..df780388d6 100644 --- a/docs/cli-help.md +++ b/docs/cli-help.md @@ -16,7 +16,8 @@ Flags: | Name | Type | Description | |------------------------------|----------|---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| -| `gateway-ctlr-name` | `string` | The name of the Gateway controller. The controller name must be of the form: `DOMAIN/PATH`. The controller's domain is `gateway.nginx.org`. | -| `gatewayclass` | `string` | The name of the GatewayClass resource. Every NGINX Gateway must have a unique corresponding GatewayClass resource. | +| `gateway-ctlr-name` | `string` | The name of the Gateway controller. The controller name must be of the form: `DOMAIN/PATH`. The controller's domain is `gateway.nginx.org`. | +| `gatewayclass` | `string` | The name of the GatewayClass resource. Every NGINX Gateway must have a unique corresponding GatewayClass resource. | | `gateway` | `string` | The namespaced name of the Gateway resource to use. Must be of the form: `NAMESPACE/NAME`. If not specified, the control plane will process all Gateways for the configured GatewayClass. However, among them, it will choose the oldest resource by creation timestamp. If the timestamps are equal, it will choose the resource that appears first in alphabetical order by {namespace}/{name}. | -| `update-gatewayclass-status` | `bool` | Update the status of the GatewayClass resource. (default true) | +| `config` | `string` | The name of the NginxGateway resource to be used for this controller's dynamic configuration. Lives in the same Namespace as the controller. | +| `update-gatewayclass-status` | `bool` | Update the status of the GatewayClass resource. (default true) | diff --git a/docs/control-plane-configuration.md b/docs/control-plane-configuration.md index 6292b971ce..e2372ed51a 100644 --- a/docs/control-plane-configuration.md +++ b/docs/control-plane-configuration.md @@ -5,22 +5,35 @@ This document describes how to dynamically update the NGINX Kubernetes Gateway c ## Overview NGINX Kubernetes Gateway offers a way to update the control plane configuration dynamically without the need for a -restart. These configuration options include: +restart. The control plane configuration is stored in the NginxGateway custom resource. This resource is created +during the installation of NGINX Kubernetes Gateway. -| Option | Available values | Default value | -|---------------|--------------------|---------------| -| Logging Level | info, debug, error | info | +If using manifests, the default name of the resource is `nginx-gateway-config`. If using Helm, the default name +of the resource is `-config`. It is deployed in the same Namespace as the controller +(default `nginx-gateway`). +The control plane only watches this single instance of the custom resource. If the resource is invalid per the OpenAPI +schema, the Kubernetes API server will reject the changes. If the resource is deleted or deemed invalid by NGINX +Kubernetes Gateway, an error Event is created in the `nginx-gateway` Namespace, and the default values will be used by +the control plane for its configuration. -The control plane configuration is stored in the NginxGateway custom resource. This resource is created during the -installation of NGINX Kubernetes Gateway. The default name of the resource is `nginx-gateway-config` and is deployed -in the same Namespace as the controller (`nginx-gateway`). +### Spec -The control plane only watches this single instance of the custom resource. If the resource is deleted or invalid, an -error is emitted and the default values will be used by the control plane for its configuration. +| name | description | type | required | +|---------|-----------------------------------------------------------------|--------------------------|----------| +| logging | Logging defines logging related settings for the control plane. | [logging](#speclogging) | no | + +### Spec.Logging + +| name | description | type | required | +|-------|------------------------------------------------------------------------|--------|----------| +| level | Level defines the logging level. Supported values: info, debug, error. | string | no | ## Viewing and Updating the Configuration +> For the following examples, the name `nginx-gateway-config` should be updated to the name of the resource that +> was created by your installation. + To view the current configuration: ```shell @@ -35,3 +48,9 @@ kubectl -n nginx-gateway edit nginxgateways nginx-gateway-config This will open the configuration in your default editor. You can then update and save the configuration, which is applied automatically to the control plane. + +To view the status of the configuration: + +```shell +kubectl -n nginx-gateway describe nginxgateways nginx-gateway-config +``` diff --git a/examples/advanced-routing/gateway.yaml b/examples/advanced-routing/gateway.yaml index e531e6e467..03566e3f99 100644 --- a/examples/advanced-routing/gateway.yaml +++ b/examples/advanced-routing/gateway.yaml @@ -2,8 +2,6 @@ apiVersion: gateway.networking.k8s.io/v1beta1 kind: Gateway metadata: name: gateway - labels: - domain: gateway.nginx.org spec: gatewayClassName: nginx listeners: diff --git a/examples/cafe-example/gateway.yaml b/examples/cafe-example/gateway.yaml index 9e507147f1..81601058c4 100644 --- a/examples/cafe-example/gateway.yaml +++ b/examples/cafe-example/gateway.yaml @@ -2,8 +2,6 @@ apiVersion: gateway.networking.k8s.io/v1beta1 kind: Gateway metadata: name: gateway - labels: - domain: gateway.nginx.org spec: gatewayClassName: nginx listeners: diff --git a/examples/cross-namespace-routing/gateway.yaml b/examples/cross-namespace-routing/gateway.yaml index 9e507147f1..81601058c4 100644 --- a/examples/cross-namespace-routing/gateway.yaml +++ b/examples/cross-namespace-routing/gateway.yaml @@ -2,8 +2,6 @@ apiVersion: gateway.networking.k8s.io/v1beta1 kind: Gateway metadata: name: gateway - labels: - domain: gateway.nginx.org spec: gatewayClassName: nginx listeners: diff --git a/examples/http-header-filter/gateway.yaml b/examples/http-header-filter/gateway.yaml index e531e6e467..03566e3f99 100644 --- a/examples/http-header-filter/gateway.yaml +++ b/examples/http-header-filter/gateway.yaml @@ -2,8 +2,6 @@ apiVersion: gateway.networking.k8s.io/v1beta1 kind: Gateway metadata: name: gateway - labels: - domain: gateway.nginx.org spec: gatewayClassName: nginx listeners: diff --git a/examples/https-termination/gateway.yaml b/examples/https-termination/gateway.yaml index f81b6948ff..2c5f187068 100644 --- a/examples/https-termination/gateway.yaml +++ b/examples/https-termination/gateway.yaml @@ -2,8 +2,6 @@ apiVersion: gateway.networking.k8s.io/v1beta1 kind: Gateway metadata: name: gateway - labels: - domain: gateway.nginx.org spec: gatewayClassName: nginx listeners: diff --git a/examples/traffic-splitting/gateway.yaml b/examples/traffic-splitting/gateway.yaml index e531e6e467..03566e3f99 100644 --- a/examples/traffic-splitting/gateway.yaml +++ b/examples/traffic-splitting/gateway.yaml @@ -2,8 +2,6 @@ apiVersion: gateway.networking.k8s.io/v1beta1 kind: Gateway metadata: name: gateway - labels: - domain: gateway.nginx.org spec: gatewayClassName: nginx listeners: diff --git a/internal/framework/status/statuses.go b/internal/framework/status/statuses.go index fe8d7d9530..8de537d981 100644 --- a/internal/framework/status/statuses.go +++ b/internal/framework/status/statuses.go @@ -24,7 +24,7 @@ type Statuses struct { GatewayClassStatuses GatewayClassStatuses GatewayStatuses GatewayStatuses HTTPRouteStatuses HTTPRouteStatuses - NginxGatewayStatus NginxGatewayStatus + NginxGatewayStatus *NginxGatewayStatus } // GatewayStatus holds the status of the winning Gateway resource. @@ -75,8 +75,8 @@ type GatewayClassStatus struct { // NginxGatewayStatus holds status-related information about the NginxGateway resource. type NginxGatewayStatus struct { - // NSName is the NamespacedName of the NginxGateway resource. - NSName types.NamespacedName + // NsName is the NamespacedName of the NginxGateway resource. + NsName types.NamespacedName // Conditions is the list of conditions for this NginxGateway. Conditions []conditions.Condition // ObservedGeneration is the generation of the resource that was processed. diff --git a/internal/framework/status/updater.go b/internal/framework/status/updater.go index 0020535a8f..02bbfbe01e 100644 --- a/internal/framework/status/updater.go +++ b/internal/framework/status/updater.go @@ -121,8 +121,8 @@ func (upd *updaterImpl) Update(ctx context.Context, statuses Statuses) { } ngStatus := statuses.NginxGatewayStatus - if len(ngStatus.Conditions) > 0 { - upd.update(ctx, ngStatus.NSName, &nkgAPI.NginxGateway{}, func(object client.Object) { + if ngStatus != nil { + upd.update(ctx, ngStatus.NsName, &nkgAPI.NginxGateway{}, func(object client.Object) { ng := object.(*nkgAPI.NginxGateway) ng.Status = nkgAPI.NginxGatewayStatus{ Conditions: convertConditions( diff --git a/internal/framework/status/updater_test.go b/internal/framework/status/updater_test.go index ea83e7687b..c4e4ccf426 100644 --- a/internal/framework/status/updater_test.go +++ b/internal/framework/status/updater_test.go @@ -109,12 +109,12 @@ var _ = Describe("Updater", func() { }, }, }, - NginxGatewayStatus: status.NginxGatewayStatus{ - NSName: types.NamespacedName{ + NginxGatewayStatus: &status.NginxGatewayStatus{ + NsName: types.NamespacedName{ Namespace: "nginx-gateway", Name: "nginx-gateway-config", }, - ObservedGeneration: 0, + ObservedGeneration: 3, Conditions: status.CreateTestConditions("Test"), }, } @@ -364,7 +364,7 @@ var _ = Describe("Updater", func() { APIVersion: "gateway.nginx.org/v1alpha1", }, Status: nkgAPI.NginxGatewayStatus{ - Conditions: status.CreateExpectedAPIConditions("Test", 0, fakeClockTime), + Conditions: status.CreateExpectedAPIConditions("Test", 3, fakeClockTime), }, } diff --git a/internal/mode/static/config/config.go b/internal/mode/static/config/config.go index 696ce82c2e..f29b45b1de 100644 --- a/internal/mode/static/config/config.go +++ b/internal/mode/static/config/config.go @@ -9,10 +9,12 @@ import ( type Config struct { // GatewayCtlrName is the name of this controller. GatewayCtlrName string - // ConfigCRDName is the name of the NginxControlConfig CRD for this controller. - ConfigCRDName string - Logger logr.Logger - AtomicLevel zap.AtomicLevel + // ConfigName is the name of the NginxGateway resource for this controller. + ConfigName string + // Logger is the Zap Logger used by all components. + Logger logr.Logger + // AtomicLevel is an atomically changeable, dynamic logging level. + AtomicLevel zap.AtomicLevel // GatewayNsName is the namespaced name of a Gateway resource that the Gateway will use. // The Gateway will ignore all other Gateway resources. GatewayNsName *types.NamespacedName diff --git a/internal/mode/static/config_updater.go b/internal/mode/static/config_updater.go new file mode 100644 index 0000000000..912ce68fbf --- /dev/null +++ b/internal/mode/static/config_updater.go @@ -0,0 +1,105 @@ +package static + +import ( + "encoding/json" + "fmt" + + "github.com/go-logr/logr" + "go.uber.org/zap" + "go.uber.org/zap/zapcore" + apiv1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/validation/field" + "k8s.io/client-go/tools/record" + + nkgAPI "github.com/nginxinc/nginx-kubernetes-gateway/apis/v1alpha1" + "github.com/nginxinc/nginx-kubernetes-gateway/internal/framework/helpers" +) + +// ZapLogLevelSetter defines an interface for setting the logging level of a zap logger. +type ZapLogLevelSetter interface { + SetLevel(string) error + Enabled(zapcore.Level) bool +} + +type zapSetterImpl struct { + atomicLevel zap.AtomicLevel +} + +func newZapLogLevelSetter(atomicLevel zap.AtomicLevel) zapSetterImpl { + return zapSetterImpl{ + atomicLevel: atomicLevel, + } +} + +// SetLevel sets the logging level for the zap logger. +func (z zapSetterImpl) SetLevel(level string) error { + parsedLevel, err := zapcore.ParseLevel(level) + if err != nil { + fieldErr := field.NotSupported( + field.NewPath("logging.level"), + level, + []string{ + string(nkgAPI.ControllerLogLevelInfo), + string(nkgAPI.ControllerLogLevelDebug), + string(nkgAPI.ControllerLogLevelError), + }) + return fieldErr + } + z.atomicLevel.SetLevel(parsedLevel) + + return nil +} + +// Enabled returns true if the given level is at or above the current level. +func (z zapSetterImpl) Enabled(level zapcore.Level) bool { + return z.atomicLevel.Enabled(level) +} + +// updateControlPlane updates the control plane configuration with the given user spec. +// If any fields are not set within the user spec, the default configuration values are used. +func updateControlPlane( + cfg *nkgAPI.NginxGateway, + logger logr.Logger, + eventRecorder record.EventRecorder, + configNSName types.NamespacedName, + logLevelSetter ZapLogLevelSetter, +) error { + // build up default configuration + controlConfig := nkgAPI.NginxGatewaySpec{ + Logging: &nkgAPI.Logging{ + Level: helpers.GetPointer(nkgAPI.ControllerLogLevelInfo), + }, + } + + // by marshaling the user config and then unmarshaling on top of the default config, + // we ensure that any unset user values are set with the default values + if cfg != nil { + cfgBytes, err := json.Marshal(cfg.Spec) + if err != nil { + return fmt.Errorf("error marshaling control config: %w", err) + } + + if err := json.Unmarshal(cfgBytes, &controlConfig); err != nil { + return fmt.Errorf("error unmarshaling control config: %w", err) + } + } else { + msg := "NginxGateway configuration was deleted; using defaults" + logger.Error(nil, msg) + eventRecorder.Event( + &nkgAPI.NginxGateway{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: configNSName.Namespace, + Name: configNSName.Name, + }, + }, + apiv1.EventTypeWarning, + "ResourceDeleted", + msg, + ) + } + + // set the log level + return logLevelSetter.SetLevel(string(*controlConfig.Logging.Level)) +} diff --git a/internal/mode/static/handler.go b/internal/mode/static/handler.go index 0a5a6408b1..8bfd5cf216 100644 --- a/internal/mode/static/handler.go +++ b/internal/mode/static/handler.go @@ -2,14 +2,10 @@ package static import ( "context" - "encoding/json" "fmt" "github.com/go-logr/logr" - "go.uber.org/zap" - "go.uber.org/zap/zapcore" apiv1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/tools/record" "sigs.k8s.io/controller-runtime/pkg/client" @@ -43,8 +39,8 @@ type eventHandlerConfig struct { statusUpdater status.Updater // eventRecorder records events for Kubernetes resources. eventRecorder record.EventRecorder - // atomicLevel is used for updating the logger's log level. - atomicLevel zap.AtomicLevel + // logLevelSetter is used to update the logging level. + logLevelSetter ZapLogLevelSetter // logger is the logger to be used by the EventHandler. logger logr.Logger // controlConfigNSName is the NamespacedName of the NginxGateway config for this controller. @@ -72,13 +68,13 @@ func (h *eventHandlerImpl) HandleEventBatch(ctx context.Context, batch events.Ev switch e := event.(type) { case *events.UpsertEvent: if cfg, ok := e.Resource.(*nkgAPI.NginxGateway); ok { - h.updateControlPlane(ctx, cfg) + h.updateControlPlaneAndSetStatus(ctx, cfg) } else { h.cfg.processor.CaptureUpsertChange(e.Resource) } case *events.DeleteEvent: if _, ok := e.Type.(*nkgAPI.NginxGateway); ok { - h.updateControlPlane(ctx, nil) + h.updateControlPlaneAndSetStatus(ctx, nil) } else { h.cfg.processor.CaptureDeleteChange(e.Type, e.NamespacedName) } @@ -119,63 +115,23 @@ func (h *eventHandlerImpl) updateNginx(ctx context.Context, conf dataplane.Confi return nil } -// updateControlPlane updates the control plane configuration with the given user spec. -// If any fields are not set within the user spec, the default configuration values are used. -func (h *eventHandlerImpl) updateControlPlane(ctx context.Context, cfg *nkgAPI.NginxGateway) { - // build up default configuration - defaultLogLevel := nkgAPI.ControllerLogLevelInfo - controlConfig := nkgAPI.NginxGatewaySpec{ - Logging: &nkgAPI.Logging{ - Level: &defaultLogLevel, - }, - } - - updateControlPlane := func() error { - // by marshaling the user config and then unmarshaling on top of the default config, - // we ensure that any unset user values are set with the default values - if cfg != nil { - cfgBytes, err := json.Marshal(cfg.Spec) - if err != nil { - return fmt.Errorf("error marshaling control config: %w", err) - } - - if err := json.Unmarshal(cfgBytes, &controlConfig); err != nil { - return fmt.Errorf("error unmarshaling control config: %w", err) - } - } else { - msg := "NginxGateway configuration was deleted; using defaults" - h.cfg.logger.Error(nil, msg) - h.cfg.eventRecorder.Event( - &nkgAPI.NginxGateway{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: h.cfg.controlConfigNSName.Namespace, - Name: h.cfg.controlConfigNSName.Name, - }, - }, - apiv1.EventTypeWarning, - "ResourceDeleted", - msg, - ) - } - - // set the log level - level, err := zapcore.ParseLevel(string(*controlConfig.Logging.Level)) - if err != nil { - return fmt.Errorf("error parsing log level string: %w", err) - } - h.cfg.atomicLevel.SetLevel(level) - - return nil - } - +// updateControlPlaneAndSetStatus updates the control plane configuration and then sets the status +// based on the outcome +func (h *eventHandlerImpl) updateControlPlaneAndSetStatus(ctx context.Context, cfg *nkgAPI.NginxGateway) { var cond []conditions.Condition - if err := updateControlPlane(); err != nil { + if err := updateControlPlane( + cfg, + h.cfg.logger, + h.cfg.eventRecorder, + h.cfg.controlConfigNSName, + h.cfg.logLevelSetter, + ); err != nil { msg := "Failed to update control plane configuration" h.cfg.logger.Error(err, msg) h.cfg.eventRecorder.Eventf( cfg, apiv1.EventTypeWarning, - "FailedUpdate", + "UpdateFailed", "%s; "+msg, err.Error(), ) @@ -186,13 +142,14 @@ func (h *eventHandlerImpl) updateControlPlane(ctx context.Context, cfg *nkgAPI.N if cfg != nil { statuses := status.Statuses{ - NginxGatewayStatus: status.NginxGatewayStatus{ - NSName: client.ObjectKeyFromObject(cfg), + NginxGatewayStatus: &status.NginxGatewayStatus{ + NsName: client.ObjectKeyFromObject(cfg), Conditions: cond, ObservedGeneration: cfg.Generation, }, } h.cfg.statusUpdater.Update(ctx, statuses) + h.cfg.logger.Info("Reconfigured control plane.") } } diff --git a/internal/mode/static/handler_test.go b/internal/mode/static/handler_test.go index 3bdccfae58..96490e16d6 100644 --- a/internal/mode/static/handler_test.go +++ b/internal/mode/static/handler_test.go @@ -68,7 +68,7 @@ var _ = Describe("eventHandler", func() { processor: fakeProcessor, generator: fakeGenerator, logger: ctlrZap.New(), - atomicLevel: zap.NewAtomicLevel(), + logLevelSetter: newZapLogLevelSetter(zap.NewAtomicLevel()), nginxFileMgr: fakeNginxFileMgr, nginxRuntimeMgr: fakeNginxRuntimeMgr, statusUpdater: fakeStatusUpdater, @@ -164,8 +164,8 @@ var _ = Describe("eventHandler", func() { expStatuses := func(cond conditions.Condition) status.Statuses { return status.Statuses{ - NginxGatewayStatus: status.NginxGatewayStatus{ - NSName: types.NamespacedName{ + NginxGatewayStatus: &status.NginxGatewayStatus{ + NsName: types.NamespacedName{ Namespace: namespace, Name: configName, }, @@ -176,12 +176,14 @@ var _ = Describe("eventHandler", func() { } It("handles a valid config", func() { - batch := []interface{}{&events.UpsertEvent{Resource: cfg(nkgAPI.ControllerLogLevelDebug)}} + batch := []interface{}{&events.UpsertEvent{Resource: cfg(nkgAPI.ControllerLogLevelError)}} handler.HandleEventBatch(context.Background(), batch) Expect(fakeStatusUpdater.UpdateCallCount()).Should(Equal(1)) _, statuses := fakeStatusUpdater.UpdateArgsForCall(0) Expect(statuses).To(Equal(expStatuses(staticConds.NewNginxGatewayValid()))) + Expect(handler.cfg.logLevelSetter.Enabled(zap.DebugLevel)).To(BeFalse()) + Expect(handler.cfg.logLevelSetter.Enabled(zap.ErrorLevel)).To(BeTrue()) }) It("handles an invalid config", func() { @@ -191,13 +193,15 @@ var _ = Describe("eventHandler", func() { Expect(fakeStatusUpdater.UpdateCallCount()).Should(Equal(1)) _, statuses := fakeStatusUpdater.UpdateArgsForCall(0) cond := staticConds.NewNginxGatewayInvalid( - "Failed to update control plane configuration: error parsing log level string: unrecognized level: \"invalid\"") + "Failed to update control plane configuration: logging.level: Unsupported value: " + + "\"invalid\": supported values: \"info\", \"debug\", \"error\"") Expect(statuses).To(Equal(expStatuses(cond))) Expect(len(fakeEventRecorder.Events)).To(Equal(1)) event := <-fakeEventRecorder.Events Expect(event).To(Equal( - "Warning FailedUpdate error parsing log level string: " + - "unrecognized level: \"invalid\"; Failed to update control plane configuration")) + "Warning UpdateFailed logging.level: Unsupported value: " + + "\"invalid\": supported values: \"info\", \"debug\", \"error\"; " + + "Failed to update control plane configuration")) }) It("handles a deleted config", func() { diff --git a/internal/mode/static/manager.go b/internal/mode/static/manager.go index 7de6698012..f7534da873 100644 --- a/internal/mode/static/manager.go +++ b/internal/mode/static/manager.go @@ -1,15 +1,18 @@ package static import ( + "context" "fmt" "time" + "github.com/go-logr/logr" apiv1 "k8s.io/api/core/v1" discoveryV1 "k8s.io/api/discovery/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" utilruntime "k8s.io/apimachinery/pkg/util/runtime" + "k8s.io/client-go/tools/record" ctlr "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/manager" @@ -69,17 +72,17 @@ func StartManager(cfg config.Config) error { return fmt.Errorf("cannot build runtime manager: %w", err) } - controlConfigNSName := types.NamespacedName{ - Namespace: cfg.Namespace, - Name: cfg.ConfigCRDName, - } + recorderName := fmt.Sprintf("nginx-kubernetes-gateway-%s", cfg.GatewayClassName) + recorder := mgr.GetEventRecorderFor(recorderName) + logLevelSetter := newZapLogLevelSetter(cfg.AtomicLevel) // Note: for any new object type or a change to the existing one, // make sure to also update prepareFirstEventBatchPreparerArgs() - controllerRegCfgs := []struct { + type ctlrCfg struct { objectType client.Object options []controller.Option - }{ + } + controllerRegCfgs := []ctlrCfg{ { objectType: &gatewayv1beta1.GatewayClass{}, options: []controller.Option{ @@ -125,17 +128,28 @@ func StartManager(cfg config.Config) error { { objectType: &gatewayv1beta1.ReferenceGrant{}, }, - { - objectType: &nkgAPI.NginxGateway{}, - options: func() []controller.Option { - if cfg.ConfigCRDName != "" { - return []controller.Option{ - controller.WithNamespacedNameFilter(filter.CreateSingleResourceFilter(controlConfigNSName)), - } - } - return nil - }(), - }, + } + + controlConfigNSName := types.NamespacedName{ + Namespace: cfg.Namespace, + Name: cfg.ConfigName, + } + if cfg.ConfigName != "" { + controllerRegCfgs = append(controllerRegCfgs, + ctlrCfg{ + objectType: &nkgAPI.NginxGateway{}, + options: []controller.Option{ + controller.WithNamespacedNameFilter(filter.CreateSingleResourceFilter(controlConfigNSName)), + }, + }) + if err := setInitialConfig( + mgr.GetAPIReader(), + logger, recorder, + logLevelSetter, + controlConfigNSName, + ); err != nil { + return fmt.Errorf("error setting initial control plane configuration: %w", err) + } } ctx := ctlr.SetupSignalHandler() @@ -147,9 +161,6 @@ func StartManager(cfg config.Config) error { } } - recorderName := fmt.Sprintf("nginx-kubernetes-gateway-%s", cfg.GatewayClassName) - recorder := mgr.GetEventRecorderFor(recorderName) - processor := state.NewChangeProcessorImpl(state.ChangeProcessorConfig{ GatewayCtlrName: cfg.GatewayCtlrName, GatewayClassName: cfg.GatewayClassName, @@ -191,7 +202,7 @@ func StartManager(cfg config.Config) error { serviceResolver: resolver.NewServiceResolverImpl(mgr.GetClient()), generator: configGenerator, logger: cfg.Logger.WithName("eventHandler"), - atomicLevel: cfg.AtomicLevel, + logLevelSetter: logLevelSetter, nginxFileMgr: nginxFileMgr, nginxRuntimeMgr: nginxRuntimeMgr, statusUpdater: statusUpdater, @@ -244,3 +255,21 @@ func prepareFirstEventBatchPreparerArgs( return objects, objectLists } + +func setInitialConfig( + reader client.Reader, + logger logr.Logger, + eventRecorder record.EventRecorder, + logLevelSetter ZapLogLevelSetter, + configName types.NamespacedName, +) error { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + var config nkgAPI.NginxGateway + if err := reader.Get(ctx, configName, &config); err != nil { + return err + } + + return updateControlPlane(&config, logger, eventRecorder, configName, logLevelSetter) +} From 3adb59d1e6c03d4bf747cdf0e487f40dff051e25 Mon Sep 17 00:00:00 2001 From: Saylor Berman Date: Thu, 10 Aug 2023 15:33:51 -0600 Subject: [PATCH 3/5] Fix merge conflict --- .github/workflows/ci.yml | 6 ------ 1 file changed, 6 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 53625deffe..babb082dc5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -240,18 +240,12 @@ jobs: . --wait --create-namespace -<<<<<<< HEAD --set nginxGateway.image.repository=$(echo ${{ steps.nkg-meta.outputs.tags }} | cut -d ":" -f 1) --set nginxGateway.image.tag=$(echo ${{ steps.nkg-meta.outputs.tags }} | cut -d ":" -f 2) --set nginxGateway.image.pullPolicy=Never --set nginx.image.repository=$(echo ${{ steps.nginx-meta.outputs.tags }} | cut -d ":" -f 1) --set nginx.image.tag=$(echo ${{ steps.nginx-meta.outputs.tags }} | cut -d ":" -f 2) --set nginx.image.pullPolicy=Never -======= - --set nginxGateway.image.repository=$(echo ${{ steps.meta.outputs.tags }} | cut -d ":" -f 1) - --set nginxGateway.image.tag=$(echo ${{ steps.meta.outputs.tags }} | cut -d ":" -f 2) - --set nginxGateway.image.pullPolicy=Never ->>>>>>> 606489f (Dynamic control plane configuration support) --set service.type=NodePort -n nginx-gateway working-directory: ${{ github.workspace }}/deploy/helm-chart From d94566805df47477e634df414c658e96a2bf2a55 Mon Sep 17 00:00:00 2001 From: Saylor Berman Date: Fri, 11 Aug 2023 08:59:49 -0600 Subject: [PATCH 4/5] More code review --- apis/v1alpha1/nginxgateway_types.go | 6 +++--- cmd/gateway/commands_test.go | 17 +++++++++++++++++ .../{control-config.yaml => nginxgateway.yaml} | 0 deploy/manifests/nginx-gateway.yaml | 2 +- docs/control-plane-configuration.md | 5 +++-- 5 files changed, 24 insertions(+), 6 deletions(-) rename deploy/helm-chart/templates/{control-config.yaml => nginxgateway.yaml} (100%) diff --git a/apis/v1alpha1/nginxgateway_types.go b/apis/v1alpha1/nginxgateway_types.go index 463d2ceb04..77f4259083 100644 --- a/apis/v1alpha1/nginxgateway_types.go +++ b/apis/v1alpha1/nginxgateway_types.go @@ -50,13 +50,13 @@ type Logging struct { type ControllerLogLevel string const ( - // Info level for control plane logging. + // ControllerLogLevelInfo is the info level for control plane logging. ControllerLogLevelInfo ControllerLogLevel = "info" - // Debug level for control plane logging. + // ControllerLogLevelDebug is the debug level for control plane logging. ControllerLogLevelDebug ControllerLogLevel = "debug" - // Error level for control plane logging. + // ControllerLogLevelError is the error level for control plane logging. ControllerLogLevelError ControllerLogLevel = "error" ) diff --git a/cmd/gateway/commands_test.go b/cmd/gateway/commands_test.go index 42e570535c..59c9bd7bbc 100644 --- a/cmd/gateway/commands_test.go +++ b/cmd/gateway/commands_test.go @@ -116,6 +116,7 @@ func TestStaticModeCmdFlagValidation(t *testing.T) { name: "valid flags", args: []string{ "--gateway=nginx-gateway/nginx", + "--config=nginx-gateway-config", "--update-gatewayclass-status=true", }, wantErr: false, @@ -142,6 +143,22 @@ func TestStaticModeCmdFlagValidation(t *testing.T) { expectedErrPrefix: `invalid argument "nginx-gateway" for "--gateway" flag: invalid format; ` + "must be NAMESPACE/NAME", }, + { + name: "config is set to empty string", + args: []string{ + "--config=", + }, + wantErr: true, + expectedErrPrefix: `invalid argument "" for "-c, --config" flag: must be set`, + }, + { + name: "config is set to invalid string", + args: []string{ + "--config=!@#$", + }, + wantErr: true, + expectedErrPrefix: `invalid argument "!@#$" for "-c, --config" flag: invalid format`, + }, { name: "update-gatewayclass-status is set to empty string", args: []string{ diff --git a/deploy/helm-chart/templates/control-config.yaml b/deploy/helm-chart/templates/nginxgateway.yaml similarity index 100% rename from deploy/helm-chart/templates/control-config.yaml rename to deploy/helm-chart/templates/nginxgateway.yaml diff --git a/deploy/manifests/nginx-gateway.yaml b/deploy/manifests/nginx-gateway.yaml index 37977fc429..cb8a40e5d2 100644 --- a/deploy/manifests/nginx-gateway.yaml +++ b/deploy/manifests/nginx-gateway.yaml @@ -347,7 +347,7 @@ metadata: spec: controllerName: gateway.nginx.org/nginx-gateway-controller --- -# Source: nginx-kubernetes-gateway/templates/control-config.yaml +# Source: nginx-kubernetes-gateway/templates/nginxgateway.yaml apiVersion: gateway.nginx.org/v1alpha1 kind: NginxGateway metadata: diff --git a/docs/control-plane-configuration.md b/docs/control-plane-configuration.md index e2372ed51a..901160518d 100644 --- a/docs/control-plane-configuration.md +++ b/docs/control-plane-configuration.md @@ -14,8 +14,9 @@ of the resource is `-config`. It is deployed in the same Namespace The control plane only watches this single instance of the custom resource. If the resource is invalid per the OpenAPI schema, the Kubernetes API server will reject the changes. If the resource is deleted or deemed invalid by NGINX -Kubernetes Gateway, an error Event is created in the `nginx-gateway` Namespace, and the default values will be used by -the control plane for its configuration. +Kubernetes Gateway, a warning Event is created in the `nginx-gateway` Namespace, and the default values will be used by +the control plane for its configuration. Additionally, the control plane updates the status of the resource (if it exists) +to reflect whether it is valid or not. ### Spec From 27981b4492a484dee6f94f18ad95d0c64a2f6954 Mon Sep 17 00:00:00 2001 From: Saylor Berman Date: Fri, 11 Aug 2023 11:36:48 -0600 Subject: [PATCH 5/5] Fix some messaging --- internal/mode/static/config_updater.go | 2 +- internal/mode/static/handler.go | 2 +- internal/mode/static/handler_test.go | 7 ++++--- internal/mode/static/manager.go | 2 ++ 4 files changed, 8 insertions(+), 5 deletions(-) diff --git a/internal/mode/static/config_updater.go b/internal/mode/static/config_updater.go index 912ce68fbf..92eebc33a5 100644 --- a/internal/mode/static/config_updater.go +++ b/internal/mode/static/config_updater.go @@ -86,7 +86,7 @@ func updateControlPlane( } } else { msg := "NginxGateway configuration was deleted; using defaults" - logger.Error(nil, msg) + logger.Info(msg) eventRecorder.Event( &nkgAPI.NginxGateway{ ObjectMeta: metav1.ObjectMeta{ diff --git a/internal/mode/static/handler.go b/internal/mode/static/handler.go index 8bfd5cf216..3d9e783045 100644 --- a/internal/mode/static/handler.go +++ b/internal/mode/static/handler.go @@ -132,7 +132,7 @@ func (h *eventHandlerImpl) updateControlPlaneAndSetStatus(ctx context.Context, c cfg, apiv1.EventTypeWarning, "UpdateFailed", - "%s; "+msg, + msg+": %s", err.Error(), ) cond = []conditions.Condition{staticConds.NewNginxGatewayInvalid(fmt.Sprintf("%s: %v", msg, err))} diff --git a/internal/mode/static/handler_test.go b/internal/mode/static/handler_test.go index 96490e16d6..d3299e8ebf 100644 --- a/internal/mode/static/handler_test.go +++ b/internal/mode/static/handler_test.go @@ -199,9 +199,9 @@ var _ = Describe("eventHandler", func() { Expect(len(fakeEventRecorder.Events)).To(Equal(1)) event := <-fakeEventRecorder.Events Expect(event).To(Equal( - "Warning UpdateFailed logging.level: Unsupported value: " + - "\"invalid\": supported values: \"info\", \"debug\", \"error\"; " + - "Failed to update control plane configuration")) + "Warning UpdateFailed Failed to update control plane configuration: logging.level: Unsupported value: " + + "\"invalid\": supported values: \"info\", \"debug\", \"error\"")) + Expect(handler.cfg.logLevelSetter.Enabled(zap.InfoLevel)).To(BeTrue()) }) It("handles a deleted config", func() { @@ -210,6 +210,7 @@ var _ = Describe("eventHandler", func() { Expect(len(fakeEventRecorder.Events)).To(Equal(1)) event := <-fakeEventRecorder.Events Expect(event).To(Equal("Warning ResourceDeleted NginxGateway configuration was deleted; using defaults")) + Expect(handler.cfg.logLevelSetter.Enabled(zap.InfoLevel)).To(BeTrue()) }) }) diff --git a/internal/mode/static/manager.go b/internal/mode/static/manager.go index f7534da873..cf8519de03 100644 --- a/internal/mode/static/manager.go +++ b/internal/mode/static/manager.go @@ -271,5 +271,7 @@ func setInitialConfig( return err } + // status is not updated until the status updater's cache is started and the + // resource is processed by the controller return updateControlPlane(&config, logger, eventRecorder, configName, logLevelSetter) }