Skip to content

Commit c60d3bb

Browse files
bwplotkabwplotka
committed
fix: operator propagate correct compression for collection
Signed-off-by: bwplotka <bwplotka@google.com>
1 parent 37b7513 commit c60d3bb

File tree

3 files changed

+46
-25
lines changed

3 files changed

+46
-25
lines changed

‎e2e/collector_test.go‎

Lines changed: 35 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,8 @@ import (
2727
"github.com/GoogleCloudPlatform/prometheus-engine/e2e/kube"
2828
monitoringv1 "github.com/GoogleCloudPlatform/prometheus-engine/pkg/operator/apis/monitoring/v1"
2929
"github.com/google/go-cmp/cmp"
30-
promconfig "github.com/prometheus/prometheus/config"
3130
"google.golang.org/api/iterator"
3231
"google.golang.org/protobuf/types/known/timestamppb"
33-
"gopkg.in/yaml.v3"
3432
appsv1 "k8s.io/api/apps/v1"
3533
corev1 "k8s.io/api/core/v1"
3634
apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -217,44 +215,65 @@ func testCollectorOperatorConfig(ctx context.Context, kubeClient client.Client)
217215
if err := kubeClient.Get(ctx, client.ObjectKeyFromObject(&config), &config); err != nil {
218216
t.Fatalf("get operatorconfig: %s", err)
219217
}
218+
219+
// Test propagation of the custom options.
220220
config.Collection.Filter = monitoringv1.ExportFilters{
221221
MatchOneOf: []string{projectFilter, locationFilter, kubeletFilter},
222222
}
223+
config.Collection.Compression = monitoringv1.CompressionGzip
224+
config.Collection.ExternalLabels = map[string]string{
225+
"external_key": "external_val",
226+
}
223227

224-
// TODO(pintohutch): add external_labels.
225228
// Update OperatorConfig.
226229
if err := kubeClient.Update(ctx, &config); err != nil {
227230
t.Fatalf("update operatorconfig: %s", err)
228231
}
229232

233+
// Check if operator propagates collection options to enhanced fork Prometheus config.
234+
replace := func(s string) string {
235+
return strings.NewReplacer(
236+
"{projectID}", projectID,
237+
"{location}", location,
238+
"{cluster}", cluster,
239+
).Replace(s)
240+
}
241+
want := map[string]string{
242+
"config.yaml": replace(`global:
243+
external_labels:
244+
cluster: {cluster}
245+
external_key: external_val
246+
location: {location}
247+
project_id: {projectID}
248+
google_cloud:
249+
export:
250+
compression: gzip
251+
match:
252+
- '{project_id=''{projectID}''}'
253+
- '{location=~''{location}$''}'
254+
- '{job=''kubelet''}'
255+
`),
256+
}
257+
230258
var err error
231259
pollErr := wait.PollUntilContextCancel(ctx, pollDuration, false, func(ctx context.Context) (bool, error) {
232-
configMap := corev1.ConfigMap{
260+
cm := corev1.ConfigMap{
233261
ObjectMeta: metav1.ObjectMeta{
234262
Namespace: operator.DefaultOperatorNamespace,
235263
Name: operator.NameCollector,
236264
},
237265
}
238-
if err = kubeClient.Get(ctx, client.ObjectKeyFromObject(&configMap), &configMap); err != nil {
266+
if err = kubeClient.Get(ctx, client.ObjectKeyFromObject(&cm), &cm); err != nil {
239267
if apierrors.IsNotFound(err) {
240268
return false, nil
241269
}
242270
return false, fmt.Errorf("getting collector ConfigMap failed: %w", err)
243271
}
244272

245-
config := promconfig.Config{}
246-
data := configMap.Data["config.yaml"]
247-
if err = yaml.Unmarshal([]byte(data), &config); err != nil {
248-
return false, err
249-
}
250-
251-
// NOTE(bwplotka): Match logic will be removed in https://github.com/GoogleCloudPlatform/prometheus-engine/pull/1688
252-
// nolint:staticcheck
253-
if !cmp.Equal([]string{projectFilter, locationFilter, kubeletFilter}, config.GoogleCloud.Export.Match) {
254-
err = errors.New("unable to find export matchers")
273+
if diff := cmp.Diff(want, cm.Data); diff != "" {
274+
err = fmt.Errorf("unexpected collector config entry; diff: %s", diff)
255275
return false, nil
256276
}
257-
258277
return true, nil
259278
})
260279
if pollErr != nil {

‎pkg/operator/collection.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,7 @@ func setConfigMapData(cm *corev1.ConfigMap, c monitoringv1.CompressionType, key
268268
}
269269

270270
// ensureCollectorConfig generates the collector config and creates or updates it.
271-
func (r *collectionReconciler) ensureCollectorConfig(ctx context.Context, spec *monitoringv1.CollectionSpec, compression monitoringv1.CompressionType, exports []monitoringv1.ExportSpec) error {
271+
func (r *collectionReconciler) ensureCollectorConfig(ctx context.Context, spec *monitoringv1.CollectionSpec, configCompression monitoringv1.CompressionType, exports []monitoringv1.ExportSpec) error {
272272
cfg, updates, err := r.makeCollectorConfig(ctx, spec, exports)
273273
if err != nil {
274274
return fmt.Errorf("generate Prometheus config: %w", err)
@@ -277,8 +277,8 @@ func (r *collectionReconciler) ensureCollectorConfig(ctx context.Context, spec *
277277
// NOTE(bwplotka): Match logic will be removed in https://github.com/GoogleCloudPlatform/prometheus-engine/pull/1688
278278
// nolint:staticcheck
279279
cfg.GoogleCloud.Export.Match = spec.Filter.MatchOneOf
280-
if string(compression) != "" {
281-
cfg.GoogleCloud.Export.Compression = string(compression)
280+
if string(spec.Compression) != "" {
281+
cfg.GoogleCloud.Export.Compression = string(spec.Compression)
282282
}
283283
if spec.Credentials != nil {
284284
credentialsFile := path.Join(secretsDir, pathForSelector(r.opts.PublicNamespace, &monitoringv1.SecretOrConfigMap{Secret: spec.Credentials}))
@@ -296,7 +296,7 @@ func (r *collectionReconciler) ensureCollectorConfig(ctx context.Context, spec *
296296
Name: NameCollector,
297297
},
298298
}
299-
if err := setConfigMapData(cm, compression, configFilename, string(cfgEncoded)); err != nil {
299+
if err := setConfigMapData(cm, configCompression, configFilename, string(cfgEncoded)); err != nil {
300300
return err
301301
}
302302

‎pkg/operator/rules.go‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ func (r *rulesReconciler) Reconcile(ctx context.Context, req reconcile.Request)
119119
return reconcile.Result{}, fmt.Errorf("get operatorconfig for incoming: %q: %w", req.String(), err)
120120
}
121121

122-
var projectID, location, cluster = resolveLabels(r.opts.ProjectID, r.opts.Location, r.opts.Cluster, config.Rules.ExternalLabels)
122+
projectID, location, cluster := resolveLabels(r.opts.ProjectID, r.opts.Location, r.opts.Cluster, config.Rules.ExternalLabels)
123123

124124
if err := r.ensureRuleConfigs(ctx, projectID, location, cluster, config.Features.Config.Compression); err != nil {
125125
return reconcile.Result{}, fmt.Errorf("ensure rule configmaps: %w", err)
@@ -203,13 +203,15 @@ func hasRules(ctx context.Context, c client.Client) (bool, error) {
203203
}
204204
return len(rules.Items) > 0, nil
205205
}
206+
206207
func hasClusterRules(ctx context.Context, c client.Client) (bool, error) {
207208
var rules monitoringv1.ClusterRulesList
208209
if err := c.List(ctx, &rules); err != nil {
209210
return false, err
210211
}
211212
return len(rules.Items) > 0, nil
212213
}
214+
213215
func hasGlobalRules(ctx context.Context, c client.Client) (bool, error) {
214216
var rules monitoringv1.GlobalRulesList
215217
if err := c.List(ctx, &rules); err != nil {
@@ -219,7 +221,7 @@ func hasGlobalRules(ctx context.Context, c client.Client) (bool, error) {
219221
}
220222

221223
// ensureRuleConfigs updates the Prometheus Rules ConfigMap.
222-
func (r *rulesReconciler) ensureRuleConfigs(ctx context.Context, projectID, location, cluster string, compression monitoringv1.CompressionType) error {
224+
func (r *rulesReconciler) ensureRuleConfigs(ctx context.Context, projectID, location, cluster string, configCompression monitoringv1.CompressionType) error {
223225
logger, _ := logr.FromContext(ctx)
224226

225227
// Re-generate the configmap that's loaded by the rule-evaluator.
@@ -276,7 +278,7 @@ func (r *rulesReconciler) ensureRuleConfigs(ctx context.Context, projectID, loca
276278
continue
277279
}
278280
filename := fmt.Sprintf("rules__%s__%s.yaml", rs.Namespace, rs.Name)
279-
if err := setConfigMapData(cm, compression, filename, result); err != nil {
281+
if err := setConfigMapData(cm, configCompression, filename, result); err != nil {
280282
return err
281283
}
282284

@@ -306,7 +308,7 @@ func (r *rulesReconciler) ensureRuleConfigs(ctx context.Context, projectID, loca
306308
continue
307309
}
308310
filename := fmt.Sprintf("clusterrules__%s.yaml", rs.Name)
309-
if err := setConfigMapData(cm, compression, filename, result); err != nil {
311+
if err := setConfigMapData(cm, configCompression, filename, result); err != nil {
310312
return err
311313
}
312314

@@ -336,7 +338,7 @@ func (r *rulesReconciler) ensureRuleConfigs(ctx context.Context, projectID, loca
336338
continue
337339
}
338340
filename := fmt.Sprintf("globalrules__%s.yaml", rs.Name)
339-
if err := setConfigMapData(cm, compression, filename, result); err != nil {
341+
if err := setConfigMapData(cm, configCompression, filename, result); err != nil {
340342
return err
341343
}
342344

0 commit comments

Comments
 (0)