Skip to content

Commit f188c16

Browse files
jan--fjan--f
committed
telemetry: deprecate TelemeterClientConfig
This introduces a new config field `TelemetryConfig` and deprecates `TelemeterClientConfig`. The deprecated field will remain and act as a fallback if no `TelemetryConfig` is provided. Some tests relating to the deprecated field where changed to make tests pass while keeping changes minimal. Signed-off-by: Jan Fajerski <[email protected]>
1 parent 9ac1fad commit f188c16

File tree

10 files changed

+237
-200
lines changed

10 files changed

+237
-200
lines changed

‎Documentation/api.md‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ The `ClusterMonitoringConfiguration` resource defines settings that customize th
136136
| prometheusOperator | *[PrometheusOperatorConfig](#prometheusoperatorconfig) | `PrometheusOperatorConfig` defines settings for the Prometheus Operator component. |
137137
| prometheusOperatorAdmissionWebhook | *[PrometheusOperatorAdmissionWebhookConfig](#prometheusoperatoradmissionwebhookconfig) | `PrometheusOperatorAdmissionWebhookConfig` defines settings for the Prometheus Operator's admission webhook component. |
138138
| openshiftStateMetrics | *[OpenShiftStateMetricsConfig](#openshiftstatemetricsconfig) | `OpenShiftMetricsConfig` defines settings for the `openshift-state-metrics` agent. |
139-
| telemeterClient | *[TelemeterClientConfig](#telemeterclientconfig) | `TelemeterClientConfig` defines settings for the Telemeter Client component. |
139+
| telemetryConfig | *[TelemetryConfig](#telemetryconfig) | TelemetryConfig defines settings for telemetry reporting. |
140140
| thanosQuerier | *[ThanosQuerierConfig](#thanosquerierconfig) | `ThanosQuerierConfig` defines settings for the Thanos Querier component. |
141141
| nodeExporter | [NodeExporterConfig](#nodeexporterconfig) | `NodeExporterConfig` defines settings for the `node-exporter` agent. |
142142
| monitoringPlugin | *[MonitoringPluginConfig](#monitoringpluginconfig) | `MonitoringPluginConfig` defines settings for the monitoring `console-plugin`. |
@@ -568,9 +568,6 @@ The `TLSConfig` resource configures the settings for TLS connections.
568568
#### Required
569569
- ` nodeSelector `
570570
- ` tolerations `
571-
572-
<em>appears in: [ClusterMonitoringConfiguration](#clustermonitoringconfiguration)</em>
573-
574571
| Property | Type | Description |
575572
| -------- | ---- | ----------- |
576573
| nodeSelector | map[string]string | Defines the nodes on which the pods are scheduled. |

‎Documentation/openshiftdocs/modules/clustermonitoringconfiguration.adoc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ The `ClusterMonitoringConfiguration` resource defines settings that customize th
3333

3434
|openshiftStateMetrics|*link:openshiftstatemetricsconfig.adoc[OpenShiftStateMetricsConfig]|`OpenShiftMetricsConfig` defines settings for the `openshift-state-metrics` agent.
3535

36-
|telemeterClient|*link:telemeterclientconfig.adoc[TelemeterClientConfig]|`TelemeterClientConfig` defines settings for the Telemeter Client component.
36+
|telemetryConfig|*link:telemetryconfig.adoc[TelemetryConfig]|TelemetryConfig defines settings for telemetry reporting.
3737

3838
|thanosQuerier|*link:thanosquerierconfig.adoc[ThanosQuerierConfig]|`ThanosQuerierConfig` defines settings for the Thanos Querier component.
3939

‎Documentation/openshiftdocs/modules/telemeterclientconfig.adoc‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,6 @@
1515
* `nodeSelector`
1616
* `tolerations`
1717

18-
19-
Appears in: link:clustermonitoringconfiguration.adoc[ClusterMonitoringConfiguration]
20-
2118
[options="header"]
2219
|===
2320
| Property | Type | Description

‎pkg/manifests/config.go‎

Lines changed: 46 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,20 @@ type Audit struct {
261261
Profile auditv1.Level `json:"profile"`
262262
}
263263

264+
func (cfg *TelemetryConfig) IsEnabled() bool {
265+
if cfg == nil {
266+
return false
267+
}
268+
269+
if (cfg.Enabled != nil && !*cfg.Enabled) ||
270+
cfg.ClusterID == "" ||
271+
cfg.Token == "" {
272+
return false
273+
}
274+
275+
return true
276+
}
277+
264278
func (cfg *TelemeterClientConfig) IsEnabled() bool {
265279
if cfg == nil {
266280
return false
@@ -407,9 +421,18 @@ func (c *Config) applyDefaults() {
407421
if c.ClusterMonitoringConfiguration.HTTPConfig == nil {
408422
c.ClusterMonitoringConfiguration.HTTPConfig = &HTTPConfig{}
409423
}
410-
if c.ClusterMonitoringConfiguration.TelemeterClientConfig == nil {
411-
c.ClusterMonitoringConfiguration.TelemeterClientConfig = &TelemeterClientConfig{
412-
TelemeterServerURL: "https://infogw.api.openshift.com/metrics/v1/receive",
424+
if c.ClusterMonitoringConfiguration.TelemetryConfig == nil {
425+
if c.ClusterMonitoringConfiguration.TelemeterClientConfig != nil {
426+
c.ClusterMonitoringConfiguration.TelemetryConfig = &TelemetryConfig{
427+
ClusterID: c.ClusterMonitoringConfiguration.TelemeterClientConfig.ClusterID,
428+
Enabled: c.ClusterMonitoringConfiguration.TelemeterClientConfig.Enabled,
429+
TelemeterServerURL: c.ClusterMonitoringConfiguration.TelemeterClientConfig.TelemeterServerURL,
430+
Token: c.ClusterMonitoringConfiguration.TelemeterClientConfig.Token,
431+
}
432+
} else {
433+
c.ClusterMonitoringConfiguration.TelemetryConfig = &TelemetryConfig{
434+
TelemeterServerURL: "https://infogw.api.openshift.com/metrics/v1/receive",
435+
}
413436
}
414437
}
415438

@@ -486,7 +509,7 @@ func (c *Config) SetRemoteWrite(rw bool) {
486509
}
487510

488511
func (c *Config) LoadClusterID(load func() (*configv1.ClusterVersion, error)) error {
489-
if c.ClusterMonitoringConfiguration.TelemeterClientConfig.ClusterID != "" {
512+
if c.ClusterMonitoringConfiguration.TelemetryConfig.ClusterID != "" {
490513
return nil
491514
}
492515

@@ -495,12 +518,12 @@ func (c *Config) LoadClusterID(load func() (*configv1.ClusterVersion, error)) er
495518
return fmt.Errorf("error loading cluster version: %w", err)
496519
}
497520

498-
c.ClusterMonitoringConfiguration.TelemeterClientConfig.ClusterID = string(cv.Spec.ClusterID)
521+
c.ClusterMonitoringConfiguration.TelemetryConfig.ClusterID = string(cv.Spec.ClusterID)
499522
return nil
500523
}
501524

502525
func (c *Config) LoadToken(load func() (*v1.Secret, error)) error {
503-
if c.ClusterMonitoringConfiguration.TelemeterClientConfig.Token != "" {
526+
if c.ClusterMonitoringConfiguration.TelemetryConfig.Token != "" {
504527
return nil
505528
}
506529

@@ -525,7 +548,7 @@ func (c *Config) LoadToken(load func() (*v1.Secret, error)) error {
525548
return fmt.Errorf("unmarshaling pull secret failed: %w", err)
526549
}
527550

528-
c.ClusterMonitoringConfiguration.TelemeterClientConfig.Token = ps.Auths.COC.Auth
551+
c.ClusterMonitoringConfiguration.TelemetryConfig.Token = ps.Auths.COC.Auth
529552
return nil
530553
}
531554

@@ -591,13 +614,23 @@ func (c *Config) Precheck() error {
591614
}
592615

593616
// Highlight deprecated config fields.
594-
var d float64
595-
if c.ClusterMonitoringConfiguration.K8sPrometheusAdapter != nil {
596-
klog.Infof("k8sPrometheusAdapter is a deprecated config use metricsServer instead")
597-
d = 1
617+
{
618+
var d float64
619+
if c.ClusterMonitoringConfiguration.K8sPrometheusAdapter != nil {
620+
klog.Infof("k8sPrometheusAdapter is a deprecated config use metricsServer instead")
621+
d = 1
622+
}
623+
// Prometheus-Adapter is replaced with Metrics Server by default from 4.16
624+
metrics.DeprecatedConfig.WithLabelValues("openshift-monitoring/cluster-monitoring-config", "k8sPrometheusAdapter", "4.16").Set(d)
625+
}
626+
{
627+
var d float64
628+
if c.ClusterMonitoringConfiguration.TelemeterClientConfig != nil {
629+
klog.Infof("telemeterClientConfig is a deprecated config use telemetryConfig instead")
630+
d = 1
631+
}
632+
metrics.DeprecatedConfig.WithLabelValues("openshift-monitoring/cluster-monitoring-config", "telemeterClientConfig", "4.21").Set(d)
598633
}
599-
// Prometheus-Adapter is replaced with Metrics Server by default from 4.16
600-
metrics.DeprecatedConfig.WithLabelValues("openshift-monitoring/cluster-monitoring-config", "k8sPrometheusAdapter", "4.16").Set(d)
601634

602635
return nil
603636
}

‎pkg/manifests/config_test.go‎

Lines changed: 132 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,9 @@ package manifests
1717
import (
1818
"context"
1919
"errors"
20+
"fmt"
2021
"os"
22+
"strings"
2123
"testing"
2224

2325
"github.com/openshift/cluster-monitoring-operator/pkg/metrics"
@@ -278,6 +280,105 @@ thanosRuler:
278280
}
279281
}
280282

283+
func TestTelemetryConfig(t *testing.T) {
284+
truev, falsev := true, false
285+
286+
tcs := []struct {
287+
enabled bool
288+
cfg *TelemetryConfig
289+
}{
290+
{
291+
cfg: nil,
292+
enabled: false,
293+
},
294+
{
295+
cfg: &TelemetryConfig{},
296+
enabled: false,
297+
},
298+
{
299+
cfg: &TelemetryConfig{
300+
Enabled: &truev,
301+
},
302+
enabled: false,
303+
},
304+
{
305+
cfg: &TelemetryConfig{
306+
Enabled: &falsev,
307+
},
308+
enabled: false,
309+
},
310+
{
311+
cfg: &TelemetryConfig{
312+
ClusterID: "test",
313+
},
314+
enabled: false,
315+
},
316+
{
317+
cfg: &TelemetryConfig{
318+
ClusterID: "test",
319+
Enabled: &falsev,
320+
},
321+
enabled: false,
322+
},
323+
{
324+
cfg: &TelemetryConfig{
325+
ClusterID: "test",
326+
Enabled: &truev,
327+
},
328+
enabled: false,
329+
},
330+
{
331+
cfg: &TelemetryConfig{
332+
Token: "test",
333+
},
334+
enabled: false,
335+
},
336+
{
337+
cfg: &TelemetryConfig{
338+
Token: "test",
339+
Enabled: &falsev,
340+
},
341+
enabled: false,
342+
},
343+
{
344+
cfg: &TelemetryConfig{
345+
Token: "test",
346+
Enabled: &truev,
347+
},
348+
enabled: false,
349+
},
350+
{
351+
cfg: &TelemetryConfig{
352+
ClusterID: "test",
353+
Token: "test",
354+
},
355+
enabled: true, // opt-in by default
356+
},
357+
{
358+
cfg: &TelemetryConfig{
359+
ClusterID: "test",
360+
Token: "test",
361+
Enabled: &truev,
362+
},
363+
enabled: true,
364+
},
365+
{
366+
cfg: &TelemetryConfig{
367+
ClusterID: "test",
368+
Token: "test",
369+
Enabled: &falsev, // explicitely opt-out
370+
},
371+
enabled: false,
372+
},
373+
}
374+
375+
for i, tc := range tcs {
376+
if got := tc.cfg.IsEnabled(); got != tc.enabled {
377+
t.Errorf("testcase %d: expected enabled %t, got %t", i, tc.enabled, got)
378+
}
379+
}
380+
}
381+
281382
func TestTelemeterClientConfig(t *testing.T) {
282383
truev, falsev := true, false
283384

@@ -724,9 +825,9 @@ func TestCollectionProfilePreCheck(t *testing.T) {
724825

725826
func TestDeprecatedConfig(t *testing.T) {
726827
for _, tc := range []struct {
727-
name string
728-
config string
729-
expectedMetricValue float64
828+
name string
829+
config string
830+
expected [][]interface{}
730831
}{
731832
{
732833
name: "setting a field in k8sPrometheusAdapter",
@@ -736,26 +837,48 @@ func TestDeprecatedConfig(t *testing.T) {
736837
cpu: 1m
737838
memory: 20Mi
738839
`,
739-
expectedMetricValue: 1,
840+
expected: [][]interface{}{
841+
{"openshift-monitoring/cluster-monitoring-config", "4.16", "k8sPrometheusAdapter", "1"},
842+
{"openshift-monitoring/cluster-monitoring-config", "4.21", "telemeterClientConfig", "0"},
843+
},
740844
},
741845
{
742846
name: "k8sPrometheusAdapter nil",
743847
config: `k8sPrometheusAdapter:
744848
`,
745-
expectedMetricValue: 0,
849+
expected: [][]interface{}{
850+
{"openshift-monitoring/cluster-monitoring-config", "4.16", "k8sPrometheusAdapter", "0"},
851+
{"openshift-monitoring/cluster-monitoring-config", "4.21", "telemeterClientConfig", "0"},
852+
},
746853
},
747854
{
748-
name: "no config set",
749-
config: "",
750-
expectedMetricValue: 0,
855+
name: "no config set",
856+
config: "",
857+
expected: [][]interface{}{
858+
{"openshift-monitoring/cluster-monitoring-config", "4.16", "k8sPrometheusAdapter", "0"},
859+
{"openshift-monitoring/cluster-monitoring-config", "4.21", "telemeterClientConfig", "0"},
860+
},
751861
},
752862
} {
753863
t.Run(tc.name, func(t *testing.T) {
754864
c, err := NewConfigFromString(tc.config, true)
755865
require.NoError(t, err)
756866
err = c.Precheck()
757867
require.NoError(t, err)
758-
require.Equal(t, tc.expectedMetricValue, prom_testutil.ToFloat64(metrics.DeprecatedConfig))
868+
meta := `
869+
# HELP cluster_monitoring_operator_deprecated_config_in_use [ALPHA] Set to 1 for deprecated configuration fields that are still in use, else 0.
870+
# TYPE cluster_monitoring_operator_deprecated_config_in_use gauge
871+
`
872+
metric := `
873+
cluster_monitoring_operator_deprecated_config_in_use {configmap="%s", deprecation_version = "%s", field = "%s"} %s
874+
`
875+
var b strings.Builder
876+
b.WriteString(meta)
877+
for _, e := range tc.expected {
878+
b.WriteString(fmt.Sprintf(metric, e...))
879+
}
880+
err = prom_testutil.CollectAndCompare(metrics.DeprecatedConfig, strings.NewReader(b.String()))
881+
require.NoError(t, err)
759882
})
760883
}
761884
}

0 commit comments

Comments
 (0)