Skip to content

Commit 6ad8af4

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 8cb2808 commit 6ad8af4

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
@@ -293,6 +293,20 @@ type Audit struct {
293293
Profile auditv1.Level `json:"profile"`
294294
}
295295

296+
func (cfg *TelemetryConfig) IsEnabled() bool {
297+
if cfg == nil {
298+
return false
299+
}
300+
301+
if (cfg.Enabled != nil && !*cfg.Enabled) ||
302+
cfg.ClusterID == "" ||
303+
cfg.Token == "" {
304+
return false
305+
}
306+
307+
return true
308+
}
309+
296310
func (cfg *TelemeterClientConfig) IsEnabled() bool {
297311
if cfg == nil {
298312
return false
@@ -439,9 +453,18 @@ func (c *Config) applyDefaults() {
439453
if c.ClusterMonitoringConfiguration.HTTPConfig == nil {
440454
c.ClusterMonitoringConfiguration.HTTPConfig = &HTTPConfig{}
441455
}
442-
if c.ClusterMonitoringConfiguration.TelemeterClientConfig == nil {
443-
c.ClusterMonitoringConfiguration.TelemeterClientConfig = &TelemeterClientConfig{
444-
TelemeterServerURL: "https://infogw.api.openshift.com/metrics/v1/receive",
456+
if c.ClusterMonitoringConfiguration.TelemetryConfig == nil {
457+
if c.ClusterMonitoringConfiguration.TelemeterClientConfig != nil {
458+
c.ClusterMonitoringConfiguration.TelemetryConfig = &TelemetryConfig{
459+
ClusterID: c.ClusterMonitoringConfiguration.TelemeterClientConfig.ClusterID,
460+
Enabled: c.ClusterMonitoringConfiguration.TelemeterClientConfig.Enabled,
461+
TelemeterServerURL: c.ClusterMonitoringConfiguration.TelemeterClientConfig.TelemeterServerURL,
462+
Token: c.ClusterMonitoringConfiguration.TelemeterClientConfig.Token,
463+
}
464+
} else {
465+
c.ClusterMonitoringConfiguration.TelemetryConfig = &TelemetryConfig{
466+
TelemeterServerURL: "https://infogw.api.openshift.com/metrics/v1/receive",
467+
}
445468
}
446469
}
447470

@@ -518,7 +541,7 @@ func (c *Config) SetRemoteWrite(rw bool) {
518541
}
519542

520543
func (c *Config) LoadClusterID(load func() (*configv1.ClusterVersion, error)) error {
521-
if c.ClusterMonitoringConfiguration.TelemeterClientConfig.ClusterID != "" {
544+
if c.ClusterMonitoringConfiguration.TelemetryConfig.ClusterID != "" {
522545
return nil
523546
}
524547

@@ -527,12 +550,12 @@ func (c *Config) LoadClusterID(load func() (*configv1.ClusterVersion, error)) er
527550
return fmt.Errorf("error loading cluster version: %w", err)
528551
}
529552

530-
c.ClusterMonitoringConfiguration.TelemeterClientConfig.ClusterID = string(cv.Spec.ClusterID)
553+
c.ClusterMonitoringConfiguration.TelemetryConfig.ClusterID = string(cv.Spec.ClusterID)
531554
return nil
532555
}
533556

534557
func (c *Config) LoadToken(load func() (*v1.Secret, error)) error {
535-
if c.ClusterMonitoringConfiguration.TelemeterClientConfig.Token != "" {
558+
if c.ClusterMonitoringConfiguration.TelemetryConfig.Token != "" {
536559
return nil
537560
}
538561

@@ -557,7 +580,7 @@ func (c *Config) LoadToken(load func() (*v1.Secret, error)) error {
557580
return fmt.Errorf("unmarshaling pull secret failed: %w", err)
558581
}
559582

560-
c.ClusterMonitoringConfiguration.TelemeterClientConfig.Token = ps.Auths.COC.Auth
583+
c.ClusterMonitoringConfiguration.TelemetryConfig.Token = ps.Auths.COC.Auth
561584
return nil
562585
}
563586

@@ -636,13 +659,23 @@ func (c *Config) Precheck() error {
636659
}
637660

638661
// Highlight deprecated config fields.
639-
var d float64
640-
if c.ClusterMonitoringConfiguration.K8sPrometheusAdapter != nil {
641-
klog.Infof("k8sPrometheusAdapter is a deprecated config use metricsServer instead")
642-
d = 1
662+
{
663+
var d float64
664+
if c.ClusterMonitoringConfiguration.K8sPrometheusAdapter != nil {
665+
klog.Infof("k8sPrometheusAdapter is a deprecated config use metricsServer instead")
666+
d = 1
667+
}
668+
// Prometheus-Adapter is replaced with Metrics Server by default from 4.16
669+
metrics.DeprecatedConfig.WithLabelValues("openshift-monitoring/cluster-monitoring-config", "k8sPrometheusAdapter", "4.16").Set(d)
670+
}
671+
{
672+
var d float64
673+
if c.ClusterMonitoringConfiguration.TelemeterClientConfig != nil {
674+
klog.Infof("telemeterClientConfig is a deprecated config use telemetryConfig instead")
675+
d = 1
676+
}
677+
metrics.DeprecatedConfig.WithLabelValues("openshift-monitoring/cluster-monitoring-config", "telemeterClientConfig", "4.21").Set(d)
643678
}
644-
// Prometheus-Adapter is replaced with Metrics Server by default from 4.16
645-
metrics.DeprecatedConfig.WithLabelValues("openshift-monitoring/cluster-monitoring-config", "k8sPrometheusAdapter", "4.16").Set(d)
646679

647680
// TODO: remove after 4.19
648681
// Only to assist with the migration to Prometheus 3; fail early if Alertmanager v1 is still in use.

‎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"
@@ -357,6 +359,105 @@ prometheus:
357359
}
358360
}
359361

362+
func TestTelemetryConfig(t *testing.T) {
363+
truev, falsev := true, false
364+
365+
tcs := []struct {
366+
enabled bool
367+
cfg *TelemetryConfig
368+
}{
369+
{
370+
cfg: nil,
371+
enabled: false,
372+
},
373+
{
374+
cfg: &TelemetryConfig{},
375+
enabled: false,
376+
},
377+
{
378+
cfg: &TelemetryConfig{
379+
Enabled: &truev,
380+
},
381+
enabled: false,
382+
},
383+
{
384+
cfg: &TelemetryConfig{
385+
Enabled: &falsev,
386+
},
387+
enabled: false,
388+
},
389+
{
390+
cfg: &TelemetryConfig{
391+
ClusterID: "test",
392+
},
393+
enabled: false,
394+
},
395+
{
396+
cfg: &TelemetryConfig{
397+
ClusterID: "test",
398+
Enabled: &falsev,
399+
},
400+
enabled: false,
401+
},
402+
{
403+
cfg: &TelemetryConfig{
404+
ClusterID: "test",
405+
Enabled: &truev,
406+
},
407+
enabled: false,
408+
},
409+
{
410+
cfg: &TelemetryConfig{
411+
Token: "test",
412+
},
413+
enabled: false,
414+
},
415+
{
416+
cfg: &TelemetryConfig{
417+
Token: "test",
418+
Enabled: &falsev,
419+
},
420+
enabled: false,
421+
},
422+
{
423+
cfg: &TelemetryConfig{
424+
Token: "test",
425+
Enabled: &truev,
426+
},
427+
enabled: false,
428+
},
429+
{
430+
cfg: &TelemetryConfig{
431+
ClusterID: "test",
432+
Token: "test",
433+
},
434+
enabled: true, // opt-in by default
435+
},
436+
{
437+
cfg: &TelemetryConfig{
438+
ClusterID: "test",
439+
Token: "test",
440+
Enabled: &truev,
441+
},
442+
enabled: true,
443+
},
444+
{
445+
cfg: &TelemetryConfig{
446+
ClusterID: "test",
447+
Token: "test",
448+
Enabled: &falsev, // explicitely opt-out
449+
},
450+
enabled: false,
451+
},
452+
}
453+
454+
for i, tc := range tcs {
455+
if got := tc.cfg.IsEnabled(); got != tc.enabled {
456+
t.Errorf("testcase %d: expected enabled %t, got %t", i, tc.enabled, got)
457+
}
458+
}
459+
}
460+
360461
func TestTelemeterClientConfig(t *testing.T) {
361462
truev, falsev := true, false
362463

@@ -803,9 +904,9 @@ func TestCollectionProfilePreCheck(t *testing.T) {
803904

804905
func TestDeprecatedConfig(t *testing.T) {
805906
for _, tc := range []struct {
806-
name string
807-
config string
808-
expectedMetricValue float64
907+
name string
908+
config string
909+
expected [][]interface{}
809910
}{
810911
{
811912
name: "setting a field in k8sPrometheusAdapter",
@@ -815,26 +916,48 @@ func TestDeprecatedConfig(t *testing.T) {
815916
cpu: 1m
816917
memory: 20Mi
817918
`,
818-
expectedMetricValue: 1,
919+
expected: [][]interface{}{
920+
{"openshift-monitoring/cluster-monitoring-config", "4.16", "k8sPrometheusAdapter", "1"},
921+
{"openshift-monitoring/cluster-monitoring-config", "4.21", "telemeterClientConfig", "0"},
922+
},
819923
},
820924
{
821925
name: "k8sPrometheusAdapter nil",
822926
config: `k8sPrometheusAdapter:
823927
`,
824-
expectedMetricValue: 0,
928+
expected: [][]interface{}{
929+
{"openshift-monitoring/cluster-monitoring-config", "4.16", "k8sPrometheusAdapter", "0"},
930+
{"openshift-monitoring/cluster-monitoring-config", "4.21", "telemeterClientConfig", "0"},
931+
},
825932
},
826933
{
827-
name: "no config set",
828-
config: "",
829-
expectedMetricValue: 0,
934+
name: "no config set",
935+
config: "",
936+
expected: [][]interface{}{
937+
{"openshift-monitoring/cluster-monitoring-config", "4.16", "k8sPrometheusAdapter", "0"},
938+
{"openshift-monitoring/cluster-monitoring-config", "4.21", "telemeterClientConfig", "0"},
939+
},
830940
},
831941
} {
832942
t.Run(tc.name, func(t *testing.T) {
833943
c, err := NewConfigFromString(tc.config, true)
834944
require.NoError(t, err)
835945
err = c.Precheck()
836946
require.NoError(t, err)
837-
require.Equal(t, tc.expectedMetricValue, prom_testutil.ToFloat64(metrics.DeprecatedConfig))
947+
meta := `
948+
# HELP cluster_monitoring_operator_deprecated_config_in_use [ALPHA] Set to 1 for deprecated configuration fields that are still in use, else 0.
949+
# TYPE cluster_monitoring_operator_deprecated_config_in_use gauge
950+
`
951+
metric := `
952+
cluster_monitoring_operator_deprecated_config_in_use {configmap="%s", deprecation_version = "%s", field = "%s"} %s
953+
`
954+
var b strings.Builder
955+
b.WriteString(meta)
956+
for _, e := range tc.expected {
957+
b.WriteString(fmt.Sprintf(metric, e...))
958+
}
959+
err = prom_testutil.CollectAndCompare(metrics.DeprecatedConfig, strings.NewReader(b.String()))
960+
require.NoError(t, err)
838961
})
839962
}
840963
}

0 commit comments

Comments
 (0)