Skip to content

Commit

Permalink
Telemetry Job (#4896)
Browse files Browse the repository at this point in the history
  • Loading branch information
shaun-nx authored Feb 7, 2024
1 parent e1a44b0 commit fe6759b
Show file tree
Hide file tree
Showing 13 changed files with 330 additions and 3 deletions.
3 changes: 2 additions & 1 deletion charts/nginx-ingress/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -464,9 +464,10 @@ The following tables lists the configurable parameters of the NGINX Ingress Cont
|`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.defaultHTTPSListenerPort` | Sets the port for the HTTPS `default_server` listener. | 443 |
|`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. | false |
|`controller.enableSSLDynamicReload` | Enable lazy loading for SSL Certificates. | true |
|`controller.enableTelemetryReporting` | 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 |
Expand Down
1 change: 1 addition & 0 deletions charts/nginx-ingress/templates/_helpers.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -223,4 +223,5 @@ 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}}
{{- end -}}
8 changes: 8 additions & 0 deletions charts/nginx-ingress/values.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -1367,6 +1367,14 @@
"examples": [
true
]
},
"enableTelemetryReporting": {
"type": "boolean",
"default": true,
"title": "Enable telemetry reporting",
"examples": [
true
]
}
},
"examples": [
Expand Down
3 changes: 3 additions & 0 deletions charts/nginx-ingress/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -465,6 +465,9 @@ controller:
## Enable dynamic reloading of certificates
enableSSLDynamicReload: true

## Enable telemetry reporting
enableTelemetryReporting: true

rbac:
## Configures RBAC.
create: true
Expand Down
18 changes: 18 additions & 0 deletions cmd/nginx-ingress/flags.go
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
package main

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

"github.com/golang/glog"
api_v1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -201,6 +203,8 @@ 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.")

startupCheckFn func() error
)

Expand Down Expand Up @@ -489,3 +493,17 @@ 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: 24 additions & 0 deletions cmd/nginx-ingress/flags_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,3 +172,27 @@ 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)
}
}
}
5 changes: 4 additions & 1 deletion cmd/nginx-ingress/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,9 @@ import (
)

// Injected during build
var version string
var (
version string
)

const (
nginxVersionLabel = "app.nginx.org/version"
Expand Down Expand Up @@ -199,6 +201,7 @@ func main() {
ExternalDNSEnabled: *enableExternalDNS,
IsIPV6Disabled: *disableIPV6,
WatchNamespaceLabel: *watchNamespaceLabel,
EnableTelemetryReporting: *enableTelemetryReporting,
}

lbc := k8s.NewLoadBalancerController(lbcInput)
Expand Down
29 changes: 29 additions & 0 deletions internal/k8s/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ import (
"sync"
"time"

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

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

Expand Down Expand Up @@ -161,6 +163,8 @@ type LoadBalancerController struct {
enableBatchReload bool
isIPV6Disabled bool
namespaceWatcherController cache.Controller
telemetryCollector *telemetry.Collector
telemetryChan chan struct{}
}

var keyFunc = cache.DeletionHandlingMetaNamespaceKeyFunc
Expand Down Expand Up @@ -206,6 +210,7 @@ type NewLoadBalancerControllerInput struct {
ExternalDNSEnabled bool
IsIPV6Disabled bool
WatchNamespaceLabel string
EnableTelemetryReporting bool
}

// NewLoadBalancerController creates a controller
Expand Down Expand Up @@ -271,6 +276,18 @@ func NewLoadBalancerController(input NewLoadBalancerControllerInput) *LoadBalanc
lbc.externalDNSController = ed_controller.NewController(ed_controller.BuildOpts(context.TODO(), lbc.namespaceList, lbc.recorder, lbc.confClient, input.ResyncPeriod, isDynamicNs))
}

// NIC Telemetry Reporting
if input.EnableTelemetryReporting {
lbc.telemetryChan = make(chan struct{})
collector, err := telemetry.NewCollector(
telemetry.WithTimePeriod("24h"),
)
if err != nil {
glog.Fatalf("failed to initialize telemetry collector: %v", err)
}
lbc.telemetryCollector = collector
}

glog.V(3).Infof("Nginx Ingress Controller has class: %v", input.IngressClass)

lbc.namespacedInformers = make(map[string]*namespacedInformer)
Expand Down Expand Up @@ -683,10 +700,22 @@ func (lbc *LoadBalancerController) Run() {
if lbc.externalDNSController != nil {
go lbc.externalDNSController.Run(lbc.ctx.Done())
}

if lbc.leaderElector != nil {
go lbc.leaderElector.Run(lbc.ctx)
}

if lbc.telemetryCollector != nil {
go func(ctx context.Context) {
select {
case <-lbc.telemetryChan:
lbc.telemetryCollector.Start(lbc.ctx)
case <-ctx.Done():
return
}
}(lbc.ctx)
}

for _, nif := range lbc.namespacedInformers {
nif.start()
}
Expand Down
40 changes: 40 additions & 0 deletions internal/k8s/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ import (
"sort"
"strings"
"testing"
"time"

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

discovery_v1 "k8s.io/api/discovery/v1"

Expand Down Expand Up @@ -3747,3 +3750,40 @@ func TestPreSyncSecrets(t *testing.T) {
t.Errorf("GetSecret(%q) returned a reference without an expected error", unsupportedKey)
}
}

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

testCases := []struct {
testCase string
input NewLoadBalancerControllerInput
expectedCollector telemetry.Collector
}{
{
testCase: "New Telemetry Collector with default values",
input: NewLoadBalancerControllerInput{
KubeClient: fake.NewSimpleClientset(),
EnableTelemetryReporting: true,
},
expectedCollector: telemetry.Collector{
Period: 24 * time.Hour,
Exporter: telemetry.DiscardExporter,
},
},
{
testCase: "New Telemetry Collector with Telemetry Reporting set to false",
input: NewLoadBalancerControllerInput{
KubeClient: fake.NewSimpleClientset(),
EnableTelemetryReporting: false,
},
expectedCollector: telemetry.Collector{},
},
}

for _, tc := range testCases {
lbc := NewLoadBalancerController(tc.input)
if reflect.DeepEqual(tc.expectedCollector, lbc.telemetryCollector) {
t.Fatalf("Expected %x, but got %x", tc.expectedCollector, lbc.telemetryCollector)
}
}
}
4 changes: 4 additions & 0 deletions internal/k8s/leader.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@ func createLeaderHandler(lbc *LoadBalancerController) leaderelection.LeaderCallb
return leaderelection.LeaderCallbacks{
OnStartedLeading: func(ctx context.Context) {
glog.V(3).Info("started leading")
// Closing this channel allows the leader to start the telemetry reporting process
if lbc.telemetryChan != nil {
close(lbc.telemetryChan)
}
if lbc.reportIngressStatus {
ingresses := lbc.configuration.GetResourcesWithFilter(resourceFilter{Ingresses: true})

Expand Down
2 changes: 1 addition & 1 deletion internal/nginx/fake_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ func (fm *FakeManager) CreateDHParam(_ string) (string, error) {
// Version provides a fake implementation of Version.
func (*FakeManager) Version() Version {
glog.V(3).Info("Printing nginx version")
return Version{}
return NewVersion("nginx version: nginx/1.25.3 (nginx-plus-r31)")
}

// Start provides a fake implementation of Start.
Expand Down
Loading

0 comments on commit fe6759b

Please sign in to comment.