diff --git a/charts/nginx-ingress/README.md b/charts/nginx-ingress/README.md index c47d802ec4..05f72f11ee 100644 --- a/charts/nginx-ingress/README.md +++ b/charts/nginx-ingress/README.md @@ -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 | |`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 | diff --git a/charts/nginx-ingress/templates/_helpers.tpl b/charts/nginx-ingress/templates/_helpers.tpl index 0b55efd69a..6bd2bd1783 100644 --- a/charts/nginx-ingress/templates/_helpers.tpl +++ b/charts/nginx-ingress/templates/_helpers.tpl @@ -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}} {{- if .Values.nginxAgent.enable }} - -agent=true - -agent-instance-group={{ default (include "nginx-ingress.controller.fullname" .) .Values.nginxAgent.instanceGroup }} diff --git a/charts/nginx-ingress/values.schema.json b/charts/nginx-ingress/values.schema.json index 53c9b7198d..3ccd63bead 100644 --- a/charts/nginx-ingress/values.schema.json +++ b/charts/nginx-ingress/values.schema.json @@ -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 + ] + } + } } }, "examples": [ diff --git a/charts/nginx-ingress/values.yaml b/charts/nginx-ingress/values.yaml index dea57e4c50..8b09d8be87 100644 --- a/charts/nginx-ingress/values.yaml +++ b/charts/nginx-ingress/values.yaml @@ -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 rbac: ## Configures RBAC. diff --git a/cmd/nginx-ingress/flags.go b/cmd/nginx-ingress/flags.go index 257409ea75..2ad5ce294d 100644 --- a/cmd/nginx-ingress/flags.go +++ b/cmd/nginx-ingress/flags.go @@ -1,7 +1,6 @@ package main import ( - "errors" "flag" "fmt" "net" @@ -9,7 +8,6 @@ import ( "regexp" "strconv" "strings" - "time" "github.com/golang/glog" api_v1 "k8s.io/api/core/v1" @@ -207,7 +205,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.") startupCheckFn func() error ) @@ -396,12 +393,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 @@ -507,17 +498,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 -} diff --git a/cmd/nginx-ingress/flags_test.go b/cmd/nginx-ingress/flags_test.go index 07a32a3f7f..1715d2a3e8 100644 --- a/cmd/nginx-ingress/flags_test.go +++ b/cmd/nginx-ingress/flags_test.go @@ -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) - } - } -} diff --git a/cmd/nginx-ingress/main.go b/cmd/nginx-ingress/main.go index 4ccc8e0091..0c03ce266e 100644 --- a/cmd/nginx-ingress/main.go +++ b/cmd/nginx-ingress/main.go @@ -210,7 +210,6 @@ func main() { IsIPV6Disabled: *disableIPV6, WatchNamespaceLabel: *watchNamespaceLabel, EnableTelemetryReporting: *enableTelemetryReporting, - TelemetryReportingPeriod: *telemetryReportingPeriod, NICVersion: version, } diff --git a/docs/content/configuration/global-configuration/command-line-arguments.md b/docs/content/configuration/global-configuration/command-line-arguments.md index de3f47cb4f..33c13fa7c9 100644 --- a/docs/content/configuration/global-configuration/command-line-arguments.md +++ b/docs/content/configuration/global-configuration/command-line-arguments.md @@ -536,6 +536,14 @@ The default value is `true`. +### -enable-telemetry-reporting + +Enable gathering and reporting of product related telemetry. + +The default value is `true`. + + + ### -agent Enable NGINX Agent which can used with `-enable-app-protect` to send events to Security Monitoring. diff --git a/docs/content/installation/installing-nic/installation-with-helm.md b/docs/content/installation/installing-nic/installation-with-helm.md index 52670a7941..8d6a7eafb4 100644 --- a/docs/content/installation/installing-nic/installation-with-helm.md +++ b/docs/content/installation/installing-nic/installation-with-helm.md @@ -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 ``` - 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 ``` - Copy the key=value under ```Selector```, such as: + Copy the key=value under ```Selector```, such as: ```shell Selector: app=-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 | diff --git a/go.mod b/go.mod index cb42e78dda..ac3cd005af 100644 --- a/go.mod +++ b/go.mod @@ -21,6 +21,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 @@ -101,7 +102,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 diff --git a/internal/k8s/controller.go b/internal/k8s/controller.go index 10623103bd..8009fcb7fa 100644 --- a/internal/k8s/controller.go +++ b/internal/k8s/controller.go @@ -27,7 +27,6 @@ import ( "time" "github.com/nginxinc/kubernetes-ingress/internal/telemetry" - "github.com/nginxinc/kubernetes-ingress/pkg/apis/dos/v1beta1" "golang.org/x/exp/maps" @@ -213,7 +212,6 @@ type NewLoadBalancerControllerInput struct { IsIPV6Disabled bool WatchNamespaceLabel string EnableTelemetryReporting bool - TelemetryReportingPeriod string NICVersion string } @@ -349,12 +347,18 @@ func NewLoadBalancerController(input NewLoadBalancerControllerInput) *LoadBalanc // NIC Telemetry Reporting if input.EnableTelemetryReporting { - lbc.telemetryChan = make(chan struct{}) + 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{ @@ -362,11 +366,15 @@ func NewLoadBalancerController(input NewLoadBalancerControllerInput) *LoadBalanc Name: os.Getenv("POD_NAME"), }, } - collector, err := telemetry.NewCollector(collectorConfig) + 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 diff --git a/internal/k8s/controller_test.go b/internal/k8s/controller_test.go index cb8b30f274..50725ea714 100644 --- a/internal/k8s/controller_test.go +++ b/internal/k8s/controller_test.go @@ -3772,7 +3772,6 @@ func TestNewTelemetryCollector(t *testing.T) { input: NewLoadBalancerControllerInput{ KubeClient: fake.NewSimpleClientset(), EnableTelemetryReporting: true, - TelemetryReportingPeriod: "24h", }, expectedCollector: telemetry.Collector{ Config: telemetry.CollectorConfig{ diff --git a/internal/telemetry/collector.go b/internal/telemetry/collector.go index 956c5f3ec5..b7f906ed30 100644 --- a/internal/telemetry/collector.go +++ b/internal/telemetry/collector.go @@ -116,7 +116,7 @@ func (c *Collector) Collect(ctx context.Context) { 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 diff --git a/internal/telemetry/exporter.go b/internal/telemetry/exporter.go index 412cb9e6af..e827ca353b 100644 --- a/internal/telemetry/exporter.go +++ b/internal/telemetry/exporter.go @@ -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 diff --git a/tests/suite/utils/resources_utils.py b/tests/suite/utils/resources_utils.py index cb2ab4c087..bd9be37b08 100644 --- a/tests/suite/utils/resources_utils.py +++ b/tests/suite/utils/resources_utils.py @@ -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)