Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Weight changes Dynamic Reload #5212

Merged
merged 41 commits into from
Mar 19, 2024
Merged
Changes from 1 commit
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
4229818
generate split clients tests
Mar 7, 2024
edd08f1
Merge branch 'main' into weight-changes-without-reload
Mar 7, 2024
2db6149
go mod tidy
Mar 7, 2024
382001e
pre-commit had told me to remove this before ¯\_(ツ)_/¯
Mar 7, 2024
65843d4
capitalise VariableNamer for linter
Mar 7, 2024
de0ee07
fix helm value
Mar 7, 2024
37d209f
remove unnecessary printlns
Mar 7, 2024
f73ea63
add python test for weight changes without reload flag
Mar 8, 2024
ae43a02
Merge branch 'main' into weight-changes-without-reload
Mar 8, 2024
e7e1e0c
add vsr python test
Mar 11, 2024
e8d5d98
delete unnecessary logs
Mar 11, 2024
2bcf130
remove magic number
Mar 11, 2024
1e296c2
Merge branch 'main' into weight-changes-without-reload
Mar 11, 2024
b673f8b
move functionality from handlers.go to controller.go
Mar 13, 2024
381dd7e
Merge branch 'main' into weight-changes-without-reload
Mar 13, 2024
6dee37d
feature off by default
Mar 13, 2024
ecfac67
Merge branch 'main' into weight-changes-without-reload
Mar 13, 2024
106b30c
use go design reflect instead
Mar 13, 2024
1b530fb
remove redundant variable, add v3 logging
Mar 13, 2024
901fdbd
add variable
Mar 14, 2024
3fb9e5a
fix state_files folder permissions and add copier back
Mar 14, 2024
81b1ac7
Merge main
Mar 14, 2024
0049706
Revert "Merge main"
Mar 14, 2024
ad1ce82
Merge branch 'main' into weight-changes-without-reload
Mar 14, 2024
6272254
Merge branch 'main' into weight-changes-without-reload
Mar 14, 2024
bc4ea35
improvement of description of feature, disable v3 logging in python t…
Mar 15, 2024
e3703bd
description of feature
Mar 15, 2024
e752bfa
Test using many splits when using larger map bucket and variable buck…
Mar 15, 2024
900df19
wait before test
Mar 15, 2024
231ad88
isolate tests
Mar 15, 2024
526f4a8
further isolate tests
Mar 15, 2024
4094efe
Merge branch 'main' into weight-changes-without-reload
Mar 15, 2024
87b4311
fix virtual_server_test.go
Mar 15, 2024
14fc319
fix status and event processing
Mar 19, 2024
4f09779
Merge branch 'main' into weight-changes-without-reload
Mar 19, 2024
e149fb5
change to without-reload to dynamic reload
Mar 19, 2024
7523d4a
Merge branch 'main' into weight-changes-without-reload
Mar 19, 2024
00084c5
add warning log
Mar 19, 2024
e5ff2ea
Add link to configmap and improve logs
Mar 19, 2024
68a9cdf
Merge branch 'main' into weight-changes-without-reload
Mar 19, 2024
d396110
Merge branch 'main' into weight-changes-without-reload
Mar 19, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Merge main
Jim Ryan committed Mar 14, 2024
commit 81b1ac7ce0de8cd49fab87df61d980143a20ff39
29 changes: 29 additions & 0 deletions .github/scripts/variables.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
#!/usr/bin/env bash

if [ "$1" = "" ]; then
echo "ERROR: paramater needed"
exit 2
fi

INPUT=$1
ROOTDIR=$(git rev-parse --show-toplevel || echo ".")
if [ "$PWD" != "$ROOTDIR" ]; then
# shellcheck disable=SC2164
cd "$ROOTDIR";
fi

case $INPUT in
docker_md5)
docker_md5=$(find . -type f \( -name "Dockerfile" -o -name version.txt \) -not -path "./tests*" -exec md5sum {} + | LC_ALL=C sort | md5sum | awk '{ print $1 }')
echo "docker_md5=${docker_md5:0:8}"
;;

go_code_md5)
echo "go_code_md5=$(find . -type f \( -name "*.go" -o -name go.mod -o -name go.sum -o -name "*.tmpl" \) -not -path "./docs*" -exec md5sum {} + | LC_ALL=C sort | md5sum | awk '{ print $1 }')"
;;

*)
echo "ERROR: option not found"
exit 2
;;
esac
3 changes: 1 addition & 2 deletions .github/workflows/build-base-images.yml
Original file line number Diff line number Diff line change
@@ -32,8 +32,7 @@ jobs:
- name: Output Variables
id: vars
run: |
docker_md5=$(find . -type f \( -name "Dockerfile" -o -name version.txt \) -not -path "./tests*" -exec md5sum {} + | LC_ALL=C sort | md5sum | awk '{ print $1 }')
echo "docker_md5=${docker_md5:0:8}" >> $GITHUB_OUTPUT
./.github/scripts/variables.sh docker_md5 >> $GITHUB_OUTPUT
source .github/data/version.txt
echo "ic_version=${IC_VERSION}" >> $GITHUB_OUTPUT
cat $GITHUB_OUTPUT
2 changes: 1 addition & 1 deletion .github/workflows/cache-update.yml
Original file line number Diff line number Diff line change
@@ -28,7 +28,7 @@ jobs:
- name: Output Variables
id: vars
run: |
echo go_code_md5=$(find . -type f \( -name "*.go" -o -name go.mod -o -name go.sum -o -name "*.tmpl" -o -name .goreleaser.yml -o -name .github/data/version.txt \) -not -path "./docs*" -exec md5sum {} + | LC_ALL=C sort | md5sum | awk '{ print $1 }') >> $GITHUB_OUTPUT
./.github/scripts/variables.sh go_code_md5 >> $GITHUB_OUTPUT
source .github/data/version.txt
echo "chart_version=${HELM_CHART_VERSION}" >> $GITHUB_OUTPUT
cat $GITHUB_OUTPUT
5 changes: 2 additions & 3 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
@@ -81,7 +81,6 @@ jobs:
| jq -R -s -c 'split("\n")[:-1]')
echo "latest_kindest_node_versions=$kindest_versions" >> $GITHUB_OUTPUT
echo "go_path=$(go env GOPATH)" >> $GITHUB_OUTPUT
echo go_code_md5=$(find . -type f \( -name "*.go" -o -name go.mod -o -name go.sum -o -name "*.tmpl" -o -name .goreleaser.yml -o -name .github/data/version.txt \) -not -path "./docs*" -exec md5sum {} + | LC_ALL=C sort | md5sum | awk '{ print $1 }') >> $GITHUB_OUTPUT
source .github/data/version.txt
echo "ic_version=${IC_VERSION}" >> $GITHUB_OUTPUT
echo "chart_version=${HELM_CHART_VERSION}" >> $GITHUB_OUTPUT
@@ -94,8 +93,8 @@ jobs:
publish=true
fi
echo "publish=$publish" >> $GITHUB_OUTPUT
docker_md5=$(find . -type f \( -name "Dockerfile" -o -name version.txt \) -not -path "./tests*" -exec md5sum {} + | LC_ALL=C sort | md5sum | awk '{ print $1 }')
echo "docker_md5=${docker_md5:0:8}" >> $GITHUB_OUTPUT
./.github/scripts/variables.sh go_code_md5 >> $GITHUB_OUTPUT
./.github/scripts/variables.sh docker_md5 >> $GITHUB_OUTPUT
cat $GITHUB_OUTPUT
- name: Fetch Cached Binary Artifacts
8 changes: 7 additions & 1 deletion .github/workflows/update-docker-images.yml
Original file line number Diff line number Diff line change
@@ -30,6 +30,7 @@ jobs:
versions: ${{ steps.versions.outputs.matrix }}
go-md5: ${{ steps.md5.outputs.go_code_md5 }}
binary-cache-hit: ${{ steps.binary-cache.outputs.cache-hit }}
base-image-md5: ${{ steps.md5.outputs.docker_md5 }}
steps:
- name: Checkout Repository
uses: actions/checkout@9bb56186c3b09b4f86b1c65136769dd318469633 # v4.1.2
@@ -57,7 +58,9 @@ jobs:
- name: Set Go MD5sums
id: md5
run: echo go_code_md5=$(find . -type f \( -name "*.go" -o -name go.mod -o -name go.sum -o -name "*.tmpl" \) -not -path "./docs*" -exec md5sum {} + | LC_ALL=C sort | md5sum | awk '{ print $1 }') >> $GITHUB_OUTPUT
run: |
./.github/scripts/variables.sh go_code_md5 >> $GITHUB_OUTPUT
./.github/scripts/variables.sh docker_md5 >> $GITHUB_OUTPUT
- name: Fetch Cached Binary Artifacts
id: binary-cache
@@ -144,6 +147,7 @@ jobs:
image: debian
tag: ${{ needs.variables.outputs.kic-tag }}
go-md5: ${{ needs.variables.outputs.go-md5 }}
base-image-md5: ${{ needs.variables.outputs.base-image-md5 }}
permissions:
contents: read
actions: read
@@ -162,6 +166,7 @@ jobs:
image: alpine
tag: ${{ needs.variables.outputs.kic-tag }}
go-md5: ${{ needs.variables.outputs.go-md5 }}
base-image-md5: ${{ needs.variables.outputs.base-image-md5 }}
permissions:
contents: read
actions: read
@@ -180,6 +185,7 @@ jobs:
image: ubi
tag: ${{ needs.variables.outputs.kic-tag }}
go-md5: ${{ needs.variables.outputs.go-md5 }}
base-image-md5: ${{ needs.variables.outputs.base-image-md5 }}
permissions:
contents: read
actions: read
6 changes: 3 additions & 3 deletions charts/nginx-ingress/README.md
Original file line number Diff line number Diff line change
@@ -387,7 +387,7 @@ The following tables lists the configurable parameters of the NGINX Ingress Cont
|`controller.replicaCount` | The number of replicas of the Ingress Controller deployment. | 1 |
|`controller.ingressClass.name` | A class of the Ingress Controller. An IngressClass resource with the name equal to the class must be deployed. Otherwise, the Ingress Controller will fail to start. The Ingress Controller only processes resources that belong to its class - i.e. have the "ingressClassName" field resource equal to the class. The Ingress Controller processes all the VirtualServer/VirtualServerRoute/TransportServer resources that do not have the "ingressClassName" field for all versions of Kubernetes. | nginx |
|`controller.ingressClass.create` | Creates a new IngressClass object with the name `controller.ingressClass.name`. Set to `false` to use an existing ingressClass created using `kubectl` with the same name. If you use `helm upgrade`, do not change the values from the previous release as helm will delete IngressClass objects managed by helm. If you are upgrading from a release earlier than 3.4.3, do not set the value to false. | true |
|`controller.ingressClass.setAsDefaultIngress` | New Ingresses without an `"ingressClassName"` field specified will be assigned the class specified in `controller.ingressClass.name`. Requires `controller.ingressClass.create`. | false |
|`controller.ingressClass.setAsDefaultIngress` | New Ingresses without an `"ingressClassName"` field specified will be assigned the class specified in `controller.ingressClass.name`. Requires `controller.ingressClass.create`. | false |
|`controller.watchNamespace` | Comma separated list of namespaces the Ingress Controller should watch for resources. By default the Ingress Controller watches all namespaces. Mutually exclusive with `controller.watchNamespaceLabel`. Please note that if configuring multiple namespaces using the Helm cli `--set` option, the string needs to wrapped in double quotes and the commas escaped using a backslash - e.g. `--set controller.watchNamespace="default\,nginx-ingress"`. | "" |
|`controller.watchNamespaceLabel` | Configures the Ingress Controller to watch only those namespaces with label foo=bar. By default the Ingress Controller watches all namespaces. Mutually exclusive with `controller.watchNamespace`. | "" |
|`controller.watchSecretNamespace` | Comma separated list of namespaces the Ingress Controller should watch for resources of type Secret. If this arg is not configured, the Ingress Controller watches the same namespaces for all resources. See `controller.watchNamespace` and `controller.watchNamespaceLabel`. Please note that if configuring multiple namespaces using the Helm cli `--set` option, the string needs to wrapped in double quotes and the commas escaped using a backslash - e.g. `--set controller.watchSecretNamespace="default\,nginx-ingress"`. | "" |
@@ -466,11 +466,11 @@ The following tables lists the configurable parameters of the NGINX Ingress Cont
|`controller.podDisruptionBudget.maxUnavailable` | The number of Ingress Controller pods that can be unavailable. This is a mutually exclusive setting with "minAvailable". | 0 |
|`controller.strategy` | Specifies the strategy used to replace old Pods with new ones. Docs for [Deployment update strategy](https://kubernetes.io/docs/concepts/workloads/controllers/deployment/#strategy) and [Daemonset update strategy](https://kubernetes.io/docs/tasks/manage-daemon/update-daemon-set/#daemonset-update-strategy) | {} |
|`controller.disableIPV6` | Disable IPV6 listeners explicitly for nodes that do not support the IPV6 stack. | false |
|`controller.defaultHTTPListenerPort` | Sets the port for the HTTP `default_server` listener. | 80 |
|`controller.defaultHTTPListenerPort` | Sets the port for the HTTP `default_server` listener. | 80 |
|`controller.defaultHTTPSListenerPort` | Sets the port for the HTTPS `default_server` listener. | 443 |
|`controller.readOnlyRootFilesystem` | Configure root filesystem as read-only and add volumes for temporary data. Three major releases after 3.5.x this argument will be moved permanently to the `controller.securityContext` section. | false |
|`controller.enableSSLDynamicReload` | Enable lazy loading for SSL Certificates. | true |
|`controller.enableTelemetryReporting` | Enable telemetry reporting. | true |
|`controller.telemetryReporting.enable` | Enable telemetry reporting. | true |
|`controller.enableWeightChangesWithoutReload` | Enable weight changes without reloading the NGINX configuration. | false |
j1m-ryan marked this conversation as resolved.
Show resolved Hide resolved
|`rbac.create` | Configures RBAC. | true |
|`prometheus.create` | Expose NGINX or NGINX Plus metrics in the Prometheus format. | true |
2 changes: 1 addition & 1 deletion charts/nginx-ingress/templates/_helpers.tpl
Original file line number Diff line number Diff line change
@@ -277,7 +277,7 @@ Build the args for the service binary.
- -ready-status-port={{ .Values.controller.readyStatus.port }}
- -enable-latency-metrics={{ .Values.controller.enableLatencyMetrics }}
- -ssl-dynamic-reload={{ .Values.controller.enableSSLDynamicReload }}
- -enable-telemetry-reporting={{ .Values.controller.enableTelemetryReporting}}
- -enable-telemetry-reporting={{ .Values.controller.telemetryReporting.enable}}
- -weight-changes-without-reload={{ .Values.controller.enableWeightChangesWithoutReload}}
{{- if .Values.nginxAgent.enable }}
- -agent=true
7 changes: 7 additions & 0 deletions charts/nginx-ingress/templates/clusterrole.yaml
Original file line number Diff line number Diff line change
@@ -55,6 +55,13 @@ rules:
- nodes
verbs:
- list
- apiGroups:
- "apps"
resources:
- replicasets
- daemonsets
verbs:
- get
- apiGroups:
- networking.k8s.io
resources:
22 changes: 15 additions & 7 deletions charts/nginx-ingress/values.schema.json
Original file line number Diff line number Diff line change
@@ -1417,13 +1417,21 @@
true
]
},
"enableTelemetryReporting": {
"type": "boolean",
"default": true,
"title": "Enable telemetry reporting",
"examples": [
true
]
"telemetryReporting": {
"type": "object",
"default": {},
"title": "Configure telemetry reporting options",
"required": [],
"properties": {
"enable": {
"type": "boolean",
"default": true,
"title": "Enable telemetry reporting",
"examples": [
true
]
}
}
},
"enableWeightChangesWithoutReload": {
"type": "boolean",
6 changes: 4 additions & 2 deletions charts/nginx-ingress/values.yaml
Original file line number Diff line number Diff line change
@@ -487,8 +487,10 @@ controller:
## Enable dynamic reloading of certificates
enableSSLDynamicReload: true

## Enable telemetry reporting
enableTelemetryReporting: true
## Configure telemetry reporting options
telemetryReporting:
## Enable telemetry reporting
enable: true

## Enables weight changes without reloading the NGINX Configuration for two way splits in NGINX Plus.
## Enabling this feature may require increasing map_hash_bucket_size, map_hash_max_size,
23 changes: 0 additions & 23 deletions cmd/nginx-ingress/flags.go
Original file line number Diff line number Diff line change
@@ -1,15 +1,13 @@
package main

import (
"errors"
"flag"
"fmt"
"net"
"os"
"regexp"
"strconv"
"strings"
"time"

"github.com/golang/glog"
api_v1 "k8s.io/api/core/v1"
@@ -208,7 +206,6 @@ var (
enableDynamicSSLReload = flag.Bool(dynamicSSLReloadParam, true, "Enable reloading of SSL Certificates without restarting the NGINX process.")

enableTelemetryReporting = flag.Bool("enable-telemetry-reporting", true, "Enable gathering and reporting of product related telemetry.")
telemetryReportingPeriod = flag.String("telemetry-reporting-period", "24h", "Sets a telemetry reporting period.")

enableWeightChangesWithoutReload = flag.Bool(weightChangesWithoutReloadParam, false, "Enable changing weights of split clients without reloading NGINX. Requires -nginx-plus")

@@ -403,12 +400,6 @@ func validationChecks() {
glog.Fatalf("Invalid value for app-protect-log-level: %v", *appProtectLogLevel)
}
}

if telemetryReportingPeriod != nil {
if err := validateReportingPeriod(*telemetryReportingPeriod); err != nil {
glog.Fatalf("Invalid value for telemetry-reporting-period: %v", err)
}
}
}

// validateNamespaceNames validates the namespaces are in the correct format
@@ -514,17 +505,3 @@ func validateLocation(location string) error {
}
return nil
}

// validateReportingPeriod checks if the reporting period parameter can be parsed.
//
// This function will be deprecated in NIC v3.5. It is used only for demo and testing purpose.
func validateReportingPeriod(period string) error {
duration, err := time.ParseDuration(period)
if err != nil {
return err
}
if duration.Minutes() < 1 {
return errors.New("invalid reporting period, expected minimum 1m")
}
return nil
}
24 changes: 0 additions & 24 deletions cmd/nginx-ingress/flags_test.go
Original file line number Diff line number Diff line change
@@ -172,27 +172,3 @@ func TestValidateNamespaces(t *testing.T) {
}
}
}

func TestValidateReportingPeriodWithInvalidInput(t *testing.T) {
t.Parallel()

periods := []string{"", "-1", "1x", "abc", "-", "30s", "10ms", "0h"}
for _, p := range periods {
err := validateReportingPeriod(p)
if err == nil {
t.Errorf("want error on invalid period %s, got nil", p)
}
}
}

func TestValidateReportingPeriodWithValidInput(t *testing.T) {
t.Parallel()

periods := []string{"1m", "1h", "24h"}
for _, p := range periods {
err := validateReportingPeriod(p)
if err != nil {
t.Error(err)
}
}
}
1 change: 0 additions & 1 deletion cmd/nginx-ingress/main.go
Original file line number Diff line number Diff line change
@@ -212,7 +212,6 @@ func main() {
IsIPV6Disabled: *disableIPV6,
WatchNamespaceLabel: *watchNamespaceLabel,
EnableTelemetryReporting: *enableTelemetryReporting,
TelemetryReportingPeriod: *telemetryReportingPeriod,
NICVersion: version,
WeightChangesWithoutReload: *enableWeightChangesWithoutReload,
}
7 changes: 7 additions & 0 deletions deployments/rbac/rbac.yaml
Original file line number Diff line number Diff line change
@@ -11,6 +11,13 @@ rules:
- get
- list
- watch
- apiGroups:
- "apps"
resources:
- replicasets
- daemonsets
verbs:
- get
- apiGroups:
- ""
resources:
Original file line number Diff line number Diff line change
@@ -536,6 +536,14 @@ The default value is `true`.

<a name="cmdoption-ssl-dynamic-reload"></a>

### -enable-telemetry-reporting

Enable gathering and reporting of product related telemetry.

The default value is `true`.

<a name="cmdoption-enable-telemetry-reporting"></a>

### -weight-changes-without-reload

Enables the ability to change the weight of split clients with two splits without reloading NGINX.
Original file line number Diff line number Diff line change
@@ -22,10 +22,10 @@ h2 {
- A [Kubernetes Version Supported by the Ingress Controller](https://docs.nginx.com/nginx-ingress-controller/technical-specifications/#supported-kubernetes-versions)
- Helm 3.0+.
- If you’d like to use NGINX Plus:
- To pull from the F5 Container registry, configure a docker registry secret using your JWT token from the MyF5 portal by following the instructions from [here](https://docs.nginx.com/nginx-ingress-controller/installation/nic-images/using-the-jwt-token-docker-secret). Make sure to specify the secret using `controller.serviceAccount.imagePullSecretName` or `controller.serviceAccount.imagePullSecretsNames` parameter.
- Alternatively, pull an Ingress Controller image with NGINX Plus and push it to your private registry by following the instructions from [here]({{< relref "installation/nic-images/pulling-ingress-controller-image" >}}).
- Alternatively, you can build an Ingress Controller image with NGINX Plus and push it to your private registry by following the instructions from [here]({{< relref "installation/building-nginx-ingress-controller.md" >}}).
- Update the `controller.image.repository` field of the `values-plus.yaml` accordingly.
- To pull from the F5 Container registry, configure a docker registry secret using your JWT token from the MyF5 portal by following the instructions from [here](https://docs.nginx.com/nginx-ingress-controller/installation/nic-images/using-the-jwt-token-docker-secret). Make sure to specify the secret using `controller.serviceAccount.imagePullSecretName` or `controller.serviceAccount.imagePullSecretsNames` parameter.
- Alternatively, pull an Ingress Controller image with NGINX Plus and push it to your private registry by following the instructions from [here]({{< relref "installation/nic-images/pulling-ingress-controller-image" >}}).
- Alternatively, you can build an Ingress Controller image with NGINX Plus and push it to your private registry by following the instructions from [here]({{< relref "installation/building-nginx-ingress-controller.md" >}}).
- Update the `controller.image.repository` field of the `values-plus.yaml` accordingly.
- If you’d like to use App Protect DoS, please install App Protect DoS Arbitrator [helm chart](https://github.com/nginxinc/nap-dos-arbitrator-helm-chart). Make sure to install in the same namespace as the NGINX Ingress Controller. Note that if you install multiple NGINX Ingress Controllers in the same namespace, they will need to share the same Arbitrator because it is not possible to install more than one Arbitrator in a single namespace.

## CRDs
@@ -204,7 +204,7 @@ The steps you should follow depend on the Helm release name:
kubectl describe deployments -n <namespace>
```
Copy the key=value under `Selector`, such as:
Copy the key=value under `Selector`, such as:
```shell
Selector: app=nginx-ingress-nginx-ingress
@@ -225,7 +225,7 @@ The steps you should follow depend on the Helm release name:
--set controller.name=""
--set fullnameOverride="nginx-ingress-nginx-ingress"
```
It could look as follows:
It could look as follows:
```shell
helm upgrade nginx-ingress oci://ghcr.io/nginxinc/charts/nginx-ingress --version 0.19.0 --set controller.kind=deployment/daemonset --set controller.nginxplus=false/true --set controller.image.pullPolicy=Always --set serviceNameOverride="nginx-ingress-nginx-ingress" --set controller.name="" --set fullnameOverride="nginx-ingress-nginx-ingress" -f values.yaml
@@ -249,7 +249,7 @@ The steps you should follow depend on the Helm release name:
kubectl describe deployment/daemonset -n <namespace>
```
Copy the key=value under ```Selector```, such as:
Copy the key=value under ```Selector```, such as:
```shell
Selector: app=<helm_release_name>-nginx-ingress
@@ -272,7 +272,7 @@ The steps you should follow depend on the Helm release name:
--set controller.name=""
```
It could look as follows:
It could look as follows:
```shell
helm upgrade test-release oci://ghcr.io/nginxinc/charts/nginx-ingress --version 0.19.0 --set controller.kind=deployment/daemonset --set controller.nginxplus=false/true --set controller.image.pullPolicy=Always --set serviceNameOverride="test-release-nginx-ingress" --set controller.name="" -f values.yaml
@@ -433,6 +433,7 @@ The following tables lists the configurable parameters of the NGINX Ingress Cont
| **controller.defaultHTTPSListenerPort** | Sets the port for the HTTPS `default_server` listener. | 443 |
| **controller.readOnlyRootFilesystem** | Configure root filesystem as read-only and add volumes for temporary data. Three major releases after 3.5.x this argument will be moved permanently to the `controller.securityContext` section. | false |
| **controller.enableSSLDynamicReload** | Enable lazy loading for SSL Certificates. | true |
| **controller.telemetryReporting.enable** | Enable telemetry reporting. | true |
| **rbac.create** | Configures RBAC. | true |
| **prometheus.create** | Expose NGINX or NGINX Plus metrics in the Prometheus format. | true |
| **prometheus.port** | Configures the port to scrape the metrics. | 9113 |
Original file line number Diff line number Diff line change
@@ -122,7 +122,6 @@ For example, say you want to [log state changing requests](/nginx-app-protect-wa
},
"content": {
"format": "default",
"max_request_size": "any",
"max_message_size": "5k"
}
}
@@ -140,7 +139,6 @@ spec:
request_type: all
content:
format: default
max_request_size: any
max_message_size: 5k
```

2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
@@ -22,6 +22,7 @@ require (
github.com/spiffe/go-spiffe/v2 v2.1.7
github.com/stretchr/testify v1.9.0
go.opentelemetry.io/otel v1.24.0
go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.24.0
golang.org/x/exp v0.0.0-20231226003508-02704c960a9b
k8s.io/api v0.29.2
k8s.io/apimachinery v0.29.2
@@ -102,7 +103,6 @@ require (
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.46.1 // indirect
go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.46.1 // indirect
go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.24.0 // indirect
go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.24.0 // indirect
go.opentelemetry.io/otel/metric v1.24.0 // indirect
go.opentelemetry.io/otel/sdk v1.24.0 // indirect
go.opentelemetry.io/otel/trace v1.24.0 // indirect
21 changes: 17 additions & 4 deletions internal/k8s/controller.go
Original file line number Diff line number Diff line change
@@ -20,13 +20,13 @@ import (
"context"
"fmt"
"net"
"os"
"strconv"
"strings"
"sync"
"time"

"github.com/nginxinc/kubernetes-ingress/internal/telemetry"

"github.com/nginxinc/kubernetes-ingress/pkg/apis/dos/v1beta1"
"golang.org/x/exp/maps"

@@ -43,6 +43,7 @@ import (

"k8s.io/apimachinery/pkg/fields"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/intstr"
"k8s.io/client-go/kubernetes"
"k8s.io/client-go/kubernetes/scheme"
@@ -213,7 +214,6 @@ type NewLoadBalancerControllerInput struct {
IsIPV6Disabled bool
WatchNamespaceLabel string
EnableTelemetryReporting bool
TelemetryReportingPeriod string
NICVersion string
WeightChangesWithoutReload bool
}
@@ -351,21 +351,34 @@ func NewLoadBalancerController(input NewLoadBalancerControllerInput) *LoadBalanc

// NIC Telemetry Reporting
if input.EnableTelemetryReporting {
exporterCfg := telemetry.ExporterCfg{
Endpoint: "oss.edge.df.f5.com:443",
}

exporter, err := telemetry.NewExporter(exporterCfg)
if err != nil {
glog.Fatalf("failed to initialize telemetry exporter: %v", err)
}
collectorConfig := telemetry.CollectorConfig{
K8sClientReader: input.KubeClient,
CustomK8sClientReader: input.ConfClient,
Period: 5 * time.Second,
Period: 24 * time.Hour,
Configurator: lbc.configurator,
Version: input.NICVersion,
PodNSName: types.NamespacedName{
Namespace: os.Getenv("POD_NAMESPACE"),
Name: os.Getenv("POD_NAME"),
},
}
lbc.telemetryChan = make(chan struct{})
collector, err := telemetry.NewCollector(
collectorConfig,
telemetry.WithExporter(exporter),
)
if err != nil {
glog.Fatalf("failed to initialize telemetry collector: %v", err)
}
lbc.telemetryCollector = collector
lbc.telemetryChan = make(chan struct{})
}

return lbc
1 change: 0 additions & 1 deletion internal/k8s/controller_test.go
Original file line number Diff line number Diff line change
@@ -3772,7 +3772,6 @@ func TestNewTelemetryCollector(t *testing.T) {
input: NewLoadBalancerControllerInput{
KubeClient: fake.NewSimpleClientset(),
EnableTelemetryReporting: true,
TelemetryReportingPeriod: "24h",
},
expectedCollector: telemetry.Collector{
Config: telemetry.CollectorConfig{
30 changes: 30 additions & 0 deletions internal/telemetry/cluster.go
Original file line number Diff line number Diff line change
@@ -3,6 +3,7 @@ package telemetry
import (
"context"
"errors"
"fmt"
"strings"

metaV1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -18,6 +19,35 @@ func (c *Collector) NodeCount(ctx context.Context) (int, error) {
return len(nodes.Items), nil
}

// ReplicaCount returns a number of running NIC replicas.
func (c *Collector) ReplicaCount(ctx context.Context) (int, error) {
pod, err := c.Config.K8sClientReader.CoreV1().Pods(c.Config.PodNSName.Namespace).Get(ctx, c.Config.PodNSName.Name, metaV1.GetOptions{})
if err != nil {
return 0, err
}
podRef := pod.GetOwnerReferences()
if len(podRef) != 1 {
return 0, fmt.Errorf("expected pod owner reference to be 1, got %d", len(podRef))
}

switch podRef[0].Kind {
case "ReplicaSet":
rs, err := c.Config.K8sClientReader.AppsV1().ReplicaSets(c.Config.PodNSName.Namespace).Get(ctx, podRef[0].Name, metaV1.GetOptions{})
if err != nil {
return 0, err
}
return int(*rs.Spec.Replicas), nil
case "DaemonSet":
ds, err := c.Config.K8sClientReader.AppsV1().DaemonSets(c.Config.PodNSName.Namespace).Get(ctx, podRef[0].Name, metaV1.GetOptions{})
if err != nil {
return 0, err
}
return int(ds.Status.CurrentNumberScheduled), nil
default:
return 0, fmt.Errorf("expected pod owner reference to be ReplicaSet or DeamonSet, got %s", podRef[0].Kind)
}
}

// ClusterID returns the UID of the kube-system namespace representing cluster id.
// It returns an error if the underlying k8s API client errors.
func (c *Collector) ClusterID(ctx context.Context) (string, error) {
179 changes: 166 additions & 13 deletions internal/telemetry/cluster_test.go
Original file line number Diff line number Diff line change
@@ -5,9 +5,11 @@ import (
"testing"

"github.com/nginxinc/kubernetes-ingress/internal/telemetry"
appsV1 "k8s.io/api/apps/v1"
apiCoreV1 "k8s.io/api/core/v1"
metaV1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
)

func TestNodeCountInAClusterWithThreeNodes(t *testing.T) {
@@ -350,6 +352,37 @@ func TestPlatformLookupOnMalformedPartialPlatformIDField(t *testing.T) {
}
}

func TestReplicaCountReturnsNumberOfNICReplicas(t *testing.T) {
t.Parallel()

c := newTestCollectorForClusterWithNodes(t, node1, pod1, replica)

got, err := c.ReplicaCount(context.Background())
if err != nil {
t.Fatal(err)
}

want := 1
if want != got {
t.Errorf("want %d, got %d", want, got)
}
}

func TestReplicaCountReturnsNumberOfNICDaemonSets(t *testing.T) {
t.Parallel()

c := newTestCollectorForClusterWithNodes(t, node1, pod2, daemon)
got, err := c.ReplicaCount(context.Background())
if err != nil {
t.Fatal(err)
}

want := 1
if want != got {
t.Errorf("want %d, got %d", want, got)
}
}

// newTestCollectorForClusterWithNodes returns a telemetry collector configured
// to simulate collecting data on a cluser with provided nodes.
func newTestCollectorForClusterWithNodes(t *testing.T, nodes ...runtime.Object) *telemetry.Collector {
@@ -362,9 +395,141 @@ func newTestCollectorForClusterWithNodes(t *testing.T, nodes ...runtime.Object)
t.Fatal(err)
}
c.Config.K8sClientReader = newTestClientset(nodes...)
c.Config.PodNSName = types.NamespacedName{
Namespace: "nginx-ingress",
Name: "nginx-ingress",
}
return c
}

// Pod and ReplicaSet for testing NIC replica sets.
var (
pod1 = &apiCoreV1.Pod{
TypeMeta: metaV1.TypeMeta{
Kind: "Pod",
APIVersion: "v1",
},
ObjectMeta: metaV1.ObjectMeta{
Name: "nginx-ingress",
Namespace: "nginx-ingress",
OwnerReferences: []metaV1.OwnerReference{
{
Kind: "ReplicaSet",
Name: "nginx-ingress",
},
},
Labels: map[string]string{
"app": "nginx-ingress",
"app.kubernetes.io/name": "nginx-ingress",
},
},
Spec: apiCoreV1.PodSpec{
Containers: []apiCoreV1.Container{
{
Name: "nginx-ingress",
Image: "nginx-ingress",
ImagePullPolicy: "Always",
Env: []apiCoreV1.EnvVar{
{
Name: "POD_NAMESPACE",
Value: "nginx-ingress",
},
{
Name: "POD_NAME",
Value: "nginx-ingress",
},
},
},
},
},
}

replicaNum int32 = 1
replica = &appsV1.ReplicaSet{
TypeMeta: metaV1.TypeMeta{
Kind: "ReplicaSet",
APIVersion: "apps/v1",
},
ObjectMeta: metaV1.ObjectMeta{
Name: "nginx-ingress",
Namespace: "nginx-ingress",
Labels: map[string]string{
"app": "nginx-ingress",
"app.kubernetes.io/name": "nginx-ingress",
},
},

Spec: appsV1.ReplicaSetSpec{
Replicas: &replicaNum,
},
Status: appsV1.ReplicaSetStatus{
Replicas: replicaNum,
ReadyReplicas: replicaNum,
AvailableReplicas: replicaNum,
},
}

pod2 = &apiCoreV1.Pod{
TypeMeta: metaV1.TypeMeta{
Kind: "Pod",
APIVersion: "v1",
},
ObjectMeta: metaV1.ObjectMeta{
Name: "nginx-ingress",
Namespace: "nginx-ingress",
OwnerReferences: []metaV1.OwnerReference{
{
Kind: "DaemonSet",
Name: "nginx-ingress",
},
},
Labels: map[string]string{
"app": "nginx-ingress",
"app.kubernetes.io/name": "nginx-ingress",
},
},
Spec: apiCoreV1.PodSpec{
Containers: []apiCoreV1.Container{
{
Name: "nginx-ingress",
Image: "nginx-ingress",
ImagePullPolicy: "Always",
Env: []apiCoreV1.EnvVar{
{
Name: "POD_NAMESPACE",
Value: "nginx-ingress",
},
{
Name: "POD_NAME",
Value: "nginx-ingress",
},
},
},
},
},
}

daemonNum int32 = 1
daemon = &appsV1.DaemonSet{
TypeMeta: metaV1.TypeMeta{
Kind: "DaemonSet",
APIVersion: "apps/v1",
},
ObjectMeta: metaV1.ObjectMeta{
Name: "nginx-ingress",
Namespace: "nginx-ingress",
Labels: map[string]string{"app": "nginx-ingress"},
},
Spec: appsV1.DaemonSetSpec{},
Status: appsV1.DaemonSetStatus{
CurrentNumberScheduled: daemonNum,
NumberReady: daemonNum,
NumberAvailable: daemonNum,
},
}
)

// Nodes for testing NIC namespaces.
var (
node1 = &apiCoreV1.Node{
TypeMeta: metaV1.TypeMeta{
@@ -373,7 +538,7 @@ var (
},
ObjectMeta: metaV1.ObjectMeta{
Name: "test-node-1",
Namespace: "default",
Namespace: "nginx-ingress",
},
Spec: apiCoreV1.NodeSpec{},
}
@@ -413,18 +578,6 @@ var (
},
Spec: apiCoreV1.NamespaceSpec{},
}

dummyKubeNS = &apiCoreV1.Namespace{
TypeMeta: metaV1.TypeMeta{
Kind: "Namespace",
APIVersion: "v1",
},
ObjectMeta: metaV1.ObjectMeta{
Name: "kube-system",
UID: "",
},
Spec: apiCoreV1.NamespaceSpec{},
}
)

// Cloud providers' nodes for testing ProviderID lookups.
14 changes: 13 additions & 1 deletion internal/telemetry/collector.go
Original file line number Diff line number Diff line change
@@ -12,6 +12,7 @@ import (
"github.com/nginxinc/kubernetes-ingress/internal/configs"

k8s_nginx "github.com/nginxinc/kubernetes-ingress/pkg/client/clientset/versioned"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/client-go/kubernetes"

@@ -58,6 +59,9 @@ type CollectorConfig struct {

// Version represents NIC version.
Version string

// PodNSName represents NIC Pod's NamespacedName.
PodNSName types.NamespacedName
}

// NewCollector takes 0 or more options and creates a new TraceReporter.
@@ -104,14 +108,15 @@ func (c *Collector) Collect(ctx context.Context) {
VirtualServers: int64(report.VirtualServers),
VirtualServerRoutes: int64(report.VirtualServerRoutes),
TransportServers: int64(report.TransportServers),
Replicas: int64(report.NICReplicaCount),
},
}

err = c.Exporter.Export(ctx, &nicData)
if err != nil {
glog.Errorf("Error exporting telemetry data: %v", err)
}
glog.V(3).Infof("Exported telemetry data: %+v", nicData)
glog.V(3).Infof("Telemetry data collected: %+v", nicData)
}

// Report holds collected NIC telemetry data. It is the package internal
@@ -125,6 +130,7 @@ type Report struct {
ClusterVersion string
ClusterPlatform string
ClusterNodeCount int
NICReplicaCount int
VirtualServers int
VirtualServerRoutes int
TransportServers int
@@ -161,6 +167,11 @@ func (c *Collector) BuildReport(ctx context.Context) (Report, error) {
glog.Errorf("Error collecting telemetry data: Platform: %v", err)
}

replicas, err := c.ReplicaCount(ctx)
if err != nil {
glog.Errorf("Error collecting telemetry data: Replicas: %v", err)
}

return Report{
Name: "NIC",
Version: c.Config.Version,
@@ -169,6 +180,7 @@ func (c *Collector) BuildReport(ctx context.Context) (Report, error) {
ClusterVersion: version,
ClusterPlatform: platform,
ClusterNodeCount: nodes,
NICReplicaCount: replicas,
VirtualServers: vsCount,
VirtualServerRoutes: vsrCount,
TransportServers: tsCount,
14 changes: 12 additions & 2 deletions internal/telemetry/collector_test.go
Original file line number Diff line number Diff line change
@@ -19,8 +19,10 @@ import (
tel "github.com/nginxinc/telemetry-exporter/pkg/telemetry"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
k8sruntime "k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/version"
fakediscovery "k8s.io/client-go/discovery/fake"

testClient "k8s.io/client-go/kubernetes/fake"
)

@@ -342,13 +344,17 @@ func TestCountVirtualServers(t *testing.T) {
configurator := newConfigurator(t)

c, err := telemetry.NewCollector(telemetry.CollectorConfig{
K8sClientReader: newTestClientset(kubeNS, node1),
K8sClientReader: newTestClientset(kubeNS, node1, pod1, replica),
Configurator: configurator,
Version: telemetryNICData.ProjectVersion,
})
if err != nil {
t.Fatal(err)
}
c.Config.PodNSName = types.NamespacedName{
Namespace: "nginx-ingress",
Name: "nginx-ingress",
}

for _, vs := range test.virtualServers {
_, err := configurator.AddOrUpdateVirtualServer(vs)
@@ -503,13 +509,17 @@ func TestCountTransportServers(t *testing.T) {
configurator := newConfigurator(t)

c, err := telemetry.NewCollector(telemetry.CollectorConfig{
K8sClientReader: newTestClientset(kubeNS, node1),
K8sClientReader: newTestClientset(kubeNS, node1, pod1, replica),
Configurator: configurator,
Version: telemetryNICData.ProjectVersion,
})
if err != nil {
t.Fatal(err)
}
c.Config.PodNSName = types.NamespacedName{
Namespace: "nginx-ingress",
Name: "nginx-ingress",
}

for _, ts := range test.transportServers {
_, err := configurator.AddOrUpdateTransportServer(ts)
29 changes: 29 additions & 0 deletions internal/telemetry/exporter.go
Original file line number Diff line number Diff line change
@@ -5,6 +5,8 @@ import (
"fmt"
"io"

"go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc"

tel "github.com/nginxinc/telemetry-exporter/pkg/telemetry"
)

@@ -24,6 +26,30 @@ func (e *StdoutExporter) Export(_ context.Context, data tel.Exportable) error {
return nil
}

// ExporterCfg is a configuration struct for an Exporter.
type ExporterCfg struct {
Endpoint string
}

// NewExporter creates an Exporter with the provided ExporterCfg.
func NewExporter(cfg ExporterCfg) (Exporter, error) {
providerOptions := []otlptracegrpc.Option{
otlptracegrpc.WithEndpoint(cfg.Endpoint),
// This header option will be removed when https://github.com/nginxinc/telemetry-exporter/issues/41 is resolved.
otlptracegrpc.WithHeaders(map[string]string{
"X-F5-OTEL": "GRPC",
}),
}

exporter, err := tel.NewExporter(
tel.ExporterConfig{
SpanProvider: tel.CreateOTLPSpanProvider(providerOptions...),
},
)

return exporter, err
}

// Data holds collected telemetry data.
//
//go:generate go run -tags=generator github.com/nginxinc/telemetry-exporter/cmd/generator -type Data -scheme -scheme-protocol=NICProductTelemetry -scheme-df-datatype=nic-product-telemetry -scheme-namespace=ingress.nginx.com
@@ -42,4 +68,7 @@ type NICResourceCounts struct {
VirtualServerRoutes int64
// TransportServers is the number of TransportServers managed by the Ingress Controller.
TransportServers int64

// Replicas is the number of NIC replicas.
Replicas int64
}
5 changes: 4 additions & 1 deletion tests/suite/utils/resources_utils.py
Original file line number Diff line number Diff line change
@@ -1182,7 +1182,10 @@ def create_ingress_controller(v1: CoreV1Api, apps_v1_api: AppsV1Api, cli_argumen
dep["spec"]["template"]["spec"]["containers"][0]["image"] = cli_arguments["image"]
dep["spec"]["template"]["spec"]["containers"][0]["imagePullPolicy"] = cli_arguments["image-pull-policy"]
dep["spec"]["template"]["spec"]["containers"][0]["args"].extend(
["-default-server-tls-secret=$(POD_NAMESPACE)/default-server-secret"]
[
f"-default-server-tls-secret=$(POD_NAMESPACE)/default-server-secret",
f"-enable-telemetry-reporting=false",
]
)
if args is not None:
dep["spec"]["template"]["spec"]["containers"][0]["args"].extend(args)