From 9161f8052edee252c08273b0c802adab11166234 Mon Sep 17 00:00:00 2001 From: LorcanMcVeigh Date: Mon, 17 Aug 2020 13:05:41 +0100 Subject: [PATCH 1/5] Add reload reason --- docs-web/logging-and-monitoring/prometheus.md | 2 +- internal/configs/configurator.go | 44 +++++++++---------- internal/metrics/collectors/manager.go | 19 +++++--- internal/nginx/fake_manager.go | 2 +- internal/nginx/manager.go | 6 +-- 5 files changed, 40 insertions(+), 33 deletions(-) diff --git a/docs-web/logging-and-monitoring/prometheus.md b/docs-web/logging-and-monitoring/prometheus.md index 3301c67ed9..8c37f85a94 100644 --- a/docs-web/logging-and-monitoring/prometheus.md +++ b/docs-web/logging-and-monitoring/prometheus.md @@ -26,7 +26,7 @@ The Ingress Controller exports the following metrics: * NGINX/NGINX Plus metrics. Please see this [doc](https://github.com/nginxinc/nginx-prometheus-exporter#exported-metrics) to find more information about the exported metrics. * Ingress Controller metrics - * `controller_nginx_reloads_total`. Number of successful NGINX reloads. + * `controller_nginx_reloads_total`. Number of successful NGINX reloads. This includes the label `reason` with 2 possible values `endpoint` ( the reason for the reload was an endpoint was updated ) and `other` ( the reload was caused by something other than a reload ). * `controller_nginx_reload_errors_total`. Number of unsuccessful NGINX reloads. * `controller_nginx_last_reload_status`. Status of the last NGINX reload, 0 meaning down and 1 up. * `controller_nginx_last_reload_milliseconds`. Duration in milliseconds of the last NGINX reload. diff --git a/internal/configs/configurator.go b/internal/configs/configurator.go index 4998e9083b..1bdf4dbef1 100644 --- a/internal/configs/configurator.go +++ b/internal/configs/configurator.go @@ -186,7 +186,7 @@ func (cnf *Configurator) AddOrUpdateIngress(ingEx *IngressEx) error { return fmt.Errorf("Error adding or updating ingress %v/%v: %v", ingEx.Ingress.Namespace, ingEx.Ingress.Name, err) } - if err := cnf.nginxManager.Reload(); err != nil { + if err := cnf.nginxManager.Reload(true); err != nil { return fmt.Errorf("Error reloading NGINX for %v/%v: %v", ingEx.Ingress.Namespace, ingEx.Ingress.Name, err) } @@ -222,7 +222,7 @@ func (cnf *Configurator) AddOrUpdateMergeableIngress(mergeableIngs *MergeableIng return fmt.Errorf("Error when adding or updating ingress %v/%v: %v", mergeableIngs.Master.Ingress.Namespace, mergeableIngs.Master.Ingress.Name, err) } - if err := cnf.nginxManager.Reload(); err != nil { + if err := cnf.nginxManager.Reload(true); err != nil { return fmt.Errorf("Error reloading NGINX for %v/%v: %v", mergeableIngs.Master.Ingress.Namespace, mergeableIngs.Master.Ingress.Name, err) } @@ -309,7 +309,7 @@ func (cnf *Configurator) AddOrUpdateVirtualServer(virtualServerEx *VirtualServer return warnings, fmt.Errorf("Error adding or updating VirtualServer %v/%v: %v", virtualServerEx.VirtualServer.Namespace, virtualServerEx.VirtualServer.Name, err) } - if err := cnf.nginxManager.Reload(); err != nil { + if err := cnf.nginxManager.Reload(true); err != nil { return warnings, fmt.Errorf("Error reloading NGINX for VirtualServer %v/%v: %v", virtualServerEx.VirtualServer.Namespace, virtualServerEx.VirtualServer.Name, err) } @@ -356,7 +356,7 @@ func (cnf *Configurator) AddOrUpdateVirtualServers(virtualServerExes []*VirtualS allWarnings.Add(warnings) } - if err := cnf.nginxManager.Reload(); err != nil { + if err := cnf.nginxManager.Reload(false); err != nil { return allWarnings, fmt.Errorf("Error when reloading NGINX when updating Policy: %v", err) } @@ -371,7 +371,7 @@ func (cnf *Configurator) AddOrUpdateTransportServer(transportServerEx *Transport return fmt.Errorf("Error adding or updating TransportServer %v/%v: %v", transportServerEx.TransportServer.Namespace, transportServerEx.TransportServer.Name, err) } - if err := cnf.nginxManager.Reload(); err != nil { + if err := cnf.nginxManager.Reload(true); err != nil { return fmt.Errorf("Error reloading NGINX for TransportServer %v/%v: %v", transportServerEx.TransportServer.Namespace, transportServerEx.TransportServer.Name, err) } @@ -530,7 +530,7 @@ func (cnf *Configurator) AddOrUpdateTLSSecret(secret *api_v1.Secret, ingExes []I } } - if err := cnf.nginxManager.Reload(); err != nil { + if err := cnf.nginxManager.Reload(false); err != nil { return fmt.Errorf("Error when reloading NGINX when updating Secret: %v", err) } @@ -551,7 +551,7 @@ func (cnf *Configurator) AddOrUpdateSpecialTLSSecrets(secret *api_v1.Secret, sec cnf.nginxManager.CreateSecret(secretName, data, nginx.TLSSecretFileMode) } - if err := cnf.nginxManager.Reload(); err != nil { + if err := cnf.nginxManager.Reload(false); err != nil { return fmt.Errorf("Error when reloading NGINX when updating the special Secrets: %v", err) } @@ -597,7 +597,7 @@ func (cnf *Configurator) DeleteSecret(key string, ingExes []IngressEx, mergeable } if len(ingExes)+len(mergeableIngresses)+len(virtualServerExes) > 0 { - if err := cnf.nginxManager.Reload(); err != nil { + if err := cnf.nginxManager.Reload(false); err != nil { return fmt.Errorf("Error when reloading NGINX when deleting Secret %v: %v", key, err) } } @@ -617,7 +617,7 @@ func (cnf *Configurator) DeleteIngress(key string) error { cnf.deleteIngressMetricsLabels(key) } - if err := cnf.nginxManager.Reload(); err != nil { + if err := cnf.nginxManager.Reload(true); err != nil { return fmt.Errorf("Error when removing ingress %v: %v", key, err) } @@ -634,7 +634,7 @@ func (cnf *Configurator) DeleteVirtualServer(key string) error { cnf.deleteVirtualServerMetricsLabels(fmt.Sprintf(key)) } - if err := cnf.nginxManager.Reload(); err != nil { + if err := cnf.nginxManager.Reload(true); err != nil { return fmt.Errorf("Error when removing VirtualServer %v: %v", key, err) } @@ -648,7 +648,7 @@ func (cnf *Configurator) DeleteTransportServer(key string) error { return fmt.Errorf("Error when removing TransportServer %v: %v", key, err) } - err = cnf.nginxManager.Reload() + err = cnf.nginxManager.Reload(true) if err != nil { return fmt.Errorf("Error when removing TransportServer %v: %v", key, err) } @@ -694,7 +694,7 @@ func (cnf *Configurator) UpdateEndpoints(ingExes []*IngressEx) error { return nil } - if err := cnf.nginxManager.Reload(); err != nil { + if err := cnf.nginxManager.Reload(true); err != nil { return fmt.Errorf("Error reloading NGINX when updating endpoints: %v", err) } @@ -727,7 +727,7 @@ func (cnf *Configurator) UpdateEndpointsMergeableIngress(mergeableIngresses []*M return nil } - if err := cnf.nginxManager.Reload(); err != nil { + if err := cnf.nginxManager.Reload(true); err != nil { return fmt.Errorf("Error reloading NGINX when updating endpoints for %v: %v", mergeableIngresses, err) } @@ -759,7 +759,7 @@ func (cnf *Configurator) UpdateEndpointsForVirtualServers(virtualServerExes []*V return nil } - if err := cnf.nginxManager.Reload(); err != nil { + if err := cnf.nginxManager.Reload(true); err != nil { return fmt.Errorf("Error reloading NGINX when updating endpoints: %v", err) } @@ -806,7 +806,7 @@ func (cnf *Configurator) UpdateEndpointsForTransportServers(transportServerExes return nil } - if err := cnf.nginxManager.Reload(); err != nil { + if err := cnf.nginxManager.Reload(true); err != nil { return fmt.Errorf("Error reloading NGINX when updating endpoints: %v", err) } @@ -948,7 +948,7 @@ func (cnf *Configurator) UpdateConfig(cfgParams *ConfigParams, ingExes []*Ingres } cnf.nginxManager.SetOpenTracing(mainCfg.OpenTracingLoadModule) - if err := cnf.nginxManager.Reload(); err != nil { + if err := cnf.nginxManager.Reload(false); err != nil { return allWarnings, fmt.Errorf("Error when updating config from ConfigMap: %v", err) } @@ -981,7 +981,7 @@ func (cnf *Configurator) UpdateGlobalConfiguration(globalConfiguration *conf_v1a } } - if err := cnf.nginxManager.Reload(); err != nil { + if err := cnf.nginxManager.Reload(false); err != nil { return updatedTransportServerExes, deletedTransportServerExes, fmt.Errorf("Error when updating global configuration: %v", err) } @@ -1097,7 +1097,7 @@ func (cnf *Configurator) AddOrUpdateSpiffeCerts(svidResponse *workload.X509SVIDs cnf.nginxManager.CreateSecret(spiffeCertFileName, createSpiffeCert(svid.Certificates), spiffeCertsFileMode) cnf.nginxManager.CreateSecret(spiffeBundleFileName, createSpiffeCert(svid.TrustBundle), spiffeCertsFileMode) - err = cnf.nginxManager.Reload() + err = cnf.nginxManager.Reload(false) if err != nil { return fmt.Errorf("error when reloading NGINX when updating the SPIFFE Certs: %v", err) } @@ -1174,7 +1174,7 @@ func (cnf *Configurator) AddOrUpdateAppProtectResource(resource *unstructured.Un } } - if err := cnf.nginxManager.Reload(); err != nil { + if err := cnf.nginxManager.Reload(false); err != nil { return fmt.Errorf("Error when reloading NGINX when updating %v: %v", resource.GetKind(), err) } @@ -1201,7 +1201,7 @@ func (cnf *Configurator) DeleteAppProtectPolicy(polNamespaceame string, ingExes } } - if err := cnf.nginxManager.Reload(); err != nil { + if err := cnf.nginxManager.Reload(false); err != nil { return fmt.Errorf("Error when reloading NGINX when removing App Protect Policy: %v", err) } @@ -1228,7 +1228,7 @@ func (cnf *Configurator) DeleteAppProtectLogConf(logConfNamespaceame string, ing } } - if err := cnf.nginxManager.Reload(); err != nil { + if err := cnf.nginxManager.Reload(false); err != nil { return fmt.Errorf("Error when reloading NGINX when removing App Protect Log Configuration: %v", err) } @@ -1246,7 +1246,7 @@ func (cnf *Configurator) AddInternalRouteConfig() error { return fmt.Errorf("Error when writing main Config: %v", err) } cnf.nginxManager.CreateMainConfig(mainCfgContent) - if err := cnf.nginxManager.Reload(); err != nil { + if err := cnf.nginxManager.Reload(false); err != nil { return fmt.Errorf("Error when reloading nginx: %v", err) } return nil diff --git a/internal/metrics/collectors/manager.go b/internal/metrics/collectors/manager.go index f45690ed4c..8ea11fff04 100644 --- a/internal/metrics/collectors/manager.go +++ b/internal/metrics/collectors/manager.go @@ -8,7 +8,7 @@ import ( // ManagerCollector is an interface for the metrics of the Nginx Manager type ManagerCollector interface { - IncNginxReloadCount() + IncNginxReloadCount(isEndPoint bool) IncNginxReloadErrors() UpdateLastReloadTime(ms time.Duration) Register(registry *prometheus.Registry) error @@ -17,7 +17,7 @@ type ManagerCollector interface { // LocalManagerMetricsCollector implements NginxManagerCollector interface and prometheus.Collector interface type LocalManagerMetricsCollector struct { // Metrics - reloadsTotal prometheus.Counter + reloadsTotal *prometheus.CounterVec reloadsError prometheus.Counter lastReloadStatus prometheus.Gauge lastReloadTime prometheus.Gauge @@ -26,13 +26,14 @@ type LocalManagerMetricsCollector struct { // NewLocalManagerMetricsCollector creates a new LocalManagerMetricsCollector func NewLocalManagerMetricsCollector(constLabels map[string]string) *LocalManagerMetricsCollector { nc := &LocalManagerMetricsCollector{ - reloadsTotal: prometheus.NewCounter( + reloadsTotal: prometheus.NewCounterVec( prometheus.CounterOpts{ Name: "nginx_reloads_total", Namespace: metricsNamespace, Help: "Number of successful NGINX reloads", ConstLabels: constLabels, }, + []string{"reason"}, ), reloadsError: prometheus.NewCounter( prometheus.CounterOpts{ @@ -63,8 +64,14 @@ func NewLocalManagerMetricsCollector(constLabels map[string]string) *LocalManage } // IncNginxReloadCount increments the counter of successful NGINX reloads and sets the last reload status to true -func (nc *LocalManagerMetricsCollector) IncNginxReloadCount() { - nc.reloadsTotal.Inc() +func (nc *LocalManagerMetricsCollector) IncNginxReloadCount(isEndPoint bool) { + var label string + if isEndPoint { + label = "endpoint" + } else { + label = "other" + } + nc.reloadsTotal.WithLabelValues(label).Inc() nc.updateLastReloadStatus(true) } @@ -121,7 +128,7 @@ func NewManagerFakeCollector() *ManagerFakeCollector { func (nc *ManagerFakeCollector) Register(registry *prometheus.Registry) error { return nil } // IncNginxReloadCount implements a fake IncNginxReloadCount -func (nc *ManagerFakeCollector) IncNginxReloadCount() {} +func (nc *ManagerFakeCollector) IncNginxReloadCount(isEnPoint bool) {} // IncNginxReloadErrors implements a fake IncNginxReloadErrors func (nc *ManagerFakeCollector) IncNginxReloadErrors() {} diff --git a/internal/nginx/fake_manager.go b/internal/nginx/fake_manager.go index 460bd7276f..1e9ab59032 100644 --- a/internal/nginx/fake_manager.go +++ b/internal/nginx/fake_manager.go @@ -97,7 +97,7 @@ func (*FakeManager) Start(done chan error) { } // Reload provides a fake implementation of Reload. -func (*FakeManager) Reload() error { +func (*FakeManager) Reload(isEndpoint bool) error { glog.V(3).Infof("Reloading nginx") return nil } diff --git a/internal/nginx/manager.go b/internal/nginx/manager.go index 90c34e362e..ac743a80da 100644 --- a/internal/nginx/manager.go +++ b/internal/nginx/manager.go @@ -61,7 +61,7 @@ type Manager interface { CreateDHParam(content string) (string, error) CreateOpenTracingTracerConfig(content string) error Start(done chan error) - Reload() error + Reload(isEndpoint bool) error Quit() UpdateConfigVersionFile(openTracing bool) SetPlusClients(plusClient *client.NginxClient, plusConfigVersionCheckClient *http.Client) @@ -268,7 +268,7 @@ func (lm *LocalManager) Start(done chan error) { } // Reload reloads NGINX. -func (lm *LocalManager) Reload() error { +func (lm *LocalManager) Reload(isEndPoint bool) error { // write a new config version lm.configVersion++ lm.UpdateConfigVersionFile(lm.OpenTracing) @@ -287,7 +287,7 @@ func (lm *LocalManager) Reload() error { return fmt.Errorf("could not get newest config version: %v", err) } - lm.metricsCollector.IncNginxReloadCount() + lm.metricsCollector.IncNginxReloadCount(isEndPoint) t2 := time.Now() lm.metricsCollector.UpdateLastReloadTime(t2.Sub(t1)) From 30955c1e4c07203b9648d46f8c6539c460d51744 Mon Sep 17 00:00:00 2001 From: LorcanMcVeigh Date: Thu, 20 Aug 2020 10:49:19 +0100 Subject: [PATCH 2/5] Feedback --- docs-web/logging-and-monitoring/prometheus.md | 2 +- internal/configs/configurator.go | 45 ++++++++++--------- internal/metrics/collectors/manager.go | 17 ++++--- internal/nginx/manager.go | 4 +- 4 files changed, 38 insertions(+), 30 deletions(-) diff --git a/docs-web/logging-and-monitoring/prometheus.md b/docs-web/logging-and-monitoring/prometheus.md index 8c37f85a94..1d1b30732a 100644 --- a/docs-web/logging-and-monitoring/prometheus.md +++ b/docs-web/logging-and-monitoring/prometheus.md @@ -26,7 +26,7 @@ The Ingress Controller exports the following metrics: * NGINX/NGINX Plus metrics. Please see this [doc](https://github.com/nginxinc/nginx-prometheus-exporter#exported-metrics) to find more information about the exported metrics. * Ingress Controller metrics - * `controller_nginx_reloads_total`. Number of successful NGINX reloads. This includes the label `reason` with 2 possible values `endpoint` ( the reason for the reload was an endpoint was updated ) and `other` ( the reload was caused by something other than a reload ). + * `controller_nginx_reloads_total`. Number of successful NGINX reloads. This includes the label `reason` with 2 possible values `endpoints` ( the reason for the reload was an endpoints update ) and `other` ( the reload was caused by something other than an endpoint update like an ingress update ). * `controller_nginx_reload_errors_total`. Number of unsuccessful NGINX reloads. * `controller_nginx_last_reload_status`. Status of the last NGINX reload, 0 meaning down and 1 up. * `controller_nginx_last_reload_milliseconds`. Duration in milliseconds of the last NGINX reload. diff --git a/internal/configs/configurator.go b/internal/configs/configurator.go index 1bdf4dbef1..7d1f9f976e 100644 --- a/internal/configs/configurator.go +++ b/internal/configs/configurator.go @@ -15,6 +15,7 @@ import ( "github.com/spiffe/go-spiffe/workload" "github.com/nginxinc/kubernetes-ingress/internal/configs/version2" + "github.com/nginxinc/kubernetes-ingress/internal/metrics/collectors" conf_v1alpha1 "github.com/nginxinc/kubernetes-ingress/pkg/apis/configuration/v1alpha1" "github.com/golang/glog" @@ -186,7 +187,7 @@ func (cnf *Configurator) AddOrUpdateIngress(ingEx *IngressEx) error { return fmt.Errorf("Error adding or updating ingress %v/%v: %v", ingEx.Ingress.Namespace, ingEx.Ingress.Name, err) } - if err := cnf.nginxManager.Reload(true); err != nil { + if err := cnf.nginxManager.Reload(collectors.ReloadForOtherUpdate); err != nil { return fmt.Errorf("Error reloading NGINX for %v/%v: %v", ingEx.Ingress.Namespace, ingEx.Ingress.Name, err) } @@ -222,7 +223,7 @@ func (cnf *Configurator) AddOrUpdateMergeableIngress(mergeableIngs *MergeableIng return fmt.Errorf("Error when adding or updating ingress %v/%v: %v", mergeableIngs.Master.Ingress.Namespace, mergeableIngs.Master.Ingress.Name, err) } - if err := cnf.nginxManager.Reload(true); err != nil { + if err := cnf.nginxManager.Reload(collectors.ReloadForOtherUpdate); err != nil { return fmt.Errorf("Error reloading NGINX for %v/%v: %v", mergeableIngs.Master.Ingress.Namespace, mergeableIngs.Master.Ingress.Name, err) } @@ -309,7 +310,7 @@ func (cnf *Configurator) AddOrUpdateVirtualServer(virtualServerEx *VirtualServer return warnings, fmt.Errorf("Error adding or updating VirtualServer %v/%v: %v", virtualServerEx.VirtualServer.Namespace, virtualServerEx.VirtualServer.Name, err) } - if err := cnf.nginxManager.Reload(true); err != nil { + if err := cnf.nginxManager.Reload(collectors.ReloadForOtherUpdate); err != nil { return warnings, fmt.Errorf("Error reloading NGINX for VirtualServer %v/%v: %v", virtualServerEx.VirtualServer.Namespace, virtualServerEx.VirtualServer.Name, err) } @@ -356,7 +357,7 @@ func (cnf *Configurator) AddOrUpdateVirtualServers(virtualServerExes []*VirtualS allWarnings.Add(warnings) } - if err := cnf.nginxManager.Reload(false); err != nil { + if err := cnf.nginxManager.Reload(collectors.ReloadForOtherUpdate); err != nil { return allWarnings, fmt.Errorf("Error when reloading NGINX when updating Policy: %v", err) } @@ -371,7 +372,7 @@ func (cnf *Configurator) AddOrUpdateTransportServer(transportServerEx *Transport return fmt.Errorf("Error adding or updating TransportServer %v/%v: %v", transportServerEx.TransportServer.Namespace, transportServerEx.TransportServer.Name, err) } - if err := cnf.nginxManager.Reload(true); err != nil { + if err := cnf.nginxManager.Reload(collectors.ReloadForOtherUpdate); err != nil { return fmt.Errorf("Error reloading NGINX for TransportServer %v/%v: %v", transportServerEx.TransportServer.Namespace, transportServerEx.TransportServer.Name, err) } @@ -530,7 +531,7 @@ func (cnf *Configurator) AddOrUpdateTLSSecret(secret *api_v1.Secret, ingExes []I } } - if err := cnf.nginxManager.Reload(false); err != nil { + if err := cnf.nginxManager.Reload(collectors.ReloadForOtherUpdate); err != nil { return fmt.Errorf("Error when reloading NGINX when updating Secret: %v", err) } @@ -551,7 +552,7 @@ func (cnf *Configurator) AddOrUpdateSpecialTLSSecrets(secret *api_v1.Secret, sec cnf.nginxManager.CreateSecret(secretName, data, nginx.TLSSecretFileMode) } - if err := cnf.nginxManager.Reload(false); err != nil { + if err := cnf.nginxManager.Reload(collectors.ReloadForOtherUpdate); err != nil { return fmt.Errorf("Error when reloading NGINX when updating the special Secrets: %v", err) } @@ -597,7 +598,7 @@ func (cnf *Configurator) DeleteSecret(key string, ingExes []IngressEx, mergeable } if len(ingExes)+len(mergeableIngresses)+len(virtualServerExes) > 0 { - if err := cnf.nginxManager.Reload(false); err != nil { + if err := cnf.nginxManager.Reload(collectors.ReloadForOtherUpdate); err != nil { return fmt.Errorf("Error when reloading NGINX when deleting Secret %v: %v", key, err) } } @@ -617,7 +618,7 @@ func (cnf *Configurator) DeleteIngress(key string) error { cnf.deleteIngressMetricsLabels(key) } - if err := cnf.nginxManager.Reload(true); err != nil { + if err := cnf.nginxManager.Reload(collectors.ReloadForOtherUpdate); err != nil { return fmt.Errorf("Error when removing ingress %v: %v", key, err) } @@ -634,7 +635,7 @@ func (cnf *Configurator) DeleteVirtualServer(key string) error { cnf.deleteVirtualServerMetricsLabels(fmt.Sprintf(key)) } - if err := cnf.nginxManager.Reload(true); err != nil { + if err := cnf.nginxManager.Reload(collectors.ReloadForOtherUpdate); err != nil { return fmt.Errorf("Error when removing VirtualServer %v: %v", key, err) } @@ -648,7 +649,7 @@ func (cnf *Configurator) DeleteTransportServer(key string) error { return fmt.Errorf("Error when removing TransportServer %v: %v", key, err) } - err = cnf.nginxManager.Reload(true) + err = cnf.nginxManager.Reload(collectors.ReloadForOtherUpdate) if err != nil { return fmt.Errorf("Error when removing TransportServer %v: %v", key, err) } @@ -694,7 +695,7 @@ func (cnf *Configurator) UpdateEndpoints(ingExes []*IngressEx) error { return nil } - if err := cnf.nginxManager.Reload(true); err != nil { + if err := cnf.nginxManager.Reload(collectors.ReloadForEndpointsUpdate); err != nil { return fmt.Errorf("Error reloading NGINX when updating endpoints: %v", err) } @@ -727,7 +728,7 @@ func (cnf *Configurator) UpdateEndpointsMergeableIngress(mergeableIngresses []*M return nil } - if err := cnf.nginxManager.Reload(true); err != nil { + if err := cnf.nginxManager.Reload(collectors.ReloadForEndpointsUpdate); err != nil { return fmt.Errorf("Error reloading NGINX when updating endpoints for %v: %v", mergeableIngresses, err) } @@ -759,7 +760,7 @@ func (cnf *Configurator) UpdateEndpointsForVirtualServers(virtualServerExes []*V return nil } - if err := cnf.nginxManager.Reload(true); err != nil { + if err := cnf.nginxManager.Reload(collectors.ReloadForEndpointsUpdate); err != nil { return fmt.Errorf("Error reloading NGINX when updating endpoints: %v", err) } @@ -806,7 +807,7 @@ func (cnf *Configurator) UpdateEndpointsForTransportServers(transportServerExes return nil } - if err := cnf.nginxManager.Reload(true); err != nil { + if err := cnf.nginxManager.Reload(collectors.ReloadForEndpointsUpdate); err != nil { return fmt.Errorf("Error reloading NGINX when updating endpoints: %v", err) } @@ -948,7 +949,7 @@ func (cnf *Configurator) UpdateConfig(cfgParams *ConfigParams, ingExes []*Ingres } cnf.nginxManager.SetOpenTracing(mainCfg.OpenTracingLoadModule) - if err := cnf.nginxManager.Reload(false); err != nil { + if err := cnf.nginxManager.Reload(collectors.ReloadForOtherUpdate); err != nil { return allWarnings, fmt.Errorf("Error when updating config from ConfigMap: %v", err) } @@ -981,7 +982,7 @@ func (cnf *Configurator) UpdateGlobalConfiguration(globalConfiguration *conf_v1a } } - if err := cnf.nginxManager.Reload(false); err != nil { + if err := cnf.nginxManager.Reload(collectors.ReloadForOtherUpdate); err != nil { return updatedTransportServerExes, deletedTransportServerExes, fmt.Errorf("Error when updating global configuration: %v", err) } @@ -1097,7 +1098,7 @@ func (cnf *Configurator) AddOrUpdateSpiffeCerts(svidResponse *workload.X509SVIDs cnf.nginxManager.CreateSecret(spiffeCertFileName, createSpiffeCert(svid.Certificates), spiffeCertsFileMode) cnf.nginxManager.CreateSecret(spiffeBundleFileName, createSpiffeCert(svid.TrustBundle), spiffeCertsFileMode) - err = cnf.nginxManager.Reload(false) + err = cnf.nginxManager.Reload(collectors.ReloadForOtherUpdate) if err != nil { return fmt.Errorf("error when reloading NGINX when updating the SPIFFE Certs: %v", err) } @@ -1174,7 +1175,7 @@ func (cnf *Configurator) AddOrUpdateAppProtectResource(resource *unstructured.Un } } - if err := cnf.nginxManager.Reload(false); err != nil { + if err := cnf.nginxManager.Reload(collectors.ReloadForOtherUpdate); err != nil { return fmt.Errorf("Error when reloading NGINX when updating %v: %v", resource.GetKind(), err) } @@ -1201,7 +1202,7 @@ func (cnf *Configurator) DeleteAppProtectPolicy(polNamespaceame string, ingExes } } - if err := cnf.nginxManager.Reload(false); err != nil { + if err := cnf.nginxManager.Reload(collectors.ReloadForOtherUpdate); err != nil { return fmt.Errorf("Error when reloading NGINX when removing App Protect Policy: %v", err) } @@ -1228,7 +1229,7 @@ func (cnf *Configurator) DeleteAppProtectLogConf(logConfNamespaceame string, ing } } - if err := cnf.nginxManager.Reload(false); err != nil { + if err := cnf.nginxManager.Reload(collectors.ReloadForOtherUpdate); err != nil { return fmt.Errorf("Error when reloading NGINX when removing App Protect Log Configuration: %v", err) } @@ -1246,7 +1247,7 @@ func (cnf *Configurator) AddInternalRouteConfig() error { return fmt.Errorf("Error when writing main Config: %v", err) } cnf.nginxManager.CreateMainConfig(mainCfgContent) - if err := cnf.nginxManager.Reload(false); err != nil { + if err := cnf.nginxManager.Reload(collectors.ReloadForOtherUpdate); err != nil { return fmt.Errorf("Error when reloading nginx: %v", err) } return nil diff --git a/internal/metrics/collectors/manager.go b/internal/metrics/collectors/manager.go index 8ea11fff04..4305a64d38 100644 --- a/internal/metrics/collectors/manager.go +++ b/internal/metrics/collectors/manager.go @@ -6,9 +6,16 @@ import ( "github.com/prometheus/client_golang/prometheus" ) +const ( + // ReloadForEndpointsUpdate means that is caused by an endpoints update. + ReloadForEndpointsUpdate = true + // ReloadForOtherUpdate means that a reload is caused by an update for a resource(s) other than endpoints. + ReloadForOtherUpdate = false +) + // ManagerCollector is an interface for the metrics of the Nginx Manager type ManagerCollector interface { - IncNginxReloadCount(isEndPoint bool) + IncNginxReloadCount(isEndPointUpdate bool) IncNginxReloadErrors() UpdateLastReloadTime(ms time.Duration) Register(registry *prometheus.Registry) error @@ -64,10 +71,10 @@ func NewLocalManagerMetricsCollector(constLabels map[string]string) *LocalManage } // IncNginxReloadCount increments the counter of successful NGINX reloads and sets the last reload status to true -func (nc *LocalManagerMetricsCollector) IncNginxReloadCount(isEndPoint bool) { +func (nc *LocalManagerMetricsCollector) IncNginxReloadCount(isEndPointUpdate bool) { var label string - if isEndPoint { - label = "endpoint" + if isEndPointUpdate { + label = "endpoints" } else { label = "other" } @@ -128,7 +135,7 @@ func NewManagerFakeCollector() *ManagerFakeCollector { func (nc *ManagerFakeCollector) Register(registry *prometheus.Registry) error { return nil } // IncNginxReloadCount implements a fake IncNginxReloadCount -func (nc *ManagerFakeCollector) IncNginxReloadCount(isEnPoint bool) {} +func (nc *ManagerFakeCollector) IncNginxReloadCount(isEndPointUpdate bool) {} // IncNginxReloadErrors implements a fake IncNginxReloadErrors func (nc *ManagerFakeCollector) IncNginxReloadErrors() {} diff --git a/internal/nginx/manager.go b/internal/nginx/manager.go index ac743a80da..5c33c369c2 100644 --- a/internal/nginx/manager.go +++ b/internal/nginx/manager.go @@ -268,7 +268,7 @@ func (lm *LocalManager) Start(done chan error) { } // Reload reloads NGINX. -func (lm *LocalManager) Reload(isEndPoint bool) error { +func (lm *LocalManager) Reload(isEndPointUpdate bool) error { // write a new config version lm.configVersion++ lm.UpdateConfigVersionFile(lm.OpenTracing) @@ -287,7 +287,7 @@ func (lm *LocalManager) Reload(isEndPoint bool) error { return fmt.Errorf("could not get newest config version: %v", err) } - lm.metricsCollector.IncNginxReloadCount(isEndPoint) + lm.metricsCollector.IncNginxReloadCount(isEndPointUpdate) t2 := time.Now() lm.metricsCollector.UpdateLastReloadTime(t2.Sub(t1)) From 0b76849bf24d07b1f27a05e92d8d8cf24cca061c Mon Sep 17 00:00:00 2001 From: LorcanMcVeigh Date: Thu, 20 Aug 2020 13:53:31 +0100 Subject: [PATCH 3/5] Feedback 2 --- deployments/helm-chart/values.yaml | 2 +- internal/metrics/collectors/manager.go | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/deployments/helm-chart/values.yaml b/deployments/helm-chart/values.yaml index 16e14ba373..caa734f308 100644 --- a/deployments/helm-chart/values.yaml +++ b/deployments/helm-chart/values.yaml @@ -20,7 +20,7 @@ controller: enable: false ## Enables the Ingress controller pods to use the host's network namespace. - hostNetwork: false + hostNetwork: true ## Enables debugging for NGINX. Uses the nginx-debug binary. Requires error-log-level: debug in the ConfigMap via `controller.config.entries`. nginxDebug: false diff --git a/internal/metrics/collectors/manager.go b/internal/metrics/collectors/manager.go index 4305a64d38..11f719b64e 100644 --- a/internal/metrics/collectors/manager.go +++ b/internal/metrics/collectors/manager.go @@ -72,13 +72,13 @@ func NewLocalManagerMetricsCollector(constLabels map[string]string) *LocalManage // IncNginxReloadCount increments the counter of successful NGINX reloads and sets the last reload status to true func (nc *LocalManagerMetricsCollector) IncNginxReloadCount(isEndPointUpdate bool) { - var label string if isEndPointUpdate { - label = "endpoints" + nc.reloadsTotal.WithLabelValues("endpoints").Inc() + nc.reloadsTotal.WithLabelValues("other") } else { - label = "other" + nc.reloadsTotal.WithLabelValues("other").Inc() + nc.reloadsTotal.WithLabelValues("endpoints") } - nc.reloadsTotal.WithLabelValues(label).Inc() nc.updateLastReloadStatus(true) } From a88ef29c3075c7335fa737106044002b0a0f4823 Mon Sep 17 00:00:00 2001 From: LorcanMcVeigh Date: Fri, 21 Aug 2020 10:45:40 +0100 Subject: [PATCH 4/5] Feedback 3 --- deployments/helm-chart/values.yaml | 2 +- internal/configs/configurator.go | 45 +++++++++++++------------- internal/metrics/collectors/manager.go | 17 ++++------ internal/nginx/fake_manager.go | 2 +- internal/nginx/manager.go | 12 +++++-- 5 files changed, 39 insertions(+), 39 deletions(-) diff --git a/deployments/helm-chart/values.yaml b/deployments/helm-chart/values.yaml index caa734f308..16e14ba373 100644 --- a/deployments/helm-chart/values.yaml +++ b/deployments/helm-chart/values.yaml @@ -20,7 +20,7 @@ controller: enable: false ## Enables the Ingress controller pods to use the host's network namespace. - hostNetwork: true + hostNetwork: false ## Enables debugging for NGINX. Uses the nginx-debug binary. Requires error-log-level: debug in the ConfigMap via `controller.config.entries`. nginxDebug: false diff --git a/internal/configs/configurator.go b/internal/configs/configurator.go index 7d1f9f976e..93bfb4950f 100644 --- a/internal/configs/configurator.go +++ b/internal/configs/configurator.go @@ -15,7 +15,6 @@ import ( "github.com/spiffe/go-spiffe/workload" "github.com/nginxinc/kubernetes-ingress/internal/configs/version2" - "github.com/nginxinc/kubernetes-ingress/internal/metrics/collectors" conf_v1alpha1 "github.com/nginxinc/kubernetes-ingress/pkg/apis/configuration/v1alpha1" "github.com/golang/glog" @@ -187,7 +186,7 @@ func (cnf *Configurator) AddOrUpdateIngress(ingEx *IngressEx) error { return fmt.Errorf("Error adding or updating ingress %v/%v: %v", ingEx.Ingress.Namespace, ingEx.Ingress.Name, err) } - if err := cnf.nginxManager.Reload(collectors.ReloadForOtherUpdate); err != nil { + if err := cnf.nginxManager.Reload(nginx.ReloadForOtherUpdate); err != nil { return fmt.Errorf("Error reloading NGINX for %v/%v: %v", ingEx.Ingress.Namespace, ingEx.Ingress.Name, err) } @@ -223,7 +222,7 @@ func (cnf *Configurator) AddOrUpdateMergeableIngress(mergeableIngs *MergeableIng return fmt.Errorf("Error when adding or updating ingress %v/%v: %v", mergeableIngs.Master.Ingress.Namespace, mergeableIngs.Master.Ingress.Name, err) } - if err := cnf.nginxManager.Reload(collectors.ReloadForOtherUpdate); err != nil { + if err := cnf.nginxManager.Reload(nginx.ReloadForOtherUpdate); err != nil { return fmt.Errorf("Error reloading NGINX for %v/%v: %v", mergeableIngs.Master.Ingress.Namespace, mergeableIngs.Master.Ingress.Name, err) } @@ -310,7 +309,7 @@ func (cnf *Configurator) AddOrUpdateVirtualServer(virtualServerEx *VirtualServer return warnings, fmt.Errorf("Error adding or updating VirtualServer %v/%v: %v", virtualServerEx.VirtualServer.Namespace, virtualServerEx.VirtualServer.Name, err) } - if err := cnf.nginxManager.Reload(collectors.ReloadForOtherUpdate); err != nil { + if err := cnf.nginxManager.Reload(nginx.ReloadForOtherUpdate); err != nil { return warnings, fmt.Errorf("Error reloading NGINX for VirtualServer %v/%v: %v", virtualServerEx.VirtualServer.Namespace, virtualServerEx.VirtualServer.Name, err) } @@ -357,7 +356,7 @@ func (cnf *Configurator) AddOrUpdateVirtualServers(virtualServerExes []*VirtualS allWarnings.Add(warnings) } - if err := cnf.nginxManager.Reload(collectors.ReloadForOtherUpdate); err != nil { + if err := cnf.nginxManager.Reload(nginx.ReloadForOtherUpdate); err != nil { return allWarnings, fmt.Errorf("Error when reloading NGINX when updating Policy: %v", err) } @@ -372,7 +371,7 @@ func (cnf *Configurator) AddOrUpdateTransportServer(transportServerEx *Transport return fmt.Errorf("Error adding or updating TransportServer %v/%v: %v", transportServerEx.TransportServer.Namespace, transportServerEx.TransportServer.Name, err) } - if err := cnf.nginxManager.Reload(collectors.ReloadForOtherUpdate); err != nil { + if err := cnf.nginxManager.Reload(nginx.ReloadForOtherUpdate); err != nil { return fmt.Errorf("Error reloading NGINX for TransportServer %v/%v: %v", transportServerEx.TransportServer.Namespace, transportServerEx.TransportServer.Name, err) } @@ -531,7 +530,7 @@ func (cnf *Configurator) AddOrUpdateTLSSecret(secret *api_v1.Secret, ingExes []I } } - if err := cnf.nginxManager.Reload(collectors.ReloadForOtherUpdate); err != nil { + if err := cnf.nginxManager.Reload(nginx.ReloadForOtherUpdate); err != nil { return fmt.Errorf("Error when reloading NGINX when updating Secret: %v", err) } @@ -552,7 +551,7 @@ func (cnf *Configurator) AddOrUpdateSpecialTLSSecrets(secret *api_v1.Secret, sec cnf.nginxManager.CreateSecret(secretName, data, nginx.TLSSecretFileMode) } - if err := cnf.nginxManager.Reload(collectors.ReloadForOtherUpdate); err != nil { + if err := cnf.nginxManager.Reload(nginx.ReloadForOtherUpdate); err != nil { return fmt.Errorf("Error when reloading NGINX when updating the special Secrets: %v", err) } @@ -598,7 +597,7 @@ func (cnf *Configurator) DeleteSecret(key string, ingExes []IngressEx, mergeable } if len(ingExes)+len(mergeableIngresses)+len(virtualServerExes) > 0 { - if err := cnf.nginxManager.Reload(collectors.ReloadForOtherUpdate); err != nil { + if err := cnf.nginxManager.Reload(nginx.ReloadForOtherUpdate); err != nil { return fmt.Errorf("Error when reloading NGINX when deleting Secret %v: %v", key, err) } } @@ -618,7 +617,7 @@ func (cnf *Configurator) DeleteIngress(key string) error { cnf.deleteIngressMetricsLabels(key) } - if err := cnf.nginxManager.Reload(collectors.ReloadForOtherUpdate); err != nil { + if err := cnf.nginxManager.Reload(nginx.ReloadForOtherUpdate); err != nil { return fmt.Errorf("Error when removing ingress %v: %v", key, err) } @@ -635,7 +634,7 @@ func (cnf *Configurator) DeleteVirtualServer(key string) error { cnf.deleteVirtualServerMetricsLabels(fmt.Sprintf(key)) } - if err := cnf.nginxManager.Reload(collectors.ReloadForOtherUpdate); err != nil { + if err := cnf.nginxManager.Reload(nginx.ReloadForOtherUpdate); err != nil { return fmt.Errorf("Error when removing VirtualServer %v: %v", key, err) } @@ -649,7 +648,7 @@ func (cnf *Configurator) DeleteTransportServer(key string) error { return fmt.Errorf("Error when removing TransportServer %v: %v", key, err) } - err = cnf.nginxManager.Reload(collectors.ReloadForOtherUpdate) + err = cnf.nginxManager.Reload(nginx.ReloadForOtherUpdate) if err != nil { return fmt.Errorf("Error when removing TransportServer %v: %v", key, err) } @@ -695,7 +694,7 @@ func (cnf *Configurator) UpdateEndpoints(ingExes []*IngressEx) error { return nil } - if err := cnf.nginxManager.Reload(collectors.ReloadForEndpointsUpdate); err != nil { + if err := cnf.nginxManager.Reload(nginx.ReloadForEndpointsUpdate); err != nil { return fmt.Errorf("Error reloading NGINX when updating endpoints: %v", err) } @@ -728,7 +727,7 @@ func (cnf *Configurator) UpdateEndpointsMergeableIngress(mergeableIngresses []*M return nil } - if err := cnf.nginxManager.Reload(collectors.ReloadForEndpointsUpdate); err != nil { + if err := cnf.nginxManager.Reload(nginx.ReloadForEndpointsUpdate); err != nil { return fmt.Errorf("Error reloading NGINX when updating endpoints for %v: %v", mergeableIngresses, err) } @@ -760,7 +759,7 @@ func (cnf *Configurator) UpdateEndpointsForVirtualServers(virtualServerExes []*V return nil } - if err := cnf.nginxManager.Reload(collectors.ReloadForEndpointsUpdate); err != nil { + if err := cnf.nginxManager.Reload(nginx.ReloadForEndpointsUpdate); err != nil { return fmt.Errorf("Error reloading NGINX when updating endpoints: %v", err) } @@ -807,7 +806,7 @@ func (cnf *Configurator) UpdateEndpointsForTransportServers(transportServerExes return nil } - if err := cnf.nginxManager.Reload(collectors.ReloadForEndpointsUpdate); err != nil { + if err := cnf.nginxManager.Reload(nginx.ReloadForEndpointsUpdate); err != nil { return fmt.Errorf("Error reloading NGINX when updating endpoints: %v", err) } @@ -949,7 +948,7 @@ func (cnf *Configurator) UpdateConfig(cfgParams *ConfigParams, ingExes []*Ingres } cnf.nginxManager.SetOpenTracing(mainCfg.OpenTracingLoadModule) - if err := cnf.nginxManager.Reload(collectors.ReloadForOtherUpdate); err != nil { + if err := cnf.nginxManager.Reload(nginx.ReloadForOtherUpdate); err != nil { return allWarnings, fmt.Errorf("Error when updating config from ConfigMap: %v", err) } @@ -982,7 +981,7 @@ func (cnf *Configurator) UpdateGlobalConfiguration(globalConfiguration *conf_v1a } } - if err := cnf.nginxManager.Reload(collectors.ReloadForOtherUpdate); err != nil { + if err := cnf.nginxManager.Reload(nginx.ReloadForOtherUpdate); err != nil { return updatedTransportServerExes, deletedTransportServerExes, fmt.Errorf("Error when updating global configuration: %v", err) } @@ -1098,7 +1097,7 @@ func (cnf *Configurator) AddOrUpdateSpiffeCerts(svidResponse *workload.X509SVIDs cnf.nginxManager.CreateSecret(spiffeCertFileName, createSpiffeCert(svid.Certificates), spiffeCertsFileMode) cnf.nginxManager.CreateSecret(spiffeBundleFileName, createSpiffeCert(svid.TrustBundle), spiffeCertsFileMode) - err = cnf.nginxManager.Reload(collectors.ReloadForOtherUpdate) + err = cnf.nginxManager.Reload(nginx.ReloadForOtherUpdate) if err != nil { return fmt.Errorf("error when reloading NGINX when updating the SPIFFE Certs: %v", err) } @@ -1175,7 +1174,7 @@ func (cnf *Configurator) AddOrUpdateAppProtectResource(resource *unstructured.Un } } - if err := cnf.nginxManager.Reload(collectors.ReloadForOtherUpdate); err != nil { + if err := cnf.nginxManager.Reload(nginx.ReloadForOtherUpdate); err != nil { return fmt.Errorf("Error when reloading NGINX when updating %v: %v", resource.GetKind(), err) } @@ -1202,7 +1201,7 @@ func (cnf *Configurator) DeleteAppProtectPolicy(polNamespaceame string, ingExes } } - if err := cnf.nginxManager.Reload(collectors.ReloadForOtherUpdate); err != nil { + if err := cnf.nginxManager.Reload(nginx.ReloadForOtherUpdate); err != nil { return fmt.Errorf("Error when reloading NGINX when removing App Protect Policy: %v", err) } @@ -1229,7 +1228,7 @@ func (cnf *Configurator) DeleteAppProtectLogConf(logConfNamespaceame string, ing } } - if err := cnf.nginxManager.Reload(collectors.ReloadForOtherUpdate); err != nil { + if err := cnf.nginxManager.Reload(nginx.ReloadForOtherUpdate); err != nil { return fmt.Errorf("Error when reloading NGINX when removing App Protect Log Configuration: %v", err) } @@ -1247,7 +1246,7 @@ func (cnf *Configurator) AddInternalRouteConfig() error { return fmt.Errorf("Error when writing main Config: %v", err) } cnf.nginxManager.CreateMainConfig(mainCfgContent) - if err := cnf.nginxManager.Reload(collectors.ReloadForOtherUpdate); err != nil { + if err := cnf.nginxManager.Reload(nginx.ReloadForOtherUpdate); err != nil { return fmt.Errorf("Error when reloading nginx: %v", err) } return nil diff --git a/internal/metrics/collectors/manager.go b/internal/metrics/collectors/manager.go index 11f719b64e..1c64de5b4d 100644 --- a/internal/metrics/collectors/manager.go +++ b/internal/metrics/collectors/manager.go @@ -6,13 +6,6 @@ import ( "github.com/prometheus/client_golang/prometheus" ) -const ( - // ReloadForEndpointsUpdate means that is caused by an endpoints update. - ReloadForEndpointsUpdate = true - // ReloadForOtherUpdate means that a reload is caused by an update for a resource(s) other than endpoints. - ReloadForOtherUpdate = false -) - // ManagerCollector is an interface for the metrics of the Nginx Manager type ManagerCollector interface { IncNginxReloadCount(isEndPointUpdate bool) @@ -67,18 +60,20 @@ func NewLocalManagerMetricsCollector(constLabels map[string]string) *LocalManage }, ), } + nc.reloadsTotal.WithLabelValues("other") + nc.reloadsTotal.WithLabelValues("endpoints") return nc } // IncNginxReloadCount increments the counter of successful NGINX reloads and sets the last reload status to true func (nc *LocalManagerMetricsCollector) IncNginxReloadCount(isEndPointUpdate bool) { + var label string if isEndPointUpdate { - nc.reloadsTotal.WithLabelValues("endpoints").Inc() - nc.reloadsTotal.WithLabelValues("other") + label = "endpoints" } else { - nc.reloadsTotal.WithLabelValues("other").Inc() - nc.reloadsTotal.WithLabelValues("endpoints") + label = "other" } + nc.reloadsTotal.WithLabelValues(label).Inc() nc.updateLastReloadStatus(true) } diff --git a/internal/nginx/fake_manager.go b/internal/nginx/fake_manager.go index 1e9ab59032..e5045c2efd 100644 --- a/internal/nginx/fake_manager.go +++ b/internal/nginx/fake_manager.go @@ -97,7 +97,7 @@ func (*FakeManager) Start(done chan error) { } // Reload provides a fake implementation of Reload. -func (*FakeManager) Reload(isEndpoint bool) error { +func (*FakeManager) Reload(isEndpointsUpdate bool) error { glog.V(3).Infof("Reloading nginx") return nil } diff --git a/internal/nginx/manager.go b/internal/nginx/manager.go index 5c33c369c2..d479f17eed 100644 --- a/internal/nginx/manager.go +++ b/internal/nginx/manager.go @@ -15,6 +15,12 @@ import ( "github.com/nginxinc/nginx-plus-go-client/client" ) +// ReloadForEndpointsUpdate means that is caused by an endpoints update. +const ReloadForEndpointsUpdate = true + +// ReloadForOtherUpdate means that a reload is caused by an update for a resource(s) other than endpoints. +const ReloadForOtherUpdate = false + // TLSSecretFileMode defines the default filemode for files with TLS Secrets. const TLSSecretFileMode = 0600 @@ -61,7 +67,7 @@ type Manager interface { CreateDHParam(content string) (string, error) CreateOpenTracingTracerConfig(content string) error Start(done chan error) - Reload(isEndpoint bool) error + Reload(isEndpointsUpdate bool) error Quit() UpdateConfigVersionFile(openTracing bool) SetPlusClients(plusClient *client.NginxClient, plusConfigVersionCheckClient *http.Client) @@ -268,7 +274,7 @@ func (lm *LocalManager) Start(done chan error) { } // Reload reloads NGINX. -func (lm *LocalManager) Reload(isEndPointUpdate bool) error { +func (lm *LocalManager) Reload(isEndpointsUpdate bool) error { // write a new config version lm.configVersion++ lm.UpdateConfigVersionFile(lm.OpenTracing) @@ -287,7 +293,7 @@ func (lm *LocalManager) Reload(isEndPointUpdate bool) error { return fmt.Errorf("could not get newest config version: %v", err) } - lm.metricsCollector.IncNginxReloadCount(isEndPointUpdate) + lm.metricsCollector.IncNginxReloadCount(isEndpointsUpdate) t2 := time.Now() lm.metricsCollector.UpdateLastReloadTime(t2.Sub(t1)) From 2799044eb2fc491287a42cfbe39aebeed6293904 Mon Sep 17 00:00:00 2001 From: Lorcan McVeigh Date: Fri, 21 Aug 2020 10:46:50 +0100 Subject: [PATCH 5/5] Update docs-web Co-authored-by: Michael Pleshakov --- docs-web/logging-and-monitoring/prometheus.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs-web/logging-and-monitoring/prometheus.md b/docs-web/logging-and-monitoring/prometheus.md index 1d1b30732a..39a60a1624 100644 --- a/docs-web/logging-and-monitoring/prometheus.md +++ b/docs-web/logging-and-monitoring/prometheus.md @@ -26,7 +26,7 @@ The Ingress Controller exports the following metrics: * NGINX/NGINX Plus metrics. Please see this [doc](https://github.com/nginxinc/nginx-prometheus-exporter#exported-metrics) to find more information about the exported metrics. * Ingress Controller metrics - * `controller_nginx_reloads_total`. Number of successful NGINX reloads. This includes the label `reason` with 2 possible values `endpoints` ( the reason for the reload was an endpoints update ) and `other` ( the reload was caused by something other than an endpoint update like an ingress update ). + * `controller_nginx_reloads_total`. Number of successful NGINX reloads. This includes the label `reason` with 2 possible values `endpoints` (the reason for the reload was an endpoints update) and `other` (the reload was caused by something other than an endpoint update like an ingress update). * `controller_nginx_reload_errors_total`. Number of unsuccessful NGINX reloads. * `controller_nginx_last_reload_status`. Status of the last NGINX reload, 0 meaning down and 1 up. * `controller_nginx_last_reload_milliseconds`. Duration in milliseconds of the last NGINX reload.