Skip to content

Commit 5b98690

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 5b98690

File tree

7 files changed

+235
-192
lines changed

7 files changed

+235
-192
lines changed

‎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)