PO to GMP Migration Tool: Add Prometheus Operator Golden Tests - #2071
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive golden file testing framework in golden_test.go along with several golden test fixtures to validate the migration of Prometheus Operator configurations to Google Cloud Managed Service for Prometheus (GMP). The feedback primarily focuses on enhancing the robustness of the test harness by adding defensive nil checks to prevent potential nil pointer dereferences when processing relabel configurations and scrape configs. Additionally, it identifies an unused golden file, NamespaceSetCorrectlyForPodMonitor.golden, which should either be integrated into a test case or removed.
|
It looks like the main point of contention is about using expectedPO values to normalize the GMP Config and also whether some tests are necessary. I copied those values over so that all the tests copied over from upstream would pass against the golden files (just copied all tests over for completeness). A bunch of the upstream tests are testing stuff that GMP doesn't support (and in some cases intentionally won't support, like honorLabels / honorTimestamps). Do we want to keep these tests in golden_test.go? Now that I'm typing it out, having tests that pass vacuously without actually testing using tool output feels pretty useless. Just that if we ever do, we should copy those cases over from upstream. Should I just remove the unsupported tests from golden_test.go and only keep the ones for features that we currently support (like |
|
I would remove tests for things that we can't support. |
af4b3ee to
7c36d29
Compare
dashpole
left a comment
dashpole
left a comment
There was a problem hiding this comment.
Some suggestions for additional coverage. Could also be done as a follow-up if you'd rather.
| out = append(out, extractPhaseDropRule(gmpRelabelConfigs)...) | ||
|
|
||
| // 2. Port Keep and Metadata replacements. | ||
| // TODO: Add support for node, cluster, or namespace metadata label extraction if future upstream test fixtures |
There was a problem hiding this comment.
We could add a test case for attachMetadata.node here (similar to upstream TestNamespaceSetCorrectlyForPodMonitor) to verify node metadata extraction and cover resolveAttachMetadata.
|
|
||
| // extractCustomRelabelRules filters out GMP internal Monarch rules and preserves user-defined relabel configs. | ||
| // | ||
| // TODO: If future upstream test cases introduce custom pre-scrape relabelings that gmp-migrate converts into |
There was a problem hiding this comment.
Pre-scrape relabeling conversions (keep to selector, simple copy to fromPod) and metricRelabelConfigs currently have 0% golden test coverage. We should add test cases for those to verify relabel translation end-to-end.
| if err := runtime.DefaultUnstructuredConverter.FromUnstructured(output.Object, &gmpPM); err != nil { | ||
| t.Fatalf("FromUnstructured PodMonitoring error: %v", err) | ||
| } | ||
| // TODO: Inject mock Secret pool references into ScrapeConfigs() if future upstream test cases test |
There was a problem hiding this comment.
Passing a mock secret pool here would let us add golden tests for basicAuth, TLS, and authorization secret mappings.
| // 6. PodMonitor Test Suite. | ||
|
|
||
| // TestPodTargetLabelsFromPodMonitor mirrors upstream TestPodTargetLabelsFromPodMonitor in promcfg_test.go. | ||
| func TestPodTargetLabelsFromPodMonitor(t *testing.T) { |
There was a problem hiding this comment.
ClusterPodMonitoring (via namespaceSelector.any: true) is supported by testGoldenEquivalence, but there is no test exercising it yet. Adding a cluster-scoped test would cover buildClusterPodMonitoring.
| testGoldenEquivalence(t, "TargetLabels.golden", []runtime.Object{sm, svc}, "web") | ||
| } | ||
|
|
||
| // TestPodTargetLabels mirrors upstream TestPodTargetLabels in promcfg_test.go for ServiceMonitor. |
There was a problem hiding this comment.
We could add a test case with numeric or remapped targetPort on the Service to cover resolveServicePort mappings.
2 participants
This PR introduces a Golden Test Equivalence Suite (
pkg/migrate/golden_test.go) that assertsgmp-migrateoutputs against upstream golden fixtures from prometheus-operator/pkg/prometheus/promcfg_test.go.normalizeGMPScrapeConfigbridges known differences to verify that user-defined scraping, relabeling, metadata, and endpoint configs match upstream 1:1.Implemented Test Suite (4 Test Functions)
All tests evaluate input manifests against exact upstream golden fixtures
PodMonitor Golden Tests
TestPodTargetLabelsFromPodMonitor: Pod target label promotion (podTargetLabels: [example, env]).TestPodMonitorPhaseFilter: Pod phase dropping (filterRunning: false).ServiceMonitor Golden Tests
TestTargetLabels: Service target label extraction from backing Kubernetes Service objects.TestPodTargetLabels: Pod target labels defined onServiceMonitorresources.